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

Access data from removed components #1655

Closed
alice-i-cecile opened this issue Mar 14, 2021 · 8 comments
Closed

Access data from removed components #1655

alice-i-cecile opened this issue Mar 14, 2021 · 8 comments
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Mar 14, 2021

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

When removing components (or despawning entities with those components), we often want to perform cleanup. Unfortunately, this work typically requires data from that removed component, rather than just the entity that was returned. See our example for the current behavior.

What solution would you like?

Store the Removed data in a special "purgatory" sparse set that we keep around until the frame-after-next, using the same double-buffer strategy as Events.

What alternative(s) have you considered?

Write custom commands to remove components that require cleanup, like despawn_recursive. This is hard to extend though and forces the user to not clean stuff up.

Always emit a "to despawn" event, and then clean things up before the data is lost by making sure that the cleanup systems run first.

Users manually archive and then access the components that they need to access once removed.

Additional context

Relevant to how we despawn entities in a parent-child hierarchy, e.g. despawn_recursive.

We will need to do the same thing with Resources as well.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Mar 14, 2021
@alice-i-cecile
Copy link
Member Author

Thought: rather than moving the data at all, we could use a Removed flag, ala change detection. By default, entities with this flag are always excluded from queries. You can access them by including Removed<T> in your query filter.

Then, only actually remove the data at the end of the frame after they were removed, with no copying required. Build off of #1471.

@cart
Copy link
Member

cart commented Mar 14, 2021

I think storing removed components in a SparseSet is good if "removal tracking" is opt-in, but it seems overly expensive as a default.

An option that has zero cost(*) that I've proposed previously: both storage types do a "swap remove", which leaves removed components packed at the end of the component storage vector (ex: len <= index < capacity). Provided new values aren't written to that space (ex a new component is added) and we track the number of removed components, we can assume that these indices contain the removed component values.

For correctness, we need to ensure that components removed directly (ex: let x = world.remove::<T>(entity).unwrap()) are not exposed in this way (which will complicate state management).

(*) this is not actually zero cost in all cases. it opens up the doors to "allocation garbage" if components are removed, then other components are spawned in the same frame. We can't reuse the existing capacity (which is occupied by the removed components), so we need to allocate new space, then do a "swap insert" to ensure "active" components are packed at the front of the vec.

Thought: rather than moving the data at all, we could use a Removed flag, ala change detection. By default, entities with this flag are always excluded from queries. You can access them by including Removed in your query filter.

This is reasonable, but it means that all queries need to incur the cost of per-entity "filters" to check for the removed flag. I'm assuming Unity pays this cost as they use flags to temporarily remove components (which means there is precedent in a widely used ECS), but I'm still a little hesitant to do it.

@alice-i-cecile
Copy link
Member Author

alice-i-cecile commented Mar 14, 2021

This is reasonable, but it means that all queries need to incur the cost of per-entity "filters" to check for the removed flag. I'm assuming Unity pays this cost as they use flags to temporarily remove components (which means there is precedent in a widely used ECS), but I'm still a little hesitant to do it.

Does query caching save us here? My intuition is that in most cases this is a one time cost or a twice-per removal cost.

I'm assuming Unity pays this cost as they use flags to temporarily remove components

@BoxyUwU was telling me that this feature was also present in Shipyard, and very useful there. It looks like they return the removed component if it existed; which isn't a great choice for our model.

@cart
Copy link
Member

cart commented Mar 14, 2021

Does query caching save us here? My intuition is that in most cases this is a one time cost or a twice-per removal cost.

Nope. We cache archetype and table matches, not entity matches (which would be too expensive to justify)

@BoxyUwU was telling me that this feature was also present in Shipyard, and very useful there. It looks like they return the removed component if it existed; which isn't a great choice for our model.

It looks like that api does "actual removes" just like we do:
https://docs.rs/shipyard/0.4.1/src/shipyard/remove.rs.html#140
https://github.com/leudz/shipyard/blob/cd208b6cba3395986b4a9948eab5ab21f9cfe24a/src/sparse_set/mod.rs#L219

@BoxyUwU
Copy link
Member

BoxyUwU commented Mar 14, 2021

Shipyard does provide this functionality just not through regular removes. In shipyard when you remove a component it returns it immediately. But you can also delete a component to not have it returned immediately and then later iterate over deleted components. Components on despawned entities are considered "deleted". I believe in shipyard deletion tracking is opt-in but this would all be fine for the use-case I need this for.

Edit: Bevy doesn't have instant removes so really every remove component operation can be considered a delete

@alice-i-cecile
Copy link
Member Author

Opt-in removal checking, as discussed above, is much more feasible to implement with #2254.

bors bot pushed a commit that referenced this issue Jan 5, 2022
# Objective

- Fixes #1920.
- Users often want to know how to get the values of removed components (#1655).
- Stand-alone `bevy_ecs` behavior is very unintuitive, as `World::clear_trackers()` must be manually called.
- Fixes #2999 by extending the existing test (thanks @hymm for pointing me to it) to be clearer and check for component removal as well.

## Solution

- Better docs!
- Better tests!
@ItsDoot
Copy link
Contributor

ItsDoot commented Dec 14, 2023

Solved when #10756 is merged.

@james7132
Copy link
Member

#10756 has been merged and component hooks do provide a way to access the data in a removed component before it's removed from the World.

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 C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

No branches or pull requests

5 participants