-1

I am working on a small game in SFML, I created a game class that will store various classes; for example, the player class.

I initialized the class on the heap with "new" keyword, but after hours of trying to figure out why my player class was going null after a certain scope. I decided not to initialize the game on the heap and after I did that, everything was fine.

Can someone please explain why this was happening. By the way, I am not creating the game on the heap anymore, I am showing the code I previously had when I was getting the null reference.

int main()
{
    sf::RenderWindow renderWindow(sf::VideoMode(640, 480), "Dungeon Run");

    //Asset test;
    //test.CreateAsset("GameAssets/Player.png");

    sf::Event event;
    renderWindow.setKeyRepeatEnabled(true);

    Game* game = new Game(renderWindow);
// Player initialized here.
    game->Initialize(); 
    while (renderWindow.isOpen()) 
    {
// Game->Player goes null in here.
        while (renderWindow.pollEvent(event))
        {

            if (event.type == sf::Event::EventType::Closed)
            {
                renderWindow.close();
            }
            //set window to random color to check if working
            if (event.type == sf::Event::EventType::KeyPressed) {
                if (sf::Keyboard::isKeyPressed(sf::Keyboard::A)) {
                }
                else if (sf::Keyboard::isKeyPressed(sf::Keyboard::W)) {
                }
                else if (sf::Keyboard::isKeyPressed(sf::Keyboard::S)) {
                }
                else if (sf::Keyboard::isKeyPressed(sf::Keyboard::D)) {
                }
                else if (sf::Keyboard::isKeyPressed(sf::Keyboard::Escape)) {
                    renderWindow.close();
                }
            }
            else if (event.type == sf::Event::EventType::MouseButtonPressed) {
                if (sf::Mouse::isButtonPressed(sf::Mouse::Left)) {
                }
                else if (sf::Mouse::isButtonPressed(sf::Mouse::Right)) {
                }
            }

            renderWindow.clear();
            game->Draw(); // Here player was null when I tried to draw the sprite for the player.
            renderWindow.display();
        }
    }

delete game;
game = nullptr;
}

Player class:

class Player
{
public:
    Player();
    Player(std::string name) 
        : name(name) 
    {
    }


    std::string name = "";
    double health = 100;
    double stamina = 100;
    int level = 1;
    Asset sprite;

    void attack();

    ~Player();
};


Asset class:

Asset::Asset() {

}

void Asset::CreateAsset(std::string path) {
    if (!texture.loadFromFile(path)) {
        std::cout << "Failed to load texture from file: " << path << std::endl;
    }

    sprite.setTexture(texture);
}

sf::Sprite& Asset::GetSprite() {
    return sprite;
}

Asset::~Asset() {

}

Game.h and Game.cpp:

class Game {
private:
    Player player;
    sf::RenderWindow* window;
public:
    Game(sf::RenderWindow& window);

    void Initialize();
    void Upadte();
    void Draw();

    ~Game();
};


Game::Game(sf::RenderWindow& window) {
    this->window = &window;
    player = Player("Player");
}

void Game::Initialize() {
    player.sprite.CreateAsset("GameAssets/player.png");
}

void Game::Upadte() {
}

void Game::Draw() {
    window->draw(player.sprite.GetSprite()); // Drawing his sprite
}

Game::~Game() {
}

trincot
  • 317,000
  • 35
  • 244
  • 286
