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

Command to clone entities #1515

Open
alice-i-cecile opened this issue Feb 24, 2021 · 17 comments
Open

Command to clone entities #1515

alice-i-cecile opened this issue Feb 24, 2021 · 17 comments
Labels
A-ECS Entities, components, systems, and events A-Reflection Runtime information about types C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged

Comments

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Feb 24, 2021

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

Creating copies of an entity is a powerful but simple abstraction. It's obviously useful for prefab workflows, but I'm also running into it when trying to create multiple copies of attacks and in similar gameplay code.

What solution would you like?

Create a fn clone_entity(entity: Entity) method for Commands. It would create a new entity with the same components and the same value of those components as the entity passed in.

What alternative(s) have you considered?

You can copy-paste the entity cloning code, but this is a lot of boilerplate and doesn't work well at all if the components in the entity aren't constant.

Additional context

Solving this is one of the proposed steps to resolve #1446.

This would only work on entities whose components all implement Clone, which is common and not onerous, but requires some cleverness to capture. Alternatively, it could clone only those components that do implement clone, although you'd want a warning there.

Initial discussion on solving this in Discord.

@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 Feb 24, 2021
@alice-i-cecile
Copy link
Member Author

I think the correct behavior is actually to try to Clone every attached component, and then omit an error (that can either be handled or panic) if it fails. It's unfortunate that it probably has to be runtime, but it should fail very quickly, and you'll want clone on most of your components anyways.

