alecthomas / alecthomas/entityx
Assertion failure when removing a component while an entity is being destroyed
- Langage dominant
- C++
- Étoiles
- 2.3k
- Forks
- 301
- Métriques de merge des PR
- Aucune PR mergée en 30 j
Description
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(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(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.
Guide de contribution
Aucun guide de contribution indexé pour ce dépôt
Évaluation
Cette issue n'a pas encore été évaluée.