Reputation: 321
In a basic game code that I'm developing, I have each separate area of the "map" have a separate Area class that handles combat and a variety of other basic functions. Each entity contained within the Area has a pointer to that Area so it can access Area functions and whatnot, however that pointer is apparently getting deleted or something wonky is happening, because when it tries to dereference it, I get all sorts of run-time errors.
Is it happening because I'm giving each NPC a pointer to the Area in its constructor?
Area Constructor
Area::Area(Game& game, std::string name, std::string id)
: GameState(game), game(game), player(game.getPlayer()), log(game, "log", 0.0f, 90.0f, 32, game.getFont("Times"), sf::Color::White)
{
game.clearStrings();
this->id = id;
this->name = name;
game.timer.restart();
player.setArea(this);
game.addString(DisplayString(&game, name, name + "title", game.getFont("Celt"), 10.0f, 10.0f, 64, sf::Color::White));
//
this->addEnemy(NPC(game, this, "Skeleton", "skel", 30, 0, 7, 10, 5, 5));
this->addEnemy(NPC(game, this, "Skeleton", "skel", 30, 0, 7, 10, 5, 5));
int offsetY = 0;
for (std::map<std::string, NPC>::iterator it = enemies.begin(); it != enemies.end(); ++it)
{
game.addString(DisplayString(&game, it->second.getName(), it->first, game.getFont("Times"), WINDOW_WIDTH, 0, 64, sf::Color::White));
DisplayString& name = game.getString(it->second.getID());
game.addString(DisplayString(&game, std::to_string(it->second.hp) + " / " + std::to_string(it->second.maxhp), it->second.getID() + "hp", game.getFont("Times"), WINDOW_WIDTH, name.Text.getGlobalBounds().height, 64, sf::Color::White));
name.setPos(WINDOW_WIDTH - name.Text.getGlobalBounds().width, offsetY);
offsetY += name.Text.getGlobalBounds().height;
DisplayString& hp = game.getString(it->second.getID() + "hp");
hp.setPos(WINDOW_WIDTH - hp.Text.getGlobalBounds().width, offsetY+name.Text.getGlobalBounds().height);
offsetY += hp.Text.getGlobalBounds().height;
}
}
NPC Constructor
NPC::NPC(Game& game, Area* area, std::string name, std::string id, int hp, int ap, int agil, int str, int dmg, int speed)
: game(game)
{
this->name = name;
this->hp = hp;
this->maxhp = hp;
this->ap = ap;
this->dmg = dmg;
this->agil = agil;
this->str = str;
this->speed = speed;
this->area = area;
this->id = id + std::to_string(game.enemyID++);
}
This is where I run into troubles dereferencing the area pointer of the NPC class
void NPC::attack(Player& player)
{
if (rand() % 100 + 1 < 5)
{
int damage = 2 * (this->dmg + (rand() % 6 - 3));
player.hp -= damage;
//any of these area pointers in the debugger show up with garbage data
area->log.addLine(this->name + " critically strikes " + player.getName() + " for " + std::to_string(damage) + "!");
}
else
{
if (rand() % 100 + 1 > 15)
{
int damage = this->dmg + (rand() % 6 - 3);
player.hp -= damage;
area->log.addLine(this->name + " hits " + player.getName() + " for " + std::to_string(damage) + "!");
}
else
{
area->log.addLine(player.getName() + " dodges!");
}
}
}
Keep in mind that there's no shenanigans going on from the Area constructor, to the NPC constructor, to the Attack function. The Area constructor completes, then the game calls the Area's update function, which immediately calls Attack, and nowhere in between is the area pointer being deleted or in any other way modified.
I apologize for the incredibly verbose answer and for kinda dumping a bunch of code at you, but I'm new at all the stuff and really didn't know what to cut out to condense it. If you need any more code to get a better idea of what's going on, I will gladly provide it, I've just banged my head at this for days trying to sort it out with no headway made.
Edit: adding some code that leads to the construction of Area (cut out extraneous code)
void InitState::update()
{
if(main.isOpen())
{
sf::Event event;
while (main.pollEvent(event))
{
switch (event.type)
{
case sf::Event::Closed:
main.close();
game.Stop();
break;
case sf::Event::KeyReleased:
switch (event.key.code)
{
break;
case sf::Keyboard::Key::Return:
switch (var)
{
case NAME:
if (writing)
{
game.delIcon("inputbox");
player.setName(game.getString("input").Text.getString());
game.addArea(Area(game, "The Cave", "cave"));
game.setState(&game.getArea("cave"));
}
default:
break;
}
break;
}
break;
}
}
}
InitState is a GameState, which Area is as well. When the game loops, it calls state->update() and render(), so when the game sets a state, it switches over to that class's update and render functions.
Area's update function
void Area::update()
{
if (game.getWindow().isOpen())
{
sf::Event event;
while (game.getWindow().pollEvent(event))
{
if (event.type == sf::Event::Closed)
{
game.getWindow().close();
game.Stop();
}
if (event.type == sf::Event::KeyReleased)
{
if (event.key.code == sf::Keyboard::Key::Return)
{
doCombat(); // calls attack functions for NPC and player
}
}
}
}
}
EDIT EDIT: More code! areas is an std::map
void Game::addArea(Area area)
{
areas.emplace(area.getID(), area);
}
Area& Game::getArea(std::string id)
{
return areas.find(id)->second;
}
Upvotes: 2
Views: 2879
Reputation: 7157
You create an instance of your Area class on the stack:
In void InitState::update()
:
//...
game.addArea(Area(game, "The Cave", "cave"));
//...
Then your Game::addArea() creates a copy of it.
But you probably have not defined a copy constructor for class Area
, which handles all the gory details. So, the (automatically created) copy of your area object probably contains pointers pointing to objects managed by the original area instance, which was deleted exactly after the above line.
There are some different strategies to solve your problem: a simple one: use heap allocation (new Area(..)
instead of just Area(..)
) everywhere, and use smart pointers where applicable.
You should read about C++ heap and stack allocation, automatic memory, references, especially const references, and so on.
Maybe this link is an introduction for you: http://www.learncpp.com/cpp-tutorial/79-the-stack-and-the-heap/
EDIT: By the way, it is good style to mark your objects "non-copyable", whenever you don't want to or cannot provide a copy constructor. Or if you just want to disallow copies of them. Non-copyable Mixin
Upvotes: 2
Reputation: 3102
This is just a guess, but it may be possible that the call to "game.timer.restart();" before the call to "player.setArea(this);" is the problem. Perhaps this is causing something to be called that is de-referencing the variable. Try moving the "player.setArea(this);" line to before the "game.timer.restart();" line. Or if restart clears that variable, separate the restart operation into a stop and a start operation as follows.
game.timer.stop();
player.setArea(this);
game.timer.start();
I hope this helps.
Upvotes: 0