From d132239bb1fb4fcfeded438e1eeef03ee8de5d3f Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Thu, 12 Dec 2024 13:24:13 -0600 Subject: [PATCH] Misc. docs and renames for niche ECS internals (#16786) ## Objective Some structs and methods in the ECS internals have names that don't describe their purpose very well, and sometimes don't have docs either. Also, the function `remove_bundle_from_archetype` is a counterpart to `BundleInfo::add_bundle_to_archetype`, but isn't a method and is in a different file. ## Solution - Renamed the following structs and added docs: | Before | After | |----------------------|------------------------------| | `AddBundle` | `ArchetypeAfterBundleInsert` | | `InsertBundleResult` | `ArchetypeMoveType` | - Renamed the following methods: | Before | After | |---------------------------------------|----------------------------------------------| | `Edges::get_add_bundle` | `Edges::get_archetype_after_bundle_insert` | | `Edges::insert_add_bundle` | `Edges::cache_archetype_after_bundle_insert` | | `Edges::get_remove_bundle` | `Edges::get_archetype_after_bundle_remove` | | `Edges::insert_remove_bundle` | `Edges::cache_archetype_after_bundle_remove` | | `Edges::get_take_bundle` | `Edges::get_archetype_after_bundle_take` | | `Edges::insert_take_bundle` | `Edges::cache_archetype_after_bundle_take` | - Moved `remove_bundle_from_archetype` from `world/entity_ref.rs` to `BundleInfo`. I left the function in entity_ref in the first commit for comparison, look there for the diff of comments and whatnot. - Tidied up docs: - General grammar and spacing. - Made the usage of "insert" and "add" more consistent. - Removed references to information that isn't there. - Renamed `BundleInfo::add_bundle_to_archetype` to `BundleInfo::insert_bundle_into_archetype` for consistency. --- crates/bevy_ecs/src/archetype.rs | 97 ++++---- crates/bevy_ecs/src/bundle.rs | 317 ++++++++++++++++++------ crates/bevy_ecs/src/world/entity_ref.rs | 168 +------------ 3 files changed, 311 insertions(+), 271 deletions(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index 022e550948d66..c117fbe09dc61 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -109,7 +109,7 @@ impl ArchetypeId { } } -/// Used in [`AddBundle`] to track whether components in the bundle are newly +/// Used in [`ArchetypeAfterBundleInsert`] to track whether components in the bundle are newly /// added or already existed in the entity's archetype. #[derive(Copy, Clone, Eq, PartialEq)] pub(crate) enum ComponentStatus { @@ -117,11 +117,12 @@ pub(crate) enum ComponentStatus { Existing, } -pub(crate) struct AddBundle { - /// The target archetype after the bundle is added to the source archetype +/// Used in [`Edges`] to cache the result of inserting a bundle into the source archetype. +pub(crate) struct ArchetypeAfterBundleInsert { + /// The target archetype after the bundle is inserted into the source archetype. pub archetype_id: ArchetypeId, /// For each component iterated in the same order as the source [`Bundle`](crate::bundle::Bundle), - /// indicate if the component is newly added to the target archetype or if it already existed + /// indicate if the component is newly added to the target archetype or if it already existed. pub bundle_status: Vec, /// The set of additional required components that must be initialized immediately when adding this Bundle. /// @@ -134,7 +135,7 @@ pub(crate) struct AddBundle { pub existing: Vec, } -impl AddBundle { +impl ArchetypeAfterBundleInsert { pub(crate) fn iter_inserted(&self) -> impl Iterator + Clone + '_ { self.added.iter().chain(self.existing.iter()).copied() } @@ -149,10 +150,10 @@ impl AddBundle { } /// This trait is used to report the status of [`Bundle`](crate::bundle::Bundle) components -/// being added to a given entity, relative to that entity's original archetype. +/// being inserted into a given entity, relative to that entity's original archetype. /// See [`crate::bundle::BundleInfo::write_components`] for more info. pub(crate) trait BundleComponentStatus { - /// Returns the Bundle's component status for the given "bundle index" + /// Returns the Bundle's component status for the given "bundle index". /// /// # Safety /// Callers must ensure that index is always a valid bundle index for the @@ -160,7 +161,7 @@ pub(crate) trait BundleComponentStatus { unsafe fn get_status(&self, index: usize) -> ComponentStatus; } -impl BundleComponentStatus for AddBundle { +impl BundleComponentStatus for ArchetypeAfterBundleInsert { #[inline] unsafe fn get_status(&self, index: usize) -> ComponentStatus { // SAFETY: caller has ensured index is a valid bundle index for this bundle @@ -173,7 +174,7 @@ pub(crate) struct SpawnBundleStatus; impl BundleComponentStatus for SpawnBundleStatus { #[inline] unsafe fn get_status(&self, _index: usize) -> ComponentStatus { - // Components added during a spawn call are always treated as added + // Components inserted during a spawn call are always treated as added. ComponentStatus::Added } } @@ -194,37 +195,36 @@ impl BundleComponentStatus for SpawnBundleStatus { /// [`World`]: crate::world::World #[derive(Default)] pub struct Edges { - add_bundle: SparseArray, + insert_bundle: SparseArray, remove_bundle: SparseArray>, take_bundle: SparseArray>, } impl Edges { - /// Checks the cache for the target archetype when adding a bundle to the - /// source archetype. For more information, see [`EntityWorldMut::insert`]. + /// Checks the cache for the target archetype when inserting a bundle into the + /// source archetype. /// /// If this returns `None`, it means there has not been a transition from /// the source archetype via the provided bundle. - /// - /// [`EntityWorldMut::insert`]: crate::world::EntityWorldMut::insert #[inline] - pub fn get_add_bundle(&self, bundle_id: BundleId) -> Option { - self.get_add_bundle_internal(bundle_id) + pub fn get_archetype_after_bundle_insert(&self, bundle_id: BundleId) -> Option { + self.get_archetype_after_bundle_insert_internal(bundle_id) .map(|bundle| bundle.archetype_id) } - /// Internal version of `get_add_bundle` that fetches the full `AddBundle`. + /// Internal version of `get_archetype_after_bundle_insert` that + /// fetches the full `ArchetypeAfterBundleInsert`. #[inline] - pub(crate) fn get_add_bundle_internal(&self, bundle_id: BundleId) -> Option<&AddBundle> { - self.add_bundle.get(bundle_id) + pub(crate) fn get_archetype_after_bundle_insert_internal( + &self, + bundle_id: BundleId, + ) -> Option<&ArchetypeAfterBundleInsert> { + self.insert_bundle.get(bundle_id) } - /// Caches the target archetype when adding a bundle to the source archetype. - /// For more information, see [`EntityWorldMut::insert`]. - /// - /// [`EntityWorldMut::insert`]: crate::world::EntityWorldMut::insert + /// Caches the target archetype when inserting a bundle into the source archetype. #[inline] - pub(crate) fn insert_add_bundle( + pub(crate) fn cache_archetype_after_bundle_insert( &mut self, bundle_id: BundleId, archetype_id: ArchetypeId, @@ -233,9 +233,9 @@ impl Edges { added: Vec, existing: Vec, ) { - self.add_bundle.insert( + self.insert_bundle.insert( bundle_id, - AddBundle { + ArchetypeAfterBundleInsert { archetype_id, bundle_status, required_components, @@ -245,27 +245,25 @@ impl Edges { ); } - /// Checks the cache for the target archetype when removing a bundle to the - /// source archetype. For more information, see [`EntityWorldMut::remove`]. + /// Checks the cache for the target archetype when removing a bundle from the + /// source archetype. /// /// If this returns `None`, it means there has not been a transition from /// the source archetype via the provided bundle. /// /// If this returns `Some(None)`, it means that the bundle cannot be removed /// from the source archetype. - /// - /// [`EntityWorldMut::remove`]: crate::world::EntityWorldMut::remove #[inline] - pub fn get_remove_bundle(&self, bundle_id: BundleId) -> Option> { + pub fn get_archetype_after_bundle_remove( + &self, + bundle_id: BundleId, + ) -> Option> { self.remove_bundle.get(bundle_id).cloned() } - /// Caches the target archetype when removing a bundle to the source archetype. - /// For more information, see [`EntityWorldMut::remove`]. - /// - /// [`EntityWorldMut::remove`]: crate::world::EntityWorldMut::remove + /// Caches the target archetype when removing a bundle from the source archetype. #[inline] - pub(crate) fn insert_remove_bundle( + pub(crate) fn cache_archetype_after_bundle_remove( &mut self, bundle_id: BundleId, archetype_id: Option, @@ -273,24 +271,31 @@ impl Edges { self.remove_bundle.insert(bundle_id, archetype_id); } - /// Checks the cache for the target archetype when removing a bundle to the - /// source archetype. For more information, see [`EntityWorldMut::remove`]. + /// Checks the cache for the target archetype when taking a bundle from the + /// source archetype. + /// + /// Unlike `remove`, `take` will only succeed if the source archetype + /// contains all of the components in the bundle. /// /// If this returns `None`, it means there has not been a transition from /// the source archetype via the provided bundle. /// - /// [`EntityWorldMut::remove`]: crate::world::EntityWorldMut::remove + /// If this returns `Some(None)`, it means that the bundle cannot be taken + /// from the source archetype. #[inline] - pub fn get_take_bundle(&self, bundle_id: BundleId) -> Option> { + pub fn get_archetype_after_bundle_take( + &self, + bundle_id: BundleId, + ) -> Option> { self.take_bundle.get(bundle_id).cloned() } - /// Caches the target archetype when removing a bundle to the source archetype. - /// For more information, see [`EntityWorldMut::take`]. + /// Caches the target archetype when taking a bundle from the source archetype. /// - /// [`EntityWorldMut::take`]: crate::world::EntityWorldMut::take + /// Unlike `remove`, `take` will only succeed if the source archetype + /// contains all of the components in the bundle. #[inline] - pub(crate) fn insert_take_bundle( + pub(crate) fn cache_archetype_after_bundle_take( &mut self, bundle_id: BundleId, archetype_id: Option, @@ -577,11 +582,11 @@ impl Archetype { self.entities.reserve(additional); } - /// Removes the entity at `index` by swapping it out. Returns the table row the entity is stored + /// Removes the entity at `row` by swapping it out. Returns the table row the entity is stored /// in. /// /// # Panics - /// This function will panic if `index >= self.len()` + /// This function will panic if `row >= self.entities.len()` #[inline] pub(crate) fn swap_remove(&mut self, row: ArchetypeRow) -> ArchetypeSwapRemoveResult { let is_last = row.index() == self.entities.len() - 1; diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 8c552b3763eff..4a49fc39f1aa5 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -6,8 +6,8 @@ pub use bevy_ecs_macros::Bundle; use crate::{ archetype::{ - AddBundle, Archetype, ArchetypeId, Archetypes, BundleComponentStatus, ComponentStatus, - SpawnBundleStatus, + Archetype, ArchetypeAfterBundleInsert, ArchetypeId, Archetypes, BundleComponentStatus, + ComponentStatus, SpawnBundleStatus, }, component::{ Component, ComponentId, Components, RequiredComponentConstructor, RequiredComponents, @@ -488,13 +488,16 @@ impl BundleInfo { /// # Safety /// /// `bundle_component_status` must return the "correct" [`ComponentStatus`] for each component - /// in the [`Bundle`], with respect to the entity's original archetype (prior to the bundle being added) + /// in the [`Bundle`], with respect to the entity's original archetype (prior to the bundle being added). + /// /// For example, if the original archetype already has `ComponentA` and `T` also has `ComponentA`, the status - /// should be `Mutated`. If the original archetype does not have `ComponentA`, the status should be `Added`. - /// When "inserting" a bundle into an existing entity, [`AddBundle`] - /// should be used, which will report `Added` vs `Mutated` status based on the current archetype's structure. + /// should be `Existing`. If the original archetype does not have `ComponentA`, the status should be `Added`. + /// + /// When "inserting" a bundle into an existing entity, [`ArchetypeAfterBundleInsert`] + /// should be used, which will report `Added` vs `Existing` status based on the current archetype's structure. + /// /// When spawning a bundle, [`SpawnBundleStatus`] can be used instead, which removes the need - /// to look up the [`AddBundle`] in the archetype graph, which requires + /// to look up the [`ArchetypeAfterBundleInsert`] in the archetype graph, which requires /// ownership of the entity's current archetype. /// /// `table` must be the "new" table for `entity`. `table_row` must have space allocated for the @@ -634,12 +637,15 @@ impl BundleInfo { } } - /// Adds a bundle to the given archetype and returns the resulting archetype. This could be the - /// same [`ArchetypeId`], in the event that adding the given bundle does not result in an - /// [`Archetype`] change. Results are cached in the [`Archetype`] graph to avoid redundant work. + /// Inserts a bundle into the given archetype and returns the resulting archetype. + /// This could be the same [`ArchetypeId`], in the event that inserting the given bundle + /// does not result in an [`Archetype`] change. + /// + /// Results are cached in the [`Archetype`] graph to avoid redundant work. + /// /// # Safety /// `components` must be the same components as passed in [`Self::new`] - pub(crate) unsafe fn add_bundle_to_archetype( + pub(crate) unsafe fn insert_bundle_into_archetype( &self, archetypes: &mut Archetypes, storages: &mut Storages, @@ -647,8 +653,11 @@ impl BundleInfo { observers: &Observers, archetype_id: ArchetypeId, ) -> ArchetypeId { - if let Some(add_bundle_id) = archetypes[archetype_id].edges().get_add_bundle(self.id) { - return add_bundle_id; + if let Some(archetype_after_insert_id) = archetypes[archetype_id] + .edges() + .get_archetype_after_bundle_insert(self.id) + { + return archetype_after_insert_id; } let mut new_table_components = Vec::new(); let mut new_sparse_set_components = Vec::new(); @@ -693,8 +702,8 @@ impl BundleInfo { if new_table_components.is_empty() && new_sparse_set_components.is_empty() { let edges = current_archetype.edges_mut(); - // the archetype does not change when we add this bundle - edges.insert_add_bundle( + // The archetype does not change when we insert this bundle. + edges.cache_archetype_after_bundle_insert( self.id, archetype_id, bundle_status, @@ -707,16 +716,16 @@ impl BundleInfo { let table_id; let table_components; let sparse_set_components; - // the archetype changes when we add this bundle. prepare the new archetype and storages + // The archetype changes when we insert this bundle. Prepare the new archetype and storages. { let current_archetype = &archetypes[archetype_id]; table_components = if new_table_components.is_empty() { - // if there are no new table components, we can keep using this table + // If there are no new table components, we can keep using this table. table_id = current_archetype.table_id(); current_archetype.table_components().collect() } else { new_table_components.extend(current_archetype.table_components()); - // sort to ignore order while hashing + // Sort to ignore order while hashing. new_table_components.sort_unstable(); // SAFETY: all component ids in `new_table_components` exist table_id = unsafe { @@ -732,7 +741,7 @@ impl BundleInfo { current_archetype.sparse_set_components().collect() } else { new_sparse_set_components.extend(current_archetype.sparse_set_components()); - // sort to ignore order while hashing + // Sort to ignore order while hashing. new_sparse_set_components.sort_unstable(); new_sparse_set_components }; @@ -745,36 +754,156 @@ impl BundleInfo { table_components, sparse_set_components, ); - // add an edge from the old archetype to the new archetype - archetypes[archetype_id].edges_mut().insert_add_bundle( - self.id, - new_archetype_id, - bundle_status, - added_required_components, - added, - existing, - ); + // Add an edge from the old archetype to the new archetype. + archetypes[archetype_id] + .edges_mut() + .cache_archetype_after_bundle_insert( + self.id, + new_archetype_id, + bundle_status, + added_required_components, + added, + existing, + ); new_archetype_id } } + + /// Removes a bundle from the given archetype and returns the resulting archetype + /// (or `None` if the removal was invalid). + /// This could be the same [`ArchetypeId`], in the event that removing the given bundle + /// does not result in an [`Archetype`] change. + /// + /// Results are cached in the [`Archetype`] graph to avoid redundant work. + /// + /// If `intersection` is false, attempting to remove a bundle with components not contained in the + /// current archetype will fail, returning `None`. + /// + /// If `intersection` is true, components in the bundle but not in the current archetype + /// will be ignored. + /// + /// # Safety + /// `archetype_id` must exist and components in `bundle_info` must exist + pub(crate) unsafe fn remove_bundle_from_archetype( + &self, + archetypes: &mut Archetypes, + storages: &mut Storages, + components: &Components, + observers: &Observers, + archetype_id: ArchetypeId, + intersection: bool, + ) -> Option { + // Check the archetype graph to see if the bundle has been + // removed from this archetype in the past. + let archetype_after_remove_result = { + let edges = archetypes[archetype_id].edges(); + if intersection { + edges.get_archetype_after_bundle_remove(self.id()) + } else { + edges.get_archetype_after_bundle_take(self.id()) + } + }; + let result = if let Some(result) = archetype_after_remove_result { + // This bundle removal result is cached. Just return that! + result + } else { + let mut next_table_components; + let mut next_sparse_set_components; + let next_table_id; + { + let current_archetype = &mut archetypes[archetype_id]; + let mut removed_table_components = Vec::new(); + let mut removed_sparse_set_components = Vec::new(); + for component_id in self.iter_explicit_components() { + if current_archetype.contains(component_id) { + // SAFETY: bundle components were already initialized by bundles.get_info + let component_info = unsafe { components.get_info_unchecked(component_id) }; + match component_info.storage_type() { + StorageType::Table => removed_table_components.push(component_id), + StorageType::SparseSet => { + removed_sparse_set_components.push(component_id); + } + } + } else if !intersection { + // A component in the bundle was not present in the entity's archetype, so this + // removal is invalid. Cache the result in the archetype graph. + current_archetype + .edges_mut() + .cache_archetype_after_bundle_take(self.id(), None); + return None; + } + } + + // Sort removed components so we can do an efficient "sorted remove". + // Archetype components are already sorted. + removed_table_components.sort_unstable(); + removed_sparse_set_components.sort_unstable(); + next_table_components = current_archetype.table_components().collect(); + next_sparse_set_components = current_archetype.sparse_set_components().collect(); + sorted_remove(&mut next_table_components, &removed_table_components); + sorted_remove( + &mut next_sparse_set_components, + &removed_sparse_set_components, + ); + + next_table_id = if removed_table_components.is_empty() { + current_archetype.table_id() + } else { + // SAFETY: all components in next_table_components exist + unsafe { + storages + .tables + .get_id_or_insert(&next_table_components, components) + } + }; + } + + let new_archetype_id = archetypes.get_id_or_insert( + components, + observers, + next_table_id, + next_table_components, + next_sparse_set_components, + ); + Some(new_archetype_id) + }; + let current_archetype = &mut archetypes[archetype_id]; + // Cache the result in an edge. + if intersection { + current_archetype + .edges_mut() + .cache_archetype_after_bundle_remove(self.id(), result); + } else { + current_archetype + .edges_mut() + .cache_archetype_after_bundle_take(self.id(), result); + } + result + } } // SAFETY: We have exclusive world access so our pointers can't be invalidated externally pub(crate) struct BundleInserter<'w> { world: UnsafeWorldCell<'w>, bundle_info: ConstNonNull, - add_bundle: ConstNonNull, + archetype_after_insert: ConstNonNull, table: NonNull, archetype: NonNull, - result: InsertBundleResult, + archetype_move_type: ArchetypeMoveType, change_tick: Tick, } -pub(crate) enum InsertBundleResult { +/// The type of archetype move (or lack thereof) that will result from a bundle +/// being inserted into an entity. +pub(crate) enum ArchetypeMoveType { + /// If the entity already has all of the components that are being inserted, + /// its archetype won't change. SameArchetype, - NewArchetypeSameTable { - new_archetype: NonNull, - }, + /// If only [`sparse set`](StorageType::SparseSet) components are being added, + /// the entity's archetype will change while keeping the same table. + NewArchetypeSameTable { new_archetype: NonNull }, + /// If any [`table-stored`](StorageType::Table) components are being added, + /// both the entity's archetype and table will change. NewArchetypeNewTable { new_archetype: NonNull, new_table: NonNull
, @@ -809,7 +938,7 @@ impl<'w> BundleInserter<'w> { // SAFETY: We will not make any accesses to the command queue, component or resource data of this world let bundle_info = world.bundles.get_unchecked(bundle_id); let bundle_id = bundle_info.id(); - let new_archetype_id = bundle_info.add_bundle_to_archetype( + let new_archetype_id = bundle_info.insert_bundle_into_archetype( &mut world.archetypes, &mut world.storages, &world.components, @@ -818,32 +947,32 @@ impl<'w> BundleInserter<'w> { ); if new_archetype_id == archetype_id { let archetype = &mut world.archetypes[archetype_id]; - // SAFETY: The edge is assured to be initialized when we called add_bundle_to_archetype - let add_bundle = unsafe { + // SAFETY: The edge is assured to be initialized when we called insert_bundle_into_archetype + let archetype_after_insert = unsafe { archetype .edges() - .get_add_bundle_internal(bundle_id) + .get_archetype_after_bundle_insert_internal(bundle_id) .debug_checked_unwrap() }; let table_id = archetype.table_id(); let table = &mut world.storages.tables[table_id]; Self { - add_bundle: add_bundle.into(), + archetype_after_insert: archetype_after_insert.into(), archetype: archetype.into(), bundle_info: bundle_info.into(), table: table.into(), - result: InsertBundleResult::SameArchetype, + archetype_move_type: ArchetypeMoveType::SameArchetype, change_tick, world: world.as_unsafe_world_cell(), } } else { let (archetype, new_archetype) = world.archetypes.get_2_mut(archetype_id, new_archetype_id); - // SAFETY: The edge is assured to be initialized when we called add_bundle_to_archetype - let add_bundle = unsafe { + // SAFETY: The edge is assured to be initialized when we called insert_bundle_into_archetype + let archetype_after_insert = unsafe { archetype .edges() - .get_add_bundle_internal(bundle_id) + .get_archetype_after_bundle_insert_internal(bundle_id) .debug_checked_unwrap() }; let table_id = archetype.table_id(); @@ -851,11 +980,11 @@ impl<'w> BundleInserter<'w> { if table_id == new_table_id { let table = &mut world.storages.tables[table_id]; Self { - add_bundle: add_bundle.into(), + archetype_after_insert: archetype_after_insert.into(), archetype: archetype.into(), bundle_info: bundle_info.into(), table: table.into(), - result: InsertBundleResult::NewArchetypeSameTable { + archetype_move_type: ArchetypeMoveType::NewArchetypeSameTable { new_archetype: new_archetype.into(), }, change_tick, @@ -864,11 +993,11 @@ impl<'w> BundleInserter<'w> { } else { let (table, new_table) = world.storages.tables.get_2_mut(table_id, new_table_id); Self { - add_bundle: add_bundle.into(), + archetype_after_insert: archetype_after_insert.into(), archetype: archetype.into(), bundle_info: bundle_info.into(), table: table.into(), - result: InsertBundleResult::NewArchetypeNewTable { + archetype_move_type: ArchetypeMoveType::NewArchetypeNewTable { new_archetype: new_archetype.into(), new_table: new_table.into(), }, @@ -892,7 +1021,7 @@ impl<'w> BundleInserter<'w> { #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, ) -> EntityLocation { let bundle_info = self.bundle_info.as_ref(); - let add_bundle = self.add_bundle.as_ref(); + let archetype_after_insert = self.archetype_after_insert.as_ref(); let table = self.table.as_mut(); let archetype = self.archetype.as_ref(); @@ -907,10 +1036,14 @@ impl<'w> BundleInserter<'w> { deferred_world.trigger_observers( ON_REPLACE, entity, - add_bundle.iter_existing(), + archetype_after_insert.iter_existing(), ); } - deferred_world.trigger_on_replace(archetype, entity, add_bundle.iter_existing()); + deferred_world.trigger_on_replace( + archetype, + entity, + archetype_after_insert.iter_existing(), + ); } } @@ -918,8 +1051,8 @@ impl<'w> BundleInserter<'w> { // so this reference can only be promoted from shared to &mut down here, after they have been ran let archetype = self.archetype.as_mut(); - let (new_archetype, new_location) = match &mut self.result { - InsertBundleResult::SameArchetype => { + let (new_archetype, new_location) = match &mut self.archetype_move_type { + ArchetypeMoveType::SameArchetype => { // SAFETY: Mutable references do not alias and will be dropped after this block let sparse_sets = { let world = self.world.world_mut(); @@ -929,8 +1062,8 @@ impl<'w> BundleInserter<'w> { bundle_info.write_components( table, sparse_sets, - add_bundle, - add_bundle.required_components.iter(), + archetype_after_insert, + archetype_after_insert.required_components.iter(), entity, location.table_row, self.change_tick, @@ -942,7 +1075,7 @@ impl<'w> BundleInserter<'w> { (archetype, location) } - InsertBundleResult::NewArchetypeSameTable { new_archetype } => { + ArchetypeMoveType::NewArchetypeSameTable { new_archetype } => { let new_archetype = new_archetype.as_mut(); // SAFETY: Mutable references do not alias and will be dropped after this block @@ -971,8 +1104,8 @@ impl<'w> BundleInserter<'w> { bundle_info.write_components( table, sparse_sets, - add_bundle, - add_bundle.required_components.iter(), + archetype_after_insert, + archetype_after_insert.required_components.iter(), entity, result.table_row, self.change_tick, @@ -984,7 +1117,7 @@ impl<'w> BundleInserter<'w> { (new_archetype, new_location) } - InsertBundleResult::NewArchetypeNewTable { + ArchetypeMoveType::NewArchetypeNewTable { new_archetype, new_table, } => { @@ -1022,7 +1155,7 @@ impl<'w> BundleInserter<'w> { let new_location = new_archetype.allocate(entity, move_result.new_row); entities.set(entity.index(), new_location); - // if an entity was moved into this entity's table spot, update its table row + // If an entity was moved into this entity's table spot, update its table row. if let Some(swapped_entity) = move_result.swapped_entity { let swapped_location = // SAFETY: If the swap was successful, swapped_entity must be valid. @@ -1054,8 +1187,8 @@ impl<'w> BundleInserter<'w> { bundle_info.write_components( new_table, sparse_sets, - add_bundle, - add_bundle.required_components.iter(), + archetype_after_insert, + archetype_after_insert.required_components.iter(), entity, move_result.new_row, self.change_tick, @@ -1076,39 +1209,47 @@ impl<'w> BundleInserter<'w> { // SAFETY: All components in the bundle are guaranteed to exist in the World // as they must be initialized before creating the BundleInfo. unsafe { - deferred_world.trigger_on_add(new_archetype, entity, add_bundle.iter_added()); + deferred_world.trigger_on_add( + new_archetype, + entity, + archetype_after_insert.iter_added(), + ); if new_archetype.has_add_observer() { - deferred_world.trigger_observers(ON_ADD, entity, add_bundle.iter_added()); + deferred_world.trigger_observers( + ON_ADD, + entity, + archetype_after_insert.iter_added(), + ); } match insert_mode { InsertMode::Replace => { - // insert triggers for both new and existing components if we're replacing them + // Insert triggers for both new and existing components if we're replacing them. deferred_world.trigger_on_insert( new_archetype, entity, - add_bundle.iter_inserted(), + archetype_after_insert.iter_inserted(), ); if new_archetype.has_insert_observer() { deferred_world.trigger_observers( ON_INSERT, entity, - add_bundle.iter_inserted(), + archetype_after_insert.iter_inserted(), ); } } InsertMode::Keep => { - // insert triggers only for new components if we're not replacing them (since + // Insert triggers only for new components if we're not replacing them (since // nothing is actually inserted). deferred_world.trigger_on_insert( new_archetype, entity, - add_bundle.iter_added(), + archetype_after_insert.iter_added(), ); if new_archetype.has_insert_observer() { deferred_world.trigger_observers( ON_INSERT, entity, - add_bundle.iter_added(), + archetype_after_insert.iter_added(), ); } } @@ -1155,7 +1296,7 @@ impl<'w> BundleSpawner<'w> { change_tick: Tick, ) -> Self { let bundle_info = world.bundles.get_unchecked(bundle_id); - let new_archetype_id = bundle_info.add_bundle_to_archetype( + let new_archetype_id = bundle_info.insert_bundle_into_archetype( &mut world.archetypes, &mut world.storages, &world.components, @@ -1482,6 +1623,21 @@ fn initialize_dynamic_bundle( (id, storage_types) } +fn sorted_remove(source: &mut Vec, remove: &[T]) { + let mut remove_index = 0; + source.retain(|value| { + while remove_index < remove.len() && *value > remove[remove_index] { + remove_index += 1; + } + + if remove_index < remove.len() { + *value != remove[remove_index] + } else { + true + } + }); +} + #[cfg(test)] mod tests { use crate as bevy_ecs; @@ -1678,4 +1834,25 @@ mod tests { assert!(entity.contains::()); assert_eq!(entity.get(), Some(&V("one"))); } + + #[test] + fn sorted_remove() { + let mut a = vec![1, 2, 3, 4, 5, 6, 7]; + let b = vec![1, 2, 3, 5, 7]; + super::sorted_remove(&mut a, &b); + + assert_eq!(a, vec![4, 6]); + + let mut a = vec![1]; + let b = vec![1]; + super::sorted_remove(&mut a, &b); + + assert_eq!(a, vec![]); + + let mut a = vec![1]; + let b = vec![2]; + super::sorted_remove(&mut a, &b); + + assert_eq!(a, vec![1]); + } } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 22a84f69f9134..4315f80fa5ad9 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -5,7 +5,7 @@ use crate::{ component::{Component, ComponentId, ComponentTicks, Components, Mutable, StorageType}, entity::{Entities, Entity, EntityLocation}, event::Event, - observer::{Observer, Observers}, + observer::Observer, query::{Access, ReadOnlyQueryData}, removal_detection::RemovedComponentEvents, storage::Storages, @@ -1468,13 +1468,12 @@ impl<'w> EntityWorldMut<'w> { // SAFETY: `archetype_id` exists because it is referenced in the old `EntityLocation` which is valid, // components exist in `bundle_info` because `Bundles::init_info` initializes a `BundleInfo` containing all components of the bundle type `T` let new_archetype_id = unsafe { - remove_bundle_from_archetype( + bundle_info.remove_bundle_from_archetype( &mut world.archetypes, storages, components, &world.observers, old_location.archetype_id, - bundle_info, false, )? }; @@ -1650,17 +1649,17 @@ impl<'w> EntityWorldMut<'w> { // 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 = remove_bundle_from_archetype( - &mut world.archetypes, - &mut world.storages, - &world.components, - &world.observers, - location.archetype_id, - bundle_info, - // 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 = 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; @@ -3271,126 +3270,6 @@ unsafe fn insert_dynamic_bundle< } } -/// Removes a bundle from the given archetype and returns the resulting archetype (or None if the -/// removal was invalid). in the event that adding the given bundle does not result in an Archetype -/// change. Results are cached in the Archetype Graph to avoid redundant work. -/// if `intersection` is false, attempting to remove a bundle with components _not_ contained in the -/// current archetype will fail, returning None. if `intersection` is true, components in the bundle -/// but not in the current archetype will be ignored -/// -/// # Safety -/// `archetype_id` must exist and components in `bundle_info` must exist -unsafe fn remove_bundle_from_archetype( - archetypes: &mut Archetypes, - storages: &mut Storages, - components: &Components, - observers: &Observers, - archetype_id: ArchetypeId, - bundle_info: &BundleInfo, - intersection: bool, -) -> Option { - // check the archetype graph to see if the Bundle has been removed from this archetype in the - // past - let remove_bundle_result = { - let edges = archetypes[archetype_id].edges(); - if intersection { - edges.get_remove_bundle(bundle_info.id()) - } else { - edges.get_take_bundle(bundle_info.id()) - } - }; - let result = if let Some(result) = remove_bundle_result { - // this Bundle removal result is cached. just return that! - result - } else { - let mut next_table_components; - let mut next_sparse_set_components; - let next_table_id; - { - let current_archetype = &mut archetypes[archetype_id]; - let mut removed_table_components = Vec::new(); - let mut removed_sparse_set_components = Vec::new(); - for component_id in bundle_info.iter_explicit_components() { - if current_archetype.contains(component_id) { - // SAFETY: bundle components were already initialized by bundles.get_info - let component_info = unsafe { components.get_info_unchecked(component_id) }; - match component_info.storage_type() { - StorageType::Table => removed_table_components.push(component_id), - StorageType::SparseSet => removed_sparse_set_components.push(component_id), - } - } else if !intersection { - // a component in the bundle was not present in the entity's archetype, so this - // removal is invalid cache the result in the archetype - // graph - current_archetype - .edges_mut() - .insert_take_bundle(bundle_info.id(), None); - return None; - } - } - - // sort removed components so we can do an efficient "sorted remove". archetype - // components are already sorted - removed_table_components.sort_unstable(); - removed_sparse_set_components.sort_unstable(); - next_table_components = current_archetype.table_components().collect(); - next_sparse_set_components = current_archetype.sparse_set_components().collect(); - sorted_remove(&mut next_table_components, &removed_table_components); - sorted_remove( - &mut next_sparse_set_components, - &removed_sparse_set_components, - ); - - next_table_id = if removed_table_components.is_empty() { - current_archetype.table_id() - } else { - // SAFETY: all components in next_table_components exist - unsafe { - storages - .tables - .get_id_or_insert(&next_table_components, components) - } - }; - } - - let new_archetype_id = archetypes.get_id_or_insert( - components, - observers, - next_table_id, - next_table_components, - next_sparse_set_components, - ); - Some(new_archetype_id) - }; - let current_archetype = &mut archetypes[archetype_id]; - // cache the result in an edge - if intersection { - current_archetype - .edges_mut() - .insert_remove_bundle(bundle_info.id(), result); - } else { - current_archetype - .edges_mut() - .insert_take_bundle(bundle_info.id(), result); - } - result -} - -fn sorted_remove(source: &mut Vec, remove: &[T]) { - let mut remove_index = 0; - source.retain(|value| { - while remove_index < remove.len() && *value > remove[remove_index] { - remove_index += 1; - } - - if remove_index < remove.len() { - *value != remove[remove_index] - } else { - true - } - }); -} - /// Moves component data out of storage. /// /// This function leaves the underlying memory unchanged, but the component behind @@ -3700,27 +3579,6 @@ mod tests { use super::{EntityMutExcept, EntityRefExcept}; - #[test] - fn sorted_remove() { - let mut a = vec![1, 2, 3, 4, 5, 6, 7]; - let b = vec![1, 2, 3, 5, 7]; - super::sorted_remove(&mut a, &b); - - assert_eq!(a, vec![4, 6]); - - let mut a = vec![1]; - let b = vec![1]; - super::sorted_remove(&mut a, &b); - - assert_eq!(a, vec![]); - - let mut a = vec![1]; - let b = vec![2]; - super::sorted_remove(&mut a, &b); - - assert_eq!(a, vec![1]); - } - #[derive(Component, Clone, Copy, Debug, PartialEq)] struct TestComponent(u32);