diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index a9887c5248673..075d804d5fca1 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1,5 +1,5 @@ use crate::{ - archetype::{Archetype, ArchetypeId, Archetypes}, + archetype::{Archetype, ArchetypeId}, bundle::{ Bundle, BundleEffect, BundleFromComponents, BundleId, BundleInfo, BundleInserter, DynamicBundle, InsertMode, @@ -10,8 +10,7 @@ use crate::{ StorageType, }, entity::{ - ContainsEntity, Entities, Entity, EntityCloner, EntityClonerBuilder, EntityEquivalent, - EntityLocation, + ContainsEntity, Entity, EntityCloner, EntityClonerBuilder, EntityEquivalent, EntityLocation, }, event::Event, observer::Observer, @@ -2032,10 +2031,8 @@ impl<'w> EntityWorldMut<'w> { ); } - let archetypes = &mut world.archetypes; let storages = &mut world.storages; let components = &mut world.components; - let entities = &mut world.entities; let removed_components = &mut world.removed_components; let entity = self.entity; @@ -2061,46 +2058,38 @@ impl<'w> EntityWorldMut<'w> { }) }; - #[expect( - clippy::undocumented_unsafe_blocks, - reason = "Needs to be documented; see #17345." - )] + // SAFETY: `new_archetype_id` has a subset of the components in the entity's current archetype + // because it was created by removing a bundle from that archetype. unsafe { - Self::move_entity_from_remove::( - entity, - &mut self.location, - old_location.archetype_id, - old_location, - entities, - archetypes, - storages, - new_archetype_id, - ); + self.move_entity_from_remove::(new_archetype_id); } self.world.flush(); self.update_location(); Some(result) } + /// Moves the entity to a new archetype, where the new archetype + /// was a result of removing components from the entity's current archetype. + /// + /// When `DROP` is `true`, removed components will be dropped. + /// + /// When `DROP` is `false`, removed components will be forgotten, making someone else + /// responsible for dropping them. + /// /// # Safety /// /// `new_archetype_id` must have the same or a subset of the components - /// in `old_archetype_id`. Probably more safety stuff too, audit a call to - /// this fn as if the code here was written inline + /// in the entity's current archetype. /// - /// when DROP is true removed components will be dropped otherwise they will be forgotten // We use a const generic here so that we are less reliant on - // inlining for rustc to optimize out the `match DROP` - unsafe fn move_entity_from_remove( - entity: Entity, - self_location: &mut EntityLocation, - old_archetype_id: ArchetypeId, - old_location: EntityLocation, - entities: &mut Entities, - archetypes: &mut Archetypes, - storages: &mut Storages, - new_archetype_id: ArchetypeId, - ) { + // inlining for rustc to optimize out the `if DROP` + unsafe fn move_entity_from_remove(&mut self, new_archetype_id: ArchetypeId) { + let old_archetype_id = self.location.archetype_id; + let old_location = self.location; + let entities = &mut self.world.entities; + let archetypes = &mut self.world.archetypes; + let storages = &mut self.world.storages; + let old_archetype = &mut archetypes[old_archetype_id]; let remove_result = old_archetype.swap_remove(old_location.archetype_row); // if an entity was moved into this entity's archetype row, update its archetype row @@ -2122,7 +2111,7 @@ impl<'w> EntityWorldMut<'w> { let new_archetype = &mut archetypes[new_archetype_id]; let new_location = if old_table_id == new_archetype.table_id() { - new_archetype.allocate(entity, old_table_row) + new_archetype.allocate(self.entity, old_table_row) } else { let (old_table, new_table) = storages .tables @@ -2137,7 +2126,7 @@ impl<'w> EntityWorldMut<'w> { }; // SAFETY: move_result.new_row is a valid position in new_archetype's table - let new_location = unsafe { new_archetype.allocate(entity, move_result.new_row) }; + let new_location = unsafe { new_archetype.allocate(self.entity, move_result.new_row) }; // if an entity was moved into this entity's table row, update its table row if let Some(swapped_entity) = move_result.swapped_entity { @@ -2159,10 +2148,10 @@ impl<'w> EntityWorldMut<'w> { new_location }; - *self_location = new_location; + self.location = new_location; // SAFETY: The entity is valid and has been moved to the new location already. unsafe { - entities.set(entity.index(), new_location); + entities.set(self.entity.index(), new_location); } } @@ -2170,29 +2159,31 @@ impl<'w> EntityWorldMut<'w> { /// /// # Safety /// - A `BundleInfo` with the corresponding `BundleId` must have been initialized. - unsafe fn remove_bundle(&mut self, bundle: BundleId, caller: MaybeLocation) -> EntityLocation { + unsafe fn remove_bundle(&mut self, bundle: BundleId, caller: MaybeLocation) { let entity = self.entity; let world = &mut self.world; let location = self.location; // SAFETY: the caller guarantees that the BundleInfo for this id has been initialized. - let bundle_info = world.bundles.get_unchecked(bundle); + let bundle_info = unsafe { world.bundles.get_unchecked(bundle) }; // SAFETY: `archetype_id` exists because it is referenced in `location` which is valid // and components in `bundle_info` must exist due to this function's safety invariants. - let new_archetype_id = bundle_info - .remove_bundle_from_archetype( - &mut world.archetypes, - &mut world.storages, - &world.components, - &world.observers, - location.archetype_id, - // components from the bundle that are not present on the entity are ignored - true, - ) - .expect("intersections should always return a result"); + let new_archetype_id = unsafe { + bundle_info + .remove_bundle_from_archetype( + &mut world.archetypes, + &mut world.storages, + &world.components, + &world.observers, + location.archetype_id, + // components from the bundle that are not present on the entity are ignored + true, + ) + .expect("intersections should always return a result") + }; if new_archetype_id == location.archetype_id { - return location; + return; } // SAFETY: Archetypes and Bundles cannot be mutably aliased through DeferredWorld @@ -2236,21 +2227,11 @@ impl<'w> EntityWorldMut<'w> { } } - // SAFETY: `new_archetype_id` is a subset of the components in `old_location.archetype_id` - // because it is created by removing a bundle from these components. - let mut new_location = location; - Self::move_entity_from_remove::( - entity, - &mut new_location, - location.archetype_id, - location, - &mut world.entities, - &mut world.archetypes, - &mut world.storages, - new_archetype_id, - ); - - new_location + // SAFETY: `new_archetype_id` has a subset of the components in the entity's current archetype + // because it was created by removing a bundle from that archetype. + unsafe { + self.move_entity_from_remove::(new_archetype_id); + } } /// Removes any components in the [`Bundle`] from the entity. @@ -2274,13 +2255,15 @@ impl<'w> EntityWorldMut<'w> { let mut registrator = unsafe { ComponentsRegistrator::new(&mut self.world.components, &mut self.world.component_ids) }; - let bundle_info = self + let bundle_id = self .world .bundles .register_info::(&mut registrator, storages); - // SAFETY: the `BundleInfo` is initialized above - self.location = unsafe { self.remove_bundle(bundle_info, caller) }; + // SAFETY: the `BundleInfo` for this `bundle_id` is initialized above + unsafe { + self.remove_bundle(bundle_id, caller); + } self.world.flush(); self.update_location(); self @@ -2310,8 +2293,10 @@ impl<'w> EntityWorldMut<'w> { let bundle_id = bundles.register_contributed_bundle_info::(&mut registrator, storages); - // SAFETY: the dynamic `BundleInfo` is initialized above - self.location = unsafe { self.remove_bundle(bundle_id, caller) }; + // SAFETY: the `BundleInfo` for this `bundle_id` is initialized above + unsafe { + self.remove_bundle(bundle_id, caller); + } self.world.flush(); self.update_location(); self @@ -2358,8 +2343,10 @@ impl<'w> EntityWorldMut<'w> { .bundles .init_dynamic_info(&mut self.world.storages, ®istrator, to_remove); - // SAFETY: the `BundleInfo` for the components to remove is initialized above - self.location = unsafe { self.remove_bundle(remove_bundle, caller) }; + // SAFETY: the `BundleInfo` for `remove_bundle` is initialized above + unsafe { + self.remove_bundle(remove_bundle, caller); + } self.world.flush(); self.update_location(); self @@ -2393,8 +2380,10 @@ impl<'w> EntityWorldMut<'w> { component_id, ); - // SAFETY: the `BundleInfo` for this `component_id` is initialized above - self.location = unsafe { self.remove_bundle(bundle_id, caller) }; + // SAFETY: the `BundleInfo` for this `bundle_id` is initialized above + unsafe { + self.remove_bundle(bundle_id, caller); + } self.world.flush(); self.update_location(); self @@ -2420,8 +2409,9 @@ impl<'w> EntityWorldMut<'w> { ); // SAFETY: the `BundleInfo` for this `bundle_id` is initialized above - unsafe { self.remove_bundle(bundle_id, MaybeLocation::caller()) }; - + unsafe { + self.remove_bundle(bundle_id, MaybeLocation::caller()); + } self.world.flush(); self.update_location(); self @@ -2449,8 +2439,10 @@ impl<'w> EntityWorldMut<'w> { component_ids.as_slice(), ); - // SAFETY: the `BundleInfo` for this `component_id` is initialized above - self.location = unsafe { self.remove_bundle(bundle_id, caller) }; + // SAFETY: the `BundleInfo` for this `bundle_id` is initialized above + unsafe { + self.remove_bundle(bundle_id, caller); + } self.world.flush(); self.update_location(); self