diff --git a/benches/benches/bevy_ecs/entity_cloning.rs b/benches/benches/bevy_ecs/entity_cloning.rs index 12343dd2ee248..6d6561c02b836 100644 --- a/benches/benches/bevy_ecs/entity_cloning.rs +++ b/benches/benches/bevy_ecs/entity_cloning.rs @@ -55,7 +55,7 @@ type ComplexBundle = (C1, C2, C3, C4, C5, C6, C7, C8, C9, C10); /// use the [`Reflect`] trait instead of [`Clone`]. fn reflection_cloner( world: &mut World, - recursive: bool, + linked_cloning: bool, ) -> EntityCloner { // Get mutable access to the type registry, creating it if it does not exist yet. let registry = world.get_resource_or_init::(); @@ -77,7 +77,7 @@ fn reflection_cloner( for component in component_ids { builder.override_clone_behavior_with_id(component, ComponentCloneBehavior::reflect()); } - builder.recursive(recursive); + builder.linked_cloning(linked_cloning); builder.finish() } @@ -136,7 +136,7 @@ fn bench_clone_hierarchy( reflection_cloner::(&mut world, true) } else { let mut builder = EntityCloner::build(&mut world); - builder.recursive(true); + builder.linked_cloning(true); builder.finish() }; diff --git a/crates/bevy_ecs/macros/src/component.rs b/crates/bevy_ecs/macros/src/component.rs index b42b5b2f55dbf..182bc8e08eebe 100644 --- a/crates/bevy_ecs/macros/src/component.rs +++ b/crates/bevy_ecs/macros/src/component.rs @@ -71,7 +71,12 @@ pub fn derive_component(input: TokenStream) -> TokenStream { Err(err) => err.into_compile_error().into(), }; - let visit_entities = visit_entities(&ast.data, &bevy_ecs_path, relationship.is_some()); + let visit_entities = visit_entities( + &ast.data, + &bevy_ecs_path, + relationship.is_some(), + relationship_target.is_some(), + ); let storage = storage_path(&bevy_ecs_path, attrs.storage); @@ -207,7 +212,7 @@ pub fn derive_component(input: TokenStream) -> TokenStream { .unwrap_or(quote! { #bevy_ecs_path::component::Mutable }); let clone_behavior = if relationship_target.is_some() { - quote!(#bevy_ecs_path::component::ComponentCloneBehavior::RelationshipTarget(#bevy_ecs_path::relationship::clone_relationship_target::)) + quote!(#bevy_ecs_path::component::ComponentCloneBehavior::Custom(#bevy_ecs_path::relationship::clone_relationship_target::)) } else { quote!( use #bevy_ecs_path::component::{DefaultCloneBehaviorBase, DefaultCloneBehaviorViaClone}; @@ -255,7 +260,12 @@ pub fn derive_component(input: TokenStream) -> TokenStream { }) } -fn visit_entities(data: &Data, bevy_ecs_path: &Path, is_relationship: bool) -> TokenStream2 { +fn visit_entities( + data: &Data, + bevy_ecs_path: &Path, + is_relationship: bool, + is_relationship_target: bool, +) -> TokenStream2 { match data { Data::Struct(DataStruct { fields, .. }) => { let mut visited_fields = Vec::new(); @@ -288,7 +298,9 @@ fn visit_entities(data: &Data, bevy_ecs_path: &Path, is_relationship: bool) -> T } Fields::Unnamed(fields) => { for (index, field) in fields.unnamed.iter().enumerate() { - if field + if index == 0 && is_relationship_target { + visited_indices.push(Index::from(0)); + } else if field .attrs .iter() .any(|a| a.meta.path().is_ident(ENTITIES_ATTR)) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index fcfd514f68838..e91a95a731139 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -18,6 +18,7 @@ use crate::{ observer::Observers, prelude::World, query::DebugCheckedUnwrap, + relationship::RelationshipInsertHookMode, storage::{SparseSetIndex, SparseSets, Storages, Table, TableRow}, world::{unsafe_world_cell::UnsafeWorldCell, EntityWorldMut, ON_ADD, ON_INSERT, ON_REPLACE}, }; @@ -1100,6 +1101,7 @@ impl<'w> BundleInserter<'w> { bundle: T, insert_mode: InsertMode, caller: MaybeLocation, + relationship_insert_hook_mode: RelationshipInsertHookMode, ) -> (EntityLocation, T::Effect) { let bundle_info = self.bundle_info.as_ref(); let archetype_after_insert = self.archetype_after_insert.as_ref(); @@ -1312,6 +1314,7 @@ impl<'w> BundleInserter<'w> { entity, archetype_after_insert.iter_inserted(), caller, + relationship_insert_hook_mode, ); if new_archetype.has_insert_observer() { deferred_world.trigger_observers( @@ -1330,6 +1333,7 @@ impl<'w> BundleInserter<'w> { entity, archetype_after_insert.iter_added(), caller, + relationship_insert_hook_mode, ); if new_archetype.has_insert_observer() { deferred_world.trigger_observers( @@ -1474,6 +1478,7 @@ impl<'w> BundleSpawner<'w> { entity, bundle_info.iter_contributed_components(), caller, + RelationshipInsertHookMode::Run, ); if archetype.has_insert_observer() { deferred_world.trigger_observers( diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index eeb6820bae69d..cf48c7da03e93 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -4,8 +4,9 @@ use crate::{ archetype::ArchetypeFlags, bundle::BundleInfo, change_detection::{MaybeLocation, MAX_CHANGE_AGE}, - entity::{ComponentCloneCtx, Entity}, + entity::{ComponentCloneCtx, Entity, SourceComponent}, query::DebugCheckedUnwrap, + relationship::RelationshipInsertHookMode, resource::Resource, storage::{SparseSetIndex, SparseSets, Table, TableRow}, system::{Commands, Local, SystemParam}, @@ -544,6 +545,8 @@ pub struct HookContext { pub component_id: ComponentId, /// The caller location is `Some` if the `track_caller` feature is enabled. pub caller: MaybeLocation, + /// Configures how relationship hooks will run + pub relationship_insert_hook_mode: RelationshipInsertHookMode, } /// [`World`]-mutating functions that run as part of lifecycle events of a [`Component`]. @@ -1085,7 +1088,7 @@ impl ComponentDescriptor { } /// Function type that can be used to clone an entity. -pub type ComponentCloneFn = fn(&mut Commands, &mut ComponentCloneCtx); +pub type ComponentCloneFn = fn(&mut Commands, &SourceComponent, &mut ComponentCloneCtx); /// The clone behavior to use when cloning a [`Component`]. #[derive(Clone, Debug, Default, PartialEq, Eq)] @@ -1097,11 +1100,6 @@ pub enum ComponentCloneBehavior { Ignore, /// Uses a custom [`ComponentCloneFn`]. Custom(ComponentCloneFn), - /// Uses a [`ComponentCloneFn`] that produces an empty version of the given relationship target. - // TODO: this exists so that the current scene spawning code can know when to skip these components. - // When we move to actually cloning entities in scene spawning code, this should be removed in favor of Custom, as the - // distinction will no longer be necessary. - RelationshipTarget(ComponentCloneFn), } impl ComponentCloneBehavior { @@ -1132,8 +1130,7 @@ impl ComponentCloneBehavior { match self { ComponentCloneBehavior::Default => default, ComponentCloneBehavior::Ignore => component_clone_ignore, - ComponentCloneBehavior::Custom(custom) - | ComponentCloneBehavior::RelationshipTarget(custom) => *custom, + ComponentCloneBehavior::Custom(custom) => *custom, } } } @@ -2166,9 +2163,10 @@ pub fn enforce_no_required_components_recursion( /// pub fn component_clone_via_clone( _commands: &mut Commands, + source: &SourceComponent, ctx: &mut ComponentCloneCtx, ) { - if let Some(component) = ctx.read_source_component::() { + if let Some(component) = source.read::() { ctx.write_target_component(component.clone()); } } @@ -2189,17 +2187,21 @@ pub fn component_clone_via_clone( /// /// See [`EntityClonerBuilder`](crate::entity::EntityClonerBuilder) for details. #[cfg(feature = "bevy_reflect")] -pub fn component_clone_via_reflect(commands: &mut Commands, ctx: &mut ComponentCloneCtx) { +pub fn component_clone_via_reflect( + commands: &mut Commands, + source: &SourceComponent, + ctx: &mut ComponentCloneCtx, +) { let Some(app_registry) = ctx.type_registry().cloned() else { return; }; - let Some(source_component_reflect) = ctx.read_source_component_reflect() else { + let registry = app_registry.read(); + let Some(source_component_reflect) = source.read_reflect(®istry) else { return; }; let component_info = ctx.component_info(); // checked in read_source_component_reflect let type_id = component_info.type_id().unwrap(); - let registry = app_registry.read(); // Try to clone using ReflectFromReflect if let Some(reflect_from_reflect) = @@ -2284,7 +2286,12 @@ pub fn component_clone_via_reflect(commands: &mut Commands, ctx: &mut ComponentC /// Noop implementation of component clone handler function. /// /// See [`EntityClonerBuilder`](crate::entity::EntityClonerBuilder) for details. -pub fn component_clone_ignore(_commands: &mut Commands, _ctx: &mut ComponentCloneCtx) {} +pub fn component_clone_ignore( + _commands: &mut Commands, + _source: &SourceComponent, + _ctx: &mut ComponentCloneCtx, +) { +} /// Wrapper for components clone specialization using autoderef. #[doc(hidden)] diff --git a/crates/bevy_ecs/src/entity/clone_entities.rs b/crates/bevy_ecs/src/entity/clone_entities.rs index 61aa1b9a4e689..2220ad29bc7fe 100644 --- a/crates/bevy_ecs/src/entity/clone_entities.rs +++ b/crates/bevy_ecs/src/entity/clone_entities.rs @@ -1,25 +1,77 @@ -use alloc::{borrow::ToOwned, vec::Vec}; +use alloc::{borrow::ToOwned, collections::VecDeque, vec::Vec}; use bevy_platform_support::collections::{HashMap, HashSet}; use bevy_ptr::{Ptr, PtrMut}; use bumpalo::Bump; -use core::{any::TypeId, ptr::NonNull}; +use core::any::TypeId; #[cfg(feature = "bevy_reflect")] use alloc::boxed::Box; use crate::component::{ComponentCloneBehavior, ComponentCloneFn}; use crate::entity::hash_map::EntityHashMap; -use crate::entity::EntityMapper; +use crate::entity::{Entities, EntityMapper}; +use crate::relationship::RelationshipInsertHookMode; use crate::system::Commands; use crate::{ bundle::Bundle, - component::{Component, ComponentId, ComponentInfo, Components}, + component::{Component, ComponentId, ComponentInfo}, entity::Entity, query::DebugCheckedUnwrap, world::World, }; -use alloc::collections::VecDeque; -use core::cell::RefCell; + +/// Provides read access to the source component (the component being cloned) in a [`ComponentCloneFn`]. +pub struct SourceComponent<'a> { + ptr: Ptr<'a>, + info: &'a ComponentInfo, +} + +impl<'a> SourceComponent<'a> { + /// 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(&self) -> Option<&C> { + if self + .info + .type_id() + .is_some_and(|id| id == TypeId::of::()) + { + // SAFETY: + // - Components and ComponentId are from the same world + // - source_component_ptr holds valid data of the type referenced by ComponentId + unsafe { Some(self.ptr.deref::()) } + } else { + None + } + } + + /// Returns the "raw" pointer to the source component. + pub fn ptr(&self) -> Ptr<'a> { + self.ptr + } + + /// Returns a reference to the component on the source entity as [`&dyn Reflect`](bevy_reflect::Reflect). + /// + /// Will return `None` if: + /// - World does not have [`AppTypeRegistry`](`crate::reflect::AppTypeRegistry`). + /// - Component does not implement [`ReflectFromPtr`](bevy_reflect::ReflectFromPtr). + /// - Component is not registered. + /// - Component does not have [`TypeId`] + /// - Registered [`ReflectFromPtr`](bevy_reflect::ReflectFromPtr)'s [`TypeId`] does not match component's [`TypeId`] + #[cfg(feature = "bevy_reflect")] + pub fn read_reflect( + &self, + registry: &bevy_reflect::TypeRegistry, + ) -> Option<&dyn bevy_reflect::Reflect> { + let type_id = self.info.type_id()?; + let reflect_from_ptr = registry.get_type_data::(type_id)?; + if reflect_from_ptr.type_id() != type_id { + return None; + } + // SAFETY: `source_component_ptr` stores data represented by `component_id`, which we used to get `ReflectFromPtr`. + unsafe { Some(reflect_from_ptr.as_reflect(self.ptr)) } + } +} /// Context for component clone handlers. /// @@ -27,13 +79,12 @@ use core::cell::RefCell; /// and allows component clone handler to get information about component being cloned. pub struct ComponentCloneCtx<'a, 'b> { component_id: ComponentId, - source_component_ptr: Ptr<'a>, target_component_written: bool, bundle_scratch: &'a mut BundleScratch<'b>, bundle_scratch_allocator: &'b Bump, + entities: &'a Entities, source: Entity, target: Entity, - components: &'a Components, component_info: &'a ComponentInfo, entity_cloner: &'a mut EntityCloner, mapper: &'a mut dyn EntityMapper, @@ -49,16 +100,16 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { /// /// # Safety /// Caller must ensure that: - /// - `components` and `component_id` are from the same world. + /// - `component_info` corresponds to the `component_id` in the same world,. /// - `source_component_ptr` points to a valid component of type represented by `component_id`. unsafe fn new( component_id: ComponentId, source: Entity, target: Entity, - source_component_ptr: Ptr<'a>, bundle_scratch_allocator: &'b Bump, bundle_scratch: &'a mut BundleScratch<'b>, - components: &'a Components, + entities: &'a Entities, + component_info: &'a ComponentInfo, entity_cloner: &'a mut EntityCloner, mapper: &'a mut dyn EntityMapper, #[cfg(feature = "bevy_reflect")] type_registry: Option<&'a crate::reflect::AppTypeRegistry>, @@ -68,13 +119,12 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { component_id, source, target, - source_component_ptr, bundle_scratch, target_component_written: false, bundle_scratch_allocator, - components, + entities, mapper, - component_info: components.get_info_unchecked(component_id), + component_info, entity_cloner, type_registry, } @@ -109,8 +159,8 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { /// entities stored in a cloned entity's [`RelationshipTarget`](crate::relationship::RelationshipTarget) component with /// [`RelationshipTarget::LINKED_SPAWN`](crate::relationship::RelationshipTarget::LINKED_SPAWN) will also be cloned. #[inline] - pub fn is_recursive(&self) -> bool { - self.entity_cloner.is_recursive + pub fn linked_cloning(&self) -> bool { + self.entity_cloner.linked_cloning } /// Returns this context's [`EntityMapper`]. @@ -118,44 +168,6 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { self.mapper } - /// 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> { - if self - .component_info - .type_id() - .is_some_and(|id| id == TypeId::of::()) - { - // SAFETY: - // - Components and ComponentId are from the same world - // - source_component_ptr holds valid data of the type referenced by ComponentId - unsafe { Some(self.source_component_ptr.deref::()) } - } else { - None - } - } - - /// Returns a reference to the component on the source entity as [`&dyn Reflect`](bevy_reflect::Reflect). - /// - /// Will return `None` if: - /// - World does not have [`AppTypeRegistry`](`crate::reflect::AppTypeRegistry`). - /// - Component does not implement [`ReflectFromPtr`](bevy_reflect::ReflectFromPtr). - /// - Component is not registered. - /// - Component does not have [`TypeId`] - /// - Registered [`ReflectFromPtr`](bevy_reflect::ReflectFromPtr)'s [`TypeId`] does not match component's [`TypeId`] - #[cfg(feature = "bevy_reflect")] - pub fn read_source_component_reflect(&self) -> Option<&dyn bevy_reflect::Reflect> { - let registry = self.type_registry?.read(); - let type_id = self.component_info.type_id()?; - let reflect_from_ptr = registry.get_type_data::(type_id)?; - if reflect_from_ptr.type_id() != type_id { - return None; - } - // SAFETY: `source_component_ptr` stores data represented by `component_id`, which we used to get `ReflectFromPtr`. - unsafe { Some(reflect_from_ptr.as_reflect(self.source_component_ptr)) } - } - /// Writes component data to target entity. /// /// # Panics @@ -186,33 +198,24 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { self.target_component_written = true; } - /// Writes component data to target entity by providing a pointer to source component data and a pointer to uninitialized target component data. - /// - /// This method allows caller to provide a function (`clone_fn`) to clone component using untyped pointers. - /// First argument to `clone_fn` points to source component data ([`Ptr`]), second argument points to uninitialized buffer ([`NonNull`]) allocated with layout - /// described by [`ComponentInfo`] stored in this [`ComponentCloneCtx`]. If cloning is successful and uninitialized buffer contains a valid clone of - /// source component, `clone_fn` should return `true`, otherwise it should return `false`. + /// Writes component data to target entity by providing a pointer to source component data. /// /// # Safety - /// Caller must ensure that if `clone_fn` is called and returns `true`, the second argument ([`NonNull`] pointer) points to a valid component data - /// described by [`ComponentInfo`] stored in this [`ComponentCloneCtx`]. + /// Caller must ensure that the passed in `ptr` references data that corresponds to the type of the source / target [`ComponentId`]. + /// `ptr` must also contain data that the written component can "own" (for example, this should not directly copy non-Copy data). + /// /// # Panics /// This will panic if component has already been written once. - pub unsafe fn write_target_component_ptr( - &mut self, - clone_fn: impl FnOnce(Ptr, NonNull) -> bool, - ) { + pub unsafe fn write_target_component_ptr(&mut self, ptr: Ptr) { if self.target_component_written { panic!("Trying to write component multiple times") } let layout = self.component_info.layout(); - let target_component_data_ptr = self.bundle_scratch_allocator.alloc_layout(layout); - - if clone_fn(self.source_component_ptr, target_component_data_ptr) { - self.bundle_scratch - .push_ptr(self.component_id, PtrMut::new(target_component_data_ptr)); - self.target_component_written = true; - } + let target_ptr = self.bundle_scratch_allocator.alloc_layout(layout); + core::ptr::copy_nonoverlapping(ptr.as_ptr(), target_ptr.as_ptr(), layout.size()); + self.bundle_scratch + .push_ptr(self.component_id, PtrMut::new(target_ptr)); + self.target_component_written = true; } /// Writes component data to target entity. @@ -259,11 +262,6 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { self.target_component_written = true; } - /// 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. @@ -273,11 +271,10 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> { } /// Queues the `entity` to be cloned by the current [`EntityCloner`] - pub fn queue_entity_clone(&self, entity: Entity) { - self.entity_cloner - .clone_queue - .borrow_mut() - .push_back(entity); + pub fn queue_entity_clone(&mut self, entity: Entity) { + let target = self.entities.reserve_entity(); + self.mapper.set_mapped(entity, target); + self.entity_cloner.clone_queue.push_back(entity); } } @@ -346,9 +343,9 @@ pub struct EntityCloner { filter: HashSet, clone_behavior_overrides: HashMap, move_components: bool, - is_recursive: bool, + linked_cloning: bool, default_clone_fn: ComponentCloneFn, - clone_queue: RefCell>, + clone_queue: VecDeque, } impl Default for EntityCloner { @@ -358,7 +355,7 @@ impl Default for EntityCloner { filter: Default::default(), clone_behavior_overrides: Default::default(), move_components: false, - is_recursive: false, + linked_cloning: false, default_clone_fn: ComponentCloneBehavior::global_default_fn(), clone_queue: Default::default(), } @@ -410,14 +407,20 @@ impl<'a> BundleScratch<'a> { /// /// # Safety /// All [`ComponentId`] values in this instance must come from `world`. - pub(crate) unsafe fn write(self, world: &mut World, entity: Entity) { + pub(crate) unsafe fn write( + self, + world: &mut World, + entity: Entity, + relationship_hook_insert_mode: RelationshipInsertHookMode, + ) { // SAFETY: // - All `component_ids` are from the same world as `target` entity // - All `component_data_ptrs` are valid types represented by `component_ids` unsafe { - world.entity_mut(entity).insert_by_ids( + world.entity_mut(entity).insert_by_ids_internal( &self.component_ids, self.component_ptrs.into_iter().map(|ptr| ptr.promote()), + relationship_hook_insert_mode, ); } } @@ -433,10 +436,11 @@ impl EntityCloner { } } - /// Returns `true` if this cloner is configured to clone entities recursively. + /// Returns `true` if this cloner is configured to clone entities referenced in cloned components via [`RelationshipTarget::LINKED_SPAWN`](crate::relationship::RelationshipTarget::LINKED_SPAWN). + /// This will produce "deep" / recursive clones of relationship trees that have "linked spawn". #[inline] - pub fn is_recursive(&self) -> bool { - self.is_recursive + pub fn linked_cloning(&self) -> bool { + self.linked_cloning } /// Clones and inserts components from the `source` entity into the entity mapped by `mapper` from `source` using the stored configuration. @@ -445,6 +449,7 @@ impl EntityCloner { world: &mut World, source: Entity, mapper: &mut dyn EntityMapper, + relationship_hook_insert_mode: RelationshipInsertHookMode, ) -> Entity { let target = mapper.get_mapped(source); // PERF: reusing allocated space across clones would be more efficient. Consider an allocation model similar to `Commands`. @@ -486,12 +491,20 @@ impl EntityCloner { .unwrap_or(self.default_clone_fn), }; + // SAFETY: This component exists because it is present on the archetype. + let info = unsafe { world.components().get_info_unchecked(component) }; + // SAFETY: // - There are no other mutable references to source entity. // - `component` is from `source_entity`'s archetype let source_component_ptr = unsafe { source_entity.get_by_id(component).debug_checked_unwrap() }; + let source_component = SourceComponent { + info, + ptr: source_component_ptr, + }; + // SAFETY: // - `components` and `component` are from the same world // - `source_component_ptr` is valid and points to the same type as represented by `component` @@ -500,17 +513,17 @@ impl EntityCloner { component, source, target, - source_component_ptr, &bundle_scratch_allocator, &mut bundle_scratch, - world.components(), + world.entities(), + info, self, mapper, app_registry.as_ref(), ) }; - (handler)(&mut commands, &mut ctx); + (handler)(&mut commands, &source_component, &mut ctx); } } @@ -529,12 +542,12 @@ impl EntityCloner { // SAFETY: // - All `component_ids` are from the same world as `target` entity // - All `component_data_ptrs` are valid types represented by `component_ids` - unsafe { bundle_scratch.write(world, target) }; + unsafe { bundle_scratch.write(world, target, relationship_hook_insert_mode) }; target } /// Clones and inserts components from the `source` entity into `target` entity using the stored configuration. - /// If this [`EntityCloner`] has [`EntityCloner::is_recursive`], then it will recursively spawn entities as defined + /// If this [`EntityCloner`] has [`EntityCloner::linked_cloning`], then it will recursively spawn entities as defined /// by [`RelationshipTarget`](crate::relationship::RelationshipTarget) components with /// [`RelationshipTarget::LINKED_SPAWN`](crate::relationship::RelationshipTarget::LINKED_SPAWN) #[track_caller] @@ -545,7 +558,7 @@ impl EntityCloner { } /// Clones and inserts components from the `source` entity into a newly spawned entity using the stored configuration. - /// If this [`EntityCloner`] has [`EntityCloner::is_recursive`], then it will recursively spawn entities as defined + /// If this [`EntityCloner`] has [`EntityCloner::linked_cloning`], then it will recursively spawn entities as defined /// by [`RelationshipTarget`](crate::relationship::RelationshipTarget) components with /// [`RelationshipTarget::LINKED_SPAWN`](crate::relationship::RelationshipTarget::LINKED_SPAWN) #[track_caller] @@ -563,13 +576,22 @@ impl EntityCloner { source: Entity, mapper: &mut dyn EntityMapper, ) -> Entity { - let target = self.clone_entity_internal(world, source, mapper); + // All relationships on the root should have their hooks run + let target = + self.clone_entity_internal(world, source, mapper, RelationshipInsertHookMode::Run); + let child_hook_insert_mode = if self.linked_cloning { + // When spawning "linked relationships", we want to ignore hooks for relationships we are spawning, while + // still registering with original relationship targets that are "not linked" to the current recursive spawn. + RelationshipInsertHookMode::RunIfNotLinked + } else { + // If we are not cloning "linked relationships" recursively, then we want any cloned relationship components to + // register themselves with their original relationship target. + RelationshipInsertHookMode::Run + }; loop { - let queued = self.clone_queue.borrow_mut().pop_front(); + let queued = self.clone_queue.pop_front(); if let Some(queued) = queued { - let target = world.entities.reserve_entity(); - mapper.set_mapped(queued, target); - self.clone_entity_internal(world, queued, mapper); + self.clone_entity_internal(world, queued, mapper, child_hook_insert_mode); } else { break; } @@ -764,10 +786,10 @@ impl<'w> EntityClonerBuilder<'w> { self } - /// If `true`, makes the built [`EntityCloner`] recursively clone entities, as defined by - /// [`RelationshipTarget::LINKED_SPAWN`](crate::relationship::RelationshipTarget::LINKED_SPAWN). - pub fn recursive(&mut self, is_recursive: bool) -> &mut Self { - self.entity_cloner.is_recursive = is_recursive; + /// When true this cloner will be configured to clone entities referenced in cloned components via [`RelationshipTarget::LINKED_SPAWN`](crate::relationship::RelationshipTarget::LINKED_SPAWN). + /// This will produce "deep" / recursive clones of relationship trees that have "linked spawn". + pub fn linked_cloning(&mut self, linked_cloning: bool) -> &mut Self { + self.entity_cloner.linked_cloning = linked_cloning; self } @@ -817,10 +839,9 @@ mod tests { use super::ComponentCloneCtx; use crate::{ component::{Component, ComponentCloneBehavior, ComponentDescriptor, StorageType}, - entity::{hash_map::EntityHashMap, Entity, EntityCloner}, + entity::{hash_map::EntityHashMap, Entity, EntityCloner, SourceComponent}, prelude::{ChildOf, Children, Resource}, - reflect::AppTypeRegistry, - reflect::{ReflectComponent, ReflectFromWorld}, + reflect::{AppTypeRegistry, ReflectComponent, ReflectFromWorld}, system::Commands, world::{FromWorld, World}, }; @@ -835,7 +856,7 @@ mod tests { use super::*; use crate::{ component::{Component, ComponentCloneBehavior}, - entity::EntityCloner, + entity::{EntityCloner, SourceComponent}, reflect::{AppTypeRegistry, ReflectComponent, ReflectFromWorld}, system::Commands, }; @@ -939,8 +960,13 @@ mod tests { #[derive(Component, Reflect)] struct B; - fn test_handler(_commands: &mut Commands, ctx: &mut ComponentCloneCtx) { - assert!(ctx.read_source_component_reflect().is_none()); + fn test_handler( + _commands: &mut Commands, + source: &SourceComponent, + ctx: &mut ComponentCloneCtx, + ) { + let registry = ctx.type_registry().unwrap(); + assert!(source.read_reflect(®istry.read()).is_none()); } let mut world = World::default(); @@ -1230,17 +1256,14 @@ mod tests { #[test] fn clone_entity_with_dynamic_components() { const COMPONENT_SIZE: usize = 10; - fn test_handler(_commands: &mut Commands, ctx: &mut ComponentCloneCtx) { - // SAFETY: this handler is only going to be used with a component represented by [u8; COMPONENT_SIZE] + fn test_handler( + _commands: &mut Commands, + source: &SourceComponent, + ctx: &mut ComponentCloneCtx, + ) { + // SAFETY: the passed in ptr corresponds to copy-able data that matches the type of the source / target component unsafe { - ctx.write_target_component_ptr(move |source_ptr, target_ptr| { - core::ptr::copy_nonoverlapping( - source_ptr.as_ptr(), - target_ptr.as_ptr(), - COMPONENT_SIZE, - ); - true - }); + ctx.write_target_component_ptr(source.ptr()); } } @@ -1297,7 +1320,7 @@ mod tests { let clone_root = world.spawn_empty().id(); EntityCloner::build(&mut world) - .recursive(true) + .linked_cloning(true) .clone_entity(root, clone_root); let root_children = world diff --git a/crates/bevy_ecs/src/observer/entity_observer.rs b/crates/bevy_ecs/src/observer/entity_observer.rs index 556ca0d8aa523..32afa41cc8f8e 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, ComponentCloneBehavior, ComponentHook, HookContext, Mutable, StorageType, }, - entity::{ComponentCloneCtx, Entity, EntityClonerBuilder}, + entity::{ComponentCloneCtx, Entity, EntityClonerBuilder, SourceComponent}, observer::ObserverState, system::Commands, world::World, @@ -64,7 +64,11 @@ impl EntityClonerBuilder<'_> { } } -fn component_clone_observed_by(commands: &mut Commands, ctx: &mut ComponentCloneCtx) { +fn component_clone_observed_by( + commands: &mut Commands, + _source: &SourceComponent, + ctx: &mut ComponentCloneCtx, +) { let target = ctx.target(); let source = ctx.source(); diff --git a/crates/bevy_ecs/src/reflect/bundle.rs b/crates/bevy_ecs/src/reflect/bundle.rs index b7acf69d6aad9..6216adf40c2f2 100644 --- a/crates/bevy_ecs/src/reflect/bundle.rs +++ b/crates/bevy_ecs/src/reflect/bundle.rs @@ -11,6 +11,7 @@ use crate::{ bundle::BundleFromComponents, entity::EntityMapper, prelude::Bundle, + relationship::RelationshipInsertHookMode, world::{EntityMut, EntityWorldMut}, }; use bevy_reflect::{ @@ -36,8 +37,13 @@ pub struct ReflectBundleFns { /// Function pointer implementing [`ReflectBundle::apply`]. pub apply: fn(EntityMut, &dyn PartialReflect, &TypeRegistry), /// Function pointer implementing [`ReflectBundle::apply_or_insert_mapped`]. - pub apply_or_insert_mapped: - fn(&mut EntityWorldMut, &dyn PartialReflect, &TypeRegistry, &mut dyn EntityMapper), + pub apply_or_insert_mapped: fn( + &mut EntityWorldMut, + &dyn PartialReflect, + &TypeRegistry, + &mut dyn EntityMapper, + RelationshipInsertHookMode, + ), /// Function pointer implementing [`ReflectBundle::remove`]. pub remove: fn(&mut EntityWorldMut), /// Function pointer implementing [`ReflectBundle::take`]. @@ -87,8 +93,15 @@ impl ReflectBundle { bundle: &dyn PartialReflect, registry: &TypeRegistry, mapper: &mut dyn EntityMapper, + relationship_insert_hook_mode: RelationshipInsertHookMode, ) { - (self.0.apply_or_insert_mapped)(entity, bundle, registry, mapper); + (self.0.apply_or_insert_mapped)( + entity, + bundle, + registry, + mapper, + relationship_insert_hook_mode, + ); } /// Removes this [`Bundle`] type from the entity. Does nothing if it doesn't exist. @@ -170,32 +183,46 @@ impl FromType for Refl } } }, - apply_or_insert_mapped: |entity, reflected_bundle, registry, mapper| { - if let Some(reflect_component) = - registry.get_type_data::(TypeId::of::()) - { - reflect_component.apply_or_insert_mapped( - entity, - reflected_bundle, - registry, - mapper, - ); - } else { - match reflected_bundle.reflect_ref() { - ReflectRef::Struct(bundle) => bundle.iter_fields().for_each(|field| { - apply_or_insert_field_mapped(entity, field, registry, mapper); - }), - ReflectRef::Tuple(bundle) => bundle.iter_fields().for_each(|field| { - apply_or_insert_field_mapped(entity, field, registry, mapper); - }), - _ => panic!( - "expected bundle `{}` to be a named struct or tuple", - // FIXME: once we have unique reflect, use `TypePath`. - core::any::type_name::(), - ), + apply_or_insert_mapped: + |entity, reflected_bundle, registry, mapper, relationship_insert_hook_mode| { + if let Some(reflect_component) = + registry.get_type_data::(TypeId::of::()) + { + reflect_component.apply_or_insert_mapped( + entity, + reflected_bundle, + registry, + mapper, + relationship_insert_hook_mode, + ); + } else { + match reflected_bundle.reflect_ref() { + ReflectRef::Struct(bundle) => bundle.iter_fields().for_each(|field| { + apply_or_insert_field_mapped( + entity, + field, + registry, + mapper, + relationship_insert_hook_mode, + ); + }), + ReflectRef::Tuple(bundle) => bundle.iter_fields().for_each(|field| { + apply_or_insert_field_mapped( + entity, + field, + registry, + mapper, + relationship_insert_hook_mode, + ); + }), + _ => panic!( + "expected bundle `{}` to be a named struct or tuple", + // FIXME: once we have unique reflect, use `TypePath`. + core::any::type_name::(), + ), + } } - } - }, + }, remove: |entity| { entity.remove::(); }, @@ -232,6 +259,7 @@ fn apply_or_insert_field_mapped( field: &dyn PartialReflect, registry: &TypeRegistry, mapper: &mut dyn EntityMapper, + relationship_insert_hook_mode: RelationshipInsertHookMode, ) { let Some(type_id) = field.try_as_reflect().map(Any::type_id) else { panic!( @@ -241,9 +269,21 @@ fn apply_or_insert_field_mapped( }; if let Some(reflect_component) = registry.get_type_data::(type_id) { - reflect_component.apply_or_insert_mapped(entity, field, registry, mapper); + reflect_component.apply_or_insert_mapped( + entity, + field, + registry, + mapper, + relationship_insert_hook_mode, + ); } else if let Some(reflect_bundle) = registry.get_type_data::(type_id) { - reflect_bundle.apply_or_insert_mapped(entity, field, registry, mapper); + reflect_bundle.apply_or_insert_mapped( + entity, + field, + registry, + mapper, + relationship_insert_hook_mode, + ); } else { let is_component = entity.world().components().get_id(type_id).is_some(); diff --git a/crates/bevy_ecs/src/reflect/component.rs b/crates/bevy_ecs/src/reflect/component.rs index bffd2e9c290b7..da772b079fc5b 100644 --- a/crates/bevy_ecs/src/reflect/component.rs +++ b/crates/bevy_ecs/src/reflect/component.rs @@ -63,6 +63,7 @@ use crate::{ component::{ComponentId, ComponentMutability}, entity::{Entity, EntityMapper}, prelude::Component, + relationship::RelationshipInsertHookMode, world::{ unsafe_world_cell::UnsafeEntityCell, EntityMut, EntityWorldMut, FilteredEntityMut, FilteredEntityRef, World, @@ -105,8 +106,13 @@ pub struct ReflectComponentFns { /// Function pointer implementing [`ReflectComponent::apply()`]. pub apply: fn(EntityMut, &dyn PartialReflect), /// Function pointer implementing [`ReflectComponent::apply_or_insert_mapped()`]. - pub apply_or_insert_mapped: - fn(&mut EntityWorldMut, &dyn PartialReflect, &TypeRegistry, &mut dyn EntityMapper), + pub apply_or_insert_mapped: fn( + &mut EntityWorldMut, + &dyn PartialReflect, + &TypeRegistry, + &mut dyn EntityMapper, + RelationshipInsertHookMode, + ), /// Function pointer implementing [`ReflectComponent::remove()`]. pub remove: fn(&mut EntityWorldMut), /// Function pointer implementing [`ReflectComponent::contains()`]. @@ -174,8 +180,15 @@ impl ReflectComponent { component: &dyn PartialReflect, registry: &TypeRegistry, map: &mut dyn EntityMapper, + relationship_insert_hook_mode: RelationshipInsertHookMode, ) { - (self.0.apply_or_insert_mapped)(entity, component, registry, map); + (self.0.apply_or_insert_mapped)( + entity, + component, + registry, + map, + relationship_insert_hook_mode, + ); } /// Removes this [`Component`] type from the entity. Does nothing if it doesn't exist. @@ -320,31 +333,40 @@ impl FromType for ReflectComponent { let mut component = unsafe { entity.get_mut_assume_mutable::() }.unwrap(); component.apply(reflected_component); }, - apply_or_insert_mapped: |entity, reflected_component, registry, mapper| { - // TODO: if we can externalize this impl to cut down on monomorphization that would be great - let map_fn = move |entity: &mut Entity| { - *entity = mapper.get_mapped(*entity); - }; - if C::Mutability::MUTABLE { - // SAFETY: guard ensures `C` is a mutable component - if let Some(mut component) = unsafe { entity.get_mut_assume_mutable::() } { - component.apply(reflected_component.as_partial_reflect()); - C::visit_entities_mut(&mut component, map_fn); + apply_or_insert_mapped: + |entity, reflected_component, registry, mapper, relationship_insert_hook_mode| { + let map_fn = map_function(mapper); + if C::Mutability::MUTABLE { + // SAFETY: guard ensures `C` is a mutable component + if let Some(mut component) = unsafe { entity.get_mut_assume_mutable::() } + { + component.apply(reflected_component.as_partial_reflect()); + C::visit_entities_mut(&mut component, map_fn); + } else { + let mut component = entity.world_scope(|world| { + from_reflect_with_fallback::( + reflected_component, + world, + registry, + ) + }); + C::visit_entities_mut(&mut component, map_fn); + entity.insert_with_relationship_insert_hook_mode( + component, + relationship_insert_hook_mode, + ); + } } else { let mut component = entity.world_scope(|world| { from_reflect_with_fallback::(reflected_component, world, registry) }); C::visit_entities_mut(&mut component, map_fn); - entity.insert(component); + entity.insert_with_relationship_insert_hook_mode( + component, + relationship_insert_hook_mode, + ); } - } else { - let mut component = entity.world_scope(|world| { - from_reflect_with_fallback::(reflected_component, world, registry) - }); - C::visit_entities_mut(&mut component, map_fn); - entity.insert(component); - } - }, + }, remove: |entity| { entity.remove::(); }, @@ -397,3 +419,9 @@ impl FromType for ReflectComponent { }) } } + +fn map_function(mapper: &mut dyn EntityMapper) -> impl FnMut(&mut Entity) + '_ { + move |entity: &mut Entity| { + *entity = mapper.get_mapped(*entity); + } +} diff --git a/crates/bevy_ecs/src/relationship/mod.rs b/crates/bevy_ecs/src/relationship/mod.rs index da9e66d1a8060..e7f35240bdc90 100644 --- a/crates/bevy_ecs/src/relationship/mod.rs +++ b/crates/bevy_ecs/src/relationship/mod.rs @@ -12,7 +12,7 @@ pub use relationship_source_collection::*; use crate::{ component::{Component, HookContext, Mutable}, - entity::{ComponentCloneCtx, Entity}, + entity::{ComponentCloneCtx, Entity, SourceComponent}, system::{ command::HandleError, entity_command::{self, CommandWithEntity}, @@ -85,7 +85,24 @@ pub trait Relationship: Component + Sized { fn from(entity: Entity) -> Self; /// The `on_insert` component hook that maintains the [`Relationship`] / [`RelationshipTarget`] connection. - fn on_insert(mut world: DeferredWorld, HookContext { entity, caller, .. }: HookContext) { + fn on_insert( + mut world: DeferredWorld, + HookContext { + entity, + caller, + relationship_insert_hook_mode, + .. + }: HookContext, + ) { + match relationship_insert_hook_mode { + RelationshipInsertHookMode::Run => {} + RelationshipInsertHookMode::Skip => return, + RelationshipInsertHookMode::RunIfNotLinked => { + if ::LINKED_SPAWN { + return; + } + } + } let target_entity = world.entity(entity).get::().unwrap().get(); if target_entity == entity { warn!( @@ -155,7 +172,7 @@ pub type SourceIter<'w, R> = /// A [`Component`] containing the collection of entities that relate to this [`Entity`] via the associated `Relationship` type. /// See the [`Relationship`] documentation for more information. pub trait RelationshipTarget: Component + Sized { - /// If this is true, when despawning or cloning (when [recursion is enabled](crate::entity::EntityClonerBuilder::recursive)), the related entities targeting this entity will also be despawned or cloned. + /// If this is true, when despawning or cloning (when [linked cloning is enabled](crate::entity::EntityClonerBuilder::linked_cloning)), the related entities targeting this entity will also be despawned or cloned. /// /// For example, this is set to `true` for Bevy's built-in parent-child relation, defined by [`ChildOf`](crate::prelude::ChildOf) and [`Children`](crate::prelude::Children). /// This means that when a parent is despawned, any children targeting that parent are also despawned (and the same applies to cloning). @@ -284,18 +301,33 @@ pub trait RelationshipTarget: Component + Sized { /// to spawn recursively. pub fn clone_relationship_target( _commands: &mut Commands, + source: &SourceComponent, context: &mut ComponentCloneCtx, ) { - if let Some(component) = context.read_source_component::() { - if context.is_recursive() && T::LINKED_SPAWN { + if let Some(component) = source.read::() { + let mut cloned = T::with_capacity(component.len()); + if context.linked_cloning() && T::LINKED_SPAWN { + let collection = cloned.collection_mut_risky(); for entity in component.iter() { + collection.add(entity); context.queue_entity_clone(entity); } } - context.write_target_component(T::with_capacity(component.len())); + context.write_target_component(cloned); } } +/// Configures the conditions under which the Relationship insert hook will be run. +#[derive(Copy, Clone, Debug)] +pub enum RelationshipInsertHookMode { + /// Relationship insert hooks will always run + Run, + /// Relationship insert hooks will run if [`RelationshipTarget::LINKED_SPAWN`] is false + RunIfNotLinked, + /// Relationship insert hooks will always be skipped + Skip, +} + #[cfg(test)] mod tests { use crate::world::World; diff --git a/crates/bevy_ecs/src/relationship/relationship_source_collection.rs b/crates/bevy_ecs/src/relationship/relationship_source_collection.rs index 013cdd63aaf2b..b543b5d1e605e 100644 --- a/crates/bevy_ecs/src/relationship/relationship_source_collection.rs +++ b/crates/bevy_ecs/src/relationship/relationship_source_collection.rs @@ -143,27 +143,6 @@ mod tests { assert_eq!(collection, &alloc::vec!(a)); } - #[test] - fn entity_hash_set_relationship_source_collection() { - #[derive(Component)] - #[relationship(relationship_target = RelTarget)] - struct Rel(Entity); - - #[derive(Component)] - #[relationship_target(relationship = Rel, linked_spawn)] - struct RelTarget(EntityHashSet); - - let mut world = World::new(); - let a = world.spawn_empty().id(); - let b = world.spawn_empty().id(); - - world.entity_mut(a).insert(Rel(b)); - - let rel_target = world.get::(b).unwrap(); - let collection = rel_target.collection(); - assert_eq!(collection, &EntityHashSet::from([a])); - } - #[test] fn smallvec_relationship_source_collection() { #[derive(Component)] diff --git a/crates/bevy_ecs/src/system/commands/entity_command.rs b/crates/bevy_ecs/src/system/commands/entity_command.rs index ab0c47bf4e0e5..541fe865d5cda 100644 --- a/crates/bevy_ecs/src/system/commands/entity_command.rs +++ b/crates/bevy_ecs/src/system/commands/entity_command.rs @@ -14,6 +14,7 @@ use crate::{ component::{Component, ComponentId, ComponentInfo}, entity::{Entity, EntityClonerBuilder}, event::Event, + relationship::RelationshipInsertHookMode, result::Result, system::{command::HandleError, Command, IntoObserverSystem}, world::{error::EntityMutableFetchError, EntityWorldMut, FromWorld, World}, @@ -156,7 +157,7 @@ where pub fn insert(bundle: impl Bundle, mode: InsertMode) -> impl EntityCommand { let caller = MaybeLocation::caller(); move |mut entity: EntityWorldMut| { - entity.insert_with_caller(bundle, mode, caller); + entity.insert_with_caller(bundle, mode, caller, RelationshipInsertHookMode::Run); } } @@ -178,7 +179,13 @@ pub unsafe fn insert_by_id( // - `component_id` safety is ensured by the caller // - `ptr` is valid within the `make` block OwningPtr::make(value, |ptr| unsafe { - entity.insert_by_id_with_caller(component_id, ptr, mode, caller); + entity.insert_by_id_with_caller( + component_id, + ptr, + mode, + caller, + RelationshipInsertHookMode::Run, + ); }); } } @@ -190,7 +197,7 @@ pub fn insert_from_world(mode: InsertMode) -> impl Ent let caller = MaybeLocation::caller(); move |mut entity: EntityWorldMut| { let value = entity.world_scope(|world| T::from_world(world)); - entity.insert_with_caller(value, mode, caller); + entity.insert_with_caller(value, mode, caller, RelationshipInsertHookMode::Run); } } diff --git a/crates/bevy_ecs/src/world/deferred_world.rs b/crates/bevy_ecs/src/world/deferred_world.rs index 581f1af25e71e..adaab018ec758 100644 --- a/crates/bevy_ecs/src/world/deferred_world.rs +++ b/crates/bevy_ecs/src/world/deferred_world.rs @@ -9,6 +9,7 @@ use crate::{ observer::{Observers, TriggerTargets}, prelude::{Component, QueryState}, query::{QueryData, QueryFilter}, + relationship::RelationshipInsertHookMode, resource::Resource, system::{Commands, Query}, traversal::Traversal, @@ -159,6 +160,7 @@ impl<'w> DeferredWorld<'w> { entity, [component_id].into_iter(), MaybeLocation::caller(), + RelationshipInsertHookMode::Run, ); if archetype.has_insert_observer() { self.trigger_observers( @@ -515,6 +517,7 @@ impl<'w> DeferredWorld<'w> { entity, component_id, caller, + relationship_insert_hook_mode: RelationshipInsertHookMode::Run, }, ); } @@ -533,6 +536,7 @@ impl<'w> DeferredWorld<'w> { entity: Entity, targets: impl Iterator, caller: MaybeLocation, + relationship_insert_hook_mode: RelationshipInsertHookMode, ) { if archetype.has_insert_hook() { for component_id in targets { @@ -545,6 +549,7 @@ impl<'w> DeferredWorld<'w> { entity, component_id, caller, + relationship_insert_hook_mode, }, ); } @@ -575,6 +580,7 @@ impl<'w> DeferredWorld<'w> { entity, component_id, caller, + relationship_insert_hook_mode: RelationshipInsertHookMode::Run, }, ); } @@ -605,6 +611,7 @@ impl<'w> DeferredWorld<'w> { entity, component_id, caller, + relationship_insert_hook_mode: RelationshipInsertHookMode::Run, }, ); } @@ -635,6 +642,7 @@ impl<'w> DeferredWorld<'w> { entity, component_id, caller, + relationship_insert_hook_mode: RelationshipInsertHookMode::Run, }, ); } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index b4fe20b3a4d3d..eccc318d271ff 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -13,6 +13,7 @@ use crate::{ event::Event, observer::Observer, query::{Access, ReadOnlyQueryData}, + relationship::RelationshipInsertHookMode, removal_detection::RemovedComponentEvents, resource::Resource, storage::Storages, @@ -1526,7 +1527,40 @@ impl<'w> EntityWorldMut<'w> { /// If the entity has been despawned while this `EntityWorldMut` is still alive. #[track_caller] pub fn insert(&mut self, bundle: T) -> &mut Self { - self.insert_with_caller(bundle, InsertMode::Replace, MaybeLocation::caller()) + self.insert_with_caller( + bundle, + InsertMode::Replace, + MaybeLocation::caller(), + RelationshipInsertHookMode::Run, + ) + } + + /// Adds a [`Bundle`] of components to the entity. + /// [`Relationship`](crate::relationship::Relationship) components in the bundle will follow the configuration + /// in `relationship_insert_hook_mode`. + /// + /// This will overwrite any previous value(s) of the same component type. + /// + /// # Warning + /// + /// This can easily break the integrity of relationships. This is intended to be used for cloning and spawning code internals, + /// not most user-facing scenarios. + /// + /// # Panics + /// + /// If the entity has been despawned while this `EntityWorldMut` is still alive. + #[track_caller] + pub fn insert_with_relationship_insert_hook_mode( + &mut self, + bundle: T, + relationship_insert_hook_mode: RelationshipInsertHookMode, + ) -> &mut Self { + self.insert_with_caller( + bundle, + InsertMode::Replace, + MaybeLocation::caller(), + relationship_insert_hook_mode, + ) } /// Adds a [`Bundle`] of components to the entity without overwriting. @@ -1539,7 +1573,12 @@ impl<'w> EntityWorldMut<'w> { /// If the entity has been despawned while this `EntityWorldMut` is still alive. #[track_caller] pub fn insert_if_new(&mut self, bundle: T) -> &mut Self { - self.insert_with_caller(bundle, InsertMode::Keep, MaybeLocation::caller()) + self.insert_with_caller( + bundle, + InsertMode::Keep, + MaybeLocation::caller(), + RelationshipInsertHookMode::Run, + ) } /// Split into a new function so we can pass the calling location into the function when using @@ -1550,14 +1589,23 @@ impl<'w> EntityWorldMut<'w> { bundle: T, mode: InsertMode, caller: MaybeLocation, + relationship_insert_hook_mode: RelationshipInsertHookMode, ) -> &mut Self { self.assert_not_despawned(); let change_tick = self.world.change_tick(); let mut bundle_inserter = BundleInserter::new::(self.world, self.location.archetype_id, change_tick); // SAFETY: location matches current entity. `T` matches `bundle_info` - let (location, after_effect) = - unsafe { bundle_inserter.insert(self.entity, self.location, bundle, mode, caller) }; + let (location, after_effect) = unsafe { + bundle_inserter.insert( + self.entity, + self.location, + bundle, + mode, + caller, + relationship_insert_hook_mode, + ) + }; self.location = location; self.world.flush(); self.update_location(); @@ -1590,6 +1638,7 @@ impl<'w> EntityWorldMut<'w> { component, InsertMode::Replace, MaybeLocation::caller(), + RelationshipInsertHookMode::Run, ) } @@ -1604,6 +1653,7 @@ impl<'w> EntityWorldMut<'w> { component: OwningPtr<'_>, mode: InsertMode, caller: MaybeLocation, + relationship_hook_insert_mode: RelationshipInsertHookMode, ) -> &mut Self { self.assert_not_despawned(); let change_tick = self.world.change_tick(); @@ -1629,6 +1679,7 @@ impl<'w> EntityWorldMut<'w> { Some(storage_type).iter().cloned(), mode, caller, + relationship_hook_insert_mode, ); self.world.flush(); self.update_location(); @@ -1656,6 +1707,20 @@ impl<'w> EntityWorldMut<'w> { &mut self, component_ids: &[ComponentId], iter_components: I, + ) -> &mut Self { + self.insert_by_ids_internal( + component_ids, + iter_components, + RelationshipInsertHookMode::Run, + ) + } + + #[track_caller] + pub(crate) unsafe fn insert_by_ids_internal<'a, I: Iterator>>( + &mut self, + component_ids: &[ComponentId], + iter_components: I, + relationship_hook_insert_mode: RelationshipInsertHookMode, ) -> &mut Self { self.assert_not_despawned(); let change_tick = self.world.change_tick(); @@ -1681,6 +1746,7 @@ impl<'w> EntityWorldMut<'w> { (*storage_types).iter().cloned(), InsertMode::Replace, MaybeLocation::caller(), + relationship_hook_insert_mode, ); *self.world.bundles.get_storages_unchecked(bundle_id) = core::mem::take(&mut storage_types); self.world.flush(); @@ -4165,6 +4231,7 @@ unsafe fn insert_dynamic_bundle< storage_types: S, mode: InsertMode, caller: MaybeLocation, + relationship_hook_insert_mode: RelationshipInsertHookMode, ) -> EntityLocation { struct DynamicInsertBundle<'a, I: Iterator)>> { components: I, @@ -4186,7 +4253,14 @@ unsafe fn insert_dynamic_bundle< // SAFETY: location matches current entity. unsafe { bundle_inserter - .insert(entity, location, bundle, mode, caller) + .insert( + entity, + location, + bundle, + mode, + caller, + relationship_hook_insert_mode, + ) .0 } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 294145de23049..10db34bac3cde 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -48,6 +48,7 @@ use crate::{ event::{Event, EventId, Events, SendBatchIds}, observer::Observers, query::{DebugCheckedUnwrap, QueryData, QueryFilter, QueryState}, + relationship::RelationshipInsertHookMode, removal_detection::RemovedComponentEvents, resource::Resource, schedule::{Schedule, ScheduleLabel, Schedules}, @@ -2219,6 +2220,7 @@ impl World { bundle, InsertMode::Replace, caller, + RelationshipInsertHookMode::Run, ) }; } @@ -2240,6 +2242,7 @@ impl World { bundle, InsertMode::Replace, caller, + RelationshipInsertHookMode::Run, ) }; spawn_or_insert = @@ -2374,6 +2377,7 @@ impl World { first_bundle, insert_mode, caller, + RelationshipInsertHookMode::Run, ) }; @@ -2395,9 +2399,14 @@ impl World { } // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter unsafe { - cache - .inserter - .insert(entity, location, bundle, insert_mode, caller) + cache.inserter.insert( + entity, + location, + bundle, + insert_mode, + caller, + RelationshipInsertHookMode::Run, + ) }; } else { panic!("error[B0003]: Could not insert a bundle (of type `{}`) for entity {entity}, which {}. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::(), self.entities.entity_does_not_exist_error_details(entity)); @@ -2515,6 +2524,7 @@ impl World { first_bundle, insert_mode, caller, + RelationshipInsertHookMode::Run, ) }; break Some(cache); @@ -2545,9 +2555,14 @@ impl World { } // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter unsafe { - cache - .inserter - .insert(entity, location, bundle, insert_mode, caller) + cache.inserter.insert( + entity, + location, + bundle, + insert_mode, + caller, + RelationshipInsertHookMode::Run, + ) }; } else { invalid_entities.push(entity); diff --git a/crates/bevy_scene/src/dynamic_scene.rs b/crates/bevy_scene/src/dynamic_scene.rs index be45b54a811a9..de96cd707c72e 100644 --- a/crates/bevy_scene/src/dynamic_scene.rs +++ b/crates/bevy_scene/src/dynamic_scene.rs @@ -11,6 +11,7 @@ use bevy_reflect::{PartialReflect, TypePath, TypeRegistry}; #[cfg(feature = "serialize")] use crate::serde::SceneSerializer; use bevy_ecs::component::ComponentCloneBehavior; +use bevy_ecs::relationship::RelationshipInsertHookMode; #[cfg(feature = "serialize")] use serde::Serialize; @@ -110,10 +111,8 @@ impl DynamicScene { #[expect(unsafe_code, reason = "this is faster")] let component_info = unsafe { world.components().get_info_unchecked(component_id) }; - match component_info.clone_behavior() { - ComponentCloneBehavior::Ignore - | ComponentCloneBehavior::RelationshipTarget(_) => continue, - _ => {} + if *component_info.clone_behavior() == ComponentCloneBehavior::Ignore { + continue; } } @@ -123,6 +122,7 @@ impl DynamicScene { component.as_partial_reflect(), &type_registry, mapper, + RelationshipInsertHookMode::Skip, ); }); } diff --git a/crates/bevy_scene/src/scene.rs b/crates/bevy_scene/src/scene.rs index 9040b15e85f7c..7cbfb597712a6 100644 --- a/crates/bevy_scene/src/scene.rs +++ b/crates/bevy_scene/src/scene.rs @@ -7,6 +7,7 @@ use bevy_ecs::{ entity::{hash_map::EntityHashMap, Entity, SceneEntityMapper}, entity_disabling::DefaultQueryFilters, reflect::{AppTypeRegistry, ReflectComponent, ReflectResource}, + relationship::RelationshipInsertHookMode, world::World, }; use bevy_reflect::{PartialReflect, TypePath}; @@ -124,10 +125,8 @@ impl Scene { .get_info(component_id) .expect("component_ids in archetypes should have ComponentInfo"); - match component_info.clone_behavior() { - ComponentCloneBehavior::Ignore - | ComponentCloneBehavior::RelationshipTarget(_) => continue, - _ => {} + if *component_info.clone_behavior() == ComponentCloneBehavior::Ignore { + continue; } let registration = type_registry @@ -157,6 +156,7 @@ impl Scene { component.as_partial_reflect(), &type_registry, mapper, + RelationshipInsertHookMode::Skip, ); }); } diff --git a/crates/bevy_scene/src/scene_spawner.rs b/crates/bevy_scene/src/scene_spawner.rs index cf8195e676d37..4470179f2ee1a 100644 --- a/crates/bevy_scene/src/scene_spawner.rs +++ b/crates/bevy_scene/src/scene_spawner.rs @@ -870,4 +870,73 @@ mod tests { app.update(); check(app.world_mut(), 0); } + + #[test] + fn scene_child_order_preserved_when_archetype_order_mismatched() { + let mut app = App::new(); + + app.add_plugins(ScheduleRunnerPlugin::default()) + .add_plugins(AssetPlugin::default()) + .add_plugins(ScenePlugin) + .register_type::() + .register_type::(); + app.update(); + + let mut scene_world = World::new(); + let root = scene_world.spawn_empty().id(); + let temporary_root = scene_world.spawn_empty().id(); + // Spawn entities with different parent first before parenting them to the actual root, allowing us + // to decouple child order from archetype-creation-order + let child1 = scene_world + .spawn(( + ChildOf { + parent: temporary_root, + }, + ComponentA { x: 1.0, y: 1.0 }, + )) + .id(); + let child2 = scene_world + .spawn(( + ChildOf { + parent: temporary_root, + }, + ComponentA { x: 2.0, y: 2.0 }, + )) + .id(); + // the "first" child is intentionally spawned with a different component to force it into a "newer" archetype, + // meaning it will be iterated later in the spawn code. + let child0 = scene_world + .spawn(( + ChildOf { + parent: temporary_root, + }, + ComponentF, + )) + .id(); + + scene_world + .entity_mut(root) + .add_children(&[child0, child1, child2]); + + let scene = Scene::new(scene_world); + let scene_handle = app.world_mut().resource_mut::>().add(scene); + + let spawned = app.world_mut().spawn(SceneRoot(scene_handle.clone())).id(); + + app.update(); + let world = app.world_mut(); + + let spawned_root = world.entity(spawned).get::().unwrap()[0]; + let children = world.entity(spawned_root).get::().unwrap(); + assert_eq!(children.len(), 3); + assert!(world.entity(children[0]).get::().is_some()); + assert_eq!( + world.entity(children[1]).get::().unwrap().x, + 1.0 + ); + assert_eq!( + world.entity(children[2]).get::().unwrap().x, + 2.0 + ); + } } diff --git a/examples/ecs/component_hooks.rs b/examples/ecs/component_hooks.rs index ba9606fe57dc6..11c469ad1506a 100644 --- a/examples/ecs/component_hooks.rs +++ b/examples/ecs/component_hooks.rs @@ -79,6 +79,7 @@ fn setup(world: &mut World) { entity, component_id, caller, + .. }| { // You can access component data from within the hook let value = world.get::(entity).unwrap().0; @@ -116,6 +117,7 @@ fn setup(world: &mut World) { entity, component_id, caller, + .. }| { let value = world.get::(entity).unwrap().0; println!(