From 61637698d50f2e150da62994e204d5fdb87af35d Mon Sep 17 00:00:00 2001 From: eugineerd Date: Sun, 5 Jan 2025 10:58:41 +0000 Subject: [PATCH 01/15] basic world cloning --- crates/bevy_ecs/src/component.rs | 4 +- crates/bevy_ecs/src/entity/clone_entities.rs | 132 +++++++++++++++++-- 2 files changed, 126 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 66d5db98f8fcb..6e1dec468af5d 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1025,7 +1025,7 @@ impl ComponentCloneHandler { } /// A registry of component clone handlers. Allows to set global default and per-component clone function for all components in the world. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct ComponentCloneHandlers { handlers: Vec>, default_handler: ComponentCloneFn, @@ -1089,7 +1089,7 @@ impl Default for ComponentCloneHandlers { } /// Stores metadata associated with each kind of [`Component`] in a given [`World`]. -#[derive(Debug, Default)] +#[derive(Debug, Default, Clone)] pub struct Components { components: Vec, indices: TypeIdMap, diff --git a/crates/bevy_ecs/src/entity/clone_entities.rs b/crates/bevy_ecs/src/entity/clone_entities.rs index 2b51c1f937b34..22205b91777ca 100644 --- a/crates/bevy_ecs/src/entity/clone_entities.rs +++ b/crates/bevy_ecs/src/entity/clone_entities.rs @@ -1,7 +1,7 @@ use alloc::{borrow::ToOwned, vec::Vec}; use bevy_ptr::{Ptr, PtrMut}; use bumpalo::Bump; -use core::{any::TypeId, ptr::NonNull}; +use core::{any::TypeId, panic::Location, ptr::NonNull}; use bevy_utils::{HashMap, HashSet}; @@ -280,13 +280,26 @@ pub struct EntityCloner { impl EntityCloner { /// Clones and inserts components from the `source` entity into `target` entity using the stored configuration. pub fn clone_entity(&mut self, world: &mut World) { + // SAFETY: target_world is None + unsafe { + self.clone_entity_impl(world, None); + } + } + + /// # Safety + /// - If `target_world` is not `None`, caller must ensure that it contains the same [`Components`] as `source_world` + unsafe fn clone_entity_impl( + &mut self, + source_world: &mut World, + target_world: Option<&mut World>, + ) { // SAFETY: // - `source_entity` is read-only. // - `type_registry` is read-only. // - `components` is read-only. // - `deferred_world` disallows structural ecs changes, which means all read-only resources above a not affected. let (type_registry, source_entity, components, mut deferred_world) = unsafe { - let world = world.as_unsafe_world_cell(); + let world = source_world.as_unsafe_world_cell(); let source_entity = world .get_entity(self.source) .expect("Source entity must exist"); @@ -350,26 +363,30 @@ impl EntityCloner { } } - world.flush(); + source_world.flush(); + + let target_world = target_world.unwrap_or(source_world); - if !world.entities.contains(self.target) { + if !target_world.entities.contains(self.target) { panic!("Target entity does not exist"); } debug_assert_eq!(component_data_ptrs.len(), component_ids.len()); // SAFETY: - // - All `component_ids` are from the same world as `target` entity + // - All `component_ids` are from `source_world`, and caller ensures that `target_world` has same `Components` as `source_world` // - All `component_data_ptrs` are valid types represented by `component_ids` unsafe { - world.entity_mut(self.target).insert_by_ids( + target_world.entity_mut(self.target).insert_by_ids( &component_ids, component_data_ptrs.into_iter().map(|ptr| ptr.promote()), ); } if self.move_components { - world.entity_mut(self.source).remove_by_ids(&component_ids); + target_world + .entity_mut(self.source) + .remove_by_ids(&component_ids); } } @@ -458,6 +475,35 @@ pub struct EntityCloneBuilder<'w> { move_components: bool, } +fn clone_world(source_world: &mut World, target_world: &mut World) { + target_world.components = source_world.components.clone(); + target_world.entities.reserve(source_world.entities.len()); + + for entity in source_world + .iter_entities() + .map(|e| e.id()) + .collect::>() + { + if target_world + .get_or_spawn_with_caller(entity, Location::caller()) + .is_none() + { + continue; + }; + unsafe { + EntityCloner { + source: entity, + target: entity, + filter_allows_components: false, + filter: Arc::new(Default::default()), + clone_handlers_overrides: Arc::new(Default::default()), + move_components: false, + } + .clone_entity_impl(source_world, Some(target_world)) + }; + } +} + impl<'w> EntityCloneBuilder<'w> { /// Creates a new [`EntityCloneBuilder`] for world. pub fn new(world: &'w mut World) -> Self { @@ -668,7 +714,8 @@ mod tests { use crate::{ self as bevy_ecs, component::{Component, ComponentCloneHandler, ComponentDescriptor, StorageType}, - entity::EntityCloneBuilder, + entity::{clone_entities::clone_world, EntityCloneBuilder}, + system::Resource, world::{DeferredWorld, World}, }; use alloc::vec::Vec; @@ -1114,4 +1161,73 @@ mod tests { ); } } + + #[test] + fn clone_world_works() { + #[derive(Component, Clone, PartialEq, Eq, Debug)] + struct A { + field: usize, + } + + let mut source_world = World::default(); + + let c1 = A { field: 5 }; + let c2 = A { field: 6 }; + + let e1 = source_world.spawn(c1.clone()).id(); + let e2 = source_world.spawn(c2.clone()).id(); + + let mut target_world = World::default(); + + clone_world(&mut source_world, &mut target_world); + + assert_eq!(target_world.get::(e1), Some(&c1)); + assert_eq!(target_world.get::(e2), Some(&c2)); + } + + #[test] + fn clone_world_with_commands_in_clone_handler_works() { + #[derive(Component, Clone, PartialEq, Eq, Debug)] + struct A { + field: usize, + } + + #[derive(Resource, PartialEq, Eq, Debug)] + struct R { + field: usize, + } + + let mut source_world = World::default(); + let c_id = source_world.register_component::(); + source_world + .get_component_clone_handlers_mut() + .set_component_handler( + c_id, + ComponentCloneHandler::custom_handler(|world, ctx| { + let a = ctx.read_source_component::().unwrap().clone(); + ctx.write_target_component(a); + world.commands().queue(|world: &mut World| { + if let Some(mut res) = world.get_resource_mut::() { + res.field += 1; + } else { + world.insert_resource(R { field: 10 }); + } + }); + }), + ); + + let c1 = A { field: 5 }; + let c2 = A { field: 6 }; + + let e1 = source_world.spawn(c1.clone()).id(); + let e2 = source_world.spawn(c2.clone()).id(); + + let mut target_world = World::default(); + + clone_world(&mut source_world, &mut target_world); + + assert_eq!(target_world.get::(e1), Some(&c1)); + assert_eq!(target_world.get::(e2), Some(&c2)); + assert_eq!(target_world.get_resource::(), Some(&R { field: 11 })); + } } From 79e3a5d4427e350e38f9b777ebb573ae5d56cb48 Mon Sep 17 00:00:00 2001 From: eugineerd Date: Wed, 8 Jan 2025 14:15:40 +0000 Subject: [PATCH 02/15] change component clone handlers to take `&World` instead of `&mut DeferredWorld` --- crates/bevy_ecs/src/component.rs | 31 +- crates/bevy_ecs/src/entity/clone_entities.rs | 372 +++++++----------- .../bevy_ecs/src/observer/entity_observer.rs | 11 +- crates/bevy_hierarchy/src/hierarchy.rs | 28 +- 4 files changed, 182 insertions(+), 260 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 6e1dec468af5d..2600ecaf4ccef 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -983,7 +983,7 @@ impl ComponentDescriptor { } /// Function type that can be used to clone an entity. -pub type ComponentCloneFn = fn(&mut DeferredWorld, &mut ComponentCloneCtx); +pub type ComponentCloneFn = fn(&World, &mut ComponentCloneCtx); /// A struct instructing which clone handler to use when cloning a component. #[derive(Debug)] @@ -1003,8 +1003,8 @@ impl ComponentCloneHandler { /// Set clone handler based on `Clone` trait. /// /// If set as a handler for a component that is not the same as the one used to create this handler, it will panic. - pub fn clone_handler() -> Self { - Self(Some(component_clone_via_clone::)) + pub fn clone_handler() -> Self { + Self(Some(component_clone_via_clone::)) } /// Set clone handler based on `Reflect` trait. @@ -2170,11 +2170,8 @@ pub fn enforce_no_required_components_recursion( /// It will panic if set as handler for any other component. /// /// See [`ComponentCloneHandlers`] for more details. -pub fn component_clone_via_clone( - _world: &mut DeferredWorld, - ctx: &mut ComponentCloneCtx, -) { - if let Some(component) = ctx.read_source_component::() { +pub fn component_clone_via_clone(_world: &World, ctx: &mut ComponentCloneCtx) { + if let Some(component) = ctx.read_source_component::() { ctx.write_target_component(component.clone()); } } @@ -2195,7 +2192,7 @@ pub fn component_clone_via_clone( /// /// See [`EntityCloneBuilder`](crate::entity::EntityCloneBuilder) for details. #[cfg(feature = "bevy_reflect")] -pub fn component_clone_via_reflect(world: &mut DeferredWorld, ctx: &mut ComponentCloneCtx) { +pub fn component_clone_via_reflect(_world: &World, ctx: &mut ComponentCloneCtx) { let Some(registry) = ctx.type_registry() else { return; }; @@ -2233,12 +2230,16 @@ pub fn component_clone_via_reflect(world: &mut DeferredWorld, ctx: &mut Componen if let Some(reflect_from_world) = registry.get_type_data::(type_id) { + let Some(mut commands) = ctx.commands() else { + return; + }; + let Some(target) = ctx.target() else { return }; let reflect_from_world = reflect_from_world.clone(); let source_component_cloned = source_component_reflect.clone_value(); + drop(registry); let component_layout = component_info.layout(); - let target = ctx.target(); let component_id = ctx.component_id(); - world.commands().queue(move |world: &mut World| { + commands.queue(move |world: &mut World| { let mut component = reflect_from_world.from_world(world); assert_eq!(type_id, (*component).type_id()); component.apply(source_component_cloned.as_partial_reflect()); @@ -2260,7 +2261,7 @@ pub fn component_clone_via_reflect(world: &mut DeferredWorld, ctx: &mut Componen /// Noop implementation of component clone handler function. /// /// See [`EntityCloneBuilder`](crate::entity::EntityCloneBuilder) for details. -pub fn component_clone_ignore(_world: &mut DeferredWorld, _ctx: &mut ComponentCloneCtx) {} +pub fn component_clone_ignore(_world: &World, _ctx: &mut ComponentCloneCtx) {} /// Wrapper for components clone specialization using autoderef. #[doc(hidden)] @@ -2277,7 +2278,7 @@ impl Default for ComponentCloneSpecializationWrapper { pub trait ComponentCloneBase { fn get_component_clone_handler(&self) -> ComponentCloneHandler; } -impl ComponentCloneBase for ComponentCloneSpecializationWrapper { +impl ComponentCloneBase for ComponentCloneSpecializationWrapper { fn get_component_clone_handler(&self) -> ComponentCloneHandler { ComponentCloneHandler::default_handler() } @@ -2288,8 +2289,8 @@ impl ComponentCloneBase for ComponentCloneSpecializationWrapper pub trait ComponentCloneViaClone { fn get_component_clone_handler(&self) -> ComponentCloneHandler; } -impl ComponentCloneViaClone for &ComponentCloneSpecializationWrapper { +impl ComponentCloneViaClone for &ComponentCloneSpecializationWrapper { fn get_component_clone_handler(&self) -> ComponentCloneHandler { - ComponentCloneHandler::clone_handler::() + ComponentCloneHandler::clone_handler::() } } diff --git a/crates/bevy_ecs/src/entity/clone_entities.rs b/crates/bevy_ecs/src/entity/clone_entities.rs index 22205b91777ca..4bf1b99f57ada 100644 --- a/crates/bevy_ecs/src/entity/clone_entities.rs +++ b/crates/bevy_ecs/src/entity/clone_entities.rs @@ -1,7 +1,7 @@ use alloc::{borrow::ToOwned, vec::Vec}; use bevy_ptr::{Ptr, PtrMut}; use bumpalo::Bump; -use core::{any::TypeId, panic::Location, ptr::NonNull}; +use core::{any::TypeId, ptr::NonNull}; use bevy_utils::{HashMap, HashSet}; @@ -17,11 +17,22 @@ use alloc::sync::Arc; use crate::{ bundle::Bundle, component::{Component, ComponentCloneHandler, ComponentId, ComponentInfo, Components}, - entity::Entity, + entity::{Entities, Entity}, query::DebugCheckedUnwrap, - world::World, + system::Commands, + world::{command_queue::RawCommandQueue, CommandQueue, World}, }; +struct BumpWriteBuffer<'a, 'b> { + target_components_ptrs: &'a mut Vec>, + target_components_buffer: &'b Bump, +} + +enum ComponentWriteBuffer<'a, 'b> { + BumpWriteBuffer(BumpWriteBuffer<'a, 'b>), + Ptr(NonNull), +} + /// Context for component clone handlers. /// /// Provides fast access to useful resources like [`AppTypeRegistry`](crate::reflect::AppTypeRegistry) @@ -29,46 +40,38 @@ use crate::{ pub struct ComponentCloneCtx<'a, 'b> { component_id: ComponentId, source_component_ptr: Ptr<'a>, - target_component_written: bool, - target_components_ptrs: &'a mut Vec>, - target_components_buffer: &'b Bump, components: &'a Components, + target_write_buffer: ComponentWriteBuffer<'a, 'b>, component_info: &'a ComponentInfo, - entity_cloner: &'a EntityCloner, + target_component_written: bool, + entity_cloner: Option<&'a EntityCloner>, + command_queue: Option, + entities: &'a Entities, #[cfg(feature = "bevy_reflect")] type_registry: Option<&'a crate::reflect::AppTypeRegistry>, - #[cfg(not(feature = "bevy_reflect"))] - #[expect(dead_code)] - type_registry: Option<()>, } impl<'a, 'b> ComponentCloneCtx<'a, 'b> { - /// Create a new instance of `ComponentCloneCtx` that can be passed to component clone handlers. - /// - /// # Safety - /// Caller must ensure that: - /// - `components` and `component_id` are from the same world. - /// - `source_component_ptr` points to a valid component of type represented by `component_id`. - unsafe fn new( + pub unsafe fn new_for_component( component_id: ComponentId, source_component_ptr: Ptr<'a>, - target_components_ptrs: &'a mut Vec>, - target_components_buffer: &'b Bump, - components: &'a Components, - entity_cloner: &'a EntityCloner, - #[cfg(feature = "bevy_reflect")] type_registry: Option<&'a crate::reflect::AppTypeRegistry>, - #[cfg(not(feature = "bevy_reflect"))] type_registry: Option<()>, + target_component_ptr: NonNull, + world: &'a World, ) -> Self { + let components = world.components(); + let target_write_buffer = ComponentWriteBuffer::Ptr(target_component_ptr); Self { component_id, source_component_ptr, - target_components_ptrs, + target_write_buffer, target_component_written: false, - target_components_buffer, - components, component_info: components.get_info_unchecked(component_id), - entity_cloner, - type_registry, + entities: world.entities(), + command_queue: None, + components, + entity_cloner: None, + #[cfg(feature = "bevy_reflect")] + type_registry: world.get_resource::(), } } @@ -78,13 +81,13 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { } /// Returns the current source entity. - pub fn source(&self) -> Entity { - self.entity_cloner.source + pub fn source(&self) -> Option { + self.entity_cloner.map(|c| c.source) } /// Returns the current target entity. - pub fn target(&self) -> Entity { - self.entity_cloner.target + pub fn target(&self) -> Option { + self.entity_cloner.map(|c| c.target) } /// Returns the [`ComponentId`] of the component being cloned. @@ -97,10 +100,21 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { self.component_info } + /// Returns instance of [`Components`]. + pub fn components(&self) -> &Components { + self.components + } + + pub fn commands(&self) -> Option> { + self.command_queue + .as_ref() + .map(|queue| unsafe { Commands::new_raw_from_entities(queue.clone(), self.entities) }) + } + /// Returns a reference to the component on the source entity. /// /// Will return `None` if `ComponentId` of requested component does not match `ComponentId` of source component - pub fn read_source_component(&self) -> Option<&T> { + pub fn read_source_component(&self) -> Option<&T> { if self .component_info .type_id() @@ -135,6 +149,28 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { unsafe { Some(reflect_from_ptr.as_reflect(self.source_component_ptr)) } } + /// # Safety + /// - Passed `write_fn` must initialize component at passed pointer and return `true`, otherwise it should return `false`. + unsafe fn write_target_component_impl(&mut self, write_fn: impl FnOnce(NonNull) -> bool) { + match &mut self.target_write_buffer { + ComponentWriteBuffer::BumpWriteBuffer(buffer) => { + let target_component_data_ptr = buffer + .target_components_buffer + .alloc_layout(self.component_info.layout()); + + if write_fn(target_component_data_ptr) { + buffer + .target_components_ptrs + .push(PtrMut::new(target_component_data_ptr)); + self.target_component_written = true; + } + } + ComponentWriteBuffer::Ptr(ptr) => { + self.target_component_written = write_fn(*ptr); + } + } + } + /// Writes component data to target entity. /// /// # Panics @@ -142,9 +178,9 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { /// - Component has already been written once. /// - Component being written is not registered in the world. /// - `ComponentId` of component being written does not match expected `ComponentId`. - pub fn write_target_component(&mut self, component: T) { + pub fn write_target_component(&mut self, component: T) { let short_name = disqualified::ShortName::of::(); - if self.target_component_written { + if self.target_component_written() { panic!("Trying to write component '{short_name}' multiple times") } if self @@ -154,10 +190,12 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { { panic!("TypeId of component '{short_name}' does not match source component TypeId") }; - let component_ref = self.target_components_buffer.alloc(component); - self.target_components_ptrs - .push(PtrMut::from(component_ref)); - self.target_component_written = true; + unsafe { + self.write_target_component_impl(|target_ptr| { + target_ptr.cast::().write(component); + true + }); + } } /// Writes component data to target entity by providing a pointer to source component data and a pointer to uninitialized target component data. @@ -176,17 +214,11 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { &mut self, clone_fn: impl FnOnce(Ptr, NonNull) -> bool, ) { - if self.target_component_written { + if self.target_component_written() { panic!("Trying to write component multiple times") } - let layout = self.component_info.layout(); - let target_component_data_ptr = self.target_components_buffer.alloc_layout(layout); - - if clone_fn(self.source_component_ptr, target_component_data_ptr) { - self.target_components_ptrs - .push(PtrMut::new(target_component_data_ptr)); - self.target_component_written = true; - } + let source_component_ptr = self.source_component_ptr; + self.write_target_component_impl(|target_ptr| clone_fn(source_component_ptr, target_ptr)); } /// Writes component data to target entity. @@ -200,7 +232,7 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { /// - Component has already been written once. #[cfg(feature = "bevy_reflect")] pub fn write_target_component_reflect(&mut self, component: Box) { - if self.target_component_written { + if self.target_component_written() { panic!("Trying to write component multiple times") } let source_type_id = self @@ -211,26 +243,19 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { if source_type_id != component_type_id { panic!("Passed component TypeId does not match source component TypeId") } - let component_layout = self.component_info.layout(); - + let layout = self.component_info.layout(); let component_data_ptr = Box::into_raw(component).cast::(); - let target_component_data_ptr = - self.target_components_buffer.alloc_layout(component_layout); - // SAFETY: - // - target_component_data_ptr and component_data have the same data type. - // - component_data_ptr has layout of component_layout unsafe { - core::ptr::copy_nonoverlapping( - component_data_ptr, - target_component_data_ptr.as_ptr(), - component_layout.size(), - ); - self.target_components_ptrs - .push(PtrMut::new(target_component_data_ptr)); - alloc::alloc::dealloc(component_data_ptr, component_layout); + self.write_target_component_impl(|target_ptr| { + core::ptr::copy_nonoverlapping( + component_data_ptr, + target_ptr.as_ptr(), + layout.size(), + ); + alloc::alloc::dealloc(component_data_ptr, layout); + true + }); } - - self.target_component_written = true; } /// Return a reference to this context's `EntityCloner` instance. @@ -249,15 +274,10 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { /// }); /// } /// ``` - pub fn entity_cloner(&self) -> &EntityCloner { + pub fn entity_cloner(&self) -> Option<&EntityCloner> { self.entity_cloner } - /// Returns instance of [`Components`]. - pub fn components(&self) -> &Components { - self.components - } - /// Returns [`AppTypeRegistry`](`crate::reflect::AppTypeRegistry`) if it exists in the world. /// /// NOTE: Prefer this method instead of manually reading the resource from the world. @@ -277,46 +297,35 @@ pub struct EntityCloner { move_components: bool, } +pub(crate) unsafe fn clone_component_world( + component_id: ComponentId, + source_component_ptr: Ptr, + target_component_ptr: NonNull, + world: &World, +) -> bool { + let handlers = world.components().get_component_clone_handlers(); + let handler = handlers.get_handler(component_id); + let mut ctx = ComponentCloneCtx::new_for_component( + component_id, + source_component_ptr, + target_component_ptr, + world, + ); + + handler(&world, &mut ctx); + + ctx.target_component_written() +} + impl EntityCloner { /// Clones and inserts components from the `source` entity into `target` entity using the stored configuration. pub fn clone_entity(&mut self, world: &mut World) { - // SAFETY: target_world is None - unsafe { - self.clone_entity_impl(world, None); - } - } - - /// # Safety - /// - If `target_world` is not `None`, caller must ensure that it contains the same [`Components`] as `source_world` - unsafe fn clone_entity_impl( - &mut self, - source_world: &mut World, - target_world: Option<&mut World>, - ) { - // SAFETY: - // - `source_entity` is read-only. - // - `type_registry` is read-only. - // - `components` is read-only. - // - `deferred_world` disallows structural ecs changes, which means all read-only resources above a not affected. - let (type_registry, source_entity, components, mut deferred_world) = unsafe { - let world = source_world.as_unsafe_world_cell(); - let source_entity = world - .get_entity(self.source) - .expect("Source entity must exist"); - - #[cfg(feature = "bevy_reflect")] - let app_registry = world.get_resource::(); - #[cfg(not(feature = "bevy_reflect"))] - let app_registry = Option::<()>::None; - - ( - app_registry, - source_entity, - world.components(), - world.into_deferred(), - ) - }; + let source_entity = world + .get_entity(self.source) + .expect("Source entity must exist"); + let components = world.components(); let archetype = source_entity.archetype(); + let mut command_queue = CommandQueue::default(); let component_data = Bump::new(); let mut component_ids: Vec = Vec::with_capacity(archetype.component_count()); @@ -345,48 +354,47 @@ impl EntityCloner { // - `components` and `component` are from the same world // - `source_component_ptr` is valid and points to the same type as represented by `component` let mut ctx = unsafe { - ComponentCloneCtx::new( - component, - source_component_ptr, - &mut component_data_ptrs, - &component_data, + ComponentCloneCtx { + command_queue: Some(command_queue.get_raw()), + component_id: component, + component_info: components.get_info_unchecked(component), components, - self, - type_registry, - ) + entities: world.entities(), + entity_cloner: Some(self), + source_component_ptr, + target_component_written: false, + target_write_buffer: ComponentWriteBuffer::BumpWriteBuffer(BumpWriteBuffer { + target_components_buffer: &component_data, + target_components_ptrs: &mut component_data_ptrs, + }), + #[cfg(feature = "bevy_reflect")] + type_registry: world.get_resource::(), + } }; - (handler)(&mut deferred_world, &mut ctx); + (handler)(world, &mut ctx); - if ctx.target_component_written { + if ctx.target_component_written() { component_ids.push(component); } } - source_world.flush(); - - let target_world = target_world.unwrap_or(source_world); - - if !target_world.entities.contains(self.target) { - panic!("Target entity does not exist"); - } - debug_assert_eq!(component_data_ptrs.len(), component_ids.len()); // SAFETY: // - All `component_ids` are from `source_world`, and caller ensures that `target_world` has same `Components` as `source_world` // - All `component_data_ptrs` are valid types represented by `component_ids` unsafe { - target_world.entity_mut(self.target).insert_by_ids( + world.entity_mut(self.target).insert_by_ids( &component_ids, component_data_ptrs.into_iter().map(|ptr| ptr.promote()), ); } + command_queue.apply(world); + if self.move_components { - target_world - .entity_mut(self.source) - .remove_by_ids(&component_ids); + world.entity_mut(self.source).remove_by_ids(&component_ids); } } @@ -475,35 +483,6 @@ pub struct EntityCloneBuilder<'w> { move_components: bool, } -fn clone_world(source_world: &mut World, target_world: &mut World) { - target_world.components = source_world.components.clone(); - target_world.entities.reserve(source_world.entities.len()); - - for entity in source_world - .iter_entities() - .map(|e| e.id()) - .collect::>() - { - if target_world - .get_or_spawn_with_caller(entity, Location::caller()) - .is_none() - { - continue; - }; - unsafe { - EntityCloner { - source: entity, - target: entity, - filter_allows_components: false, - filter: Arc::new(Default::default()), - clone_handlers_overrides: Arc::new(Default::default()), - move_components: false, - } - .clone_entity_impl(source_world, Some(target_world)) - }; - } -} - impl<'w> EntityCloneBuilder<'w> { /// Creates a new [`EntityCloneBuilder`] for world. pub fn new(world: &'w mut World) -> Self { @@ -714,7 +693,7 @@ mod tests { use crate::{ self as bevy_ecs, component::{Component, ComponentCloneHandler, ComponentDescriptor, StorageType}, - entity::{clone_entities::clone_world, EntityCloneBuilder}, + entity::EntityCloneBuilder, system::Resource, world::{DeferredWorld, World}, }; @@ -831,7 +810,7 @@ mod tests { #[derive(Component, Reflect)] struct B; - fn test_handler(_world: &mut DeferredWorld, ctx: &mut ComponentCloneCtx) { + fn test_handler(_world: &World, ctx: &mut ComponentCloneCtx) { assert!(ctx.read_source_component_reflect().is_none()); } @@ -1099,7 +1078,7 @@ mod tests { #[test] fn clone_entity_with_dynamic_components() { const COMPONENT_SIZE: usize = 10; - fn test_handler(_world: &mut DeferredWorld, ctx: &mut ComponentCloneCtx) { + fn test_handler(_world: &World, ctx: &mut ComponentCloneCtx) { // SAFETY: this handler is only going to be used with a component represented by [u8; COMPONENT_SIZE] unsafe { ctx.write_target_component_ptr(move |source_ptr, target_ptr| { @@ -1161,73 +1140,4 @@ mod tests { ); } } - - #[test] - fn clone_world_works() { - #[derive(Component, Clone, PartialEq, Eq, Debug)] - struct A { - field: usize, - } - - let mut source_world = World::default(); - - let c1 = A { field: 5 }; - let c2 = A { field: 6 }; - - let e1 = source_world.spawn(c1.clone()).id(); - let e2 = source_world.spawn(c2.clone()).id(); - - let mut target_world = World::default(); - - clone_world(&mut source_world, &mut target_world); - - assert_eq!(target_world.get::(e1), Some(&c1)); - assert_eq!(target_world.get::(e2), Some(&c2)); - } - - #[test] - fn clone_world_with_commands_in_clone_handler_works() { - #[derive(Component, Clone, PartialEq, Eq, Debug)] - struct A { - field: usize, - } - - #[derive(Resource, PartialEq, Eq, Debug)] - struct R { - field: usize, - } - - let mut source_world = World::default(); - let c_id = source_world.register_component::(); - source_world - .get_component_clone_handlers_mut() - .set_component_handler( - c_id, - ComponentCloneHandler::custom_handler(|world, ctx| { - let a = ctx.read_source_component::().unwrap().clone(); - ctx.write_target_component(a); - world.commands().queue(|world: &mut World| { - if let Some(mut res) = world.get_resource_mut::() { - res.field += 1; - } else { - world.insert_resource(R { field: 10 }); - } - }); - }), - ); - - let c1 = A { field: 5 }; - let c2 = A { field: 6 }; - - let e1 = source_world.spawn(c1.clone()).id(); - let e2 = source_world.spawn(c2.clone()).id(); - - let mut target_world = World::default(); - - clone_world(&mut source_world, &mut target_world); - - assert_eq!(target_world.get::(e1), Some(&c1)); - assert_eq!(target_world.get::(e2), Some(&c2)); - assert_eq!(target_world.get_resource::(), Some(&R { field: 11 })); - } } diff --git a/crates/bevy_ecs/src/observer/entity_observer.rs b/crates/bevy_ecs/src/observer/entity_observer.rs index e86f6814a8ff6..ea20415172ab0 100644 --- a/crates/bevy_ecs/src/observer/entity_observer.rs +++ b/crates/bevy_ecs/src/observer/entity_observer.rs @@ -66,11 +66,14 @@ impl CloneEntityWithObserversExt for EntityCloneBuilder<'_> { } } -fn component_clone_observed_by(world: &mut DeferredWorld, ctx: &mut ComponentCloneCtx) { - let target = ctx.target(); - let source = ctx.source(); +fn component_clone_observed_by(_world: &World, ctx: &mut ComponentCloneCtx) { + let Some(target) = ctx.target() else { return }; + let Some(source) = ctx.source() else { return }; + let Some(mut commands) = ctx.commands() else { + return; + }; - world.commands().queue(move |world: &mut World| { + commands.queue(move |world: &mut World| { let observed_by = world .get::(source) .map(|observed_by| observed_by.0.clone()) diff --git a/crates/bevy_hierarchy/src/hierarchy.rs b/crates/bevy_hierarchy/src/hierarchy.rs index 7ab31a588535c..e8d0cb8b7807c 100644 --- a/crates/bevy_hierarchy/src/hierarchy.rs +++ b/crates/bevy_hierarchy/src/hierarchy.rs @@ -218,18 +218,22 @@ impl CloneEntityHierarchyExt for EntityCloneBuilder<'_> { } /// Clone handler for the [`Children`] component. Allows to clone the entity recursively. -fn component_clone_children(world: &mut DeferredWorld, ctx: &mut ComponentCloneCtx) { +fn component_clone_children(_world: &World, ctx: &mut ComponentCloneCtx) { + let Some(mut commands) = ctx.commands() else { + return; + }; + let Some(entity_cloner) = ctx.entity_cloner() else { + return; + }; + let Some(parent) = ctx.target() else { return }; let children = ctx .read_source_component::() .expect("Source entity must have Children component") .iter(); - let parent = ctx.target(); for child in children { - let child_clone = world.commands().spawn_empty().id(); - let mut clone_entity = ctx - .entity_cloner() - .with_source_and_target(*child, child_clone); - world.commands().queue(move |world: &mut World| { + let child_clone = commands.spawn_empty().id(); + let mut clone_entity = entity_cloner.with_source_and_target(*child, child_clone); + commands.queue(move |world: &mut World| { clone_entity.clone_entity(world); world.entity_mut(child_clone).set_parent(parent); }); @@ -237,12 +241,16 @@ fn component_clone_children(world: &mut DeferredWorld, ctx: &mut ComponentCloneC } /// Clone handler for the [`Parent`] component. Allows to add clone as a child to the parent entity. -fn component_clone_parent(world: &mut DeferredWorld, ctx: &mut ComponentCloneCtx) { +fn component_clone_parent(_world: &World, ctx: &mut ComponentCloneCtx) { + let Some(target) = ctx.target() else { return }; + let Some(mut commands) = ctx.commands() else { + return; + }; let parent = ctx .read_source_component::() - .map(|p| p.0) + .map(Parent::get) .expect("Source entity must have Parent component"); - world.commands().entity(ctx.target()).set_parent(parent); + commands.entity(target).set_parent(parent); } #[cfg(test)] From 7038a37c8d7c2e2c44bd423a53712d99f127d9df Mon Sep 17 00:00:00 2001 From: eugineerd Date: Wed, 8 Jan 2025 14:18:01 +0000 Subject: [PATCH 03/15] allow to register clone handlers for resources --- crates/bevy_ecs/macros/src/component.rs | 5 +++++ crates/bevy_ecs/src/component.rs | 7 +++++-- crates/bevy_ecs/src/system/system_param.rs | 11 +++++++++-- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/macros/src/component.rs b/crates/bevy_ecs/macros/src/component.rs index cd7cd52bf6570..62638eebf89d0 100644 --- a/crates/bevy_ecs/macros/src/component.rs +++ b/crates/bevy_ecs/macros/src/component.rs @@ -51,6 +51,11 @@ pub fn derive_resource(input: TokenStream) -> TokenStream { TokenStream::from(quote! { impl #impl_generics #bevy_ecs_path::system::Resource for #struct_name #type_generics #where_clause { + fn get_component_clone_handler() -> #bevy_ecs_path::component::ComponentCloneHandler { + use #bevy_ecs_path::component::{ComponentCloneViaClone, ComponentCloneBase}; + (&&&#bevy_ecs_path::component::ComponentCloneSpecializationWrapper::::default()) + .get_component_clone_handler() + } } }) } diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 2600ecaf4ccef..026ccf28eebc7 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1609,11 +1609,14 @@ impl Components { #[inline] pub fn register_resource(&mut self) -> ComponentId { // SAFETY: The [`ComponentDescriptor`] matches the [`TypeId`] - unsafe { + let component_id = unsafe { self.get_or_register_resource_with(TypeId::of::(), || { ComponentDescriptor::new_resource::() }) - } + }; + self.component_clone_handlers + .set_component_handler(component_id, T::get_component_clone_handler()); + component_id } /// Registers a [`Resource`] described by `descriptor`. diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 94c78c85e936e..36de1573ff78f 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -3,7 +3,7 @@ use crate::{ archetype::{Archetype, Archetypes}, bundle::Bundles, change_detection::{Ticks, TicksMut}, - component::{ComponentId, ComponentTicks, Components, Tick}, + component::{ComponentCloneHandler, ComponentId, ComponentTicks, Components, Tick}, entity::Entities, query::{ Access, FilteredAccess, FilteredAccessSet, QueryData, QueryFilter, QuerySingleError, @@ -839,7 +839,14 @@ all_tuples_enumerated!(impl_param_set, 1, 8, P, m, p); label = "invalid `Resource`", note = "consider annotating `{Self}` with `#[derive(Resource)]`" )] -pub trait Resource: Send + Sync + 'static {} +pub trait Resource: Send + Sync + 'static { + /// Called when registering this component, allowing to override clone function (or disable cloning altogether) for this component. + /// + /// See [Handlers section of `EntityCloneBuilder`](crate::entity::EntityCloneBuilder#handlers) to understand how this affects handler priority. + fn get_component_clone_handler() -> ComponentCloneHandler { + ComponentCloneHandler::default_handler() + } +} // SAFETY: Res only reads a single World resource unsafe impl<'a, T: Resource> ReadOnlySystemParam for Res<'a, T> {} From 3efc2ee8e05641f3cee9cc350cb6c58235228925 Mon Sep 17 00:00:00 2001 From: eugineerd Date: Wed, 8 Jan 2025 14:23:09 +0000 Subject: [PATCH 04/15] implement `Clone` for trivially cloneable structs --- crates/bevy_ecs/src/archetype.rs | 10 ++++++++-- crates/bevy_ecs/src/bundle.rs | 3 ++- crates/bevy_ecs/src/entity/mod.rs | 11 +++++++++++ crates/bevy_ecs/src/event/base.rs | 2 +- crates/bevy_ecs/src/event/collections.rs | 4 ++-- crates/bevy_ecs/src/observer/mod.rs | 6 +++--- crates/bevy_ecs/src/removal_detection.rs | 2 +- crates/bevy_ecs/src/storage/sparse_set.rs | 8 ++++---- 8 files changed, 32 insertions(+), 14 deletions(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index e0d85242f9206..81859e5358760 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -119,6 +119,7 @@ pub(crate) enum ComponentStatus { } /// Used in [`Edges`] to cache the result of inserting a bundle into the source archetype. +#[derive(Clone)] pub(crate) struct ArchetypeAfterBundleInsert { /// The target archetype after the bundle is inserted into the source archetype. pub archetype_id: ArchetypeId, @@ -194,7 +195,7 @@ impl BundleComponentStatus for SpawnBundleStatus { /// yet. /// /// [`World`]: crate::world::World -#[derive(Default)] +#[derive(Default, Clone)] pub struct Edges { insert_bundle: SparseArray, remove_bundle: SparseArray>, @@ -306,6 +307,7 @@ impl Edges { } /// Metadata about an [`Entity`] in a [`Archetype`]. +#[derive(Clone)] pub struct ArchetypeEntity { entity: Entity, table_row: TableRow, @@ -339,6 +341,7 @@ pub(crate) struct ArchetypeSwapRemoveResult { /// Internal metadata for a [`Component`] within a given [`Archetype`]. /// /// [`Component`]: crate::component::Component +#[derive(Clone)] struct ArchetypeComponentInfo { storage_type: StorageType, archetype_component_id: ArchetypeComponentId, @@ -367,6 +370,7 @@ bitflags::bitflags! { /// /// [`World`]: crate::world::World /// [module level documentation]: crate::archetype +#[derive(Clone)] pub struct Archetype { id: ArchetypeId, table_id: TableId, @@ -720,7 +724,7 @@ impl ArchetypeGeneration { } } -#[derive(Hash, PartialEq, Eq)] +#[derive(Clone, Hash, PartialEq, Eq)] struct ArchetypeComponents { table_components: Box<[ComponentId]>, sparse_set_components: Box<[ComponentId]>, @@ -776,6 +780,7 @@ pub type ComponentIndex = HashMap, archetype_component_count: usize, @@ -786,6 +791,7 @@ pub struct Archetypes { } /// Metadata about how a component is stored in an [`Archetype`]. +#[derive(Clone)] pub struct ArchetypeRecord { /// Index of the component in the archetype's [`Table`](crate::storage::Table), /// or None if the component is a sparse set component. diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 9a0403f2d9aae..4e8d3d4572b88 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -348,6 +348,7 @@ pub(crate) enum InsertMode { /// Stores metadata associated with a specific type of [`Bundle`] for a given [`World`]. /// /// [`World`]: crate::world::World +#[derive(Clone)] pub struct BundleInfo { id: BundleId, /// The list of all components contributed by the bundle (including Required Components). This is in @@ -1435,7 +1436,7 @@ impl<'w> BundleSpawner<'w> { } /// Metadata for bundles. Stores a [`BundleInfo`] for each type of [`Bundle`] in a given world. -#[derive(Default)] +#[derive(Default, Clone)] pub struct Bundles { bundle_infos: Vec, /// Cache static [`BundleId`] diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 44e500a15bb36..53a38740decb9 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -579,6 +579,17 @@ pub struct Entities { len: u32, } +impl Clone for Entities { + fn clone(&self) -> Self { + Self { + meta: self.meta.clone(), + pending: self.pending.clone(), + free_cursor: AtomicIdCursor::new(self.free_cursor.load(Ordering::Relaxed)), + len: self.len.clone(), + } + } +} + impl Entities { pub(crate) const fn new() -> Self { Entities { diff --git a/crates/bevy_ecs/src/event/base.rs b/crates/bevy_ecs/src/event/base.rs index a7974c89e8dfd..9b5123317b5ad 100644 --- a/crates/bevy_ecs/src/event/base.rs +++ b/crates/bevy_ecs/src/event/base.rs @@ -119,7 +119,7 @@ impl Hash for EventId { } } -#[derive(Debug)] +#[derive(Clone, Debug)] #[cfg_attr(feature = "bevy_reflect", derive(Reflect))] pub(crate) struct EventInstance { pub event_id: EventId, diff --git a/crates/bevy_ecs/src/event/collections.rs b/crates/bevy_ecs/src/event/collections.rs index d35c4743ee648..918e93da5ad4a 100644 --- a/crates/bevy_ecs/src/event/collections.rs +++ b/crates/bevy_ecs/src/event/collections.rs @@ -91,7 +91,7 @@ use { /// [`EventReader`]: super::EventReader /// [`EventWriter`]: super::EventWriter /// [`event_update_system`]: super::event_update_system -#[derive(Debug, Resource)] +#[derive(Clone, Debug, Resource)] #[cfg_attr(feature = "bevy_reflect", derive(Reflect), reflect(Resource, Default))] pub struct Events { /// Holds the oldest still active events. @@ -331,7 +331,7 @@ impl Extend for Events { } } -#[derive(Debug)] +#[derive(Clone, Debug)] #[cfg_attr(feature = "bevy_reflect", derive(Reflect))] pub(crate) struct EventSequence { pub(crate) events: Vec>, diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index 8194c1e04ea65..570fbf3293969 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -325,7 +325,7 @@ impl ObserverTrigger { type ObserverMap = EntityHashMap; /// Collection of [`ObserverRunner`] for [`Observer`] registered to a particular trigger targeted at a specific component. -#[derive(Default, Debug)] +#[derive(Clone, Default, Debug)] pub struct CachedComponentObservers { // Observers listening to triggers targeting this component map: ObserverMap, @@ -334,7 +334,7 @@ pub struct CachedComponentObservers { } /// Collection of [`ObserverRunner`] for [`Observer`] registered to a particular trigger. -#[derive(Default, Debug)] +#[derive(Clone, Default, Debug)] pub struct CachedObservers { // Observers listening for any time this trigger is fired map: ObserverMap, @@ -345,7 +345,7 @@ pub struct CachedObservers { } /// Metadata for observers. Stores a cache mapping trigger ids to the registered observers. -#[derive(Default, Debug)] +#[derive(Clone, Default, Debug)] pub struct Observers { // Cached ECS observers to save a lookup most common triggers. on_add: CachedObservers, diff --git a/crates/bevy_ecs/src/removal_detection.rs b/crates/bevy_ecs/src/removal_detection.rs index c81a5cfa1eb2c..0e837e7ee43c5 100644 --- a/crates/bevy_ecs/src/removal_detection.rs +++ b/crates/bevy_ecs/src/removal_detection.rs @@ -64,7 +64,7 @@ impl DerefMut for RemovedComponentReader { } /// Stores the [`RemovedComponents`] event buffers for all types of component in a given [`World`]. -#[derive(Default, Debug)] +#[derive(Clone, Default, Debug)] pub struct RemovedComponentEvents { event_sets: SparseSet>, } diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 518333fa272d8..0a01a7e4b6e25 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -13,7 +13,7 @@ use nonmax::NonMaxUsize; type EntityIndex = u32; -#[derive(Debug)] +#[derive(Debug, Clone)] pub(crate) struct SparseArray { values: Vec>, marker: PhantomData, @@ -21,7 +21,7 @@ pub(crate) struct SparseArray { /// A space-optimized version of [`SparseArray`] that cannot be changed /// after construction. -#[derive(Debug)] +#[derive(Debug, Clone)] pub(crate) struct ImmutableSparseArray { values: Box<[Option]>, marker: PhantomData, @@ -368,7 +368,7 @@ impl ComponentSparseSet { /// A data structure that blends dense and sparse storage /// /// `I` is the type of the indices, while `V` is the type of data stored in the dense storage. -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct SparseSet { dense: Vec, indices: Vec, @@ -377,7 +377,7 @@ pub struct SparseSet { /// A space-optimized version of [`SparseSet`] that cannot be changed /// after construction. -#[derive(Debug)] +#[derive(Debug, Clone)] pub(crate) struct ImmutableSparseSet { dense: Box<[V]>, indices: Box<[I]>, From 2f1ac5fd0963615edf2baa1df45e3bc48543af02 Mon Sep 17 00:00:00 2001 From: eugineerd Date: Wed, 8 Jan 2025 14:26:10 +0000 Subject: [PATCH 05/15] implement `try_clone` for world --- crates/bevy_ecs/src/storage/blob_vec.rs | 15 ++++ crates/bevy_ecs/src/storage/mod.rs | 16 ++++ crates/bevy_ecs/src/storage/resource.rs | 71 +++++++++++++-- crates/bevy_ecs/src/storage/sparse_set.rs | 28 +++++- crates/bevy_ecs/src/storage/table/column.rs | 96 ++++++++++++++++++++- crates/bevy_ecs/src/storage/table/mod.rs | 31 +++++++ crates/bevy_ecs/src/world/error.rs | 10 +++ crates/bevy_ecs/src/world/mod.rs | 63 +++++++++++++- 8 files changed, 321 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/storage/blob_vec.rs b/crates/bevy_ecs/src/storage/blob_vec.rs index 51a3d49e3c08d..61a2a2e4fb993 100644 --- a/crates/bevy_ecs/src/storage/blob_vec.rs +++ b/crates/bevy_ecs/src/storage/blob_vec.rs @@ -184,6 +184,21 @@ impl BlobVec { core::ptr::copy_nonoverlapping::(value.as_ptr(), ptr.as_ptr(), self.item_layout.size()); } + pub(crate) unsafe fn try_initialize_next( + &mut self, + init_fn: impl Fn(NonNull) -> bool, + ) -> bool { + self.reserve(1); + let index = self.len; + self.len += 1; + let ptr = self.get_unchecked_mut(index); + if init_fn(ptr.into()) { + return true; + } + self.len = index; + false + } + /// Replaces the value at `index` with `value`. This function does not do any bounds checking. /// /// # Safety diff --git a/crates/bevy_ecs/src/storage/mod.rs b/crates/bevy_ecs/src/storage/mod.rs index eaeff97a8cccb..c9c4faa3acad3 100644 --- a/crates/bevy_ecs/src/storage/mod.rs +++ b/crates/bevy_ecs/src/storage/mod.rs @@ -27,10 +27,15 @@ mod sparse_set; mod table; mod thin_array_ptr; +use core::ptr::NonNull; + +use bevy_ptr::{OwningPtr, Ptr}; pub use resource::*; pub use sparse_set::*; pub use table::*; +use crate::world::{error::WorldCloneError, World}; + /// The raw data stores of a [`World`](crate::world::World) #[derive(Default)] pub struct Storages { @@ -43,3 +48,14 @@ pub struct Storages { /// Backing storage for `!Send` resources. pub non_send_resources: Resources, } + +impl Storages { + pub(crate) unsafe fn try_clone<'a>(&self, world: &World) -> Result { + Ok(Storages { + sparse_sets: self.sparse_sets.try_clone(world)?, + tables: self.tables.try_clone(world)?, + resources: self.resources.try_clone(world)?, + non_send_resources: self.non_send_resources.try_clone(world)?, + }) + } +} diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index 501c6e80a535b..7081f4f21d371 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -1,8 +1,10 @@ use crate::{ archetype::ArchetypeComponentId, change_detection::{MaybeLocation, MaybeUnsafeCellLocation, MutUntyped, TicksMut}, - component::{ComponentId, ComponentTicks, Components, Tick, TickCells}, + component::{ComponentId, ComponentInfo, ComponentTicks, Components, Tick, TickCells}, + entity::clone_component_world, storage::{blob_vec::BlobVec, SparseSet}, + world::{error::WorldCloneError, World}, }; use alloc::string::String; use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; @@ -69,24 +71,35 @@ impl ResourceData { /// If `SEND` is false, this will panic if called from a different thread than the one it was inserted from. #[inline] fn validate_access(&self) { - if SEND { + if self.try_validate_access() { return; } - #[cfg(feature = "std")] - if self.origin_thread_id != Some(std::thread::current().id()) { - // Panic in tests, as testing for aborting is nearly impossible - panic!( + // Panic in tests, as testing for aborting is nearly impossible + panic!( "Attempted to access or drop non-send resource {} from thread {:?} on a thread {:?}. This is not allowed. Aborting.", self.type_name, self.origin_thread_id, std::thread::current().id() ); + } + + #[inline] + fn try_validate_access(&self) -> bool { + #[cfg(feature = "std")] + { + return if SEND { + true + } else { + self.origin_thread_id == Some(std::thread::current().id()) + }; } // TODO: Handle no_std non-send. // Currently, no_std is single-threaded only, so this is safe to ignore. // To support no_std multithreading, an alternative will be required. + #[cfg(not(feature = "std"))] + true } /// Returns true if the resource is populated. @@ -304,6 +317,41 @@ impl ResourceData { self.added_ticks.get_mut().check_tick(change_tick); self.changed_ticks.get_mut().check_tick(change_tick); } + + pub(crate) unsafe fn try_clone( + &self, + component_info: &ComponentInfo, + world: &World, + ) -> Result { + if !self.try_validate_access() { + return Err(WorldCloneError::NonSendResourceCloned(component_info.id())); + } + let mut data = BlobVec::new(component_info.layout(), component_info.drop(), 1); + let is_initialized = data.try_initialize_next(|target_component_ptr| { + let source_component_ptr = self.data.get_unchecked(Self::ROW); + clone_component_world( + component_info.id(), + source_component_ptr, + target_component_ptr, + world, + ) + }); + if !is_initialized { + return Err(WorldCloneError::FailedToCloneComponent(component_info.id())); + } + + Ok(Self { + data: ManuallyDrop::new(data), + added_ticks: UnsafeCell::new(self.added_ticks.read()), + changed_ticks: UnsafeCell::new(self.changed_ticks.read()), + type_name: self.type_name.clone(), + id: self.id.clone(), + #[cfg(feature = "std")] + origin_thread_id: self.origin_thread_id.clone(), + #[cfg(feature = "track_location")] + changed_by: UnsafeCell::new(self.changed_by.read()), + }) + } } /// The backing store for all [`Resource`]s stored in the [`World`]. @@ -403,4 +451,15 @@ impl Resources { info.check_change_ticks(change_tick); } } + + pub(crate) unsafe fn try_clone(&self, world: &World) -> Result { + let mut resources = SparseSet::with_capacity(self.resources.len()); + for (component_id, res) in self.resources.iter() { + resources.insert( + *component_id, + res.try_clone(world.components().get_info_unchecked(*component_id), world)?, + ); + } + Ok(Self { resources }) + } } diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 0a01a7e4b6e25..1d84bbc094377 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -3,12 +3,13 @@ use crate::{ component::{ComponentId, ComponentInfo, ComponentTicks, Tick, TickCells}, entity::Entity, storage::{Column, TableRow}, + world::{error::WorldCloneError, World}, }; use alloc::{boxed::Box, vec::Vec}; use bevy_ptr::{OwningPtr, Ptr}; #[cfg(feature = "track_location")] use core::panic::Location; -use core::{cell::UnsafeCell, hash::Hash, marker::PhantomData}; +use core::{cell::UnsafeCell, hash::Hash, marker::PhantomData, ptr::NonNull}; use nonmax::NonMaxUsize; type EntityIndex = u32; @@ -140,6 +141,18 @@ impl ComponentSparseSet { } } + pub(crate) unsafe fn try_clone<'a>( + &self, + component_info: &ComponentInfo, + world: &World, + ) -> Result { + Ok(Self { + dense: unsafe { self.dense.try_clone(component_info, world)? }, + entities: self.entities.clone(), + sparse: self.sparse.clone(), + }) + } + /// Removes all of the values stored within. pub(crate) fn clear(&mut self) { self.dense.clear(); @@ -610,6 +623,19 @@ impl SparseSets { self.sets.is_empty() } + pub(crate) unsafe fn try_clone<'a>(&self, world: &World) -> Result { + let mut sets = SparseSet::with_capacity(self.sets.len()); + let components = world.components(); + for (component_id, set) in self.sets.iter() { + let component_info = components.get_info_unchecked(*component_id); + let set = set.try_clone(component_info, world)?; + sets.insert(*component_id, set); + } + let mut sparse_sets = SparseSets::default(); + sparse_sets.sets = sets; + Ok(sparse_sets) + } + /// An Iterator visiting all ([`ComponentId`], [`ComponentSparseSet`]) pairs. /// NOTE: Order is not guaranteed. pub fn iter(&self) -> impl Iterator { diff --git a/crates/bevy_ecs/src/storage/table/column.rs b/crates/bevy_ecs/src/storage/table/column.rs index 4054b5c15fd54..764318c27a5e0 100644 --- a/crates/bevy_ecs/src/storage/table/column.rs +++ b/crates/bevy_ecs/src/storage/table/column.rs @@ -1,7 +1,11 @@ +use core::ptr::NonNull; + use super::*; use crate::{ - component::TickCells, + component::{self, TickCells}, + entity::clone_component_world, storage::{blob_array::BlobArray, thin_array_ptr::ThinArrayPtr}, + world::{error::WorldCloneError, World}, }; use alloc::vec::Vec; use bevy_ptr::PtrMut; @@ -326,6 +330,53 @@ impl ThinColumn { ) -> &[UnsafeCell<&'static Location<'static>>] { self.changed_by.as_slice(len) } + + pub(crate) unsafe fn try_clone( + &self, + component_info: &ComponentInfo, + world: &World, + len: usize, + ) -> Result { + let mut column = Self::with_capacity(component_info, len); + column.added_ticks = ThinArrayPtr::from( + self.added_ticks + .as_slice(len) + .iter() + .map(|cell| UnsafeCell::new(cell.read())) + .collect::>(), + ); + column.changed_ticks = ThinArrayPtr::from( + self.changed_ticks + .as_slice(len) + .iter() + .map(|cell| UnsafeCell::new(cell.read())) + .collect::>(), + ); + #[cfg(feature = "track_location")] + { + column.changed_by = ThinArrayPtr::from( + self.changed_by + .as_slice(len) + .iter() + .map(|tick| UnsafeCell::new(tick.read())) + .collect::>(), + ); + } + for i in 0..len { + let source_component_ptr = self.data.get_unchecked(i); + let target_component_ptr = column.data.get_unchecked_mut(i).into(); + let is_initialized = clone_component_world( + component_info.id(), + source_component_ptr, + target_component_ptr, + world, + ); + if !is_initialized { + return Err(WorldCloneError::FailedToCloneComponent(component_info.id())); + } + } + Ok(column) + } } /// A type-erased contiguous container for data of a homogeneous type. @@ -686,4 +737,47 @@ impl Column { debug_assert!(row.as_usize() < self.changed_by.len()); self.changed_by.get_unchecked(row.as_usize()) } + + pub(crate) unsafe fn try_clone( + &self, + component_info: &ComponentInfo, + world: &World, + ) -> Result { + let len = self.added_ticks.len(); + let mut column = Self::with_capacity(component_info, len); + column.added_ticks = self + .added_ticks + .iter() + .map(|cell| UnsafeCell::new(cell.read())) + .collect(); + column.changed_ticks = self + .changed_ticks + .iter() + .map(|cell| UnsafeCell::new(cell.read())) + .collect(); + #[cfg(feature = "track_location")] + { + column.changed_by = self + .changed_by + .iter() + .map(|tick| UnsafeCell::new(tick.read())) + .collect(); + } + for i in 0..self.len() { + let row = TableRow::from_usize(i); + let source_component_ptr = self.get_data_unchecked(row); + let is_initialized = column.data.try_initialize_next(|target_component_ptr| { + clone_component_world( + component_info.id(), + source_component_ptr, + target_component_ptr, + world, + ) + }); + if !is_initialized { + return Err(WorldCloneError::FailedToCloneComponent(component_info.id())); + } + } + Ok(column) + } } diff --git a/crates/bevy_ecs/src/storage/table/mod.rs b/crates/bevy_ecs/src/storage/table/mod.rs index ce33890751ccd..ad16d8c565f92 100644 --- a/crates/bevy_ecs/src/storage/table/mod.rs +++ b/crates/bevy_ecs/src/storage/table/mod.rs @@ -4,6 +4,7 @@ use crate::{ entity::Entity, query::DebugCheckedUnwrap, storage::{blob_vec::BlobVec, ImmutableSparseSet, SparseSet}, + world::{error::WorldCloneError, World}, }; use alloc::{boxed::Box, vec, vec::Vec}; use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; @@ -671,6 +672,25 @@ impl Table { self.get_column(component_id) .map(|col| col.data.get_unchecked(row.as_usize())) } + + pub(crate) unsafe fn try_clone(&self, world: &World) -> Result { + let mut columns = SparseSet::with_capacity(self.columns.len()); + let column_len = self.entities.len(); + for (component_id, column) in self.columns.iter() { + columns.insert( + *component_id, + column.try_clone( + world.components().get_info_unchecked(*component_id), + world, + column_len, + )?, + ); + } + Ok(Self { + entities: self.entities.clone(), + columns: columns.into_immutable(), + }) + } } /// A collection of [`Table`] storages, indexed by [`TableId`] @@ -781,6 +801,17 @@ impl Tables { table.check_change_ticks(change_tick); } } + + pub(crate) unsafe fn try_clone(&self, world: &World) -> Result { + let mut tables = Vec::with_capacity(self.tables.len()); + for table in &self.tables { + tables.push(table.try_clone(world)?) + } + Ok(Self { + table_ids: self.table_ids.clone(), + tables, + }) + } } impl Index for Tables { diff --git a/crates/bevy_ecs/src/world/error.rs b/crates/bevy_ecs/src/world/error.rs index 1c6b5043bcea8..12552ec294f57 100644 --- a/crates/bevy_ecs/src/world/error.rs +++ b/crates/bevy_ecs/src/world/error.rs @@ -48,3 +48,13 @@ impl PartialEq for EntityFetchError { } impl Eq for EntityFetchError {} + +#[derive(Error, Debug, Clone, Copy, PartialEq, Eq)] +pub enum WorldCloneError { + #[error("More `bevy` `World`s have been created than is supported")] + WorldIdExhausted, + #[error("Component clone handler for component with ID {0:?} failed to clone the component")] + FailedToCloneComponent(ComponentId), + #[error("Component clone handler for component with ID {0:?} failed to clone the component")] + NonSendResourceCloned(ComponentId), +} diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 16948c5a9f740..e98c599f22a95 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -205,6 +205,29 @@ impl World { self.id } + pub fn try_clone(&self) -> Result { + let id = WorldId::new().ok_or(error::WorldCloneError::WorldIdExhausted)?; + let storages = unsafe { self.storages.try_clone(self)? }; + + let world = World { + id, + entities: self.entities.clone(), + components: self.components.clone(), + archetypes: self.archetypes.clone(), + storages, + bundles: self.bundles.clone(), + observers: self.observers.clone(), + removed_components: self.removed_components.clone(), + change_tick: AtomicU32::new(self.change_tick.load(Ordering::Relaxed)), + last_change_tick: self.last_change_tick.clone(), + last_check_tick: self.last_check_tick.clone(), + last_trigger_id: self.last_trigger_id, + command_queue: RawCommandQueue::new(), + }; + + Ok(world) + } + /// Creates a new [`UnsafeWorldCell`] view with complete read+write access. #[inline] pub fn as_unsafe_world_cell(&mut self) -> UnsafeWorldCell<'_> { @@ -3720,7 +3743,7 @@ mod tests { use crate::{ change_detection::DetectChangesMut, component::{ComponentDescriptor, ComponentInfo, StorageType}, - entity::EntityHashSet, + entity::{Entity, EntityHashSet}, ptr::OwningPtr, system::Resource, world::error::EntityFetchError, @@ -4372,4 +4395,42 @@ mod tests { .map(|_| {}), Err(EntityFetchError::NoSuchEntity(e, ..)) if e == e1)); } + + #[test] + fn clone_world() { + #[derive(Component, Clone, PartialEq, Eq, Debug)] + struct Comp { + value: i32, + alloc_value: Vec, + } + + #[derive(Component, Clone, PartialEq, Eq, Debug)] + struct ZSTComp; + + #[derive(Resource, Clone, PartialEq, Eq, Debug)] + struct Res { + value: i32, + alloc_value: Vec, + } + + let comp = Comp { + value: 5, + alloc_value: vec![1, 2, 3, 4, 5], + }; + let zst = ZSTComp; + let res = Res { + value: 1, + alloc_value: vec![7, 8, 9], + }; + + let mut world = World::default(); + let e_id = world.spawn((comp.clone(), zst.clone())).id(); + world.insert_resource(res.clone()); + + let mut world2 = world.try_clone().unwrap(); + + let mut query = world2.query::<(Entity, &Comp, &ZSTComp)>(); + assert_eq!(query.single(&world2), (e_id, &comp, &zst)); + assert_eq!(world2.get_resource::(), Some(&res)); + } } From 32ce2bc50fd88b8c237edbd1f38675b892d4f3c2 Mon Sep 17 00:00:00 2001 From: eugineerd Date: Thu, 9 Jan 2025 14:36:50 +0000 Subject: [PATCH 06/15] implement `Copy`-based component clone handler --- benches/benches/bevy_ecs/entity_cloning.rs | 4 +- crates/bevy_ecs/macros/src/component.rs | 22 ++ crates/bevy_ecs/src/component.rs | 202 ++++++++++-------- crates/bevy_ecs/src/entity/clone_entities.rs | 122 ++++++----- .../bevy_ecs/src/observer/entity_observer.rs | 3 +- crates/bevy_ecs/src/storage/blob_array.rs | 17 ++ crates/bevy_ecs/src/storage/blob_vec.rs | 21 ++ crates/bevy_ecs/src/storage/mod.rs | 38 +++- crates/bevy_ecs/src/storage/resource.rs | 70 ++++-- crates/bevy_ecs/src/storage/sparse_set.rs | 33 ++- crates/bevy_ecs/src/storage/table/column.rs | 59 ++++- crates/bevy_ecs/src/storage/table/mod.rs | 28 ++- crates/bevy_ecs/src/world/error.rs | 4 +- crates/bevy_ecs/src/world/mod.rs | 72 +++---- .../bevy_hierarchy/src/components/children.rs | 10 +- .../bevy_hierarchy/src/components/parent.rs | 10 +- examples/ecs/dynamic.rs | 5 +- examples/ecs/immutable_components.rs | 3 +- examples/stress_tests/many_components.rs | 3 +- 19 files changed, 496 insertions(+), 230 deletions(-) diff --git a/benches/benches/bevy_ecs/entity_cloning.rs b/benches/benches/bevy_ecs/entity_cloning.rs index 80577b9a9d0b5..d45ff7e78b036 100644 --- a/benches/benches/bevy_ecs/entity_cloning.rs +++ b/benches/benches/bevy_ecs/entity_cloning.rs @@ -67,11 +67,9 @@ fn set_reflect_clone_handler(world: &mut World) // this bundle are saved. let component_ids: Vec<_> = world.register_bundle::().contributed_components().into(); - let clone_handlers = world.get_component_clone_handlers_mut(); - // Overwrite the clone handler for all components in the bundle to use `Reflect`, not `Clone`. for component in component_ids { - clone_handlers.set_component_handler(component, ComponentCloneHandler::reflect_handler()); + world.set_component_clone_handler(component, ComponentCloneHandler::reflect_handler()); } } diff --git a/crates/bevy_ecs/macros/src/component.rs b/crates/bevy_ecs/macros/src/component.rs index 62638eebf89d0..ef12a340af545 100644 --- a/crates/bevy_ecs/macros/src/component.rs +++ b/crates/bevy_ecs/macros/src/component.rs @@ -53,6 +53,17 @@ pub fn derive_resource(input: TokenStream) -> TokenStream { impl #impl_generics #bevy_ecs_path::system::Resource for #struct_name #type_generics #where_clause { fn get_component_clone_handler() -> #bevy_ecs_path::component::ComponentCloneHandler { use #bevy_ecs_path::component::{ComponentCloneViaClone, ComponentCloneBase}; + + trait ComponentCloneViaCopy { + fn get_component_clone_handler(&self) -> #bevy_ecs_path::component::ComponentCloneHandler; + } + impl ComponentCloneViaCopy for &&#bevy_ecs_path::component::ComponentCloneSpecializationWrapper { + fn get_component_clone_handler(&self) -> #bevy_ecs_path::component::ComponentCloneHandler { + // SAFETY: T is Self and this handler will only be set for Self + unsafe { #bevy_ecs_path::component::ComponentCloneHandler::copy_handler::() } + } + } + (&&&#bevy_ecs_path::component::ComponentCloneSpecializationWrapper::::default()) .get_component_clone_handler() } @@ -174,6 +185,17 @@ pub fn derive_component(input: TokenStream) -> TokenStream { fn get_component_clone_handler() -> #bevy_ecs_path::component::ComponentCloneHandler { use #bevy_ecs_path::component::{ComponentCloneViaClone, ComponentCloneBase}; + + trait ComponentCloneViaCopy { + fn get_component_clone_handler(&self) -> #bevy_ecs_path::component::ComponentCloneHandler; + } + impl ComponentCloneViaCopy for &&#bevy_ecs_path::component::ComponentCloneSpecializationWrapper { + fn get_component_clone_handler(&self) -> #bevy_ecs_path::component::ComponentCloneHandler { + // SAFETY: T is Self and this handler will only be set for Self + unsafe { #bevy_ecs_path::component::ComponentCloneHandler::copy_handler::() } + } + } + (&&&#bevy_ecs_path::component::ComponentCloneSpecializationWrapper::::default()) .get_component_clone_handler() } diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 026ccf28eebc7..c3639223de9b8 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -787,6 +787,10 @@ impl ComponentInfo { pub fn required_components(&self) -> &RequiredComponents { &self.required_components } + + pub(crate) fn clone_handler(&self) -> &ComponentCloneHandler { + &self.descriptor.clone_handler + } } /// A value which uniquely identifies the type of a [`Component`] or [`Resource`] within a @@ -848,7 +852,6 @@ impl SparseSetIndex for ComponentId { } /// A value describing a component or resource, which may or may not correspond to a Rust type. -#[derive(Clone)] pub struct ComponentDescriptor { name: Cow<'static, str>, // SAFETY: This must remain private. It must match the statically known StorageType of the @@ -864,6 +867,23 @@ pub struct ComponentDescriptor { // None if the underlying type doesn't need to be dropped drop: Option unsafe fn(OwningPtr<'a>)>, mutable: bool, + clone_handler: ComponentCloneHandler, +} + +impl Clone for ComponentDescriptor { + fn clone(&self) -> Self { + Self { + name: self.name.clone(), + storage_type: self.storage_type.clone(), + is_send_and_sync: self.is_send_and_sync.clone(), + type_id: self.type_id.clone(), + layout: self.layout.clone(), + drop: self.drop.clone(), + mutable: self.mutable.clone(), + // SAFETY: This clone handler will be used with the same component + clone_handler: unsafe { self.clone_handler.clone_unchecked() }, + } + } } // We need to ignore the `drop` field in our `Debug` impl @@ -901,6 +921,7 @@ impl ComponentDescriptor { layout: Layout::new::(), drop: needs_drop::().then_some(Self::drop_ptr:: as _), mutable: T::Mutability::MUTABLE, + clone_handler: T::get_component_clone_handler(), } } @@ -915,6 +936,7 @@ impl ComponentDescriptor { layout: Layout, drop: Option unsafe fn(OwningPtr<'a>)>, mutable: bool, + clone_handler: ComponentCloneHandler, ) -> Self { Self { name: name.into(), @@ -924,6 +946,7 @@ impl ComponentDescriptor { layout, drop, mutable, + clone_handler, } } @@ -941,6 +964,7 @@ impl ComponentDescriptor { layout: Layout::new::(), drop: needs_drop::().then_some(Self::drop_ptr:: as _), mutable: true, + clone_handler: T::get_component_clone_handler(), } } @@ -953,6 +977,7 @@ impl ComponentDescriptor { layout: Layout::new::(), drop: needs_drop::().then_some(Self::drop_ptr:: as _), mutable: true, + clone_handler: ComponentCloneHandler::default_handler(), } } @@ -980,121 +1005,125 @@ impl ComponentDescriptor { pub fn mutable(&self) -> bool { self.mutable } + + pub fn clone_handler(&self) -> &ComponentCloneHandler { + &self.clone_handler + } } /// Function type that can be used to clone an entity. pub type ComponentCloneFn = fn(&World, &mut ComponentCloneCtx); +#[derive(Debug, Clone, Copy)] +pub(crate) enum ComponentCloneHandlerKind { + Default, + Ignore, + Copy, + Custom(ComponentCloneFn), +} + /// A struct instructing which clone handler to use when cloning a component. #[derive(Debug)] -pub struct ComponentCloneHandler(Option); +pub struct ComponentCloneHandler { + entity_handler: ComponentCloneHandlerKind, + world_handler: Option, +} impl ComponentCloneHandler { /// Use the global default function to clone the component with this handler. pub fn default_handler() -> Self { - Self(None) + Self { + entity_handler: ComponentCloneHandlerKind::Default, + world_handler: None, + } } /// Do not clone the component. When a command to clone an entity is issued, component with this handler will be skipped. pub fn ignore() -> Self { - Self(Some(component_clone_ignore)) + Self { + entity_handler: ComponentCloneHandlerKind::Ignore, + world_handler: None, + } } /// Set clone handler based on `Clone` trait. /// /// If set as a handler for a component that is not the same as the one used to create this handler, it will panic. pub fn clone_handler() -> Self { - Self(Some(component_clone_via_clone::)) + Self { + entity_handler: ComponentCloneHandlerKind::Custom(component_clone_via_clone::), + world_handler: None, + } + } + + pub unsafe fn copy_handler() -> Self { + Self { + entity_handler: ComponentCloneHandlerKind::Copy, + world_handler: None, + } } /// Set clone handler based on `Reflect` trait. #[cfg(feature = "bevy_reflect")] pub fn reflect_handler() -> Self { - Self(Some(component_clone_via_reflect)) + Self { + entity_handler: ComponentCloneHandlerKind::Custom(component_clone_via_reflect), + world_handler: None, + } } /// Set a custom handler for the component. pub fn custom_handler(handler: ComponentCloneFn) -> Self { - Self(Some(handler)) - } - - /// Get [`ComponentCloneFn`] representing this handler or `None` if set to default handler. - pub fn get_handler(&self) -> Option { - self.0 - } -} - -/// A registry of component clone handlers. Allows to set global default and per-component clone function for all components in the world. -#[derive(Debug, Clone)] -pub struct ComponentCloneHandlers { - handlers: Vec>, - default_handler: ComponentCloneFn, -} - -impl ComponentCloneHandlers { - /// Sets the default handler for this registry. All components with [`default`](ComponentCloneHandler::default_handler) handler, as well as any component that does not have an - /// explicitly registered clone function will use this handler. - /// - /// See [Handlers section of `EntityCloneBuilder`](crate::entity::EntityCloneBuilder#handlers) to understand how this affects handler priority. - pub fn set_default_handler(&mut self, handler: ComponentCloneFn) { - self.default_handler = handler; - } - - /// Returns the currently registered default handler. - pub fn get_default_handler(&self) -> ComponentCloneFn { - self.default_handler + Self { + entity_handler: ComponentCloneHandlerKind::Custom(handler), + world_handler: None, + } } - /// Sets a handler for a specific component. - /// - /// See [Handlers section of `EntityCloneBuilder`](crate::entity::EntityCloneBuilder#handlers) to understand how this affects handler priority. - pub fn set_component_handler(&mut self, id: ComponentId, handler: ComponentCloneHandler) { - if id.0 >= self.handlers.len() { - self.handlers.resize(id.0 + 1, None); + pub fn with_world_clone_handler(self, handler: Self) -> Self { + Self { + entity_handler: self.entity_handler, + world_handler: Some(handler.entity_handler), } - self.handlers[id.0] = handler.0; } - /// Checks if the specified component is registered. If not, the component will use the default global handler. - /// - /// This will return an incorrect result if `id` did not come from the same world as `self`. - pub fn is_handler_registered(&self, id: ComponentId) -> bool { - self.handlers.get(id.0).is_some_and(Option::is_some) + pub(crate) fn get_entity_handler(&self) -> ComponentCloneHandlerKind { + self.entity_handler } - /// Gets a handler to clone a component. This can be one of the following: - /// - Custom clone function for this specific component. - /// - Default global handler. - /// - A [`component_clone_ignore`] (no cloning). - /// - /// This will return an incorrect result if `id` did not come from the same world as `self`. - pub fn get_handler(&self, id: ComponentId) -> ComponentCloneFn { - match self.handlers.get(id.0) { - Some(Some(handler)) => *handler, - Some(None) | None => self.default_handler, - } + pub(crate) fn get_world_handler(&self) -> Option { + self.world_handler } -} -impl Default for ComponentCloneHandlers { - fn default() -> Self { + pub(crate) unsafe fn clone_unchecked(&self) -> Self { Self { - handlers: Default::default(), - #[cfg(feature = "bevy_reflect")] - default_handler: component_clone_via_reflect, - #[cfg(not(feature = "bevy_reflect"))] - default_handler: component_clone_ignore, + entity_handler: self.entity_handler.clone(), + world_handler: self.world_handler.clone(), } } } /// Stores metadata associated with each kind of [`Component`] in a given [`World`]. -#[derive(Debug, Default, Clone)] +#[derive(Debug, Clone)] pub struct Components { components: Vec, indices: TypeIdMap, resource_indices: TypeIdMap, - component_clone_handlers: ComponentCloneHandlers, + default_component_clone_handler: ComponentCloneFn, +} + +impl Default for Components { + fn default() -> Self { + Self { + components: Default::default(), + indices: Default::default(), + resource_indices: Default::default(), + #[cfg(feature = "bevy_reflect")] + default_component_clone_handler: component_clone_via_reflect, + #[cfg(not(feature = "bevy_reflect"))] + default_component_clone_handler: component_clone_ignore, + } + } } impl Components { @@ -1148,9 +1177,6 @@ impl Components { let info = &mut self.components[id.index()]; T::register_component_hooks(&mut info.hooks); info.required_components = required_components; - let clone_handler = T::get_component_clone_handler(); - self.component_clone_handlers - .set_component_handler(id, clone_handler); } id } @@ -1513,16 +1539,6 @@ impl Components { .map(|info| &mut info.required_by) } - /// Retrieves the [`ComponentCloneHandlers`]. Can be used to get clone functions for components. - pub fn get_component_clone_handlers(&self) -> &ComponentCloneHandlers { - &self.component_clone_handlers - } - - /// Retrieves a mutable reference to the [`ComponentCloneHandlers`]. Can be used to set and update clone functions for components. - pub fn get_component_clone_handlers_mut(&mut self) -> &mut ComponentCloneHandlers { - &mut self.component_clone_handlers - } - /// Type-erased equivalent of [`Components::component_id()`]. #[inline] pub fn get_id(&self, type_id: TypeId) -> Option { @@ -1609,14 +1625,11 @@ impl Components { #[inline] pub fn register_resource(&mut self) -> ComponentId { // SAFETY: The [`ComponentDescriptor`] matches the [`TypeId`] - let component_id = unsafe { + unsafe { self.get_or_register_resource_with(TypeId::of::(), || { ComponentDescriptor::new_resource::() }) - }; - self.component_clone_handlers - .set_component_handler(component_id, T::get_component_clone_handler()); - component_id + } } /// Registers a [`Resource`] described by `descriptor`. @@ -1680,6 +1693,25 @@ impl Components { pub fn iter(&self) -> impl Iterator + '_ { self.components.iter() } + + /// Sets the default handler for this registry. All components with [`default`](ComponentCloneHandler::default_handler) handler, as well as any component that does not have an + /// explicitly registered clone function will use this handler. + /// + /// See [Handlers section of `EntityCloneBuilder`](crate::entity::EntityCloneBuilder#handlers) to understand how this affects handler priority. + pub fn set_default_clone_handler(&mut self, handler: ComponentCloneFn) { + self.default_component_clone_handler = handler; + } + + /// Returns the currently registered default handler. + pub fn get_default_clone_handler(&self) -> ComponentCloneFn { + self.default_component_clone_handler + } + + pub fn set_clone_handler(&mut self, component_id: ComponentId, handler: ComponentCloneHandler) { + if let Some(info) = self.components.get_mut(component_id.0) { + info.descriptor.clone_handler = handler; + } + } } /// A value that tracks when a system ran relative to other systems. diff --git a/crates/bevy_ecs/src/entity/clone_entities.rs b/crates/bevy_ecs/src/entity/clone_entities.rs index 4bf1b99f57ada..bf26dc319b31a 100644 --- a/crates/bevy_ecs/src/entity/clone_entities.rs +++ b/crates/bevy_ecs/src/entity/clone_entities.rs @@ -16,7 +16,10 @@ use alloc::sync::Arc; use crate::{ bundle::Bundle, - component::{Component, ComponentCloneHandler, ComponentId, ComponentInfo, Components}, + component::{ + Component, ComponentCloneHandler, ComponentCloneHandlerKind, ComponentId, ComponentInfo, + Components, + }, entity::{Entities, Entity}, query::DebugCheckedUnwrap, system::Commands, @@ -53,25 +56,26 @@ pub struct ComponentCloneCtx<'a, 'b> { impl<'a, 'b> ComponentCloneCtx<'a, 'b> { pub unsafe fn new_for_component( - component_id: ComponentId, + component_info: &'a ComponentInfo, source_component_ptr: Ptr<'a>, target_component_ptr: NonNull, world: &'a World, + #[cfg(feature = "bevy_reflect")] type_registry: Option<&'a crate::reflect::AppTypeRegistry>, ) -> Self { let components = world.components(); let target_write_buffer = ComponentWriteBuffer::Ptr(target_component_ptr); Self { - component_id, + component_id: component_info.id(), source_component_ptr, target_write_buffer, target_component_written: false, - component_info: components.get_info_unchecked(component_id), + component_info, entities: world.entities(), command_queue: None, components, entity_cloner: None, #[cfg(feature = "bevy_reflect")] - type_registry: world.get_resource::(), + type_registry, } } @@ -297,26 +301,6 @@ pub struct EntityCloner { move_components: bool, } -pub(crate) unsafe fn clone_component_world( - component_id: ComponentId, - source_component_ptr: Ptr, - target_component_ptr: NonNull, - world: &World, -) -> bool { - let handlers = world.components().get_component_clone_handlers(); - let handler = handlers.get_handler(component_id); - let mut ctx = ComponentCloneCtx::new_for_component( - component_id, - source_component_ptr, - target_component_ptr, - world, - ); - - handler(&world, &mut ctx); - - ctx.target_component_written() -} - impl EntityCloner { /// Clones and inserts components from the `source` entity into `target` entity using the stored configuration. pub fn clone_entity(&mut self, world: &mut World) { @@ -331,17 +315,32 @@ impl EntityCloner { let mut component_ids: Vec = Vec::with_capacity(archetype.component_count()); let mut component_data_ptrs: Vec = Vec::with_capacity(archetype.component_count()); + fn copy_clone_handler(_world: &World, ctx: &mut ComponentCloneCtx) { + let count = ctx.component_info().layout().size(); + unsafe { + ctx.write_target_component_ptr(|source_ptr, target_ptr| { + core::ptr::copy_nonoverlapping(source_ptr.as_ptr(), target_ptr.as_ptr(), count); + true + }) + } + } + for component in archetype.components() { if !self.is_cloning_allowed(&component) { continue; } + let component_info = unsafe { components.get_info_unchecked(component) }; - let global_handlers = components.get_component_clone_handlers(); - let handler = match self.clone_handlers_overrides.get(&component) { - Some(handler) => handler - .get_handler() - .unwrap_or_else(|| global_handlers.get_default_handler()), - None => global_handlers.get_handler(component), + let handler = match self + .clone_handlers_overrides + .get(&component) + .unwrap_or_else(|| component_info.clone_handler()) + .get_entity_handler() + { + ComponentCloneHandlerKind::Default => components.get_default_clone_handler(), + ComponentCloneHandlerKind::Ignore => continue, + ComponentCloneHandlerKind::Copy => copy_clone_handler, + ComponentCloneHandlerKind::Custom(handler) => handler, }; // SAFETY: @@ -357,7 +356,7 @@ impl EntityCloner { ComponentCloneCtx { command_queue: Some(command_queue.get_raw()), component_id: component, - component_info: components.get_info_unchecked(component), + component_info, components, entities: world.entities(), entity_cloner: Some(self), @@ -724,9 +723,7 @@ mod tests { world.register_component::(); let id = world.component_id::().unwrap(); - world - .get_component_clone_handlers_mut() - .set_component_handler(id, ComponentCloneHandler::reflect_handler()); + world.set_component_clone_handler(id, ComponentCloneHandler::reflect_handler()); let component = A { field: 5 }; @@ -774,10 +771,9 @@ mod tests { let a_id = world.register_component::(); let b_id = world.register_component::(); let c_id = world.register_component::(); - let handlers = world.get_component_clone_handlers_mut(); - handlers.set_component_handler(a_id, ComponentCloneHandler::reflect_handler()); - handlers.set_component_handler(b_id, ComponentCloneHandler::reflect_handler()); - handlers.set_component_handler(c_id, ComponentCloneHandler::reflect_handler()); + world.set_component_clone_handler(a_id, ComponentCloneHandler::reflect_handler()); + world.set_component_clone_handler(b_id, ComponentCloneHandler::reflect_handler()); + world.set_component_clone_handler(c_id, ComponentCloneHandler::reflect_handler()); let component_a = A { field: 5, @@ -827,9 +823,10 @@ mod tests { } let a_id = world.register_component::(); - let handlers = world.get_component_clone_handlers_mut(); - handlers - .set_component_handler(a_id, ComponentCloneHandler::custom_handler(test_handler)); + world.set_component_clone_handler( + a_id, + ComponentCloneHandler::custom_handler(test_handler), + ); let e = world.spawn(A).id(); let e_clone = world.spawn_empty().id(); @@ -883,9 +880,8 @@ mod tests { let mut world = World::default(); let a_id = world.register_component::(); let b_id = world.register_component::(); - let handlers = world.get_component_clone_handlers_mut(); - handlers.set_component_handler(a_id, ComponentCloneHandler::reflect_handler()); - handlers.set_component_handler(b_id, ComponentCloneHandler::reflect_handler()); + world.set_component_clone_handler(a_id, ComponentCloneHandler::reflect_handler()); + world.set_component_clone_handler(b_id, ComponentCloneHandler::reflect_handler()); // No AppTypeRegistry let e = world.spawn((A, B)).id(); @@ -926,6 +922,35 @@ mod tests { assert!(world.get::(e_clone).is_some_and(|c| *c == component)); } + #[test] + fn clone_entity_using_copy() { + #[derive(Component, Copy, PartialEq, Eq, Debug)] + struct A { + field: usize, + } + + impl Clone for A { + #[expect( + clippy::non_canonical_clone_impl, + reason = "This is required to check that Copy implementation is selected instead of Clone" + )] + fn clone(&self) -> Self { + Self { field: 1 } + } + } + + let mut world = World::default(); + + let component = A { field: 5 }; + + let e = world.spawn(component).id(); + let e_clone = world.spawn_empty().id(); + + EntityCloneBuilder::new(&mut world).clone_entity(e, e_clone); + + assert_eq!(world.get::(e_clone), Some(&component)); + } + #[test] fn clone_entity_with_allow_filter() { #[derive(Component, Clone, PartialEq, Eq)] @@ -1105,16 +1130,11 @@ mod tests { layout, None, true, + ComponentCloneHandler::custom_handler(test_handler), ) }; let component_id = world.register_component_with_descriptor(descriptor); - let handlers = world.get_component_clone_handlers_mut(); - handlers.set_component_handler( - component_id, - ComponentCloneHandler::custom_handler(test_handler), - ); - let mut entity = world.spawn_empty(); let data = [5u8; COMPONENT_SIZE]; diff --git a/crates/bevy_ecs/src/observer/entity_observer.rs b/crates/bevy_ecs/src/observer/entity_observer.rs index ea20415172ab0..cc62792880f0a 100644 --- a/crates/bevy_ecs/src/observer/entity_observer.rs +++ b/crates/bevy_ecs/src/observer/entity_observer.rs @@ -7,7 +7,7 @@ use crate::{ use alloc::vec::Vec; /// Tracks a list of entity observers for the [`Entity`] [`ObservedBy`] is added to. -#[derive(Default)] +#[derive(Default, Clone)] pub(crate) struct ObservedBy(pub(crate) Vec); impl Component for ObservedBy { @@ -45,6 +45,7 @@ impl Component for ObservedBy { fn get_component_clone_handler() -> ComponentCloneHandler { ComponentCloneHandler::ignore() + .with_world_clone_handler(ComponentCloneHandler::clone_handler::()) } } diff --git a/crates/bevy_ecs/src/storage/blob_array.rs b/crates/bevy_ecs/src/storage/blob_array.rs index 86315386a8d1f..a221e89e13a22 100644 --- a/crates/bevy_ecs/src/storage/blob_array.rs +++ b/crates/bevy_ecs/src/storage/blob_array.rs @@ -475,6 +475,23 @@ impl BlobArray { drop(value); } } + + pub unsafe fn copy_from_unchecked(&mut self, other: &Self, len: usize) { + if other.is_zst() { + return; + } + let num_bytes_to_copy = array_layout_unchecked(&other.layout(), len).size(); + debug_assert!( + num_bytes_to_copy + <= array_layout(&other.layout(), self.capacity) + .expect("Calculating capacity to copy to BlobVec failed") + .size() + ); + debug_assert!(other.layout() == self.layout()); + let source_ptr = other.get_ptr().as_ptr(); + let target_ptr = self.get_ptr_mut().as_ptr(); + core::ptr::copy_nonoverlapping(source_ptr, target_ptr, num_bytes_to_copy); + } } #[cfg(test)] diff --git a/crates/bevy_ecs/src/storage/blob_vec.rs b/crates/bevy_ecs/src/storage/blob_vec.rs index 61a2a2e4fb993..5cf301be5d25f 100644 --- a/crates/bevy_ecs/src/storage/blob_vec.rs +++ b/crates/bevy_ecs/src/storage/blob_vec.rs @@ -406,6 +406,27 @@ impl BlobVec { } } } + + pub unsafe fn copy_from_unchecked(&mut self, other: &Self) { + // Skip ZSTs + if other.item_layout.size() == 0 { + return; + } + let len = other.len(); + // SAFETY: This must be valid because other is a valid BlobVec + let num_bytes_to_copy = array_layout_unchecked(&other.layout(), other.len()).size(); + debug_assert!( + num_bytes_to_copy + <= array_layout(&other.layout(), self.capacity) + .expect("Calculating capacity to copy to BlobVec failed") + .size() + ); + debug_assert!(other.layout() == self.layout()); + let source_ptr = other.get_ptr().as_ptr(); + let target_ptr = self.get_ptr_mut().as_ptr(); + core::ptr::copy_nonoverlapping(source_ptr, target_ptr, num_bytes_to_copy); + self.len = len; + } } impl Drop for BlobVec { diff --git a/crates/bevy_ecs/src/storage/mod.rs b/crates/bevy_ecs/src/storage/mod.rs index c9c4faa3acad3..15b2a60ec7f56 100644 --- a/crates/bevy_ecs/src/storage/mod.rs +++ b/crates/bevy_ecs/src/storage/mod.rs @@ -34,7 +34,11 @@ pub use resource::*; pub use sparse_set::*; pub use table::*; -use crate::world::{error::WorldCloneError, World}; +use crate::{ + component::{ComponentInfo, Components}, + entity::ComponentCloneCtx, + world::{error::WorldCloneError, World}, +}; /// The raw data stores of a [`World`](crate::world::World) #[derive(Default)] @@ -49,13 +53,35 @@ pub struct Storages { pub non_send_resources: Resources, } +// &impl Fn(&ComponentInfo, Ptr, NonNull); + impl Storages { - pub(crate) unsafe fn try_clone<'a>(&self, world: &World) -> Result { + pub(crate) unsafe fn try_clone( + &self, + world: &World, + #[cfg(feature = "bevy_reflect")] type_registry: Option<&crate::reflect::AppTypeRegistry>, + ) -> Result { Ok(Storages { - sparse_sets: self.sparse_sets.try_clone(world)?, - tables: self.tables.try_clone(world)?, - resources: self.resources.try_clone(world)?, - non_send_resources: self.non_send_resources.try_clone(world)?, + sparse_sets: self.sparse_sets.try_clone( + world, + #[cfg(feature = "bevy_reflect")] + type_registry, + )?, + tables: self.tables.try_clone( + world, + #[cfg(feature = "bevy_reflect")] + type_registry, + )?, + resources: self.resources.try_clone( + world, + #[cfg(feature = "bevy_reflect")] + type_registry, + )?, + non_send_resources: self.non_send_resources.try_clone( + world, + #[cfg(feature = "bevy_reflect")] + type_registry, + )?, }) } } diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index 7081f4f21d371..6dfd4c413558e 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -1,8 +1,11 @@ use crate::{ archetype::ArchetypeComponentId, change_detection::{MaybeLocation, MaybeUnsafeCellLocation, MutUntyped, TicksMut}, - component::{ComponentId, ComponentInfo, ComponentTicks, Components, Tick, TickCells}, - entity::clone_component_world, + component::{ + ComponentCloneHandlerKind, ComponentId, ComponentInfo, ComponentTicks, Components, Tick, + TickCells, + }, + entity::ComponentCloneCtx, storage::{blob_vec::BlobVec, SparseSet}, world::{error::WorldCloneError, World}, }; @@ -10,7 +13,7 @@ use alloc::string::String; use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; #[cfg(feature = "track_location")] use core::panic::Location; -use core::{cell::UnsafeCell, mem::ManuallyDrop}; +use core::{cell::UnsafeCell, mem::ManuallyDrop, ptr::NonNull}; #[cfg(feature = "std")] use std::thread::ThreadId; @@ -322,22 +325,47 @@ impl ResourceData { &self, component_info: &ComponentInfo, world: &World, + #[cfg(feature = "bevy_reflect")] type_registry: Option<&crate::reflect::AppTypeRegistry>, ) -> Result { if !self.try_validate_access() { return Err(WorldCloneError::NonSendResourceCloned(component_info.id())); } let mut data = BlobVec::new(component_info.layout(), component_info.drop(), 1); - let is_initialized = data.try_initialize_next(|target_component_ptr| { - let source_component_ptr = self.data.get_unchecked(Self::ROW); - clone_component_world( - component_info.id(), - source_component_ptr, - target_component_ptr, - world, - ) - }); - if !is_initialized { - return Err(WorldCloneError::FailedToCloneComponent(component_info.id())); + + let handler = match component_info + .clone_handler() + .get_world_handler() + .unwrap_or_else(|| component_info.clone_handler().get_entity_handler()) + { + ComponentCloneHandlerKind::Default => { + Some(world.components.get_default_clone_handler()) + } + ComponentCloneHandlerKind::Ignore => { + return Err(WorldCloneError::ComponentCantBeCloned(component_info.id())) + } + ComponentCloneHandlerKind::Copy => { + data.copy_from_unchecked(&self.data); + None + } + ComponentCloneHandlerKind::Custom(handler) => Some(handler), + }; + + if let Some(handler) = handler { + let is_initialized = data.try_initialize_next(|target_component_ptr| { + let source_component_ptr = self.data.get_unchecked(Self::ROW); + let mut ctx = ComponentCloneCtx::new_for_component( + component_info, + source_component_ptr, + target_component_ptr, + world, + type_registry, + ); + handler(world, &mut ctx); + ctx.target_component_written() + }); + if !is_initialized { + return Err(WorldCloneError::FailedToCloneComponent(component_info.id())); + } } Ok(Self { @@ -452,12 +480,22 @@ impl Resources { } } - pub(crate) unsafe fn try_clone(&self, world: &World) -> Result { + pub(crate) unsafe fn try_clone( + &self, + world: &World, + #[cfg(feature = "bevy_reflect")] type_registry: Option<&crate::reflect::AppTypeRegistry>, + ) -> Result { let mut resources = SparseSet::with_capacity(self.resources.len()); + let components = world.components(); for (component_id, res) in self.resources.iter() { resources.insert( *component_id, - res.try_clone(world.components().get_info_unchecked(*component_id), world)?, + res.try_clone( + components.get_info_unchecked(*component_id), + world, + #[cfg(feature = "bevy_reflect")] + type_registry, + )?, ); } Ok(Self { resources }) diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 1d84bbc094377..1fbc4765614ff 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -1,7 +1,7 @@ use crate::{ change_detection::MaybeUnsafeCellLocation, - component::{ComponentId, ComponentInfo, ComponentTicks, Tick, TickCells}, - entity::Entity, + component::{ComponentId, ComponentInfo, ComponentTicks, Components, Tick, TickCells}, + entity::{ComponentCloneCtx, Entity}, storage::{Column, TableRow}, world::{error::WorldCloneError, World}, }; @@ -141,13 +141,21 @@ impl ComponentSparseSet { } } - pub(crate) unsafe fn try_clone<'a>( + pub(crate) unsafe fn try_clone( &self, component_info: &ComponentInfo, world: &World, + #[cfg(feature = "bevy_reflect")] type_registry: Option<&crate::reflect::AppTypeRegistry>, ) -> Result { Ok(Self { - dense: unsafe { self.dense.try_clone(component_info, world)? }, + dense: unsafe { + self.dense.try_clone( + component_info, + world, + #[cfg(feature = "bevy_reflect")] + type_registry, + )? + }, entities: self.entities.clone(), sparse: self.sparse.clone(), }) @@ -623,17 +631,24 @@ impl SparseSets { self.sets.is_empty() } - pub(crate) unsafe fn try_clone<'a>(&self, world: &World) -> Result { + pub(crate) unsafe fn try_clone( + &self, + world: &World, + #[cfg(feature = "bevy_reflect")] type_registry: Option<&crate::reflect::AppTypeRegistry>, + ) -> Result { let mut sets = SparseSet::with_capacity(self.sets.len()); let components = world.components(); for (component_id, set) in self.sets.iter() { let component_info = components.get_info_unchecked(*component_id); - let set = set.try_clone(component_info, world)?; + let set = set.try_clone( + component_info, + world, + #[cfg(feature = "bevy_reflect")] + type_registry, + )?; sets.insert(*component_id, set); } - let mut sparse_sets = SparseSets::default(); - sparse_sets.sets = sets; - Ok(sparse_sets) + Ok(SparseSets { sets }) } /// An Iterator visiting all ([`ComponentId`], [`ComponentSparseSet`]) pairs. diff --git a/crates/bevy_ecs/src/storage/table/column.rs b/crates/bevy_ecs/src/storage/table/column.rs index 764318c27a5e0..237c7a8e32378 100644 --- a/crates/bevy_ecs/src/storage/table/column.rs +++ b/crates/bevy_ecs/src/storage/table/column.rs @@ -2,8 +2,8 @@ use core::ptr::NonNull; use super::*; use crate::{ - component::{self, TickCells}, - entity::clone_component_world, + component::{self, ComponentCloneHandlerKind, TickCells}, + entity::ComponentCloneCtx, storage::{blob_array::BlobArray, thin_array_ptr::ThinArrayPtr}, world::{error::WorldCloneError, World}, }; @@ -334,8 +334,9 @@ impl ThinColumn { pub(crate) unsafe fn try_clone( &self, component_info: &ComponentInfo, - world: &World, len: usize, + world: &World, + #[cfg(feature = "bevy_reflect")] type_registry: Option<&crate::reflect::AppTypeRegistry>, ) -> Result { let mut column = Self::with_capacity(component_info, len); column.added_ticks = ThinArrayPtr::from( @@ -362,16 +363,35 @@ impl ThinColumn { .collect::>(), ); } + + let handler = match component_info + .clone_handler() + .get_world_handler() + .unwrap_or_else(|| component_info.clone_handler().get_entity_handler()) + { + ComponentCloneHandlerKind::Default => world.components.get_default_clone_handler(), + ComponentCloneHandlerKind::Ignore => { + return Err(WorldCloneError::ComponentCantBeCloned(component_info.id())) + } + ComponentCloneHandlerKind::Copy => { + column.data.copy_from_unchecked(&self.data, len); + return Ok(column); + } + ComponentCloneHandlerKind::Custom(handler) => handler, + }; + for i in 0..len { let source_component_ptr = self.data.get_unchecked(i); let target_component_ptr = column.data.get_unchecked_mut(i).into(); - let is_initialized = clone_component_world( - component_info.id(), + let mut ctx = ComponentCloneCtx::new_for_component( + component_info, source_component_ptr, target_component_ptr, world, + type_registry, ); - if !is_initialized { + handler(world, &mut ctx); + if !ctx.target_component_written() { return Err(WorldCloneError::FailedToCloneComponent(component_info.id())); } } @@ -742,6 +762,7 @@ impl Column { &self, component_info: &ComponentInfo, world: &World, + #[cfg(feature = "bevy_reflect")] type_registry: Option<&crate::reflect::AppTypeRegistry>, ) -> Result { let len = self.added_ticks.len(); let mut column = Self::with_capacity(component_info, len); @@ -763,16 +784,36 @@ impl Column { .map(|tick| UnsafeCell::new(tick.read())) .collect(); } + + let handler = match component_info + .clone_handler() + .get_world_handler() + .unwrap_or_else(|| component_info.clone_handler().get_entity_handler()) + { + ComponentCloneHandlerKind::Default => world.components.get_default_clone_handler(), + ComponentCloneHandlerKind::Ignore => { + return Err(WorldCloneError::ComponentCantBeCloned(component_info.id())) + } + ComponentCloneHandlerKind::Copy => { + column.data.copy_from_unchecked(&self.data); + return Ok(column); + } + ComponentCloneHandlerKind::Custom(handler) => handler, + }; + for i in 0..self.len() { let row = TableRow::from_usize(i); let source_component_ptr = self.get_data_unchecked(row); let is_initialized = column.data.try_initialize_next(|target_component_ptr| { - clone_component_world( - component_info.id(), + let mut ctx = ComponentCloneCtx::new_for_component( + component_info, source_component_ptr, target_component_ptr, world, - ) + type_registry, + ); + handler(world, &mut ctx); + ctx.target_component_written() }); if !is_initialized { return Err(WorldCloneError::FailedToCloneComponent(component_info.id())); diff --git a/crates/bevy_ecs/src/storage/table/mod.rs b/crates/bevy_ecs/src/storage/table/mod.rs index ad16d8c565f92..ce554ce01161a 100644 --- a/crates/bevy_ecs/src/storage/table/mod.rs +++ b/crates/bevy_ecs/src/storage/table/mod.rs @@ -1,7 +1,7 @@ use crate::{ change_detection::MaybeLocation, component::{ComponentId, ComponentInfo, ComponentTicks, Components, Tick}, - entity::Entity, + entity::{ComponentCloneCtx, Entity}, query::DebugCheckedUnwrap, storage::{blob_vec::BlobVec, ImmutableSparseSet, SparseSet}, world::{error::WorldCloneError, World}, @@ -17,6 +17,7 @@ use core::{ cell::UnsafeCell, num::NonZeroUsize, ops::{Index, IndexMut}, + ptr::NonNull, }; mod column; @@ -673,16 +674,23 @@ impl Table { .map(|col| col.data.get_unchecked(row.as_usize())) } - pub(crate) unsafe fn try_clone(&self, world: &World) -> Result { + pub(crate) unsafe fn try_clone( + &self, + world: &World, + #[cfg(feature = "bevy_reflect")] type_registry: Option<&crate::reflect::AppTypeRegistry>, + ) -> Result { let mut columns = SparseSet::with_capacity(self.columns.len()); + let components = world.components(); let column_len = self.entities.len(); for (component_id, column) in self.columns.iter() { columns.insert( *component_id, column.try_clone( - world.components().get_info_unchecked(*component_id), - world, + components.get_info_unchecked(*component_id), column_len, + world, + #[cfg(feature = "bevy_reflect")] + type_registry, )?, ); } @@ -802,10 +810,18 @@ impl Tables { } } - pub(crate) unsafe fn try_clone(&self, world: &World) -> Result { + pub(crate) unsafe fn try_clone( + &self, + world: &World, + #[cfg(feature = "bevy_reflect")] type_registry: Option<&crate::reflect::AppTypeRegistry>, + ) -> Result { let mut tables = Vec::with_capacity(self.tables.len()); for table in &self.tables { - tables.push(table.try_clone(world)?) + tables.push(table.try_clone( + world, + #[cfg(feature = "bevy_reflect")] + type_registry, + )?) } Ok(Self { table_ids: self.table_ids.clone(), diff --git a/crates/bevy_ecs/src/world/error.rs b/crates/bevy_ecs/src/world/error.rs index 12552ec294f57..e6563d5f3fdcf 100644 --- a/crates/bevy_ecs/src/world/error.rs +++ b/crates/bevy_ecs/src/world/error.rs @@ -55,6 +55,8 @@ pub enum WorldCloneError { WorldIdExhausted, #[error("Component clone handler for component with ID {0:?} failed to clone the component")] FailedToCloneComponent(ComponentId), - #[error("Component clone handler for component with ID {0:?} failed to clone the component")] + #[error("")] NonSendResourceCloned(ComponentId), + #[error("")] + ComponentCantBeCloned(ComponentId), } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index e98c599f22a95..9b7fec38c230d 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -34,11 +34,11 @@ use crate::{ bundle::{Bundle, BundleInfo, BundleInserter, BundleSpawner, Bundles, InsertMode}, change_detection::{MutUntyped, TicksMut}, component::{ - Component, ComponentCloneHandlers, ComponentDescriptor, ComponentHooks, ComponentId, - ComponentInfo, ComponentTicks, Components, Mutable, RequiredComponents, + Component, ComponentCloneFn, ComponentCloneHandler, ComponentDescriptor, ComponentHooks, + ComponentId, ComponentInfo, ComponentTicks, Components, Mutable, RequiredComponents, RequiredComponentsError, Tick, }, - entity::{AllocAtWithoutReplacement, Entities, Entity, EntityLocation}, + entity::{AllocAtWithoutReplacement, ComponentCloneCtx, Entities, Entity, EntityLocation}, event::{Event, EventId, Events, SendBatchIds}, observer::Observers, query::{DebugCheckedUnwrap, QueryData, QueryFilter, QueryState}, @@ -207,7 +207,13 @@ impl World { pub fn try_clone(&self) -> Result { let id = WorldId::new().ok_or(error::WorldCloneError::WorldIdExhausted)?; - let storages = unsafe { self.storages.try_clone(self)? }; + let storages = unsafe { + self.storages.try_clone( + self, + #[cfg(feature = "bevy_reflect")] + self.get_resource::(), + )? + }; let world = World { id, @@ -219,8 +225,8 @@ impl World { observers: self.observers.clone(), removed_components: self.removed_components.clone(), change_tick: AtomicU32::new(self.change_tick.load(Ordering::Relaxed)), - last_change_tick: self.last_change_tick.clone(), - last_check_tick: self.last_check_tick.clone(), + last_change_tick: self.last_change_tick, + last_check_tick: self.last_check_tick, last_trigger_id: self.last_trigger_id, command_queue: RawCommandQueue::new(), }; @@ -3182,33 +3188,16 @@ impl World { unsafe { self.bundles.get(id).debug_checked_unwrap() } } - /// Retrieves a mutable reference to the [`ComponentCloneHandlers`]. Can be used to set and update clone functions for components. - /// - /// ``` - /// # use bevy_ecs::prelude::*; - /// use bevy_ecs::component::{ComponentId, ComponentCloneHandler}; - /// use bevy_ecs::entity::ComponentCloneCtx; - /// use bevy_ecs::world::DeferredWorld; - /// - /// fn custom_clone_handler( - /// _world: &mut DeferredWorld, - /// _ctx: &mut ComponentCloneCtx, - /// ) { - /// // Custom cloning logic for component - /// } - /// - /// #[derive(Component)] - /// struct ComponentA; - /// - /// let mut world = World::new(); - /// - /// let component_id = world.register_component::(); - /// - /// world.get_component_clone_handlers_mut() - /// .set_component_handler(component_id, ComponentCloneHandler::custom_handler(custom_clone_handler)) - /// ``` - pub fn get_component_clone_handlers_mut(&mut self) -> &mut ComponentCloneHandlers { - self.components.get_component_clone_handlers_mut() + pub fn set_default_component_clone_handler(&mut self, handler: ComponentCloneFn) { + self.components.set_default_clone_handler(handler); + } + + pub fn set_component_clone_handler( + &mut self, + component_id: ComponentId, + handler: ComponentCloneHandler, + ) { + self.components.set_clone_handler(component_id, handler) } } @@ -3742,7 +3731,7 @@ mod tests { use super::{FromWorld, World}; use crate::{ change_detection::DetectChangesMut, - component::{ComponentDescriptor, ComponentInfo, StorageType}, + component::{ComponentCloneHandler, ComponentDescriptor, ComponentInfo, StorageType}, entity::{Entity, EntityHashSet}, ptr::OwningPtr, system::Resource, @@ -4041,6 +4030,7 @@ mod tests { DROP_COUNT.fetch_add(1, Ordering::SeqCst); }), true, + ComponentCloneHandler::default_handler(), ) }; @@ -4404,7 +4394,12 @@ mod tests { alloc_value: Vec, } - #[derive(Component, Clone, PartialEq, Eq, Debug)] + #[derive(Component, Clone, Copy, PartialEq, Eq, Debug)] + struct CopyComp { + value: i32, + } + + #[derive(Component, Clone, Copy, PartialEq, Eq, Debug)] struct ZSTComp; #[derive(Resource, Clone, PartialEq, Eq, Debug)] @@ -4417,6 +4412,7 @@ mod tests { value: 5, alloc_value: vec![1, 2, 3, 4, 5], }; + let copy_comp = CopyComp { value: 10 }; let zst = ZSTComp; let res = Res { value: 1, @@ -4424,13 +4420,13 @@ mod tests { }; let mut world = World::default(); - let e_id = world.spawn((comp.clone(), zst.clone())).id(); + let e_id = world.spawn((comp.clone(), copy_comp, zst)).id(); world.insert_resource(res.clone()); let mut world2 = world.try_clone().unwrap(); - let mut query = world2.query::<(Entity, &Comp, &ZSTComp)>(); - assert_eq!(query.single(&world2), (e_id, &comp, &zst)); + let mut query = world2.query::<(Entity, &Comp, &CopyComp, &ZSTComp)>(); + assert_eq!(query.single(&world2), (e_id, &comp, ©_comp, &zst)); assert_eq!(world2.get_resource::(), Some(&res)); } } diff --git a/crates/bevy_hierarchy/src/components/children.rs b/crates/bevy_hierarchy/src/components/children.rs index 4780d31eb2e67..7b382b3d7fd0d 100644 --- a/crates/bevy_hierarchy/src/components/children.rs +++ b/crates/bevy_hierarchy/src/components/children.rs @@ -45,7 +45,15 @@ impl Component for Children { type Mutability = Mutable; fn get_component_clone_handler() -> ComponentCloneHandler { - ComponentCloneHandler::ignore() + // Custom handler to avoid adding `Clone` to Children + ComponentCloneHandler::ignore().with_world_clone_handler( + ComponentCloneHandler::custom_handler(|_world, ctx| { + let component = ctx.read_source_component::(); + if let Some(component) = component { + ctx.write_target_component(Self(component.0.clone())); + } + }), + ) } } diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index 4fc97aa914a24..43845796fd562 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -45,7 +45,15 @@ impl Component for Parent { type Mutability = Mutable; fn get_component_clone_handler() -> ComponentCloneHandler { - ComponentCloneHandler::ignore() + // Custom handler to avoid adding `Clone` to Parent + ComponentCloneHandler::ignore().with_world_clone_handler( + ComponentCloneHandler::custom_handler(|_world, ctx| { + let component = ctx.read_source_component::(); + if let Some(component) = component { + ctx.write_target_component(Self(component.0)); + } + }), + ) } } diff --git a/examples/ecs/dynamic.rs b/examples/ecs/dynamic.rs index b2138be85a7f7..f3fbf35d4b417 100644 --- a/examples/ecs/dynamic.rs +++ b/examples/ecs/dynamic.rs @@ -7,7 +7,9 @@ use std::{alloc::Layout, collections::HashMap, io::Write, ptr::NonNull}; use bevy::{ ecs::{ - component::{ComponentDescriptor, ComponentId, ComponentInfo, StorageType}, + component::{ + ComponentCloneHandler, ComponentDescriptor, ComponentId, ComponentInfo, StorageType, + }, query::QueryData, world::FilteredEntityMut, }, @@ -91,6 +93,7 @@ fn main() { Layout::array::(size).unwrap(), None, true, + ComponentCloneHandler::default_handler(), ) }); let Some(info) = world.components().get_info(id) else { diff --git a/examples/ecs/immutable_components.rs b/examples/ecs/immutable_components.rs index 2e9b54522cd3d..98877d02764dc 100644 --- a/examples/ecs/immutable_components.rs +++ b/examples/ecs/immutable_components.rs @@ -2,7 +2,7 @@ use bevy::{ ecs::{ - component::{ComponentDescriptor, ComponentId, StorageType}, + component::{ComponentCloneHandler, ComponentDescriptor, ComponentId, StorageType}, world::DeferredWorld, }, prelude::*, @@ -149,6 +149,7 @@ fn demo_3(world: &mut World) { Layout::array::(size).unwrap(), None, false, + ComponentCloneHandler::default_handler(), ) }; diff --git a/examples/stress_tests/many_components.rs b/examples/stress_tests/many_components.rs index 4bb87d322d5c7..0da3b7de46ebb 100644 --- a/examples/stress_tests/many_components.rs +++ b/examples/stress_tests/many_components.rs @@ -17,7 +17,7 @@ use bevy::{ DiagnosticPath, DiagnosticsPlugin, FrameTimeDiagnosticsPlugin, LogDiagnosticsPlugin, }, ecs::{ - component::{ComponentDescriptor, ComponentId, StorageType}, + component::{ComponentCloneHandler, ComponentDescriptor, ComponentId, StorageType}, system::QueryParamBuilder, world::FilteredEntityMut, }, @@ -93,6 +93,7 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { Layout::new::(), None, true, // is mutable + ComponentCloneHandler::default_handler(), ) }, ) From fbf2f5980d82bc7b94335d2830a42b55b632d7cf Mon Sep 17 00:00:00 2001 From: eugineerd Date: Fri, 10 Jan 2025 18:54:21 +0000 Subject: [PATCH 07/15] Add some documentation --- crates/bevy_ecs/src/component.rs | 28 ++++-- crates/bevy_ecs/src/entity/clone_entities.rs | 95 +++++++++++-------- crates/bevy_ecs/src/entity/mod.rs | 2 +- .../bevy_ecs/src/observer/entity_observer.rs | 2 +- crates/bevy_ecs/src/storage/blob_array.rs | 9 ++ crates/bevy_ecs/src/storage/blob_vec.rs | 13 +++ crates/bevy_ecs/src/storage/mod.rs | 16 ++-- crates/bevy_ecs/src/storage/resource.rs | 24 ++++- crates/bevy_ecs/src/storage/sparse_set.rs | 33 ++++--- crates/bevy_ecs/src/storage/table/column.rs | 24 ++++- crates/bevy_ecs/src/storage/table/mod.rs | 16 +++- crates/bevy_ecs/src/world/error.rs | 16 +++- crates/bevy_ecs/src/world/mod.rs | 17 +++- crates/bevy_hierarchy/src/hierarchy.rs | 2 +- 14 files changed, 206 insertions(+), 91 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index c3639223de9b8..1fed4cebe9372 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -874,12 +874,12 @@ impl Clone for ComponentDescriptor { fn clone(&self) -> Self { Self { name: self.name.clone(), - storage_type: self.storage_type.clone(), - is_send_and_sync: self.is_send_and_sync.clone(), - type_id: self.type_id.clone(), - layout: self.layout.clone(), - drop: self.drop.clone(), - mutable: self.mutable.clone(), + storage_type: self.storage_type, + is_send_and_sync: self.is_send_and_sync, + type_id: self.type_id, + layout: self.layout, + drop: self.drop, + mutable: self.mutable, // SAFETY: This clone handler will be used with the same component clone_handler: unsafe { self.clone_handler.clone_unchecked() }, } @@ -1006,6 +1006,8 @@ impl ComponentDescriptor { self.mutable } + /// Return currently set [`ComponentCloneHandler`] for this component. + #[inline] pub fn clone_handler(&self) -> &ComponentCloneHandler { &self.clone_handler } @@ -1056,6 +1058,10 @@ impl ComponentCloneHandler { } } + /// Set clone handler to use copy. + /// + /// # Safety + /// - Must ensure that this handler is used only with the same type as T pub unsafe fn copy_handler() -> Self { Self { entity_handler: ComponentCloneHandlerKind::Copy, @@ -1080,6 +1086,9 @@ impl ComponentCloneHandler { } } + /// Set a different component clone handler when performing world cloning instead of entity cloning. + /// + /// Provided handler should generally be infallible and access only source component data. pub fn with_world_clone_handler(self, handler: Self) -> Self { Self { entity_handler: self.entity_handler, @@ -1095,10 +1104,12 @@ impl ComponentCloneHandler { self.world_handler } + /// # Safety + /// - Cloned handler can be used only with the same component as the one used to get this handler. pub(crate) unsafe fn clone_unchecked(&self) -> Self { Self { - entity_handler: self.entity_handler.clone(), - world_handler: self.world_handler.clone(), + entity_handler: self.entity_handler, + world_handler: self.world_handler, } } } @@ -1707,6 +1718,7 @@ impl Components { self.default_component_clone_handler } + /// Sets [`ComponentCloneHandler`] for component with specified [`ComponentId`] pub fn set_clone_handler(&mut self, component_id: ComponentId, handler: ComponentCloneHandler) { if let Some(info) = self.components.get_mut(component_id.0) { info.descriptor.clone_handler = handler; diff --git a/crates/bevy_ecs/src/entity/clone_entities.rs b/crates/bevy_ecs/src/entity/clone_entities.rs index bf26dc319b31a..41c891abd48b6 100644 --- a/crates/bevy_ecs/src/entity/clone_entities.rs +++ b/crates/bevy_ecs/src/entity/clone_entities.rs @@ -55,7 +55,14 @@ pub struct ComponentCloneCtx<'a, 'b> { } impl<'a, 'b> ComponentCloneCtx<'a, 'b> { - pub unsafe fn new_for_component( + /// Create a new [`ComponentCloneCtx`] for specified component. + /// + /// # Safety + /// - `component_info` must be from the `world`. + /// - `source_component_ptr` must point to a valid component described by `component_info`. + /// - `target_component_ptr` must have enough space allocated to write component described by `component_info`. + /// - `type_registry` must be from the `world`. + pub(crate) unsafe fn new_for_component( component_info: &'a ComponentInfo, source_component_ptr: Ptr<'a>, target_component_ptr: NonNull, @@ -84,12 +91,12 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { self.target_component_written } - /// Returns the current source entity. + /// Returns the current source entity (if available in current context). pub fn source(&self) -> Option { self.entity_cloner.map(|c| c.source) } - /// Returns the current target entity. + /// Returns the current target entity (if available in current context). pub fn target(&self) -> Option { self.entity_cloner.map(|c| c.target) } @@ -109,10 +116,12 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { self.components } + /// Returns an instance of [`Commands`] (if available for current context). pub fn commands(&self) -> Option> { - self.command_queue - .as_ref() - .map(|queue| unsafe { Commands::new_raw_from_entities(queue.clone(), self.entities) }) + self.command_queue.as_ref().map(|queue| { + // SAFETY: queue outlives 'a + unsafe { Commands::new_raw_from_entities(queue.clone(), self.entities) } + }) } /// Returns a reference to the component on the source entity. @@ -194,6 +203,8 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { { panic!("TypeId of component '{short_name}' does not match source component TypeId") }; + // SAFETY: + // - target_ptr and component have layout described by component_info unsafe { self.write_target_component_impl(|target_ptr| { target_ptr.cast::().write(component); @@ -230,7 +241,6 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { /// # Panics /// This will panic if: /// - World does not have [`AppTypeRegistry`](`crate::reflect::AppTypeRegistry`). - /// - Component does not implement [`ReflectFromPtr`](bevy_reflect::ReflectFromPtr). /// - Source component does not have [`TypeId`]. /// - Passed component's [`TypeId`] does not match source component [`TypeId`]. /// - Component has already been written once. @@ -249,6 +259,9 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { } let layout = self.component_info.layout(); let component_data_ptr = Box::into_raw(component).cast::(); + // SAFETY: + // - component_data_ptr and target_ptr don't overlap + // - target_ptr and component_data_ptr point to data with layout described by component_info unsafe { self.write_target_component_impl(|target_ptr| { core::ptr::copy_nonoverlapping( @@ -262,18 +275,23 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { } } - /// Return a reference to this context's `EntityCloner` instance. + /// Return a reference to this context's `EntityCloner` instance (if available for current context). /// /// This can be used to issue clone commands using the same cloning configuration: /// ``` - /// # use bevy_ecs::world::{DeferredWorld, World}; + /// # use bevy_ecs::world::World; /// # use bevy_ecs::entity::ComponentCloneCtx; - /// fn clone_handler(world: &mut DeferredWorld, ctx: &mut ComponentCloneCtx) { - /// let another_target = world.commands().spawn_empty().id(); - /// let mut entity_cloner = ctx - /// .entity_cloner() - /// .with_source_and_target(ctx.source(), another_target); - /// world.commands().queue(move |world: &mut World| { + /// fn clone_handler(_world: &World, ctx: &mut ComponentCloneCtx) { + /// let Some(commands) = ctx.commands() else { return }; + /// let Some(mut entity_cloner) = ctx.entity_cloner() + /// .and_then(|entity_cloner| { + /// let another_target = commands.spawn_empty().id(); + /// entity_cloner.with_source_and_target(ctx.source(), another_target)) + /// } + /// else { + /// return + /// }; + /// commands.queue(move |world: &mut World| { /// entity_cloner.clone_entity(world); /// }); /// } @@ -310,6 +328,8 @@ impl EntityCloner { let components = world.components(); let archetype = source_entity.archetype(); let mut command_queue = CommandQueue::default(); + #[cfg(feature = "bevy_reflect")] + let type_registry = world.get_resource::(); let component_data = Bump::new(); let mut component_ids: Vec = Vec::with_capacity(archetype.component_count()); @@ -317,11 +337,15 @@ impl EntityCloner { fn copy_clone_handler(_world: &World, ctx: &mut ComponentCloneCtx) { let count = ctx.component_info().layout().size(); + // SAFETY: + // - This handler is only used for copyable components + // - source_ptr and target_ptr don't overlap + // - function passed to write_target_component_ptr writes valid data of proper type to target_ptr unsafe { ctx.write_target_component_ptr(|source_ptr, target_ptr| { core::ptr::copy_nonoverlapping(source_ptr.as_ptr(), target_ptr.as_ptr(), count); true - }) + }); } } @@ -329,6 +353,7 @@ impl EntityCloner { if !self.is_cloning_allowed(&component) { continue; } + // SAFETY: component is a valid component from the same world as components let component_info = unsafe { components.get_info_unchecked(component) }; let handler = match self @@ -349,26 +374,21 @@ impl EntityCloner { let source_component_ptr = unsafe { source_entity.get_by_id(component).debug_checked_unwrap() }; - // SAFETY: - // - `components` and `component` are from the same world - // - `source_component_ptr` is valid and points to the same type as represented by `component` - let mut ctx = unsafe { - ComponentCloneCtx { - command_queue: Some(command_queue.get_raw()), - component_id: component, - component_info, - components, - entities: world.entities(), - entity_cloner: Some(self), - source_component_ptr, - target_component_written: false, - target_write_buffer: ComponentWriteBuffer::BumpWriteBuffer(BumpWriteBuffer { - target_components_buffer: &component_data, - target_components_ptrs: &mut component_data_ptrs, - }), - #[cfg(feature = "bevy_reflect")] - type_registry: world.get_resource::(), - } + let mut ctx = ComponentCloneCtx { + command_queue: Some(command_queue.get_raw()), + component_id: component, + component_info, + components, + entities: world.entities(), + entity_cloner: Some(self), + source_component_ptr, + target_component_written: false, + target_write_buffer: ComponentWriteBuffer::BumpWriteBuffer(BumpWriteBuffer { + target_components_buffer: &component_data, + target_components_ptrs: &mut component_data_ptrs, + }), + #[cfg(feature = "bevy_reflect")] + type_registry, }; (handler)(world, &mut ctx); @@ -693,8 +713,7 @@ mod tests { self as bevy_ecs, component::{Component, ComponentCloneHandler, ComponentDescriptor, StorageType}, entity::EntityCloneBuilder, - system::Resource, - world::{DeferredWorld, World}, + world::World, }; use alloc::vec::Vec; use bevy_ecs_macros::require; diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 53a38740decb9..417cc3fc65c79 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -585,7 +585,7 @@ impl Clone for Entities { meta: self.meta.clone(), pending: self.pending.clone(), free_cursor: AtomicIdCursor::new(self.free_cursor.load(Ordering::Relaxed)), - len: self.len.clone(), + len: self.len, } } } diff --git a/crates/bevy_ecs/src/observer/entity_observer.rs b/crates/bevy_ecs/src/observer/entity_observer.rs index cc62792880f0a..8e2181f451302 100644 --- a/crates/bevy_ecs/src/observer/entity_observer.rs +++ b/crates/bevy_ecs/src/observer/entity_observer.rs @@ -2,7 +2,7 @@ use crate::{ component::{Component, ComponentCloneHandler, ComponentHooks, Mutable, StorageType}, entity::{ComponentCloneCtx, Entity, EntityCloneBuilder}, observer::ObserverState, - world::{DeferredWorld, World}, + world::World, }; use alloc::vec::Vec; diff --git a/crates/bevy_ecs/src/storage/blob_array.rs b/crates/bevy_ecs/src/storage/blob_array.rs index a221e89e13a22..37689f41e30fe 100644 --- a/crates/bevy_ecs/src/storage/blob_array.rs +++ b/crates/bevy_ecs/src/storage/blob_array.rs @@ -476,10 +476,19 @@ impl BlobArray { } } + /// Copy data from other [`BlobArray`]. + /// + /// # Safety + /// Caller must ensure that: + /// - `other` stores data of the same layout as `self`. + /// - `other`'s length is `len`. + /// - `self` has enough `capacity` to store `len` elements. pub unsafe fn copy_from_unchecked(&mut self, other: &Self, len: usize) { if other.is_zst() { return; } + // SAFETY: + // - other is a valid BlobArray, so it must have a valid array layout let num_bytes_to_copy = array_layout_unchecked(&other.layout(), len).size(); debug_assert!( num_bytes_to_copy diff --git a/crates/bevy_ecs/src/storage/blob_vec.rs b/crates/bevy_ecs/src/storage/blob_vec.rs index 5cf301be5d25f..0b23aac9d4255 100644 --- a/crates/bevy_ecs/src/storage/blob_vec.rs +++ b/crates/bevy_ecs/src/storage/blob_vec.rs @@ -184,6 +184,13 @@ impl BlobVec { core::ptr::copy_nonoverlapping::(value.as_ptr(), ptr.as_ptr(), self.item_layout.size()); } + /// Try to initialize a value at next index in this [`BlobVec`] using passed initialization function. + /// If `init_fn` returns `true`, the value will be considered initialized, `len` increased and this function will return `true`. + /// Otherwise, `len` will be left as it was before and this function will return `false` + /// + /// # Safety + /// - caller must ensure that if `init_fn` returns `true`, a valid value of the layout stored in this [`BlobVec`] + /// was written to [`NonNull`] passed to `init_fn`. pub(crate) unsafe fn try_initialize_next( &mut self, init_fn: impl Fn(NonNull) -> bool, @@ -407,6 +414,12 @@ impl BlobVec { } } + /// Copy data from other [`BlobVec`]. + /// + /// # Safety + /// Caller must ensure that: + /// - `other` stores data of the same layout as `self`. + /// - `self` has enough `capacity` to store `other.len` elements. pub unsafe fn copy_from_unchecked(&mut self, other: &Self) { // Skip ZSTs if other.item_layout.size() == 0 { diff --git a/crates/bevy_ecs/src/storage/mod.rs b/crates/bevy_ecs/src/storage/mod.rs index 15b2a60ec7f56..1583a6459b8a9 100644 --- a/crates/bevy_ecs/src/storage/mod.rs +++ b/crates/bevy_ecs/src/storage/mod.rs @@ -27,18 +27,11 @@ mod sparse_set; mod table; mod thin_array_ptr; -use core::ptr::NonNull; - -use bevy_ptr::{OwningPtr, Ptr}; pub use resource::*; pub use sparse_set::*; pub use table::*; -use crate::{ - component::{ComponentInfo, Components}, - entity::ComponentCloneCtx, - world::{error::WorldCloneError, World}, -}; +use crate::world::{error::WorldCloneError, World}; /// The raw data stores of a [`World`](crate::world::World) #[derive(Default)] @@ -53,9 +46,12 @@ pub struct Storages { pub non_send_resources: Resources, } -// &impl Fn(&ComponentInfo, Ptr, NonNull); - impl Storages { + /// Try to clone [`Storages`]. This is only possible if all components and resources can be cloned, + /// otherwise [`WorldCloneError`] will be returned. + /// + /// # Safety + /// - Caller must ensure that [`Storages`] and `AppTypeRegistry` are from `world`. pub(crate) unsafe fn try_clone( &self, world: &World, diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index 6dfd4c413558e..7082c0667d67c 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -13,7 +13,7 @@ use alloc::string::String; use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; #[cfg(feature = "track_location")] use core::panic::Location; -use core::{cell::UnsafeCell, mem::ManuallyDrop, ptr::NonNull}; +use core::{cell::UnsafeCell, mem::ManuallyDrop}; #[cfg(feature = "std")] use std::thread::ThreadId; @@ -88,14 +88,15 @@ impl ResourceData { } #[inline] + /// Returns `true` if access to `!Send` resources is done on the thread they were created from or resources are `Send`. fn try_validate_access(&self) -> bool { #[cfg(feature = "std")] { - return if SEND { + if SEND { true } else { self.origin_thread_id == Some(std::thread::current().id()) - }; + } } // TODO: Handle no_std non-send. @@ -321,6 +322,13 @@ impl ResourceData { self.changed_ticks.get_mut().check_tick(change_tick); } + /// Try to clone [`ResourceData`]. This is only possible if all components can be cloned, + /// otherwise [`WorldCloneError`] will be returned. + /// + /// # Safety + /// Caller must ensure that: + /// - [`ComponentInfo`] is the same as the one used to create this [`ResourceData`]. + /// - [`ResourceData`] and `AppTypeRegistry` are from `world`. pub(crate) unsafe fn try_clone( &self, component_info: &ComponentInfo, @@ -373,9 +381,9 @@ impl ResourceData { added_ticks: UnsafeCell::new(self.added_ticks.read()), changed_ticks: UnsafeCell::new(self.changed_ticks.read()), type_name: self.type_name.clone(), - id: self.id.clone(), + id: self.id, #[cfg(feature = "std")] - origin_thread_id: self.origin_thread_id.clone(), + origin_thread_id: self.origin_thread_id, #[cfg(feature = "track_location")] changed_by: UnsafeCell::new(self.changed_by.read()), }) @@ -480,6 +488,11 @@ impl Resources { } } + /// Try to clone [`Resources`]. This is only possible if all resources can be cloned, + /// otherwise [`WorldCloneError`] will be returned. + /// + /// # Safety + /// - Caller must ensure that [`Resources`] and `AppTypeRegistry` are from `world`. pub(crate) unsafe fn try_clone( &self, world: &World, @@ -491,6 +504,7 @@ impl Resources { resources.insert( *component_id, res.try_clone( + // SAFETY: component_id is valid because this Table is valid and from the same world as Components. components.get_info_unchecked(*component_id), world, #[cfg(feature = "bevy_reflect")] diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 1fbc4765614ff..efb321591b971 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -1,7 +1,7 @@ use crate::{ change_detection::MaybeUnsafeCellLocation, - component::{ComponentId, ComponentInfo, ComponentTicks, Components, Tick, TickCells}, - entity::{ComponentCloneCtx, Entity}, + component::{ComponentId, ComponentInfo, ComponentTicks, Tick, TickCells}, + entity::Entity, storage::{Column, TableRow}, world::{error::WorldCloneError, World}, }; @@ -9,7 +9,7 @@ use alloc::{boxed::Box, vec::Vec}; use bevy_ptr::{OwningPtr, Ptr}; #[cfg(feature = "track_location")] use core::panic::Location; -use core::{cell::UnsafeCell, hash::Hash, marker::PhantomData, ptr::NonNull}; +use core::{cell::UnsafeCell, hash::Hash, marker::PhantomData}; use nonmax::NonMaxUsize; type EntityIndex = u32; @@ -141,6 +141,13 @@ impl ComponentSparseSet { } } + /// Try to clone [`ComponentSparseSet`]. This is only possible if all components can be cloned, + /// otherwise [`WorldCloneError`] will be returned. + /// + /// # Safety + /// Caller must ensure that: + /// - [`ComponentInfo`] is the same as the one used to create this [`ComponentSparseSet`]. + /// - [`ComponentSparseSet`] and `AppTypeRegistry` are from `world`. pub(crate) unsafe fn try_clone( &self, component_info: &ComponentInfo, @@ -148,14 +155,12 @@ impl ComponentSparseSet { #[cfg(feature = "bevy_reflect")] type_registry: Option<&crate::reflect::AppTypeRegistry>, ) -> Result { Ok(Self { - dense: unsafe { - self.dense.try_clone( - component_info, - world, - #[cfg(feature = "bevy_reflect")] - type_registry, - )? - }, + dense: self.dense.try_clone( + component_info, + world, + #[cfg(feature = "bevy_reflect")] + type_registry, + )?, entities: self.entities.clone(), sparse: self.sparse.clone(), }) @@ -631,6 +636,11 @@ impl SparseSets { self.sets.is_empty() } + /// Try to clone [`SparseSets`]. This is only possible if all components can be cloned, + /// otherwise [`WorldCloneError`] will be returned. + /// + /// # Safety + /// - Caller must ensure that [`SparseSets`] and `AppTypeRegistry` are from `world`. pub(crate) unsafe fn try_clone( &self, world: &World, @@ -639,6 +649,7 @@ impl SparseSets { let mut sets = SparseSet::with_capacity(self.sets.len()); let components = world.components(); for (component_id, set) in self.sets.iter() { + // SAFETY: component_id is valid because this SparseSets is valid and from the same world as Components. let component_info = components.get_info_unchecked(*component_id); let set = set.try_clone( component_info, diff --git a/crates/bevy_ecs/src/storage/table/column.rs b/crates/bevy_ecs/src/storage/table/column.rs index 237c7a8e32378..142c78d15a43c 100644 --- a/crates/bevy_ecs/src/storage/table/column.rs +++ b/crates/bevy_ecs/src/storage/table/column.rs @@ -1,8 +1,6 @@ -use core::ptr::NonNull; - use super::*; use crate::{ - component::{self, ComponentCloneHandlerKind, TickCells}, + component::{ComponentCloneHandlerKind, TickCells}, entity::ComponentCloneCtx, storage::{blob_array::BlobArray, thin_array_ptr::ThinArrayPtr}, world::{error::WorldCloneError, World}, @@ -331,6 +329,13 @@ impl ThinColumn { self.changed_by.as_slice(len) } + /// Try to clone [`ThinColumn`]. This is only possible if all components can be cloned, + /// otherwise [`WorldCloneError`] will be returned. + /// + /// # Safety + /// Caller must ensure that: + /// - [`ComponentInfo`] is the same as the one used to create this [`ThinColumn`]. + /// - [`ThinColumn`] and `AppTypeRegistry` are from `world`. pub(crate) unsafe fn try_clone( &self, component_info: &ComponentInfo, @@ -758,14 +763,20 @@ impl Column { self.changed_by.get_unchecked(row.as_usize()) } + /// Try to clone [`Column`]. This is only possible if all components can be cloned, + /// otherwise [`WorldCloneError`] will be returned. + /// + /// # Safety + /// Caller must ensure that: + /// - [`ComponentInfo`] is the same as the one used to create this [`Column`]. + /// - [`Column`] and `AppTypeRegistry` are from `world`. pub(crate) unsafe fn try_clone( &self, component_info: &ComponentInfo, world: &World, #[cfg(feature = "bevy_reflect")] type_registry: Option<&crate::reflect::AppTypeRegistry>, ) -> Result { - let len = self.added_ticks.len(); - let mut column = Self::with_capacity(component_info, len); + let mut column = Self::with_capacity(component_info, self.len()); column.added_ticks = self .added_ticks .iter() @@ -795,6 +806,9 @@ impl Column { return Err(WorldCloneError::ComponentCantBeCloned(component_info.id())) } ComponentCloneHandlerKind::Copy => { + // SAFETY: + // - column.data layout is from the same ComponentInfo as the one used to create self + // - column.data has capacity of at least self.len column.data.copy_from_unchecked(&self.data); return Ok(column); } diff --git a/crates/bevy_ecs/src/storage/table/mod.rs b/crates/bevy_ecs/src/storage/table/mod.rs index ce554ce01161a..b4622399f2612 100644 --- a/crates/bevy_ecs/src/storage/table/mod.rs +++ b/crates/bevy_ecs/src/storage/table/mod.rs @@ -1,7 +1,7 @@ use crate::{ change_detection::MaybeLocation, component::{ComponentId, ComponentInfo, ComponentTicks, Components, Tick}, - entity::{ComponentCloneCtx, Entity}, + entity::Entity, query::DebugCheckedUnwrap, storage::{blob_vec::BlobVec, ImmutableSparseSet, SparseSet}, world::{error::WorldCloneError, World}, @@ -17,7 +17,6 @@ use core::{ cell::UnsafeCell, num::NonZeroUsize, ops::{Index, IndexMut}, - ptr::NonNull, }; mod column; @@ -674,6 +673,11 @@ impl Table { .map(|col| col.data.get_unchecked(row.as_usize())) } + /// Try to clone [`Table`]. This is only possible if all components can be cloned, + /// otherwise [`WorldCloneError`] will be returned. + /// + /// # Safety + /// - Caller must ensure that [`Table`] and `AppTypeRegistry` are from `world`. pub(crate) unsafe fn try_clone( &self, world: &World, @@ -686,6 +690,7 @@ impl Table { columns.insert( *component_id, column.try_clone( + // SAFETY: component_id is valid because this Table is valid and from the same world as Components. components.get_info_unchecked(*component_id), column_len, world, @@ -810,6 +815,11 @@ impl Tables { } } + /// Try to clone [`Tables`]. This is only possible if all components can be cloned, + /// otherwise [`WorldCloneError`] will be returned. + /// + /// # Safety + /// - Caller must ensure that [`Tables`] and `AppTypeRegistry` are from `world`. pub(crate) unsafe fn try_clone( &self, world: &World, @@ -821,7 +831,7 @@ impl Tables { world, #[cfg(feature = "bevy_reflect")] type_registry, - )?) + )?); } Ok(Self { table_ids: self.table_ids.clone(), diff --git a/crates/bevy_ecs/src/world/error.rs b/crates/bevy_ecs/src/world/error.rs index e6563d5f3fdcf..8f00f3d1954b5 100644 --- a/crates/bevy_ecs/src/world/error.rs +++ b/crates/bevy_ecs/src/world/error.rs @@ -49,14 +49,22 @@ impl PartialEq for EntityFetchError { impl Eq for EntityFetchError {} +/// An error that occurs when cloning world failed. #[derive(Error, Debug, Clone, Copy, PartialEq, Eq)] pub enum WorldCloneError { - #[error("More `bevy` `World`s have been created than is supported")] + /// World id allocation failed. + #[error("More `bevy` `World`s have been created than is supported.")] WorldIdExhausted, - #[error("Component clone handler for component with ID {0:?} failed to clone the component")] + /// Component clone handler failed to clone component. + #[error("Component clone handler for component with ID {0:?} failed to clone the component.")] FailedToCloneComponent(ComponentId), - #[error("")] + /// Component clone handler failed to clone resource. + #[error("Component clone handler for resource with ID {0:?} failed to clone the resource.")] + FailedToCloneResource(ComponentId), + /// Resource cloned from different thread than the one used to create it. + #[error("Tried to clone non-send resource with ID {0:?} from a different thread than the one it was created from.")] NonSendResourceCloned(ComponentId), - #[error("")] + /// Component clone handler is set to `Ignore`. + #[error("Component clone handler for component or resource with ID {0:?} is set to Ignore and can't be cloned.")] ComponentCantBeCloned(ComponentId), } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 9b7fec38c230d..32d18fe8a9505 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -25,6 +25,7 @@ pub use entity_ref::{ DynamicComponentFetch, EntityMut, EntityMutExcept, EntityRef, EntityRefExcept, EntityWorldMut, Entry, FilteredEntityMut, FilteredEntityRef, OccupiedEntry, TryFromFilteredError, VacantEntry, }; +use error::WorldCloneError; pub use filtered_resource::*; pub use identifier::WorldId; pub use spawn_batch::*; @@ -38,7 +39,7 @@ use crate::{ ComponentId, ComponentInfo, ComponentTicks, Components, Mutable, RequiredComponents, RequiredComponentsError, Tick, }, - entity::{AllocAtWithoutReplacement, ComponentCloneCtx, Entities, Entity, EntityLocation}, + entity::{AllocAtWithoutReplacement, Entities, Entity, EntityLocation}, event::{Event, EventId, Events, SendBatchIds}, observer::Observers, query::{DebugCheckedUnwrap, QueryData, QueryFilter, QueryState}, @@ -205,8 +206,14 @@ impl World { self.id } - pub fn try_clone(&self) -> Result { - let id = WorldId::new().ok_or(error::WorldCloneError::WorldIdExhausted)?; + /// Try to perform full world cloning. + /// + /// This function will return a clone if all components and resources in this world + /// can be cloned. If that is not possible, [`WorldCloneError`] will be returned instead. + pub fn try_clone(&self) -> Result { + let id = WorldId::new().ok_or(WorldCloneError::WorldIdExhausted)?; + // SAFETY: + // - storages and type_registry is from self let storages = unsafe { self.storages.try_clone( self, @@ -3188,16 +3195,18 @@ impl World { unsafe { self.bundles.get(id).debug_checked_unwrap() } } + /// Sets custom [`ComponentCloneFn`] as default clone handler for this world's components. pub fn set_default_component_clone_handler(&mut self, handler: ComponentCloneFn) { self.components.set_default_clone_handler(handler); } + /// Sets custom [`ComponentCloneHandler`] as clone handler for component with provided [`ComponentId`]. pub fn set_component_clone_handler( &mut self, component_id: ComponentId, handler: ComponentCloneHandler, ) { - self.components.set_clone_handler(component_id, handler) + self.components.set_clone_handler(component_id, handler); } } diff --git a/crates/bevy_hierarchy/src/hierarchy.rs b/crates/bevy_hierarchy/src/hierarchy.rs index e8d0cb8b7807c..fe3f0f44d778c 100644 --- a/crates/bevy_hierarchy/src/hierarchy.rs +++ b/crates/bevy_hierarchy/src/hierarchy.rs @@ -6,7 +6,7 @@ use bevy_ecs::{ component::ComponentCloneHandler, entity::{ComponentCloneCtx, Entity, EntityCloneBuilder}, system::EntityCommands, - world::{DeferredWorld, EntityWorldMut, World}, + world::{EntityWorldMut, World}, }; use log::debug; From 173eac54823c15496f02d16f2476cd6a1c111b9f Mon Sep 17 00:00:00 2001 From: eugineerd Date: Fri, 10 Jan 2025 21:37:26 +0000 Subject: [PATCH 08/15] move `source` and `target` getters to `EntityCloner` --- crates/bevy_ecs/src/component.rs | 4 ++- crates/bevy_ecs/src/entity/clone_entities.rs | 33 +++++++++---------- .../bevy_ecs/src/observer/entity_observer.rs | 7 ++-- crates/bevy_hierarchy/src/hierarchy.rs | 8 +++-- 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 1fed4cebe9372..4d738147ce1f1 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -2280,7 +2280,9 @@ pub fn component_clone_via_reflect(_world: &World, ctx: &mut ComponentCloneCtx) let Some(mut commands) = ctx.commands() else { return; }; - let Some(target) = ctx.target() else { return }; + let Some(target) = ctx.entity_cloner().map(crate::entity::EntityCloner::target) else { + return; + }; let reflect_from_world = reflect_from_world.clone(); let source_component_cloned = source_component_reflect.clone_value(); drop(registry); diff --git a/crates/bevy_ecs/src/entity/clone_entities.rs b/crates/bevy_ecs/src/entity/clone_entities.rs index 41c891abd48b6..581e9c4d84eee 100644 --- a/crates/bevy_ecs/src/entity/clone_entities.rs +++ b/crates/bevy_ecs/src/entity/clone_entities.rs @@ -91,16 +91,6 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { self.target_component_written } - /// Returns the current source entity (if available in current context). - pub fn source(&self) -> Option { - self.entity_cloner.map(|c| c.source) - } - - /// Returns the current target entity (if available in current context). - pub fn target(&self) -> Option { - self.entity_cloner.map(|c| c.target) - } - /// Returns the [`ComponentId`] of the component being cloned. pub fn component_id(&self) -> ComponentId { self.component_id @@ -282,15 +272,14 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { /// # use bevy_ecs::world::World; /// # use bevy_ecs::entity::ComponentCloneCtx; /// fn clone_handler(_world: &World, ctx: &mut ComponentCloneCtx) { - /// let Some(commands) = ctx.commands() else { return }; - /// let Some(mut entity_cloner) = ctx.entity_cloner() - /// .and_then(|entity_cloner| { - /// let another_target = commands.spawn_empty().id(); - /// entity_cloner.with_source_and_target(ctx.source(), another_target)) - /// } - /// else { + /// let Some(entity_cloner) = ctx.entity_cloner() else { + /// return + /// }; + /// let Some(mut commands) = ctx.commands() else { /// return /// }; + /// let another_target = commands.spawn_empty().id(); + /// let mut entity_cloner = entity_cloner.with_source_and_target(entity_cloner.source(), another_target); /// commands.queue(move |world: &mut World| { /// entity_cloner.clone_entity(world); /// }); @@ -432,6 +421,16 @@ impl EntityCloner { ..*self } } + + /// Returns the current source entity. + pub fn source(&self) -> Entity { + self.source + } + + /// Returns the current target entity. + pub fn target(&self) -> Entity { + self.target + } } /// Builder struct to clone an entity. Allows configuring which components to clone, as well as how to clone them. diff --git a/crates/bevy_ecs/src/observer/entity_observer.rs b/crates/bevy_ecs/src/observer/entity_observer.rs index 8e2181f451302..228dfbd8eaaeb 100644 --- a/crates/bevy_ecs/src/observer/entity_observer.rs +++ b/crates/bevy_ecs/src/observer/entity_observer.rs @@ -68,11 +68,14 @@ impl CloneEntityWithObserversExt for EntityCloneBuilder<'_> { } fn component_clone_observed_by(_world: &World, ctx: &mut ComponentCloneCtx) { - let Some(target) = ctx.target() else { return }; - let Some(source) = ctx.source() else { return }; + let Some(entity_cloner) = ctx.entity_cloner() else { + return; + }; let Some(mut commands) = ctx.commands() else { return; }; + let target = entity_cloner.target(); + let source = entity_cloner.source(); commands.queue(move |world: &mut World| { let observed_by = world diff --git a/crates/bevy_hierarchy/src/hierarchy.rs b/crates/bevy_hierarchy/src/hierarchy.rs index dfa19868d2111..06f0dcc1f1c44 100644 --- a/crates/bevy_hierarchy/src/hierarchy.rs +++ b/crates/bevy_hierarchy/src/hierarchy.rs @@ -4,7 +4,7 @@ use crate::{ }; use bevy_ecs::{ component::ComponentCloneHandler, - entity::{ComponentCloneCtx, Entity, EntityCloneBuilder}, + entity::{ComponentCloneCtx, Entity, EntityCloneBuilder, EntityCloner}, system::{error_handler, EntityCommands}, world::{EntityWorldMut, World}, }; @@ -249,7 +249,7 @@ fn component_clone_children(_world: &World, ctx: &mut ComponentCloneCtx) { let Some(entity_cloner) = ctx.entity_cloner() else { return; }; - let Some(parent) = ctx.target() else { return }; + let parent = entity_cloner.target(); let children = ctx .read_source_component::() .expect("Source entity must have Children component") @@ -266,7 +266,9 @@ fn component_clone_children(_world: &World, ctx: &mut ComponentCloneCtx) { /// Clone handler for the [`Parent`] component. Allows to add clone as a child to the parent entity. fn component_clone_parent(_world: &World, ctx: &mut ComponentCloneCtx) { - let Some(target) = ctx.target() else { return }; + let Some(target) = ctx.entity_cloner().map(EntityCloner::target) else { + return; + }; let Some(mut commands) = ctx.commands() else { return; }; From dceecf411de31ed20e644cda2d2faf3bf0e47317 Mon Sep 17 00:00:00 2001 From: eugineerd Date: Fri, 10 Jan 2025 21:51:41 +0000 Subject: [PATCH 09/15] return error if there are unapplied commands during world cloning --- crates/bevy_ecs/src/world/error.rs | 3 +++ crates/bevy_ecs/src/world/mod.rs | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/crates/bevy_ecs/src/world/error.rs b/crates/bevy_ecs/src/world/error.rs index 8f00f3d1954b5..93388056cb388 100644 --- a/crates/bevy_ecs/src/world/error.rs +++ b/crates/bevy_ecs/src/world/error.rs @@ -55,6 +55,9 @@ pub enum WorldCloneError { /// World id allocation failed. #[error("More `bevy` `World`s have been created than is supported.")] WorldIdExhausted, + /// World has unapplied commands queued. + #[error("World cannot be cloned while there are unapplied commands queued.")] + UnappliedCommands, /// Component clone handler failed to clone component. #[error("Component clone handler for component with ID {0:?} failed to clone the component.")] FailedToCloneComponent(ComponentId), diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 55a5226cad905..651b8d3cb2313 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -176,6 +176,11 @@ impl World { /// This function will return a clone if all components and resources in this world /// can be cloned. If that is not possible, [`WorldCloneError`] will be returned instead. pub fn try_clone(&self) -> Result { + // SAFETY: `self.command_queue` is only de-allocated in `World`'s `Drop` + if !unsafe { self.command_queue.is_empty() } { + return Err(WorldCloneError::UnappliedCommands); + } + let id = WorldId::new().ok_or(WorldCloneError::WorldIdExhausted)?; // SAFETY: // - storages and type_registry is from self From d5d34231feee7dbc40fa859ad4e77c83b6457508 Mon Sep 17 00:00:00 2001 From: eugineerd Date: Sat, 11 Jan 2025 12:19:23 +0000 Subject: [PATCH 10/15] add `is_copy` flag to component descriptor --- crates/bevy_ecs/macros/src/component.rs | 40 ++--- crates/bevy_ecs/src/component.rs | 164 ++++++++++++------- crates/bevy_ecs/src/entity/clone_entities.rs | 9 +- crates/bevy_ecs/src/storage/resource.rs | 3 + crates/bevy_ecs/src/storage/table/column.rs | 6 + crates/bevy_ecs/src/system/system_param.rs | 10 +- crates/bevy_ecs/src/world/mod.rs | 24 +++ examples/ecs/dynamic.rs | 1 + examples/ecs/immutable_components.rs | 1 + examples/stress_tests/many_components.rs | 1 + 10 files changed, 180 insertions(+), 79 deletions(-) diff --git a/crates/bevy_ecs/macros/src/component.rs b/crates/bevy_ecs/macros/src/component.rs index ef12a340af545..f7562a3c2a6b2 100644 --- a/crates/bevy_ecs/macros/src/component.rs +++ b/crates/bevy_ecs/macros/src/component.rs @@ -52,21 +52,19 @@ pub fn derive_resource(input: TokenStream) -> TokenStream { TokenStream::from(quote! { impl #impl_generics #bevy_ecs_path::system::Resource for #struct_name #type_generics #where_clause { fn get_component_clone_handler() -> #bevy_ecs_path::component::ComponentCloneHandler { - use #bevy_ecs_path::component::{ComponentCloneViaClone, ComponentCloneBase}; - - trait ComponentCloneViaCopy { - fn get_component_clone_handler(&self) -> #bevy_ecs_path::component::ComponentCloneHandler; - } - impl ComponentCloneViaCopy for &&#bevy_ecs_path::component::ComponentCloneSpecializationWrapper { - fn get_component_clone_handler(&self) -> #bevy_ecs_path::component::ComponentCloneHandler { - // SAFETY: T is Self and this handler will only be set for Self - unsafe { #bevy_ecs_path::component::ComponentCloneHandler::copy_handler::() } - } - } + use #bevy_ecs_path::component::{ComponentCloneViaClone, ComponentCloneViaCopy, ComponentCloneBase}; (&&&#bevy_ecs_path::component::ComponentCloneSpecializationWrapper::::default()) .get_component_clone_handler() } + + unsafe fn is_copy() -> bool { + use #bevy_ecs_path::component::{IsCopyBase, IsCopySpec}; + + // Safety: IsCopyWrapper use autoderef specialization to determine if type implements [`Copy`] + (&&&#bevy_ecs_path::component::IsCopyWrapper::::default()) + .is_copy() + } } }) } @@ -184,21 +182,19 @@ pub fn derive_component(input: TokenStream) -> TokenStream { } fn get_component_clone_handler() -> #bevy_ecs_path::component::ComponentCloneHandler { - use #bevy_ecs_path::component::{ComponentCloneViaClone, ComponentCloneBase}; - - trait ComponentCloneViaCopy { - fn get_component_clone_handler(&self) -> #bevy_ecs_path::component::ComponentCloneHandler; - } - impl ComponentCloneViaCopy for &&#bevy_ecs_path::component::ComponentCloneSpecializationWrapper { - fn get_component_clone_handler(&self) -> #bevy_ecs_path::component::ComponentCloneHandler { - // SAFETY: T is Self and this handler will only be set for Self - unsafe { #bevy_ecs_path::component::ComponentCloneHandler::copy_handler::() } - } - } + use #bevy_ecs_path::component::{ComponentCloneViaClone, ComponentCloneViaCopy, ComponentCloneBase}; (&&&#bevy_ecs_path::component::ComponentCloneSpecializationWrapper::::default()) .get_component_clone_handler() } + + unsafe fn is_copy() -> bool { + use #bevy_ecs_path::component::{IsCopyBase, IsCopySpec}; + + // Safety: IsCopyWrapper use autoderef specialization to determine if type implements [`Copy`] + (&&&#bevy_ecs_path::component::IsCopyWrapper::::default()) + .is_copy() + } } }) } diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 4d738147ce1f1..e13a0213be766 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -423,6 +423,14 @@ pub trait Component: Send + Sync + 'static { fn get_component_clone_handler() -> ComponentCloneHandler { ComponentCloneHandler::default_handler() } + + /// Called when registering this component, can be set to `true` for components that are [`Copy`] to enable optimizations + /// when performing component cloning. + /// # Safety + /// Must return `true` **only** if component is actually [`Copy`] + unsafe fn is_copy() -> bool { + false + } } mod private { @@ -788,7 +796,15 @@ impl ComponentInfo { &self.required_components } - pub(crate) fn clone_handler(&self) -> &ComponentCloneHandler { + /// Returns `true` if component is [`Copy`]. + /// + /// Unless this is `true`, it is not safe to clone this component's data using memory copy. + pub fn is_copy(&self) -> bool { + self.descriptor.is_copy + } + + /// Returns [`ComponentCloneHandler`] set for this component. + pub fn clone_handler(&self) -> &ComponentCloneHandler { &self.descriptor.clone_handler } } @@ -852,6 +868,7 @@ impl SparseSetIndex for ComponentId { } /// A value describing a component or resource, which may or may not correspond to a Rust type. +#[derive(Clone)] pub struct ComponentDescriptor { name: Cow<'static, str>, // SAFETY: This must remain private. It must match the statically known StorageType of the @@ -867,25 +884,12 @@ pub struct ComponentDescriptor { // None if the underlying type doesn't need to be dropped drop: Option unsafe fn(OwningPtr<'a>)>, mutable: bool, + // SAFETY: This must remain private. It must only be set to "true" if this component is + // actually Copy + is_copy: bool, clone_handler: ComponentCloneHandler, } -impl Clone for ComponentDescriptor { - fn clone(&self) -> Self { - Self { - name: self.name.clone(), - storage_type: self.storage_type, - is_send_and_sync: self.is_send_and_sync, - type_id: self.type_id, - layout: self.layout, - drop: self.drop, - mutable: self.mutable, - // SAFETY: This clone handler will be used with the same component - clone_handler: unsafe { self.clone_handler.clone_unchecked() }, - } - } -} - // We need to ignore the `drop` field in our `Debug` impl impl Debug for ComponentDescriptor { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { @@ -921,6 +925,8 @@ impl ComponentDescriptor { layout: Layout::new::(), drop: needs_drop::().then_some(Self::drop_ptr:: as _), mutable: T::Mutability::MUTABLE, + // Safety: is_copy will be true only if component is actually Copy + is_copy: unsafe { T::is_copy() }, clone_handler: T::get_component_clone_handler(), } } @@ -930,12 +936,14 @@ impl ComponentDescriptor { /// # Safety /// - the `drop` fn must be usable on a pointer with a value of the layout `layout` /// - the component type must be safe to access from any thread (Send + Sync in rust terms) + /// - `is_copy` should be `true` only if component is safe to clone using memory copy. pub unsafe fn new_with_layout( name: impl Into>, storage_type: StorageType, layout: Layout, drop: Option unsafe fn(OwningPtr<'a>)>, mutable: bool, + is_copy: bool, clone_handler: ComponentCloneHandler, ) -> Self { Self { @@ -946,6 +954,7 @@ impl ComponentDescriptor { layout, drop, mutable, + is_copy, clone_handler, } } @@ -964,6 +973,8 @@ impl ComponentDescriptor { layout: Layout::new::(), drop: needs_drop::().then_some(Self::drop_ptr:: as _), mutable: true, + // Safety: is_copy will be true only if resource is actually Copy + is_copy: unsafe { T::is_copy() }, clone_handler: T::get_component_clone_handler(), } } @@ -977,6 +988,7 @@ impl ComponentDescriptor { layout: Layout::new::(), drop: needs_drop::().then_some(Self::drop_ptr:: as _), mutable: true, + is_copy: false, clone_handler: ComponentCloneHandler::default_handler(), } } @@ -1025,70 +1037,65 @@ pub(crate) enum ComponentCloneHandlerKind { } /// A struct instructing which clone handler to use when cloning a component. -#[derive(Debug)] +#[derive(Debug, Clone, Copy)] pub struct ComponentCloneHandler { entity_handler: ComponentCloneHandlerKind, world_handler: Option, } impl ComponentCloneHandler { - /// Use the global default function to clone the component with this handler. - pub fn default_handler() -> Self { + fn new(entity_handler: ComponentCloneHandlerKind) -> Self { Self { - entity_handler: ComponentCloneHandlerKind::Default, + entity_handler, world_handler: None, } } + /// Use the global default function to clone the component with this handler. + pub fn default_handler() -> Self { + Self::new(ComponentCloneHandlerKind::Default) + } + /// Do not clone the component. When a command to clone an entity is issued, component with this handler will be skipped. pub fn ignore() -> Self { - Self { - entity_handler: ComponentCloneHandlerKind::Ignore, - world_handler: None, - } + Self::new(ComponentCloneHandlerKind::Ignore) } /// Set clone handler based on `Clone` trait. /// /// If set as a handler for a component that is not the same as the one used to create this handler, it will panic. pub fn clone_handler() -> Self { - Self { - entity_handler: ComponentCloneHandlerKind::Custom(component_clone_via_clone::), - world_handler: None, - } + Self::new(ComponentCloneHandlerKind::Custom( + component_clone_via_clone::, + )) } /// Set clone handler to use copy. /// - /// # Safety - /// - Must ensure that this handler is used only with the same type as T - pub unsafe fn copy_handler() -> Self { - Self { - entity_handler: ComponentCloneHandlerKind::Copy, - world_handler: None, - } + /// If component does not implement `Copy`, this will be equivalent to [`ignore`](Self::ignore) + pub fn copy_handler() -> Self { + Self::new(ComponentCloneHandlerKind::Copy) } /// Set clone handler based on `Reflect` trait. + /// + /// If component does not implement `Reflect`, this will be equivalent to [`ignore`](Self::ignore) #[cfg(feature = "bevy_reflect")] pub fn reflect_handler() -> Self { - Self { - entity_handler: ComponentCloneHandlerKind::Custom(component_clone_via_reflect), - world_handler: None, - } + Self::new(ComponentCloneHandlerKind::Custom( + component_clone_via_reflect, + )) } /// Set a custom handler for the component. pub fn custom_handler(handler: ComponentCloneFn) -> Self { - Self { - entity_handler: ComponentCloneHandlerKind::Custom(handler), - world_handler: None, - } + Self::new(ComponentCloneHandlerKind::Custom(handler)) } /// Set a different component clone handler when performing world cloning instead of entity cloning. + /// If world clone handler is not set, entity clone handler will be used to clone component during world cloning instead. /// - /// Provided handler should generally be infallible and access only source component data. + /// Provided handler should generally be infallible and rely only on source component data. pub fn with_world_clone_handler(self, handler: Self) -> Self { Self { entity_handler: self.entity_handler, @@ -1096,22 +1103,17 @@ impl ComponentCloneHandler { } } + /// Gets inner [`ComponentCloneHandlerKind`] describing how component should be cloned when performing entity cloning. pub(crate) fn get_entity_handler(&self) -> ComponentCloneHandlerKind { self.entity_handler } + /// Gets inner [`ComponentCloneHandlerKind`] describing how component should be cloned when performing world cloning. + /// + /// If [`ComponentCloneHandler`] does not have explicit world handler, use [`entity handler`](Self::get_entity_handler). pub(crate) fn get_world_handler(&self) -> Option { self.world_handler } - - /// # Safety - /// - Cloned handler can be used only with the same component as the one used to get this handler. - pub(crate) unsafe fn clone_unchecked(&self) -> Self { - Self { - entity_handler: self.entity_handler, - world_handler: self.world_handler, - } - } } /// Stores metadata associated with each kind of [`Component`] in a given [`World`]. @@ -1718,12 +1720,23 @@ impl Components { self.default_component_clone_handler } - /// Sets [`ComponentCloneHandler`] for component with specified [`ComponentId`] + /// Sets [`ComponentCloneHandler`] for component with specified [`ComponentId`]. + /// + /// If component with [`ComponentId`] is not registered, this does nothing. pub fn set_clone_handler(&mut self, component_id: ComponentId, handler: ComponentCloneHandler) { if let Some(info) = self.components.get_mut(component_id.0) { info.descriptor.clone_handler = handler; } } + + /// Gets [`ComponentCloneHandler`] for component with specified [`ComponentId`]. + /// + /// Returns `None` if component with provided [`ComponentId`] is not registered. + pub fn get_clone_handler(&self, component_id: ComponentId) -> Option { + self.components + .get(component_id.0) + .map(|info| info.descriptor.clone_handler) + } } /// A value that tracks when a system ran relative to other systems. @@ -2343,3 +2356,44 @@ impl ComponentCloneViaClone for &ComponentCloneSpecializatio ComponentCloneHandler::clone_handler::() } } + +/// Specialized trait for components clone specialization using autoderef. +#[doc(hidden)] +pub trait ComponentCloneViaCopy { + fn get_component_clone_handler(&self) -> ComponentCloneHandler; +} +impl ComponentCloneViaCopy for &&ComponentCloneSpecializationWrapper { + fn get_component_clone_handler(&self) -> ComponentCloneHandler { + ComponentCloneHandler::copy_handler() + } +} + +/// Wrapper to determine if `T` implements [`Copy`] using autoderef. +#[doc(hidden)] +pub struct IsCopyWrapper(PhantomData); + +impl Default for IsCopyWrapper { + fn default() -> Self { + Self(PhantomData) + } +} +/// Base trait for is copy specialization using autoderef. +#[doc(hidden)] +pub trait IsCopyBase { + fn is_copy(&self) -> bool; +} +impl IsCopyBase for IsCopyWrapper { + fn is_copy(&self) -> bool { + false + } +} +/// Specialized trait for is copy specialization using autoderef. +#[doc(hidden)] +pub trait IsCopySpec { + fn is_copy(&self) -> bool; +} +impl IsCopySpec for &IsCopyWrapper { + fn is_copy(&self) -> bool { + true + } +} diff --git a/crates/bevy_ecs/src/entity/clone_entities.rs b/crates/bevy_ecs/src/entity/clone_entities.rs index 581e9c4d84eee..660c7e744c52f 100644 --- a/crates/bevy_ecs/src/entity/clone_entities.rs +++ b/crates/bevy_ecs/src/entity/clone_entities.rs @@ -353,7 +353,13 @@ impl EntityCloner { { ComponentCloneHandlerKind::Default => components.get_default_clone_handler(), ComponentCloneHandlerKind::Ignore => continue, - ComponentCloneHandlerKind::Copy => copy_clone_handler, + ComponentCloneHandlerKind::Copy => { + if component_info.is_copy() { + copy_clone_handler + } else { + continue; + } + } ComponentCloneHandlerKind::Custom(handler) => handler, }; @@ -1148,6 +1154,7 @@ mod tests { layout, None, true, + true, ComponentCloneHandler::custom_handler(test_handler), ) }; diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index 7082c0667d67c..74a8b5b32381b 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -352,6 +352,9 @@ impl ResourceData { return Err(WorldCloneError::ComponentCantBeCloned(component_info.id())) } ComponentCloneHandlerKind::Copy => { + if !component_info.is_copy() { + return Err(WorldCloneError::FailedToCloneResource(component_info.id())); + } data.copy_from_unchecked(&self.data); None } diff --git a/crates/bevy_ecs/src/storage/table/column.rs b/crates/bevy_ecs/src/storage/table/column.rs index 142c78d15a43c..9d9d8014f87ab 100644 --- a/crates/bevy_ecs/src/storage/table/column.rs +++ b/crates/bevy_ecs/src/storage/table/column.rs @@ -379,6 +379,9 @@ impl ThinColumn { return Err(WorldCloneError::ComponentCantBeCloned(component_info.id())) } ComponentCloneHandlerKind::Copy => { + if !component_info.is_copy() { + return Err(WorldCloneError::FailedToCloneComponent(component_info.id())); + } column.data.copy_from_unchecked(&self.data, len); return Ok(column); } @@ -806,6 +809,9 @@ impl Column { return Err(WorldCloneError::ComponentCantBeCloned(component_info.id())) } ComponentCloneHandlerKind::Copy => { + if !component_info.is_copy() { + return Err(WorldCloneError::FailedToCloneComponent(component_info.id())); + } // SAFETY: // - column.data layout is from the same ComponentInfo as the one used to create self // - column.data has capacity of at least self.len diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 36de1573ff78f..dc0ad49aa82ac 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -840,12 +840,20 @@ all_tuples_enumerated!(impl_param_set, 1, 8, P, m, p); note = "consider annotating `{Self}` with `#[derive(Resource)]`" )] pub trait Resource: Send + Sync + 'static { - /// Called when registering this component, allowing to override clone function (or disable cloning altogether) for this component. + /// Called when registering this resource, allowing to override clone function (or disable cloning altogether) for this resource. /// /// See [Handlers section of `EntityCloneBuilder`](crate::entity::EntityCloneBuilder#handlers) to understand how this affects handler priority. fn get_component_clone_handler() -> ComponentCloneHandler { ComponentCloneHandler::default_handler() } + + /// Called when registering this resource, can be set to `true` for components that are [`Copy`] to enable optimizations + /// when performing resource cloning. + /// # Safety + /// Must return `true` **only** if resource is actually [`Copy`] + unsafe fn is_copy() -> bool { + false + } } // SAFETY: Res only reads a single World resource diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 651b8d3cb2313..f8961c3d58a1e 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -3162,11 +3162,24 @@ impl World { } /// Sets custom [`ComponentCloneFn`] as default clone handler for this world's components. + /// All components with [`default`](ComponentCloneHandler::default_handler) handler, as well as any component that does not have an + /// explicitly registered clone function will use this handler. + /// + /// See [Handlers section of `EntityCloneBuilder`](crate::entity::EntityCloneBuilder#handlers) to understand how this affects handler priority. pub fn set_default_component_clone_handler(&mut self, handler: ComponentCloneFn) { self.components.set_default_clone_handler(handler); } + /// Get default clone handler set for this world's components. + pub fn get_default_component_clone_handler(&self) -> ComponentCloneFn { + self.components.get_default_clone_handler() + } + /// Sets custom [`ComponentCloneHandler`] as clone handler for component with provided [`ComponentId`]. + /// + /// If component with provided [`ComponentId`] is not registered, this does nothing. + /// + /// See [Handlers section of `EntityCloneBuilder`](crate::entity::EntityCloneBuilder#handlers) to understand how this affects handler priority. pub fn set_component_clone_handler( &mut self, component_id: ComponentId, @@ -3174,6 +3187,16 @@ impl World { ) { self.components.set_clone_handler(component_id, handler); } + + /// Get [`ComponentCloneHandler`] set for component with provided [`ComponentId`] + /// + /// Returns `None` if component with provided [`ComponentId`] is not registered. + pub fn get_component_clone_handler( + &self, + component_id: ComponentId, + ) -> Option { + self.components.get_clone_handler(component_id) + } } impl World { @@ -3993,6 +4016,7 @@ mod tests { DROP_COUNT.fetch_add(1, Ordering::SeqCst); }), true, + false, ComponentCloneHandler::default_handler(), ) }; diff --git a/examples/ecs/dynamic.rs b/examples/ecs/dynamic.rs index f3fbf35d4b417..fa896c8e5d40b 100644 --- a/examples/ecs/dynamic.rs +++ b/examples/ecs/dynamic.rs @@ -93,6 +93,7 @@ fn main() { Layout::array::(size).unwrap(), None, true, + false, ComponentCloneHandler::default_handler(), ) }); diff --git a/examples/ecs/immutable_components.rs b/examples/ecs/immutable_components.rs index 98877d02764dc..f78bc0955de5e 100644 --- a/examples/ecs/immutable_components.rs +++ b/examples/ecs/immutable_components.rs @@ -149,6 +149,7 @@ fn demo_3(world: &mut World) { Layout::array::(size).unwrap(), None, false, + false, ComponentCloneHandler::default_handler(), ) }; diff --git a/examples/stress_tests/many_components.rs b/examples/stress_tests/many_components.rs index 0da3b7de46ebb..05bd0716c181b 100644 --- a/examples/stress_tests/many_components.rs +++ b/examples/stress_tests/many_components.rs @@ -93,6 +93,7 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { Layout::new::(), None, true, // is mutable + false, ComponentCloneHandler::default_handler(), ) }, From ca11bf976f6a6599cd04108840036d3a22689b15 Mon Sep 17 00:00:00 2001 From: eugineerd Date: Sat, 11 Jan 2025 12:59:33 +0000 Subject: [PATCH 11/15] get `is_copy` for components using a hack instead of associated function on `Component` and `Resource` --- crates/bevy_ecs/macros/src/component.rs | 16 ---- crates/bevy_ecs/src/component.rs | 102 +++++++++++++-------- crates/bevy_ecs/src/system/system_param.rs | 8 -- 3 files changed, 64 insertions(+), 62 deletions(-) diff --git a/crates/bevy_ecs/macros/src/component.rs b/crates/bevy_ecs/macros/src/component.rs index f7562a3c2a6b2..54031cee966da 100644 --- a/crates/bevy_ecs/macros/src/component.rs +++ b/crates/bevy_ecs/macros/src/component.rs @@ -57,14 +57,6 @@ pub fn derive_resource(input: TokenStream) -> TokenStream { (&&&#bevy_ecs_path::component::ComponentCloneSpecializationWrapper::::default()) .get_component_clone_handler() } - - unsafe fn is_copy() -> bool { - use #bevy_ecs_path::component::{IsCopyBase, IsCopySpec}; - - // Safety: IsCopyWrapper use autoderef specialization to determine if type implements [`Copy`] - (&&&#bevy_ecs_path::component::IsCopyWrapper::::default()) - .is_copy() - } } }) } @@ -187,14 +179,6 @@ pub fn derive_component(input: TokenStream) -> TokenStream { (&&&#bevy_ecs_path::component::ComponentCloneSpecializationWrapper::::default()) .get_component_clone_handler() } - - unsafe fn is_copy() -> bool { - use #bevy_ecs_path::component::{IsCopyBase, IsCopySpec}; - - // Safety: IsCopyWrapper use autoderef specialization to determine if type implements [`Copy`] - (&&&#bevy_ecs_path::component::IsCopyWrapper::::default()) - .is_copy() - } } }) } diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index e13a0213be766..a04be38931ed1 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -423,14 +423,6 @@ pub trait Component: Send + Sync + 'static { fn get_component_clone_handler() -> ComponentCloneHandler { ComponentCloneHandler::default_handler() } - - /// Called when registering this component, can be set to `true` for components that are [`Copy`] to enable optimizations - /// when performing component cloning. - /// # Safety - /// Must return `true` **only** if component is actually [`Copy`] - unsafe fn is_copy() -> bool { - false - } } mod private { @@ -925,8 +917,8 @@ impl ComponentDescriptor { layout: Layout::new::(), drop: needs_drop::().then_some(Self::drop_ptr:: as _), mutable: T::Mutability::MUTABLE, - // Safety: is_copy will be true only if component is actually Copy - is_copy: unsafe { T::is_copy() }, + // Safety: is_copy() will be true only if T is actually Copy + is_copy: is_copy::(), clone_handler: T::get_component_clone_handler(), } } @@ -973,8 +965,8 @@ impl ComponentDescriptor { layout: Layout::new::(), drop: needs_drop::().then_some(Self::drop_ptr:: as _), mutable: true, - // Safety: is_copy will be true only if resource is actually Copy - is_copy: unsafe { T::is_copy() }, + // Safety: is_copy() will be true only if T is actually Copy + is_copy: is_copy::(), clone_handler: T::get_component_clone_handler(), } } @@ -988,7 +980,8 @@ impl ComponentDescriptor { layout: Layout::new::(), drop: needs_drop::().then_some(Self::drop_ptr:: as _), mutable: true, - is_copy: false, + // Safety: is_copy() will be true only if T is actually Copy + is_copy: is_copy::(), clone_handler: ComponentCloneHandler::default_handler(), } } @@ -2368,32 +2361,65 @@ impl ComponentCloneViaCopy for &&ComponentCloneSpecialization } } -/// Wrapper to determine if `T` implements [`Copy`] using autoderef. -#[doc(hidden)] -pub struct IsCopyWrapper(PhantomData); - -impl Default for IsCopyWrapper { - fn default() -> Self { - Self(PhantomData) - } -} -/// Base trait for is copy specialization using autoderef. -#[doc(hidden)] -pub trait IsCopyBase { - fn is_copy(&self) -> bool; -} -impl IsCopyBase for IsCopyWrapper { - fn is_copy(&self) -> bool { - false +/// HACK: Determine if T is [`Copy`] by (ab)using array copy specialization. +/// This utilizes a maybe bug in std which is maybe unsound when used with lifetimes (see: ). +/// At the same time there doesn't seem to be any timeline on when (or if) this will be fixed, so maybe it is ok to use? +/// Either way, we can always fall back to autoderef-based specialization if this ever gets fixed +/// (or maybe proper specialization will be available by that time?). +/// This hack is used mostly to avoid forcing users to write unsafe code to indicate if component if actually `Copy` +/// when implementing [`Component`] trait by hand. +#[inline] +fn is_copy() -> bool { + struct MaybeCopy<'a, T>(&'a core::cell::Cell, PhantomData); + + impl Clone for MaybeCopy<'_, T> { + fn clone(&self) -> Self { + self.0.set(false); + Self(self.0, self.1) + } } + + impl Copy for MaybeCopy<'_, T> {} + + let is_copy = core::cell::Cell::new(true); + + _ = [MaybeCopy::(&is_copy, Default::default()); 1].clone(); + + is_copy.get() } -/// Specialized trait for is copy specialization using autoderef. -#[doc(hidden)] -pub trait IsCopySpec { - fn is_copy(&self) -> bool; -} -impl IsCopySpec for &IsCopyWrapper { - fn is_copy(&self) -> bool { - true + +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn is_copy_working() { + struct A; + + #[derive(Clone, Copy)] + struct B; + + #[derive(Clone)] + struct C; + + impl Copy for C {} + + struct D; + + impl Clone for D { + fn clone(&self) -> Self { + *self + } + } + + impl Copy for D {} + + #[derive(Clone)] + struct E; + + assert!(!is_copy::()); + assert!(is_copy::()); + assert!(is_copy::()); + assert!(is_copy::()); + assert!(!is_copy::()); } } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index dc0ad49aa82ac..3ed1c96a1133a 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -846,14 +846,6 @@ pub trait Resource: Send + Sync + 'static { fn get_component_clone_handler() -> ComponentCloneHandler { ComponentCloneHandler::default_handler() } - - /// Called when registering this resource, can be set to `true` for components that are [`Copy`] to enable optimizations - /// when performing resource cloning. - /// # Safety - /// Must return `true` **only** if resource is actually [`Copy`] - unsafe fn is_copy() -> bool { - false - } } // SAFETY: Res only reads a single World resource From a3f52ccbd2850b144bb6eace762da9b5e5184fea Mon Sep 17 00:00:00 2001 From: eugineerd Date: Sat, 11 Jan 2025 13:09:45 +0000 Subject: [PATCH 12/15] fix trying to get capacity in `BlobArray` in release mode --- crates/bevy_ecs/src/storage/blob_array.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_ecs/src/storage/blob_array.rs b/crates/bevy_ecs/src/storage/blob_array.rs index 37689f41e30fe..ed36239b8641a 100644 --- a/crates/bevy_ecs/src/storage/blob_array.rs +++ b/crates/bevy_ecs/src/storage/blob_array.rs @@ -490,6 +490,7 @@ impl BlobArray { // SAFETY: // - other is a valid BlobArray, so it must have a valid array layout let num_bytes_to_copy = array_layout_unchecked(&other.layout(), len).size(); + #[cfg(debug_assertions)] debug_assert!( num_bytes_to_copy <= array_layout(&other.layout(), self.capacity) From ecdd8bdfb85f1b71bef7f81486a83c8902c59e8c Mon Sep 17 00:00:00 2001 From: eugineerd Date: Sat, 11 Jan 2025 13:25:15 +0000 Subject: [PATCH 13/15] add missing `Debug` fields to `ComponentDescriptor` --- crates/bevy_ecs/src/component.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index a04be38931ed1..221c19d88d7e1 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -892,6 +892,8 @@ impl Debug for ComponentDescriptor { .field("type_id", &self.type_id) .field("layout", &self.layout) .field("mutable", &self.mutable) + .field("is_copy", &self.is_copy) + .field("clone_handler", &self.clone_handler) .finish() } } From 0b5daa9005fd39340973e365fa7864d77bf80116 Mon Sep 17 00:00:00 2001 From: eugineerd Date: Sat, 11 Jan 2025 22:50:15 +0000 Subject: [PATCH 14/15] fix docs --- crates/bevy_ecs/src/component.rs | 6 ++---- crates/bevy_ecs/src/entity/clone_entities.rs | 8 ++++---- crates/bevy_ecs/src/storage/mod.rs | 2 +- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 221c19d88d7e1..d7362ba8e7aa5 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -2221,10 +2221,8 @@ pub fn enforce_no_required_components_recursion( } /// Component [clone handler function](ComponentCloneFn) implemented using the [`Clone`] trait. -/// Can be [set](ComponentCloneHandlers::set_component_handler) as clone handler for the specific component it is implemented for. +/// Can be [set](World::set_component_clone_handler) as clone handler for the specific component it is implemented for. /// It will panic if set as handler for any other component. -/// -/// See [`ComponentCloneHandlers`] for more details. pub fn component_clone_via_clone(_world: &World, ctx: &mut ComponentCloneCtx) { if let Some(component) = ctx.read_source_component::() { ctx.write_target_component(component.clone()); @@ -2232,7 +2230,7 @@ pub fn component_clone_via_clone(_world: &World, ctx: &mut C } /// Component [clone handler function](ComponentCloneFn) implemented using reflect. -/// Can be [set](ComponentCloneHandlers::set_component_handler) as clone handler for any registered component, +/// Can be [set](World::set_component_clone_handler) as clone handler for any registered component, /// but only reflected components will be cloned. /// /// To clone a component using this handler, the following must be true: diff --git a/crates/bevy_ecs/src/entity/clone_entities.rs b/crates/bevy_ecs/src/entity/clone_entities.rs index 660c7e744c52f..09e8833c76bd4 100644 --- a/crates/bevy_ecs/src/entity/clone_entities.rs +++ b/crates/bevy_ecs/src/entity/clone_entities.rs @@ -469,7 +469,7 @@ impl EntityCloner { /// /// It should be noted that if `Component` is implemented manually or if `Clone` implementation is conditional /// (like when deriving `Clone` for a type with a generic parameter without `Clone` bound), -/// the component will be cloned using the [default cloning strategy](crate::component::ComponentCloneHandlers::get_default_handler). +/// the component will be cloned using the [default cloning strategy](World::get_default_component_clone_handler). /// To use `Clone`-based handler ([`ComponentCloneHandler::clone_handler`]) in this case it should be set manually using one /// of the methods mentioned in the [Handlers](#handlers) section /// @@ -493,9 +493,9 @@ impl EntityCloner { /// `EntityCloneBuilder` clones entities by cloning components using [`handlers`](ComponentCloneHandler), and there are multiple layers /// to decide which handler to use for which component. The overall hierarchy looks like this (priority from most to least): /// 1. local overrides using [`override_component_clone_handler`](Self::override_component_clone_handler) -/// 2. global overrides using [`set_component_handler`](crate::component::ComponentCloneHandlers::set_component_handler) +/// 2. global overrides using [`set_component_handler`](World::set_component_clone_handler) /// 3. component-defined handler using [`get_component_clone_handler`](Component::get_component_clone_handler) -/// 4. default handler override using [`set_default_handler`](crate::component::ComponentCloneHandlers::set_default_handler) +/// 4. default handler override using [`set_default_handler`](World::set_default_component_clone_handler) /// 5. reflect-based or noop default clone handler depending on if `bevy_reflect` feature is enabled or not. #[derive(Debug)] pub struct EntityCloneBuilder<'w> { @@ -649,7 +649,7 @@ impl<'w> EntityCloneBuilder<'w> { } /// Overrides the [`ComponentCloneHandler`] for a component in this builder. - /// This handler will be used to clone the component instead of the global one defined by [`ComponentCloneHandlers`](crate::component::ComponentCloneHandlers) + /// This handler will be used to clone the component instead of the global one [`World::get_default_component_clone_handler`]. /// /// See [Handlers section of `EntityCloneBuilder`](EntityCloneBuilder#handlers) to understand how this affects handler priority. pub fn override_component_clone_handler( diff --git a/crates/bevy_ecs/src/storage/mod.rs b/crates/bevy_ecs/src/storage/mod.rs index 1583a6459b8a9..fb53b3d194b11 100644 --- a/crates/bevy_ecs/src/storage/mod.rs +++ b/crates/bevy_ecs/src/storage/mod.rs @@ -33,7 +33,7 @@ pub use table::*; use crate::world::{error::WorldCloneError, World}; -/// The raw data stores of a [`World`](crate::world::World) +/// The raw data stores of a [`World`] #[derive(Default)] pub struct Storages { /// Backing storage for [`SparseSet`] components. From 342bbfdcbd5fa06c3725a12676198d8ca6f0b94a Mon Sep 17 00:00:00 2001 From: eugineerd Date: Sat, 11 Jan 2025 22:59:25 +0000 Subject: [PATCH 15/15] fix `no_std` --- crates/bevy_ecs/src/storage/resource.rs | 6 ++---- crates/bevy_ecs/src/storage/table/column.rs | 2 ++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index 74a8b5b32381b..b0465ad012494 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -27,10 +27,6 @@ pub struct ResourceData { data: ManuallyDrop, added_ticks: UnsafeCell, changed_ticks: UnsafeCell, - #[cfg_attr( - not(feature = "std"), - expect(dead_code, reason = "currently only used with the std feature") - )] type_name: String, id: ArchetypeComponentId, #[cfg(feature = "std")] @@ -79,6 +75,7 @@ impl ResourceData { } // Panic in tests, as testing for aborting is nearly impossible + #[cfg(feature = "std")] panic!( "Attempted to access or drop non-send resource {} from thread {:?} on a thread {:?}. This is not allowed. Aborting.", self.type_name, @@ -369,6 +366,7 @@ impl ResourceData { source_component_ptr, target_component_ptr, world, + #[cfg(feature = "bevy_reflect")] type_registry, ); handler(world, &mut ctx); diff --git a/crates/bevy_ecs/src/storage/table/column.rs b/crates/bevy_ecs/src/storage/table/column.rs index 9d9d8014f87ab..72eff67b806d2 100644 --- a/crates/bevy_ecs/src/storage/table/column.rs +++ b/crates/bevy_ecs/src/storage/table/column.rs @@ -396,6 +396,7 @@ impl ThinColumn { source_component_ptr, target_component_ptr, world, + #[cfg(feature = "bevy_reflect")] type_registry, ); handler(world, &mut ctx); @@ -830,6 +831,7 @@ impl Column { source_component_ptr, target_component_ptr, world, + #[cfg(feature = "bevy_reflect")] type_registry, ); handler(world, &mut ctx);