From 646c0d1ca6c32eba1a84913f53483e2f9ef0c0d3 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 16:10:29 +0200 Subject: [PATCH 01/44] Introduce StaticBundle --- crates/bevy_ecs/macros/src/lib.rs | 11 +++--- crates/bevy_ecs/src/bundle.rs | 57 +++++++++++++++++++++++-------- crates/bevy_ecs/src/lib.rs | 22 +++++++----- crates/bevy_ecs/src/spawn.rs | 23 ++++++++----- 4 files changed, 78 insertions(+), 35 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 114aff642b58c..eeba9da060b7c 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -88,13 +88,13 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { match field_kind { BundleFieldKind::Component => { field_component_ids.push(quote! { - <#field_type as #ecs_path::bundle::Bundle>::component_ids(components, &mut *ids); + <#field_type as #ecs_path::bundle::StaticBundle>::component_ids(components, &mut *ids); }); field_required_components.push(quote! { - <#field_type as #ecs_path::bundle::Bundle>::register_required_components(components, required_components); + <#field_type as #ecs_path::bundle::StaticBundle>::register_required_components(components, required_components); }); field_get_component_ids.push(quote! { - <#field_type as #ecs_path::bundle::Bundle>::get_component_ids(components, &mut *ids); + <#field_type as #ecs_path::bundle::StaticBundle>::get_component_ids(components, &mut *ids); }); match field { Some(field) => { @@ -134,7 +134,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { // - `Bundle::get_components` is exactly once for each member. Rely's on the Component -> Bundle implementation to properly pass // the correct `StorageType` into the callback. #[allow(deprecated)] - unsafe impl #impl_generics #ecs_path::bundle::Bundle for #struct_name #ty_generics #where_clause { + unsafe impl #impl_generics #ecs_path::bundle::StaticBundle for #struct_name #ty_generics #where_clause { fn component_ids( components: &mut #ecs_path::component::ComponentsRegistrator, ids: &mut impl FnMut(#ecs_path::component::ComponentId) @@ -157,6 +157,9 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { } } + // SAFETY: see the corresponding implementation of `StaticBundle` + unsafe impl #impl_generics #ecs_path::bundle::Bundle for #struct_name #ty_generics #where_clause {} + // SAFETY: // - ComponentId is returned in field-definition-order. [from_components] uses field-definition-order #[allow(deprecated)] diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index f0641ff80c859..3c5a6e3f065c8 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -141,25 +141,47 @@ use variadics_please::all_tuples; /// If you want a type to implement [`Bundle`], you must use [`derive@Bundle`](derive@Bundle). /// /// [`Query`]: crate::system::Query +#[diagnostic::on_unimplemented( + message = "`{Self}` is not a `Bundle`", + label = "invalid `Bundle`", + note = "consider annotating `{Self}` with `#[derive(Component)]` or `#[derive(Bundle)]`" +)] +pub unsafe trait Bundle: StaticBundle + DynamicBundle + Send + Sync + 'static {} + +/// Each `StaticBundle` represents a static set of [`Component`] types. +/// Currently, bundles can only contain one of each [`Component`], and will +/// panic once initialized if this is not met. +/// +/// Implementers of the `StaticBundle` trait are called 'static bundles'. +/// +/// See also the [`Bundle`] trait. +/// +/// # Safety +/// +/// Manual implementations of this trait are unsupported. +/// That is, there is no safe way to implement this trait, and you must not do so. +/// If you want a type to implement [`StaticBundle`], you must use [`derive@Bundle`](derive@Bundle). // Some safety points: // - [`Bundle::component_ids`] must return the [`ComponentId`] for each component type in the // bundle, in the _exact_ order that [`DynamicBundle::get_components`] is called. // - [`Bundle::from_components`] must call `func` exactly once for each [`ComponentId`] returned by // [`Bundle::component_ids`]. #[diagnostic::on_unimplemented( - message = "`{Self}` is not a `Bundle`", - label = "invalid `Bundle`", + message = "`{Self}` is not a `StaticBundle`", + label = "invalid `StaticBundle`", note = "consider annotating `{Self}` with `#[derive(Component)]` or `#[derive(Bundle)]`" )] -pub unsafe trait Bundle: DynamicBundle + Send + Sync + 'static { - /// Gets this [`Bundle`]'s component ids, in the order of this bundle's [`Component`]s +pub unsafe trait StaticBundle: Send + Sync + 'static { + /// Gets this [`StaticBundle`]'s component ids, in the order of this bundle's [`Component`]s #[doc(hidden)] 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. + /// Gets this [`StaticBundle`]'s component ids. This will be [`None`] if the component has not been registered. + #[doc(hidden)] fn get_component_ids(components: &Components, ids: &mut impl FnMut(Option)); - /// Registers components that are required by the components in this [`Bundle`]. + /// Registers components that are required by the components in this [`StaticBundle`]. + #[doc(hidden)] fn register_required_components( _components: &mut ComponentsRegistrator, _required_components: &mut RequiredComponents, @@ -225,7 +247,7 @@ pub trait BundleEffect { // SAFETY: // - `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 { +unsafe impl StaticBundle for C { fn component_ids(components: &mut ComponentsRegistrator, ids: &mut impl FnMut(ComponentId)) { ids(components.register_component::()); } @@ -249,6 +271,9 @@ unsafe impl Bundle for C { } } +// SAFETY: see the corresponding implementation of `StaticBundle` +unsafe impl Bundle for C {} + // SAFETY: // - `Bundle::from_components` calls `func` exactly once for C, which is the exact value returned by `Bundle::component_ids`. unsafe impl BundleFromComponents for C { @@ -285,28 +310,32 @@ macro_rules! tuple_impl { )] $(#[$meta])* // SAFETY: - // - `Bundle::component_ids` calls `ids` for each component type in the + // - `StaticBundle::component_ids` calls `ids` for each component type in the // bundle, in the exact order that `DynamicBundle::get_components` is called. - // - `Bundle::from_components` calls `func` exactly once for each `ComponentId` returned by `Bundle::component_ids`. - // - `Bundle::get_components` is called exactly once for each member. Relies on the above implementation to pass the correct + // - `StaticBundle::from_components` calls `func` exactly once for each `ComponentId` returned by `Bundle::component_ids`. + // - `StaticBundle::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,)*) { + unsafe impl<$($name: StaticBundle),*> StaticBundle for ($($name,)*) { fn component_ids(components: &mut ComponentsRegistrator, ids: &mut impl FnMut(ComponentId)){ - $(<$name as Bundle>::component_ids(components, ids);)* + $(<$name as StaticBundle>::component_ids(components, ids);)* } fn get_component_ids(components: &Components, ids: &mut impl FnMut(Option)){ - $(<$name as Bundle>::get_component_ids(components, ids);)* + $(<$name as StaticBundle>::get_component_ids(components, ids);)* } fn register_required_components( components: &mut ComponentsRegistrator, required_components: &mut RequiredComponents, ) { - $(<$name as Bundle>::register_required_components(components, required_components);)* + $(<$name as StaticBundle>::register_required_components(components, required_components);)* } } + $(#[$meta])* + // SAFETY: see the corresponding implementation of `StaticBundle` + unsafe impl<$($name: Bundle),*> Bundle for ($($name,)*) {} + #[expect( clippy::allow_attributes, reason = "This is a tuple-related macro; as such, the lints below may not always apply." diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 7dfb60292cc15..3692418d7a0d5 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -136,7 +136,7 @@ pub struct HotPatched; #[cfg(test)] mod tests { use crate::{ - bundle::Bundle, + bundle::{Bundle, StaticBundle}, change_detection::Ref, component::{Component, ComponentId, RequiredComponents, RequiredComponentsError}, entity::{Entity, EntityMapper}, @@ -233,9 +233,12 @@ mod tests { y: SparseStored, } let mut ids = Vec::new(); - ::component_ids(&mut world.components_registrator(), &mut |id| { - ids.push(id); - }); + ::component_ids( + &mut world.components_registrator(), + &mut |id| { + ids.push(id); + }, + ); assert_eq!( ids, @@ -283,9 +286,12 @@ mod tests { } let mut ids = Vec::new(); - ::component_ids(&mut world.components_registrator(), &mut |id| { - ids.push(id); - }); + ::component_ids( + &mut world.components_registrator(), + &mut |id| { + ids.push(id); + }, + ); assert_eq!( ids, @@ -335,7 +341,7 @@ mod tests { } let mut ids = Vec::new(); - ::component_ids( + ::component_ids( &mut world.components_registrator(), &mut |id| { ids.push(id); diff --git a/crates/bevy_ecs/src/spawn.rs b/crates/bevy_ecs/src/spawn.rs index d5014f22409fd..68e766ac9d8b7 100644 --- a/crates/bevy_ecs/src/spawn.rs +++ b/crates/bevy_ecs/src/spawn.rs @@ -2,7 +2,7 @@ //! for the best entry points into these APIs and examples of how to use them. use crate::{ - bundle::{Bundle, BundleEffect, DynamicBundle, NoBundleEffect}, + bundle::{Bundle, BundleEffect, DynamicBundle, NoBundleEffect, StaticBundle}, entity::Entity, relationship::{RelatedSpawner, Relationship, RelationshipTarget}, world::{EntityWorldMut, World}, @@ -183,33 +183,37 @@ impl> BundleEffect for SpawnRelatedBundle + Send + Sync + 'static> Bundle +unsafe impl + Send + Sync + 'static> StaticBundle for SpawnRelatedBundle { fn component_ids( components: &mut crate::component::ComponentsRegistrator, ids: &mut impl FnMut(crate::component::ComponentId), ) { - ::component_ids(components, ids); + ::component_ids(components, ids); } fn get_component_ids( components: &crate::component::Components, ids: &mut impl FnMut(Option), ) { - ::get_component_ids(components, ids); + ::get_component_ids(components, ids); } fn register_required_components( components: &mut crate::component::ComponentsRegistrator, required_components: &mut crate::component::RequiredComponents, ) { - ::register_required_components( + ::register_required_components( components, required_components, ); } } +unsafe impl + Send + Sync + 'static> Bundle + for SpawnRelatedBundle +{ +} impl> DynamicBundle for SpawnRelatedBundle { type Effect = Self; @@ -252,31 +256,32 @@ impl DynamicBundle for SpawnOneRelated { } // SAFETY: This internally relies on the RelationshipTarget's Bundle implementation, which is sound. -unsafe impl Bundle for SpawnOneRelated { +unsafe impl StaticBundle for SpawnOneRelated { fn component_ids( components: &mut crate::component::ComponentsRegistrator, ids: &mut impl FnMut(crate::component::ComponentId), ) { - ::component_ids(components, ids); + ::component_ids(components, ids); } fn get_component_ids( components: &crate::component::Components, ids: &mut impl FnMut(Option), ) { - ::get_component_ids(components, ids); + ::get_component_ids(components, ids); } fn register_required_components( components: &mut crate::component::ComponentsRegistrator, required_components: &mut crate::component::RequiredComponents, ) { - ::register_required_components( + ::register_required_components( components, required_components, ); } } +unsafe impl Bundle for SpawnOneRelated {} /// [`RelationshipTarget`] methods that create a [`Bundle`] with a [`DynamicBundle::Effect`] that: /// From bc7628306d45e0db23dbcc2e2ece614d1f06918b Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 16:16:53 +0200 Subject: [PATCH 02/44] Switch register_bundle to StaticBundle --- crates/bevy_ecs/src/bundle.rs | 2 +- crates/bevy_ecs/src/world/mod.rs | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 3c5a6e3f065c8..d185e24ce6c67 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1918,7 +1918,7 @@ impl Bundles { /// Registers a new [`BundleInfo`] for a statically known type. /// /// Also registers all the components in the bundle. - pub(crate) fn register_info( + pub(crate) fn register_info( &mut self, components: &mut ComponentsRegistrator, storages: &mut Storages, diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index b79f189963a84..a24dc915e58b8 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -14,7 +14,10 @@ pub mod unsafe_world_cell; #[cfg(feature = "bevy_reflect")] pub mod reflect; -use crate::error::{DefaultErrorHandler, ErrorHandler}; +use crate::{ + bundle::StaticBundle, + error::{DefaultErrorHandler, ErrorHandler}, +}; pub use crate::{ change_detection::{Mut, Ref, CHECK_TICK_THRESHOLD}, world::command_queue::CommandQueue, @@ -3000,7 +3003,7 @@ impl World { /// This is largely equivalent to calling [`register_component`](Self::register_component) on each /// component in the bundle. #[inline] - pub fn register_bundle(&mut self) -> &BundleInfo { + pub fn register_bundle(&mut self) -> &BundleInfo { // 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) }; From 50a8670274dd30be4bc5fdcbceaad6101e74d688 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 16:14:21 +0200 Subject: [PATCH 03/44] Switch Entity{Ref,Mut}Except to StaticBundle --- crates/bevy_ecs/src/query/fetch.rs | 12 +++--- crates/bevy_ecs/src/query/iter.rs | 6 +-- crates/bevy_ecs/src/world/entity_ref.rs | 50 ++++++++++++------------- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index cffba8cda1689..ecf4041a98d07 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1,6 +1,6 @@ use crate::{ archetype::{Archetype, Archetypes}, - bundle::Bundle, + bundle::StaticBundle, change_detection::{MaybeLocation, Ticks, TicksMut}, component::{Component, ComponentId, Components, Mutable, StorageType, Tick}, entity::{Entities, Entity, EntityLocation}, @@ -1080,7 +1080,7 @@ unsafe impl<'a> QueryData for FilteredEntityMut<'a> { /// are rejected. unsafe impl<'a, B> WorldQuery for EntityRefExcept<'a, B> where - B: Bundle, + B: StaticBundle, { type Fetch<'w> = EntityFetch<'w>; type State = SmallVec<[ComponentId; 4]>; @@ -1155,7 +1155,7 @@ where /// SAFETY: `Self` is the same as `Self::ReadOnly`. unsafe impl<'a, B> QueryData for EntityRefExcept<'a, B> where - B: Bundle, + B: StaticBundle, { const IS_READ_ONLY: bool = true; type ReadOnly = Self; @@ -1180,14 +1180,14 @@ where /// SAFETY: `EntityRefExcept` enforces read-only access to its contained /// components. -unsafe impl<'a, B> ReadOnlyQueryData for EntityRefExcept<'a, B> where B: Bundle {} +unsafe impl<'a, B> ReadOnlyQueryData for EntityRefExcept<'a, B> where B: StaticBundle {} /// SAFETY: `EntityMutExcept` guards access to all components in the bundle `B` /// and populates `Access` values so that queries that conflict with this access /// are rejected. unsafe impl<'a, B> WorldQuery for EntityMutExcept<'a, B> where - B: Bundle, + B: StaticBundle, { type Fetch<'w> = EntityFetch<'w>; type State = SmallVec<[ComponentId; 4]>; @@ -1263,7 +1263,7 @@ where /// `EntityMutExcept` provides. unsafe impl<'a, B> QueryData for EntityMutExcept<'a, B> where - B: Bundle, + B: StaticBundle, { const IS_READ_ONLY: bool = false; type ReadOnly = EntityRefExcept<'a, B>; diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index baf0d7269715d..acbd96dfb795a 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -1,7 +1,7 @@ use super::{QueryData, QueryFilter, ReadOnlyQueryData}; use crate::{ archetype::{Archetype, ArchetypeEntity, Archetypes}, - bundle::Bundle, + bundle::StaticBundle, component::Tick, entity::{ContainsEntity, Entities, Entity, EntityEquivalent, EntitySet, EntitySetIterator}, query::{ArchetypeFilter, DebugCheckedUnwrap, QueryState, StorageId}, @@ -939,13 +939,13 @@ unsafe impl<'w, 's, F: QueryFilter> EntitySetIterator } // SAFETY: [`QueryIter`] is guaranteed to return every matching entity once and only once. -unsafe impl<'w, 's, F: QueryFilter, B: Bundle> EntitySetIterator +unsafe impl<'w, 's, F: QueryFilter, B: StaticBundle> EntitySetIterator for QueryIter<'w, 's, EntityRefExcept<'_, B>, F> { } // SAFETY: [`QueryIter`] is guaranteed to return every matching entity once and only once. -unsafe impl<'w, 's, F: QueryFilter, B: Bundle> EntitySetIterator +unsafe impl<'w, 's, F: QueryFilter, B: StaticBundle> EntitySetIterator for QueryIter<'w, 's, EntityMutExcept<'_, B>, F> { } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 9842ee54e365d..3102989c14bc5 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -2,7 +2,7 @@ use crate::{ archetype::Archetype, bundle::{ Bundle, BundleEffect, BundleFromComponents, BundleInserter, BundleRemover, DynamicBundle, - InsertMode, + InsertMode, StaticBundle, }, change_detection::{MaybeLocation, MutUntyped}, component::{ @@ -3446,7 +3446,7 @@ impl<'a> From<&'a EntityWorldMut<'_>> for FilteredEntityRef<'a> { } } -impl<'a, B: Bundle> From<&'a EntityRefExcept<'_, B>> for FilteredEntityRef<'a> { +impl<'a, B: StaticBundle> From<&'a EntityRefExcept<'_, B>> for FilteredEntityRef<'a> { fn from(value: &'a EntityRefExcept<'_, B>) -> Self { // SAFETY: // - The FilteredEntityRef has the same component access as the given EntityRefExcept. @@ -3786,7 +3786,7 @@ impl<'a> From<&'a mut EntityWorldMut<'_>> for FilteredEntityMut<'a> { } } -impl<'a, B: Bundle> From<&'a EntityMutExcept<'_, B>> for FilteredEntityMut<'a> { +impl<'a, B: StaticBundle> From<&'a EntityMutExcept<'_, B>> for FilteredEntityMut<'a> { fn from(value: &'a EntityMutExcept<'_, B>) -> Self { // SAFETY: // - The FilteredEntityMut has the same component access as the given EntityMutExcept. @@ -3860,7 +3860,7 @@ pub enum TryFromFilteredError { /// for an explicitly-enumerated set. pub struct EntityRefExcept<'w, B> where - B: Bundle, + B: StaticBundle, { entity: UnsafeEntityCell<'w>, phantom: PhantomData, @@ -3868,7 +3868,7 @@ where impl<'w, B> EntityRefExcept<'w, B> where - B: Bundle, + B: StaticBundle, { /// # Safety /// Other users of `UnsafeEntityCell` must only have mutable access to the components in `B`. @@ -4025,7 +4025,7 @@ where impl<'a, B> From<&'a EntityMutExcept<'_, B>> for EntityRefExcept<'a, B> where - B: Bundle, + B: StaticBundle, { fn from(entity: &'a EntityMutExcept<'_, B>) -> Self { // SAFETY: All accesses that `EntityRefExcept` provides are also @@ -4034,23 +4034,23 @@ where } } -impl Clone for EntityRefExcept<'_, B> { +impl Clone for EntityRefExcept<'_, B> { fn clone(&self) -> Self { *self } } -impl Copy for EntityRefExcept<'_, B> {} +impl Copy for EntityRefExcept<'_, B> {} -impl PartialEq for EntityRefExcept<'_, B> { +impl PartialEq for EntityRefExcept<'_, B> { fn eq(&self, other: &Self) -> bool { self.entity() == other.entity() } } -impl Eq for EntityRefExcept<'_, B> {} +impl Eq for EntityRefExcept<'_, B> {} -impl PartialOrd for EntityRefExcept<'_, B> { +impl PartialOrd for EntityRefExcept<'_, B> { /// [`EntityRefExcept`]'s comparison trait implementations match the underlying [`Entity`], /// and cannot discern between different worlds. fn partial_cmp(&self, other: &Self) -> Option { @@ -4058,26 +4058,26 @@ impl PartialOrd for EntityRefExcept<'_, B> { } } -impl Ord for EntityRefExcept<'_, B> { +impl Ord for EntityRefExcept<'_, B> { fn cmp(&self, other: &Self) -> Ordering { self.entity().cmp(&other.entity()) } } -impl Hash for EntityRefExcept<'_, B> { +impl Hash for EntityRefExcept<'_, B> { fn hash(&self, state: &mut H) { self.entity().hash(state); } } -impl ContainsEntity for EntityRefExcept<'_, B> { +impl ContainsEntity for EntityRefExcept<'_, B> { fn entity(&self) -> Entity { self.id() } } // SAFETY: This type represents one Entity. We implement the comparison traits based on that Entity. -unsafe impl EntityEquivalent for EntityRefExcept<'_, B> {} +unsafe impl EntityEquivalent for EntityRefExcept<'_, B> {} /// Provides mutable access to all components of an entity, with the exception /// of an explicit set. @@ -4089,7 +4089,7 @@ unsafe impl EntityEquivalent for EntityRefExcept<'_, B> {} /// [`crate::query::Without`] filter. pub struct EntityMutExcept<'w, B> where - B: Bundle, + B: StaticBundle, { entity: UnsafeEntityCell<'w>, phantom: PhantomData, @@ -4097,7 +4097,7 @@ where impl<'w, B> EntityMutExcept<'w, B> where - B: Bundle, + B: StaticBundle, { /// # Safety /// Other users of `UnsafeEntityCell` must not have access to any components not in `B`. @@ -4257,15 +4257,15 @@ where } } -impl PartialEq for EntityMutExcept<'_, B> { +impl PartialEq for EntityMutExcept<'_, B> { fn eq(&self, other: &Self) -> bool { self.entity() == other.entity() } } -impl Eq for EntityMutExcept<'_, B> {} +impl Eq for EntityMutExcept<'_, B> {} -impl PartialOrd for EntityMutExcept<'_, B> { +impl PartialOrd for EntityMutExcept<'_, B> { /// [`EntityMutExcept`]'s comparison trait implementations match the underlying [`Entity`], /// and cannot discern between different worlds. fn partial_cmp(&self, other: &Self) -> Option { @@ -4273,30 +4273,30 @@ impl PartialOrd for EntityMutExcept<'_, B> { } } -impl Ord for EntityMutExcept<'_, B> { +impl Ord for EntityMutExcept<'_, B> { fn cmp(&self, other: &Self) -> Ordering { self.entity().cmp(&other.entity()) } } -impl Hash for EntityMutExcept<'_, B> { +impl Hash for EntityMutExcept<'_, B> { fn hash(&self, state: &mut H) { self.entity().hash(state); } } -impl ContainsEntity for EntityMutExcept<'_, B> { +impl ContainsEntity for EntityMutExcept<'_, B> { fn entity(&self) -> Entity { self.id() } } // SAFETY: This type represents one Entity. We implement the comparison traits based on that Entity. -unsafe impl EntityEquivalent for EntityMutExcept<'_, B> {} +unsafe impl EntityEquivalent for EntityMutExcept<'_, B> {} fn bundle_contains_component(components: &Components, query_id: ComponentId) -> bool where - B: Bundle, + B: StaticBundle, { let mut found = false; B::get_component_ids(components, &mut |maybe_id| { From 762f525e09250e2b209f3b51819cf04f8c9bb7ef Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 16:18:04 +0200 Subject: [PATCH 04/44] Switch EntityClonerBuilder to StaticBundle --- crates/bevy_ecs/src/entity/clone_entities.rs | 6 +++--- crates/bevy_ecs/src/system/commands/entity_command.rs | 4 ++-- crates/bevy_ecs/src/system/commands/mod.rs | 4 ++-- crates/bevy_ecs/src/world/entity_ref.rs | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/entity/clone_entities.rs b/crates/bevy_ecs/src/entity/clone_entities.rs index a7a1f84403218..fbcd10927dd0e 100644 --- a/crates/bevy_ecs/src/entity/clone_entities.rs +++ b/crates/bevy_ecs/src/entity/clone_entities.rs @@ -6,7 +6,7 @@ use core::any::TypeId; use crate::{ archetype::Archetype, - bundle::Bundle, + bundle::StaticBundle, component::{Component, ComponentCloneBehavior, ComponentCloneFn, ComponentId, ComponentInfo}, entity::{hash_map::EntityHashMap, Entities, Entity, EntityMapper}, query::DebugCheckedUnwrap, @@ -679,7 +679,7 @@ impl<'w> EntityClonerBuilder<'w> { /// /// Note that all components are allowed by default, to clone only explicitly allowed components make sure to call /// [`deny_all`](`Self::deny_all`) before calling any of the `allow` methods. - pub fn allow(&mut self) -> &mut Self { + pub fn allow(&mut self) -> &mut Self { let bundle = self.world.register_bundle::(); let ids = bundle.explicit_components().to_owned(); for id in ids { @@ -720,7 +720,7 @@ impl<'w> EntityClonerBuilder<'w> { } /// Disallows all components of the bundle from being cloned. - pub fn deny(&mut self) -> &mut Self { + pub fn deny(&mut self) -> &mut Self { let bundle = self.world.register_bundle::(); let ids = bundle.explicit_components().to_owned(); for id in ids { diff --git a/crates/bevy_ecs/src/system/commands/entity_command.rs b/crates/bevy_ecs/src/system/commands/entity_command.rs index 317ad8476abe8..3ea3204cfcd69 100644 --- a/crates/bevy_ecs/src/system/commands/entity_command.rs +++ b/crates/bevy_ecs/src/system/commands/entity_command.rs @@ -254,7 +254,7 @@ pub fn clone_with( /// An [`EntityCommand`] that clones the specified components of an entity /// and inserts them into another entity. -pub fn clone_components(target: Entity) -> impl EntityCommand { +pub fn clone_components(target: Entity) -> impl EntityCommand { move |mut entity: EntityWorldMut| { entity.clone_components::(target); } @@ -262,7 +262,7 @@ pub fn clone_components(target: Entity) -> impl EntityCommand { /// An [`EntityCommand`] that clones the specified components of an entity /// and inserts them into another entity, then removes them from the original entity. -pub fn move_components(target: Entity) -> impl EntityCommand { +pub fn move_components(target: Entity) -> impl EntityCommand { move |mut entity: EntityWorldMut| { entity.move_components::(target); } diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 3012a654586e6..160a2da85deb6 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -2097,7 +2097,7 @@ impl<'a> EntityCommands<'a> { /// # Panics /// /// The command will panic when applied if the target entity does not exist. - pub fn clone_components(&mut self, target: Entity) -> &mut Self { + pub fn clone_components(&mut self, target: Entity) -> &mut Self { self.queue(entity_command::clone_components::(target)) } @@ -2110,7 +2110,7 @@ impl<'a> EntityCommands<'a> { /// # Panics /// /// The command will panic when applied if the target entity does not exist. - pub fn move_components(&mut self, target: Entity) -> &mut Self { + pub fn move_components(&mut self, target: Entity) -> &mut Self { self.queue(entity_command::move_components::(target)) } } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 3102989c14bc5..d0ec0e1958288 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -2770,7 +2770,7 @@ impl<'w> EntityWorldMut<'w> { /// /// - If this entity has been despawned while this `EntityWorldMut` is still alive. /// - If the target entity does not exist. - pub fn clone_components(&mut self, target: Entity) -> &mut Self { + pub fn clone_components(&mut self, target: Entity) -> &mut Self { self.assert_not_despawned(); EntityCloner::build(self.world) @@ -2793,7 +2793,7 @@ impl<'w> EntityWorldMut<'w> { /// /// - If this entity has been despawned while this `EntityWorldMut` is still alive. /// - If the target entity does not exist. - pub fn move_components(&mut self, target: Entity) -> &mut Self { + pub fn move_components(&mut self, target: Entity) -> &mut Self { self.assert_not_despawned(); EntityCloner::build(self.world) From c482ee0b21058d24aa176ceb914db111f5a39399 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 16:24:28 +0200 Subject: [PATCH 05/44] Switch Observer and Trigger to StaticBundle --- crates/bevy_app/src/app.rs | 3 ++- crates/bevy_ecs/src/observer/mod.rs | 13 +++++++------ crates/bevy_ecs/src/observer/runner.rs | 7 ++++--- .../src/system/commands/entity_command.rs | 2 +- crates/bevy_ecs/src/system/commands/mod.rs | 4 ++-- crates/bevy_ecs/src/system/input.rs | 4 ++-- crates/bevy_ecs/src/system/observer_system.rs | 17 +++++++++-------- crates/bevy_ecs/src/world/entity_ref.rs | 4 ++-- 8 files changed, 29 insertions(+), 25 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 61bbad3aedbd6..1b2c5751642a9 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -9,6 +9,7 @@ use alloc::{ }; pub use bevy_derive::AppLabel; use bevy_ecs::{ + bundle::StaticBundle, component::RequiredComponentsError, error::{DefaultErrorHandler, ErrorHandler}, event::{event_update_system, EventCursor}, @@ -1337,7 +1338,7 @@ impl App { /// } /// }); /// ``` - pub fn add_observer( + pub fn add_observer( &mut self, observer: impl IntoObserverSystem, ) -> &mut Self { diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index 767dc7ec95d37..525397f5610f4 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -9,6 +9,7 @@ use variadics_please::all_tuples; use crate::{ archetype::ArchetypeFlags, + bundle::StaticBundle, change_detection::MaybeLocation, component::ComponentId, entity::EntityHashMap, @@ -29,14 +30,14 @@ use smallvec::SmallVec; /// Type containing triggered [`Event`] information for a given run of an [`Observer`]. This contains the /// [`Event`] data itself. If it was triggered for a specific [`Entity`], it includes that as well. It also /// contains event propagation information. See [`Trigger::propagate`] for more information. -pub struct Trigger<'w, E, B: Bundle = ()> { +pub struct Trigger<'w, E, B: StaticBundle = ()> { event: &'w mut E, propagate: &'w mut bool, trigger: ObserverTrigger, _marker: PhantomData, } -impl<'w, E, B: Bundle> Trigger<'w, E, B> { +impl<'w, E, B: StaticBundle> Trigger<'w, E, B> { /// Creates a new trigger for the given event and observer information. pub fn new(event: &'w mut E, propagate: &'w mut bool, trigger: ObserverTrigger) -> Self { Self { @@ -147,7 +148,7 @@ impl<'w, E, B: Bundle> Trigger<'w, E, B> { } } -impl<'w, E: Debug, B: Bundle> Debug for Trigger<'w, E, B> { +impl<'w, E: Debug, B: StaticBundle> Debug for Trigger<'w, E, B> { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { f.debug_struct("Trigger") .field("event", &self.event) @@ -158,7 +159,7 @@ impl<'w, E: Debug, B: Bundle> Debug for Trigger<'w, E, B> { } } -impl<'w, E, B: Bundle> Deref for Trigger<'w, E, B> { +impl<'w, E, B: StaticBundle> Deref for Trigger<'w, E, B> { type Target = E; fn deref(&self) -> &Self::Target { @@ -166,7 +167,7 @@ impl<'w, E, B: Bundle> Deref for Trigger<'w, E, B> { } } -impl<'w, E, B: Bundle> DerefMut for Trigger<'w, E, B> { +impl<'w, E, B: StaticBundle> DerefMut for Trigger<'w, E, B> { fn deref_mut(&mut self) -> &mut Self::Target { self.event } @@ -561,7 +562,7 @@ impl World { /// # Panics /// /// Panics if the given system is an exclusive system. - pub fn add_observer( + pub fn add_observer( &mut self, system: impl IntoObserverSystem, ) -> EntityWorldMut { diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index 43ece18ff5151..9f264fbcda295 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -2,6 +2,7 @@ use alloc::{boxed::Box, vec}; use core::any::Any; use crate::{ + bundle::StaticBundle, component::{ComponentHook, ComponentId, HookContext, Mutable, StorageType}, error::{ErrorContext, ErrorHandler}, observer::{ObserverDescriptor, ObserverTrigger}, @@ -205,7 +206,7 @@ impl Observer { /// # Panics /// /// Panics if the given system is an exclusive system. - pub fn new>(system: I) -> Self { + pub fn new>(system: I) -> Self { let system = Box::new(IntoObserverSystem::into_system(system)); assert!( !system.is_exclusive(), @@ -327,7 +328,7 @@ impl Component for Observer { } } -fn observer_system_runner>( +fn observer_system_runner>( mut world: DeferredWorld, observer_trigger: ObserverTrigger, ptr: PtrMut, @@ -418,7 +419,7 @@ fn observer_system_runner>( /// The type parameters of this function _must_ match those used to create the [`Observer`]. /// As such, it is recommended to only use this function within the [`Observer::new`] method to /// ensure type parameters match. -fn hook_on_add>( +fn hook_on_add>( mut world: DeferredWorld<'_>, HookContext { entity, .. }: HookContext, ) { diff --git a/crates/bevy_ecs/src/system/commands/entity_command.rs b/crates/bevy_ecs/src/system/commands/entity_command.rs index 3ea3204cfcd69..ab81184d8f351 100644 --- a/crates/bevy_ecs/src/system/commands/entity_command.rs +++ b/crates/bevy_ecs/src/system/commands/entity_command.rs @@ -218,7 +218,7 @@ pub fn despawn() -> impl EntityCommand { /// An [`EntityCommand`] that creates an [`Observer`](crate::observer::Observer) /// listening for events of type `E` targeting an entity #[track_caller] -pub fn observe( +pub fn observe( observer: impl IntoObserverSystem, ) -> impl EntityCommand { let caller = MaybeLocation::caller(); diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 160a2da85deb6..2cb18b1dbeebc 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1102,7 +1102,7 @@ impl<'w, 's> Commands<'w, 's> { /// Panics if the given system is an exclusive system. /// /// [`Trigger`]: crate::observer::Trigger - pub fn add_observer( + pub fn add_observer( &mut self, observer: impl IntoObserverSystem, ) -> EntityCommands { @@ -1956,7 +1956,7 @@ impl<'a> EntityCommands<'a> { } /// Creates an [`Observer`] listening for events of type `E` targeting this entity. - pub fn observe( + pub fn observe( &mut self, observer: impl IntoObserverSystem, ) -> &mut Self { diff --git a/crates/bevy_ecs/src/system/input.rs b/crates/bevy_ecs/src/system/input.rs index 12087fdf6a64a..7c3d63e5143d7 100644 --- a/crates/bevy_ecs/src/system/input.rs +++ b/crates/bevy_ecs/src/system/input.rs @@ -2,7 +2,7 @@ use core::ops::{Deref, DerefMut}; use variadics_please::all_tuples; -use crate::{bundle::Bundle, prelude::Trigger, system::System}; +use crate::{bundle::StaticBundle, prelude::Trigger, system::System}; /// Trait for types that can be used as input to [`System`]s. /// @@ -222,7 +222,7 @@ impl<'i, T: ?Sized> DerefMut for InMut<'i, T> { /// Used for [`ObserverSystem`]s. /// /// [`ObserverSystem`]: crate::system::ObserverSystem -impl SystemInput for Trigger<'_, E, B> { +impl SystemInput for Trigger<'_, E, B> { type Param<'i> = Trigger<'i, E, B>; type Inner<'i> = Trigger<'i, E, B>; diff --git a/crates/bevy_ecs/src/system/observer_system.rs b/crates/bevy_ecs/src/system/observer_system.rs index c0ac5e8de094c..5ed5706b37255 100644 --- a/crates/bevy_ecs/src/system/observer_system.rs +++ b/crates/bevy_ecs/src/system/observer_system.rs @@ -2,10 +2,11 @@ use alloc::{borrow::Cow, vec::Vec}; use core::marker::PhantomData; use crate::{ + bundle::StaticBundle, component::{ComponentId, Tick}, error::Result, never::Never, - prelude::{Bundle, Trigger}, + prelude::Trigger, query::{Access, FilteredAccessSet}, schedule::{Fallible, Infallible}, system::{input::SystemIn, System}, @@ -15,12 +16,12 @@ use crate::{ use super::{IntoSystem, SystemParamValidationError}; /// Implemented for [`System`]s that have a [`Trigger`] as the first argument. -pub trait ObserverSystem: +pub trait ObserverSystem: System, Out = Out> + Send + 'static { } -impl ObserverSystem for T where +impl ObserverSystem for T where T: System, Out = Out> + Send + 'static { } @@ -37,7 +38,7 @@ impl ObserverSystem for T where label = "the trait `IntoObserverSystem` is not implemented", note = "for function `ObserverSystem`s, ensure the first argument is a `Trigger` and any subsequent ones are `SystemParam`" )] -pub trait IntoObserverSystem: Send + 'static { +pub trait IntoObserverSystem: Send + 'static { /// The type of [`System`] that this instance converts into. type System: ObserverSystem; @@ -50,7 +51,7 @@ where S: IntoSystem, Out, M> + Send + 'static, S::System: ObserverSystem, E: 'static, - B: Bundle, + B: StaticBundle, { type System = S::System; @@ -64,7 +65,7 @@ where S: IntoSystem, (), M> + Send + 'static, S::System: ObserverSystem, E: Send + Sync + 'static, - B: Bundle, + B: StaticBundle, { type System = InfallibleObserverWrapper; @@ -76,7 +77,7 @@ impl IntoObserverSystem for S where S: IntoSystem, Never, M> + Send + 'static, E: Send + Sync + 'static, - B: Bundle, + B: StaticBundle, { type System = InfallibleObserverWrapper; @@ -105,7 +106,7 @@ impl System for InfallibleObserverWrapper where S: ObserverSystem, E: Send + Sync + 'static, - B: Bundle, + B: StaticBundle, Out: Send + Sync + 'static, { type In = Trigger<'static, E, B>; diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index d0ec0e1958288..55d3729920549 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -2636,14 +2636,14 @@ impl<'w> EntityWorldMut<'w> { /// /// Panics if the given system is an exclusive system. #[track_caller] - pub fn observe( + pub fn observe( &mut self, observer: impl IntoObserverSystem, ) -> &mut Self { self.observe_with_caller(observer, MaybeLocation::caller()) } - pub(crate) fn observe_with_caller( + pub(crate) fn observe_with_caller( &mut self, observer: impl IntoObserverSystem, caller: MaybeLocation, From 89f9530769c2421f694f6aa77a1566c473aeec1f Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 18:40:20 +0200 Subject: [PATCH 06/44] Switch component removal to StaticBundle --- crates/bevy_ecs/src/bundle.rs | 4 ++-- crates/bevy_ecs/src/relationship/related_methods.rs | 6 +++--- .../bevy_ecs/src/system/commands/entity_command.rs | 4 ++-- crates/bevy_ecs/src/system/commands/mod.rs | 13 ++++++++----- crates/bevy_ecs/src/world/entity_ref.rs | 13 ++++++++----- 5 files changed, 23 insertions(+), 17 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index d185e24ce6c67..bb10cf5ca5189 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1430,7 +1430,7 @@ impl<'w> BundleRemover<'w> { /// # Safety /// Caller must ensure that `archetype_id` is valid #[inline] - pub(crate) unsafe fn new( + pub(crate) unsafe fn new( world: &'w mut World, archetype_id: ArchetypeId, require_all: bool, @@ -1942,7 +1942,7 @@ impl Bundles { /// Registers a new [`BundleInfo`], which contains both explicit and required components for a statically known type. /// /// Also registers all the components in the bundle. - pub(crate) fn register_contributed_bundle_info( + pub(crate) fn register_contributed_bundle_info( &mut self, components: &mut ComponentsRegistrator, storages: &mut Storages, diff --git a/crates/bevy_ecs/src/relationship/related_methods.rs b/crates/bevy_ecs/src/relationship/related_methods.rs index 5a2321446348e..74ebf7b774011 100644 --- a/crates/bevy_ecs/src/relationship/related_methods.rs +++ b/crates/bevy_ecs/src/relationship/related_methods.rs @@ -1,5 +1,5 @@ use crate::{ - bundle::Bundle, + bundle::{Bundle, StaticBundle}, entity::{hash_set::EntityHashSet, Entity}, relationship::{ Relationship, RelationshipHookMode, RelationshipSourceCollection, RelationshipTarget, @@ -337,7 +337,7 @@ impl<'w> EntityWorldMut<'w> { /// /// This method should only be called on relationships that form a tree-like structure. /// Any cycles will cause this method to loop infinitely. - pub fn remove_recursive(&mut self) -> &mut Self { + pub fn remove_recursive(&mut self) -> &mut Self { self.remove::(); if let Some(relationship_target) = self.get::() { let related_vec: Vec = relationship_target.iter().collect(); @@ -490,7 +490,7 @@ impl<'a> EntityCommands<'a> { /// /// This method should only be called on relationships that form a tree-like structure. /// Any cycles will cause this method to loop infinitely. - pub fn remove_recursive(&mut self) -> &mut Self { + pub fn remove_recursive(&mut self) -> &mut Self { self.queue(move |mut entity: EntityWorldMut| { entity.remove_recursive::(); }) diff --git a/crates/bevy_ecs/src/system/commands/entity_command.rs b/crates/bevy_ecs/src/system/commands/entity_command.rs index ab81184d8f351..3836bbf86ec79 100644 --- a/crates/bevy_ecs/src/system/commands/entity_command.rs +++ b/crates/bevy_ecs/src/system/commands/entity_command.rs @@ -154,7 +154,7 @@ pub fn insert_from_world(mode: InsertMode) -> impl Ent /// An [`EntityCommand`] that removes the components in a [`Bundle`] from an entity. #[track_caller] -pub fn remove() -> impl EntityCommand { +pub fn remove() -> impl EntityCommand { let caller = MaybeLocation::caller(); move |mut entity: EntityWorldMut| { entity.remove_with_caller::(caller); @@ -164,7 +164,7 @@ pub fn remove() -> impl EntityCommand { /// An [`EntityCommand`] that removes the components in a [`Bundle`] from an entity, /// as well as the required components for each component removed. #[track_caller] -pub fn remove_with_requires() -> impl EntityCommand { +pub fn remove_with_requires() -> impl EntityCommand { let caller = MaybeLocation::caller(); move |mut entity: EntityWorldMut| { entity.remove_with_requires_with_caller::(caller); diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 2cb18b1dbeebc..f335a21c3850b 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1610,7 +1610,7 @@ impl<'a> EntityCommands<'a> { /// # bevy_ecs::system::assert_is_system(remove_combat_stats_system); /// ``` #[track_caller] - pub fn remove(&mut self) -> &mut Self { + pub fn remove(&mut self) -> &mut Self { self.queue_handled(entity_command::remove::(), warn) } @@ -1646,7 +1646,7 @@ impl<'a> EntityCommands<'a> { /// # bevy_ecs::system::assert_is_system(remove_combat_stats_system); /// ``` #[track_caller] - pub fn remove_if(&mut self, condition: impl FnOnce() -> bool) -> &mut Self { + pub fn remove_if(&mut self, condition: impl FnOnce() -> bool) -> &mut Self { if condition() { self.remove::() } else { @@ -1663,7 +1663,10 @@ impl<'a> EntityCommands<'a> { /// If the entity does not exist when this command is executed, /// the resulting error will be ignored. #[track_caller] - pub fn try_remove_if(&mut self, condition: impl FnOnce() -> bool) -> &mut Self { + pub fn try_remove_if( + &mut self, + condition: impl FnOnce() -> bool, + ) -> &mut Self { if condition() { self.try_remove::() } else { @@ -1711,7 +1714,7 @@ impl<'a> EntityCommands<'a> { /// } /// # bevy_ecs::system::assert_is_system(remove_combat_stats_system); /// ``` - pub fn try_remove(&mut self) -> &mut Self { + pub fn try_remove(&mut self) -> &mut Self { self.queue_handled(entity_command::remove::(), ignore) } @@ -1743,7 +1746,7 @@ impl<'a> EntityCommands<'a> { /// # bevy_ecs::system::assert_is_system(remove_with_requires_system); /// ``` #[track_caller] - pub fn remove_with_requires(&mut self) -> &mut Self { + pub fn remove_with_requires(&mut self) -> &mut Self { self.queue(entity_command::remove_with_requires::()) } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 55d3729920549..637201eee5e3b 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1994,7 +1994,7 @@ impl<'w> EntityWorldMut<'w> { /// If the entity has been despawned while this `EntityWorldMut` is still alive. #[must_use] #[track_caller] - pub fn take(&mut self) -> Option { + pub fn take(&mut self) -> Option { let location = self.location(); let entity = self.entity; @@ -2050,12 +2050,15 @@ impl<'w> EntityWorldMut<'w> { /// /// If the entity has been despawned while this `EntityWorldMut` is still alive. #[track_caller] - pub fn remove(&mut self) -> &mut Self { + pub fn remove(&mut self) -> &mut Self { self.remove_with_caller::(MaybeLocation::caller()) } #[inline] - pub(crate) fn remove_with_caller(&mut self, caller: MaybeLocation) -> &mut Self { + pub(crate) fn remove_with_caller( + &mut self, + caller: MaybeLocation, + ) -> &mut Self { let location = self.location(); let Some(mut remover) = @@ -2087,11 +2090,11 @@ impl<'w> EntityWorldMut<'w> { /// /// If the entity has been despawned while this `EntityWorldMut` is still alive. #[track_caller] - pub fn remove_with_requires(&mut self) -> &mut Self { + pub fn remove_with_requires(&mut self) -> &mut Self { self.remove_with_requires_with_caller::(MaybeLocation::caller()) } - pub(crate) fn remove_with_requires_with_caller( + pub(crate) fn remove_with_requires_with_caller( &mut self, caller: MaybeLocation, ) -> &mut Self { From 8d3991dbe02bf482ea0861dd318ec34e71d29d0b Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 18:44:49 +0200 Subject: [PATCH 07/44] Switch EntityWorldMut::retain to StaticBundle --- crates/bevy_ecs/src/system/commands/entity_command.rs | 6 +++--- crates/bevy_ecs/src/system/commands/mod.rs | 2 +- crates/bevy_ecs/src/world/entity_ref.rs | 7 +++++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/entity_command.rs b/crates/bevy_ecs/src/system/commands/entity_command.rs index 3836bbf86ec79..16b40b26c5adf 100644 --- a/crates/bevy_ecs/src/system/commands/entity_command.rs +++ b/crates/bevy_ecs/src/system/commands/entity_command.rs @@ -8,7 +8,7 @@ use alloc::vec::Vec; use log::info; use crate::{ - bundle::{Bundle, InsertMode}, + bundle::{Bundle, InsertMode, StaticBundle}, change_detection::MaybeLocation, component::{Component, ComponentId, ComponentInfo}, entity::{Entity, EntityClonerBuilder}, @@ -190,9 +190,9 @@ pub fn clear() -> impl EntityCommand { } /// An [`EntityCommand`] that removes all components from an entity, -/// except for those in the given [`Bundle`]. +/// except for those in the given [`StaticBundle`]. #[track_caller] -pub fn retain() -> impl EntityCommand { +pub fn retain() -> impl EntityCommand { let caller = MaybeLocation::caller(); move |mut entity: EntityWorldMut| { entity.retain_with_caller::(caller); diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index f335a21c3850b..9d27d1643ff19 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1931,7 +1931,7 @@ impl<'a> EntityCommands<'a> { /// # bevy_ecs::system::assert_is_system(remove_combat_stats_system); /// ``` #[track_caller] - pub fn retain(&mut self) -> &mut Self { + pub fn retain(&mut self) -> &mut Self { self.queue(entity_command::retain::()) } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 637201eee5e3b..53fd88bdf6639 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -2138,12 +2138,15 @@ impl<'w> EntityWorldMut<'w> { /// /// If the entity has been despawned while this `EntityWorldMut` is still alive. #[track_caller] - pub fn retain(&mut self) -> &mut Self { + pub fn retain(&mut self) -> &mut Self { self.retain_with_caller::(MaybeLocation::caller()) } #[inline] - pub(crate) fn retain_with_caller(&mut self, caller: MaybeLocation) -> &mut Self { + pub(crate) fn retain_with_caller( + &mut self, + caller: MaybeLocation, + ) -> &mut Self { let old_location = self.location(); let archetypes = &mut self.world.archetypes; let storages = &mut self.world.storages; From 77688c90eb79e27a6964282422655289e2c53369 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 18:43:06 +0200 Subject: [PATCH 08/44] Require StaticBundle in batch spawning --- crates/bevy_ecs/src/bundle.rs | 14 +++++++++++++- crates/bevy_ecs/src/spawn.rs | 4 +++- crates/bevy_ecs/src/system/commands/command.rs | 6 +++--- crates/bevy_ecs/src/system/commands/mod.rs | 12 ++++++------ crates/bevy_ecs/src/world/mod.rs | 16 ++++++++-------- crates/bevy_ecs/src/world/spawn_batch.rs | 18 +++++++++--------- 6 files changed, 42 insertions(+), 28 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index bb10cf5ca5189..b7c366291e4e0 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1702,7 +1702,19 @@ pub(crate) struct BundleSpawner<'w> { impl<'w> BundleSpawner<'w> { #[inline] - pub fn new(world: &'w mut World, change_tick: Tick) -> Self { + pub fn new(bundle: &T, world: &'w mut World, change_tick: Tick) -> Self { + // 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 + .bundles + .register_info(bundle, &mut registrator, &mut world.storages); + // SAFETY: we initialized this bundle_id in `init_info` + unsafe { Self::new_with_id(world, bundle_id, change_tick) } + } + + #[inline] + pub fn new_static(world: &'w mut World, change_tick: Tick) -> Self { // 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) }; diff --git a/crates/bevy_ecs/src/spawn.rs b/crates/bevy_ecs/src/spawn.rs index 68e766ac9d8b7..7e46b4d3f882d 100644 --- a/crates/bevy_ecs/src/spawn.rs +++ b/crates/bevy_ecs/src/spawn.rs @@ -46,7 +46,9 @@ pub trait SpawnableList { fn size_hint(&self) -> usize; } -impl> SpawnableList for Vec { +impl + StaticBundle> SpawnableList + for Vec +{ fn spawn(self, world: &mut World, entity: Entity) { let mapped_bundles = self.into_iter().map(|b| (R::from(entity), b)); world.spawn_batch(mapped_bundles); diff --git a/crates/bevy_ecs/src/system/commands/command.rs b/crates/bevy_ecs/src/system/commands/command.rs index af7b88edfc77c..dcbc883ea96ca 100644 --- a/crates/bevy_ecs/src/system/commands/command.rs +++ b/crates/bevy_ecs/src/system/commands/command.rs @@ -5,7 +5,7 @@ //! [`Commands`](crate::system::Commands). use crate::{ - bundle::{Bundle, InsertMode, NoBundleEffect}, + bundle::{Bundle, InsertMode, NoBundleEffect, StaticBundle}, change_detection::MaybeLocation, entity::Entity, error::Result, @@ -70,7 +70,7 @@ where pub fn spawn_batch(bundles_iter: I) -> impl Command where I: IntoIterator + Send + Sync + 'static, - I::Item: Bundle, + I::Item: Bundle + StaticBundle, { let caller = MaybeLocation::caller(); move |world: &mut World| { @@ -88,7 +88,7 @@ where pub fn insert_batch(batch: I, insert_mode: InsertMode) -> impl Command where I: IntoIterator + Send + Sync + 'static, - B: Bundle, + B: Bundle + StaticBundle, { let caller = MaybeLocation::caller(); move |world: &mut World| -> Result { diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 9d27d1643ff19..9fde89cc79496 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -15,7 +15,7 @@ use core::marker::PhantomData; use crate::{ self as bevy_ecs, - bundle::{Bundle, InsertMode, NoBundleEffect}, + bundle::{Bundle, InsertMode, NoBundleEffect, StaticBundle}, change_detection::{MaybeLocation, Mut}, component::{Component, ComponentId, Mutable}, entity::{Entities, Entity, EntityClonerBuilder, EntityDoesNotExistError}, @@ -523,7 +523,7 @@ impl<'w, 's> Commands<'w, 's> { pub fn spawn_batch(&mut self, batch: I) where I: IntoIterator + Send + Sync + 'static, - I::Item: Bundle, + I::Item: Bundle + StaticBundle, { self.queue(command::spawn_batch(batch)); } @@ -671,7 +671,7 @@ impl<'w, 's> Commands<'w, 's> { pub fn insert_batch(&mut self, batch: I) where I: IntoIterator + Send + Sync + 'static, - B: Bundle, + B: Bundle + StaticBundle, { self.queue(command::insert_batch(batch, InsertMode::Replace)); } @@ -702,7 +702,7 @@ impl<'w, 's> Commands<'w, 's> { pub fn insert_batch_if_new(&mut self, batch: I) where I: IntoIterator + Send + Sync + 'static, - B: Bundle, + B: Bundle + StaticBundle, { self.queue(command::insert_batch(batch, InsertMode::Keep)); } @@ -732,7 +732,7 @@ impl<'w, 's> Commands<'w, 's> { pub fn try_insert_batch(&mut self, batch: I) where I: IntoIterator + Send + Sync + 'static, - B: Bundle, + B: Bundle + StaticBundle, { self.queue(command::insert_batch(batch, InsertMode::Replace).handle_error_with(warn)); } @@ -763,7 +763,7 @@ impl<'w, 's> Commands<'w, 's> { pub fn try_insert_batch_if_new(&mut self, batch: I) where I: IntoIterator + Send + Sync + 'static, - B: Bundle, + B: Bundle + StaticBundle, { self.queue(command::insert_batch(batch, InsertMode::Keep).handle_error_with(warn)); } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index a24dc915e58b8..36bbf4ce1d5df 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1155,7 +1155,7 @@ impl World { self.flush(); let change_tick = self.change_tick(); let entity = self.entities.alloc(); - let mut bundle_spawner = BundleSpawner::new::(self, change_tick); + let mut bundle_spawner = BundleSpawner::new(&bundle, self, change_tick); // SAFETY: bundle's type matches `bundle_info`, entity is allocated but non-existent let (entity_location, after_effect) = unsafe { bundle_spawner.spawn_non_existent(entity, bundle, caller) }; @@ -1221,7 +1221,7 @@ impl World { pub fn spawn_batch(&mut self, iter: I) -> SpawnBatchIter<'_, I::IntoIter> where I: IntoIterator, - I::Item: Bundle, + I::Item: Bundle + StaticBundle, { SpawnBatchIter::new(self, iter.into_iter(), MaybeLocation::caller()) } @@ -2226,7 +2226,7 @@ impl World { where I: IntoIterator, I::IntoIter: Iterator, - B: Bundle, + B: Bundle + StaticBundle, { self.insert_batch_with_caller(batch, InsertMode::Replace, MaybeLocation::caller()); } @@ -2251,7 +2251,7 @@ impl World { where I: IntoIterator, I::IntoIter: Iterator, - B: Bundle, + B: Bundle + StaticBundle, { self.insert_batch_with_caller(batch, InsertMode::Keep, MaybeLocation::caller()); } @@ -2270,7 +2270,7 @@ impl World { ) where I: IntoIterator, I::IntoIter: Iterator, - B: Bundle, + B: Bundle + StaticBundle, { struct InserterArchetypeCache<'w> { inserter: BundleInserter<'w>, @@ -2369,7 +2369,7 @@ impl World { where I: IntoIterator, I::IntoIter: Iterator, - B: Bundle, + B: Bundle + StaticBundle, { self.try_insert_batch_with_caller(batch, InsertMode::Replace, MaybeLocation::caller()) } @@ -2391,7 +2391,7 @@ impl World { where I: IntoIterator, I::IntoIter: Iterator, - B: Bundle, + B: Bundle + StaticBundle, { self.try_insert_batch_with_caller(batch, InsertMode::Keep, MaybeLocation::caller()) } @@ -2415,7 +2415,7 @@ impl World { where I: IntoIterator, I::IntoIter: Iterator, - B: Bundle, + B: Bundle + StaticBundle, { struct InserterArchetypeCache<'w> { inserter: BundleInserter<'w>, diff --git a/crates/bevy_ecs/src/world/spawn_batch.rs b/crates/bevy_ecs/src/world/spawn_batch.rs index 16bd9bb8059b4..c2bbd8c0d05fd 100644 --- a/crates/bevy_ecs/src/world/spawn_batch.rs +++ b/crates/bevy_ecs/src/world/spawn_batch.rs @@ -1,5 +1,5 @@ use crate::{ - bundle::{Bundle, BundleSpawner, NoBundleEffect}, + bundle::{Bundle, BundleSpawner, NoBundleEffect, StaticBundle}, change_detection::MaybeLocation, entity::{Entity, EntitySetIterator}, world::World, @@ -13,7 +13,7 @@ use core::iter::FusedIterator; pub struct SpawnBatchIter<'w, I> where I: Iterator, - I::Item: Bundle, + I::Item: Bundle + StaticBundle, { inner: I, spawner: BundleSpawner<'w>, @@ -23,7 +23,7 @@ where impl<'w, I> SpawnBatchIter<'w, I> where I: Iterator, - I::Item: Bundle, + I::Item: Bundle + StaticBundle, { #[inline] #[track_caller] @@ -38,7 +38,7 @@ where let length = upper.unwrap_or(lower); world.entities.reserve(length as u32); - let mut spawner = BundleSpawner::new::(world, change_tick); + let mut spawner = BundleSpawner::new_static::(world, change_tick); spawner.reserve_storage(length); Self { @@ -52,7 +52,7 @@ where impl Drop for SpawnBatchIter<'_, I> where I: Iterator, - I::Item: Bundle, + I::Item: Bundle + StaticBundle, { fn drop(&mut self) { // Iterate through self in order to spawn remaining bundles. @@ -66,7 +66,7 @@ where impl Iterator for SpawnBatchIter<'_, I> where I: Iterator, - I::Item: Bundle, + I::Item: Bundle + StaticBundle, { type Item = Entity; @@ -84,7 +84,7 @@ where impl ExactSizeIterator for SpawnBatchIter<'_, I> where I: ExactSizeIterator, - T: Bundle, + T: Bundle + StaticBundle, { fn len(&self) -> usize { self.inner.len() @@ -94,7 +94,7 @@ where impl FusedIterator for SpawnBatchIter<'_, I> where I: FusedIterator, - T: Bundle, + T: Bundle + StaticBundle, { } @@ -102,6 +102,6 @@ where unsafe impl EntitySetIterator for SpawnBatchIter<'_, I> where I: FusedIterator, - T: Bundle, + T: Bundle + StaticBundle, { } From 6cd5783684d600891d09b318ffef20129a9cd34e Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 20:57:02 +0200 Subject: [PATCH 09/44] Require StaticBundle in ReflectBundle --- crates/bevy_ecs/src/reflect/bundle.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/reflect/bundle.rs b/crates/bevy_ecs/src/reflect/bundle.rs index ee02aff86e7fe..f6a9034ac2958 100644 --- a/crates/bevy_ecs/src/reflect/bundle.rs +++ b/crates/bevy_ecs/src/reflect/bundle.rs @@ -8,7 +8,7 @@ use alloc::boxed::Box; use core::any::{Any, TypeId}; use crate::{ - bundle::BundleFromComponents, + bundle::{BundleFromComponents, StaticBundle}, entity::EntityMapper, prelude::Bundle, relationship::RelationshipHookMode, @@ -56,7 +56,7 @@ impl ReflectBundleFns { /// /// This is useful if you want to start with the default implementation before overriding some /// of the functions to create a custom implementation. - pub fn new() -> Self { + pub fn new() -> Self { >::from_type().0 } } @@ -147,7 +147,9 @@ impl ReflectBundle { } } -impl FromType for ReflectBundle { +impl FromType + for ReflectBundle +{ fn from_type() -> Self { ReflectBundle(ReflectBundleFns { insert: |entity, reflected_bundle, registry| { From c9ceb5a23ba9f7bfcc84afb93f30beefea3d6b32 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 21:08:47 +0200 Subject: [PATCH 10/44] Require StaticBundle in ExtractComponent --- crates/bevy_render/src/extract_component.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_render/src/extract_component.rs b/crates/bevy_render/src/extract_component.rs index e1f528d6ab2bd..1459dc29cee31 100644 --- a/crates/bevy_render/src/extract_component.rs +++ b/crates/bevy_render/src/extract_component.rs @@ -8,7 +8,7 @@ use crate::{ }; use bevy_app::{App, Plugin}; use bevy_ecs::{ - bundle::NoBundleEffect, + bundle::{NoBundleEffect, StaticBundle}, component::Component, prelude::*, query::{QueryFilter, QueryItem, ReadOnlyQueryData}, @@ -54,7 +54,7 @@ pub trait ExtractComponent: Component { /// /// `Out` has a [`Bundle`] trait bound instead of a [`Component`] trait bound in order to allow use cases /// such as tuples of components as output. - type Out: Bundle; + type Out: Bundle + StaticBundle; // TODO: https://github.com/rust-lang/rust/issues/29661 // type Out: Component = Self; From 756aeeae59044d49cdb67cb3a6df49c108ef9eaf Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 16:44:05 +0200 Subject: [PATCH 11/44] Add IS_STATIC and IS_BOUNDED flags to Bundle --- crates/bevy_ecs/macros/src/lib.rs | 11 ++++++++++- crates/bevy_ecs/src/bundle.rs | 28 +++++++++++++++++++++++++--- crates/bevy_ecs/src/spawn.rs | 11 ++++++++++- 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index eeba9da060b7c..e86e3a137c687 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -77,6 +77,8 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { let mut field_component_ids = Vec::new(); let mut field_get_component_ids = Vec::new(); let mut field_get_components = Vec::new(); + let mut field_is_static = Vec::new(); + let mut field_is_bounded = Vec::new(); let mut field_from_components = Vec::new(); let mut field_required_components = Vec::new(); for (((i, field_type), field_kind), field) in field_type @@ -96,6 +98,10 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { field_get_component_ids.push(quote! { <#field_type as #ecs_path::bundle::StaticBundle>::get_component_ids(components, &mut *ids); }); + field_is_static + .push(quote! { <#field_type as #ecs_path::bundle::Bundle>::IS_STATIC }); + field_is_bounded + .push(quote! { <#field_type as #ecs_path::bundle::Bundle>::IS_BOUNDED }); match field { Some(field) => { field_get_components.push(quote! { @@ -158,7 +164,10 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { } // SAFETY: see the corresponding implementation of `StaticBundle` - unsafe impl #impl_generics #ecs_path::bundle::Bundle for #struct_name #ty_generics #where_clause {} + unsafe impl #impl_generics #ecs_path::bundle::Bundle for #struct_name #ty_generics #where_clause { + const IS_STATIC: bool = true #(&& #field_is_static)*; + const IS_BOUNDED: bool = true #(&& #field_is_bounded)*; + } // SAFETY: // - ComponentId is returned in field-definition-order. [from_components] uses field-definition-order diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index b7c366291e4e0..a45028bc35c89 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -146,7 +146,23 @@ use variadics_please::all_tuples; label = "invalid `Bundle`", note = "consider annotating `{Self}` with `#[derive(Component)]` or `#[derive(Bundle)]`" )] -pub unsafe trait Bundle: StaticBundle + DynamicBundle + Send + Sync + 'static {} +pub unsafe trait Bundle: StaticBundle + DynamicBundle + Send + Sync + 'static { + /// Whether this is a [`StaticBundle`] or not. In case this is a [`StaticBundle`] the associated + /// [`BundleId`] and [`BundleInfo`] are known to be unique and can be cached by this type's [`TypeId`]. + /// + /// This is a hack to work around the lack of specialization. + #[doc(hidden)] + const IS_STATIC: bool; + + /// Whether the set of components this bundle includes is bounded or not. In case it is bounded we + /// can produce an additional cache key based on which components from the bounded set are included in + /// `self` or not, and use that to reuse an existing [`BundleId`] and [`BundleInfo`] associated with the + /// dynamic component set of `self`. + /// + /// This is a hack to work around the lack of specialization. + #[doc(hidden)] + const IS_BOUNDED: bool; +} /// Each `StaticBundle` represents a static set of [`Component`] types. /// Currently, bundles can only contain one of each [`Component`], and will @@ -272,7 +288,10 @@ unsafe impl StaticBundle for C { } // SAFETY: see the corresponding implementation of `StaticBundle` -unsafe impl Bundle for C {} +unsafe impl Bundle for C { + const IS_STATIC: bool = true; + const IS_BOUNDED: bool = true; +} // SAFETY: // - `Bundle::from_components` calls `func` exactly once for C, which is the exact value returned by `Bundle::component_ids`. @@ -334,7 +353,10 @@ macro_rules! tuple_impl { $(#[$meta])* // SAFETY: see the corresponding implementation of `StaticBundle` - unsafe impl<$($name: Bundle),*> Bundle for ($($name,)*) {} + unsafe impl<$($name: Bundle),*> Bundle for ($($name,)*) { + const IS_STATIC: bool = true $(&& <$name as Bundle>::IS_STATIC)*; + const IS_BOUNDED: bool = true $(&& <$name as Bundle>::IS_STATIC)*; + } #[expect( clippy::allow_attributes, diff --git a/crates/bevy_ecs/src/spawn.rs b/crates/bevy_ecs/src/spawn.rs index 7e46b4d3f882d..e468796d6e677 100644 --- a/crates/bevy_ecs/src/spawn.rs +++ b/crates/bevy_ecs/src/spawn.rs @@ -215,6 +215,10 @@ unsafe impl + Send + Sync + 'static> Static unsafe impl + Send + Sync + 'static> Bundle for SpawnRelatedBundle { + // We are inserting `R::RelationshipTarget`, which is a `Component` and thus + // always a `StaticBundle`. + const IS_STATIC: bool = true; + const IS_BOUNDED: bool = true; } impl> DynamicBundle for SpawnRelatedBundle { type Effect = Self; @@ -283,7 +287,12 @@ unsafe impl StaticBundle for SpawnOneRelated { ); } } -unsafe impl Bundle for SpawnOneRelated {} +unsafe impl Bundle for SpawnOneRelated { + // We are inserting `R::RelationshipTarget`, which is a `Component` and thus + // always a `StaticBundle`. + const IS_STATIC: bool = true; + const IS_BOUNDED: bool = true; +} /// [`RelationshipTarget`] methods that create a [`Bundle`] with a [`DynamicBundle::Effect`] that: /// From b875cb4c2c413cee270a9eeaa9fdd9aae4dcf1cc Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 17:05:04 +0200 Subject: [PATCH 12/44] Add cache_key method to Bundle --- crates/bevy_ecs/macros/src/lib.rs | 28 ++++++++++++++++++++ crates/bevy_ecs/src/bundle.rs | 44 +++++++++++++++++++++++++++++++ crates/bevy_ecs/src/spawn.rs | 16 ++++++++--- 3 files changed, 84 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index e86e3a137c687..596c56ae992a7 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -79,6 +79,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { let mut field_get_components = Vec::new(); let mut field_is_static = Vec::new(); let mut field_is_bounded = Vec::new(); + let mut field_cache_key = Vec::new(); let mut field_from_components = Vec::new(); let mut field_required_components = Vec::new(); for (((i, field_type), field_kind), field) in field_type @@ -110,6 +111,15 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { field_from_components.push(quote! { #field: <#field_type as #ecs_path::bundle::BundleFromComponents>::from_components(ctx, &mut *func), }); + field_cache_key.push(quote! { + let (sub_key, sub_size) = <#field_type as #ecs_path::bundle::Bundle>::cache_key(&self.#field); + key |= sub_key << size; + size += sub_size; + // Bail out if size is too big + if size > 64 { + return (0, 65); + } + }); } None => { let index = Index::from(i); @@ -119,6 +129,15 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { field_from_components.push(quote! { #index: <#field_type as #ecs_path::bundle::BundleFromComponents>::from_components(ctx, &mut *func), }); + field_cache_key.push(quote! { + let (sub_key, sub_size) = <#field_type as #ecs_path::bundle::Bundle>::cache_key(&self.#index); + key |= sub_key << size; + size += sub_size; + // Bail out if size is too big + if size > 64 { + return (0, 65); + } + }); } } } @@ -167,6 +186,15 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { unsafe impl #impl_generics #ecs_path::bundle::Bundle for #struct_name #ty_generics #where_clause { const IS_STATIC: bool = true #(&& #field_is_static)*; const IS_BOUNDED: bool = true #(&& #field_is_bounded)*; + + fn cache_key(&self) -> (u64, usize) { + let mut key = 0; + let mut size = 0; + + #(#field_cache_key)* + + (key, size) + } } // SAFETY: diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index a45028bc35c89..96f967441ea91 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -162,6 +162,11 @@ pub unsafe trait Bundle: StaticBundle + DynamicBundle + Send + Sync + 'static { /// This is a hack to work around the lack of specialization. #[doc(hidden)] const IS_BOUNDED: bool; + + /// Computes the cache key associated with `self` and its size. The key is limited to 64 bits, and if the + /// size exceeds that it should be ignored. + #[doc(hidden)] + fn cache_key(&self) -> (u64, usize); } /// Each `StaticBundle` represents a static set of [`Component`] types. @@ -291,6 +296,10 @@ unsafe impl StaticBundle for C { unsafe impl Bundle for C { const IS_STATIC: bool = true; const IS_BOUNDED: bool = true; + + fn cache_key(&self) -> (u64, usize) { + (0, 0) + } } // SAFETY: @@ -351,11 +360,46 @@ macro_rules! tuple_impl { } } + #[expect( + clippy::allow_attributes, + reason = "This is a tuple-related macro; as such, the lints below may not always apply." + )] + #[allow( + unused_mut, + unused_variables, + reason = "Zero-length tuples won't use any of the parameters." + )] $(#[$meta])* // SAFETY: see the corresponding implementation of `StaticBundle` unsafe impl<$($name: Bundle),*> Bundle for ($($name,)*) { const IS_STATIC: bool = true $(&& <$name as Bundle>::IS_STATIC)*; const IS_BOUNDED: bool = true $(&& <$name as Bundle>::IS_STATIC)*; + + fn cache_key(&self) -> (u64, usize) { + #[allow( + non_snake_case, + reason = "The names of these variables are provided by the caller, not by us." + )] + let ($($name,)*) = self; + + let mut key = 0; + let mut size = 0; + + $( + let (sub_key, sub_size) = $name.cache_key(); + + key |= sub_key << size; + + size += sub_size; + + // Bail out if size is too big + if size > 64 { + return (0, 65); + } + )* + + (key, size) + } } #[expect( diff --git a/crates/bevy_ecs/src/spawn.rs b/crates/bevy_ecs/src/spawn.rs index e468796d6e677..fae7bb2835652 100644 --- a/crates/bevy_ecs/src/spawn.rs +++ b/crates/bevy_ecs/src/spawn.rs @@ -217,8 +217,12 @@ unsafe impl + Send + Sync + 'static> Bundle { // We are inserting `R::RelationshipTarget`, which is a `Component` and thus // always a `StaticBundle`. - const IS_STATIC: bool = true; - const IS_BOUNDED: bool = true; + const IS_STATIC: bool = ::IS_STATIC; + const IS_BOUNDED: bool = ::IS_BOUNDED; + + fn cache_key(&self) -> (u64, usize) { + (0, 0) + } } impl> DynamicBundle for SpawnRelatedBundle { type Effect = Self; @@ -290,8 +294,12 @@ unsafe impl StaticBundle for SpawnOneRelated { unsafe impl Bundle for SpawnOneRelated { // We are inserting `R::RelationshipTarget`, which is a `Component` and thus // always a `StaticBundle`. - const IS_STATIC: bool = true; - const IS_BOUNDED: bool = true; + const IS_STATIC: bool = ::IS_STATIC; + const IS_BOUNDED: bool = ::IS_BOUNDED; + + fn cache_key(&self) -> (u64, usize) { + (0, 0) + } } /// [`RelationshipTarget`] methods that create a [`Bundle`] with a [`DynamicBundle::Effect`] that: From 6daa55f2146b758c421b546ea4c18369eecd78e6 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 17:14:25 +0200 Subject: [PATCH 13/44] Add component_ids taking &self to Bundle --- crates/bevy_ecs/macros/src/lib.rs | 37 ++++++++++++++++++++++--------- crates/bevy_ecs/src/bundle.rs | 30 +++++++++++++++++++++++++ crates/bevy_ecs/src/spawn.rs | 16 +++++++++++++ 3 files changed, 72 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 596c56ae992a7..44beb65d2d91f 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -74,14 +74,15 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { .map(|field| &field.ty) .collect::>(); + let mut field_static_component_ids = Vec::new(); + let mut field_static_required_components = Vec::new(); + let mut field_static_get_component_ids = Vec::new(); let mut field_component_ids = Vec::new(); - let mut field_get_component_ids = Vec::new(); let mut field_get_components = Vec::new(); let mut field_is_static = Vec::new(); let mut field_is_bounded = Vec::new(); let mut field_cache_key = Vec::new(); let mut field_from_components = Vec::new(); - let mut field_required_components = Vec::new(); for (((i, field_type), field_kind), field) in field_type .iter() .enumerate() @@ -90,13 +91,13 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { { match field_kind { BundleFieldKind::Component => { - field_component_ids.push(quote! { - <#field_type as #ecs_path::bundle::StaticBundle>::component_ids(components, &mut *ids); + field_static_component_ids.push(quote! { + <#field_type as #ecs_path::bundle::StaticBundle>::component_ids(components, &mut *ids); }); - field_required_components.push(quote! { + field_static_required_components.push(quote! { <#field_type as #ecs_path::bundle::StaticBundle>::register_required_components(components, required_components); }); - field_get_component_ids.push(quote! { + field_static_get_component_ids.push(quote! { <#field_type as #ecs_path::bundle::StaticBundle>::get_component_ids(components, &mut *ids); }); field_is_static @@ -105,8 +106,11 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { .push(quote! { <#field_type as #ecs_path::bundle::Bundle>::IS_BOUNDED }); match field { Some(field) => { + field_component_ids.push(quote! { + <#field_type as #ecs_path::bundle::Bundle>::component_ids(&self.#field, components, ids); + }); field_get_components.push(quote! { - self.#field.get_components(&mut *func); + <#field_type as #ecs_path::bundle::DynamicBundle>::get_components(self.#field, &mut *func); }); field_from_components.push(quote! { #field: <#field_type as #ecs_path::bundle::BundleFromComponents>::from_components(ctx, &mut *func), @@ -123,8 +127,11 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { } None => { let index = Index::from(i); + field_component_ids.push(quote! { + <#field_type as #ecs_path::bundle::Bundle>::component_ids(&self.#index, components, ids); + }); field_get_components.push(quote! { - self.#index.get_components(&mut *func); + <#field_type as #ecs_path::bundle::DynamicBundle>::get_components(self.#index, &mut *func); }); field_from_components.push(quote! { #index: <#field_type as #ecs_path::bundle::BundleFromComponents>::from_components(ctx, &mut *func), @@ -164,21 +171,21 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { components: &mut #ecs_path::component::ComponentsRegistrator, ids: &mut impl FnMut(#ecs_path::component::ComponentId) ){ - #(#field_component_ids)* + #(#field_static_component_ids)* } fn get_component_ids( components: &#ecs_path::component::Components, ids: &mut impl FnMut(Option<#ecs_path::component::ComponentId>) ){ - #(#field_get_component_ids)* + #(#field_static_get_component_ids)* } fn register_required_components( components: &mut #ecs_path::component::ComponentsRegistrator, required_components: &mut #ecs_path::component::RequiredComponents ){ - #(#field_required_components)* + #(#field_static_required_components)* } } @@ -195,6 +202,14 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { (key, size) } + + fn component_ids( + &self, + components: &mut #ecs_path::component::ComponentsRegistrator, + ids: &mut impl FnMut(#ecs_path::component::ComponentId), + ) { + #(#field_component_ids)* + } } // SAFETY: diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 96f967441ea91..758529d3e9927 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -167,6 +167,14 @@ pub unsafe trait Bundle: StaticBundle + DynamicBundle + Send + Sync + 'static { /// size exceeds that it should be ignored. #[doc(hidden)] fn cache_key(&self) -> (u64, usize); + + /// Gets `self`'s component ids, in the order of this bundle's [`Component`]s + #[doc(hidden)] + fn component_ids( + &self, + components: &mut ComponentsRegistrator, + ids: &mut impl FnMut(ComponentId), + ); } /// Each `StaticBundle` represents a static set of [`Component`] types. @@ -300,6 +308,14 @@ unsafe impl Bundle for C { fn cache_key(&self) -> (u64, usize) { (0, 0) } + + fn component_ids( + &self, + components: &mut ComponentsRegistrator, + ids: &mut impl FnMut(ComponentId), + ) { + ::component_ids(components, ids); + } } // SAFETY: @@ -400,6 +416,20 @@ macro_rules! tuple_impl { (key, size) } + + fn component_ids( + &self, + components: &mut ComponentsRegistrator, + ids: &mut impl FnMut(ComponentId), + ) { + #[allow( + non_snake_case, + reason = "The names of these variables are provided by the caller, not by us." + )] + let ($($name,)*) = self; + + $($name.component_ids(components, ids);)* + } } #[expect( diff --git a/crates/bevy_ecs/src/spawn.rs b/crates/bevy_ecs/src/spawn.rs index fae7bb2835652..d6365b22646e3 100644 --- a/crates/bevy_ecs/src/spawn.rs +++ b/crates/bevy_ecs/src/spawn.rs @@ -223,6 +223,14 @@ unsafe impl + Send + Sync + 'static> Bundle fn cache_key(&self) -> (u64, usize) { (0, 0) } + + fn component_ids( + &self, + components: &mut crate::component::ComponentsRegistrator, + ids: &mut impl FnMut(crate::component::ComponentId), + ) { + ::component_ids(components, ids); + } } impl> DynamicBundle for SpawnRelatedBundle { type Effect = Self; @@ -300,6 +308,14 @@ unsafe impl Bundle for SpawnOneRelated { fn cache_key(&self) -> (u64, usize) { (0, 0) } + + fn component_ids( + &self, + components: &mut crate::component::ComponentsRegistrator, + ids: &mut impl FnMut(crate::component::ComponentId), + ) { + ::component_ids(components, ids); + } } /// [`RelationshipTarget`] methods that create a [`Bundle`] with a [`DynamicBundle::Effect`] that: From a22c29b0549d4ea287f9d742b46449b56de0b0b3 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 18:22:30 +0200 Subject: [PATCH 14/44] Add register_required_components taking &self to Bundle --- crates/bevy_ecs/macros/src/lib.rs | 15 +++++++++++++++ crates/bevy_ecs/src/bundle.rs | 30 ++++++++++++++++++++++++++++++ crates/bevy_ecs/src/spawn.rs | 16 ++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 44beb65d2d91f..1e67d5b683168 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -78,6 +78,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { let mut field_static_required_components = Vec::new(); let mut field_static_get_component_ids = Vec::new(); let mut field_component_ids = Vec::new(); + let mut field_required_components = Vec::new(); let mut field_get_components = Vec::new(); let mut field_is_static = Vec::new(); let mut field_is_bounded = Vec::new(); @@ -109,6 +110,9 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { field_component_ids.push(quote! { <#field_type as #ecs_path::bundle::Bundle>::component_ids(&self.#field, components, ids); }); + field_required_components.push(quote! { + <#field_type as #ecs_path::bundle::Bundle>::register_required_components(&self.#field, components, required_components); + }); field_get_components.push(quote! { <#field_type as #ecs_path::bundle::DynamicBundle>::get_components(self.#field, &mut *func); }); @@ -130,6 +134,9 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { field_component_ids.push(quote! { <#field_type as #ecs_path::bundle::Bundle>::component_ids(&self.#index, components, ids); }); + field_required_components.push(quote! { + <#field_type as #ecs_path::bundle::Bundle>::register_required_components(&self.#index, components, required_components); + }); field_get_components.push(quote! { <#field_type as #ecs_path::bundle::DynamicBundle>::get_components(self.#index, &mut *func); }); @@ -210,6 +217,14 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { ) { #(#field_component_ids)* } + + fn register_required_components( + &self, + components: &mut #ecs_path::component::ComponentsRegistrator, + required_components: &mut #ecs_path::component::RequiredComponents, + ) { + #(#field_required_components)* + } } // SAFETY: diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 758529d3e9927..b0ca084d1bfef 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -175,6 +175,14 @@ pub unsafe trait Bundle: StaticBundle + DynamicBundle + Send + Sync + 'static { components: &mut ComponentsRegistrator, ids: &mut impl FnMut(ComponentId), ); + + /// Registers components that are required by the components in this [`Bundle`]. + #[doc(hidden)] + fn register_required_components( + &self, + _components: &mut ComponentsRegistrator, + _required_components: &mut RequiredComponents, + ); } /// Each `StaticBundle` represents a static set of [`Component`] types. @@ -316,6 +324,14 @@ unsafe impl Bundle for C { ) { ::component_ids(components, ids); } + + fn register_required_components( + &self, + components: &mut ComponentsRegistrator, + required_components: &mut RequiredComponents, + ) { + ::register_required_components(components, required_components); + } } // SAFETY: @@ -430,6 +446,20 @@ macro_rules! tuple_impl { $($name.component_ids(components, ids);)* } + + fn register_required_components( + &self, + components: &mut ComponentsRegistrator, + required_components: &mut RequiredComponents, + ) { + #[allow( + non_snake_case, + reason = "The names of these variables are provided by the caller, not by us." + )] + let ($($name,)*) = self; + + $($name.register_required_components(components, required_components);)* + } } #[expect( diff --git a/crates/bevy_ecs/src/spawn.rs b/crates/bevy_ecs/src/spawn.rs index d6365b22646e3..2822489477c9d 100644 --- a/crates/bevy_ecs/src/spawn.rs +++ b/crates/bevy_ecs/src/spawn.rs @@ -231,6 +231,14 @@ unsafe impl + Send + Sync + 'static> Bundle ) { ::component_ids(components, ids); } + + fn register_required_components( + &self, + components: &mut crate::component::ComponentsRegistrator, + required_components: &mut crate::component::RequiredComponents, + ) { + ::register_required_components(components, required_components); + } } impl> DynamicBundle for SpawnRelatedBundle { type Effect = Self; @@ -316,6 +324,14 @@ unsafe impl Bundle for SpawnOneRelated { ) { ::component_ids(components, ids); } + + fn register_required_components( + &self, + components: &mut crate::component::ComponentsRegistrator, + required_components: &mut crate::component::RequiredComponents, + ) { + ::register_required_components(components, required_components); + } } /// [`RelationshipTarget`] methods that create a [`Bundle`] with a [`DynamicBundle::Effect`] that: From f9eb711ad50f9d395f2450ce68789907c235cffd Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 18:28:43 +0200 Subject: [PATCH 15/44] Rename Bundles::bundle_ids to Bundles::static_bundle_ids --- crates/bevy_ecs/src/bundle.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index b0ca084d1bfef..ab929bfd88144 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -2011,7 +2011,7 @@ impl<'w> BundleSpawner<'w> { pub struct Bundles { bundle_infos: Vec, /// Cache static [`BundleId`] - bundle_ids: TypeIdMap, + static_bundle_ids: TypeIdMap, /// Cache bundles, which contains both explicit and required components of [`Bundle`] contributed_bundle_ids: TypeIdMap, /// Cache dynamic [`BundleId`] with multiple components @@ -2050,7 +2050,7 @@ impl Bundles { /// or if `type_id` does not correspond to a type of bundle. #[inline] pub fn get_id(&self, type_id: TypeId) -> Option { - self.bundle_ids.get(&type_id).cloned() + self.static_bundle_ids.get(&type_id).cloned() } /// Registers a new [`BundleInfo`] for a statically known type. @@ -2062,7 +2062,7 @@ impl Bundles { storages: &mut Storages, ) -> BundleId { let bundle_infos = &mut self.bundle_infos; - *self.bundle_ids.entry(TypeId::of::()).or_insert_with(|| { + *self.static_bundle_ids.entry(TypeId::of::()).or_insert_with(|| { let mut component_ids= Vec::new(); T::component_ids(components, &mut |id| component_ids.push(id)); let id = BundleId(bundle_infos.len()); From 977be663a624950f80f2063acc2bc4c4bf97dd94 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 18:37:43 +0200 Subject: [PATCH 16/44] Rename Bundles::register_info to register_static_info and add more dynamic register_info --- crates/bevy_ecs/src/bundle.rs | 80 ++++++++++++++++++++++--- crates/bevy_ecs/src/world/entity_ref.rs | 2 +- crates/bevy_ecs/src/world/mod.rs | 6 +- 3 files changed, 75 insertions(+), 13 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index ab929bfd88144..29239ec16080f 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1173,7 +1173,7 @@ impl<'w> BundleInserter<'w> { unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) }; let bundle_id = world .bundles - .register_info::(&mut registrator, &mut world.storages); + .register_static_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) } } @@ -1566,7 +1566,7 @@ impl<'w> BundleRemover<'w> { unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) }; let bundle_id = world .bundles - .register_info::(&mut registrator, &mut world.storages); + .register_static_info::(&mut registrator, &mut world.storages); // SAFETY: we initialized this bundle_id in `init_info`, and caller ensures archetype is valid. unsafe { Self::new_with_id(world, archetype_id, bundle_id, require_all) } } @@ -1846,7 +1846,7 @@ impl<'w> BundleSpawner<'w> { unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) }; let bundle_id = world .bundles - .register_info::(&mut registrator, &mut world.storages); + .register_static_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) } } @@ -2010,12 +2010,16 @@ impl<'w> BundleSpawner<'w> { #[derive(Default)] pub struct Bundles { bundle_infos: Vec, - /// Cache static [`BundleId`] + /// Cache [`BundleId`]s for static bundles static_bundle_ids: TypeIdMap, + /// Cache [`BundleId`]s for bounded but non-static bundles + bounded_bundle_ids: HashMap<(TypeId, u64), BundleId>, + /// Cache [`BundleId`]s for dynamic bundles + dynamic_bundle_ids: HashMap, BundleId>, + /// Cache bundles, which contains both explicit and required components of [`Bundle`] contributed_bundle_ids: TypeIdMap, - /// Cache dynamic [`BundleId`] with multiple components - dynamic_bundle_ids: HashMap, BundleId>, + dynamic_bundle_storages: HashMap>, /// Cache optimized dynamic [`BundleId`] with single component dynamic_component_bundle_ids: HashMap, @@ -2056,14 +2060,14 @@ impl Bundles { /// Registers a new [`BundleInfo`] for a statically known type. /// /// Also registers all the components in the bundle. - pub(crate) fn register_info( + pub(crate) fn register_static_info( &mut self, components: &mut ComponentsRegistrator, storages: &mut Storages, ) -> BundleId { let bundle_infos = &mut self.bundle_infos; *self.static_bundle_ids.entry(TypeId::of::()).or_insert_with(|| { - let mut component_ids= Vec::new(); + let mut component_ids = Vec::new(); T::component_ids(components, &mut |id| component_ids.push(id)); let id = BundleId(bundle_infos.len()); let bundle_info = @@ -2077,6 +2081,64 @@ impl Bundles { }) } + pub(crate) fn register_info( + &mut self, + bundle: &T, + components: &mut ComponentsRegistrator, + storages: &mut Storages, + ) -> BundleId { + let bundle_infos = &mut self.bundle_infos; + if T::IS_STATIC { + return *self + .static_bundle_ids + .entry(TypeId::of::()) + .or_insert_with(|| { + Self::register_new_bundle(bundle, components, storages, bundle_infos) + }); + } + + if T::IS_BOUNDED { + let (key, size) = bundle.cache_key(); + if size <= 64 { + return *self + .bounded_bundle_ids + .entry((TypeId::of::(), key)) + .or_insert_with(|| { + Self::register_new_bundle(bundle, components, storages, bundle_infos) + }); + } + } + + let mut component_ids = Vec::new(); + bundle.component_ids(components, &mut |id| component_ids.push(id)); + + *self + .dynamic_bundle_ids + .entry(component_ids.into_boxed_slice()) + .or_insert_with(|| { + Self::register_new_bundle(bundle, components, storages, bundle_infos) + }) + } + + fn register_new_bundle( + bundle: &T, + components: &mut ComponentsRegistrator, + storages: &mut Storages, + bundle_infos: &mut Vec, + ) -> BundleId { + let mut component_ids = Vec::new(); + bundle.component_ids(components, &mut |id| component_ids.push(id)); + let id = BundleId(bundle_infos.len()); + let bundle_info = + // SAFETY: T::component_ids ensures: + // - its info was created + // - appropriate storage for it has been initialized. + // - it was created in the same order as the components in T + unsafe { BundleInfo::new(core::any::type_name::(), storages, components, component_ids, id) }; + bundle_infos.push(bundle_info); + id + } + /// Registers a new [`BundleInfo`], which contains both explicit and required components for a statically known type. /// /// Also registers all the components in the bundle. @@ -2088,7 +2150,7 @@ impl Bundles { if let Some(id) = self.contributed_bundle_ids.get(&TypeId::of::()).cloned() { id } else { - let explicit_bundle_id = self.register_info::(components, storages); + let explicit_bundle_id = self.register_static_info::(components, storages); // SAFETY: reading from `explicit_bundle_id` and creating new bundle in same time. Its valid because bundle hashmap allow this let id = unsafe { let (ptr, len) = { diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 53fd88bdf6639..7314964f6e78f 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -2158,7 +2158,7 @@ impl<'w> EntityWorldMut<'w> { let retained_bundle = self .world .bundles - .register_info::(&mut registrator, storages); + .register_static_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_archetype = &mut archetypes[old_location.archetype_id]; diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 36bbf4ce1d5df..d008872ec3658 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2284,7 +2284,7 @@ impl World { unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) }; let bundle_id = self .bundles - .register_info::(&mut registrator, &mut self.storages); + .register_static_info::(&mut registrator, &mut self.storages); let mut batch_iter = batch.into_iter(); @@ -2429,7 +2429,7 @@ impl World { unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) }; let bundle_id = self .bundles - .register_info::(&mut registrator, &mut self.storages); + .register_static_info::(&mut registrator, &mut self.storages); let mut invalid_entities = Vec::::new(); let mut batch_iter = batch.into_iter(); @@ -3009,7 +3009,7 @@ impl World { unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) }; let id = self .bundles - .register_info::(&mut registrator, &mut self.storages); + .register_static_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 cce68066c4c97b598a0b9e64149cf98b61ea6f68 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 18:39:07 +0200 Subject: [PATCH 17/44] Convert BundleInserter to using the new register_info --- crates/bevy_ecs/src/bundle.rs | 19 +++++++++++-------- crates/bevy_ecs/src/world/entity_ref.rs | 2 +- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 29239ec16080f..d0da301ac9514 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1164,6 +1164,7 @@ pub(crate) enum ArchetypeMoveType { impl<'w> BundleInserter<'w> { #[inline] pub(crate) fn new( + bundle: &T, world: &'w mut World, archetype_id: ArchetypeId, change_tick: Tick, @@ -1173,7 +1174,7 @@ impl<'w> BundleInserter<'w> { unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) }; let bundle_id = world .bundles - .register_static_info::(&mut registrator, &mut world.storages); + .register_info(bundle, &mut registrator, &mut world.storages); // SAFETY: We just ensured this bundle exists unsafe { Self::new_with_id(world, archetype_id, bundle_id, change_tick) } } @@ -2010,17 +2011,18 @@ impl<'w> BundleSpawner<'w> { #[derive(Default)] pub struct Bundles { bundle_infos: Vec, + /// Cache [`BundleId`]s for static bundles static_bundle_ids: TypeIdMap, /// Cache [`BundleId`]s for bounded but non-static bundles bounded_bundle_ids: HashMap<(TypeId, u64), BundleId>, /// Cache [`BundleId`]s for dynamic bundles dynamic_bundle_ids: HashMap, BundleId>, + dynamic_bundle_storages: HashMap>, /// Cache bundles, which contains both explicit and required components of [`Bundle`] contributed_bundle_ids: TypeIdMap, - dynamic_bundle_storages: HashMap>, /// Cache optimized dynamic [`BundleId`] with single component dynamic_component_bundle_ids: HashMap, dynamic_component_storages: HashMap, @@ -2088,6 +2090,8 @@ impl Bundles { storages: &mut Storages, ) -> BundleId { let bundle_infos = &mut self.bundle_infos; + + // Fastest case, we have a static bundle if T::IS_STATIC { return *self .static_bundle_ids @@ -2097,6 +2101,8 @@ impl Bundles { }); } + // Optimized case for bounded bundles, e.g. those with conditional components whose + // key fits in 64 bits. if T::IS_BOUNDED { let (key, size) = bundle.cache_key(); if size <= 64 { @@ -2109,15 +2115,12 @@ impl Bundles { } } + // Fallback to registering a dynamic bundle + let mut component_ids = Vec::new(); bundle.component_ids(components, &mut |id| component_ids.push(id)); - *self - .dynamic_bundle_ids - .entry(component_ids.into_boxed_slice()) - .or_insert_with(|| { - Self::register_new_bundle(bundle, components, storages, bundle_infos) - }) + self.init_dynamic_info(storages, components, &component_ids) } fn register_new_bundle( diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 7314964f6e78f..f03b0f8a008c5 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1836,7 +1836,7 @@ impl<'w> EntityWorldMut<'w> { let location = self.location(); let change_tick = self.world.change_tick(); let mut bundle_inserter = - BundleInserter::new::(self.world, location.archetype_id, change_tick); + BundleInserter::new::(&bundle, self.world, location.archetype_id, change_tick); // SAFETY: location matches current entity. `T` matches `bundle_info` let (location, after_effect) = unsafe { bundle_inserter.insert( From 1dfd29c0cb92f142d8ad60973c1a3b577b2ffb70 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 18:53:40 +0200 Subject: [PATCH 18/44] Rename World::register_bundle to World::register_static_bundle --- benches/benches/bevy_ecs/entity_cloning.rs | 5 ++++- crates/bevy_ecs/src/entity/clone_entities.rs | 4 ++-- crates/bevy_ecs/src/world/mod.rs | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/benches/benches/bevy_ecs/entity_cloning.rs b/benches/benches/bevy_ecs/entity_cloning.rs index 0eaae27ce4b00..1ec72160ca31e 100644 --- a/benches/benches/bevy_ecs/entity_cloning.rs +++ b/benches/benches/bevy_ecs/entity_cloning.rs @@ -69,7 +69,10 @@ fn reflection_cloner( // Recursively register all components in the bundle, then save the component IDs to a list. // This uses `contributed_components()`, meaning both explicit and required component IDs in // this bundle are saved. - let component_ids: Vec<_> = world.register_bundle::().contributed_components().into(); + let component_ids: Vec<_> = world + .register_static_bundle::() + .contributed_components() + .into(); let mut builder = EntityCloner::build(world); diff --git a/crates/bevy_ecs/src/entity/clone_entities.rs b/crates/bevy_ecs/src/entity/clone_entities.rs index fbcd10927dd0e..7f1ed7290c243 100644 --- a/crates/bevy_ecs/src/entity/clone_entities.rs +++ b/crates/bevy_ecs/src/entity/clone_entities.rs @@ -680,7 +680,7 @@ impl<'w> EntityClonerBuilder<'w> { /// Note that all components are allowed by default, to clone only explicitly allowed components make sure to call /// [`deny_all`](`Self::deny_all`) before calling any of the `allow` methods. pub fn allow(&mut self) -> &mut Self { - let bundle = self.world.register_bundle::(); + let bundle = self.world.register_static_bundle::(); let ids = bundle.explicit_components().to_owned(); for id in ids { self.filter_allow(id); @@ -721,7 +721,7 @@ impl<'w> EntityClonerBuilder<'w> { /// Disallows all components of the bundle from being cloned. pub fn deny(&mut self) -> &mut Self { - let bundle = self.world.register_bundle::(); + let bundle = self.world.register_static_bundle::(); let ids = bundle.explicit_components().to_owned(); for id in ids { self.filter_deny(id); diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index d008872ec3658..c85f63964cad6 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -3003,7 +3003,7 @@ impl World { /// This is largely equivalent to calling [`register_component`](Self::register_component) on each /// component in the bundle. #[inline] - pub fn register_bundle(&mut self) -> &BundleInfo { + pub fn register_static_bundle(&mut self) -> &BundleInfo { // 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) }; From 1931130297e12c1e28de5033dece0656f84136f3 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 18:56:55 +0200 Subject: [PATCH 19/44] Rename register_contributed_bundle_info to register_static_contributed_bundle_info --- crates/bevy_ecs/src/bundle.rs | 2 +- crates/bevy_ecs/src/world/entity_ref.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index d0da301ac9514..f96d7a6751a4f 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -2145,7 +2145,7 @@ impl Bundles { /// Registers a new [`BundleInfo`], which contains both explicit and required components for a statically known type. /// /// Also registers all the components in the bundle. - pub(crate) fn register_contributed_bundle_info( + pub(crate) fn register_static_contributed_bundle_info( &mut self, components: &mut ComponentsRegistrator, storages: &mut Storages, diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index f03b0f8a008c5..8fe8f67b07da3 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -2105,7 +2105,8 @@ impl<'w> EntityWorldMut<'w> { let mut registrator = unsafe { ComponentsRegistrator::new(&mut self.world.components, &mut self.world.component_ids) }; - let bundle_id = bundles.register_contributed_bundle_info::(&mut registrator, storages); + let bundle_id = + bundles.register_static_contributed_bundle_info::(&mut registrator, storages); // SAFETY: We just created the bundle, and the archetype is valid, since we are in it. let Some(mut remover) = (unsafe { From 26d8fc85fb6e2a5afee85737c8c6a511ead64177 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 20:28:50 +0200 Subject: [PATCH 20/44] Add missing safety comments --- crates/bevy_ecs/src/spawn.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/spawn.rs b/crates/bevy_ecs/src/spawn.rs index 2822489477c9d..5ded15162d338 100644 --- a/crates/bevy_ecs/src/spawn.rs +++ b/crates/bevy_ecs/src/spawn.rs @@ -184,7 +184,7 @@ impl> BundleEffect for SpawnRelatedBundle + Send + Sync + 'static> StaticBundle for SpawnRelatedBundle { @@ -212,6 +212,8 @@ unsafe impl + Send + Sync + 'static> Static ); } } + +// SAFETY: This internally relies on the RelationshipTarget's Bundle implementation, which is sound. unsafe impl + Send + Sync + 'static> Bundle for SpawnRelatedBundle { @@ -281,7 +283,7 @@ impl DynamicBundle for SpawnOneRelated { } } -// SAFETY: This internally relies on the RelationshipTarget's Bundle implementation, which is sound. +// SAFETY: This internally relies on the RelationshipTarget's StaticBundle implementation, which is sound. unsafe impl StaticBundle for SpawnOneRelated { fn component_ids( components: &mut crate::component::ComponentsRegistrator, @@ -307,6 +309,8 @@ unsafe impl StaticBundle for SpawnOneRelated { ); } } + +// SAFETY: This internally relies on the RelationshipTarget's Bundle implementation, which is sound. unsafe impl Bundle for SpawnOneRelated { // We are inserting `R::RelationshipTarget`, which is a `Component` and thus // always a `StaticBundle`. From 787c16803e12eaa5ceccd16839b703ef6a523051 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 21:05:59 +0200 Subject: [PATCH 21/44] No longer require Bundle: StaticBundle --- crates/bevy_ecs/src/bundle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index f96d7a6751a4f..2aa9334171391 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -146,7 +146,7 @@ use variadics_please::all_tuples; label = "invalid `Bundle`", note = "consider annotating `{Self}` with `#[derive(Component)]` or `#[derive(Bundle)]`" )] -pub unsafe trait Bundle: StaticBundle + DynamicBundle + Send + Sync + 'static { +pub unsafe trait Bundle: DynamicBundle + Send + Sync + 'static { /// Whether this is a [`StaticBundle`] or not. In case this is a [`StaticBundle`] the associated /// [`BundleId`] and [`BundleInfo`] are known to be unique and can be cached by this type's [`TypeId`]. /// From a2ab6f45493ca2baa279fec478f4ec5291d69d95 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 21:24:10 +0200 Subject: [PATCH 22/44] Improve error message for an invalid dynamic Bundle --- crates/bevy_ecs/src/bundle.rs | 22 ++++++++++++++++++---- crates/bevy_ecs/src/world/entity_ref.rs | 13 +++++++++---- crates/bevy_ecs/src/world/mod.rs | 9 ++++++--- 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 2aa9334171391..30c58c5aabf0a 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -630,7 +630,7 @@ impl BundleInfo { /// Every ID in `component_ids` must be valid within the World that owns the `BundleInfo` /// and must be in the same order as the source bundle type writes its components in. unsafe fn new( - bundle_type_name: &'static str, + bundle_type_name: &str, storages: &mut Storages, components: &Components, mut component_ids: Vec, @@ -2120,7 +2120,12 @@ impl Bundles { let mut component_ids = Vec::new(); bundle.component_ids(components, &mut |id| component_ids.push(id)); - self.init_dynamic_info(storages, components, &component_ids) + self.init_dynamic_info( + core::any::type_name::(), + storages, + components, + &component_ids, + ) } fn register_new_bundle( @@ -2165,7 +2170,12 @@ impl Bundles { }; // SAFETY: this is sound because the contributed_components Vec for explicit_bundle_id will not be accessed mutably as // part of init_dynamic_info. No mutable references will be created and the allocation will remain valid. - self.init_dynamic_info(storages, components, core::slice::from_raw_parts(ptr, len)) + self.init_dynamic_info( + "", + storages, + components, + core::slice::from_raw_parts(ptr, len), + ) }; self.contributed_bundle_ids.insert(TypeId::of::(), id); id @@ -2203,6 +2213,7 @@ impl Bundles { /// provided [`Components`]. pub(crate) fn init_dynamic_info( &mut self, + name: &str, storages: &mut Storages, components: &Components, component_ids: &[ComponentId], @@ -2216,6 +2227,7 @@ impl Bundles { .from_key(component_ids) .or_insert_with(|| { let (id, storages) = initialize_dynamic_bundle( + name, bundle_infos, storages, components, @@ -2248,6 +2260,7 @@ impl Bundles { .entry(component_id) .or_insert_with(|| { let (id, storage_type) = initialize_dynamic_bundle( + "", bundle_infos, storages, components, @@ -2263,6 +2276,7 @@ impl Bundles { /// Asserts that all components are part of [`Components`] /// and initializes a [`BundleInfo`]. fn initialize_dynamic_bundle( + bundle_name: &str, bundle_infos: &mut Vec, storages: &mut Storages, components: &Components, @@ -2280,7 +2294,7 @@ fn initialize_dynamic_bundle( let id = BundleId(bundle_infos.len()); let bundle_info = // SAFETY: `component_ids` are valid as they were just checked - unsafe { BundleInfo::new("", storages, components, component_ids, id) }; + unsafe { BundleInfo::new(bundle_name, storages, components, component_ids, id) }; bundle_infos.push(bundle_info); (id, storage_types) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 8fe8f67b07da3..b7bdc191990f0 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1959,6 +1959,7 @@ impl<'w> EntityWorldMut<'w> { let location = self.location(); let change_tick = self.world.change_tick(); let bundle_id = self.world.bundles.init_dynamic_info( + "", &mut self.world.storages, &self.world.components, component_ids, @@ -2169,10 +2170,12 @@ impl<'w> EntityWorldMut<'w> { .components() .filter(|c| !retained_bundle_info.contributed_components().contains(c)) .collect::>(); - let remove_bundle = - self.world - .bundles - .init_dynamic_info(&mut self.world.storages, ®istrator, to_remove); + let remove_bundle = self.world.bundles.init_dynamic_info( + "", + &mut self.world.storages, + ®istrator, + to_remove, + ); // SAFETY: We just created the bundle, and the archetype is valid, since we are in it. let Some(mut remover) = (unsafe { @@ -2262,6 +2265,7 @@ impl<'w> EntityWorldMut<'w> { let components = &mut self.world.components; let bundle_id = self.world.bundles.init_dynamic_info( + "", &mut self.world.storages, components, component_ids, @@ -2307,6 +2311,7 @@ impl<'w> EntityWorldMut<'w> { let components = &mut self.world.components; let bundle_id = self.world.bundles.init_dynamic_info( + "", &mut self.world.storages, components, component_ids.as_slice(), diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index c85f63964cad6..c34aef879b2f5 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -3026,9 +3026,12 @@ impl World { /// This function will panic if any of the provided component ids do not belong to a component known to this [`World`]. #[inline] pub fn register_dynamic_bundle(&mut self, component_ids: &[ComponentId]) -> &BundleInfo { - let id = - self.bundles - .init_dynamic_info(&mut self.storages, &self.components, component_ids); + let id = self.bundles.init_dynamic_info( + "", + &mut self.storages, + &self.components, + component_ids, + ); // SAFETY: We just initialized the bundle so its id should definitely be valid. unsafe { self.bundles.get(id).debug_checked_unwrap() } } From d39bee26391c22fa74c0b29b58de815a0edfd204 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 2 Jun 2025 21:38:48 +0200 Subject: [PATCH 23/44] WIP Implement Bundle for Option --- crates/bevy_ecs/src/bundle.rs | 59 +++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 30c58c5aabf0a..e71b4cdf9ff40 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -2315,6 +2315,65 @@ fn sorted_remove(source: &mut Vec, remove: &[T]) { }); } +// SAFETY: TODO +unsafe impl Bundle for Option { + const IS_STATIC: bool = false; + const IS_BOUNDED: bool = true; + + fn cache_key(&self) -> (u64, usize) { + if let Some(this) = self { + let (key, len) = this.cache_key(); + if len >= 64 { + return (0, 65); + } + + // key ends in 0 + (key << 1, len + 1) + } else { + // key ends in 1 + (1, 1) + } + } + + fn component_ids( + &self, + components: &mut ComponentsRegistrator, + ids: &mut impl FnMut(ComponentId), + ) { + if let Some(this) = self { + this.component_ids(components, ids); + } + } + + fn register_required_components( + &self, + components: &mut ComponentsRegistrator, + required_components: &mut RequiredComponents, + ) { + if let Some(this) = self { + this.register_required_components(components, required_components); + } + } +} + +impl DynamicBundle for Option { + type Effect = Option; + + fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) -> Self::Effect { + self.map(|this| this.get_components(func)) + } +} + +impl BundleEffect for Option { + fn apply(self, entity: &mut EntityWorldMut) { + if let Some(this) = self { + this.apply(entity); + } + } +} + +impl NoBundleEffect for Option {} + #[cfg(test)] mod tests { use crate::{ From 3167fc3e6928e69b65da2b7a09b98a4cc49de4e0 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Tue, 3 Jun 2025 20:47:06 +0200 Subject: [PATCH 24/44] Simplify Bundle macro implementation --- crates/bevy_ecs/macros/src/lib.rs | 110 ++++++++---------------------- 1 file changed, 30 insertions(+), 80 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 1e67d5b683168..24c43934dafa7 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -15,7 +15,7 @@ use crate::{ use bevy_macro_utils::{derive_label, ensure_no_collision, get_struct_fields, BevyManifest}; use proc_macro::TokenStream; use proc_macro2::{Ident, Span}; -use quote::{format_ident, quote}; +use quote::{format_ident, quote, ToTokens}; use syn::{ parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned, token::Comma, ConstParam, Data, DataStruct, DeriveInput, GenericParam, Index, TypeParam, @@ -74,15 +74,8 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { .map(|field| &field.ty) .collect::>(); - let mut field_static_component_ids = Vec::new(); - let mut field_static_required_components = Vec::new(); - let mut field_static_get_component_ids = Vec::new(); - let mut field_component_ids = Vec::new(); - let mut field_required_components = Vec::new(); - let mut field_get_components = Vec::new(); - let mut field_is_static = Vec::new(); - let mut field_is_bounded = Vec::new(); - let mut field_cache_key = Vec::new(); + let mut active_field_types = Vec::new(); + let mut active_field_tokens = Vec::new(); let mut field_from_components = Vec::new(); for (((i, field_type), field_kind), field) in field_type .iter() @@ -92,68 +85,17 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { { match field_kind { BundleFieldKind::Component => { - field_static_component_ids.push(quote! { - <#field_type as #ecs_path::bundle::StaticBundle>::component_ids(components, &mut *ids); - }); - field_static_required_components.push(quote! { - <#field_type as #ecs_path::bundle::StaticBundle>::register_required_components(components, required_components); - }); - field_static_get_component_ids.push(quote! { - <#field_type as #ecs_path::bundle::StaticBundle>::get_component_ids(components, &mut *ids); + let field_tokens = match field { + Some(field) => field.to_token_stream(), + None => Index::from(i).to_token_stream(), + }; + + field_from_components.push(quote! { + #field_tokens: <#field_type as #ecs_path::bundle::BundleFromComponents>::from_components(ctx, &mut *func), }); - field_is_static - .push(quote! { <#field_type as #ecs_path::bundle::Bundle>::IS_STATIC }); - field_is_bounded - .push(quote! { <#field_type as #ecs_path::bundle::Bundle>::IS_BOUNDED }); - match field { - Some(field) => { - field_component_ids.push(quote! { - <#field_type as #ecs_path::bundle::Bundle>::component_ids(&self.#field, components, ids); - }); - field_required_components.push(quote! { - <#field_type as #ecs_path::bundle::Bundle>::register_required_components(&self.#field, components, required_components); - }); - field_get_components.push(quote! { - <#field_type as #ecs_path::bundle::DynamicBundle>::get_components(self.#field, &mut *func); - }); - field_from_components.push(quote! { - #field: <#field_type as #ecs_path::bundle::BundleFromComponents>::from_components(ctx, &mut *func), - }); - field_cache_key.push(quote! { - let (sub_key, sub_size) = <#field_type as #ecs_path::bundle::Bundle>::cache_key(&self.#field); - key |= sub_key << size; - size += sub_size; - // Bail out if size is too big - if size > 64 { - return (0, 65); - } - }); - } - None => { - let index = Index::from(i); - field_component_ids.push(quote! { - <#field_type as #ecs_path::bundle::Bundle>::component_ids(&self.#index, components, ids); - }); - field_required_components.push(quote! { - <#field_type as #ecs_path::bundle::Bundle>::register_required_components(&self.#index, components, required_components); - }); - field_get_components.push(quote! { - <#field_type as #ecs_path::bundle::DynamicBundle>::get_components(self.#index, &mut *func); - }); - field_from_components.push(quote! { - #index: <#field_type as #ecs_path::bundle::BundleFromComponents>::from_components(ctx, &mut *func), - }); - field_cache_key.push(quote! { - let (sub_key, sub_size) = <#field_type as #ecs_path::bundle::Bundle>::cache_key(&self.#index); - key |= sub_key << size; - size += sub_size; - // Bail out if size is too big - if size > 64 { - return (0, 65); - } - }); - } - } + + active_field_types.push(field_type); + active_field_tokens.push(field_tokens); } BundleFieldKind::Ignore => { @@ -178,34 +120,42 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { components: &mut #ecs_path::component::ComponentsRegistrator, ids: &mut impl FnMut(#ecs_path::component::ComponentId) ){ - #(#field_static_component_ids)* + #(<#active_field_types as #ecs_path::bundle::StaticBundle>::component_ids(components, &mut *ids);)* } fn get_component_ids( components: &#ecs_path::component::Components, ids: &mut impl FnMut(Option<#ecs_path::component::ComponentId>) ){ - #(#field_static_get_component_ids)* + #(<#active_field_types as #ecs_path::bundle::StaticBundle>::get_component_ids(components, &mut *ids);)* } fn register_required_components( components: &mut #ecs_path::component::ComponentsRegistrator, required_components: &mut #ecs_path::component::RequiredComponents ){ - #(#field_static_required_components)* + #(<#active_field_types as #ecs_path::bundle::StaticBundle>::register_required_components(components, &mut *required_components);)* } } // SAFETY: see the corresponding implementation of `StaticBundle` unsafe impl #impl_generics #ecs_path::bundle::Bundle for #struct_name #ty_generics #where_clause { - const IS_STATIC: bool = true #(&& #field_is_static)*; - const IS_BOUNDED: bool = true #(&& #field_is_bounded)*; + const IS_STATIC: bool = true #(&& <#active_field_types as #ecs_path::bundle::Bundle>::IS_STATIC)*; + const IS_BOUNDED: bool = true #(&& <#active_field_types as #ecs_path::bundle::Bundle>::IS_BOUNDED)*; fn cache_key(&self) -> (u64, usize) { let mut key = 0; let mut size = 0; - #(#field_cache_key)* + #( + let (sub_key, sub_size) = <#active_field_types as #ecs_path::bundle::Bundle>::cache_key(&self.#active_field_tokens); + key |= sub_key << size; + size += sub_size; + // Bail out if size is too big + if size > 64 { + return (0, 65); + } + )* (key, size) } @@ -215,7 +165,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { components: &mut #ecs_path::component::ComponentsRegistrator, ids: &mut impl FnMut(#ecs_path::component::ComponentId), ) { - #(#field_component_ids)* + #(<#active_field_types as #ecs_path::bundle::Bundle>::component_ids(&self.#active_field_tokens, components, ids);)* } fn register_required_components( @@ -223,7 +173,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { components: &mut #ecs_path::component::ComponentsRegistrator, required_components: &mut #ecs_path::component::RequiredComponents, ) { - #(#field_required_components)* + #(<#active_field_types as #ecs_path::bundle::Bundle>::register_required_components(&self.#active_field_tokens, components, required_components);)* } } @@ -251,7 +201,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { self, func: &mut impl FnMut(#ecs_path::component::StorageType, #ecs_path::ptr::OwningPtr<'_>) ) { - #(#field_get_components)* + #(<#active_field_types as #ecs_path::bundle::DynamicBundle>::get_components(self.#active_field_tokens, &mut *func);)* } } }) From 2e6997c891130989ccb08cbe58e04908fcc2fb9f Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Tue, 3 Jun 2025 21:19:52 +0200 Subject: [PATCH 25/44] Fix bug when #[bundle(ignore)] is followed by another one --- crates/bevy_ecs/macros/src/lib.rs | 18 ++++++++++-------- crates/bevy_ecs/src/lib.rs | 7 +++++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 24c43934dafa7..039e63872ecad 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -42,6 +42,8 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { let mut field_kind = Vec::with_capacity(named_fields.len()); for field in named_fields { + let mut kind = BundleFieldKind::Component; + for attr in field .attrs .iter() @@ -49,7 +51,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { { if let Err(error) = attr.parse_nested_meta(|meta| { if meta.path.is_ident(BUNDLE_ATTRIBUTE_IGNORE_NAME) { - field_kind.push(BundleFieldKind::Ignore); + kind = BundleFieldKind::Ignore; Ok(()) } else { Err(meta.error(format!( @@ -61,7 +63,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { } } - field_kind.push(BundleFieldKind::Component); + field_kind.push(kind); } let field = named_fields @@ -83,13 +85,13 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { .zip(field_kind.iter()) .zip(field.iter()) { + let field_tokens = match field { + Some(field) => field.to_token_stream(), + None => Index::from(i).to_token_stream(), + }; + match field_kind { BundleFieldKind::Component => { - let field_tokens = match field { - Some(field) => field.to_token_stream(), - None => Index::from(i).to_token_stream(), - }; - field_from_components.push(quote! { #field_tokens: <#field_type as #ecs_path::bundle::BundleFromComponents>::from_components(ctx, &mut *func), }); @@ -100,7 +102,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { BundleFieldKind::Ignore => { field_from_components.push(quote! { - #field: ::core::default::Default::default(), + #field_tokens: ::core::default::Default::default(), }); } } diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 3692418d7a0d5..8d38caa170480 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -2730,6 +2730,13 @@ mod tests { field1: ComponentB, } + #[expect( + dead_code, + reason = "This struct is used as a compilation test to test the derive macros, and as such is intentionally never constructed." + )] + #[derive(Bundle)] + struct IgnoredFields(#[bundle(ignore)] i32, #[bundle(ignore)] i32); + #[derive(Component)] struct MyEntities { #[entities] From cded294a338c517357dd2dbde03462cc4241f3b5 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Tue, 3 Jun 2025 21:26:05 +0200 Subject: [PATCH 26/44] Add support for #[bundle(dynamic)] attribute --- crates/bevy_ecs/macros/src/lib.rs | 36 ++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 039e63872ecad..be3255e383929 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -27,6 +27,7 @@ enum BundleFieldKind { } const BUNDLE_ATTRIBUTE_NAME: &str = "bundle"; +const BUNDLE_ATTRIBUTE_DYNAMIC: &str = "dynamic"; const BUNDLE_ATTRIBUTE_IGNORE_NAME: &str = "ignore"; #[proc_macro_derive(Bundle, attributes(bundle))] @@ -34,6 +35,26 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { let ast = parse_macro_input!(input as DeriveInput); let ecs_path = bevy_ecs_path(); + let mut is_dynamic = false; + for attr in ast + .attrs + .iter() + .filter(|a| a.path().is_ident(BUNDLE_ATTRIBUTE_NAME)) + { + if let Err(error) = attr.parse_nested_meta(|meta| { + if meta.path.is_ident(BUNDLE_ATTRIBUTE_DYNAMIC) { + is_dynamic = true; + Ok(()) + } else { + Err(meta.error(format!( + "Invalid bundle attribute. Use `{BUNDLE_ATTRIBUTE_DYNAMIC}`" + ))) + } + }) { + return error.into_compile_error().into(); + } + } + let named_fields = match get_struct_fields(&ast.data, "derive(Bundle)") { Ok(fields) => fields, Err(e) => return e.into_compile_error().into(), @@ -111,7 +132,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); let struct_name = &ast.ident; - TokenStream::from(quote! { + let static_bundle_impl = (!is_dynamic).then(|| quote! { // SAFETY: // - ComponentId is returned in field-definition-order. [get_components] uses field-definition-order // - `Bundle::get_components` is exactly once for each member. Rely's on the Component -> Bundle implementation to properly pass @@ -139,7 +160,9 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { #(<#active_field_types as #ecs_path::bundle::StaticBundle>::register_required_components(components, &mut *required_components);)* } } + }); + let bundle_impl = quote! { // SAFETY: see the corresponding implementation of `StaticBundle` unsafe impl #impl_generics #ecs_path::bundle::Bundle for #struct_name #ty_generics #where_clause { const IS_STATIC: bool = true #(&& <#active_field_types as #ecs_path::bundle::Bundle>::IS_STATIC)*; @@ -178,7 +201,9 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { #(<#active_field_types as #ecs_path::bundle::Bundle>::register_required_components(&self.#active_field_tokens, components, required_components);)* } } + }; + let bundle_from_components_impl = (!is_dynamic).then(|| quote! { // SAFETY: // - ComponentId is returned in field-definition-order. [from_components] uses field-definition-order #[allow(deprecated)] @@ -193,7 +218,9 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { } } } + }); + let dynamic_bundle_impl = quote! { #[allow(deprecated)] impl #impl_generics #ecs_path::bundle::DynamicBundle for #struct_name #ty_generics #where_clause { type Effect = (); @@ -206,6 +233,13 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { #(<#active_field_types as #ecs_path::bundle::DynamicBundle>::get_components(self.#active_field_tokens, &mut *func);)* } } + }; + + TokenStream::from(quote! { + #static_bundle_impl + #bundle_impl + #bundle_from_components_impl + #dynamic_bundle_impl }) } From 1b0b20c8a1c4bd215b04a3184422346404079f25 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Tue, 3 Jun 2025 21:27:22 +0200 Subject: [PATCH 27/44] Add simple test for Option bundles --- crates/bevy_ecs/src/bundle.rs | 42 +++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index e71b4cdf9ff40..cce561b5d4a3e 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -2634,4 +2634,46 @@ mod tests { assert_eq!(world.resource::().0, 3); } + + #[derive(Bundle)] + #[bundle(dynamic)] + struct WithOptionalField { + a: A, + b: Option, + v: Option, + } + + #[test] + fn optional_bundle() { + let mut w = World::new(); + + let e = w.spawn(None::); + assert!(!e.contains::()); + + let e = w.spawn(Some(A)); + assert!(e.contains::()); + + let e = w.spawn((None::, Some(V("Some")))); + assert!(!e.contains::()); + assert!(e.contains::()); + assert!(e.get::() == Some(&V("Some"))); + + let e = w.spawn((Some(B), Some(V("Some2")))); + assert!(e.contains::()); + assert!(e.contains::()); + assert!(e.get::() == Some(&V("Some2"))); + + let e = w.spawn((Some(B), None::)); + assert!(e.contains::()); + assert!(!e.contains::()); + + let e = w.spawn(WithOptionalField { + a: A, + b: None, + v: Some(V("V")), + }); + assert!(e.contains::()); + assert!(!e.contains::()); + assert!(e.get::() == Some(&V("V"))); + } } From 22eb8a316a6d2d22855a965a7a3c24d5986ed42d Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Wed, 4 Jun 2025 18:31:55 +0200 Subject: [PATCH 28/44] Switch IS_STATIC and IS_BOUNDED to associated functions --- crates/bevy_ecs/macros/src/lib.rs | 8 ++++++-- crates/bevy_ecs/src/bundle.rs | 32 +++++++++++++++++++++---------- crates/bevy_ecs/src/spawn.rs | 16 ++++++++++++---- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index be3255e383929..6cdf94d246bca 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -165,8 +165,12 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { let bundle_impl = quote! { // SAFETY: see the corresponding implementation of `StaticBundle` unsafe impl #impl_generics #ecs_path::bundle::Bundle for #struct_name #ty_generics #where_clause { - const IS_STATIC: bool = true #(&& <#active_field_types as #ecs_path::bundle::Bundle>::IS_STATIC)*; - const IS_BOUNDED: bool = true #(&& <#active_field_types as #ecs_path::bundle::Bundle>::IS_BOUNDED)*; + fn is_static() -> bool { + true #(&& <#active_field_types as #ecs_path::bundle::Bundle>::is_static())* + } + fn is_bounded() -> bool { + true #(&& <#active_field_types as #ecs_path::bundle::Bundle>::is_bounded())* + } fn cache_key(&self) -> (u64, usize) { let mut key = 0; diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index cce561b5d4a3e..80623fb6be629 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -152,7 +152,7 @@ pub unsafe trait Bundle: DynamicBundle + Send + Sync + 'static { /// /// This is a hack to work around the lack of specialization. #[doc(hidden)] - const IS_STATIC: bool; + fn is_static() -> bool; /// Whether the set of components this bundle includes is bounded or not. In case it is bounded we /// can produce an additional cache key based on which components from the bounded set are included in @@ -161,7 +161,7 @@ pub unsafe trait Bundle: DynamicBundle + Send + Sync + 'static { /// /// This is a hack to work around the lack of specialization. #[doc(hidden)] - const IS_BOUNDED: bool; + fn is_bounded() -> bool; /// Computes the cache key associated with `self` and its size. The key is limited to 64 bits, and if the /// size exceeds that it should be ignored. @@ -310,8 +310,12 @@ unsafe impl StaticBundle for C { // SAFETY: see the corresponding implementation of `StaticBundle` unsafe impl Bundle for C { - const IS_STATIC: bool = true; - const IS_BOUNDED: bool = true; + fn is_static() -> bool { + true + } + fn is_bounded() -> bool { + true + } fn cache_key(&self) -> (u64, usize) { (0, 0) @@ -404,8 +408,12 @@ macro_rules! tuple_impl { $(#[$meta])* // SAFETY: see the corresponding implementation of `StaticBundle` unsafe impl<$($name: Bundle),*> Bundle for ($($name,)*) { - const IS_STATIC: bool = true $(&& <$name as Bundle>::IS_STATIC)*; - const IS_BOUNDED: bool = true $(&& <$name as Bundle>::IS_STATIC)*; + fn is_static() -> bool { + true $(&& <$name as Bundle>::is_static())* + } + fn is_bounded() -> bool { + true $(&& <$name as Bundle>::is_bounded())* + } fn cache_key(&self) -> (u64, usize) { #[allow( @@ -2092,7 +2100,7 @@ impl Bundles { let bundle_infos = &mut self.bundle_infos; // Fastest case, we have a static bundle - if T::IS_STATIC { + if T::is_static() { return *self .static_bundle_ids .entry(TypeId::of::()) @@ -2103,7 +2111,7 @@ impl Bundles { // Optimized case for bounded bundles, e.g. those with conditional components whose // key fits in 64 bits. - if T::IS_BOUNDED { + if T::is_bounded() { let (key, size) = bundle.cache_key(); if size <= 64 { return *self @@ -2317,8 +2325,12 @@ fn sorted_remove(source: &mut Vec, remove: &[T]) { // SAFETY: TODO unsafe impl Bundle for Option { - const IS_STATIC: bool = false; - const IS_BOUNDED: bool = true; + fn is_static() -> bool { + false + } + fn is_bounded() -> bool { + true + } fn cache_key(&self) -> (u64, usize) { if let Some(this) = self { diff --git a/crates/bevy_ecs/src/spawn.rs b/crates/bevy_ecs/src/spawn.rs index 5ded15162d338..f7a02664078f5 100644 --- a/crates/bevy_ecs/src/spawn.rs +++ b/crates/bevy_ecs/src/spawn.rs @@ -219,8 +219,12 @@ unsafe impl + Send + Sync + 'static> Bundle { // We are inserting `R::RelationshipTarget`, which is a `Component` and thus // always a `StaticBundle`. - const IS_STATIC: bool = ::IS_STATIC; - const IS_BOUNDED: bool = ::IS_BOUNDED; + fn is_static() -> bool { + ::is_static() + } + fn is_bounded() -> bool { + ::is_bounded() + } fn cache_key(&self) -> (u64, usize) { (0, 0) @@ -314,8 +318,12 @@ unsafe impl StaticBundle for SpawnOneRelated { unsafe impl Bundle for SpawnOneRelated { // We are inserting `R::RelationshipTarget`, which is a `Component` and thus // always a `StaticBundle`. - const IS_STATIC: bool = ::IS_STATIC; - const IS_BOUNDED: bool = ::IS_BOUNDED; + fn is_static() -> bool { + ::is_static() + } + fn is_bounded() -> bool { + ::is_bounded() + } fn cache_key(&self) -> (u64, usize) { (0, 0) From fa493ce4c496a7c4aa2de84f52d09f31abde9525 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Wed, 4 Jun 2025 18:33:10 +0200 Subject: [PATCH 29/44] Rename DynamicBundle to ComponentsFromBundle --- crates/bevy_ecs/macros/src/lib.rs | 4 ++-- crates/bevy_ecs/src/bundle.rs | 28 ++++++++++++------------- crates/bevy_ecs/src/spawn.rs | 8 +++---- crates/bevy_ecs/src/world/entity_ref.rs | 6 +++--- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 6cdf94d246bca..3f92966832121 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -226,7 +226,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { let dynamic_bundle_impl = quote! { #[allow(deprecated)] - impl #impl_generics #ecs_path::bundle::DynamicBundle for #struct_name #ty_generics #where_clause { + impl #impl_generics #ecs_path::bundle::ComponentsFromBundle for #struct_name #ty_generics #where_clause { type Effect = (); #[allow(unused_variables)] #[inline] @@ -234,7 +234,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { self, func: &mut impl FnMut(#ecs_path::component::StorageType, #ecs_path::ptr::OwningPtr<'_>) ) { - #(<#active_field_types as #ecs_path::bundle::DynamicBundle>::get_components(self.#active_field_tokens, &mut *func);)* + #(<#active_field_types as #ecs_path::bundle::ComponentsFromBundle>::get_components(self.#active_field_tokens, &mut *func);)* } } }; diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 80623fb6be629..861c75ccd9760 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -146,7 +146,7 @@ use variadics_please::all_tuples; label = "invalid `Bundle`", note = "consider annotating `{Self}` with `#[derive(Component)]` or `#[derive(Bundle)]`" )] -pub unsafe trait Bundle: DynamicBundle + Send + Sync + 'static { +pub unsafe trait Bundle: ComponentsFromBundle + Send + Sync + 'static { /// Whether this is a [`StaticBundle`] or not. In case this is a [`StaticBundle`] the associated /// [`BundleId`] and [`BundleInfo`] are known to be unique and can be cached by this type's [`TypeId`]. /// @@ -200,7 +200,7 @@ pub unsafe trait Bundle: DynamicBundle + Send + Sync + 'static { /// If you want a type to implement [`StaticBundle`], you must use [`derive@Bundle`](derive@Bundle). // Some safety points: // - [`Bundle::component_ids`] must return the [`ComponentId`] for each component type in the -// bundle, in the _exact_ order that [`DynamicBundle::get_components`] is called. +// bundle, in the _exact_ order that [`ComponentsFromBundle::get_components`] is called. // - [`Bundle::from_components`] must call `func` exactly once for each [`ComponentId`] returned by // [`Bundle::component_ids`]. #[diagnostic::on_unimplemented( @@ -236,7 +236,7 @@ pub unsafe trait StaticBundle: Send + Sync + 'static { /// [`Query`]: crate::system::Query // Some safety points: // - [`Bundle::component_ids`] must return the [`ComponentId`] for each component type in the -// bundle, in the _exact_ order that [`DynamicBundle::get_components`] is called. +// bundle, in the _exact_ order that [`ComponentsFromBundle::get_components`] is called. // - [`Bundle::from_components`] must call `func` exactly once for each [`ComponentId`] returned by // [`Bundle::component_ids`]. pub unsafe trait BundleFromComponents { @@ -255,7 +255,7 @@ pub unsafe trait BundleFromComponents { } /// The parts from [`Bundle`] that don't require statically knowing the components of the bundle. -pub trait DynamicBundle { +pub trait ComponentsFromBundle { /// An operation on the entity that happens _after_ inserting this bundle. type Effect: BundleEffect; // SAFETY: @@ -275,7 +275,7 @@ pub trait DynamicBundle { /// 2. Relevant Hooks are run for the insert, then Observers /// 3. The [`BundleEffect`] is run. /// -/// See [`DynamicBundle::Effect`]. +/// See [`ComponentsFromBundle::Effect`]. pub trait BundleEffect { /// Applies this effect to the given `entity`. fn apply(self, entity: &mut EntityWorldMut); @@ -353,7 +353,7 @@ unsafe impl BundleFromComponents for C { } } -impl DynamicBundle for C { +impl ComponentsFromBundle for C { type Effect = (); #[inline] fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) -> Self::Effect { @@ -375,7 +375,7 @@ macro_rules! tuple_impl { $(#[$meta])* // SAFETY: // - `StaticBundle::component_ids` calls `ids` for each component type in the - // bundle, in the exact order that `DynamicBundle::get_components` is called. + // bundle, in the exact order that `ComponentsFromBundle::get_components` is called. // - `StaticBundle::from_components` calls `func` exactly once for each `ComponentId` returned by `Bundle::component_ids`. // - `StaticBundle::get_components` is called exactly once for each member. Relies on the above implementation to pass the correct // `StorageType` into the callback. @@ -482,7 +482,7 @@ macro_rules! tuple_impl { $(#[$meta])* // SAFETY: // - `Bundle::component_ids` calls `ids` for each component type in the - // bundle, in the exact order that `DynamicBundle::get_components` is called. + // bundle, in the exact order that `ComponentsFromBundle::get_components` is called. // - `Bundle::from_components` calls `func` exactly once for each `ComponentId` returned by `Bundle::component_ids`. // - `Bundle::get_components` is called exactly once for each member. Relies on the above implementation to pass the correct // `StorageType` into the callback. @@ -515,7 +515,7 @@ macro_rules! tuple_impl { reason = "Zero-length tuples won't use any of the parameters." )] $(#[$meta])* - impl<$($name: Bundle),*> DynamicBundle for ($($name,)*) { + impl<$($name: Bundle),*> ComponentsFromBundle for ($($name,)*) { type Effect = ($($name::Effect,)*); #[allow( clippy::unused_unit, @@ -545,7 +545,7 @@ all_tuples!( ); /// A trait implemented for [`BundleEffect`] implementations that do nothing. This is used as a type constraint for -/// [`Bundle`] APIs that do not / cannot run [`DynamicBundle::Effect`], such as "batch spawn" APIs. +/// [`Bundle`] APIs that do not / cannot run [`ComponentsFromBundle::Effect`], such as "batch spawn" APIs. pub trait NoBundleEffect {} macro_rules! after_effect_impl { @@ -778,7 +778,7 @@ impl BundleInfo { /// `table` must be the "new" table for `entity`. `table_row` must have space allocated for the /// `entity`, `bundle` must match this [`BundleInfo`]'s type #[inline] - unsafe fn write_components<'a, T: DynamicBundle, S: BundleComponentStatus>( + unsafe fn write_components<'a, T: ComponentsFromBundle, S: BundleComponentStatus>( &self, table: &mut Table, sparse_sets: &mut SparseSets, @@ -1284,7 +1284,7 @@ impl<'w> BundleInserter<'w> { /// `entity` must currently exist in the source archetype for this inserter. `location` /// must be `entity`'s location in the archetype. `T` must match this [`BundleInfo`]'s type #[inline] - pub(crate) unsafe fn insert( + pub(crate) unsafe fn insert( &mut self, entity: Entity, location: EntityLocation, @@ -1909,7 +1909,7 @@ impl<'w> BundleSpawner<'w> { /// `entity` must be allocated (but non-existent), `T` must match this [`BundleInfo`]'s type #[inline] #[track_caller] - pub unsafe fn spawn_non_existent( + pub unsafe fn spawn_non_existent( &mut self, entity: Entity, bundle: T, @@ -2368,7 +2368,7 @@ unsafe impl Bundle for Option { } } -impl DynamicBundle for Option { +impl ComponentsFromBundle for Option { type Effect = Option; fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) -> Self::Effect { diff --git a/crates/bevy_ecs/src/spawn.rs b/crates/bevy_ecs/src/spawn.rs index f7a02664078f5..2d48481bcbc77 100644 --- a/crates/bevy_ecs/src/spawn.rs +++ b/crates/bevy_ecs/src/spawn.rs @@ -2,7 +2,7 @@ //! for the best entry points into these APIs and examples of how to use them. use crate::{ - bundle::{Bundle, BundleEffect, DynamicBundle, NoBundleEffect, StaticBundle}, + bundle::{Bundle, BundleEffect, ComponentsFromBundle, NoBundleEffect, StaticBundle}, entity::Entity, relationship::{RelatedSpawner, Relationship, RelationshipTarget}, world::{EntityWorldMut, World}, @@ -246,7 +246,7 @@ unsafe impl + Send + Sync + 'static> Bundle ::register_required_components(components, required_components); } } -impl> DynamicBundle for SpawnRelatedBundle { +impl> ComponentsFromBundle for SpawnRelatedBundle { type Effect = Self; fn get_components( @@ -275,7 +275,7 @@ impl BundleEffect for SpawnOneRelated { } } -impl DynamicBundle for SpawnOneRelated { +impl ComponentsFromBundle for SpawnOneRelated { type Effect = Self; fn get_components( @@ -346,7 +346,7 @@ unsafe impl Bundle for SpawnOneRelated { } } -/// [`RelationshipTarget`] methods that create a [`Bundle`] with a [`DynamicBundle::Effect`] that: +/// [`RelationshipTarget`] methods that create a [`Bundle`] with a [`ComponentsFromBundle::Effect`] that: /// /// 1. Contains the [`RelationshipTarget`] component, pre-allocated with the necessary space for spawned entities. /// 2. Spawns an entity (or a list of entities) that relate to the entity the [`Bundle`] is added to via the [`RelationshipTarget::Relationship`]. diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index b7bdc191990f0..1072fab9b1252 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1,8 +1,8 @@ use crate::{ archetype::Archetype, bundle::{ - Bundle, BundleEffect, BundleFromComponents, BundleInserter, BundleRemover, DynamicBundle, - InsertMode, StaticBundle, + Bundle, BundleEffect, BundleFromComponents, BundleInserter, BundleRemover, + ComponentsFromBundle, InsertMode, StaticBundle, }, change_detection::{MaybeLocation, MutUntyped}, component::{ @@ -4344,7 +4344,7 @@ unsafe fn insert_dynamic_bundle< components: I, } - impl<'a, I: Iterator)>> DynamicBundle + impl<'a, I: Iterator)>> ComponentsFromBundle for DynamicInsertBundle<'a, I> { type Effect = (); From f64aa7ae15880dbcd275824e1feef3a0d85f2250 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Wed, 4 Jun 2025 18:42:39 +0200 Subject: [PATCH 30/44] (WIP) Implement Bundle for Box --- crates/bevy_ecs/src/bundle.rs | 112 +++++++++++++++++++++++++++++++--- 1 file changed, 105 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 861c75ccd9760..809717ec2c5c8 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -146,13 +146,15 @@ use variadics_please::all_tuples; label = "invalid `Bundle`", note = "consider annotating `{Self}` with `#[derive(Component)]` or `#[derive(Bundle)]`" )] -pub unsafe trait Bundle: ComponentsFromBundle + Send + Sync + 'static { +pub unsafe trait Bundle: BundleDyn + ComponentsFromBundle + Send + Sync + 'static { /// Whether this is a [`StaticBundle`] or not. In case this is a [`StaticBundle`] the associated /// [`BundleId`] and [`BundleInfo`] are known to be unique and can be cached by this type's [`TypeId`]. /// /// This is a hack to work around the lack of specialization. #[doc(hidden)] - fn is_static() -> bool; + fn is_static() -> bool + where + Self: Sized; /// Whether the set of components this bundle includes is bounded or not. In case it is bounded we /// can produce an additional cache key based on which components from the bounded set are included in @@ -161,7 +163,9 @@ pub unsafe trait Bundle: ComponentsFromBundle + Send + Sync + 'static { /// /// This is a hack to work around the lack of specialization. #[doc(hidden)] - fn is_bounded() -> bool; + fn is_bounded() -> bool + where + Self: Sized; /// Computes the cache key associated with `self` and its size. The key is limited to 64 bits, and if the /// size exceeds that it should be ignored. @@ -174,7 +178,8 @@ pub unsafe trait Bundle: ComponentsFromBundle + Send + Sync + 'static { &self, components: &mut ComponentsRegistrator, ids: &mut impl FnMut(ComponentId), - ); + ) where + Self: Sized; /// Registers components that are required by the components in this [`Bundle`]. #[doc(hidden)] @@ -257,7 +262,9 @@ pub unsafe trait BundleFromComponents { /// The parts from [`Bundle`] that don't require statically knowing the components of the bundle. pub trait ComponentsFromBundle { /// An operation on the entity that happens _after_ inserting this bundle. - type Effect: BundleEffect; + type Effect: BundleEffect + where + Self: Sized; // SAFETY: // The `StorageType` argument passed into [`Bundle::get_components`] must be correct for the // component being fetched. @@ -265,7 +272,80 @@ pub trait ComponentsFromBundle { /// Calls `func` on each value, in the order of this bundle's [`Component`]s. This passes /// ownership of the component values to `func`. #[doc(hidden)] - fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) -> Self::Effect; + fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) -> Self::Effect + where + Self: Sized; +} + +/// TODO +pub trait BundleDyn { + #[doc(hidden)] + fn component_ids_dyn( + &self, + components: &mut ComponentsRegistrator, + ids: &mut dyn FnMut(ComponentId), + ); + + #[doc(hidden)] + fn get_components_dyn( + self: Box, + func: &mut dyn FnMut(StorageType, OwningPtr<'_>), + ) -> Box; +} + +impl BundleDyn for B { + fn component_ids_dyn( + &self, + components: &mut ComponentsRegistrator, + ids: &mut dyn FnMut(ComponentId), + ) { + self.component_ids(components, &mut |id| ids(id)) + } + + fn get_components_dyn( + self: Box, + func: &mut dyn FnMut(StorageType, OwningPtr<'_>), + ) -> Box { + Box::new(self.get_components(&mut |storage_type, ptr| func(storage_type, ptr))) + } +} + +unsafe impl Bundle for Box { + fn is_static() -> bool { + false + } + + fn is_bounded() -> bool { + false + } + + fn cache_key(&self) -> (u64, usize) { + (0, 0) + } + + fn component_ids( + &self, + components: &mut ComponentsRegistrator, + ids: &mut impl FnMut(ComponentId), + ) { + (**self).component_ids_dyn(components, ids) + } + + fn register_required_components( + &self, + components: &mut ComponentsRegistrator, + required_components: &mut RequiredComponents, + ) { + (**self).register_required_components(components, required_components); + } +} + +impl ComponentsFromBundle for Box { + type Effect = Box; + + fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) -> Self::Effect { + self.get_components_dyn(func) + } } /// An operation on an [`Entity`] that occurs _after_ inserting the [`Bundle`] that defined this bundle effect. @@ -276,11 +356,29 @@ pub trait ComponentsFromBundle { /// 3. The [`BundleEffect`] is run. /// /// See [`ComponentsFromBundle::Effect`]. -pub trait BundleEffect { +pub trait BundleEffect: BundleEffectDyn { /// Applies this effect to the given `entity`. fn apply(self, entity: &mut EntityWorldMut); } +/// TODO +pub trait BundleEffectDyn { + #[doc(hidden)] + fn apply_dyn(self: Box, entity: &mut EntityWorldMut); +} + +impl BundleEffectDyn for E { + fn apply_dyn(self: Box, entity: &mut EntityWorldMut) { + self.apply(entity) + } +} + +impl BundleEffect for Box { + fn apply(self, entity: &mut EntityWorldMut) { + self.apply_dyn(entity) + } +} + // SAFETY: // - `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. From a662e60a2d40efb21c48b2d43e6dfcdc16555f74 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Wed, 4 Jun 2025 19:14:16 +0200 Subject: [PATCH 31/44] Fix formatting & doc issues --- crates/bevy_ecs/src/bundle.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 809717ec2c5c8..19a0fc41e7073 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -299,7 +299,7 @@ impl BundleDyn for B { components: &mut ComponentsRegistrator, ids: &mut dyn FnMut(ComponentId), ) { - self.component_ids(components, &mut |id| ids(id)) + self.component_ids(components, &mut |id| ids(id)); } fn get_components_dyn( @@ -310,6 +310,7 @@ impl BundleDyn for B { } } +// Safety: TODO unsafe impl Bundle for Box { fn is_static() -> bool { false @@ -328,7 +329,7 @@ unsafe impl Bundle for Box { components: &mut ComponentsRegistrator, ids: &mut impl FnMut(ComponentId), ) { - (**self).component_ids_dyn(components, ids) + (**self).component_ids_dyn(components, ids); } fn register_required_components( @@ -369,13 +370,13 @@ pub trait BundleEffectDyn { impl BundleEffectDyn for E { fn apply_dyn(self: Box, entity: &mut EntityWorldMut) { - self.apply(entity) + self.apply(entity); } } impl BundleEffect for Box { fn apply(self, entity: &mut EntityWorldMut) { - self.apply_dyn(entity) + self.apply_dyn(entity); } } From 0fddef07fa31001db7826c9524d7c981778eeeb9 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Wed, 4 Jun 2025 20:02:09 +0200 Subject: [PATCH 32/44] Fix other CI issues --- benches/benches/bevy_ecs/entity_cloning.rs | 8 ++++---- benches/benches/bevy_ecs/world/world_get.rs | 6 ++++-- crates/bevy_ecs/src/world/mod.rs | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/benches/benches/bevy_ecs/entity_cloning.rs b/benches/benches/bevy_ecs/entity_cloning.rs index 1ec72160ca31e..cebf2a81acc7e 100644 --- a/benches/benches/bevy_ecs/entity_cloning.rs +++ b/benches/benches/bevy_ecs/entity_cloning.rs @@ -1,7 +1,7 @@ use core::hint::black_box; use benches::bench; -use bevy_ecs::bundle::Bundle; +use bevy_ecs::bundle::{Bundle, StaticBundle}; use bevy_ecs::component::ComponentCloneBehavior; use bevy_ecs::entity::EntityCloner; use bevy_ecs::hierarchy::ChildOf; @@ -53,7 +53,7 @@ type ComplexBundle = (C1, C2, C3, C4, C5, C6, C7, C8, C9, C10); /// Sets the [`ComponentCloneHandler`] for all explicit and required components in a bundle `B` to /// use the [`Reflect`] trait instead of [`Clone`]. -fn reflection_cloner( +fn reflection_cloner( world: &mut World, linked_cloning: bool, ) -> EntityCloner { @@ -95,7 +95,7 @@ fn reflection_cloner( /// components (which is usually [`ComponentCloneHandler::clone_handler()`]). If `clone_via_reflect` /// is true, it will overwrite the handler for all components in the bundle to be /// [`ComponentCloneHandler::reflect_handler()`]. -fn bench_clone( +fn bench_clone( b: &mut Bencher, clone_via_reflect: bool, ) { @@ -127,7 +127,7 @@ fn bench_clone( /// For example, setting `height` to 5 and `children` to 1 creates a single chain of entities with /// no siblings. Alternatively, setting `height` to 1 and `children` to 5 will spawn 5 direct /// children of the root entity. -fn bench_clone_hierarchy( +fn bench_clone_hierarchy( b: &mut Bencher, height: usize, children: usize, diff --git a/benches/benches/bevy_ecs/world/world_get.rs b/benches/benches/bevy_ecs/world/world_get.rs index e6e2a0bb903ef..23f1dd9b9271c 100644 --- a/benches/benches/bevy_ecs/world/world_get.rs +++ b/benches/benches/bevy_ecs/world/world_get.rs @@ -2,7 +2,7 @@ use core::hint::black_box; use nonmax::NonMaxU32; use bevy_ecs::{ - bundle::{Bundle, NoBundleEffect}, + bundle::{Bundle, NoBundleEffect, StaticBundle}, component::Component, entity::{Entity, EntityRow}, system::{Query, SystemState}, @@ -37,7 +37,9 @@ fn setup(entity_count: u32) -> World { black_box(world) } -fn setup_wide + Default>(entity_count: u32) -> World { +fn setup_wide + StaticBundle + Default>( + entity_count: u32, +) -> World { let mut world = World::default(); world.spawn_batch((0..entity_count).map(|_| T::default())); black_box(world) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index c34aef879b2f5..06ad76a9c3acd 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -3019,7 +3019,7 @@ impl World { /// Note that the components need to be registered first, this function only creates a bundle combining them. Components /// can be registered with [`World::register_component`]/[`_with_descriptor`](World::register_component_with_descriptor). /// - /// **You should prefer to use the typed API [`World::register_bundle`] where possible and only use this in cases where + /// **You should prefer to use the typed API [`World::register_static_bundle`] where possible and only use this in cases where /// not all of the actual types are known at compile time.** /// /// # Panics From 086c12df72fedc692977845b762226eee7efd453 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Wed, 4 Jun 2025 22:18:18 +0200 Subject: [PATCH 33/44] Improve docs and safety comments --- crates/bevy_ecs/macros/src/lib.rs | 21 +++- crates/bevy_ecs/src/bundle.rs | 154 ++++++++++++++++-------- crates/bevy_ecs/src/spawn.rs | 5 +- crates/bevy_ecs/src/world/entity_ref.rs | 2 +- 4 files changed, 125 insertions(+), 57 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 3f92966832121..99eb03ddf1a67 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -134,9 +134,10 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { let static_bundle_impl = (!is_dynamic).then(|| quote! { // SAFETY: - // - ComponentId is returned in field-definition-order. [get_components] uses field-definition-order - // - `Bundle::get_components` is exactly once for each member. Rely's on the Component -> Bundle implementation to properly pass - // the correct `StorageType` into the callback. + // - all the active fields must implement `StaticBundle` for the function bodies to compile, and hence + // this bundle also represents a static set of components; + // - `component_ids` and `get_component_ids` delegate to the underlying implementation in the same order + // and hence are coherent; #[allow(deprecated)] unsafe impl #impl_generics #ecs_path::bundle::StaticBundle for #struct_name #ty_generics #where_clause { fn component_ids( @@ -163,7 +164,14 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { }); let bundle_impl = quote! { - // SAFETY: see the corresponding implementation of `StaticBundle` + // SAFETY: + // - if all sub-bundles are static then this bundle is static too; + // - if all sub-bundles are bounded by some sets then this bundle is bounded by their union; + // - `cache_key` ORs together the keys of the sub-bundles in order to compute a combined key; since + // no bits is overlapped and the original keys were associated to unique combinations, this new key + // is also associated with an unique combination; + // - `component_ids` calls `ids` for each component type in the bundle, in the exact order that + // `get_components` is called. unsafe impl #impl_generics #ecs_path::bundle::Bundle for #struct_name #ty_generics #where_clause { fn is_static() -> bool { true #(&& <#active_field_types as #ecs_path::bundle::Bundle>::is_static())* @@ -180,7 +188,10 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { let (sub_key, sub_size) = <#active_field_types as #ecs_path::bundle::Bundle>::cache_key(&self.#active_field_tokens); key |= sub_key << size; size += sub_size; - // Bail out if size is too big + // Bail out if size is too big, this avoids overflow errors when shifting + // left by the size. + // Returning anything with a size bigger than 64 is fine because that + // signals that the key could not be compute. if size > 64 { return (0, 65); } diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 19a0fc41e7073..a7c002491ebe2 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -32,11 +32,12 @@ use bevy_utils::TypeIdMap; use core::{any::TypeId, ptr::NonNull}; use variadics_please::all_tuples; -/// The `Bundle` trait enables insertion and removal of [`Component`]s from an entity. +/// The `Bundle` trait enables insertion of [`Component`]s to an entity. +/// For the removal of [`Component`]s from an entity see the [`StaticBundle`]`trait`. /// /// Implementers of the `Bundle` trait are called 'bundles'. /// -/// Each bundle represents a static set of [`Component`] types. +/// Each bundle represents a possibly dynamic set of [`Component`] types. /// Currently, bundles can only contain one of each [`Component`], and will /// panic once initialized if this is not met. /// @@ -66,15 +67,6 @@ use variadics_please::all_tuples; /// contains the components of a bundle. /// Queries should instead only select the components they logically operate on. /// -/// ## Removal -/// -/// Bundles are also used when removing components from an entity. -/// -/// Removing a bundle from an entity will remove any of its components attached -/// to the entity from the entity. -/// That is, if the entity does not have all the components of the bundle, those -/// which are present will be removed. -/// /// # Implementers /// /// Every type which implements [`Component`] also implements `Bundle`, since @@ -97,7 +89,23 @@ use variadics_please::all_tuples; /// implement `Bundle`. /// As explained above, this includes any [`Component`] type, and other derived bundles. /// +/// [`Option`]s containing bundles also implement `Bundle`, and will insert the contained value if they +/// are `Some`, otherwise they will insert nothing. +/// +/// Fully ynamic bundles (`Box`) are also supported, and will insert the components held +/// by the underlying concrete bundle type. +/// +/// # Deriving `Bundle` +/// +/// The `Bundle`, [`StaticBundle`] and [`BundleFromComponents`] traits can be automatically implemented +/// for structs holding other bundles by using the [`derive@Bundle`] derive macro. +/// +/// In case some of your contained bundles are dynamic, like `Option` bundles and `Box`, +/// you can use the `#[bundle(dynamic)]` attribute to opt out of deriving [`StaticBundle`] and +/// [`BundleFromComponents`]. +/// /// If you want to add `PhantomData` to your `Bundle` you have to mark it with `#[bundle(ignore)]`. +/// /// ``` /// # use std::marker::PhantomData; /// use bevy_ecs::{component::Component, bundle::Bundle}; @@ -114,6 +122,18 @@ use variadics_please::all_tuples; /// y: YPosition, /// } /// +/// #[derive(Bundle)] +/// #[bundle(dynamic)] +/// struct DynamicBundle { +/// // A bundle needs to use #[bundle(dynamic)] +/// name: Option, +/// // or if it contains fully dynamic bundles +/// extra: Box, +/// +/// x: XPosition, +/// y: YPosition, +/// } +/// /// // You have to implement `Default` for ignored field types in bundle structs. /// #[derive(Default)] /// struct Other(f32); @@ -141,13 +161,24 @@ use variadics_please::all_tuples; /// If you want a type to implement [`Bundle`], you must use [`derive@Bundle`](derive@Bundle). /// /// [`Query`]: crate::system::Query +// Some safety points: +// - [`Bundle::is_static`] must be pure and return `true` only if the set of components contained +// in this bundle type is fixed and always the same for every instance; +// - [`Bundle::is_bounded`] must be pure and return `true` only if the set of components contained +// in this bundle type's instances is a subset of a statically known set of components; +// - [`Bundle::cache_key`] must be pure and return an unique key for each combination of components +// contained in this bundle's instance; +// - [`Bundle::component_ids`] must return the [`ComponentId`] for each component type in the +// bundle, in the _exact_ order that [`ComponentsFromBundle::get_components`] is called. +// - [`ComponentsFromBundle::get_components`] must call `func` exactly once for each [`ComponentId`] returned by +// [`Bundle::component_ids`]. #[diagnostic::on_unimplemented( message = "`{Self}` is not a `Bundle`", label = "invalid `Bundle`", note = "consider annotating `{Self}` with `#[derive(Component)]` or `#[derive(Bundle)]`" )] pub unsafe trait Bundle: BundleDyn + ComponentsFromBundle + Send + Sync + 'static { - /// Whether this is a [`StaticBundle`] or not. In case this is a [`StaticBundle`] the associated + /// Whether this is a static bundle or not. In case this is a static bundle the associated /// [`BundleId`] and [`BundleInfo`] are known to be unique and can be cached by this type's [`TypeId`]. /// /// This is a hack to work around the lack of specialization. @@ -190,13 +221,19 @@ pub unsafe trait Bundle: BundleDyn + ComponentsFromBundle + Send + Sync + 'stati ); } -/// Each `StaticBundle` represents a static set of [`Component`] types. -/// Currently, bundles can only contain one of each [`Component`], and will -/// panic once initialized if this is not met. +/// Each bundle represents a static and fixed set of [`Component`] types. +/// See the [`Bundle`] trait for a possibly dynamic set of [`Component`] types. +/// +/// Implementers of the `Bundle` trait are called 'static bundles'. +/// +/// ## Removal /// -/// Implementers of the `StaticBundle` trait are called 'static bundles'. +/// Static bundles are used when removing components from an entity. /// -/// See also the [`Bundle`] trait. +/// Removing a bundle from an entity will remove any of its components attached +/// to the entity from the entity. +/// That is, if the entity does not have all the components of the bundle, those +/// which are present will be removed. /// /// # Safety /// @@ -204,10 +241,7 @@ pub unsafe trait Bundle: BundleDyn + ComponentsFromBundle + Send + Sync + 'stati /// That is, there is no safe way to implement this trait, and you must not do so. /// If you want a type to implement [`StaticBundle`], you must use [`derive@Bundle`](derive@Bundle). // Some safety points: -// - [`Bundle::component_ids`] must return the [`ComponentId`] for each component type in the -// bundle, in the _exact_ order that [`ComponentsFromBundle::get_components`] is called. -// - [`Bundle::from_components`] must call `func` exactly once for each [`ComponentId`] returned by -// [`Bundle::component_ids`]. +// - [`StaticBundle::component_ids`] and [`StaticBundle::get_component_ids`] must match the behaviour of [`Bundle::component_ids`] #[diagnostic::on_unimplemented( message = "`{Self}` is not a `StaticBundle`", label = "invalid `StaticBundle`", @@ -230,21 +264,17 @@ pub unsafe trait StaticBundle: Send + Sync + 'static { ); } -/// Creates a [`Bundle`] by taking it from internal storage. +/// Creates a bundle by taking it from the internal storage. /// /// # Safety /// /// Manual implementations of this trait are unsupported. /// That is, there is no safe way to implement this trait, and you must not do so. /// If you want a type to implement [`Bundle`], you must use [`derive@Bundle`](derive@Bundle). -/// -/// [`Query`]: crate::system::Query // Some safety points: -// - [`Bundle::component_ids`] must return the [`ComponentId`] for each component type in the -// bundle, in the _exact_ order that [`ComponentsFromBundle::get_components`] is called. // - [`Bundle::from_components`] must call `func` exactly once for each [`ComponentId`] returned by // [`Bundle::component_ids`]. -pub unsafe trait BundleFromComponents { +pub unsafe trait BundleFromComponents: StaticBundle { /// Calls `func`, which should return data for each component in the bundle, in the order of /// this bundle's [`Component`]s /// @@ -277,8 +307,10 @@ pub trait ComponentsFromBundle { Self: Sized; } -/// TODO +/// The dyn-compatible version of [`Bundle`]. This is used to support `Box`. +#[doc(hidden)] pub trait BundleDyn { + /// This is the dyn-compatible version of [`Bundle::component_ids`]. #[doc(hidden)] fn component_ids_dyn( &self, @@ -286,6 +318,7 @@ pub trait BundleDyn { ids: &mut dyn FnMut(ComponentId), ); + /// This is the dyn-compatible version of [`ComponentsFromBundle::get_components`]. #[doc(hidden)] fn get_components_dyn( self: Box, @@ -310,7 +343,11 @@ impl BundleDyn for B { } } -// Safety: TODO +// Safety: +// - `is_static` and `is_bounded` are false because `Box` could represent any bundle at runtime; +// - `cache_key` is irrelevant because `is_bounded` is false; +// - `component_ids` and `get_components` are implemented coherently because `component_ids_dyn` and +// `get_components_dyn` are implemented in terms of the underlying bundle's `component_ids` and `get_components`. unsafe impl Bundle for Box { fn is_static() -> bool { false @@ -362,7 +399,8 @@ pub trait BundleEffect: BundleEffectDyn { fn apply(self, entity: &mut EntityWorldMut); } -/// TODO +/// The dyn-compatible version of [`BundleEffect`]. This is used to support `Box`. +#[doc(hidden)] pub trait BundleEffectDyn { #[doc(hidden)] fn apply_dyn(self: Box, entity: &mut EntityWorldMut); @@ -381,13 +419,17 @@ impl BundleEffect for Box { } // SAFETY: -// - `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. +// - `C` always represents the set of components containing just `C` +// - `component_ids` and `get_component_ids` both call `ids` just once for C's component id (and nothing else). unsafe impl StaticBundle for C { fn component_ids(components: &mut ComponentsRegistrator, ids: &mut impl FnMut(ComponentId)) { ids(components.register_component::()); } + fn get_component_ids(components: &Components, ids: &mut impl FnMut(Option)) { + ids(components.get_id(TypeId::of::())); + } + fn register_required_components( components: &mut ComponentsRegistrator, required_components: &mut RequiredComponents, @@ -401,13 +443,13 @@ unsafe impl StaticBundle for C { &mut Vec::new(), ); } - - fn get_component_ids(components: &Components, ids: &mut impl FnMut(Option)) { - ids(components.get_id(TypeId::of::())); - } } -// SAFETY: see the corresponding implementation of `StaticBundle` +// SAFETY: +// - C is a static bundle, hence both `is_static` and `is_bounded` are true; +// - `cache_key` doesn't matter because `is_static` is true; +// - `component_ids` calls `ids` for C's component id (and nothing else) +// - `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 is_static() -> bool { true @@ -473,11 +515,9 @@ macro_rules! tuple_impl { )] $(#[$meta])* // SAFETY: - // - `StaticBundle::component_ids` calls `ids` for each component type in the - // bundle, in the exact order that `ComponentsFromBundle::get_components` is called. - // - `StaticBundle::from_components` calls `func` exactly once for each `ComponentId` returned by `Bundle::component_ids`. - // - `StaticBundle::get_components` is called exactly once for each member. Relies on the above implementation to pass the correct - // `StorageType` into the callback. + // - all the sub-bundles are static, and hence their combination is static too; + // - `component_ids` and `get_component_ids` both delegate to the sub-bundle's methods + // exactly once per sub-bundle, hence they are coherent. unsafe impl<$($name: StaticBundle),*> StaticBundle for ($($name,)*) { fn component_ids(components: &mut ComponentsRegistrator, ids: &mut impl FnMut(ComponentId)){ $(<$name as StaticBundle>::component_ids(components, ids);)* @@ -505,7 +545,14 @@ macro_rules! tuple_impl { reason = "Zero-length tuples won't use any of the parameters." )] $(#[$meta])* - // SAFETY: see the corresponding implementation of `StaticBundle` + // SAFETY: + // - if all sub-bundles are static then this bundle is static too; + // - if all sub-bundles are bounded by some sets then this bundle is bounded by their union; + // - `cache_key` ORs together the keys of the sub-bundles in order to compute a combined key; since + // no bits is overlapped and the original keys were associated to unique combinations, this new key + // is also associated with an unique combination; + // - `component_ids` calls `ids` for each component type in the bundle, in the exact order that + // `get_components` is called. unsafe impl<$($name: Bundle),*> Bundle for ($($name,)*) { fn is_static() -> bool { true $(&& <$name as Bundle>::is_static())* @@ -526,12 +573,12 @@ macro_rules! tuple_impl { $( let (sub_key, sub_size) = $name.cache_key(); - key |= sub_key << size; - size += sub_size; - - // Bail out if size is too big + // Bail out if size is too big, this avoids overflow errors when shifting + // left by the size. + // Returning anything with a size bigger than 64 is fine because that + // signals that the key could not be compute. if size > 64 { return (0, 65); } @@ -2422,13 +2469,22 @@ fn sorted_remove(source: &mut Vec, remove: &[T]) { }); } -// SAFETY: TODO +// SAFETY: +// - `is_static` is false because `Option` could either include all the components in `T` +// or none of them +// - `is_bounded` is the same as the underlying `T`, since `Option` adds the possibility +// of having none of the components in the bounding set, but doesn't change the bound. +// - `cache_key` returns either the key of the underlying bundle with a 0 appended to the right +// or a 1, and hence are different for every combination. +// - `component_ids` either calls `ids` for all components in the underlying bundle or none at all; +// if it calls it it means `self` is `Some` and hence `get_components` will also call `func` for +// all of them in the same order. unsafe impl Bundle for Option { fn is_static() -> bool { false } fn is_bounded() -> bool { - true + T::is_bounded() } fn cache_key(&self) -> (u64, usize) { diff --git a/crates/bevy_ecs/src/spawn.rs b/crates/bevy_ecs/src/spawn.rs index 2d48481bcbc77..52993d6074c2d 100644 --- a/crates/bevy_ecs/src/spawn.rs +++ b/crates/bevy_ecs/src/spawn.rs @@ -316,8 +316,6 @@ unsafe impl StaticBundle for SpawnOneRelated { // SAFETY: This internally relies on the RelationshipTarget's Bundle implementation, which is sound. unsafe impl Bundle for SpawnOneRelated { - // We are inserting `R::RelationshipTarget`, which is a `Component` and thus - // always a `StaticBundle`. fn is_static() -> bool { ::is_static() } @@ -326,6 +324,9 @@ unsafe impl Bundle for SpawnOneRelated { } fn cache_key(&self) -> (u64, usize) { + // We are inserting `R::RelationshipTarget`, which is a `Component` and thus + // always a `StaticBundle`. However we don't have an instance yet, so we can't delegate + // cache_key to it. (0, 0) } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 1072fab9b1252..e514daa793446 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1995,7 +1995,7 @@ impl<'w> EntityWorldMut<'w> { /// If the entity has been despawned while this `EntityWorldMut` is still alive. #[must_use] #[track_caller] - pub fn take(&mut self) -> Option { + pub fn take(&mut self) -> Option { let location = self.location(); let entity = self.entity; From afdb8381e1aa7e10193030400ecbb96aa7859081 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Wed, 4 Jun 2025 22:20:23 +0200 Subject: [PATCH 34/44] Simplify Bundle derive implementation a bit more --- crates/bevy_ecs/macros/src/lib.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 99eb03ddf1a67..3c464e180d95d 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -99,7 +99,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { let mut active_field_types = Vec::new(); let mut active_field_tokens = Vec::new(); - let mut field_from_components = Vec::new(); + let mut inactive_field_tokens = Vec::new(); for (((i, field_type), field_kind), field) in field_type .iter() .enumerate() @@ -110,21 +110,13 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { Some(field) => field.to_token_stream(), None => Index::from(i).to_token_stream(), }; - match field_kind { BundleFieldKind::Component => { - field_from_components.push(quote! { - #field_tokens: <#field_type as #ecs_path::bundle::BundleFromComponents>::from_components(ctx, &mut *func), - }); - active_field_types.push(field_type); active_field_tokens.push(field_tokens); } - BundleFieldKind::Ignore => { - field_from_components.push(quote! { - #field_tokens: ::core::default::Default::default(), - }); + inactive_field_tokens.push(field_tokens); } } } @@ -229,7 +221,8 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { __F: FnMut(&mut __T) -> #ecs_path::ptr::OwningPtr<'_> { Self{ - #(#field_from_components)* + #(#active_field_tokens: <#active_field_types as #ecs_path::bundle::BundleFromComponents>::from_components(ctx, &mut *func),)* + #(#inactive_field_tokens: ::core::default::Default::default(),)* } } } From e308c1666e7f53e0dd087970820fbd5a77d2bf3a Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Wed, 4 Jun 2025 22:25:53 +0200 Subject: [PATCH 35/44] Fix typo --- crates/bevy_ecs/src/bundle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index a7c002491ebe2..fb72f2d4fe0b5 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -241,7 +241,7 @@ pub unsafe trait Bundle: BundleDyn + ComponentsFromBundle + Send + Sync + 'stati /// That is, there is no safe way to implement this trait, and you must not do so. /// If you want a type to implement [`StaticBundle`], you must use [`derive@Bundle`](derive@Bundle). // Some safety points: -// - [`StaticBundle::component_ids`] and [`StaticBundle::get_component_ids`] must match the behaviour of [`Bundle::component_ids`] +// - [`StaticBundle::component_ids`] and [`StaticBundle::get_component_ids`] must match the behavior of [`Bundle::component_ids`] #[diagnostic::on_unimplemented( message = "`{Self}` is not a `StaticBundle`", label = "invalid `StaticBundle`", From 41f3152a3138e700393e4bba29bc778af518bb9c Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Thu, 5 Jun 2025 17:56:24 +0200 Subject: [PATCH 36/44] Implement Bundle for Vec> --- crates/bevy_ecs/src/bundle.rs | 59 +++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index fb72f2d4fe0b5..67f3552817471 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -2541,6 +2541,65 @@ impl BundleEffect for Option { impl NoBundleEffect for Option {} +// SAFETY: +// - `is_static` and `is_bounded` ares false because a `Vec>` could contain +// any bundle at runtime; +// - `cache_key` is irrelevant because this bundle is not bounded; +// - `component_ids` calls `component_ids` for all bundles in self, and `get_components` calls +// `get_components` for the bundle in the same order. Each bundle individually guarantees it +// will call `component_ids` and `get_components` in a matching way. +unsafe impl Bundle for Vec> { + fn is_static() -> bool { + false + } + + fn is_bounded() -> bool { + false + } + + fn cache_key(&self) -> (u64, usize) { + (0, 0) + } + + fn component_ids( + &self, + components: &mut ComponentsRegistrator, + ids: &mut impl FnMut(ComponentId), + ) { + for bundle in self { + bundle.component_ids(components, ids); + } + } + + fn register_required_components( + &self, + components: &mut ComponentsRegistrator, + required_components: &mut RequiredComponents, + ) { + for bundle in self { + bundle.register_required_components(components, required_components); + } + } +} + +impl ComponentsFromBundle for Vec> { + type Effect = Vec>; + + fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) -> Self::Effect { + self.into_iter() + .map(|bundle| bundle.get_components(func)) + .collect() + } +} + +impl BundleEffect for Vec { + fn apply(self, entity: &mut EntityWorldMut) { + for effect in self { + effect.apply(entity); + } + } +} + #[cfg(test)] mod tests { use crate::{ From 1140ace40787a309d4fdb2a8b806de401f4a0094 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Thu, 5 Jun 2025 21:44:11 +0200 Subject: [PATCH 37/44] Add some tests --- crates/bevy_ecs/src/bundle.rs | 67 ++++++++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 67f3552817471..c0ac7967e808e 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -2605,7 +2605,7 @@ mod tests { use crate::{ archetype::ArchetypeCreated, component::HookContext, prelude::*, world::DeferredWorld, }; - use alloc::vec; + use alloc::{boxed::Box, vec}; #[derive(Component)] struct A; @@ -2902,4 +2902,69 @@ mod tests { assert!(!e.contains::()); assert!(e.get::() == Some(&V("V"))); } + + #[derive(Bundle)] + #[bundle(dynamic)] + struct WithBoxDynBundleField { + a: A, + b: Box, + c: Box, + } + + #[test] + fn dyn_bundle() { + let mut w = World::new(); + + let e = w.spawn(Box::new(A) as Box); + assert!(e.contains::()); + + let e = w.spawn(Box::new((A, B)) as Box); + assert!(e.contains::()); + assert!(e.contains::()); + + let e = w.spawn(( + Box::new(A) as Box, + Box::new(B) as Box, + )); + assert!(e.contains::()); + assert!(e.contains::()); + + let e = w.spawn(WithBoxDynBundleField { + a: A, + b: Box::new(B), + c: Box::new((C, D)), + }); + assert!(e.contains::()); + assert!(e.contains::()); + assert!(e.contains::()); + assert!(e.contains::()); + } + + #[test] + fn vec_dyn_bundle() { + let mut w = World::new(); + + let e = w.spawn(vec![Box::new(A) as Box]); + assert!(e.contains::()); + + let e = w.spawn(vec![Box::new(A) as Box, Box::new(B)]); + assert!(e.contains::()); + assert!(e.contains::()); + } + + #[test] + fn mixed_dynamic_bundles() { + let mut w = World::new(); + + let e = w.spawn(Some(vec![ + Box::new(A) as Box, + Box::new(None::), + Box::new(vec![Box::new(C) as Box, Box::new(Some(D))]), + ])); + + assert!(e.contains::()); + assert!(!e.contains::()); + assert!(e.contains::()); + assert!(e.contains::()); + } } From a2139bb3c0566a0b253fd7d47421d2f40e3e54c5 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Sat, 7 Jun 2025 12:33:39 +0200 Subject: [PATCH 38/44] Add benchmarks for bundles --- benches/benches/bevy_ecs/bundles/mod.rs | 61 +++++ .../benches/bevy_ecs/bundles/spawn_many.rs | 256 ++++++++++++++++++ .../bevy_ecs/bundles/spawn_many_zst.rs | 206 ++++++++++++++ .../benches/bevy_ecs/bundles/spawn_one_zst.rs | 79 ++++++ benches/benches/bevy_ecs/main.rs | 2 + 5 files changed, 604 insertions(+) create mode 100644 benches/benches/bevy_ecs/bundles/mod.rs create mode 100644 benches/benches/bevy_ecs/bundles/spawn_many.rs create mode 100644 benches/benches/bevy_ecs/bundles/spawn_many_zst.rs create mode 100644 benches/benches/bevy_ecs/bundles/spawn_one_zst.rs diff --git a/benches/benches/bevy_ecs/bundles/mod.rs b/benches/benches/bevy_ecs/bundles/mod.rs new file mode 100644 index 0000000000000..28dc97520f5ef --- /dev/null +++ b/benches/benches/bevy_ecs/bundles/mod.rs @@ -0,0 +1,61 @@ +use bevy_ecs::{ + bundle::{Bundle, ComponentsFromBundle}, + component::{ComponentId, ComponentsRegistrator, RequiredComponents, StorageType}, + ptr::OwningPtr, +}; +use criterion::criterion_group; + +mod spawn_many; +mod spawn_many_zst; +mod spawn_one_zst; + +criterion_group!( + benches, + spawn_one_zst::spawn_one_zst, + spawn_many_zst::spawn_many_zst, + spawn_many::spawn_many, +); + +struct MakeDynamic(B); + +// SAFETY: +// - setting is_static and is_bounded to false is always safe and makes cache_key irrelevant +// - everything else is delegated to B, which is a valid Bundle +unsafe impl Bundle for MakeDynamic { + fn is_static() -> bool { + false + } + + fn is_bounded() -> bool { + false + } + + fn cache_key(&self) -> (u64, usize) { + (0, 0) + } + + fn component_ids( + &self, + components: &mut ComponentsRegistrator, + ids: &mut impl FnMut(ComponentId), + ) { + self.0.component_ids(components, ids); + } + + fn register_required_components( + &self, + components: &mut ComponentsRegistrator, + required_components: &mut RequiredComponents, + ) { + self.0 + .register_required_components(components, required_components); + } +} + +impl ComponentsFromBundle for MakeDynamic { + type Effect = ::Effect; + + fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) -> Self::Effect { + self.0.get_components(func) + } +} diff --git a/benches/benches/bevy_ecs/bundles/spawn_many.rs b/benches/benches/bevy_ecs/bundles/spawn_many.rs new file mode 100644 index 0000000000000..d44569809d635 --- /dev/null +++ b/benches/benches/bevy_ecs/bundles/spawn_many.rs @@ -0,0 +1,256 @@ +use core::hint::black_box; + +use benches::bench; +use bevy_ecs::{bundle::Bundle, component::Component, world::World}; +use criterion::Criterion; + +use super::MakeDynamic; + +const ENTITY_COUNT: usize = 2_000; + +#[derive(Component)] +struct C(usize); + +#[derive(Component)] +struct W(T); + +pub fn spawn_many(criterion: &mut Criterion) { + let mut group = criterion.benchmark_group(bench!("spawn_many")); + + group.bench_function("static", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box(( + C::<0>(1), + C::<1>(1), + C::<2>(1), + C::<3>(1), + C::<4>(1), + C::<5>(1), + C::<6>(1), + C::<7>(1), + C::<8>(1), + C::<9>(1), + C::<10>(1), + C::<11>(1), + C::<12>(1), + C::<13>(1), + C::<14>(1), + ))); + } + }); + }); + + group.bench_function("option_some_many", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box(Some(( + C::<0>(1), + C::<1>(1), + C::<2>(1), + C::<3>(1), + C::<4>(1), + C::<5>(1), + C::<6>(1), + C::<7>(1), + C::<8>(1), + C::<9>(1), + C::<10>(1), + C::<11>(1), + C::<12>(1), + C::<13>(1), + C::<14>(1), + )))); + } + }); + }); + + group.bench_function("option_many_some", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box(( + Some(C::<0>(1)), + Some(C::<1>(1)), + Some(C::<2>(1)), + Some(C::<3>(1)), + Some(C::<4>(1)), + Some(C::<5>(1)), + Some(C::<6>(1)), + Some(C::<7>(1)), + Some(C::<8>(1)), + Some(C::<9>(1)), + Some(C::<10>(1)), + Some(C::<11>(1)), + Some(C::<12>(1)), + Some(C::<13>(1)), + Some(C::<14>(1)), + ))); + } + }); + }); + + group.bench_function("option_none_many", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box( + None::<( + C<0>, + C<1>, + C<2>, + C<3>, + C<4>, + C<5>, + C<6>, + C<7>, + C<8>, + C<9>, + C<10>, + C<11>, + C<12>, + C<13>, + C<14>, + )>, + )); + } + }); + }); + + group.bench_function("option_many_none_and_static", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box(( + (C::<0>(1), None::>>), + (C::<1>(1), None::>>), + (C::<2>(1), None::>>), + (C::<3>(1), None::>>), + (C::<4>(1), None::>>), + (C::<5>(1), None::>>), + (C::<6>(1), None::>>), + (C::<7>(1), None::>>), + (C::<8>(1), None::>>), + (C::<9>(1), None::>>), + (C::<10>(1), None::>>), + (C::<11>(1), None::>>), + (C::<12>(1), None::>>), + (C::<13>(1), None::>>), + (C::<14>(1), None::>>), + ))); + } + }); + }); + + group.bench_function("many_box_dyn_bundle", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + // Note: C is a ZST so Box::new is not actually allocating + // this is mostly measuring the overhead of dynamic bundles + // and dynamic dispatch. + world.spawn(black_box(( + Box::new(C::<0>(1)) as Box, + Box::new(C::<1>(1)) as Box, + Box::new(C::<2>(1)) as Box, + Box::new(C::<3>(1)) as Box, + Box::new(C::<4>(1)) as Box, + Box::new(C::<5>(1)) as Box, + Box::new(C::<6>(1)) as Box, + Box::new(C::<7>(1)) as Box, + Box::new(C::<8>(1)) as Box, + Box::new(C::<9>(1)) as Box, + Box::new(C::<10>(1)) as Box, + Box::new(C::<11>(1)) as Box, + Box::new(C::<12>(1)) as Box, + Box::new(C::<13>(1)) as Box, + Box::new(C::<14>(1)) as Box, + ))); + } + }); + }); + + group.bench_function("box_dyn_bundle_many", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + // Note: This will also count the cost of allocating a box + world.spawn(black_box(Box::new(( + C::<0>(1), + C::<1>(1), + C::<2>(1), + C::<3>(1), + C::<4>(1), + C::<5>(1), + C::<6>(1), + C::<7>(1), + C::<8>(1), + C::<9>(1), + C::<10>(1), + C::<11>(1), + C::<12>(1), + C::<13>(1), + C::<14>(1), + )) as Box)); + } + }); + }); + + group.bench_function("dynamic_bundle_many", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + // This has no other overhead than opting out of the static and bounded caching + // for the whole bundle + world.spawn(MakeDynamic(( + C::<0>(1), + C::<1>(1), + C::<2>(1), + C::<3>(1), + C::<4>(1), + C::<5>(1), + C::<6>(1), + C::<7>(1), + C::<8>(1), + C::<9>(1), + C::<10>(1), + C::<11>(1), + C::<12>(1), + C::<13>(1), + C::<14>(1), + ))); + } + }); + }); + + group.bench_function("many_dynamic_bundle", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + // This has no other overhead than opting out of the static and bounded caching + // for each sub-bundle + world.spawn(black_box(( + MakeDynamic(C::<0>(1)), + MakeDynamic(C::<1>(1)), + MakeDynamic(C::<2>(1)), + MakeDynamic(C::<3>(1)), + MakeDynamic(C::<4>(1)), + MakeDynamic(C::<5>(1)), + MakeDynamic(C::<6>(1)), + MakeDynamic(C::<7>(1)), + MakeDynamic(C::<8>(1)), + MakeDynamic(C::<9>(1)), + MakeDynamic(C::<10>(1)), + MakeDynamic(C::<11>(1)), + MakeDynamic(C::<12>(1)), + MakeDynamic(C::<13>(1)), + MakeDynamic(C::<14>(1)), + ))); + } + }); + }); + + group.finish(); +} diff --git a/benches/benches/bevy_ecs/bundles/spawn_many_zst.rs b/benches/benches/bevy_ecs/bundles/spawn_many_zst.rs new file mode 100644 index 0000000000000..6013897279bd8 --- /dev/null +++ b/benches/benches/bevy_ecs/bundles/spawn_many_zst.rs @@ -0,0 +1,206 @@ +use core::hint::black_box; + +use benches::bench; +use bevy_ecs::{bundle::Bundle, component::Component, world::World}; +use criterion::Criterion; + +use super::MakeDynamic; + +const ENTITY_COUNT: usize = 2_000; + +#[derive(Component)] +struct C; + +#[derive(Component)] +struct W(T); + +pub fn spawn_many_zst(criterion: &mut Criterion) { + let mut group = criterion.benchmark_group(bench!("spawn_many_zst")); + + group.bench_function("static", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box(( + C::<0>, C::<1>, C::<2>, C::<3>, C::<4>, C::<5>, C::<6>, C::<7>, C::<8>, C::<9>, + C::<10>, C::<11>, C::<12>, C::<13>, C::<14>, + ))); + } + }); + }); + + group.bench_function("option_some_many", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box(Some(( + C::<0>, C::<1>, C::<2>, C::<3>, C::<4>, C::<5>, C::<6>, C::<7>, C::<8>, C::<9>, + C::<10>, C::<11>, C::<12>, C::<13>, C::<14>, + )))); + } + }); + }); + + group.bench_function("option_many_some", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box(( + Some(C::<0>), + Some(C::<1>), + Some(C::<2>), + Some(C::<3>), + Some(C::<4>), + Some(C::<5>), + Some(C::<6>), + Some(C::<7>), + Some(C::<8>), + Some(C::<9>), + Some(C::<10>), + Some(C::<11>), + Some(C::<12>), + Some(C::<13>), + Some(C::<14>), + ))); + } + }); + }); + + group.bench_function("option_none_many", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box( + None::<( + C<0>, + C<1>, + C<2>, + C<3>, + C<4>, + C<5>, + C<6>, + C<7>, + C<8>, + C<9>, + C<10>, + C<11>, + C<12>, + C<13>, + C<14>, + )>, + )); + } + }); + }); + + group.bench_function("option_many_none_and_static", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box(( + (C::<0>, None::>>), + (C::<1>, None::>>), + (C::<2>, None::>>), + (C::<3>, None::>>), + (C::<4>, None::>>), + (C::<5>, None::>>), + (C::<6>, None::>>), + (C::<7>, None::>>), + (C::<8>, None::>>), + (C::<9>, None::>>), + (C::<10>, None::>>), + (C::<11>, None::>>), + (C::<12>, None::>>), + (C::<13>, None::>>), + (C::<14>, None::>>), + ))); + } + }); + }); + + group.bench_function("many_box_dyn_bundle", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + // Note: C is a ZST so Box::new is not actually allocating + // this is mostly measuring the overhead of dynamic bundles + // and dynamic dispatch. + world.spawn(black_box(( + Box::new(C::<0>) as Box, + Box::new(C::<1>) as Box, + Box::new(C::<2>) as Box, + Box::new(C::<3>) as Box, + Box::new(C::<4>) as Box, + Box::new(C::<5>) as Box, + Box::new(C::<6>) as Box, + Box::new(C::<7>) as Box, + Box::new(C::<8>) as Box, + Box::new(C::<9>) as Box, + Box::new(C::<10>) as Box, + Box::new(C::<11>) as Box, + Box::new(C::<12>) as Box, + Box::new(C::<13>) as Box, + Box::new(C::<14>) as Box, + ))); + } + }); + }); + + group.bench_function("box_dyn_bundle_many", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + // Note: C is a ZST so Box::new is not actually allocating + // this is mostly measuring the overhead of dynamic bundles + // and dynamic dispatch. + world.spawn(black_box(Box::new(( + C::<0>, C::<1>, C::<2>, C::<3>, C::<4>, C::<5>, C::<6>, C::<7>, C::<8>, C::<9>, + C::<10>, C::<11>, C::<12>, C::<13>, C::<14>, + )) as Box)); + } + }); + }); + + group.bench_function("dynamic_bundle_many", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + // This has no other overhead than opting out of the static and bounded caching + // for the whole bundle + world.spawn(MakeDynamic(( + C::<0>, C::<1>, C::<2>, C::<3>, C::<4>, C::<5>, C::<6>, C::<7>, C::<8>, C::<9>, + C::<10>, C::<11>, C::<12>, C::<13>, C::<14>, + ))); + } + }); + }); + + group.bench_function("many_dynamic_bundle", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + // This has no other overhead than opting out of the static and bounded caching + // for each sub-bundle + world.spawn(black_box(( + MakeDynamic(C::<0>), + MakeDynamic(C::<1>), + MakeDynamic(C::<2>), + MakeDynamic(C::<3>), + MakeDynamic(C::<4>), + MakeDynamic(C::<5>), + MakeDynamic(C::<6>), + MakeDynamic(C::<7>), + MakeDynamic(C::<8>), + MakeDynamic(C::<9>), + MakeDynamic(C::<10>), + MakeDynamic(C::<11>), + MakeDynamic(C::<12>), + MakeDynamic(C::<13>), + MakeDynamic(C::<14>), + ))); + } + }); + }); + + group.finish(); +} diff --git a/benches/benches/bevy_ecs/bundles/spawn_one_zst.rs b/benches/benches/bevy_ecs/bundles/spawn_one_zst.rs new file mode 100644 index 0000000000000..790413cdfe0b8 --- /dev/null +++ b/benches/benches/bevy_ecs/bundles/spawn_one_zst.rs @@ -0,0 +1,79 @@ +use core::hint::black_box; + +use benches::bench; +use bevy_ecs::{bundle::Bundle, component::Component, world::World}; +use criterion::Criterion; + +use super::MakeDynamic; + +const ENTITY_COUNT: usize = 10_000; + +#[derive(Component)] +struct A; + +#[derive(Component)] +struct B; + +pub fn spawn_one_zst(criterion: &mut Criterion) { + let mut group = criterion.benchmark_group(bench!("spawn_one_zst")); + + group.bench_function("static", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box(A)); + } + }); + }); + + group.bench_function("option_some", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box(Some(A))); + } + }); + }); + + group.bench_function("option_none", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box(None::)); + } + }); + }); + + group.bench_function("option_none_and_static", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box((A, None::))); + } + }); + }); + + group.bench_function("box_dyn_bundle", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + // Note: A is a ZST so Box::new is not actually allocating + // this is mostly measuring the overhead of dynamic bundles + // and dynamic dispatch. + world.spawn(black_box(Box::new(A) as Box)); + } + }); + }); + + group.bench_function("dynamic_bundle", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + // This has no other overhead than opting out of the static and bounded caching + world.spawn(black_box(MakeDynamic(A))); + } + }); + }); + + group.finish(); +} diff --git a/benches/benches/bevy_ecs/main.rs b/benches/benches/bevy_ecs/main.rs index 4a025ab829369..59b4c1fd7326a 100644 --- a/benches/benches/bevy_ecs/main.rs +++ b/benches/benches/bevy_ecs/main.rs @@ -5,6 +5,7 @@ use criterion::criterion_main; +mod bundles; mod change_detection; mod components; mod empty_archetypes; @@ -18,6 +19,7 @@ mod scheduling; mod world; criterion_main!( + bundles::benches, change_detection::benches, components::benches, empty_archetypes::benches, From 335c9d25b82a10526899a98e0e505980751ee9fa Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Sat, 7 Jun 2025 15:05:29 +0200 Subject: [PATCH 39/44] Add BoundedBundleKey to simplify implementations of cache_key --- benches/benches/bevy_ecs/bundles/mod.rs | 6 +- crates/bevy_ecs/macros/src/lib.rs | 23 +---- crates/bevy_ecs/src/bundle.rs | 120 ++++++++++++++++-------- crates/bevy_ecs/src/spawn.rs | 17 ++-- 4 files changed, 100 insertions(+), 66 deletions(-) diff --git a/benches/benches/bevy_ecs/bundles/mod.rs b/benches/benches/bevy_ecs/bundles/mod.rs index 28dc97520f5ef..1f26e77a2651d 100644 --- a/benches/benches/bevy_ecs/bundles/mod.rs +++ b/benches/benches/bevy_ecs/bundles/mod.rs @@ -1,5 +1,5 @@ use bevy_ecs::{ - bundle::{Bundle, ComponentsFromBundle}, + bundle::{BoundedBundleKey, Bundle, ComponentsFromBundle}, component::{ComponentId, ComponentsRegistrator, RequiredComponents, StorageType}, ptr::OwningPtr, }; @@ -30,8 +30,8 @@ unsafe impl Bundle for MakeDynamic { false } - fn cache_key(&self) -> (u64, usize) { - (0, 0) + fn cache_key(&self) -> BoundedBundleKey { + BoundedBundleKey::empty() } fn component_ids( diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 3c464e180d95d..d5e7bdbc93528 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -172,24 +172,11 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { true #(&& <#active_field_types as #ecs_path::bundle::Bundle>::is_bounded())* } - fn cache_key(&self) -> (u64, usize) { - let mut key = 0; - let mut size = 0; - - #( - let (sub_key, sub_size) = <#active_field_types as #ecs_path::bundle::Bundle>::cache_key(&self.#active_field_tokens); - key |= sub_key << size; - size += sub_size; - // Bail out if size is too big, this avoids overflow errors when shifting - // left by the size. - // Returning anything with a size bigger than 64 is fine because that - // signals that the key could not be compute. - if size > 64 { - return (0, 65); - } - )* - - (key, size) + fn cache_key(&self) -> #ecs_path::bundle::BoundedBundleKey { + #ecs_path::bundle::BoundedBundleKey::empty() + #( + .merge(<#active_field_types as #ecs_path::bundle::Bundle>::cache_key(&self.#active_field_tokens)) + )* } fn component_ids( diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index c0ac7967e808e..95127254dde46 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -166,8 +166,8 @@ use variadics_please::all_tuples; // in this bundle type is fixed and always the same for every instance; // - [`Bundle::is_bounded`] must be pure and return `true` only if the set of components contained // in this bundle type's instances is a subset of a statically known set of components; -// - [`Bundle::cache_key`] must be pure and return an unique key for each combination of components -// contained in this bundle's instance; +// - if this bundle is bounded then [`Bundle::cache_key`] must be pure and return an unique key for +// each combination of components contained in this bundle's instance; // - [`Bundle::component_ids`] must return the [`ComponentId`] for each component type in the // bundle, in the _exact_ order that [`ComponentsFromBundle::get_components`] is called. // - [`ComponentsFromBundle::get_components`] must call `func` exactly once for each [`ComponentId`] returned by @@ -201,7 +201,7 @@ pub unsafe trait Bundle: BundleDyn + ComponentsFromBundle + Send + Sync + 'stati /// Computes the cache key associated with `self` and its size. The key is limited to 64 bits, and if the /// size exceeds that it should be ignored. #[doc(hidden)] - fn cache_key(&self) -> (u64, usize); + fn cache_key(&self) -> BoundedBundleKey; /// Gets `self`'s component ids, in the order of this bundle's [`Component`]s #[doc(hidden)] @@ -357,8 +357,8 @@ unsafe impl Bundle for Box { false } - fn cache_key(&self) -> (u64, usize) { - (0, 0) + fn cache_key(&self) -> BoundedBundleKey { + BoundedBundleKey::empty() } fn component_ids( @@ -458,8 +458,8 @@ unsafe impl Bundle for C { true } - fn cache_key(&self) -> (u64, usize) { - (0, 0) + fn cache_key(&self) -> BoundedBundleKey { + BoundedBundleKey::empty() } fn component_ids( @@ -561,7 +561,7 @@ macro_rules! tuple_impl { true $(&& <$name as Bundle>::is_bounded())* } - fn cache_key(&self) -> (u64, usize) { + fn cache_key(&self) -> BoundedBundleKey { #[allow( non_snake_case, reason = "The names of these variables are provided by the caller, not by us." @@ -571,20 +571,8 @@ macro_rules! tuple_impl { let mut key = 0; let mut size = 0; - $( - let (sub_key, sub_size) = $name.cache_key(); - key |= sub_key << size; - size += sub_size; - // Bail out if size is too big, this avoids overflow errors when shifting - // left by the size. - // Returning anything with a size bigger than 64 is fine because that - // signals that the key could not be compute. - if size > 64 { - return (0, 65); - } - )* - - (key, size) + BoundedBundleKey::empty() + $( .merge($name.cache_key()) )* } fn component_ids( @@ -721,6 +709,70 @@ macro_rules! after_effect_impl { all_tuples!(after_effect_impl, 0, 15, P); +#[doc(hidden)] +#[derive(Clone, Copy)] +pub struct BoundedBundleKey { + key: u64, + len_bits: u32, +} + +impl BoundedBundleKey { + /// Creates an empty `BoundedBundleKey`. This should be used for static bundles as + /// there's only one subset of components it can represent. This can also be used + /// as a dummy key in dynamic bundles, since it is not used for them. + #[inline] + pub const fn empty() -> Self { + Self { + key: 0, + len_bits: 0, + } + } + + /// Returns whether this bundle key is valid. Currently this is the case if + /// it fits in 64 bits. + #[inline] + pub const fn is_valid(&self) -> bool { + self.len_bits <= 64 + } + + /// Merge two `BoundedBundleKey`s by appending `other` to the right of `self` + /// and producing a new `BoundedBundleKey` representing both of them. + /// + /// Note that for there can be multiple combinations of `self` and `other` that will + /// produce the same output, so care should be taken to avoid them. + /// A way this can be achieved is by ensuring that there exists a way to compute back + /// the length of the key given the key itself and the type it is for. + #[inline] + pub const fn merge(self, other: Self) -> Self { + let len_bits = self.len_bits + other.len_bits; + let key = self.key.wrapping_shl(other.len_bits) | other.key; + let merged = Self { key, len_bits }; + if merged.is_valid() { + merged + } else { + // Normalize invalid keys so that `len_bits` never overflows + Self { + key: 0, + len_bits: 65, + } + } + } + + /// Creates a new `BoundedBundleKey` from a literal key and its bits length. + /// It's suggested to run this in a `const` block to run the asserts at compile time. + /// + /// # Panics + /// + /// This panics if the given key requires more than `len_bits` to be represented + /// or if `len_bits` is more than 64. + #[inline] + pub const fn literal(key: u64, len_bits: u32) -> Self { + assert!(len_bits <= 64); + assert!(u64::BITS - key.leading_zeros() <= len_bits); + Self { key, len_bits } + } +} + /// For a specific [`World`], this stores a unique value identifying a type of a registered [`Bundle`]. /// /// [`World`]: crate::world::World @@ -2258,11 +2310,11 @@ impl Bundles { // Optimized case for bounded bundles, e.g. those with conditional components whose // key fits in 64 bits. if T::is_bounded() { - let (key, size) = bundle.cache_key(); - if size <= 64 { + let cache_key = bundle.cache_key(); + if cache_key.is_valid() { return *self .bounded_bundle_ids - .entry((TypeId::of::(), key)) + .entry((TypeId::of::(), cache_key.key)) .or_insert_with(|| { Self::register_new_bundle(bundle, components, storages, bundle_infos) }); @@ -2487,18 +2539,12 @@ unsafe impl Bundle for Option { T::is_bounded() } - fn cache_key(&self) -> (u64, usize) { + fn cache_key(&self) -> BoundedBundleKey { if let Some(this) = self { - let (key, len) = this.cache_key(); - if len >= 64 { - return (0, 65); - } - - // key ends in 0 - (key << 1, len + 1) + this.cache_key() + .merge(const { BoundedBundleKey::literal(1, 1) }) } else { - // key ends in 1 - (1, 1) + const { BoundedBundleKey::literal(0, 1) } } } @@ -2557,8 +2603,8 @@ unsafe impl Bundle for Vec> { false } - fn cache_key(&self) -> (u64, usize) { - (0, 0) + fn cache_key(&self) -> BoundedBundleKey { + BoundedBundleKey::empty() } fn component_ids( diff --git a/crates/bevy_ecs/src/spawn.rs b/crates/bevy_ecs/src/spawn.rs index 52993d6074c2d..e986e300c3d91 100644 --- a/crates/bevy_ecs/src/spawn.rs +++ b/crates/bevy_ecs/src/spawn.rs @@ -2,7 +2,9 @@ //! for the best entry points into these APIs and examples of how to use them. use crate::{ - bundle::{Bundle, BundleEffect, ComponentsFromBundle, NoBundleEffect, StaticBundle}, + bundle::{ + BoundedBundleKey, Bundle, BundleEffect, ComponentsFromBundle, NoBundleEffect, StaticBundle, + }, entity::Entity, relationship::{RelatedSpawner, Relationship, RelationshipTarget}, world::{EntityWorldMut, World}, @@ -226,8 +228,8 @@ unsafe impl + Send + Sync + 'static> Bundle ::is_bounded() } - fn cache_key(&self) -> (u64, usize) { - (0, 0) + fn cache_key(&self) -> BoundedBundleKey { + BoundedBundleKey::empty() } fn component_ids( @@ -316,6 +318,8 @@ unsafe impl StaticBundle for SpawnOneRelated { // SAFETY: This internally relies on the RelationshipTarget's Bundle implementation, which is sound. unsafe impl Bundle for SpawnOneRelated { + // We are inserting `R::RelationshipTarget`, which is a `Component` and thus + // always a `StaticBundle`. fn is_static() -> bool { ::is_static() } @@ -323,11 +327,8 @@ unsafe impl Bundle for SpawnOneRelated { ::is_bounded() } - fn cache_key(&self) -> (u64, usize) { - // We are inserting `R::RelationshipTarget`, which is a `Component` and thus - // always a `StaticBundle`. However we don't have an instance yet, so we can't delegate - // cache_key to it. - (0, 0) + fn cache_key(&self) -> BoundedBundleKey { + BoundedBundleKey::empty() } fn component_ids( From b902d5f5629429d28407c4e5ed06d44af2aa51bb Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Sat, 7 Jun 2025 16:28:15 +0200 Subject: [PATCH 40/44] Add migration guide --- .../migration-guides/static-bundles.md | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 release-content/migration-guides/static-bundles.md diff --git a/release-content/migration-guides/static-bundles.md b/release-content/migration-guides/static-bundles.md new file mode 100644 index 0000000000000..3e9fcf112609b --- /dev/null +++ b/release-content/migration-guides/static-bundles.md @@ -0,0 +1,55 @@ +--- +title: StaticBundle split off from Bundle +pull_requests: [19491] +--- + +The `StaticBundle` trait has been split off from the `Bundle` trait to avoid conflating the concept of a type whose values can be inserted into an entity (`Bundle`) with the concept of a statically known set of components (`StaticBundle`). This required the update of existing APIs that were using `Bundle` as a statically known set of components to use `StaticBundle` instead. + +Changes for most users will be zero or pretty minimal, since `#[derive(Bundle)]` will automatically derive `StaticBundle` and most types that implemented `Bundle` will now also implement `StaticBundle`. The main exception will be generic APIs or types, which now will need to update or add a bound on `StaticBundle`. For example: + +```rs +// 0.16 +#[derive(Bundle)] +struct MyBundleWrapper { + inner: T +} + +fn my_register_bundle(world: &mut World) { + world.register_bundle::(); +} + + +// 0.17 +#[derive(Bundle)] +struct MyBundleWrapper { // Add a StaticBundle bound + inner: T +} + +fn my_register_bundle(world: &mut World) { // Replace Bundle with StaticBundle + world.register_bundle::(); +} +``` + +The following APIs now require the `StaticBundle` trait instead of the `Bundle` trait: + +- `World::register_bundle`, which has been renamed to `World::register_static_bundle` +- the `B` type parameter of `EntityRefExcept` and `EntityMutExcept` +- `EntityClonerBuilder::allow` and `EntityClonerBuilder::deny` +- `EntityCommands::clone_components` and `EntityCommands::move_components` +- `EntityWorldMut::clone_components` and `EntityWorldMut::move_components` +- the `B` type parameter of `IntoObserverSystem`, `Trigger`, `App::add_observer`, `World::add_observer`, `Observer::new`, `Commands::add_observer`, `EntityCommands::observe` and `EntityWorldMut::observe` +- `EntityWorldMut::remove_recursive` and `Commands::remove_recursive` +- `EntityCommands::remove`, `EntityCommands::remove_if`, `EntityCommands::try_remove_if`, `EntityCommands::try_remove`, `EntityCommands::remove_with_requires`, `EntityWorldMut::remove` and `EntityWorldMut::remove_with_requires` +- `EntityWorldMut::take` +- `EntityWorldMut::retain` and `EntityCommands::retain` + +The following APIs now require the `StaticBundle` trait in addition to the `Bundle` trait: + +- `Commands::spawn_batch`, `Commands::insert_batch`, `Commands::insert_batch_if_new`, `Commands::try_insert_batch`, `Commands::try_insert_batch_if_new`, `bevy::ecs::command::spawn_batch`, `bevy::ecs::command::insert_batch`, `World::spawn_batch`, `World::insert_batch`, `World::insert_batch_if_new`, `World::try_insert_batch` and `World::try_insert_batch_if_new` +- `ReflectBundle::new`, `impl FromType` for `ReflectBundle` and `#[reflect(Bundle)]` +- `ExtractComponent::Out` + +Moreover, some APIs have been renamed: + +- `World::register_bundle` has been renamed to `World::register_static_bundle` +- the `DynamicBundle` trait has been renamed to `ComponentsFromBundle` From 275f14a16af509f54c390ba7643e4015c36e02e2 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Sat, 7 Jun 2025 17:19:38 +0200 Subject: [PATCH 41/44] Add release notes --- .../release-notes/dynamic-bundles.md | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 release-content/release-notes/dynamic-bundles.md diff --git a/release-content/release-notes/dynamic-bundles.md b/release-content/release-notes/dynamic-bundles.md new file mode 100644 index 0000000000000..54a6be9e3efa5 --- /dev/null +++ b/release-content/release-notes/dynamic-bundles.md @@ -0,0 +1,53 @@ +--- +title: Option and dynamic bundles +authors: ["@SkiFire13"] +pull_requests: [19491] +--- + +Until now bundles have always represented a static set of components, meaning you couldn't create a `Bundle` that sometimes inserts a component and sometimes doesn't, or a totally dynamic bundle like you would expect from a `Box`. + +The `Bundle` trait has now been reworked to support these usecases, in particular: + +- `Option` implements `Bundle` +- `Vec>` also implements `Bundle` + +```rs +fn enemy_bundle(name: String, attack_power: u32, is_boss: bool) -> impl Bundle { + ( + EnemyMarker, + Name::new(name), + AttackPower(attack_power), + if is_boss { Some(BossMarker) } else { None } + ) +} + +fn bundle_from_reflected_data(components: Vec>, registry: &TypeRegistry) -> impl Bundle { + components + .into_iter() + .map(|data| { + let Some(reflect_bundle) = registry.get_type_data::(data.type_id()) else { todo!() }; + let Ok(bundle_box) = reflect_bundle.get_boxed(data) else { todo!() }; + bundle_box + }) + .collect::>() +} +``` + +## `StaticBundle` + +In order to support these changes to `Bundle` we had to introduce a new trait, `StaticBundle`, for the cases where statically knowing the set of components is required, like for example `World::remove`. + +The `Bundle` derive macro will automatically implement this trait for you, so most things should continue working like before, but in any case check out the migration guide! + +In case however that your bundle contains some kind of dynamic bundle then this won't be possible, and you'll have to opt-out of automatically implementing `StaticBundle` and `BundleFromComponents` by adding the `#[reflect(dynamic)]` attribute. + +```rs +#[derive(Bundle)] +#[derive(dynamic)] // This is required if any field is like the ones below! +struct MyBundle { + foo: Option, + extra: Box, + extras: Vec>, +} +``` From 0276d6837e38d72a5954a38e70b17b0f38f3670b Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Sat, 7 Jun 2025 20:29:35 +0200 Subject: [PATCH 42/44] Implement ReflectBundle::get_boxes --- crates/bevy_ecs/src/reflect/bundle.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/bevy_ecs/src/reflect/bundle.rs b/crates/bevy_ecs/src/reflect/bundle.rs index f6a9034ac2958..bd1e80e456a01 100644 --- a/crates/bevy_ecs/src/reflect/bundle.rs +++ b/crates/bevy_ecs/src/reflect/bundle.rs @@ -32,6 +32,8 @@ pub struct ReflectBundle(ReflectBundleFns); /// The also [`super::component::ReflectComponentFns`]. #[derive(Clone)] pub struct ReflectBundleFns { + /// Function pointer implementing [`ReflectBundle::get_boxed`]. + pub get_boxed: fn(Box) -> Result, Box>, /// Function pointer implementing [`ReflectBundle::insert`]. pub insert: fn(&mut EntityWorldMut, &dyn PartialReflect, &TypeRegistry), /// Function pointer implementing [`ReflectBundle::apply`]. @@ -62,6 +64,16 @@ impl ReflectBundleFns { } impl ReflectBundle { + /// Downcast a `Box` type to `Box`. + /// + /// If the type cannot be downcast, this will return `Err(Box)`. + pub fn get_boxed( + &self, + reflect_value: Box, + ) -> Result, Box> { + (self.0.get_boxed)(reflect_value) + } + /// Insert a reflected [`Bundle`] into the entity like [`insert()`](EntityWorldMut::insert). pub fn insert( &self, @@ -152,6 +164,9 @@ impl FromT { fn from_type() -> Self { ReflectBundle(ReflectBundleFns { + get_boxed: |reflect_value| { + ::downcast::(reflect_value).map(|value| value as Box) + }, insert: |entity, reflected_bundle, registry| { let bundle = entity.world_scope(|world| { from_reflect_with_fallback::(reflected_bundle, world, registry) From 1b141b3dd691312efdbf3b98e14abc16a54cfda8 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Sun, 8 Jun 2025 13:27:37 +0200 Subject: [PATCH 43/44] Add more benchmarks --- .../benches/bevy_ecs/bundles/insert_many.rs | 68 +++++++++++++++++++ benches/benches/bevy_ecs/bundles/mod.rs | 2 + .../benches/bevy_ecs/bundles/spawn_many.rs | 50 ++++++++++++++ 3 files changed, 120 insertions(+) create mode 100644 benches/benches/bevy_ecs/bundles/insert_many.rs diff --git a/benches/benches/bevy_ecs/bundles/insert_many.rs b/benches/benches/bevy_ecs/bundles/insert_many.rs new file mode 100644 index 0000000000000..fbc0409f4d057 --- /dev/null +++ b/benches/benches/bevy_ecs/bundles/insert_many.rs @@ -0,0 +1,68 @@ +use benches::bench; +use bevy_ecs::{component::Component, world::World}; +use criterion::Criterion; + +const ENTITY_COUNT: usize = 2_000; + +#[derive(Component)] +struct C(usize); + +#[derive(Component)] +struct W(T); + +pub fn insert_many(criterion: &mut Criterion) { + let mut group = criterion.benchmark_group(bench!("insert_many")); + + group.bench_function("all", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world + .spawn_empty() + .insert(C::<0>(1)) + .insert(C::<1>(1)) + .insert(C::<2>(1)) + .insert(C::<3>(1)) + .insert(C::<4>(1)) + .insert(C::<5>(1)) + .insert(C::<6>(1)) + .insert(C::<7>(1)) + .insert(C::<8>(1)) + .insert(C::<9>(1)) + .insert(C::<10>(1)) + .insert(C::<11>(1)) + .insert(C::<12>(1)) + .insert(C::<13>(1)) + .insert(C::<14>(1)); + } + }); + }); + + group.bench_function("only_last", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world + .spawn(( + C::<0>(1), + C::<1>(1), + C::<2>(1), + C::<3>(1), + C::<4>(1), + C::<5>(1), + C::<6>(1), + C::<7>(1), + C::<8>(1), + C::<9>(1), + C::<10>(1), + C::<11>(1), + C::<12>(1), + C::<13>(1), + )) + .insert(C::<14>(1)); + } + }); + }); + + group.finish(); +} diff --git a/benches/benches/bevy_ecs/bundles/mod.rs b/benches/benches/bevy_ecs/bundles/mod.rs index 1f26e77a2651d..1fb7421f626c9 100644 --- a/benches/benches/bevy_ecs/bundles/mod.rs +++ b/benches/benches/bevy_ecs/bundles/mod.rs @@ -5,6 +5,7 @@ use bevy_ecs::{ }; use criterion::criterion_group; +mod insert_many; mod spawn_many; mod spawn_many_zst; mod spawn_one_zst; @@ -14,6 +15,7 @@ criterion_group!( spawn_one_zst::spawn_one_zst, spawn_many_zst::spawn_many_zst, spawn_many::spawn_many, + insert_many::insert_many, ); struct MakeDynamic(B); diff --git a/benches/benches/bevy_ecs/bundles/spawn_many.rs b/benches/benches/bevy_ecs/bundles/spawn_many.rs index d44569809d635..b1f2c8098637f 100644 --- a/benches/benches/bevy_ecs/bundles/spawn_many.rs +++ b/benches/benches/bevy_ecs/bundles/spawn_many.rs @@ -67,6 +67,31 @@ pub fn spawn_many(criterion: &mut Criterion) { }); }); + group.bench_function("option_one_some", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box(( + C::<0>(1), + C::<1>(1), + C::<2>(1), + C::<3>(1), + C::<4>(1), + C::<5>(1), + C::<6>(1), + C::<7>(1), + C::<8>(1), + C::<9>(1), + C::<10>(1), + C::<11>(1), + C::<12>(1), + C::<13>(1), + Some(C::<14>(1)), + ))); + } + }); + }); + group.bench_function("option_many_some", |bencher| { bencher.iter(|| { let mut world = World::new(); @@ -119,6 +144,31 @@ pub fn spawn_many(criterion: &mut Criterion) { }); }); + group.bench_function("option_one_none", |bencher| { + bencher.iter(|| { + let mut world = World::new(); + for _ in 0..ENTITY_COUNT { + world.spawn(black_box(( + C::<0>(1), + C::<1>(1), + C::<2>(1), + C::<3>(1), + C::<4>(1), + C::<5>(1), + C::<6>(1), + C::<7>(1), + C::<8>(1), + C::<9>(1), + C::<10>(1), + C::<11>(1), + C::<12>(1), + C::<13>(1), + None::>, + ))); + } + }); + }); + group.bench_function("option_many_none_and_static", |bencher| { bencher.iter(|| { let mut world = World::new(); From 5446247bf8b9e1753ed28c46a6d8cc8fa63cd3d6 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Thu, 12 Jun 2025 21:20:59 +0200 Subject: [PATCH 44/44] Fix imports and re-exports --- crates/bevy_ecs/src/world/mod.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 8bc89dba4a48f..d4b84520bcaf4 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -14,11 +14,8 @@ pub mod unsafe_world_cell; pub mod reflect; pub use crate::{ - bundle::StaticBundle, change_detection::{Mut, Ref, CHECK_TICK_THRESHOLD}, - error::{DefaultErrorHandler, ErrorHandler}, - lifecycle::{ComponentHooks, ADD, DESPAWN, INSERT, REMOVE, REPLACE}, - prelude::{Add, Despawn, Insert, Remove, Replace}, + world::command_queue::CommandQueue, }; pub use bevy_ecs_macros::FromWorld; pub use deferred_world::DeferredWorld; @@ -36,7 +33,7 @@ use crate::{ archetype::{ArchetypeId, Archetypes}, bundle::{ Bundle, BundleEffect, BundleInfo, BundleInserter, BundleSpawner, Bundles, InsertMode, - NoBundleEffect, + NoBundleEffect, StaticBundle, }, change_detection::{MaybeLocation, MutUntyped, TicksMut}, component::{ @@ -46,8 +43,12 @@ use crate::{ }, entity::{Entities, Entity, EntityDoesNotExistError}, entity_disabling::DefaultQueryFilters, + error::{DefaultErrorHandler, ErrorHandler}, event::{Event, EventId, Events, SendBatchIds}, - lifecycle::RemovedComponentEvents, + lifecycle::{ + Add, ComponentHooks, Despawn, Insert, Remove, RemovedComponentEvents, Replace, ADD, + DESPAWN, INSERT, REMOVE, REPLACE, + }, observer::Observers, query::{DebugCheckedUnwrap, QueryData, QueryFilter, QueryState}, relationship::RelationshipHookMode,