Ralith / hecs

A handy ECS
Apache License 2.0
979 stars 84 forks source link

Empty archetypes #66

Open Ratysz opened 4 years ago

Ratysz commented 4 years ago

Despawning all entities of an archetype doesn't remove that archetype from the world, and doesn't advance archetype generation.

This is problematic for something like my library yaks, because it causes tests like this one to fail - summarized, it:

This means that as soon as a system-straddling entity is spawned those systems are forbidden from running concurrently until the world is recreated.

I see these approaches so far:

The problem with last approach is obvious - the archetypes don't actually change - but there needs to be some mechanism of informing user code.

Ralith commented 4 years ago

It's intentional that the archetype sticks around; my thinking that if a certain combination of components existed once, it'll probably exist again, and this way we save some allocator pressure. That said, I agree that empty archetypes forcing serialization of queries is a bit silly.

I'm not opposed to an explicit method to drop empty archetypes to recover memory, but I don't think it's a good solution to this specific problem due to the performance consequences described above.

I think the way forward here is to provide an interface that behaves as if empty archetypes do not exist, even though their storage is retained. This should actually be pretty easy: slap a filter on to archetypes (dropping ExactSize in the process unless there's really a need for it) and take care to skip empty archetypes in dynamic borrow checking and to increment the generation when an archetype becomes empty.

Ratysz commented 4 years ago

Well, this is how my usage of current API looks like - so, yes, adding a filter is an obvious idea, hence why I suggested exposing Archetype::is_empty(). Ditching of ExactSize is not that big of an issue either; it's the archetype generation that I'm worried about.

Ralith commented 4 years ago

Updating the generation when an archetype becomes empty should handle that fine. Doing the filtering internally avoids expanding the API, which is desirable.