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

Moving components from one entity to another #15350

Closed
urben1680 opened this issue Sep 21, 2024 · 2 comments · Fixed by #16826
Closed

Moving components from one entity to another #15350

urben1680 opened this issue Sep 21, 2024 · 2 comments · Fixed by #16826
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 D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Design This issue requires design work to think about how it would best be accomplished

Comments

@urben1680
Copy link
Contributor

urben1680 commented Sep 21, 2024

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

For my reversible systems crate I want to support reversible entity commands that do structural changes.

For example, to create a reversible variant of EntityCommands::insert, that takes a Bundle as an argument, I first have to move the components that would be overwritten out of the entity so I can make this process undoable.

BundleInfo has the methods I need to get the relevant ids including required components. However, there is no further API from here on.

What solution would you like?

What would work is to move components from one entity to another by id(s). A typed variant should be added there too because it does not exist yet. This is an elegant solution because the data structure of storing untyped components is already there: The ECS storages. This might be useful for other use cases as well.

// in EntityWorldMut and EntityCommands impl

pub fn move<T: Bundle>(&mut self, to: Entity) -> &mut Self;

pub fn move_by_id(&mut self, to: Entity, component_id: ComponentId) -> &mut Self;

pub fn move_by_ids(
    &mut self, 
    to: Entity,
    component_ids: impl IntoIterator<Item = ComponentId>
) -> &mut Self;

Each variant should also come with a _with_requires variant like #15026 that has the difference to not construct required components at the target entity but to move them from the source entity as well.

Hooks should be triggered by these operations.

Panics should happen if either the source or target entity is missing or at least one ComponentId is invalid.

Open questions

  • Should this come with variants like try_insert that fail silently?

What alternative(s) have you considered?

An alternative is to move the components entirely out of the ECS storage as some kind of blob. However, no such type exists yet as far I can tell. It would require extra work to design this blob, also because it needs to support generating OwningPtr for reinsertion.

I see no alternative in my untyped context. I could mirror the Bundle trait to create an associated function but that would not work with third-party Bundle implementors that do not implement my trait.

Until a feature like this comes out I consider to require only tuple bundles for my crate's API with further restrictions on the upcoming required components feature. This probably makes the API very panic-happy. 🫤

I see reflection not helping me here as not every component can be reflected.

@urben1680 urben1680 added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Sep 21, 2024
@urben1680 urben1680 changed the title Feature request: EntityWorldMut::take_by_ids(s) Feature request: EntityWorldMut::take_by_id(s) Sep 21, 2024
@SkiFire13
Copy link
Contributor

SkiFire13 commented Sep 21, 2024

Note that there's no valid 'a lifetime for the OwningPtr, in fact 'a is not used in the inputs at all. This might work with a callback API though.

@urben1680 urben1680 changed the title Feature request: EntityWorldMut::take_by_id(s) Feature request: moving specific components out of an entity in an untyped API Sep 23, 2024
@urben1680 urben1680 changed the title Feature request: moving specific components out of an entity in an untyped API Moving specific components out of an entity in an untyped API Sep 23, 2024
@urben1680 urben1680 changed the title Moving specific components out of an entity in an untyped API Moving components out of an entity by ids Sep 23, 2024
@BenjaminBrienen BenjaminBrienen added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Design This issue requires design work to think about how it would best be accomplished A-Pointers Relating to Bevy pointer abstractions and removed S-Needs-Triage This issue needs to be labelled labels Sep 25, 2024
@urben1680 urben1680 changed the title Moving components out of an entity by ids Moving components from one entity to another Sep 25, 2024
@urben1680
Copy link
Contributor Author

I took the liberty to rewrite parts of the initial comment to focus on the "move component from one entity to another" solution. The "return some blob data for later reinsertion" seems just not attractive to me while the first might be interesting for other use cases as well, also to be implemented for EntityCommands

I hope this brings the remaining design work forward.

@BenjaminBrienen BenjaminBrienen removed the A-Pointers Relating to Bevy pointer abstractions label Nov 23, 2024
github-merge-queue bot pushed a commit that referenced this issue Dec 16, 2024
…rldMut` and `EntityCommands` (#16826)

## Objective

Thanks to @eugineerd's work on entity cloning (#16132), we now have a
robust way to copy components between entities. We can extend this to
implement some useful functionality that would have been more
complicated before.

Closes #15350.

## Solution

`EntityCloneBuilder` now automatically includes required components
alongside any component added/removed from the component filter.

Added the following methods to `EntityCloneBuilder`:
- `move_components`
- `without_required_components`

Added the following methods to `EntityWorldMut` and `EntityCommands`:
- `clone_with`
- `clone_components`
- `move_components`

Also added `clone_and_spawn` and `clone_and_spawn_with` to
`EntityWorldMut` (`EntityCommands` already had them).

## Showcase

```
assert_eq!(world.entity(entity_a).get::<B>(), Some(&B));
assert_eq!(world.entity(entity_b).get::<B>(), None);
world.entity_mut(entity_a).clone_components::<B>(entity_b);
assert_eq!(world.entity(entity_a).get::<B>(), Some(&B));
assert_eq!(world.entity(entity_b).get::<B>(), Some(&B));

assert_eq!(world.entity(entity_a).get::<C>(), Some(&C(5)));
assert_eq!(world.entity(entity_b).get::<C>(), None);
world.entity_mut(entity_a).move_components::<C>(entity_b);
assert_eq!(world.entity(entity_a).get::<C>(), None);
assert_eq!(world.entity(entity_b).get::<C>(), Some(&C(5)));
```
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 D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Design This issue requires design work to think about how it would best be accomplished
Projects
None yet
3 participants