KylaM26
  • 17
  • 5
  • 7
    Don't `new` unless you have a reason to. By default, you should declare variables with automatic storage duration. – Fureeish Jun 01 '19 at 21:53
  • what was in `game->Initialize()` at the time and what was in `game->Draw()` at the time? –  Jun 01 '19 at 22:01
  • 2
    Your citation of "here" is not relevant. If there's a variable called `player` that gets set to null you haven't shown it. You showed a call to `Game::Draw` by way of a a variable called `game`, which is presumably **not null**, right? You'll need to show relevant code so we can see how `player` may have gotten set to null. – Wyck Jun 01 '19 at 22:13
  • 1
    Possible duplicate of [Why should I use a pointer rather than the object itself?](https://stackoverflow.com/questions/22146094/why-should-i-use-a-pointer-rather-than-the-object-itself) – R2RT Jun 01 '19 at 22:18
  • game->Initialize() player.sprite.CreateAsset("GameAssets/player.png"); The player has a sprite object and the sprite object loads the png image. Inside of the game->Draw(); window->draw(player.sprite.GetSprite()); @Christoper – KylaM26 Jun 01 '19 at 22:22
  • @KylaM26 Could you show us the code for it? – Kostas Jun 01 '19 at 22:24
  • Consider adding assert()'s: 1) on every use of new, to check that the returned pointer is not nullptr, thus confirming you have not used up dynamic memory. 2) every use of delete, to confirm not a nullptr. On your next run, the assert should identify where to put your breakpoint. On the next run, set the breakpoint, and try to reproduce ... possibly identify the problem. – 2785528 Jun 01 '19 at 22:29
  • is player a member of game? still missing the connection – skeller Jun 01 '19 at 22:32
  • 1
    @KylaM26 Even though you are lucky to have few guys trying to help you, I think the most productive and fastest solution would be using debugger. You can set breakpoint in line which crashes and inspect state of game. – R2RT Jun 01 '19 at 22:35
  • I did that and I looked at the watch window to see when the player object was going null. @R2RT, and I do appreciate how helpful everyone one is. – KylaM26 Jun 01 '19 at 22:38
  • .h of game could help, how is "player" stored there? – skeller Jun 01 '19 at 22:38
  • The next step in debugging would be checking "why is it null" - e.g. adding breakpoints in all places where `player` pointer is written. – R2RT Jun 01 '19 at 22:40
  • @R2RT, I could not figure out "why it was null" and this why I am asking this question on stack overflow. I figured perhaps some other C++ developer would know. – KylaM26 Jun 01 '19 at 22:45
  • is one of the lines "renderWindow.close();" executed? maybe this lets your "draw" fail. maybe the problem is not the player but a draw on a closed window – skeller Jun 01 '19 at 22:47
  • When the escape key is pressed, I want the window to close. Is that what you are referring to @Skeller? – KylaM26 Jun 01 '19 at 22:48
  • but in any case, you should educate yourself on use of a debugger, this will help you a lot – skeller Jun 01 '19 at 22:49
  • Any good resources on debugger you can provide? @Skeller – KylaM26 Jun 01 '19 at 22:50
  • what IDE do you use? – skeller Jun 01 '19 at 22:51
  • Visual Studio 2017 – KylaM26 Jun 01 '19 at 22:52
  • then just set a breakpoint (grey area left of the line numer) and look at the "locals" next to the outout window when progam halts in the breakpoint, use F10, F11 and F5 to step though your code (see debug menu) – skeller Jun 01 '19 at 22:55
  • What exactly does "*Game->Player goes null in here.*" mean? Since `Player` is a class and `player` is an object (not a pointer), what exactly is null? – David Schwartz Jun 01 '19 at 22:59
  • By null, I mean the player goes out of scope and when I try to call a method or member the program crash. Sorry about the confusion. – KylaM26 Jun 01 '19 at 23:05
  • 1
    @KylaM26 -- Better than setting a breakpoint, set a *watchpoint* in the debugger and the condition. What that will do is run your program until the condition occurs, in this case, when your pointer becomes null. I'm surprised no one mentioned this so far. The Visual C++ debugger has these capabilities -- maybe it isn't used that often, but it becomes a life-saver if you ever want to know why a variable has changed values. There is no need to do an exhaustive search for why a value changes -- let the debugger stop when it changes. – PaulMcKenzie Jun 01 '19 at 23:48
  • 1
    "When to use the `new` keyword?" My C++ project at work has over 30 million lines of code. I've been working on it for two years. I have yet had a need to use `new`, `new[]` or `malloc`. Such code does exist in the code base, but only in very limited circumstances: a `std::unique_ptr` workalike (which predated C++11, and we're migrating to `std::unique_ptr`, and a reference counting `std::shared_ptr` workalike (again, we're migrating to `std::shared_ptr`). – Eljay Jun 02 '19 at 01:22

1 Answers1

4

in modern c++ you almost never need the new keyword

if you want to create a object on the heap, use a unique_ptr or a shared_ptr

you create your class with

std::unique_ptr<Game> game = std::make_unique<Game>(renderWindow);

this way you do not need a delete as the unique_ptr will take care of that for you when it runs out of scope.

so just replace your new line with the above and remove the line with delete

you may need the following include to use std::unique_ptr

#include <memory>

as for the "why" you had a problem: The code responsible is not posted yet, but i'd assume you have another place where game is deleted or set to nullptr

skeller
  • 1,151
  • 6
  • 6
  • 1
    That's not an answer. True, that would fix it, but the OP explicitly said they weren't doing it this way anymore. They want to know **why** a pointer was getting corrupted. –  Jun 01 '19 at 21:59
  • 2
    the OP also wanted to know when to use the new keyword as the question title implies – skeller Jun 01 '19 at 22:02
  • Okay, fair enough. I'm considering retracting my down-vote because that's a good point. Maybe the title and content are asking two different things. –  Jun 01 '19 at 22:04
  • Retracted my down-vote. Not sure I'm a +1 yet, I'll wait to see what happens if the OP posts more code. –  Jun 01 '19 at 22:15
  • 1
    don't worry about the +-, i do not care about votes on answer as long as it helps the OP – skeller Jun 01 '19 at 22:16