Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add way to hook into entity despawns #2031

Closed
ndarilek opened this issue Apr 27, 2021 · 13 comments
Closed

Add way to hook into entity despawns #2031

ndarilek opened this issue Apr 27, 2021 · 13 comments
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged

Comments

@ndarilek
Copy link
Contributor

What problem does this solve or what need does it fill?

Sometimes, to speed up a system, I cache data in something like a Local<HashMap<Entity, _>>, and avoid heavier calculations if the cache hasn't changed. Much of this state wouldn't make sense as a separate component, and it feels cleaner to do it as a Local. Unfortunately, there's no apparent way to detect if an entity despawns and clean up these local caches.

What solution would you like?

Maybe some sort of entity despawn event. Maybe, for symmetry, it could be like:

enum EntityLifecycle {
    Spawned(Entity),
    Despawned(Entity),
    DespawnedRecursive(Entity), // Or not
}

What alternative(s) have you considered?

I could check whether components used to calculate the cache were removed, but then I'd need to either rerun the system in post-update, or only have a single copy there. That isn't always possible.

I could also externalize the cache, but that feels dirty as it pollutes the module namespace with a) state only relevant to one system and b) a secondary system to clean that state up.

Finally, I could create my own events, but this requires me to manually throw those events whenever I despawn. Given that a) this seems like a general problem and b) Bevy itself knows when it is despawning something, it probably makes more sense for the engine to notify that the entity was despawned.

@ndarilek ndarilek added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Apr 27, 2021
@Moxinilian Moxinilian added A-ECS Entities, components, systems, and events and removed S-Needs-Triage This issue needs to be labelled labels Apr 27, 2021
@Moxinilian
Copy link
Member

Have you considered accepting that the cache will only be cleared on the next frame, and thus at the beginning of your system querying components that were removed to clear the cache when necessary?

@ndarilek
Copy link
Contributor Author

ndarilek commented Apr 27, 2021 via email

@ndarilek
Copy link
Contributor Author

ndarilek commented Apr 27, 2021 via email

@Moxinilian
Copy link
Member

I am glad this can be a good workaround. Could you elaborate on how iterating over those despawn events would differ from iterating over removed components?

@ndarilek
Copy link
Contributor Author

ndarilek commented Apr 27, 2021 via email

@Moxinilian
Copy link
Member

My apologies, I was not aware of the post-update restriction. My suggested solution thus does not apply.

@ndarilek
Copy link
Contributor Author

ndarilek commented Apr 27, 2021 via email

@cart
Copy link
Member

cart commented Apr 28, 2021

My understanding is that iterating over removed components can only be
done in post-update now.

This is true, not because there is something special about PostUpdate, but because Commands are applied at the end of Update. If we were to add a new "entity despawn" event, it would still only be available in PostUpdate because commands.despawn(entity) wouldn't be "applied" until the the end of Update.

Some folks are working to make Command application more granular within a stage (we're calling it "stageless" right now), which might solve this problem.

But in the short term, the only way to make "despawns" immediate is to use "exclusive systems" (systems that take a direct &mut World reference and run before normal systems in a stage, after normal systems in a stage and before their commands are applied, or after normal systems in a stage and after their commands are applied).

"Entity Despawn Events"

I do think that this is good thing to add, even though it would have the same limitations the "component removal tracking" has in relationship to Commands. A full entity despawn is semantically different than specific component removals and I imagine there are plenty of legitimate use cases for it. The design for this should probably go through the RFC process as there are a number of ways to implement it (and a number of ways to expose it to the user).

@mockersf
Copy link
Member

If we were to add a new "entity despawn" event, it would still only be available in PostUpdate

There is a big limitation of RemovedComponents that an event wouldn't have: the RemovedComponents is cleared at the end of the frame while the event can be read after.

So yay for EntityLifecycleEvent!

@ndarilek
Copy link
Contributor Author

ndarilek commented Apr 28, 2021 via email

@cart
Copy link
Member

cart commented Apr 28, 2021

Yeah Events are probably the right call. Something to consider for the design: a global EntityLifecycleEvent for all entities might end up being expensive to consume in some cases because you need to iterate over all entity events across all contexts. Sometimes considering them globally will be necessary, but it would be interesting to consider ways to "filter down" events, maybe by storing them per-archetype? Could they be queryable?

@alice-i-cecile alice-i-cecile added the S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged label Dec 12, 2021
@TimJentzsch
Copy link
Contributor

I think this can be closed via #10756, right?

@james7132
Copy link
Member

This probably can be considered closed with component hooks as a MVP. I'd like to see easier access to the inserted or removed components to make this a bit cleaner to use though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged
Projects
None yet
Development

No branches or pull requests

7 participants