current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Game Development Stack Exchange is a question and answer site for professional and independent game developers. It's 100% free, no registration required.
up vote 2 down vote favorite
1

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?

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

2 Answers 2

up vote 1 down vote accepted

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
}
share|improve this answer
up vote 0 down vote

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.

share|improve this answer
1  
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. –  user11177 Jan 16 at 2:08
    
@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. –  RandyGaul Jan 16 at 2:51
    
"It depends on what remove_entity does." - this is stated in the question. –  immibis Jan 16 at 3:21

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.