alecthomas / alecthomas/entityx

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

Open
#224 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
2.3k
Forks
301
PR merge metrics
No merged PRs in 30d

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.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.