You can further enhance safety by adding archetype invariants (#1481) to the components that can't implement Clone to make them incompatible with the components found on entities you'll want to clone.

@alice-i-cecile
Copy link
Member Author

alice-i-cecile commented Mar 5, 2021

Desired API

/// Clones components from the each entity in `source_entities` onto a new entity
/// If the length of cloned_components is > 0, only the components in that bundle are cloned, if they exist
/// Otherwise, all clone-registered components are used
fn clone_entities(source_entities: Vec<Entity>, cloned_components: Vec<ComponentId>)

Components can be added to each cloned entity using a chained method with the builder pattern. This is commonly needed for differentiating the clone from the orginal.

Joint effort from @cart, @TheRawMeatball and myself on Discord.

@alice-i-cecile
Copy link
Member Author

alice-i-cecile commented Mar 5, 2021

Mock-up implementation by @TheRawMeatball from Discord

type CloneRegistry = Vec<fn(&mut World, Entity, &mut Commands)>
// in impl AppBuilder
fn register_clonable<T: Clone>(&mut self) {
  self.get_resource_mut::<CloneRegistry>().push(|world, entity, commands| {
    if let Ok(c) = world.get::<T>(entity) {
      commands.with(c.clone());
    }
  })
}

struct CloneEntityCommand {
  list: Vec<Entity>
}

impl Command for CloneEntityCommand {
  fn apply(self: Box<Self>, world: &mut World) {
    let registry = world.get_resource_mut::<CloneRegistry>().clone();
    let mut commands = Commands::new();
    commands.spawn();
    for f in registry.into_iter() {
      for e in self.list.into_iter() {
        (f)(world, e, &mut commands);
      }
    }
  }
}

@TheRawMeatball
Copy link
Member

Can you please use ```rust code fences?

@alice-i-cecile
Copy link
Member Author

Can you please use ```rust code fences?

Done. I'd missed that "rs" works on Discord, but not on Github.

@Davier
Copy link
Contributor

Davier commented Mar 6, 2021

I think the Clone registration could be part of Reflect, like for Hash, PartialEq and Serialize

@Davier
Copy link
Contributor

Davier commented Mar 6, 2021

Actually, it may already be possible using this:

pub fn copy_component(

It doesn't use Clone at all, instead it applies a reflected component on a new defaulted component.

@alice-i-cecile
Copy link
Member Author

More follow-up on related ideas.

@alice-i-cecile
Copy link
Member Author

Relevant discussion from Amethyst, discussing the traits needed to get asset serialization / deserialization working.

The ideas around component definitions and data transformation seem particularly transferrable here.

@alice-i-cecile alice-i-cecile added the A-Reflection Runtime information about types label Apr 14, 2021
@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 Apr 23, 2021
@alice-i-cecile
Copy link
Member Author

Another Meatball-y draft impl:

struct EntityCloneSystems(Vec<fn(&mut World, Entity, Entity)>);

fn clone_system<T: Clone + Component>(world: &mut World, target: Entity, reference: Entity) {
    if let Some(v) = world.entity(reference).get::<T>() {
        let v = v.clone();
        world.entity_mut(target).insert(v);
    }
}

// do fancy trait method extension stuff instead
impl AppBuilder {
    pub fn register_clonable<T: Clone + Component>(&mut self) -> &mut Self {
        self.world_mut()
            .get_resource_mut::<EntityCloneSystems>()
            .unwrap()
            .0
            .push(clone_system::<T>);
        self
    }
}
struct CloneEntityCommand(Entity);

impl Command for CloneEntityCommand {
    fn write(self: Box<Self>, world: &mut World) {
        world.resource_scope(|world, systems: Mut<EntityCloneSystems>| {
            let target = world.spawn().id();
            for s in systems.0.iter() {
                s(world, target, self.0);
            }
        })
    }
}
  1. You register a clonable component.
  2. That pushes a cloning function into a resource.
  3. When you go to clone each function is called and attempts to pull out a component, cloning it over if it exists.

@GianpaoloBranca
Copy link

For anyone struggling with this, here is a gist on how I solved this using the reflections API:
https://gist.github.com/GianpaoloBranca/17e5bd6ada9bdb04cca58182db8505d4

@nwtnni
Copy link

nwtnni commented Apr 8, 2023

For anyone struggling with this, here is a gist on how I solved this using the reflections API: https://gist.github.com/GianpaoloBranca/17e5bd6ada9bdb04cca58182db8505d4

Thanks for the reference! I found a way to modify your solution to remove the unsafe code and the &World and &mut World aliasing, but at the cost of a lot more copying and allocating: https://gist.github.com/nwtnni/85d6b87ae75337a522166c500c9a8418.

In particular, this version has to:

  • Clone all of the source components' ReflectComponent structs (9 function pointers each at time of writing).
  • Clone each source component via clone_value and clone_dynamic, which seems expensive.

@GianpaoloBranca
Copy link

GianpaoloBranca commented Apr 8, 2023

Nice approach! I did some quick benchmark and, at least for my use case, it's slower but it's not that slower. Cloning around 1000 entites with 2-4 components each takes roughly 3ms for the unsafe version and 8ms for the safe route.
The values scale linearly with more or less copies.

@LIMachi
Copy link

LIMachi commented Nov 10, 2023

Minor fix for the safe version: the aggressive usage of unwrap on the type registry will result in a crash if a component is not reflect, here is a fix (and should probably send an optional warning): .filter_map(|type_id| { registry .get(type_id) .and_then(|registered_type| { registered_type.data::<ReflectComponent>() }) .map(|component| { component.clone() }) }) (by using a filtermap and removing the unwraps, I managed to skip invalid components, but I'm unsure if this the best way of doing this)

@Multirious
Copy link
Contributor

Multirious commented Feb 16, 2024

I'ved modified the unsafe version a bit to account for children. The added part can be used for the safe version because it do not uses any unsafe block.

  • Updated clone_entity_components to skip parent and children component to stop from making invalid hierarchy.
  • Added clone_entity_recursive to clone an entity and their children.

https://gist.github.com/Multirious/ff049b368cfe7e9df130bdf59f14e777

Edit: after been using this for awhile, I've come across that there's some caveat like you cannot access the children in the same system with this method because of the Commands deferred nature.

Edit 2: Solved the above by creating empty entities and storing their ids to a tree data structure to be use by user.
Note: Clone components from the tree first before doing any modification to the destination entities!

https://gist.github.com/Multirious/f3fd24d4ce882085f295f80cffe4fe4e

@blueforesticarus
Copy link

blueforesticarus commented Apr 23, 2024

Slightly different from this issue, but related and simpler: is there a way to merge entities.

ie. move all components of entity A into entity B, and destroy entity A

presumably this could work without reflect or even clone

@alice-i-cecile
Copy link
Member Author

Not easily. Might be possible by playing with the internals however.

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 A-Reflection Runtime information about types C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged
Projects
Status: Open
Development

No branches or pull requests

8 participants