From 1b2cf7d6cda2c1826921af6280bd205a6f7ffd99 Mon Sep 17 00:00:00 2001 From: ElliottjPierce <79881080+ElliottjPierce@users.noreply.github.com> Date: Wed, 5 Feb 2025 14:59:30 -0500 Subject: [PATCH] Isolate component registration (#17671) # Objective Progresses #17569. The end goal here is to synchronize component registration. See the other PR for details for the motivation behind that. For this PR specifically, the objective is to decouple `Components` from `Storages`. What components are registered etc should have nothing to do with what Storages looks like. Storages should only care about what entity archetypes have been spawned. ## Solution Previously, this was used to create sparse sets for relevant components when those components were registered. Now, we do that when the component is inserted/spawned. This PR proposes doing that in `BundleInfo::new`, but there may be a better place. ## Testing In theory, this shouldn't have changed any functionality, so no new tests were created. I'm not aware of any examples that make heavy use of sparse set components either. ## Migration Guide - Remove storages from functions where it is no longer needed. - Note that SparseSets are no longer present for all registered sparse set components, only those that have been spawned. --------- Co-authored-by: SpecificProtagonist Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com> --- crates/bevy_ecs/macros/src/component.rs | 7 +-- crates/bevy_ecs/macros/src/lib.rs | 6 +- crates/bevy_ecs/src/bundle.rs | 70 ++++++++++++----------- crates/bevy_ecs/src/component.rs | 25 +++----- crates/bevy_ecs/src/lib.rs | 30 +++------- crates/bevy_ecs/src/observer/runner.rs | 2 +- crates/bevy_ecs/src/query/fetch.rs | 59 +++++++++---------- crates/bevy_ecs/src/query/filter.rs | 24 ++++++-- crates/bevy_ecs/src/storage/mod.rs | 17 ++++++ crates/bevy_ecs/src/storage/sparse_set.rs | 4 +- crates/bevy_ecs/src/storage/table/mod.rs | 5 +- crates/bevy_ecs/src/world/entity_ref.rs | 56 +++++++++++------- crates/bevy_ecs/src/world/mod.rs | 4 +- 13 files changed, 162 insertions(+), 147 deletions(-) diff --git a/crates/bevy_ecs/macros/src/component.rs b/crates/bevy_ecs/macros/src/component.rs index 826a72c8ec212..652f0a9d35331 100644 --- a/crates/bevy_ecs/macros/src/component.rs +++ b/crates/bevy_ecs/macros/src/component.rs @@ -156,7 +156,6 @@ pub fn derive_component(input: TokenStream) -> TokenStream { <#ident as #bevy_ecs_path::component::Component>::register_required_components( requiree, components, - storages, required_components, inheritance_depth + 1, recursion_check_stack @@ -166,7 +165,6 @@ pub fn derive_component(input: TokenStream) -> TokenStream { Some(RequireFunc::Path(func)) => { register_required.push(quote! { components.register_required_components_manual::( - storages, required_components, || { let x: #ident = #func().into(); x }, inheritance_depth, @@ -177,7 +175,6 @@ pub fn derive_component(input: TokenStream) -> TokenStream { Some(RequireFunc::Closure(func)) => { register_required.push(quote! { components.register_required_components_manual::( - storages, required_components, || { let x: #ident = (#func)().into(); x }, inheritance_depth, @@ -188,7 +185,6 @@ pub fn derive_component(input: TokenStream) -> TokenStream { None => { register_required.push(quote! { components.register_required_components_manual::( - storages, required_components, <#ident as Default>::default, inheritance_depth, @@ -224,13 +220,12 @@ pub fn derive_component(input: TokenStream) -> TokenStream { fn register_required_components( requiree: #bevy_ecs_path::component::ComponentId, components: &mut #bevy_ecs_path::component::Components, - storages: &mut #bevy_ecs_path::storage::Storages, required_components: &mut #bevy_ecs_path::component::RequiredComponents, inheritance_depth: u16, recursion_check_stack: &mut #bevy_ecs_path::__macro_exports::Vec<#bevy_ecs_path::component::ComponentId> ) { #bevy_ecs_path::component::enforce_no_required_components_recursion(components, recursion_check_stack); - let self_id = components.register_component::(storages); + let self_id = components.register_component::(); recursion_check_stack.push(self_id); #(#register_required)* #(#register_recursive_requires)* diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index e4cca41a4d7aa..2c6d79a03e3b8 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -86,10 +86,10 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { match field_kind { BundleFieldKind::Component => { field_component_ids.push(quote! { - <#field_type as #ecs_path::bundle::Bundle>::component_ids(components, storages, &mut *ids); + <#field_type as #ecs_path::bundle::Bundle>::component_ids(components, &mut *ids); }); field_required_components.push(quote! { - <#field_type as #ecs_path::bundle::Bundle>::register_required_components(components, storages, required_components); + <#field_type as #ecs_path::bundle::Bundle>::register_required_components(components, required_components); }); field_get_component_ids.push(quote! { <#field_type as #ecs_path::bundle::Bundle>::get_component_ids(components, &mut *ids); @@ -134,7 +134,6 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { unsafe impl #impl_generics #ecs_path::bundle::Bundle for #struct_name #ty_generics #where_clause { fn component_ids( components: &mut #ecs_path::component::Components, - storages: &mut #ecs_path::storage::Storages, ids: &mut impl FnMut(#ecs_path::component::ComponentId) ){ #(#field_component_ids)* @@ -159,7 +158,6 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { fn register_required_components( components: &mut #ecs_path::component::Components, - storages: &mut #ecs_path::storage::Storages, required_components: &mut #ecs_path::component::RequiredComponents ){ #(#field_required_components)* diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 7919a3232d1b3..c959488bbcdb6 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -151,11 +151,7 @@ use variadics_please::all_tuples; pub unsafe trait Bundle: DynamicBundle + Send + Sync + 'static { /// Gets this [`Bundle`]'s component ids, in the order of this bundle's [`Component`]s #[doc(hidden)] - fn component_ids( - components: &mut Components, - storages: &mut Storages, - ids: &mut impl FnMut(ComponentId), - ); + fn component_ids(components: &mut Components, ids: &mut impl FnMut(ComponentId)); /// Gets this [`Bundle`]'s component ids. This will be [`None`] if the component has not been registered. fn get_component_ids(components: &Components, ids: &mut impl FnMut(Option)); @@ -176,7 +172,6 @@ pub unsafe trait Bundle: DynamicBundle + Send + Sync + 'static { /// Registers components that are required by the components in this [`Bundle`]. fn register_required_components( _components: &mut Components, - _storages: &mut Storages, _required_components: &mut RequiredComponents, ); } @@ -198,12 +193,8 @@ pub trait DynamicBundle { // - `Bundle::get_components` is called exactly once for C and passes the component's storage type based on its associated constant. // - `Bundle::from_components` calls `func` exactly once for C, which is the exact value returned by `Bundle::component_ids`. unsafe impl Bundle for C { - fn component_ids( - components: &mut Components, - storages: &mut Storages, - ids: &mut impl FnMut(ComponentId), - ) { - ids(components.register_component::(storages)); + fn component_ids(components: &mut Components, ids: &mut impl FnMut(ComponentId)) { + ids(components.register_component::()); } unsafe fn from_components(ctx: &mut T, func: &mut F) -> Self @@ -219,14 +210,12 @@ unsafe impl Bundle for C { fn register_required_components( components: &mut Components, - storages: &mut Storages, required_components: &mut RequiredComponents, ) { - let component_id = components.register_component::(storages); + let component_id = components.register_component::(); ::register_required_components( component_id, components, - storages, required_components, 0, &mut Vec::new(), @@ -264,8 +253,8 @@ macro_rules! tuple_impl { // - `Bundle::get_components` is called exactly once for each member. Relies on the above implementation to pass the correct // `StorageType` into the callback. unsafe impl<$($name: Bundle),*> Bundle for ($($name,)*) { - fn component_ids(components: &mut Components, storages: &mut Storages, ids: &mut impl FnMut(ComponentId)){ - $(<$name as Bundle>::component_ids(components, storages, ids);)* + fn component_ids(components: &mut Components, ids: &mut impl FnMut(ComponentId)){ + $(<$name as Bundle>::component_ids(components, ids);)* } fn get_component_ids(components: &Components, ids: &mut impl FnMut(Option)){ @@ -291,10 +280,9 @@ macro_rules! tuple_impl { fn register_required_components( components: &mut Components, - storages: &mut Storages, required_components: &mut RequiredComponents, ) { - $(<$name as Bundle>::register_required_components(components, storages, required_components);)* + $(<$name as Bundle>::register_required_components(components, required_components);)* } } @@ -392,19 +380,19 @@ impl BundleInfo { /// /// # Safety /// - /// Every ID in `component_ids` must be valid within the World that owns the `BundleInfo`, - /// must have its storage initialized (i.e. columns created in tables, sparse set created), + /// Every ID in `component_ids` must be valid within the World that owns the `BundleInfo` /// and must be in the same order as the source bundle type writes its components in. unsafe fn new( bundle_type_name: &'static str, + storages: &mut Storages, components: &Components, mut component_ids: Vec, id: BundleId, ) -> BundleInfo { + // check for duplicates let mut deduped = component_ids.clone(); deduped.sort_unstable(); deduped.dedup(); - if deduped.len() != component_ids.len() { // TODO: Replace with `Vec::partition_dedup` once https://github.com/rust-lang/rust/issues/54279 is stabilized let mut seen = >::default(); @@ -427,18 +415,25 @@ impl BundleInfo { panic!("Bundle {bundle_type_name} has duplicate components: {names}"); } + // handle explicit components let explicit_components_len = component_ids.len(); let mut required_components = RequiredComponents::default(); for component_id in component_ids.iter().copied() { // SAFETY: caller has verified that all ids are valid let info = unsafe { components.get_info_unchecked(component_id) }; required_components.merge(info.required_components()); + storages.prepare_component(info); } required_components.remove_explicit_components(&component_ids); + + // handle required components let required_components = required_components .0 .into_iter() .map(|(component_id, v)| { + // Safety: These ids came out of the passed `components`, so they must be valid. + let info = unsafe { components.get_info_unchecked(component_id) }; + storages.prepare_component(info); // This adds required components to the component_ids list _after_ using that list to remove explicitly provided // components. This ordering is important! component_ids.push(component_id); @@ -550,7 +545,7 @@ impl BundleInfo { StorageType::Table => { // SAFETY: bundle_component is a valid index for this bundle let status = unsafe { bundle_component_status.get_status(bundle_component) }; - // SAFETY: If component_id is in self.component_ids, BundleInfo::new requires that + // SAFETY: If component_id is in self.component_ids, BundleInfo::new ensures that // the target table contains the component. let column = table.get_column_mut(component_id).debug_checked_unwrap(); match (status, insert_mode) { @@ -577,7 +572,7 @@ impl BundleInfo { } StorageType::SparseSet => { let sparse_set = - // SAFETY: If component_id is in self.component_ids, BundleInfo::new requires that + // SAFETY: If component_id is in self.component_ids, BundleInfo::new ensures that // a sparse set exists for the component. unsafe { sparse_sets.get_mut(component_id).debug_checked_unwrap() }; sparse_set.insert( @@ -1540,14 +1535,14 @@ impl Bundles { let bundle_infos = &mut self.bundle_infos; let id = *self.bundle_ids.entry(TypeId::of::()).or_insert_with(|| { let mut component_ids= Vec::new(); - T::component_ids(components, storages, &mut |id| component_ids.push(id)); + T::component_ids(components, &mut |id| component_ids.push(id)); let id = BundleId(bundle_infos.len()); let bundle_info = // SAFETY: T::component_id ensures: // - its info was created // - appropriate storage for it has been initialized. // - it was created in the same order as the components in T - unsafe { BundleInfo::new(core::any::type_name::(), components, component_ids, id) }; + unsafe { BundleInfo::new(core::any::type_name::(), storages, components, component_ids, id) }; bundle_infos.push(bundle_info); id }); @@ -1577,7 +1572,7 @@ impl Bundles { }; // SAFETY: this is sound because the contributed_components Vec for explicit_bundle_id will not be accessed mutably as // part of init_dynamic_info. No mutable references will be created and the allocation will remain valid. - self.init_dynamic_info(components, core::slice::from_raw_parts(ptr, len)) + self.init_dynamic_info(storages, components, core::slice::from_raw_parts(ptr, len)) }; self.contributed_bundle_ids.insert(TypeId::of::(), id); id @@ -1615,6 +1610,7 @@ impl Bundles { /// provided [`Components`]. pub(crate) fn init_dynamic_info( &mut self, + storages: &mut Storages, components: &Components, component_ids: &[ComponentId], ) -> BundleId { @@ -1626,8 +1622,12 @@ impl Bundles { .raw_entry_mut() .from_key(component_ids) .or_insert_with(|| { - let (id, storages) = - initialize_dynamic_bundle(bundle_infos, components, Vec::from(component_ids)); + let (id, storages) = initialize_dynamic_bundle( + bundle_infos, + storages, + components, + Vec::from(component_ids), + ); // SAFETY: The ID always increases when new bundles are added, and so, the ID is unique. unsafe { self.dynamic_bundle_storages @@ -1645,6 +1645,7 @@ impl Bundles { /// Panics if the provided [`ComponentId`] does not exist in the provided [`Components`]. pub(crate) fn init_component_info( &mut self, + storages: &mut Storages, components: &Components, component_id: ComponentId, ) -> BundleId { @@ -1653,8 +1654,12 @@ impl Bundles { .dynamic_component_bundle_ids .entry(component_id) .or_insert_with(|| { - let (id, storage_type) = - initialize_dynamic_bundle(bundle_infos, components, vec![component_id]); + let (id, storage_type) = initialize_dynamic_bundle( + bundle_infos, + storages, + components, + vec![component_id], + ); self.dynamic_component_storages.insert(id, storage_type[0]); id }); @@ -1666,6 +1671,7 @@ impl Bundles { /// and initializes a [`BundleInfo`]. fn initialize_dynamic_bundle( bundle_infos: &mut Vec, + storages: &mut Storages, components: &Components, component_ids: Vec, ) -> (BundleId, Vec) { @@ -1681,7 +1687,7 @@ fn initialize_dynamic_bundle( let id = BundleId(bundle_infos.len()); let bundle_info = // SAFETY: `component_ids` are valid as they were just checked - unsafe { BundleInfo::new("", components, component_ids, id) }; + unsafe { BundleInfo::new("", storages, components, component_ids, id) }; bundle_infos.push(bundle_info); (id, storage_types) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index dfcb20bf3ebb2..c8fedf474362b 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -8,7 +8,7 @@ use crate::{ entity::{ComponentCloneCtx, Entity}, query::DebugCheckedUnwrap, resource::Resource, - storage::{SparseSetIndex, SparseSets, Storages, Table, TableRow}, + storage::{SparseSetIndex, SparseSets, Table, TableRow}, system::{Local, SystemParam}, world::{DeferredWorld, FromWorld, World}, }; @@ -438,7 +438,6 @@ pub trait Component: Send + Sync + 'static { fn register_required_components( _component_id: ComponentId, _components: &mut Components, - _storages: &mut Storages, _required_components: &mut RequiredComponents, _inheritance_depth: u16, _recursion_check_stack: &mut Vec, @@ -1193,14 +1192,13 @@ impl Components { /// * [`Components::component_id()`] /// * [`Components::register_component_with_descriptor()`] #[inline] - pub fn register_component(&mut self, storages: &mut Storages) -> ComponentId { - self.register_component_internal::(storages, &mut Vec::new()) + pub fn register_component(&mut self) -> ComponentId { + self.register_component_internal::(&mut Vec::new()) } #[inline] fn register_component_internal( &mut self, - storages: &mut Storages, recursion_check_stack: &mut Vec, ) -> ComponentId { let mut is_new_registration = false; @@ -1214,7 +1212,6 @@ impl Components { *indices.entry(type_id).or_insert_with(|| { let id = Components::register_component_inner( components, - storages, ComponentDescriptor::new::(), ); is_new_registration = true; @@ -1226,7 +1223,6 @@ impl Components { T::register_required_components( id, self, - storages, &mut required_components, 0, recursion_check_stack, @@ -1261,23 +1257,18 @@ impl Components { /// * [`Components::register_component()`] pub fn register_component_with_descriptor( &mut self, - storages: &mut Storages, descriptor: ComponentDescriptor, ) -> ComponentId { - Components::register_component_inner(&mut self.components, storages, descriptor) + Components::register_component_inner(&mut self.components, descriptor) } #[inline] fn register_component_inner( components: &mut Vec, - storages: &mut Storages, descriptor: ComponentDescriptor, ) -> ComponentId { let component_id = ComponentId(components.len()); let info = ComponentInfo::new(component_id, descriptor); - if info.descriptor.storage_type == StorageType::SparseSet { - storages.sparse_sets.get_or_insert(&info); - } components.push(info); component_id } @@ -1512,14 +1503,13 @@ impl Components { #[doc(hidden)] pub fn register_required_components_manual( &mut self, - storages: &mut Storages, required_components: &mut RequiredComponents, constructor: fn() -> R, inheritance_depth: u16, recursion_check_stack: &mut Vec, ) { - let requiree = self.register_component_internal::(storages, recursion_check_stack); - let required = self.register_component_internal::(storages, recursion_check_stack); + let requiree = self.register_component_internal::(recursion_check_stack); + let required = self.register_component_internal::(recursion_check_stack); // SAFETY: We just created the components. unsafe { @@ -2111,11 +2101,10 @@ impl RequiredComponents { pub fn register( &mut self, components: &mut Components, - storages: &mut Storages, constructor: fn() -> C, inheritance_depth: u16, ) { - let component_id = components.register_component::(storages); + let component_id = components.register_component::(); self.register_by_id(component_id, constructor, inheritance_depth); } diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index ae325aa0857e3..8d0bdcc38f3ad 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -229,13 +229,9 @@ mod tests { y: SparseStored, } let mut ids = Vec::new(); - ::component_ids( - &mut world.components, - &mut world.storages, - &mut |id| { - ids.push(id); - }, - ); + ::component_ids(&mut world.components, &mut |id| { + ids.push(id); + }); assert_eq!( ids, @@ -283,13 +279,9 @@ mod tests { } let mut ids = Vec::new(); - ::component_ids( - &mut world.components, - &mut world.storages, - &mut |id| { - ids.push(id); - }, - ); + ::component_ids(&mut world.components, &mut |id| { + ids.push(id); + }); assert_eq!( ids, @@ -339,13 +331,9 @@ mod tests { } let mut ids = Vec::new(); - ::component_ids( - &mut world.components, - &mut world.storages, - &mut |id| { - ids.push(id); - }, - ); + ::component_ids(&mut world.components, &mut |id| { + ids.push(id); + }); assert_eq!(ids, &[world.register_component::(),]); diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index 7dab9e28f9ab3..74b6519579703 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -407,7 +407,7 @@ fn hook_on_add>( world.commands().queue(move |world: &mut World| { let event_id = E::register_component_id(world); let mut components = Vec::new(); - B::component_ids(&mut world.components, &mut world.storages, &mut |id| { + B::component_ids(&mut world.components, &mut |id| { components.push(id); }); let mut descriptor = ObserverDescriptor { diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 908a3e75d0b55..627bebd065b05 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1091,7 +1091,7 @@ pub struct ReadFetch<'w, T: Component> { // T::STORAGE_TYPE = StorageType::Table Option>>, // T::STORAGE_TYPE = StorageType::SparseSet - &'w ComponentSparseSet, + Option<&'w ComponentSparseSet>, >, } @@ -1130,13 +1130,7 @@ unsafe impl WorldQuery for &T { // which we are allowed to access since we registered it in `update_archetype_component_access`. // Note that we do not actually access any components in this function, we just get a shared // reference to the sparse set, which is used to access the components in `Self::fetch`. - unsafe { - world - .storages() - .sparse_sets - .get(component_id) - .debug_checked_unwrap() - } + unsafe { world.storages().sparse_sets.get(component_id) } }, ), } @@ -1233,7 +1227,12 @@ unsafe impl QueryData for &T { }, |sparse_set| { // SAFETY: Caller ensures `entity` is in range. - let item = unsafe { sparse_set.get(entity).debug_checked_unwrap() }; + let item = unsafe { + sparse_set + .debug_checked_unwrap() + .get(entity) + .debug_checked_unwrap() + }; item.deref() }, ) @@ -1255,7 +1254,8 @@ pub struct RefFetch<'w, T: Component> { MaybeThinSlicePtrLocation<'w>, )>, // T::STORAGE_TYPE = StorageType::SparseSet - &'w ComponentSparseSet, + // Can be `None` when the component has never been inserted + Option<&'w ComponentSparseSet>, >, last_run: Tick, this_run: Tick, @@ -1296,13 +1296,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { // which we are allowed to access since we registered it in `update_archetype_component_access`. // Note that we do not actually access any components in this function, we just get a shared // reference to the sparse set, which is used to access the components in `Self::fetch`. - unsafe { - world - .storages() - .sparse_sets - .get(component_id) - .debug_checked_unwrap() - } + unsafe { world.storages().sparse_sets.get(component_id) } }, ), last_run, @@ -1424,9 +1418,13 @@ unsafe impl<'__w, T: Component> QueryData for Ref<'__w, T> { } }, |sparse_set| { - // SAFETY: The caller ensures `entity` is in range. - let (component, ticks, _caller) = - unsafe { sparse_set.get_with_ticks(entity).debug_checked_unwrap() }; + // SAFETY: The caller ensures `entity` is in range and has the component. + let (component, ticks, _caller) = unsafe { + sparse_set + .debug_checked_unwrap() + .get_with_ticks(entity) + .debug_checked_unwrap() + }; Ref { value: component.deref(), @@ -1454,7 +1452,8 @@ pub struct WriteFetch<'w, T: Component> { MaybeThinSlicePtrLocation<'w>, )>, // T::STORAGE_TYPE = StorageType::SparseSet - &'w ComponentSparseSet, + // Can be `None` when the component has never been inserted + Option<&'w ComponentSparseSet>, >, last_run: Tick, this_run: Tick, @@ -1495,13 +1494,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { // which we are allowed to access since we registered it in `update_archetype_component_access`. // Note that we do not actually access any components in this function, we just get a shared // reference to the sparse set, which is used to access the components in `Self::fetch`. - unsafe { - world - .storages() - .sparse_sets - .get(component_id) - .debug_checked_unwrap() - } + unsafe { world.storages().sparse_sets.get(component_id) } }, ), last_run, @@ -1623,9 +1616,13 @@ unsafe impl<'__w, T: Component> QueryData for &'__w mut T } }, |sparse_set| { - // SAFETY: The caller ensures `entity` is in range. - let (component, ticks, _caller) = - unsafe { sparse_set.get_with_ticks(entity).debug_checked_unwrap() }; + // SAFETY: The caller ensures `entity` is in range and has the component. + let (component, ticks, _caller) = unsafe { + sparse_set + .debug_checked_unwrap() + .get_with_ticks(entity) + .debug_checked_unwrap() + }; Mut { value: component.assert_unique().deref_mut(), diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 20ed35101740e..9e4f36a9655f6 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -630,7 +630,8 @@ pub struct AddedFetch<'w, T: Component> { // T::STORAGE_TYPE = StorageType::Table Option>>, // T::STORAGE_TYPE = StorageType::SparseSet - &'w ComponentSparseSet, + // Can be `None` when the component has never been inserted + Option<&'w ComponentSparseSet>, >, last_run: Tick, this_run: Tick, @@ -674,7 +675,7 @@ unsafe impl WorldQuery for Added { // which we are allowed to access since we registered it in `update_archetype_component_access`. // Note that we do not actually access any components' ticks in this function, we just get a shared // reference to the sparse set, which is used to access the components' ticks in `Self::fetch`. - unsafe { world.storages().sparse_sets.get(id).debug_checked_unwrap() } + unsafe { world.storages().sparse_sets.get(id) } }, ), last_run, @@ -766,7 +767,10 @@ unsafe impl QueryFilter for Added { |sparse_set| { // SAFETY: The caller ensures `entity` is in range. let tick = unsafe { - ComponentSparseSet::get_added_tick(sparse_set, entity).debug_checked_unwrap() + sparse_set + .debug_checked_unwrap() + .get_added_tick(entity) + .debug_checked_unwrap() }; tick.deref().is_newer_than(fetch.last_run, fetch.this_run) @@ -850,7 +854,12 @@ pub struct Changed(PhantomData); #[doc(hidden)] pub struct ChangedFetch<'w, T: Component> { - ticks: StorageSwitch>>, &'w ComponentSparseSet>, + ticks: StorageSwitch< + T, + Option>>, + // Can be `None` when the component has never been inserted + Option<&'w ComponentSparseSet>, + >, last_run: Tick, this_run: Tick, } @@ -893,7 +902,7 @@ unsafe impl WorldQuery for Changed { // which we are allowed to access since we registered it in `update_archetype_component_access`. // Note that we do not actually access any components' ticks in this function, we just get a shared // reference to the sparse set, which is used to access the components' ticks in `Self::fetch`. - unsafe { world.storages().sparse_sets.get(id).debug_checked_unwrap() } + unsafe { world.storages().sparse_sets.get(id) } }, ), last_run, @@ -986,7 +995,10 @@ unsafe impl QueryFilter for Changed { |sparse_set| { // SAFETY: The caller ensures `entity` is in range. let tick = unsafe { - ComponentSparseSet::get_changed_tick(sparse_set, entity).debug_checked_unwrap() + sparse_set + .debug_checked_unwrap() + .get_changed_tick(entity) + .debug_checked_unwrap() }; tick.deref().is_newer_than(fetch.last_run, fetch.this_run) diff --git a/crates/bevy_ecs/src/storage/mod.rs b/crates/bevy_ecs/src/storage/mod.rs index eaeff97a8cccb..2a5a5f184e649 100644 --- a/crates/bevy_ecs/src/storage/mod.rs +++ b/crates/bevy_ecs/src/storage/mod.rs @@ -31,10 +31,13 @@ pub use resource::*; pub use sparse_set::*; pub use table::*; +use crate::component::{ComponentInfo, StorageType}; + /// The raw data stores of a [`World`](crate::world::World) #[derive(Default)] pub struct Storages { /// Backing storage for [`SparseSet`] components. + /// Note that sparse sets are only present for components that have been spawned or have had a relevant bundle registered. pub sparse_sets: SparseSets, /// Backing storage for [`Table`] components. pub tables: Tables, @@ -43,3 +46,17 @@ pub struct Storages { /// Backing storage for `!Send` resources. pub non_send_resources: Resources, } + +impl Storages { + /// ensures that the component has its necessary storage initialize. + pub fn prepare_component(&mut self, component: &ComponentInfo) { + match component.storage_type() { + StorageType::Table => { + // table needs no preparation + } + StorageType::SparseSet => { + self.sparse_sets.get_or_insert(component); + } + } + } +} diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 518333fa272d8..c55d00245f671 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -616,7 +616,7 @@ impl SparseSets { self.sets.iter().map(|(id, data)| (*id, data)) } - /// Gets a reference to the [`ComponentSparseSet`] of a [`ComponentId`]. + /// Gets a reference to the [`ComponentSparseSet`] of a [`ComponentId`]. This may be `None` if the component has never been spawned. #[inline] pub fn get(&self, component_id: ComponentId) -> Option<&ComponentSparseSet> { self.sets.get(component_id) @@ -638,7 +638,7 @@ impl SparseSets { self.sets.get_mut(component_info.id()).unwrap() } - /// Gets a mutable reference to the [`ComponentSparseSet`] of a [`ComponentId`]. + /// Gets a mutable reference to the [`ComponentSparseSet`] of a [`ComponentId`]. This may be `None` if the component has never been spawned. pub(crate) fn get_mut(&mut self, component_id: ComponentId) -> Option<&mut ComponentSparseSet> { self.sets.get_mut(component_id) } diff --git a/crates/bevy_ecs/src/storage/table/mod.rs b/crates/bevy_ecs/src/storage/table/mod.rs index cd823851cc06b..9f005e8c82b96 100644 --- a/crates/bevy_ecs/src/storage/table/mod.rs +++ b/crates/bevy_ecs/src/storage/table/mod.rs @@ -820,7 +820,7 @@ mod tests { component::{Component, Components, Tick}, entity::Entity, ptr::OwningPtr, - storage::{Storages, TableBuilder, TableId, TableRow, Tables}, + storage::{TableBuilder, TableId, TableRow, Tables}, }; use alloc::vec::Vec; @@ -845,8 +845,7 @@ mod tests { #[test] fn table() { let mut components = Components::default(); - let mut storages = Storages::default(); - let component_id = components.register_component::>(&mut storages); + let component_id = components.register_component::>(); let columns = &[component_id]; let mut table = TableBuilder::with_capacity(0, columns.len()) .add_column(components.get_info(component_id).unwrap()) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 912f4a968606f..1240fba66eaa0 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1614,10 +1614,11 @@ impl<'w> EntityWorldMut<'w> { ) -> &mut Self { self.assert_not_despawned(); let change_tick = self.world.change_tick(); - let bundle_id = self - .world - .bundles - .init_component_info(&self.world.components, component_id); + let bundle_id = self.world.bundles.init_component_info( + &mut self.world.storages, + &self.world.components, + component_id, + ); let storage_type = self.world.bundles.get_storage_unchecked(bundle_id); let bundle_inserter = BundleInserter::new_with_id( @@ -1665,10 +1666,11 @@ impl<'w> EntityWorldMut<'w> { ) -> &mut Self { self.assert_not_despawned(); let change_tick = self.world.change_tick(); - let bundle_id = self - .world - .bundles - .init_dynamic_info(&self.world.components, component_ids); + let bundle_id = self.world.bundles.init_dynamic_info( + &mut self.world.storages, + &self.world.components, + component_ids, + ); let mut storage_types = core::mem::take(self.world.bundles.get_storages_unchecked(bundle_id)); let bundle_inserter = BundleInserter::new_with_id( @@ -1771,6 +1773,7 @@ impl<'w> EntityWorldMut<'w> { // - entity location is valid // - table row is removed below, without dropping the contents // - `components` comes from the same world as `storages` + // - the component exists on the entity take_component( storages, components, @@ -1955,6 +1958,7 @@ impl<'w> EntityWorldMut<'w> { .storages .sparse_sets .get_mut(component_id) + // Set exists because the component existed on the entity .unwrap() .remove(entity); } @@ -2090,7 +2094,10 @@ impl<'w> EntityWorldMut<'w> { .components() .filter(|c| !retained_bundle_info.contributed_components().contains(c)) .collect::>(); - let remove_bundle = self.world.bundles.init_dynamic_info(components, to_remove); + let remove_bundle = + self.world + .bundles + .init_dynamic_info(&mut self.world.storages, components, to_remove); // SAFETY: the `BundleInfo` for the components to remove is initialized above self.location = unsafe { @@ -2131,10 +2138,11 @@ impl<'w> EntityWorldMut<'w> { self.assert_not_despawned(); let components = &mut self.world.components; - let bundle_id = self - .world - .bundles - .init_component_info(components, component_id); + let bundle_id = self.world.bundles.init_component_info( + &mut self.world.storages, + components, + component_id, + ); // SAFETY: the `BundleInfo` for this `component_id` is initialized above self.location = unsafe { @@ -2162,10 +2170,11 @@ impl<'w> EntityWorldMut<'w> { self.assert_not_despawned(); let components = &mut self.world.components; - let bundle_id = self - .world - .bundles - .init_dynamic_info(components, component_ids); + let bundle_id = self.world.bundles.init_dynamic_info( + &mut self.world.storages, + components, + component_ids, + ); // SAFETY: the `BundleInfo` for this `bundle_id` is initialized above unsafe { @@ -2203,10 +2212,11 @@ impl<'w> EntityWorldMut<'w> { let component_ids: Vec = self.archetype().components().collect(); let components = &mut self.world.components; - let bundle_id = self - .world - .bundles - .init_dynamic_info(components, component_ids.as_slice()); + let bundle_id = self.world.bundles.init_dynamic_info( + &mut self.world.storages, + components, + component_ids.as_slice(), + ); // SAFETY: the `BundleInfo` for this `component_id` is initialized above self.location = unsafe { @@ -2354,6 +2364,7 @@ impl<'w> EntityWorldMut<'w> { table_row = remove_result.table_row; for component_id in archetype.sparse_set_components() { + // set must have existed for the component to be added. let sparse_set = world.storages.sparse_sets.get_mut(component_id).unwrap(); sparse_set.remove(self.entity); } @@ -4113,6 +4124,9 @@ unsafe fn insert_dynamic_bundle< /// - `component_id` must be valid /// - `components` must come from the same world as `self` /// - The relevant table row **must be removed** by the caller once all components are taken, without dropping the value +/// +/// # Panics +/// Panics if the entity did not have the component. #[inline] pub(crate) unsafe fn take_component<'a>( storages: &'a mut Storages, diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 8ea17168a38c4..732e02e189feb 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -251,7 +251,7 @@ impl World { /// Registers a new [`Component`] type and returns the [`ComponentId`] created for it. pub fn register_component(&mut self) -> ComponentId { - self.components.register_component::(&mut self.storages) + self.components.register_component::() } /// Returns a mutable reference to the [`ComponentHooks`] for a [`Component`] type. @@ -528,7 +528,7 @@ impl World { descriptor: ComponentDescriptor, ) -> ComponentId { self.components - .register_component_with_descriptor(&mut self.storages, descriptor) + .register_component_with_descriptor(descriptor) } /// Returns the [`ComponentId`] of the given [`Component`] type `T`.