Destroying object from object method

Question

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?

Answer 1

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
}

Answer 2

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.