Assertion failure when removing a component while an entity is being destroyed

[dolphineye]

Hi,

I noticed that when you are destroying an entity, if you attempt to remove another component from that entity at the same time (e.g: from a component's destructor), you will get an assertion failure in Entity::remove()

As far as I could investigate, the issue is located in EntityManager::destroy(Entity::Id entity).

Old code:

  void destroy(Entity::Id entity) {
    assert_valid(entity);
    uint32_t index = entity.index();
    auto mask = entity_component_mask_[index];
    for (size_t i = 0; i < component_helpers_.size(); i++) {
      BaseComponentHelper *helper = component_helpers_[i];
      if (helper && mask.test(i))
        helper->remove_component(Entity(this, entity));
    }
    event_manager_.emit<EntityDestroyedEvent>(Entity(this, entity));
    entity_component_mask_[index].reset();
    entity_version_[index]++;
    free_list_.push_back(index);
  }

New code that fixes the issue:

  void destroy(Entity::Id entity) {
    assert_valid(entity);
    uint32_t index = entity.index();
    for (size_t i = 0; i < component_helpers_.size(); i++) {
      BaseComponentHelper *helper = component_helpers_[i];
      if (helper && entity_component_mask_[index].test(i))
        helper->remove_component(Entity(this, entity));
    }
    event_manager_.emit<EntityDestroyedEvent>(Entity(this, entity));
    entity_component_mask_[index].reset();
    entity_version_[index]++;
    free_list_.push_back(index);
  }

The issue is caused by the fact mask is being cached before the for loop. As a result, if there is any attempt to remove a component in an other component's destructor, the so called component will be removed twice, hence causing the assertion failure.

Since I'm not in a situation where I can commit anything and/or submit a PR for a few days, I just wanted to let you know about the issue and the fix ;)

Best regards.