2
\$\begingroup\$

My game uses an entity manager and entities to represent everything including the player, enemes, items, ..everything in the game. In my engine it's the responsibility of every entity to update itself.

Sometimes an entity finds out it has expired somehow, and thus needs to remove itself from the entity manager. My question is: Is it safe to have an object delete itself? I will illustrate what I mean.

void EntityManager::remove_entity(Entity* entity) {
    m_entities.erase(std::remove_if( //...
}

void Entity::update() {
    if (has_expired)
        m_manager.remove_entity(this); // <- safe?
}

I can't concretely prove a bug by using this, but it kind of looks and feels like it should at least be undefined behaviour. So is it OK? If not, what is a common practice for dealing with this situation?

\$\endgroup\$
4
  • \$\begingroup\$ Yes, I believe it's safe. Usually in this situation you'd leave freed pointers somewhere, though. \$\endgroup\$
    – Ben
    Jan 16, 2015 at 1:36
  • \$\begingroup\$ @Ben What do you mean exactly? \$\endgroup\$ Jan 16, 2015 at 1:44
  • \$\begingroup\$ What type is m_entities? Is Entity::update called while iterating over m_entities? \$\endgroup\$
    – user253751
    Jan 16, 2015 at 3:20
  • \$\begingroup\$ It's a vector of std::shared_ptr<Entity>. Yes, it is call when iterating m_entities. \$\endgroup\$ Jan 16, 2015 at 11:06

2 Answers 2

1
\$\begingroup\$

You could create a pool at the manager which will clean up all after or before the EntityManager::update. This way it is save. using 'delete this' will raise big issues amongst a lot of developers.

std::vector m_garbage;
void EntityManager::collect(Entity* entity) {
    entity->collected = true;
    garbage.push_back(entity);
}
void EntityManager::update(..) {
    // erase all in m_garbage
}
\$\endgroup\$
0
\$\begingroup\$

It depends on what remove_entity does. If you're freeing some memory then you shouldn't access the this pointer after calling remove_entity. If you're just marking the entity as "to be deleted later", then you can do whatever you want with it.

Usually what happens is entities add themselves to a buffer, and at the end of the game's update loop the manager loops over the buffer and deletes all the entities. This would be called delayed destruction, and is usually the appropriate way to do this sort of thing.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ remove_entity compares the argument to a vector of smart pointers and if a match is found it erases it, freeing the memory. I do not try to use the this pointer after remove entity, but I still just want to know whether the operation is 100% safe, and that I haven't just been getting lucky with undefined behaviour. \$\endgroup\$ Jan 16, 2015 at 2:08
  • \$\begingroup\$ @user11177 Yeah of course it's safe, so long as you don't do anything with that freed memory. You can think of the class method as a normal function with an implicitly defined parameter for the this pointer. When you free the entity the this pointer will be pointing to freed memory. If you don't access the this pointer after a free, you're good to go :). But of course, just because it's "safe" may not mean it's a good idea. Whether or not it's a good idea is a different question. \$\endgroup\$
    – RandyGaul
    Jan 16, 2015 at 2:51
  • \$\begingroup\$ "It depends on what remove_entity does." - this is stated in the question. \$\endgroup\$
    – user253751
    Jan 16, 2015 at 3:21

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .