From 995307c3ae2c4103f6e822982da8f11e74eaae58 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Wed, 5 Mar 2025 08:05:51 -0500 Subject: [PATCH 01/22] added id generator. --- crates/bevy_ecs/src/component.rs | 34 ++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index eeb6820bae69d..d42ce62272cea 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1146,6 +1146,40 @@ pub struct Components { resource_indices: TypeIdMap, } +/// Generates [`ComponentId`]s. +#[derive(Debug, Default)] +pub struct ComponentIds { + next: bevy_platform_support::sync::atomic::AtomicUsize, +} + +impl ComponentIds { + /// Peeks the next [`ComponentId`] to be generated without generating it. + pub fn peek(&self) -> ComponentId { + ComponentId( + self.next + .load(bevy_platform_support::sync::atomic::Ordering::Relaxed), + ) + } + + /// Generates and returns the next [`ComponentId`]. + pub fn next(&self) -> ComponentId { + ComponentId( + self.next + .fetch_add(1, bevy_platform_support::sync::atomic::Ordering::Relaxed), + ) + } + + /// Returns the number of [`ComponentId`]s generated. + pub fn len(&self) -> usize { + self.peek().0 + } + + /// Returns true if and only if no ids have been generated. + pub fn is_empty(&self) -> bool { + self.len() == 0 + } +} + impl Components { /// Registers a [`Component`] of type `T` with this instance. /// If a component of this type has already been registered, this will return From 63b0931dbdcce16455cf3074c50fdf5f02828295 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Wed, 5 Mar 2025 09:04:04 -0500 Subject: [PATCH 02/22] separated id generation form registration --- crates/bevy_ecs/src/component.rs | 402 ++++++++++++++++++------------- 1 file changed, 234 insertions(+), 168 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index d42ce62272cea..8ece5f30dde61 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -28,6 +28,7 @@ use core::{ fmt::Debug, marker::PhantomData, mem::needs_drop, + ops::{Deref, DerefMut}, }; use disqualified::ShortName; use thiserror::Error; @@ -1141,7 +1142,7 @@ impl ComponentCloneBehavior { /// Stores metadata associated with each kind of [`Component`] in a given [`World`]. #[derive(Debug, Default)] pub struct Components { - components: Vec, + components: HashMap, indices: TypeIdMap, resource_indices: TypeIdMap, } @@ -1169,6 +1170,19 @@ impl ComponentIds { ) } + /// Peeks the next [`ComponentId`] to be generated without generating it. + pub fn peek_mut(&mut self) -> ComponentId { + ComponentId(*self.next.get_mut()) + } + + /// Generates and returns the next [`ComponentId`]. + pub fn next_mut(&mut self) -> ComponentId { + let id = self.next.get_mut(); + let result = ComponentId(*id); + *id += 1; + result + } + /// Returns the number of [`ComponentId`]s generated. pub fn len(&self) -> usize { self.peek().0 @@ -1180,7 +1194,27 @@ impl ComponentIds { } } -impl Components { +/// A type that enables registering in [`Components`]. +pub struct ComponentsView<'w> { + components: &'w mut Components, + ids: &'w mut ComponentIds, +} + +impl Deref for ComponentsView<'_> { + type Target = Components; + + fn deref(&self) -> &Self::Target { + self.components + } +} + +impl DerefMut for ComponentsView<'_> { + fn deref_mut(&mut self) -> &mut Self::Target { + self.components + } +} + +impl<'w> ComponentsView<'w> { /// Registers a [`Component`] of type `T` with this instance. /// If a component of this type has already been registered, this will return /// the ID of the pre-existing component. @@ -1191,52 +1225,26 @@ impl Components { /// * [`Components::register_component_with_descriptor()`] #[inline] pub fn register_component(&mut self) -> ComponentId { - self.register_component_internal::(&mut Vec::new()) + self.component_id::().unwrap_or_else(|| { + let id = self.ids.next_mut(); + // SAFETY: The component is not currently registered, and the id is fresh. + unsafe { self.register_component_internal::(&mut Vec::new(), id) } + id + }) } + /// Same as [`register_component`] but keeps a `recursion_check_stack`. #[inline] - fn register_component_internal( + fn register_component_checked_internal( &mut self, recursion_check_stack: &mut Vec, ) -> ComponentId { - let mut is_new_registration = false; - let id = { - let Components { - indices, - components, - .. - } = self; - let type_id = TypeId::of::(); - *indices.entry(type_id).or_insert_with(|| { - let id = Components::register_component_inner( - components, - ComponentDescriptor::new::(), - ); - is_new_registration = true; - id - }) - }; - if is_new_registration { - let mut required_components = RequiredComponents::default(); - T::register_required_components( - id, - self, - &mut required_components, - 0, - recursion_check_stack, - ); - let info = &mut self.components[id.index()]; - - #[expect( - deprecated, - reason = "need to use this method until it is removed to ensure user defined components register hooks correctly" - )] - // TODO: Replace with `info.hooks.update_from_component::();` once `Component::register_component_hooks` is removed - T::register_component_hooks(&mut info.hooks); - - info.required_components = required_components; - } - id + self.component_id::().unwrap_or_else(|| { + let id = self.ids.next_mut(); + // SAFETY: The component is not currently registered, and the id is fresh. + unsafe { self.register_component_internal::(recursion_check_stack, id) } + id + }) } /// Registers a component described by `descriptor`. @@ -1254,18 +1262,174 @@ impl Components { &mut self, descriptor: ComponentDescriptor, ) -> ComponentId { - Components::register_component_inner(&mut self.components, descriptor) + let id = self.ids.next_mut(); + // SAFETY: The id is fresh. + unsafe { + self.register_component_inner(id, descriptor); + } + id + } + + // NOTE: This should maybe be private, but it is currently public so that `bevy_ecs_macros` can use it. + // We can't directly move this there either, because this uses `Components::get_required_by_mut`, + // which is private, and could be equally risky to expose to users. + /// Registers the given component `R` and [required components] inherited from it as required by `T`, + /// and adds `T` to their lists of requirees. + /// + /// The given `inheritance_depth` determines how many levels of inheritance deep the requirement is. + /// A direct requirement has a depth of `0`, and each level of inheritance increases the depth by `1`. + /// Lower depths are more specific requirements, and can override existing less specific registrations. + /// + /// The `recursion_check_stack` allows checking whether this component tried to register itself as its + /// own (indirect) required component. + /// + /// This method does *not* register any components as required by components that require `T`. + /// + /// Only use this method if you know what you are doing. In most cases, you should instead use [`World::register_required_components`], + /// or the equivalent method in `bevy_app::App`. + /// + /// [required component]: Component#required-components + #[doc(hidden)] + pub fn register_required_components_manual( + &mut self, + required_components: &mut RequiredComponents, + constructor: fn() -> R, + inheritance_depth: u16, + recursion_check_stack: &mut Vec, + ) { + let requiree = self.register_component_checked_internal::(recursion_check_stack); + let required = self.register_component_checked_internal::(recursion_check_stack); + + // SAFETY: We just created the components. + unsafe { + self.register_required_components_manual_unchecked::( + requiree, + required, + required_components, + constructor, + inheritance_depth, + ); + } + } + + /// Registers a [`Resource`] of type `T` with this instance. + /// If a resource of this type has already been registered, this will return + /// the ID of the pre-existing resource. + /// + /// # See also + /// + /// * [`Components::resource_id()`] + /// * [`Components::register_resource_with_descriptor()`] + #[inline] + pub fn register_resource(&mut self) -> ComponentId { + self.resource_id::().unwrap_or_else(|| { + let id = self.ids.next_mut(); + // SAFETY: The resource is not currently registered, the id is fresh, and the [`ComponentDescriptor`] matches the [`TypeId`] + unsafe { + self.register_resource_with(TypeId::of::(), id, || { + ComponentDescriptor::new_resource::() + }) + } + id + }) } + /// Registers a [non-send resource](crate::system::NonSend) of type `T` with this instance. + /// If a resource of this type has already been registered, this will return + /// the ID of the pre-existing resource. #[inline] - fn register_component_inner( - components: &mut Vec, + pub fn register_non_send(&mut self) -> ComponentId { + let type_id = TypeId::of::(); + self.resource_indices + .get(&type_id) + .copied() + .unwrap_or_else(|| { + let id = self.ids.next_mut(); + // SAFETY: The resource is not currently registered, the id is fresh, and the [`ComponentDescriptor`] matches the [`TypeId`] + unsafe { + self.register_resource_with(type_id, id, || { + ComponentDescriptor::new_non_send::(StorageType::default()) + }) + } + id + }) + } + + /// Registers a [`Resource`] described by `descriptor`. + /// + /// # Note + /// + /// If this method is called multiple times with identical descriptors, a distinct [`ComponentId`] + /// will be created for each one. + /// + /// # See also + /// + /// * [`Components::resource_id()`] + /// * [`Components::register_resource()`] + pub fn register_resource_with_descriptor( + &mut self, descriptor: ComponentDescriptor, ) -> ComponentId { - let component_id = ComponentId(components.len()); - let info = ComponentInfo::new(component_id, descriptor); - components.push(info); - component_id + let id = self.ids.next_mut(); + // SAFETY: The id is fresh. + unsafe { + self.register_component_inner(id, descriptor); + } + id + } +} + +impl Components { + /// # Safety + /// + /// Neither this component, nor it's id may be registered. This must be a new registration. + #[inline] + unsafe fn register_component_internal( + &mut self, + recursion_check_stack: &mut Vec, + id: ComponentId, + ) { + // SAFETY: ensured by caller. + unsafe { + self.register_component_inner(id, ComponentDescriptor::new::()); + } + let type_id = TypeId::of::(); + let prev = self.indices.insert(type_id, id); + debug_assert!(prev.is_none()); + + let mut required_components = RequiredComponents::default(); + T::register_required_components( + id, + self, + &mut required_components, + 0, + recursion_check_stack, + ); + // SAFETY: we just inserted it in `register_component_inner` + let info = unsafe { &mut self.components.get_mut(&id).debug_checked_unwrap() }; + + #[expect( + deprecated, + reason = "need to use this method until it is removed to ensure user defined components register hooks correctly" + )] + // TODO: Replace with `info.hooks.update_from_component::();` once `Component::register_component_hooks` is removed + T::register_component_hooks(&mut info.hooks); + + info.required_components = required_components; + } + + /// # Safety + /// + /// The id must have never been registered before. THis must be a fresh registration. + #[inline] + unsafe fn register_component_inner( + &mut self, + id: ComponentId, + descriptor: ComponentDescriptor, + ) { + let info = ComponentInfo::new(id, descriptor); + let prev = self.components.insert(id, info); + debug_assert!(prev.is_none()) } /// Returns the number of components registered with this instance. @@ -1285,7 +1449,7 @@ impl Components { /// This will return an incorrect result if `id` did not come from the same world as `self`. It may return `None` or a garbage value. #[inline] pub fn get_info(&self, id: ComponentId) -> Option<&ComponentInfo> { - self.components.get(id.0) + self.components.get(&id) } /// Returns the name associated with the given component. @@ -1302,14 +1466,13 @@ impl Components { /// `id` must be a valid [`ComponentId`] #[inline] pub unsafe fn get_info_unchecked(&self, id: ComponentId) -> &ComponentInfo { - debug_assert!(id.index() < self.components.len()); // SAFETY: The caller ensures `id` is valid. - unsafe { self.components.get_unchecked(id.0) } + unsafe { self.get_info(id).debug_checked_unwrap() } } #[inline] pub(crate) fn get_hooks_mut(&mut self, id: ComponentId) -> Option<&mut ComponentHooks> { - self.components.get_mut(id.0).map(|info| &mut info.hooks) + self.components.get_mut(&id).map(|info| &mut info.hooks) } #[inline] @@ -1318,7 +1481,7 @@ impl Components { id: ComponentId, ) -> Option<&mut RequiredComponents> { self.components - .get_mut(id.0) + .get_mut(&id) .map(|info| &mut info.required_components) } @@ -1484,48 +1647,6 @@ impl Components { inherited_requirements } - // NOTE: This should maybe be private, but it is currently public so that `bevy_ecs_macros` can use it. - // We can't directly move this there either, because this uses `Components::get_required_by_mut`, - // which is private, and could be equally risky to expose to users. - /// Registers the given component `R` and [required components] inherited from it as required by `T`, - /// and adds `T` to their lists of requirees. - /// - /// The given `inheritance_depth` determines how many levels of inheritance deep the requirement is. - /// A direct requirement has a depth of `0`, and each level of inheritance increases the depth by `1`. - /// Lower depths are more specific requirements, and can override existing less specific registrations. - /// - /// The `recursion_check_stack` allows checking whether this component tried to register itself as its - /// own (indirect) required component. - /// - /// This method does *not* register any components as required by components that require `T`. - /// - /// Only use this method if you know what you are doing. In most cases, you should instead use [`World::register_required_components`], - /// or the equivalent method in `bevy_app::App`. - /// - /// [required component]: Component#required-components - #[doc(hidden)] - pub fn register_required_components_manual( - &mut self, - required_components: &mut RequiredComponents, - constructor: fn() -> R, - inheritance_depth: u16, - recursion_check_stack: &mut Vec, - ) { - 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 { - self.register_required_components_manual_unchecked::( - requiree, - required, - required_components, - constructor, - inheritance_depth, - ); - } - } - /// Registers the given component `R` and [required components] inherited from it as required by `T`, /// and adds `T` to their lists of requirees. /// @@ -1567,7 +1688,7 @@ impl Components { #[inline] pub(crate) fn get_required_by(&self, id: ComponentId) -> Option<&HashSet> { - self.components.get(id.0).map(|info| &info.required_by) + self.components.get(&id).map(|info| &info.required_by) } #[inline] @@ -1576,7 +1697,7 @@ impl Components { id: ComponentId, ) -> Option<&mut HashSet> { self.components - .get_mut(id.0) + .get_mut(&id) .map(|info| &mut info.required_by) } @@ -1655,84 +1776,29 @@ impl Components { self.get_resource_id(TypeId::of::()) } - /// Registers a [`Resource`] of type `T` with this instance. - /// If a resource of this type has already been registered, this will return - /// the ID of the pre-existing resource. - /// - /// # See also - /// - /// * [`Components::resource_id()`] - /// * [`Components::register_resource_with_descriptor()`] - #[inline] - pub fn register_resource(&mut self) -> ComponentId { - // SAFETY: The [`ComponentDescriptor`] matches the [`TypeId`] - unsafe { - self.get_or_register_resource_with(TypeId::of::(), || { - ComponentDescriptor::new_resource::() - }) - } - } - - /// Registers a [`Resource`] described by `descriptor`. - /// - /// # Note - /// - /// If this method is called multiple times with identical descriptors, a distinct [`ComponentId`] - /// will be created for each one. - /// - /// # See also - /// - /// * [`Components::resource_id()`] - /// * [`Components::register_resource()`] - pub fn register_resource_with_descriptor( - &mut self, - descriptor: ComponentDescriptor, - ) -> ComponentId { - Components::register_resource_inner(&mut self.components, descriptor) - } - - /// Registers a [non-send resource](crate::system::NonSend) of type `T` with this instance. - /// If a resource of this type has already been registered, this will return - /// the ID of the pre-existing resource. - #[inline] - pub fn register_non_send(&mut self) -> ComponentId { - // SAFETY: The [`ComponentDescriptor`] matches the [`TypeId`] - unsafe { - self.get_or_register_resource_with(TypeId::of::(), || { - ComponentDescriptor::new_non_send::(StorageType::default()) - }) - } - } - /// # Safety /// - /// The [`ComponentDescriptor`] must match the [`TypeId`] + /// The [`ComponentDescriptor`] must match the [`TypeId`]. + /// The [`ComponentId`] must be unique. + /// The [`TypeId`] must have never been registered. #[inline] - unsafe fn get_or_register_resource_with( + unsafe fn register_resource_with( &mut self, type_id: TypeId, + component_id: ComponentId, func: impl FnOnce() -> ComponentDescriptor, - ) -> ComponentId { - let components = &mut self.components; - *self.resource_indices.entry(type_id).or_insert_with(|| { - let descriptor = func(); - Components::register_resource_inner(components, descriptor) - }) - } - - #[inline] - fn register_resource_inner( - components: &mut Vec, - descriptor: ComponentDescriptor, - ) -> ComponentId { - let component_id = ComponentId(components.len()); - components.push(ComponentInfo::new(component_id, descriptor)); - component_id + ) { + // SAFETY: ensured by caller + unsafe { + self.register_component_inner(component_id, func()); + } + let prev = self.resource_indices.insert(type_id, component_id); + debug_assert!(prev.is_none()) } /// Gets an iterator over all components registered with this instance. pub fn iter(&self) -> impl Iterator + '_ { - self.components.iter() + self.components.values() } } @@ -1916,7 +1982,7 @@ impl ComponentIdFor<'_, T> { } } -impl core::ops::Deref for ComponentIdFor<'_, T> { +impl Deref for ComponentIdFor<'_, T> { type Target = ComponentId; fn deref(&self) -> &Self::Target { &self.0.component_id @@ -2058,7 +2124,7 @@ impl RequiredComponents { /// is smaller than the depth of the existing registration. Otherwise, the new registration will be ignored. pub fn register( &mut self, - components: &mut Components, + components: &mut ComponentsView, constructor: fn() -> C, inheritance_depth: u16, ) { From 3efef9cd718dbf5507a658c645425e4daf3f7094 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Wed, 5 Mar 2025 10:18:19 -0500 Subject: [PATCH 03/22] handled the major errors --- crates/bevy_ecs/macros/src/component.rs | 2 +- crates/bevy_ecs/macros/src/lib.rs | 4 +- crates/bevy_ecs/src/bundle.rs | 34 ++++---- crates/bevy_ecs/src/component.rs | 104 ++++++++++++++---------- crates/bevy_ecs/src/world/mod.rs | 55 +++++++++---- 5 files changed, 121 insertions(+), 78 deletions(-) diff --git a/crates/bevy_ecs/macros/src/component.rs b/crates/bevy_ecs/macros/src/component.rs index 06dd9b1591e69..7de4cece9460b 100644 --- a/crates/bevy_ecs/macros/src/component.rs +++ b/crates/bevy_ecs/macros/src/component.rs @@ -223,7 +223,7 @@ pub fn derive_component(input: TokenStream) -> TokenStream { type Mutability = #mutable_type; fn register_required_components( requiree: #bevy_ecs_path::component::ComponentId, - components: &mut #bevy_ecs_path::component::Components, + components: &mut #bevy_ecs_path::component::ComponentsRegistrator, 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> diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index f61889651df1e..919711ef496e4 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -134,7 +134,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { #[allow(deprecated)] unsafe impl #impl_generics #ecs_path::bundle::Bundle for #struct_name #ty_generics #where_clause { fn component_ids( - components: &mut #ecs_path::component::Components, + components: &mut #ecs_path::component::ComponentsRegistrator, ids: &mut impl FnMut(#ecs_path::component::ComponentId) ){ #(#field_component_ids)* @@ -148,7 +148,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { } fn register_required_components( - components: &mut #ecs_path::component::Components, + components: &mut #ecs_path::component::ComponentsRegistrator, 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 fcfd514f68838..c917730e8c6ed 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -11,8 +11,8 @@ use crate::{ }, change_detection::MaybeLocation, component::{ - Component, ComponentId, Components, RequiredComponentConstructor, RequiredComponents, - StorageType, Tick, + Component, ComponentId, Components, ComponentsRegistrator, RequiredComponentConstructor, + RequiredComponents, StorageType, Tick, }, entity::{Entities, Entity, EntityLocation}, observer::Observers, @@ -30,7 +30,7 @@ use variadics_please::all_tuples; /// The `Bundle` trait enables insertion and removal of [`Component`]s from an entity. /// -/// Implementors of the `Bundle` trait are called 'bundles'. +/// Implementers of the `Bundle` trait are called 'bundles'. /// /// Each bundle represents a static set of [`Component`] types. /// Currently, bundles can only contain one of each [`Component`], and will @@ -71,7 +71,7 @@ use variadics_please::all_tuples; /// That is, if the entity does not have all the components of the bundle, those /// which are present will be removed. /// -/// # Implementors +/// # Implementers /// /// Every type which implements [`Component`] also implements `Bundle`, since /// [`Component`] types can be added to or removed from an entity. @@ -150,14 +150,14 @@ 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, ids: &mut impl FnMut(ComponentId)); + fn component_ids(components: &mut ComponentsRegistrator, 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)); /// Registers components that are required by the components in this [`Bundle`]. fn register_required_components( - _components: &mut Components, + _components: &mut ComponentsRegistrator, _required_components: &mut RequiredComponents, ); } @@ -222,12 +222,12 @@ pub trait BundleEffect { // - `Bundle::component_ids` calls `ids` for C's component id (and nothing else) // - `Bundle::get_components` is called exactly once for C and passes the component's storage type based on its associated constant. unsafe impl Bundle for C { - fn component_ids(components: &mut Components, ids: &mut impl FnMut(ComponentId)) { + fn component_ids(components: &mut ComponentsRegistrator, ids: &mut impl FnMut(ComponentId)) { ids(components.register_component::()); } fn register_required_components( - components: &mut Components, + components: &mut ComponentsRegistrator, required_components: &mut RequiredComponents, ) { let component_id = components.register_component::(); @@ -287,7 +287,7 @@ 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, ids: &mut impl FnMut(ComponentId)){ + fn component_ids(components: &mut ComponentsRegistrator, ids: &mut impl FnMut(ComponentId)){ $(<$name as Bundle>::component_ids(components, ids);)* } @@ -296,7 +296,7 @@ macro_rules! tuple_impl { } fn register_required_components( - components: &mut Components, + components: &mut ComponentsRegistrator, required_components: &mut RequiredComponents, ) { $(<$name as Bundle>::register_required_components(components, required_components);)* @@ -998,9 +998,12 @@ impl<'w> BundleInserter<'w> { archetype_id: ArchetypeId, change_tick: Tick, ) -> Self { + // SAFETY: These come from the same world. + let mut registrator = + unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) }; let bundle_id = world .bundles - .register_info::(&mut world.components, &mut world.storages); + .register_info::(&mut registrator, &mut world.storages); // SAFETY: We just ensured this bundle exists unsafe { Self::new_with_id(world, archetype_id, bundle_id, change_tick) } } @@ -1365,9 +1368,12 @@ pub(crate) struct BundleSpawner<'w> { impl<'w> BundleSpawner<'w> { #[inline] pub fn new(world: &'w mut World, change_tick: Tick) -> Self { + // SAFETY: These come from the same world. + let mut registrator = + unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) }; let bundle_id = world .bundles - .register_info::(&mut world.components, &mut world.storages); + .register_info::(&mut registrator, &mut world.storages); // SAFETY: we initialized this bundle_id in `init_info` unsafe { Self::new_with_id(world, bundle_id, change_tick) } } @@ -1569,7 +1575,7 @@ impl Bundles { /// Also registers all the components in the bundle. pub(crate) fn register_info( &mut self, - components: &mut Components, + components: &mut ComponentsRegistrator, storages: &mut Storages, ) -> BundleId { let bundle_infos = &mut self.bundle_infos; @@ -1594,7 +1600,7 @@ impl Bundles { /// Also registers all the components in the bundle. pub(crate) fn register_contributed_bundle_info( &mut self, - components: &mut Components, + components: &mut ComponentsRegistrator, storages: &mut Storages, ) -> BundleId { if let Some(id) = self.contributed_bundle_ids.get(&TypeId::of::()).cloned() { diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 8ece5f30dde61..3fa709c9701bf 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -432,7 +432,7 @@ pub trait Component: Send + Sync + 'static { /// Registers required components. fn register_required_components( _component_id: ComponentId, - _components: &mut Components, + _components: &mut ComponentsRegistrator, _required_components: &mut RequiredComponents, _inheritance_depth: u16, _recursion_check_stack: &mut Vec, @@ -1195,12 +1195,12 @@ impl ComponentIds { } /// A type that enables registering in [`Components`]. -pub struct ComponentsView<'w> { +pub struct ComponentsRegistrator<'w> { components: &'w mut Components, ids: &'w mut ComponentIds, } -impl Deref for ComponentsView<'_> { +impl Deref for ComponentsRegistrator<'_> { type Target = Components; fn deref(&self) -> &Self::Target { @@ -1208,13 +1208,23 @@ impl Deref for ComponentsView<'_> { } } -impl DerefMut for ComponentsView<'_> { +impl DerefMut for ComponentsRegistrator<'_> { fn deref_mut(&mut self) -> &mut Self::Target { self.components } } -impl<'w> ComponentsView<'w> { +impl<'w> ComponentsRegistrator<'w> { + /// Constructs a new [`ComponentsRegistrator`]. + /// + /// # Safety + /// + /// The [`Components`] and [`ComponentIds`] must match. + /// For example, they must be from the same world. + pub unsafe fn new(components: &'w mut Components, ids: &'w mut ComponentIds) -> Self { + Self { components, ids } + } + /// Registers a [`Component`] of type `T` with this instance. /// If a component of this type has already been registered, this will return /// the ID of the pre-existing component. @@ -1247,6 +1257,50 @@ impl<'w> ComponentsView<'w> { }) } + /// # Safety + /// + /// Neither this component, nor it's id may be registered. This must be a new registration. + #[inline] + unsafe fn register_component_internal( + &mut self, + recursion_check_stack: &mut Vec, + id: ComponentId, + ) { + // SAFETY: ensured by caller. + unsafe { + self.register_component_inner(id, ComponentDescriptor::new::()); + } + let type_id = TypeId::of::(); + let prev = self.indices.insert(type_id, id); + debug_assert!(prev.is_none()); + + let mut required_components = RequiredComponents::default(); + T::register_required_components( + id, + self, + &mut required_components, + 0, + recursion_check_stack, + ); + // SAFETY: we just inserted it in `register_component_inner` + let info = unsafe { + &mut self + .components + .components + .get_mut(&id) + .debug_checked_unwrap() + }; + + #[expect( + deprecated, + reason = "need to use this method until it is removed to ensure user defined components register hooks correctly" + )] + // TODO: Replace with `info.hooks.update_from_component::();` once `Component::register_component_hooks` is removed + T::register_component_hooks(&mut info.hooks); + + info.required_components = required_components; + } + /// Registers a component described by `descriptor`. /// /// # Note @@ -1380,44 +1434,6 @@ impl<'w> ComponentsView<'w> { } impl Components { - /// # Safety - /// - /// Neither this component, nor it's id may be registered. This must be a new registration. - #[inline] - unsafe fn register_component_internal( - &mut self, - recursion_check_stack: &mut Vec, - id: ComponentId, - ) { - // SAFETY: ensured by caller. - unsafe { - self.register_component_inner(id, ComponentDescriptor::new::()); - } - let type_id = TypeId::of::(); - let prev = self.indices.insert(type_id, id); - debug_assert!(prev.is_none()); - - let mut required_components = RequiredComponents::default(); - T::register_required_components( - id, - self, - &mut required_components, - 0, - recursion_check_stack, - ); - // SAFETY: we just inserted it in `register_component_inner` - let info = unsafe { &mut self.components.get_mut(&id).debug_checked_unwrap() }; - - #[expect( - deprecated, - reason = "need to use this method until it is removed to ensure user defined components register hooks correctly" - )] - // TODO: Replace with `info.hooks.update_from_component::();` once `Component::register_component_hooks` is removed - T::register_component_hooks(&mut info.hooks); - - info.required_components = required_components; - } - /// # Safety /// /// The id must have never been registered before. THis must be a fresh registration. @@ -2124,7 +2140,7 @@ impl RequiredComponents { /// is smaller than the depth of the existing registration. Otherwise, the new registration will be ignored. pub fn register( &mut self, - components: &mut ComponentsView, + components: &mut ComponentsRegistrator, constructor: fn() -> C, inheritance_depth: u16, ) { diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 294145de23049..b79fae715616c 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -38,8 +38,9 @@ use crate::{ }, change_detection::{MaybeLocation, MutUntyped, TicksMut}, component::{ - Component, ComponentDescriptor, ComponentHooks, ComponentId, ComponentInfo, ComponentTicks, - Components, Mutable, RequiredComponents, RequiredComponentsError, Tick, + Component, ComponentDescriptor, ComponentHooks, ComponentId, ComponentIds, ComponentInfo, + ComponentTicks, Components, ComponentsRegistrator, Mutable, RequiredComponents, + RequiredComponentsError, Tick, }, entity::{ AllocAtWithoutReplacement, Entities, Entity, EntityDoesNotExistError, EntityLocation, @@ -89,6 +90,7 @@ pub struct World { id: WorldId, pub(crate) entities: Entities, pub(crate) components: Components, + pub(crate) component_ids: ComponentIds, pub(crate) archetypes: Archetypes, pub(crate) storages: Storages, pub(crate) bundles: Bundles, @@ -119,6 +121,7 @@ impl Default for World { last_check_tick: Tick::new(0), last_trigger_id: 0, command_queue: RawCommandQueue::new(), + component_ids: ComponentIds::default(), }; world.bootstrap(); world @@ -220,6 +223,13 @@ impl World { &self.components } + /// Retrieves this world's [`Components`] collection. + #[inline] + pub fn components_registrator(&mut self) -> ComponentsRegistrator { + // SAFETY: These are from the same world. + unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) } + } + /// Retrieves this world's [`Storages`] collection. #[inline] pub fn storages(&self) -> &Storages { @@ -252,7 +262,7 @@ impl World { /// In most cases, you don't need to call this method directly since component registration /// happens automatically during system initialization. pub fn register_component(&mut self) -> ComponentId { - self.components.register_component::() + self.components_registrator().register_component::() } /// Registers a component type as "disabling", @@ -536,7 +546,7 @@ impl World { &mut self, descriptor: ComponentDescriptor, ) -> ComponentId { - self.components + self.components_registrator() .register_component_with_descriptor(descriptor) } @@ -576,7 +586,7 @@ impl World { /// to insert the [`Resource`] in the [`World`], use [`World::init_resource`] or /// [`World::insert_resource`] instead. pub fn register_resource(&mut self) -> ComponentId { - self.components.register_resource::() + self.components_registrator().register_resource::() } /// Returns the [`ComponentId`] of the given [`Resource`] type `T`. @@ -1573,7 +1583,7 @@ impl World { &mut self, descriptor: ComponentDescriptor, ) -> ComponentId { - self.components + self.components_registrator() .register_resource_with_descriptor(descriptor) } @@ -1588,7 +1598,7 @@ impl World { #[track_caller] pub fn init_resource(&mut self) -> ComponentId { let caller = MaybeLocation::caller(); - let component_id = self.components.register_resource::(); + let component_id = self.components_registrator().register_resource::(); if self .storages .resources @@ -1625,7 +1635,7 @@ impl World { value: R, caller: MaybeLocation, ) { - let component_id = self.components.register_resource::(); + let component_id = self.components_registrator().register_resource::(); OwningPtr::make(value, |ptr| { // SAFETY: component_id was just initialized and corresponds to resource of type R. unsafe { @@ -1649,7 +1659,7 @@ impl World { #[track_caller] pub fn init_non_send_resource(&mut self) -> ComponentId { let caller = MaybeLocation::caller(); - let component_id = self.components.register_non_send::(); + let component_id = self.components_registrator().register_non_send::(); if self .storages .non_send_resources @@ -1680,7 +1690,7 @@ impl World { #[track_caller] pub fn insert_non_send_resource(&mut self, value: R) { let caller = MaybeLocation::caller(); - let component_id = self.components.register_non_send::(); + let component_id = self.components_registrator().register_non_send::(); OwningPtr::make(value, |ptr| { // SAFETY: component_id was just initialized and corresponds to resource of type R. unsafe { @@ -1963,7 +1973,7 @@ impl World { let change_tick = self.change_tick(); let last_change_tick = self.last_change_tick(); - let component_id = self.components.register_resource::(); + let component_id = self.components_registrator().register_resource::(); let data = self.initialize_resource_internal(component_id); if !data.is_present() { OwningPtr::make(func(), |ptr| { @@ -2021,7 +2031,7 @@ impl World { let change_tick = self.change_tick(); let last_change_tick = self.last_change_tick(); - let component_id = self.components.register_resource::(); + let component_id = self.components_registrator().register_resource::(); if self .storages .resources @@ -2176,12 +2186,14 @@ impl World { B: Bundle, { self.flush(); - let change_tick = self.change_tick(); + // SAFETY: These come from the same world. + let mut registrator = + unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) }; let bundle_id = self .bundles - .register_info::(&mut self.components, &mut self.storages); + .register_info::(&mut registrator, &mut self.storages); enum SpawnOrInsert<'w> { Spawn(BundleSpawner<'w>), Insert(BundleInserter<'w>, ArchetypeId), @@ -2346,9 +2358,12 @@ impl World { self.flush(); let change_tick = self.change_tick(); + // SAFETY: These come from the same world. + let mut registrator = + unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) }; let bundle_id = self .bundles - .register_info::(&mut self.components, &mut self.storages); + .register_info::(&mut registrator, &mut self.storages); let mut batch_iter = batch.into_iter(); @@ -2482,9 +2497,12 @@ impl World { self.flush(); let change_tick = self.change_tick(); + // SAFETY: These come from the same world. + let mut registrator = + unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) }; let bundle_id = self .bundles - .register_info::(&mut self.components, &mut self.storages); + .register_info::(&mut registrator, &mut self.storages); let mut invalid_entities = Vec::::new(); let mut batch_iter = batch.into_iter(); @@ -3038,9 +3056,12 @@ impl World { /// component in the bundle. #[inline] pub fn register_bundle(&mut self) -> &BundleInfo { + // SAFETY: These come from the same world. + let mut registrator = + unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) }; let id = self .bundles - .register_info::(&mut self.components, &mut self.storages); + .register_info::(&mut registrator, &mut self.storages); // SAFETY: We just initialized the bundle so its id should definitely be valid. unsafe { self.bundles.get(id).debug_checked_unwrap() } } From 35d9493772aead4d5dfd25d24d3ce06f8dd6d6df Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Wed, 5 Mar 2025 10:30:46 -0500 Subject: [PATCH 04/22] fixed all remaining errors and warning --- crates/bevy_ecs/src/component.rs | 10 ++--- crates/bevy_ecs/src/lib.rs | 13 +++--- crates/bevy_ecs/src/observer/runner.rs | 2 +- crates/bevy_ecs/src/query/mod.rs | 2 +- crates/bevy_ecs/src/schedule/schedule.rs | 2 +- crates/bevy_ecs/src/spawn.rs | 8 ++-- crates/bevy_ecs/src/storage/table/mod.rs | 8 +++- crates/bevy_ecs/src/system/system_param.rs | 8 ++-- crates/bevy_ecs/src/world/entity_ref.rs | 42 ++++++++++++++----- .../bevy_ecs/src/world/filtered_resource.rs | 6 +-- 10 files changed, 64 insertions(+), 37 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 3fa709c9701bf..a5d999948408a 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1382,7 +1382,7 @@ impl<'w> ComponentsRegistrator<'w> { unsafe { self.register_resource_with(TypeId::of::(), id, || { ComponentDescriptor::new_resource::() - }) + }); } id }) @@ -1403,7 +1403,7 @@ impl<'w> ComponentsRegistrator<'w> { unsafe { self.register_resource_with(type_id, id, || { ComponentDescriptor::new_non_send::(StorageType::default()) - }) + }); } id }) @@ -1436,7 +1436,7 @@ impl<'w> ComponentsRegistrator<'w> { impl Components { /// # Safety /// - /// The id must have never been registered before. THis must be a fresh registration. + /// The id must have never been registered before. This must be a fresh registration. #[inline] unsafe fn register_component_inner( &mut self, @@ -1445,7 +1445,7 @@ impl Components { ) { let info = ComponentInfo::new(id, descriptor); let prev = self.components.insert(id, info); - debug_assert!(prev.is_none()) + debug_assert!(prev.is_none()); } /// Returns the number of components registered with this instance. @@ -1809,7 +1809,7 @@ impl Components { self.register_component_inner(component_id, func()); } let prev = self.resource_indices.insert(type_id, component_id); - debug_assert!(prev.is_none()) + debug_assert!(prev.is_none()); } /// Gets an iterator over all components registered with this instance. diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 8d415d7469183..cbf88e26a1d5e 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -229,7 +229,7 @@ mod tests { y: SparseStored, } let mut ids = Vec::new(); - ::component_ids(&mut world.components, &mut |id| { + ::component_ids(&mut world.components_registrator(), &mut |id| { ids.push(id); }); @@ -279,7 +279,7 @@ mod tests { } let mut ids = Vec::new(); - ::component_ids(&mut world.components, &mut |id| { + ::component_ids(&mut world.components_registrator(), &mut |id| { ids.push(id); }); @@ -331,9 +331,12 @@ mod tests { } let mut ids = Vec::new(); - ::component_ids(&mut world.components, &mut |id| { - ids.push(id); - }); + ::component_ids( + &mut world.components_registrator(), + &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 fdb3b4fa800ba..a89b046b751e7 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -435,7 +435,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 |id| { + B::component_ids(&mut world.components_registrator(), &mut |id| { components.push(id); }); let mut descriptor = ObserverDescriptor { diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index 9ed6995876f12..5b0126dd5061e 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -864,7 +864,7 @@ mod tests { } fn init_state(world: &mut World) -> Self::State { - world.components.register_resource::() + world.components_registrator().register_resource::() } fn get_state(components: &Components) -> Option { diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index d67ca188e8172..59764b6cb9d9c 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -146,7 +146,7 @@ impl Schedules { /// Ignore system order ambiguities caused by conflicts on [`Resource`]s of type `T`. pub fn allow_ambiguous_resource(&mut self, world: &mut World) { self.ignored_scheduling_ambiguities - .insert(world.components.register_resource::()); + .insert(world.components_registrator().register_resource::()); } /// Iterate through the [`ComponentId`]'s that will be ignored. diff --git a/crates/bevy_ecs/src/spawn.rs b/crates/bevy_ecs/src/spawn.rs index 2fb04c4c7b293..cf4f8b6fee8f9 100644 --- a/crates/bevy_ecs/src/spawn.rs +++ b/crates/bevy_ecs/src/spawn.rs @@ -175,7 +175,7 @@ unsafe impl + Send + Sync + 'static> Bundle for SpawnRelatedBundle { fn component_ids( - components: &mut crate::component::Components, + components: &mut crate::component::ComponentsRegistrator, ids: &mut impl FnMut(crate::component::ComponentId), ) { ::component_ids(components, ids); @@ -189,7 +189,7 @@ unsafe impl + Send + Sync + 'static> Bundle } fn register_required_components( - components: &mut crate::component::Components, + components: &mut crate::component::ComponentsRegistrator, required_components: &mut crate::component::RequiredComponents, ) { ::register_required_components( @@ -244,7 +244,7 @@ impl DynamicBundle for SpawnOneRelated { // SAFETY: This internally relies on the RelationshipTarget's Bundle implementation, which is sound. unsafe impl Bundle for SpawnOneRelated { fn component_ids( - components: &mut crate::component::Components, + components: &mut crate::component::ComponentsRegistrator, ids: &mut impl FnMut(crate::component::ComponentId), ) { ::component_ids(components, ids); @@ -258,7 +258,7 @@ unsafe impl Bundle for SpawnOneRelated { } fn register_required_components( - components: &mut crate::component::Components, + components: &mut crate::component::ComponentsRegistrator, required_components: &mut crate::component::RequiredComponents, ) { ::register_required_components( diff --git a/crates/bevy_ecs/src/storage/table/mod.rs b/crates/bevy_ecs/src/storage/table/mod.rs index 159a716c4ed05..d211bc10dae68 100644 --- a/crates/bevy_ecs/src/storage/table/mod.rs +++ b/crates/bevy_ecs/src/storage/table/mod.rs @@ -822,7 +822,7 @@ impl Drop for Table { mod tests { use crate::{ change_detection::MaybeLocation, - component::{Component, Components, Tick}, + component::{Component, ComponentIds, Components, ComponentsRegistrator, Tick}, entity::Entity, ptr::OwningPtr, storage::{TableBuilder, TableId, TableRow, Tables}, @@ -847,7 +847,11 @@ mod tests { #[test] fn table() { let mut components = Components::default(); - let component_id = components.register_component::>(); + let mut componentids = ComponentIds::default(); + // SAFETY: They are both new. + let mut registrator = + unsafe { ComponentsRegistrator::new(&mut components, &mut componentids) }; + let component_id = registrator.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/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index c30cc191dd4b0..ddfcddaa76e47 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -817,7 +817,7 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { type Item<'w, 's> = Res<'w, T>; fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { - let component_id = world.components.register_resource::(); + let component_id = world.components_registrator().register_resource::(); let archetype_component_id = world.initialize_resource_internal(component_id).id(); let combined_access = system_meta.component_access_set.combined_access(); @@ -926,7 +926,7 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { type Item<'w, 's> = ResMut<'w, T>; fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { - let component_id = world.components.register_resource::(); + let component_id = world.components_registrator().register_resource::(); let archetype_component_id = world.initialize_resource_internal(component_id).id(); let combined_access = system_meta.component_access_set.combined_access(); @@ -1499,7 +1499,7 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> { fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { system_meta.set_non_send(); - let component_id = world.components.register_non_send::(); + let component_id = world.components_registrator().register_non_send::(); let archetype_component_id = world.initialize_non_send_internal(component_id).id(); let combined_access = system_meta.component_access_set.combined_access(); @@ -1605,7 +1605,7 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> { fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { system_meta.set_non_send(); - let component_id = world.components.register_non_send::(); + let component_id = world.components_registrator().register_non_send::(); let archetype_component_id = world.initialize_non_send_internal(component_id).id(); let combined_access = system_meta.component_access_set.combined_access(); diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index ab4b9dd4de931..a2442b78011b8 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -5,7 +5,10 @@ use crate::{ DynamicBundle, InsertMode, }, change_detection::{MaybeLocation, MutUntyped}, - component::{Component, ComponentId, ComponentTicks, Components, Mutable, StorageType}, + component::{ + Component, ComponentId, ComponentTicks, Components, ComponentsRegistrator, Mutable, + StorageType, + }, entity::{ Entities, Entity, EntityBorrow, EntityCloner, EntityClonerBuilder, EntityLocation, TrustedEntityBorrow, @@ -1703,8 +1706,10 @@ impl<'w> EntityWorldMut<'w> { self.assert_not_despawned(); let world = &mut self.world; let storages = &mut world.storages; - let components = &mut world.components; - let bundle_id = world.bundles.register_info::(components, storages); + // SAFETY: These come from the same world. + let mut registrator = + unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) }; + let bundle_id = world.bundles.register_info::(&mut registrator, storages); // SAFETY: We just ensured this bundle exists let bundle_info = unsafe { world.bundles.get_unchecked(bundle_id) }; let old_location = self.location; @@ -1714,7 +1719,7 @@ impl<'w> EntityWorldMut<'w> { bundle_info.remove_bundle_from_archetype( &mut world.archetypes, storages, - components, + ®istrator, &world.observers, old_location.archetype_id, false, @@ -1986,8 +1991,14 @@ impl<'w> EntityWorldMut<'w> { pub(crate) fn remove_with_caller(&mut self, caller: MaybeLocation) -> &mut Self { self.assert_not_despawned(); let storages = &mut self.world.storages; - let components = &mut self.world.components; - let bundle_info = self.world.bundles.register_info::(components, storages); + // SAFETY: These come from the same world. + let mut registrator = unsafe { + ComponentsRegistrator::new(&mut self.world.components, &mut self.world.component_ids) + }; + let bundle_info = self + .world + .bundles + .register_info::(&mut registrator, storages); // SAFETY: the `BundleInfo` is initialized above self.location = unsafe { self.remove_bundle(bundle_info, caller) }; @@ -2012,10 +2023,13 @@ impl<'w> EntityWorldMut<'w> { ) -> &mut Self { self.assert_not_despawned(); let storages = &mut self.world.storages; - let components = &mut self.world.components; + // SAFETY: These come from the same world. + let mut registrator = unsafe { + ComponentsRegistrator::new(&mut self.world.components, &mut self.world.component_ids) + }; let bundles = &mut self.world.bundles; - let bundle_id = bundles.register_contributed_bundle_info::(components, storages); + 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) }; @@ -2041,9 +2055,15 @@ impl<'w> EntityWorldMut<'w> { self.assert_not_despawned(); let archetypes = &mut self.world.archetypes; let storages = &mut self.world.storages; - let components = &mut self.world.components; + // SAFETY: These come from the same world. + let mut registrator = unsafe { + ComponentsRegistrator::new(&mut self.world.components, &mut self.world.component_ids) + }; - let retained_bundle = self.world.bundles.register_info::(components, storages); + let retained_bundle = self + .world + .bundles + .register_info::(&mut registrator, storages); // SAFETY: `retained_bundle` exists as we just initialized it. let retained_bundle_info = unsafe { self.world.bundles.get_unchecked(retained_bundle) }; let old_location = self.location; @@ -2057,7 +2077,7 @@ impl<'w> EntityWorldMut<'w> { let remove_bundle = self.world .bundles - .init_dynamic_info(&mut self.world.storages, components, to_remove); + .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) }; diff --git a/crates/bevy_ecs/src/world/filtered_resource.rs b/crates/bevy_ecs/src/world/filtered_resource.rs index e5197e05703b6..dead18ff1104a 100644 --- a/crates/bevy_ecs/src/world/filtered_resource.rs +++ b/crates/bevy_ecs/src/world/filtered_resource.rs @@ -542,7 +542,7 @@ impl<'w> FilteredResourcesBuilder<'w> { /// Add accesses required to read the resource of the given type. pub fn add_read(&mut self) -> &mut Self { - let component_id = self.world.components.register_resource::(); + let component_id = self.world.components_registrator().register_resource::(); self.add_read_by_id(component_id) } @@ -588,7 +588,7 @@ impl<'w> FilteredResourcesMutBuilder<'w> { /// Add accesses required to read the resource of the given type. pub fn add_read(&mut self) -> &mut Self { - let component_id = self.world.components.register_resource::(); + let component_id = self.world.components_registrator().register_resource::(); self.add_read_by_id(component_id) } @@ -606,7 +606,7 @@ impl<'w> FilteredResourcesMutBuilder<'w> { /// Add accesses required to get mutable access to the resource of the given type. pub fn add_write(&mut self) -> &mut Self { - let component_id = self.world.components.register_resource::(); + let component_id = self.world.components_registrator().register_resource::(); self.add_write_by_id(component_id) } From f7866161fb79ebdadbfc23eb787bc0c3e7ed0a8b Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Wed, 5 Mar 2025 10:32:17 -0500 Subject: [PATCH 05/22] small improvements --- crates/bevy_ecs/src/component.rs | 2 ++ crates/bevy_ecs/src/world/mod.rs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index a5d999948408a..4d61bce1ab326 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1312,6 +1312,7 @@ impl<'w> ComponentsRegistrator<'w> { /// /// * [`Components::component_id()`] /// * [`Components::register_component()`] + #[inline] pub fn register_component_with_descriptor( &mut self, descriptor: ComponentDescriptor, @@ -1420,6 +1421,7 @@ impl<'w> ComponentsRegistrator<'w> { /// /// * [`Components::resource_id()`] /// * [`Components::register_resource()`] + #[inline] pub fn register_resource_with_descriptor( &mut self, descriptor: ComponentDescriptor, diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index b79fae715616c..d2c3ff64f113a 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -223,7 +223,7 @@ impl World { &self.components } - /// Retrieves this world's [`Components`] collection. + /// Prepares a [`ComponentsRegistrator`] for the world. #[inline] pub fn components_registrator(&mut self) -> ComponentsRegistrator { // SAFETY: These are from the same world. From a49dade80fe40dcc76b55e8f7dbeca34ba0a5881 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Wed, 5 Mar 2025 11:19:30 -0500 Subject: [PATCH 06/22] outlined queuing --- crates/bevy_ecs/src/component.rs | 120 ++++++++++++++++++++++++++++++- 1 file changed, 119 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 4d61bce1ab326..b42b71ccf8234 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -20,7 +20,10 @@ use bevy_platform_support::sync::Arc; use bevy_ptr::{OwningPtr, UnsafeCellDeref}; #[cfg(feature = "bevy_reflect")] use bevy_reflect::Reflect; -use bevy_utils::TypeIdMap; +use bevy_utils::{ + staging::{StageOnWrite, StagedChanges}, + TypeIdMap, +}; use core::{ alloc::Layout, any::{Any, TypeId}, @@ -1139,6 +1142,54 @@ impl ComponentCloneBehavior { } } +/// Coordinates a registration that is reserved, but not registered. +trait QueuedComponentRegistration { + fn register(&mut self, registrator: ComponentsRegistrator); +} + +/// Allows queueing components to be registered. +#[derive(Default)] +pub struct QueuedComponents { + components: TypeIdMap<(ComponentId, Box)>, + resources: TypeIdMap<(ComponentId, Box)>, + dynamic_registrations: Vec<(ComponentId, Box)>, +} + +impl Debug for QueuedComponents { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + let components = self + .components + .iter() + .map(|(type_id, (id, _))| (type_id, id)) + .collect::>(); + let resources = self + .resources + .iter() + .map(|(type_id, (id, _))| (type_id, id)) + .collect::>(); + let dynamic_registrations = self + .dynamic_registrations + .iter() + .map(|(id, _)| id) + .collect::>(); + write!(f, "components: {components:?}, resources: {resources:?}, dynamic_registrations: {dynamic_registrations:?}") + } +} + +impl StagedChanges for QueuedComponents { + type Cold = Components; + + fn apply_staged(&mut self, storage: &mut Self::Cold) { + todo!() + } + + fn any_staged(&self) -> bool { + !self.components.is_empty() + || !self.resources.is_empty() + || !self.dynamic_registrations.is_empty() + } +} + /// Stores metadata associated with each kind of [`Component`] in a given [`World`]. #[derive(Debug, Default)] pub struct Components { @@ -1214,6 +1265,73 @@ impl DerefMut for ComponentsRegistrator<'_> { } } +/// A type that enables queuing registration in [`Components`]. +pub struct ComponentsQueuedRegistrator<'w> { + components: &'w StageOnWrite, + ids: &'w ComponentIds, +} + +impl<'w> ComponentsQueuedRegistrator<'w> { + /// Constructs a new [`ComponentsQueuedRegistrator`]. + /// + /// # Safety + /// + /// The [`Components`] and [`ComponentIds`] must match. + /// For example, they must be from the same world. + pub unsafe fn new( + components: &'w StageOnWrite, + ids: &'w ComponentIds, + ) -> Self { + Self { components, ids } + } + + /// This is a queued version of [`ComponentsRegistrator::register_component`]. + /// This will reserve an id and queue the registration. + /// These registrations will be carried out at the next opportunity. + #[inline] + pub fn queue_register_component(&self) -> ComponentId { + todo!() + } + + /// This is a queued version of [`ComponentsRegistrator::register_component_with_descriptor`]. + /// This will reserve an id and queue the registration. + /// These registrations will be carried out at the next opportunity. + #[inline] + pub fn queue_register_component_with_descriptor( + &self, + descriptor: ComponentDescriptor, + ) -> ComponentId { + todo!() + } + + /// This is a queued version of [`ComponentsRegistrator::register_resource`]. + /// This will reserve an id and queue the registration. + /// These registrations will be carried out at the next opportunity. + #[inline] + pub fn queue_register_resource(&self) -> ComponentId { + todo!() + } + + /// This is a queued version of [`ComponentsRegistrator::register_non_send`]. + /// This will reserve an id and queue the registration. + /// These registrations will be carried out at the next opportunity. + #[inline] + pub fn queue_register_non_send(&self) -> ComponentId { + todo!() + } + + /// This is a queued version of [`ComponentsRegistrator::register_resource_with_descriptor`]. + /// This will reserve an id and queue the registration. + /// These registrations will be carried out at the next opportunity. + #[inline] + pub fn queue_register_resource_with_descriptor( + &self, + descriptor: ComponentDescriptor, + ) -> ComponentId { + todo!() + } +} + impl<'w> ComponentsRegistrator<'w> { /// Constructs a new [`ComponentsRegistrator`]. /// From 95ec1d5c6e6c04d594152df74517746e2ae63a25 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Wed, 5 Mar 2025 14:34:24 -0500 Subject: [PATCH 07/22] rearanged data --- crates/bevy_ecs/src/component.rs | 43 +++++++++----------------------- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index b42b71ccf8234..11c044e36949a 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -20,10 +20,7 @@ use bevy_platform_support::sync::Arc; use bevy_ptr::{OwningPtr, UnsafeCellDeref}; #[cfg(feature = "bevy_reflect")] use bevy_reflect::Reflect; -use bevy_utils::{ - staging::{StageOnWrite, StagedChanges}, - TypeIdMap, -}; +use bevy_utils::TypeIdMap; use core::{ alloc::Layout, any::{Any, TypeId}, @@ -1176,28 +1173,6 @@ impl Debug for QueuedComponents { } } -impl StagedChanges for QueuedComponents { - type Cold = Components; - - fn apply_staged(&mut self, storage: &mut Self::Cold) { - todo!() - } - - fn any_staged(&self) -> bool { - !self.components.is_empty() - || !self.resources.is_empty() - || !self.dynamic_registrations.is_empty() - } -} - -/// Stores metadata associated with each kind of [`Component`] in a given [`World`]. -#[derive(Debug, Default)] -pub struct Components { - components: HashMap, - indices: TypeIdMap, - resource_indices: TypeIdMap, -} - /// Generates [`ComponentId`]s. #[derive(Debug, Default)] pub struct ComponentIds { @@ -1267,7 +1242,7 @@ impl DerefMut for ComponentsRegistrator<'_> { /// A type that enables queuing registration in [`Components`]. pub struct ComponentsQueuedRegistrator<'w> { - components: &'w StageOnWrite, + components: &'w Components, ids: &'w ComponentIds, } @@ -1278,10 +1253,7 @@ impl<'w> ComponentsQueuedRegistrator<'w> { /// /// The [`Components`] and [`ComponentIds`] must match. /// For example, they must be from the same world. - pub unsafe fn new( - components: &'w StageOnWrite, - ids: &'w ComponentIds, - ) -> Self { + pub unsafe fn new(components: &'w Components, ids: &'w ComponentIds) -> Self { Self { components, ids } } @@ -1553,6 +1525,15 @@ impl<'w> ComponentsRegistrator<'w> { } } +/// Stores metadata associated with each kind of [`Component`] in a given [`World`]. +#[derive(Debug, Default)] +pub struct Components { + components: HashMap, + indices: TypeIdMap, + resource_indices: TypeIdMap, + queued: bevy_platform_support::sync::RwLock, +} + impl Components { /// # Safety /// From 825d573c73154cbd7f12c61f3b52e1e75f4c51d7 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Wed, 5 Mar 2025 14:42:54 -0500 Subject: [PATCH 08/22] small move --- crates/bevy_ecs/src/component.rs | 40 ++++++++++++++++---------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 11c044e36949a..4df251e29df12 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1220,26 +1220,6 @@ impl ComponentIds { } } -/// A type that enables registering in [`Components`]. -pub struct ComponentsRegistrator<'w> { - components: &'w mut Components, - ids: &'w mut ComponentIds, -} - -impl Deref for ComponentsRegistrator<'_> { - type Target = Components; - - fn deref(&self) -> &Self::Target { - self.components - } -} - -impl DerefMut for ComponentsRegistrator<'_> { - fn deref_mut(&mut self) -> &mut Self::Target { - self.components - } -} - /// A type that enables queuing registration in [`Components`]. pub struct ComponentsQueuedRegistrator<'w> { components: &'w Components, @@ -1304,6 +1284,26 @@ impl<'w> ComponentsQueuedRegistrator<'w> { } } +/// A type that enables registering in [`Components`]. +pub struct ComponentsRegistrator<'w> { + components: &'w mut Components, + ids: &'w mut ComponentIds, +} + +impl Deref for ComponentsRegistrator<'_> { + type Target = Components; + + fn deref(&self) -> &Self::Target { + self.components + } +} + +impl DerefMut for ComponentsRegistrator<'_> { + fn deref_mut(&mut self) -> &mut Self::Target { + self.components + } +} + impl<'w> ComponentsRegistrator<'w> { /// Constructs a new [`ComponentsRegistrator`]. /// From 53f9d9630c234e27085056737429376cccdf99aa Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Wed, 5 Mar 2025 15:24:12 -0500 Subject: [PATCH 09/22] respect the queued registrations --- crates/bevy_ecs/src/component.rs | 200 ++++++++++++++++++++++--------- 1 file changed, 141 insertions(+), 59 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 4df251e29df12..6132930c53dc5 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -15,8 +15,11 @@ use crate::{ use alloc::boxed::Box; use alloc::{borrow::Cow, format, vec::Vec}; pub use bevy_ecs_macros::Component; -use bevy_platform_support::collections::{HashMap, HashSet}; use bevy_platform_support::sync::Arc; +use bevy_platform_support::{ + collections::{HashMap, HashSet}, + sync::PoisonError, +}; use bevy_ptr::{OwningPtr, UnsafeCellDeref}; #[cfg(feature = "bevy_reflect")] use bevy_reflect::Reflect; @@ -1141,7 +1144,7 @@ impl ComponentCloneBehavior { /// Coordinates a registration that is reserved, but not registered. trait QueuedComponentRegistration { - fn register(&mut self, registrator: ComponentsRegistrator); + fn register(&mut self, registrator: &mut ComponentsRegistrator, this_id: ComponentId); } /// Allows queueing components to be registered. @@ -1284,7 +1287,7 @@ impl<'w> ComponentsQueuedRegistrator<'w> { } } -/// A type that enables registering in [`Components`]. +/// A [`Components`] wrapper that enables additional features, like registration. pub struct ComponentsRegistrator<'w> { components: &'w mut Components, ids: &'w mut ComponentIds, @@ -1325,33 +1328,46 @@ impl<'w> ComponentsRegistrator<'w> { /// * [`Components::register_component_with_descriptor()`] #[inline] pub fn register_component(&mut self) -> ComponentId { - self.component_id::().unwrap_or_else(|| { - let id = self.ids.next_mut(); - // SAFETY: The component is not currently registered, and the id is fresh. - unsafe { self.register_component_internal::(&mut Vec::new(), id) } - id - }) + self.register_component_checked::(&mut Vec::new()) } - /// Same as [`register_component`] but keeps a `recursion_check_stack`. + /// Same as [`Self::register_component_internal`] but keeps a checks for safety. #[inline] - fn register_component_checked_internal( + fn register_component_checked( &mut self, recursion_check_stack: &mut Vec, ) -> ComponentId { - self.component_id::().unwrap_or_else(|| { - let id = self.ids.next_mut(); - // SAFETY: The component is not currently registered, and the id is fresh. - unsafe { self.register_component_internal::(recursion_check_stack, id) } - id - }) + let type_id = TypeId::of::(); + if let Some(id) = self.indices.get(&type_id) { + return *id; + } + + if let Some((id, mut registrator)) = self + .queued + .get_mut() + .unwrap_or_else(PoisonError::into_inner) + .components + .remove(&type_id) + { + // If we are trying to register something that has already been queued, we respect the queue. + // Just like if we are trying to register something that already is, we respect the first registration. + registrator.register(self, id); + return id; + } + + let id = self.ids.next_mut(); + // SAFETY: The component is not currently registered, and the id is fresh. + unsafe { + self.register_component_unchecked::(recursion_check_stack, id); + } + id } /// # Safety /// - /// Neither this component, nor it's id may be registered. This must be a new registration. + /// Neither this component, nor it's id may be registered or queued. This must be a new registration. #[inline] - unsafe fn register_component_internal( + unsafe fn register_component_unchecked( &mut self, recursion_check_stack: &mut Vec, id: ComponentId, @@ -1442,8 +1458,8 @@ impl<'w> ComponentsRegistrator<'w> { inheritance_depth: u16, recursion_check_stack: &mut Vec, ) { - let requiree = self.register_component_checked_internal::(recursion_check_stack); - let required = self.register_component_checked_internal::(recursion_check_stack); + let requiree = self.register_component_checked::(recursion_check_stack); + let required = self.register_component_checked::(recursion_check_stack); // SAFETY: We just created the components. unsafe { @@ -1467,16 +1483,12 @@ impl<'w> ComponentsRegistrator<'w> { /// * [`Components::register_resource_with_descriptor()`] #[inline] pub fn register_resource(&mut self) -> ComponentId { - self.resource_id::().unwrap_or_else(|| { - let id = self.ids.next_mut(); - // SAFETY: The resource is not currently registered, the id is fresh, and the [`ComponentDescriptor`] matches the [`TypeId`] - unsafe { - self.register_resource_with(TypeId::of::(), id, || { - ComponentDescriptor::new_resource::() - }); - } - id - }) + // SAFETY: The [`ComponentDescriptor`] matches the [`TypeId`] + unsafe { + self.register_resource_with(TypeId::of::(), || { + ComponentDescriptor::new_resource::() + }) + } } /// Registers a [non-send resource](crate::system::NonSend) of type `T` with this instance. @@ -1484,20 +1496,48 @@ impl<'w> ComponentsRegistrator<'w> { /// the ID of the pre-existing resource. #[inline] pub fn register_non_send(&mut self) -> ComponentId { - let type_id = TypeId::of::(); - self.resource_indices - .get(&type_id) - .copied() - .unwrap_or_else(|| { - let id = self.ids.next_mut(); - // SAFETY: The resource is not currently registered, the id is fresh, and the [`ComponentDescriptor`] matches the [`TypeId`] - unsafe { - self.register_resource_with(type_id, id, || { - ComponentDescriptor::new_non_send::(StorageType::default()) - }); - } - id + // SAFETY: The [`ComponentDescriptor`] matches the [`TypeId`] + unsafe { + self.register_resource_with(TypeId::of::(), || { + ComponentDescriptor::new_non_send::(StorageType::default()) }) + } + } + + /// Same as [`Components::register_resource_unchecked_with`] but handles safety. + /// + /// # Safety + /// + /// The [`ComponentDescriptor`] must match the [`TypeId`]. + #[inline] + unsafe fn register_resource_with( + &mut self, + type_id: TypeId, + descriptor: impl FnOnce() -> ComponentDescriptor, + ) -> ComponentId { + if let Some(id) = self.resource_indices.get(&type_id) { + return *id; + } + + if let Some((id, mut registrator)) = self + .queued + .get_mut() + .unwrap_or_else(PoisonError::into_inner) + .resources + .remove(&type_id) + { + // If we are trying to register something that has already been queued, we respect the queue. + // Just like if we are trying to register something that already is, we respect the first registration. + registrator.register(self, id); + return id; + } + + let id = self.ids.next_mut(); + // SAFETY: The resource is not currently registered, the id is fresh, and the [`ComponentDescriptor`] matches the [`TypeId`] + unsafe { + self.register_resource_unchecked_with(type_id, id, descriptor); + } + id } /// Registers a [`Resource`] described by `descriptor`. @@ -1531,6 +1571,7 @@ pub struct Components { components: HashMap, indices: TypeIdMap, resource_indices: TypeIdMap, + // This is kept internal and local to verify that no deadlocks can occor. queued: bevy_platform_support::sync::RwLock, } @@ -1549,19 +1590,45 @@ impl Components { debug_assert!(prev.is_none()); } - /// Returns the number of components registered with this instance. + /// Returns the number of components registered or queued with this instance. #[inline] pub fn len(&self) -> usize { - self.components.len() + self.num_queued() + self.num_registered() } - /// Returns `true` if there are no components registered with this instance. Otherwise, this returns `false`. + /// Returns `true` if there are no components registered or queued with this instance. Otherwise, this returns `false`. #[inline] pub fn is_empty(&self) -> bool { - self.components.len() == 0 + self.len() == 0 } - /// Gets the metadata associated with the given component. + /// Returns the number of components registered with this instance. + #[inline] + pub fn num_queued(&self) -> usize { + let queued = self.queued.read().unwrap_or_else(PoisonError::into_inner); + queued.components.len() + queued.dynamic_registrations.len() + queued.resources.len() + } + + /// Returns `true` if there are any components registered with this instance. Otherwise, this returns `false`. + #[inline] + pub fn any_queued(&self) -> bool { + self.num_queued() > 0 + } + + /// Returns the number of components registered with this instance. + #[inline] + pub fn num_registered(&self) -> usize { + self.components.len() + } + + /// Returns `true` if there are any components registered with this instance. Otherwise, this returns `false`. + #[inline] + pub fn any_registered(&self) -> bool { + self.num_registered() > 0 + } + + /// Gets the metadata associated with the given component, if it is registered. + /// This will return `None` if the id is not regiserted or is queued. /// /// This will return an incorrect result if `id` did not come from the same world as `self`. It may return `None` or a garbage value. #[inline] @@ -1569,7 +1636,8 @@ impl Components { self.components.get(&id) } - /// Returns the name associated with the given component. + /// Returns the name associated with the given component, if it is registered. + /// This will return `None` if the id is not regiserted or is queued. /// /// This will return an incorrect result if `id` did not come from the same world as `self`. It may return `None` or a garbage value. #[inline] @@ -1580,7 +1648,7 @@ impl Components { /// Gets the metadata associated with the given component. /// # Safety /// - /// `id` must be a valid [`ComponentId`] + /// `id` must be a valid and fully registered [`ComponentId`]. #[inline] pub unsafe fn get_info_unchecked(&self, id: ComponentId) -> &ComponentInfo { // SAFETY: The caller ensures `id` is valid. @@ -1821,7 +1889,14 @@ impl Components { /// Type-erased equivalent of [`Components::component_id()`]. #[inline] pub fn get_id(&self, type_id: TypeId) -> Option { - self.indices.get(&type_id).copied() + self.indices.get(&type_id).copied().or_else(|| { + self.queued + .read() + .unwrap_or_else(PoisonError::into_inner) + .components + .get(&type_id) + .map(|queued| queued.0) + }) } /// Returns the [`ComponentId`] of the given [`Component`] type `T`. @@ -1831,7 +1906,7 @@ impl Components { /// instance. /// /// Returns [`None`] if the `Component` type has not - /// yet been initialized using [`Components::register_component()`]. + /// yet been initialized using [`Components::register_component()`] or [`Components::queue_register_component()`]. /// /// ``` /// use bevy_ecs::prelude::*; @@ -1859,7 +1934,14 @@ impl Components { /// Type-erased equivalent of [`Components::resource_id()`]. #[inline] pub fn get_resource_id(&self, type_id: TypeId) -> Option { - self.resource_indices.get(&type_id).copied() + self.resource_indices.get(&type_id).copied().or_else(|| { + self.queued + .read() + .unwrap_or_else(PoisonError::into_inner) + .resources + .get(&type_id) + .map(|queued| queued.0) + }) } /// Returns the [`ComponentId`] of the given [`Resource`] type `T`. @@ -1869,7 +1951,7 @@ impl Components { /// instance. /// /// Returns [`None`] if the `Resource` type has not - /// yet been initialized using [`Components::register_resource()`]. + /// yet been initialized using [`Components::register_resource()`] or [`Components::queue_register_resource()`]. /// /// ``` /// use bevy_ecs::prelude::*; @@ -1897,9 +1979,9 @@ impl Components { /// /// The [`ComponentDescriptor`] must match the [`TypeId`]. /// The [`ComponentId`] must be unique. - /// The [`TypeId`] must have never been registered. + /// The [`TypeId`] and [`ComponentId`] must not be registered or queued. #[inline] - unsafe fn register_resource_with( + unsafe fn register_resource_unchecked_with( &mut self, type_id: TypeId, component_id: ComponentId, @@ -1913,8 +1995,8 @@ impl Components { debug_assert!(prev.is_none()); } - /// Gets an iterator over all components registered with this instance. - pub fn iter(&self) -> impl Iterator + '_ { + /// Gets an iterator over all components fully registered with this instance. + pub fn iter_registered(&self) -> impl Iterator + '_ { self.components.values() } } From 3aab1a4cc14726b42a18c2fbf1bd87d6da154f50 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Wed, 5 Mar 2025 15:40:32 -0500 Subject: [PATCH 10/22] apply registrations --- crates/bevy_ecs/src/component.rs | 44 ++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 6132930c53dc5..f1c62aeb2306f 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1318,6 +1318,50 @@ impl<'w> ComponentsRegistrator<'w> { Self { components, ids } } + /// Applies every queued registration. + /// This ensures that every valid [`ComponentId`] is registered, + /// enabling retrieving [`ComponentInfo`], etc. + pub fn apply_queued_registrations(&mut self) { + // components + while let Some((id, mut registrator)) = { + let queued = self + .queued + .get_mut() + .unwrap_or_else(PoisonError::into_inner); + queued.components.keys().next().copied().map(|type_id| { + // SAFETY: the id just came from a valid iterator. + unsafe { queued.components.remove(&type_id).debug_checked_unwrap() } + }) + } { + registrator.register(self, id); + } + + // resources + while let Some((id, mut registrator)) = { + let queued = self + .queued + .get_mut() + .unwrap_or_else(PoisonError::into_inner); + queued.resources.keys().next().copied().map(|type_id| { + // SAFETY: the id just came from a valid iterator. + unsafe { queued.resources.remove(&type_id).debug_checked_unwrap() } + }) + } { + registrator.register(self, id); + } + + // dynamic + for (id, mut registrator) in core::mem::take( + &mut self + .queued + .get_mut() + .unwrap_or_else(PoisonError::into_inner) + .dynamic_registrations, + ) { + registrator.register(self, id); + } + } + /// Registers a [`Component`] of type `T` with this instance. /// If a component of this type has already been registered, this will return /// the ID of the pre-existing component. From 6a976ef2fa9fe851103680b4aa771a04c5200610 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Wed, 5 Mar 2025 16:43:02 -0500 Subject: [PATCH 11/22] fully implemented queueing --- crates/bevy_ecs/src/component.rs | 199 +++++++++++++++++++++++++++++-- 1 file changed, 188 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index f1c62aeb2306f..65a7879fcc9a5 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1144,14 +1144,43 @@ impl ComponentCloneBehavior { /// Coordinates a registration that is reserved, but not registered. trait QueuedComponentRegistration { - fn register(&mut self, registrator: &mut ComponentsRegistrator, this_id: ComponentId); + /// Performs the registration. + /// + /// # Safery + /// + /// This must only ever be called once with a unique id. + unsafe fn register(&mut self, registrator: &mut ComponentsRegistrator, this_id: ComponentId); +} + +/// A [`QueuedComponentRegistration`] from an arbitrary function. +struct ArbitraryQueuedComponentRegistration( + Option, +); + +impl QueuedComponentRegistration + for ArbitraryQueuedComponentRegistration +{ + unsafe fn register(&mut self, registrator: &mut ComponentsRegistrator, this_id: ComponentId) { + // SAFETY: The inner value is always `Some` until this is called, and this is only called once. + let func = unsafe { self.0.take().debug_checked_unwrap() }; + func(registrator, this_id); + } +} + +impl ArbitraryQueuedComponentRegistration { + fn new(func: F) -> Self { + Self(Some(func)) + } } /// Allows queueing components to be registered. #[derive(Default)] pub struct QueuedComponents { + // SAFETY: These `ComponentId`s must be unique components: TypeIdMap<(ComponentId, Box)>, + // SAFETY: These `ComponentId`s must be unique resources: TypeIdMap<(ComponentId, Box)>, + // SAFETY: These `ComponentId`s must be unique dynamic_registrations: Vec<(ComponentId, Box)>, } @@ -1229,6 +1258,14 @@ pub struct ComponentsQueuedRegistrator<'w> { ids: &'w ComponentIds, } +impl Deref for ComponentsQueuedRegistrator<'_> { + type Target = Components; + + fn deref(&self) -> &Self::Target { + self.components + } +} + impl<'w> ComponentsQueuedRegistrator<'w> { /// Constructs a new [`ComponentsQueuedRegistrator`]. /// @@ -1240,12 +1277,93 @@ impl<'w> ComponentsQueuedRegistrator<'w> { Self { components, ids } } + /// Queues this function to run as a component registrator. + /// + /// # Safety + /// + /// The [`TypeId`] must not already be registered or queued as a component. + unsafe fn force_register_arbitrary_component( + &self, + type_id: TypeId, + func: impl FnOnce(&mut ComponentsRegistrator, ComponentId) + 'static, + ) -> ComponentId { + let id = self.ids.next(); + self.components + .queued + .write() + .unwrap_or_else(PoisonError::into_inner) + .components + .insert( + type_id, + ( + id, + Box::new(ArbitraryQueuedComponentRegistration::new(func)), + ), + ); + id + } + + /// Queues this function to run as a component registrator. + /// + /// # Safety + /// + /// The [`TypeId`] must not already be registered or queued as a resource. + unsafe fn force_register_arbitrary_resource( + &self, + type_id: TypeId, + func: impl FnOnce(&mut ComponentsRegistrator, ComponentId) + 'static, + ) -> ComponentId { + let id = self.ids.next(); + self.components + .queued + .write() + .unwrap_or_else(PoisonError::into_inner) + .resources + .insert( + type_id, + ( + id, + Box::new(ArbitraryQueuedComponentRegistration::new(func)), + ), + ); + id + } + + /// Queues this function to run as a dynamic registrator. + fn force_register_arbitrary_dynamic( + &self, + func: impl FnOnce(&mut ComponentsRegistrator, ComponentId) + 'static, + ) -> ComponentId { + let id = self.ids.next(); + self.components + .queued + .write() + .unwrap_or_else(PoisonError::into_inner) + .dynamic_registrations + .push(( + id, + Box::new(ArbitraryQueuedComponentRegistration::new(func)), + )); + id + } + /// This is a queued version of [`ComponentsRegistrator::register_component`]. /// This will reserve an id and queue the registration. /// These registrations will be carried out at the next opportunity. #[inline] pub fn queue_register_component(&self) -> ComponentId { - todo!() + self.component_id::().unwrap_or_else(|| { + // SAFETY: We just checked that this type was not in the queue. + unsafe { + self.force_register_arbitrary_component(TypeId::of::(), |registrator, id| { + // SAFETY: We just checked that this is not currently registered or queued, and if it was registered since, this would have been dropped from the queue. + #[expect(unused_unsafe, reason = "More precise to specify.")] + unsafe { + registrator.register_component_unchecked::(&mut Vec::new(), id); + } + }) + } + }) } /// This is a queued version of [`ComponentsRegistrator::register_component_with_descriptor`]. @@ -1256,7 +1374,12 @@ impl<'w> ComponentsQueuedRegistrator<'w> { &self, descriptor: ComponentDescriptor, ) -> ComponentId { - todo!() + self.force_register_arbitrary_dynamic(|registrator, id| { + // SAFETY: Id uniqueness handled by caller. + unsafe { + registrator.register_component_inner(id, descriptor); + } + }) } /// This is a queued version of [`ComponentsRegistrator::register_resource`]. @@ -1264,7 +1387,22 @@ impl<'w> ComponentsQueuedRegistrator<'w> { /// These registrations will be carried out at the next opportunity. #[inline] pub fn queue_register_resource(&self) -> ComponentId { - todo!() + let type_id = TypeId::of::(); + self.get_resource_id(type_id).unwrap_or_else(|| { + // SAFETY: We just checked that this type was not in the queue. + unsafe { + self.force_register_arbitrary_resource(type_id, move |registrator, id| { + // SAFETY: We just checked that this is not currently registered or queued, and if it was registered since, this would have been dropped from the queue. + // SAFETY: Id uniqueness handled by caller, and the type_id matches descriptor. + #[expect(unused_unsafe, reason = "More precise to specify.")] + unsafe { + registrator.register_resource_unchecked_with(type_id, id, || { + ComponentDescriptor::new_resource::() + }); + } + }) + } + }) } /// This is a queued version of [`ComponentsRegistrator::register_non_send`]. @@ -1272,7 +1410,22 @@ impl<'w> ComponentsQueuedRegistrator<'w> { /// These registrations will be carried out at the next opportunity. #[inline] pub fn queue_register_non_send(&self) -> ComponentId { - todo!() + let type_id = TypeId::of::(); + self.get_resource_id(type_id).unwrap_or_else(|| { + // SAFETY: We just checked that this type was not in the queue. + unsafe { + self.force_register_arbitrary_resource(type_id, move |registrator, id| { + // SAFETY: We just checked that this is not currently registered or queued, and if it was registered since, this would have been dropped from the queue. + // SAFETY: Id uniqueness handled by caller, and the type_id matches descriptor. + #[expect(unused_unsafe, reason = "More precise to specify.")] + unsafe { + registrator.register_resource_unchecked_with(type_id, id, || { + ComponentDescriptor::new_non_send::(StorageType::default()) + }); + } + }) + } + }) } /// This is a queued version of [`ComponentsRegistrator::register_resource_with_descriptor`]. @@ -1283,7 +1436,12 @@ impl<'w> ComponentsQueuedRegistrator<'w> { &self, descriptor: ComponentDescriptor, ) -> ComponentId { - todo!() + self.force_register_arbitrary_dynamic(|registrator, id| { + // SAFETY: Id uniqueness handled by caller. + unsafe { + registrator.register_component_inner(id, descriptor); + } + }) } } @@ -1333,7 +1491,10 @@ impl<'w> ComponentsRegistrator<'w> { unsafe { queued.components.remove(&type_id).debug_checked_unwrap() } }) } { - registrator.register(self, id); + // SAFETY: we own this value and it is being dropped, and the id came from the unique queue. + unsafe { + registrator.register(self, id); + } } // resources @@ -1347,7 +1508,10 @@ impl<'w> ComponentsRegistrator<'w> { unsafe { queued.resources.remove(&type_id).debug_checked_unwrap() } }) } { - registrator.register(self, id); + // SAFETY: we own this value and it is being dropped, and the id came from the unique queue. + unsafe { + registrator.register(self, id); + } } // dynamic @@ -1358,7 +1522,10 @@ impl<'w> ComponentsRegistrator<'w> { .unwrap_or_else(PoisonError::into_inner) .dynamic_registrations, ) { - registrator.register(self, id); + // SAFETY: we own this value and it is being dropped, and the id came from the unique queue. + unsafe { + registrator.register(self, id); + } } } @@ -1395,7 +1562,11 @@ impl<'w> ComponentsRegistrator<'w> { { // If we are trying to register something that has already been queued, we respect the queue. // Just like if we are trying to register something that already is, we respect the first registration. - registrator.register(self, id); + // + // SAFETY: we own this value and it is being dropped, and the id came from the unique queue. + unsafe { + registrator.register(self, id); + } return id; } @@ -1572,7 +1743,11 @@ impl<'w> ComponentsRegistrator<'w> { { // If we are trying to register something that has already been queued, we respect the queue. // Just like if we are trying to register something that already is, we respect the first registration. - registrator.register(self, id); + // + // SAFETY: we own this value and it is being dropped, and the id came from the unique queue. + unsafe { + registrator.register(self, id); + } return id; } @@ -1620,6 +1795,8 @@ pub struct Components { } impl Components { + /// This registers any descriptor, component or resource. + /// /// # Safety /// /// The id must have never been registered before. This must be a fresh registration. From 1b980329f89e191e53bc66b5d5f3d20762b147d2 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Wed, 5 Mar 2025 17:27:03 -0500 Subject: [PATCH 12/22] fixed docs --- crates/bevy_ecs/src/component.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 65a7879fcc9a5..b61e8bc5ed210 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1536,13 +1536,13 @@ impl<'w> ComponentsRegistrator<'w> { /// # See also /// /// * [`Components::component_id()`] - /// * [`Components::register_component_with_descriptor()`] + /// * [`ComponentsRegistrator::register_component_with_descriptor()`] #[inline] pub fn register_component(&mut self) -> ComponentId { self.register_component_checked::(&mut Vec::new()) } - /// Same as [`Self::register_component_internal`] but keeps a checks for safety. + /// Same as [`Self::register_component_unchecked`] but keeps a checks for safety. #[inline] fn register_component_checked( &mut self, @@ -1632,7 +1632,7 @@ impl<'w> ComponentsRegistrator<'w> { /// # See also /// /// * [`Components::component_id()`] - /// * [`Components::register_component()`] + /// * [`ComponentsRegistrator::register_component()`] #[inline] pub fn register_component_with_descriptor( &mut self, @@ -1695,7 +1695,7 @@ impl<'w> ComponentsRegistrator<'w> { /// # See also /// /// * [`Components::resource_id()`] - /// * [`Components::register_resource_with_descriptor()`] + /// * [`ComponentsRegistrator::register_resource_with_descriptor()`] #[inline] pub fn register_resource(&mut self) -> ComponentId { // SAFETY: The [`ComponentDescriptor`] matches the [`TypeId`] @@ -1769,7 +1769,7 @@ impl<'w> ComponentsRegistrator<'w> { /// # See also /// /// * [`Components::resource_id()`] - /// * [`Components::register_resource()`] + /// * [`ComponentsRegistrator::register_resource()`] #[inline] pub fn register_resource_with_descriptor( &mut self, @@ -2127,7 +2127,7 @@ impl Components { /// instance. /// /// Returns [`None`] if the `Component` type has not - /// yet been initialized using [`Components::register_component()`] or [`Components::queue_register_component()`]. + /// yet been initialized using [`ComponentsRegistrator::register_component()`] or [`ComponentsQueuedRegistrator::queue_register_component()`]. /// /// ``` /// use bevy_ecs::prelude::*; @@ -2172,7 +2172,7 @@ impl Components { /// instance. /// /// Returns [`None`] if the `Resource` type has not - /// yet been initialized using [`Components::register_resource()`] or [`Components::queue_register_resource()`]. + /// yet been initialized using [`ComponentsRegistrator::register_resource()`] or [`ComponentsQueuedRegistrator::queue_register_resource()`]. /// /// ``` /// use bevy_ecs::prelude::*; @@ -2361,7 +2361,7 @@ impl ComponentTicks { /// Manually sets the change tick. /// - /// This is normally done automatically via the [`DerefMut`](std::ops::DerefMut) implementation + /// This is normally done automatically via the [`DerefMut`] implementation /// on [`Mut`](crate::change_detection::Mut), [`ResMut`](crate::change_detection::ResMut), etc. /// However, components and resources that make use of interior mutability might require manual updates. /// From 2741c775e2f9c9e782ef87e6b47396d7198d4619 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Wed, 5 Mar 2025 20:56:34 -0500 Subject: [PATCH 13/22] final touches --- crates/bevy_ecs/src/component.rs | 78 +++++++++++++++++++++++++++++++- crates/bevy_ecs/src/world/mod.rs | 23 +++++++++- 2 files changed, 98 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index b61e8bc5ed210..ce2e5d73bb6aa 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1253,6 +1253,21 @@ impl ComponentIds { } /// A type that enables queuing registration in [`Components`]. +/// +/// # Note +/// +/// These queued registrations return [`ComponentId`]s. +/// These ids are not yet valid, but they will become valid +/// when either [`ComponentRegistrator::apply_queued_registrations`] is called or the same registration is made directly. +/// In either case, the returned [`ComponentId`]s will be correct, but they are not correct yet. +/// +/// Generally, that means these [`ComponentId`]s can be safely used for read-only purposes. +/// Modifying the contents of the world through these [`ComponentId`]s directly without waiting for them to be fully registered +/// and without then confirming that they have been fully registered is not supported. +/// Hence, extra care is needed with these [`ComponentId`]s to ensure all safety rules are followed. +/// +/// As a rule of thumb, if you have mutable access to [`ComponentRegistrator`], prefer to use that instead. +/// Use this only if you need to know the id of a component but do not need to modify the contents of the world based on that id. pub struct ComponentsQueuedRegistrator<'w> { components: &'w Components, ids: &'w ComponentIds, @@ -1303,7 +1318,7 @@ impl<'w> ComponentsQueuedRegistrator<'w> { id } - /// Queues this function to run as a component registrator. + /// Queues this function to run as a resource registrator. /// /// # Safety /// @@ -1350,6 +1365,11 @@ impl<'w> ComponentsQueuedRegistrator<'w> { /// This is a queued version of [`ComponentsRegistrator::register_component`]. /// This will reserve an id and queue the registration. /// These registrations will be carried out at the next opportunity. + /// + /// # Note + /// + /// Technically speaking, the returned [`ComponentId`] is not valid, but it will become valid later. + /// See type level docs for details. #[inline] pub fn queue_register_component(&self) -> ComponentId { self.component_id::().unwrap_or_else(|| { @@ -1369,6 +1389,11 @@ impl<'w> ComponentsQueuedRegistrator<'w> { /// This is a queued version of [`ComponentsRegistrator::register_component_with_descriptor`]. /// This will reserve an id and queue the registration. /// These registrations will be carried out at the next opportunity. + /// + /// # Note + /// + /// Technically speaking, the returned [`ComponentId`] is not valid, but it will become valid later. + /// See type level docs for details. #[inline] pub fn queue_register_component_with_descriptor( &self, @@ -1385,6 +1410,11 @@ impl<'w> ComponentsQueuedRegistrator<'w> { /// This is a queued version of [`ComponentsRegistrator::register_resource`]. /// This will reserve an id and queue the registration. /// These registrations will be carried out at the next opportunity. + /// + /// # Note + /// + /// Technically speaking, the returned [`ComponentId`] is not valid, but it will become valid later. + /// See type level docs for details. #[inline] pub fn queue_register_resource(&self) -> ComponentId { let type_id = TypeId::of::(); @@ -1408,6 +1438,11 @@ impl<'w> ComponentsQueuedRegistrator<'w> { /// This is a queued version of [`ComponentsRegistrator::register_non_send`]. /// This will reserve an id and queue the registration. /// These registrations will be carried out at the next opportunity. + /// + /// # Note + /// + /// Technically speaking, the returned [`ComponentId`] is not valid, but it will become valid later. + /// See type level docs for details. #[inline] pub fn queue_register_non_send(&self) -> ComponentId { let type_id = TypeId::of::(); @@ -1431,6 +1466,11 @@ impl<'w> ComponentsQueuedRegistrator<'w> { /// This is a queued version of [`ComponentsRegistrator::register_resource_with_descriptor`]. /// This will reserve an id and queue the registration. /// These registrations will be carried out at the next opportunity. + /// + /// # Note + /// + /// Technically speaking, the returned [`ComponentId`] is not valid, but it will become valid later. + /// See type level docs for details. #[inline] pub fn queue_register_resource_with_descriptor( &self, @@ -1476,10 +1516,30 @@ impl<'w> ComponentsRegistrator<'w> { Self { components, ids } } + /// Splits this [`ComponentsRegistrator`] into its parts. + /// This can then be used to initialize either [`ComponentsRegistrator`] or [`ComponentsQueuedRegistrator`]. + /// This is intended for use to pass this value to a function that requires [`ComponentsQueuedRegistrator`] without dropping the references mutability. + /// See also [`as_queued`](Self::as_queued). + pub fn split(self) -> (&'w mut Components, &'w mut ComponentIds) { + (self.components, self.ids) + } + + /// Converts this [`ComponentsRegistrator`] into a [`ComponentsQueuedRegistrator`]. + /// This is intended for use to pass this value to a function that requires [`ComponentsQueuedRegistrator`]. + /// See also [`split`](Self::split). + pub fn as_queued(self) -> ComponentsQueuedRegistrator<'w> { + // SAFETY: ensured by the caller that created self. + unsafe { ComponentsQueuedRegistrator::new(self.components, self.ids) } + } + /// Applies every queued registration. /// This ensures that every valid [`ComponentId`] is registered, /// enabling retrieving [`ComponentInfo`], etc. pub fn apply_queued_registrations(&mut self) { + if !self.any_queued_mut() { + return; + } + // components while let Some((id, mut registrator)) = { let queued = self @@ -1836,6 +1896,22 @@ impl Components { self.num_queued() > 0 } + /// A faster version of [`Self::num_queued`]. + #[inline] + pub fn num_queued_mut(&mut self) -> usize { + let queued = self + .queued + .get_mut() + .unwrap_or_else(PoisonError::into_inner); + queued.components.len() + queued.dynamic_registrations.len() + queued.resources.len() + } + + /// A faster version of [`Self::any_queued`]. + #[inline] + pub fn any_queued_mut(&mut self) -> bool { + self.num_queued_mut() > 0 + } + /// Returns the number of components registered with this instance. #[inline] pub fn num_registered(&self) -> usize { diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index d2c3ff64f113a..d652097c52605 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -39,8 +39,8 @@ use crate::{ change_detection::{MaybeLocation, MutUntyped, TicksMut}, component::{ Component, ComponentDescriptor, ComponentHooks, ComponentId, ComponentIds, ComponentInfo, - ComponentTicks, Components, ComponentsRegistrator, Mutable, RequiredComponents, - RequiredComponentsError, Tick, + ComponentTicks, Components, ComponentsQueuedRegistrator, ComponentsRegistrator, Mutable, + RequiredComponents, RequiredComponentsError, Tick, }, entity::{ AllocAtWithoutReplacement, Entities, Entity, EntityDoesNotExistError, EntityLocation, @@ -223,6 +223,15 @@ impl World { &self.components } + /// Prepares a [`ComponentsQueuedRegistrator`] for the world. + /// **NOTE:** [`ComponentsQueuedRegistrator`] is easily misused. + /// See its docs for important notes on when and how it should be used. + #[inline] + pub fn components_queue(&self) -> ComponentsQueuedRegistrator { + // SAFETY: These are from the same world. + unsafe { ComponentsQueuedRegistrator::new(&self.components, &self.component_ids) } + } + /// Prepares a [`ComponentsRegistrator`] for the world. #[inline] pub fn components_registrator(&mut self) -> ComponentsRegistrator { @@ -2817,12 +2826,22 @@ impl World { } } + /// Applies any queued component registration. + /// For spawning vanilla rust component types and resources, this is not strictly necessary. + /// However, flushing components can make information available more quickly,and can have performance benefits. + /// Additionally, for components and resources registered dynamically through a raw descriptor or similar, + /// this is the only way to complete their registration. + pub(crate) fn flush_components(&mut self) { + self.components_registrator().apply_queued_registrations(); + } + /// Flushes queued entities and commands. /// /// Queued entities will be spawned, and then commands will be applied. #[inline] pub fn flush(&mut self) { self.flush_entities(); + self.flush_components(); self.flush_commands(); } From e84684d6a9b31224c11721753fa08b89dcb439ba Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Wed, 5 Mar 2025 20:59:06 -0500 Subject: [PATCH 14/22] simplified as_queued --- crates/bevy_ecs/src/component.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index ce2e5d73bb6aa..c23762389b8ec 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1516,18 +1516,10 @@ impl<'w> ComponentsRegistrator<'w> { Self { components, ids } } - /// Splits this [`ComponentsRegistrator`] into its parts. - /// This can then be used to initialize either [`ComponentsRegistrator`] or [`ComponentsQueuedRegistrator`]. - /// This is intended for use to pass this value to a function that requires [`ComponentsQueuedRegistrator`] without dropping the references mutability. - /// See also [`as_queued`](Self::as_queued). - pub fn split(self) -> (&'w mut Components, &'w mut ComponentIds) { - (self.components, self.ids) - } - /// Converts this [`ComponentsRegistrator`] into a [`ComponentsQueuedRegistrator`]. /// This is intended for use to pass this value to a function that requires [`ComponentsQueuedRegistrator`]. - /// See also [`split`](Self::split). - pub fn as_queued(self) -> ComponentsQueuedRegistrator<'w> { + /// It is generally not a good idea to queue a registration when you can instead register directly on this type. + pub fn as_queued(&self) -> ComponentsQueuedRegistrator<'_> { // SAFETY: ensured by the caller that created self. unsafe { ComponentsQueuedRegistrator::new(self.components, self.ids) } } From afdd3c1dffe82111a0a800e5a26b348728c736e1 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Wed, 5 Mar 2025 21:58:00 -0500 Subject: [PATCH 15/22] fixed ci hopefully --- crates/bevy_ecs/src/component.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index c23762389b8ec..895ba7937d9d6 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -11,7 +11,6 @@ use crate::{ system::{Commands, Local, SystemParam}, world::{DeferredWorld, FromWorld, World}, }; -#[cfg(feature = "bevy_reflect")] use alloc::boxed::Box; use alloc::{borrow::Cow, format, vec::Vec}; pub use bevy_ecs_macros::Component; @@ -1173,7 +1172,7 @@ impl ArbitraryQueuedComponen } } -/// Allows queueing components to be registered. +/// Allows queuing components to be registered. #[derive(Default)] pub struct QueuedComponents { // SAFETY: These `ComponentId`s must be unique @@ -1258,7 +1257,7 @@ impl ComponentIds { /// /// These queued registrations return [`ComponentId`]s. /// These ids are not yet valid, but they will become valid -/// when either [`ComponentRegistrator::apply_queued_registrations`] is called or the same registration is made directly. +/// when either [`ComponentsRegistrator::apply_queued_registrations`] is called or the same registration is made directly. /// In either case, the returned [`ComponentId`]s will be correct, but they are not correct yet. /// /// Generally, that means these [`ComponentId`]s can be safely used for read-only purposes. @@ -1266,7 +1265,7 @@ impl ComponentIds { /// and without then confirming that they have been fully registered is not supported. /// Hence, extra care is needed with these [`ComponentId`]s to ensure all safety rules are followed. /// -/// As a rule of thumb, if you have mutable access to [`ComponentRegistrator`], prefer to use that instead. +/// As a rule of thumb, if you have mutable access to [`ComponentsRegistrator`], prefer to use that instead. /// Use this only if you need to know the id of a component but do not need to modify the contents of the world based on that id. pub struct ComponentsQueuedRegistrator<'w> { components: &'w Components, From d2672b5939e8a6b2f60174cf8bac2ef304df5d15 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 6 Mar 2025 09:29:57 -0500 Subject: [PATCH 16/22] Minor docs changes. Co-Authored-By: andriyDev --- crates/bevy_ecs/src/bundle.rs | 4 ++-- crates/bevy_ecs/src/component.rs | 5 +++-- crates/bevy_ecs/src/world/mod.rs | 10 +++++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 54660f9c305ae..62bf13d3fac00 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -999,7 +999,7 @@ impl<'w> BundleInserter<'w> { archetype_id: ArchetypeId, change_tick: Tick, ) -> Self { - // SAFETY: These come from the same world. + // SAFETY: These come from the same world. `world.components_registrator` can't be used since we borrow other fields too. let mut registrator = unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) }; let bundle_id = world @@ -1372,7 +1372,7 @@ pub(crate) struct BundleSpawner<'w> { impl<'w> BundleSpawner<'w> { #[inline] pub fn new(world: &'w mut World, change_tick: Tick) -> Self { - // SAFETY: These come from the same world. + // SAFETY: These come from the same world. `world.components_registrator` can't be used since we borrow other fields too. let mut registrator = unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) }; let bundle_id = world diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 711eba1ac8bfa..8b7cf77eb4c11 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1144,7 +1144,8 @@ trait QueuedComponentRegistration { /// /// # Safery /// - /// This must only ever be called once with a unique id. + /// This must only ever be called once. + /// The [`ComponentId`] must be unique. unsafe fn register(&mut self, registrator: &mut ComponentsRegistrator, this_id: ComponentId); } @@ -1628,7 +1629,7 @@ impl<'w> ComponentsRegistrator<'w> { /// # Safety /// - /// Neither this component, nor it's id may be registered or queued. This must be a new registration. + /// Neither this component, nor its id may be registered or queued. This must be a new registration. #[inline] unsafe fn register_component_unchecked( &mut self, diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index f5da23ba607d3..44962a47080ca 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2198,7 +2198,7 @@ impl World { self.flush(); let change_tick = self.change_tick(); - // SAFETY: These come from the same world. + // SAFETY: These come from the same world. `Self.components_registrator` can't be used since we borrow other fields too. let mut registrator = unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) }; let bundle_id = self @@ -2370,7 +2370,7 @@ impl World { self.flush(); let change_tick = self.change_tick(); - // SAFETY: These come from the same world. + // SAFETY: These come from the same world. `Self.components_registrator` can't be used since we borrow other fields too. let mut registrator = unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) }; let bundle_id = self @@ -2515,7 +2515,7 @@ impl World { self.flush(); let change_tick = self.change_tick(); - // SAFETY: These come from the same world. + // SAFETY: These come from the same world. `Self.components_registrator` can't be used since we borrow other fields too. let mut registrator = unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) }; let bundle_id = self @@ -2843,7 +2843,7 @@ impl World { /// Applies any queued component registration. /// For spawning vanilla rust component types and resources, this is not strictly necessary. - /// However, flushing components can make information available more quickly,and can have performance benefits. + /// However, flushing components can make information available more quickly, and can have performance benefits. /// Additionally, for components and resources registered dynamically through a raw descriptor or similar, /// this is the only way to complete their registration. pub(crate) fn flush_components(&mut self) { @@ -3090,7 +3090,7 @@ impl World { /// component in the bundle. #[inline] pub fn register_bundle(&mut self) -> &BundleInfo { - // SAFETY: These come from the same world. + // SAFETY: These come from the same world. `Self.components_registrator` can't be used since we borrow other fields too. let mut registrator = unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) }; let id = self From a329e2d6de9e1df21bd4ab1ce4ddf08d603a30ee Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 6 Mar 2025 09:48:34 -0500 Subject: [PATCH 17/22] Redid queued registration without trait indirection Thanks for the idea @andriyDev --- crates/bevy_ecs/src/component.rs | 125 ++++++++++++------------------- 1 file changed, 48 insertions(+), 77 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 8b7cf77eb4c11..5f272e13f92d7 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1138,47 +1138,41 @@ impl ComponentCloneBehavior { } } -/// Coordinates a registration that is reserved, but not registered. -trait QueuedComponentRegistration { - /// Performs the registration. - /// - /// # Safery - /// - /// This must only ever be called once. - /// The [`ComponentId`] must be unique. - unsafe fn register(&mut self, registrator: &mut ComponentsRegistrator, this_id: ComponentId); +/// A queued component registration. +struct QueuedRegistration { + registrator: Box, + id: ComponentId, } -/// A [`QueuedComponentRegistration`] from an arbitrary function. -struct ArbitraryQueuedComponentRegistration( - Option, -); - -impl QueuedComponentRegistration - for ArbitraryQueuedComponentRegistration -{ - unsafe fn register(&mut self, registrator: &mut ComponentsRegistrator, this_id: ComponentId) { - // SAFETY: The inner value is always `Some` until this is called, and this is only called once. - let func = unsafe { self.0.take().debug_checked_unwrap() }; - func(registrator, this_id); +impl QueuedRegistration { + /// Creates the [`QueuedRegistration`]. + /// + /// # Safety + /// + /// [`ComponentId`] must be unique. + unsafe fn new( + id: ComponentId, + func: impl FnOnce(&mut ComponentsRegistrator, ComponentId) + 'static, + ) -> Self { + Self { + registrator: Box::new(func), + id, + } } -} -impl ArbitraryQueuedComponentRegistration { - fn new(func: F) -> Self { - Self(Some(func)) + /// Performs the registration, returning the now valid [`ComponentId`]. + fn register(self, registrator: &mut ComponentsRegistrator) -> ComponentId { + (self.registrator)(registrator, self.id); + self.id } } /// Allows queuing components to be registered. #[derive(Default)] pub struct QueuedComponents { - // SAFETY: These `ComponentId`s must be unique - components: TypeIdMap<(ComponentId, Box)>, - // SAFETY: These `ComponentId`s must be unique - resources: TypeIdMap<(ComponentId, Box)>, - // SAFETY: These `ComponentId`s must be unique - dynamic_registrations: Vec<(ComponentId, Box)>, + components: TypeIdMap, + resources: TypeIdMap, + dynamic_registrations: Vec, } impl Debug for QueuedComponents { @@ -1186,17 +1180,17 @@ impl Debug for QueuedComponents { let components = self .components .iter() - .map(|(type_id, (id, _))| (type_id, id)) + .map(|(type_id, queued)| (type_id, queued.id)) .collect::>(); let resources = self .resources .iter() - .map(|(type_id, (id, _))| (type_id, id)) + .map(|(type_id, queued)| (type_id, queued.id)) .collect::>(); let dynamic_registrations = self .dynamic_registrations .iter() - .map(|(id, _)| id) + .map(|queued| queued.id) .collect::>(); write!(f, "components: {components:?}, resources: {resources:?}, dynamic_registrations: {dynamic_registrations:?}") } @@ -1307,10 +1301,8 @@ impl<'w> ComponentsQueuedRegistrator<'w> { .components .insert( type_id, - ( - id, - Box::new(ArbitraryQueuedComponentRegistration::new(func)), - ), + // SAFETY: The id was just generated. + unsafe { QueuedRegistration::new(id, func) }, ); id } @@ -1333,10 +1325,8 @@ impl<'w> ComponentsQueuedRegistrator<'w> { .resources .insert( type_id, - ( - id, - Box::new(ArbitraryQueuedComponentRegistration::new(func)), - ), + // SAFETY: The id was just generated. + unsafe { QueuedRegistration::new(id, func) }, ); id } @@ -1352,10 +1342,10 @@ impl<'w> ComponentsQueuedRegistrator<'w> { .write() .unwrap_or_else(PoisonError::into_inner) .dynamic_registrations - .push(( - id, - Box::new(ArbitraryQueuedComponentRegistration::new(func)), - )); + .push( + // SAFETY: The id was just generated. + unsafe { QueuedRegistration::new(id, func) }, + ); id } @@ -1530,7 +1520,7 @@ impl<'w> ComponentsRegistrator<'w> { } // components - while let Some((id, mut registrator)) = { + while let Some(registrator) = { let queued = self .queued .get_mut() @@ -1540,14 +1530,11 @@ impl<'w> ComponentsRegistrator<'w> { unsafe { queued.components.remove(&type_id).debug_checked_unwrap() } }) } { - // SAFETY: we own this value and it is being dropped, and the id came from the unique queue. - unsafe { - registrator.register(self, id); - } + registrator.register(self); } // resources - while let Some((id, mut registrator)) = { + while let Some(registrator) = { let queued = self .queued .get_mut() @@ -1557,24 +1544,18 @@ impl<'w> ComponentsRegistrator<'w> { unsafe { queued.resources.remove(&type_id).debug_checked_unwrap() } }) } { - // SAFETY: we own this value and it is being dropped, and the id came from the unique queue. - unsafe { - registrator.register(self, id); - } + registrator.register(self); } // dynamic - for (id, mut registrator) in core::mem::take( + for registrator in core::mem::take( &mut self .queued .get_mut() .unwrap_or_else(PoisonError::into_inner) .dynamic_registrations, ) { - // SAFETY: we own this value and it is being dropped, and the id came from the unique queue. - unsafe { - registrator.register(self, id); - } + registrator.register(self); } } @@ -1602,7 +1583,7 @@ impl<'w> ComponentsRegistrator<'w> { return *id; } - if let Some((id, mut registrator)) = self + if let Some(registrator) = self .queued .get_mut() .unwrap_or_else(PoisonError::into_inner) @@ -1611,12 +1592,7 @@ impl<'w> ComponentsRegistrator<'w> { { // If we are trying to register something that has already been queued, we respect the queue. // Just like if we are trying to register something that already is, we respect the first registration. - // - // SAFETY: we own this value and it is being dropped, and the id came from the unique queue. - unsafe { - registrator.register(self, id); - } - return id; + return registrator.register(self); } let id = self.ids.next_mut(); @@ -1783,7 +1759,7 @@ impl<'w> ComponentsRegistrator<'w> { return *id; } - if let Some((id, mut registrator)) = self + if let Some(registrator) = self .queued .get_mut() .unwrap_or_else(PoisonError::into_inner) @@ -1792,12 +1768,7 @@ impl<'w> ComponentsRegistrator<'w> { { // If we are trying to register something that has already been queued, we respect the queue. // Just like if we are trying to register something that already is, we respect the first registration. - // - // SAFETY: we own this value and it is being dropped, and the id came from the unique queue. - unsafe { - registrator.register(self, id); - } - return id; + return registrator.register(self); } let id = self.ids.next_mut(); @@ -2181,7 +2152,7 @@ impl Components { .unwrap_or_else(PoisonError::into_inner) .components .get(&type_id) - .map(|queued| queued.0) + .map(|queued| queued.id) }) } @@ -2226,7 +2197,7 @@ impl Components { .unwrap_or_else(PoisonError::into_inner) .resources .get(&type_id) - .map(|queued| queued.0) + .map(|queued| queued.id) }) } From ed4dc9970b9884a5e8d8f7f6429585c452289366 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 6 Mar 2025 10:33:06 -0500 Subject: [PATCH 18/22] comments and reduced allocations Prevented allocation when dynamic_registrations is empty. --- crates/bevy_ecs/src/component.rs | 33 ++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 5f272e13f92d7..cc050921b40b4 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1519,6 +1519,23 @@ impl<'w> ComponentsRegistrator<'w> { return; } + // Note: + // + // This is not just draining the queue. We need to empty the queue without removing the information from `Components`. + // If we drained directly, we could break invariance. + // + // For example, say `ComponentA` and `ComponentB` are queued, and `ComponentA` requires `ComponentB`. + // If we drain directly, and `ComponentA` was the first to be registered, then, when `ComponentA` + // registers `ComponentB` in `Component::register_required_components`, + // `Components` will not know that `ComponentB` was queued + // (since it will have been drained from the queue.) + // If that happened, `Components` would assign a new `ComponentId` to `ComponentB` + // which would be *different* than the id it was assigned in the queue. + // Then, when the drain iterator gets to `ComponentB`, + // it would be unsafely registering `ComponentB`, which is already registered. + // + // As a result, we need to pop from each queue one by one instead of draining. + // components while let Some(registrator) = { let queued = self @@ -1548,14 +1565,14 @@ impl<'w> ComponentsRegistrator<'w> { } // dynamic - for registrator in core::mem::take( - &mut self - .queued - .get_mut() - .unwrap_or_else(PoisonError::into_inner) - .dynamic_registrations, - ) { - registrator.register(self); + let queued = &mut self + .queued + .get_mut() + .unwrap_or_else(PoisonError::into_inner); + if !queued.dynamic_registrations.is_empty() { + for registrator in core::mem::take(&mut queued.dynamic_registrations) { + registrator.register(self); + } } } From a7523bed34745aad62eac568d2cf420924ed170e Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 6 Mar 2025 11:40:10 -0500 Subject: [PATCH 19/22] clarified access to queued Co-Authored-By: andriyDev --- crates/bevy_ecs/src/component.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index cc050921b40b4..1ef92ddc688c8 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1539,6 +1539,7 @@ impl<'w> ComponentsRegistrator<'w> { // components while let Some(registrator) = { let queued = self + .components .queued .get_mut() .unwrap_or_else(PoisonError::into_inner); @@ -1553,6 +1554,7 @@ impl<'w> ComponentsRegistrator<'w> { // resources while let Some(registrator) = { let queued = self + .components .queued .get_mut() .unwrap_or_else(PoisonError::into_inner); @@ -1566,6 +1568,7 @@ impl<'w> ComponentsRegistrator<'w> { // dynamic let queued = &mut self + .components .queued .get_mut() .unwrap_or_else(PoisonError::into_inner); @@ -1601,6 +1604,7 @@ impl<'w> ComponentsRegistrator<'w> { } if let Some(registrator) = self + .components .queued .get_mut() .unwrap_or_else(PoisonError::into_inner) @@ -1777,6 +1781,7 @@ impl<'w> ComponentsRegistrator<'w> { } if let Some(registrator) = self + .components .queued .get_mut() .unwrap_or_else(PoisonError::into_inner) From 3c9943f507805a4e7877727d7b6b4587f9884fd2 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 6 Mar 2025 17:10:02 -0500 Subject: [PATCH 20/22] added ways to check registration easily --- crates/bevy_ecs/src/component.rs | 71 ++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 1ef92ddc688c8..72c05300454a6 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -2165,6 +2165,77 @@ impl Components { .map(|info| &mut info.required_by) } + /// Returns true if the [`ComponentId`] is fully registered and valid. + /// Ids may be invalid if they are still queued to be registered. + /// Those ids are still correct, but they are not usable in every context yet. + #[inline] + pub fn is_id_valid(&self, id: ComponentId) -> bool { + self.components.contains_key(&id) + } + + /// Type-erased equivalent of [`Components::component_valid_id()`]. + #[inline] + pub fn get_valid_id(&self, type_id: TypeId) -> Option { + self.indices.get(&type_id).copied() + } + + /// Returns the [`ComponentId`] of the given [`Component`] type `T` if it is fully registered. + /// If you want to include queued registration, see [`Components::component_id()`]. + /// + /// ``` + /// use bevy_ecs::prelude::*; + /// + /// let mut world = World::new(); + /// + /// #[derive(Component)] + /// struct ComponentA; + /// + /// let component_a_id = world.register_component::(); + /// + /// assert_eq!(component_a_id, world.components().valid_component_id::().unwrap()) + /// ``` + /// + /// # See also + /// + /// * [`Components::get_valid_id()`] + /// * [`Components::valid_resource_id()`] + /// * [`World::component_id()`] + #[inline] + pub fn valid_component_id(&self) -> Option { + self.get_id(TypeId::of::()) + } + + /// Type-erased equivalent of [`Components::resource_valid_id()`]. + #[inline] + pub fn get_valid_resource_id(&self, type_id: TypeId) -> Option { + self.resource_indices.get(&type_id).copied() + } + + /// Returns the [`ComponentId`] of the given [`Resource`] type `T` if it is fully registered. + /// If you want to include queued registration, see [`Components::resource_id()`]. + /// + /// ``` + /// use bevy_ecs::prelude::*; + /// + /// let mut world = World::new(); + /// + /// #[derive(Resource, Default)] + /// struct ResourceA; + /// + /// let resource_a_id = world.init_resource::(); + /// + /// assert_eq!(resource_a_id, world.components().valid_resource_id::().unwrap()) + /// ``` + /// + /// # See also + /// + /// * [`Components::valid_component_id()`] + /// * [`Components::get_resource_id()`] + #[inline] + pub fn valid_resource_id(&self) -> Option { + self.get_resource_id(TypeId::of::()) + } + /// Type-erased equivalent of [`Components::component_id()`]. #[inline] pub fn get_id(&self, type_id: TypeId) -> Option { From dddd24c8b7a7d954e0ef5a4bba3bfb653e17967d Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 6 Mar 2025 17:29:54 -0500 Subject: [PATCH 21/22] fixed docs --- crates/bevy_ecs/src/component.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 72c05300454a6..e4a8ed0f971b3 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -2173,7 +2173,7 @@ impl Components { self.components.contains_key(&id) } - /// Type-erased equivalent of [`Components::component_valid_id()`]. + /// Type-erased equivalent of [`Components::valid_component_id()`]. #[inline] pub fn get_valid_id(&self, type_id: TypeId) -> Option { self.indices.get(&type_id).copied() @@ -2205,7 +2205,7 @@ impl Components { self.get_id(TypeId::of::()) } - /// Type-erased equivalent of [`Components::resource_valid_id()`]. + /// Type-erased equivalent of [`Components::valid_resource_id()`]. #[inline] pub fn get_valid_resource_id(&self, type_id: TypeId) -> Option { self.resource_indices.get(&type_id).copied() From 2d5baaa33d1ae11f3ac4bfad38b2c1cc4579b140 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 6 Mar 2025 22:39:14 -0500 Subject: [PATCH 22/22] change component storage back to vec --- crates/bevy_ecs/src/component.rs | 47 ++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index e4a8ed0f971b3..a206f13a2d52b 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1654,7 +1654,9 @@ impl<'w> ComponentsRegistrator<'w> { &mut self .components .components - .get_mut(&id) + .get_mut(id.0) + .debug_checked_unwrap() + .as_mut() .debug_checked_unwrap() }; @@ -1829,7 +1831,7 @@ impl<'w> ComponentsRegistrator<'w> { /// Stores metadata associated with each kind of [`Component`] in a given [`World`]. #[derive(Debug, Default)] pub struct Components { - components: HashMap, + components: Vec>, indices: TypeIdMap, resource_indices: TypeIdMap, // This is kept internal and local to verify that no deadlocks can occor. @@ -1849,8 +1851,15 @@ impl Components { descriptor: ComponentDescriptor, ) { let info = ComponentInfo::new(id, descriptor); - let prev = self.components.insert(id, info); - debug_assert!(prev.is_none()); + let least_len = id.0 + 1; + if self.components.len() < least_len { + self.components.resize_with(least_len, || None); + } + // SAFETY: We just extended the vec to make this index valid. + let slot = unsafe { self.components.get_mut(id.0).debug_checked_unwrap() }; + // Caller ensures id is unique + debug_assert!(slot.is_none()); + *slot = Some(info); } /// Returns the number of components registered or queued with this instance. @@ -1912,7 +1921,7 @@ impl Components { /// This will return an incorrect result if `id` did not come from the same world as `self`. It may return `None` or a garbage value. #[inline] pub fn get_info(&self, id: ComponentId) -> Option<&ComponentInfo> { - self.components.get(&id) + self.components.get(id.0).and_then(|info| info.as_ref()) } /// Returns the name associated with the given component, if it is registered. @@ -1931,12 +1940,20 @@ impl Components { #[inline] pub unsafe fn get_info_unchecked(&self, id: ComponentId) -> &ComponentInfo { // SAFETY: The caller ensures `id` is valid. - unsafe { self.get_info(id).debug_checked_unwrap() } + unsafe { + self.components + .get(id.0) + .debug_checked_unwrap() + .as_ref() + .debug_checked_unwrap() + } } #[inline] pub(crate) fn get_hooks_mut(&mut self, id: ComponentId) -> Option<&mut ComponentHooks> { - self.components.get_mut(&id).map(|info| &mut info.hooks) + self.components + .get_mut(id.0) + .and_then(|info| info.as_mut().map(|info| &mut info.hooks)) } #[inline] @@ -1945,8 +1962,8 @@ impl Components { id: ComponentId, ) -> Option<&mut RequiredComponents> { self.components - .get_mut(&id) - .map(|info| &mut info.required_components) + .get_mut(id.0) + .and_then(|info| info.as_mut().map(|info| &mut info.required_components)) } /// Registers the given component `R` and [required components] inherited from it as required by `T`. @@ -2152,7 +2169,9 @@ impl Components { #[inline] pub(crate) fn get_required_by(&self, id: ComponentId) -> Option<&HashSet> { - self.components.get(&id).map(|info| &info.required_by) + self.components + .get(id.0) + .and_then(|info| info.as_ref().map(|info| &info.required_by)) } #[inline] @@ -2161,8 +2180,8 @@ impl Components { id: ComponentId, ) -> Option<&mut HashSet> { self.components - .get_mut(&id) - .map(|info| &mut info.required_by) + .get_mut(id.0) + .and_then(|info| info.as_mut().map(|info| &mut info.required_by)) } /// Returns true if the [`ComponentId`] is fully registered and valid. @@ -2170,7 +2189,7 @@ impl Components { /// Those ids are still correct, but they are not usable in every context yet. #[inline] pub fn is_id_valid(&self, id: ComponentId) -> bool { - self.components.contains_key(&id) + self.components.get(id.0).is_some_and(Option::is_some) } /// Type-erased equivalent of [`Components::valid_component_id()`]. @@ -2347,7 +2366,7 @@ impl Components { /// Gets an iterator over all components fully registered with this instance. pub fn iter_registered(&self) -> impl Iterator + '_ { - self.components.values() + self.components.iter().filter_map(Option::as_ref) } }