From 1ed72c508be42b51d1cb4c8aaa3e9ff790fc79f1 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Thu, 10 Jul 2025 19:27:32 +0200 Subject: [PATCH] Extract bundle impls from bundle.rs --- crates/bevy_ecs/src/bundle/impls.rs | 175 ++++++++++++++++++++++++++++ crates/bevy_ecs/src/bundle/mod.rs | 170 +-------------------------- 2 files changed, 178 insertions(+), 167 deletions(-) create mode 100644 crates/bevy_ecs/src/bundle/impls.rs diff --git a/crates/bevy_ecs/src/bundle/impls.rs b/crates/bevy_ecs/src/bundle/impls.rs new file mode 100644 index 0000000000..ca6525247e --- /dev/null +++ b/crates/bevy_ecs/src/bundle/impls.rs @@ -0,0 +1,175 @@ +use core::any::TypeId; + +use bevy_ptr::OwningPtr; +use variadics_please::all_tuples; + +use crate::{ + bundle::{Bundle, BundleEffect, BundleFromComponents, DynamicBundle, NoBundleEffect}, + component::{Component, ComponentId, Components, ComponentsRegistrator, StorageType}, + world::EntityWorldMut, +}; + +// 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 { + 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::())); + } +} + +// 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 { + unsafe fn from_components(ctx: &mut T, func: &mut F) -> Self + where + // Ensure that the `OwningPtr` is used correctly + F: for<'a> FnMut(&'a mut T) -> OwningPtr<'a>, + Self: Sized, + { + let ptr = func(ctx); + // Safety: The id given in `component_ids` is for `Self` + unsafe { ptr.read() } + } +} + +impl DynamicBundle for C { + type Effect = (); + #[inline] + fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) -> Self::Effect { + OwningPtr::make(self, |ptr| func(C::STORAGE_TYPE, ptr)); + } +} + +macro_rules! tuple_impl { + ($(#[$meta:meta])* $($name: ident),*) => { + #[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: + // - `Bundle::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 + // `StorageType` into the callback. + unsafe impl<$($name: Bundle),*> Bundle for ($($name,)*) { + fn component_ids(components: &mut ComponentsRegistrator, ids: &mut impl FnMut(ComponentId)){ + $(<$name as Bundle>::component_ids(components, ids);)* + } + + fn get_component_ids(components: &Components, ids: &mut impl FnMut(Option)){ + $(<$name as Bundle>::get_component_ids(components, ids);)* + } + } + + #[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: + // - `Bundle::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 + // `StorageType` into the callback. + unsafe impl<$($name: BundleFromComponents),*> BundleFromComponents for ($($name,)*) { + #[allow( + clippy::unused_unit, + reason = "Zero-length tuples will generate a function body equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case." + )] + unsafe fn from_components(ctx: &mut T, func: &mut F) -> Self + where + F: FnMut(&mut T) -> OwningPtr<'_> + { + #[allow( + unused_unsafe, + reason = "Zero-length tuples will not run anything in the unsafe block. Additionally, rewriting this to move the () outside of the unsafe would require putting the safety comment inside the tuple, hurting readability of the code." + )] + // SAFETY: Rust guarantees that tuple calls are evaluated 'left to right'. + // https://doc.rust-lang.org/reference/expressions.html#evaluation-order-of-operands + unsafe { ($(<$name as BundleFromComponents>::from_components(ctx, func),)*) } + } + } + + #[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])* + impl<$($name: Bundle),*> DynamicBundle for ($($name,)*) { + type Effect = ($($name::Effect,)*); + #[allow( + clippy::unused_unit, + reason = "Zero-length tuples will generate a function body equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case." + )] + #[inline(always)] + fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) -> Self::Effect { + #[allow( + non_snake_case, + reason = "The names of these variables are provided by the caller, not by us." + )] + let ($(mut $name,)*) = self; + ($( + $name.get_components(&mut *func), + )*) + } + } + } +} + +all_tuples!( + #[doc(fake_variadic)] + tuple_impl, + 0, + 15, + B +); + +macro_rules! after_effect_impl { + ($($after_effect: ident),*) => { + #[expect( + clippy::allow_attributes, + reason = "This is a tuple-related macro; as such, the lints below may not always apply." + )] + impl<$($after_effect: BundleEffect),*> BundleEffect for ($($after_effect,)*) { + #[allow( + clippy::unused_unit, + reason = "Zero-length tuples will generate a function body equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case.") + ] + fn apply(self, _entity: &mut EntityWorldMut) { + #[allow( + non_snake_case, + reason = "The names of these variables are provided by the caller, not by us." + )] + let ($($after_effect,)*) = self; + $($after_effect.apply(_entity);)* + } + } + + impl<$($after_effect: NoBundleEffect),*> NoBundleEffect for ($($after_effect,)*) { } + } +} + +all_tuples!(after_effect_impl, 0, 15, P); diff --git a/crates/bevy_ecs/src/bundle/mod.rs b/crates/bevy_ecs/src/bundle/mod.rs index 75c2c89ff8..a64940f151 100644 --- a/crates/bevy_ecs/src/bundle/mod.rs +++ b/crates/bevy_ecs/src/bundle/mod.rs @@ -2,6 +2,8 @@ //! //! This module contains the [`Bundle`] trait and some other helper types. +mod impls; + /// Derive the [`Bundle`] trait /// /// You can apply this derive macro to structs that are @@ -62,7 +64,7 @@ use crate::{ }, change_detection::MaybeLocation, component::{ - Component, ComponentId, Components, ComponentsRegistrator, RequiredComponentConstructor, + ComponentId, Components, ComponentsRegistrator, RequiredComponentConstructor, RequiredComponents, StorageType, Tick, }, entity::{Entities, Entity, EntityLocation}, @@ -79,7 +81,6 @@ use bevy_platform::collections::{HashMap, HashSet}; use bevy_ptr::{ConstNonNull, OwningPtr}; 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. /// @@ -265,175 +266,10 @@ pub trait BundleEffect { fn apply(self, entity: &mut EntityWorldMut); } -// 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 { - 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::())); - } -} - -// 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 { - unsafe fn from_components(ctx: &mut T, func: &mut F) -> Self - where - // Ensure that the `OwningPtr` is used correctly - F: for<'a> FnMut(&'a mut T) -> OwningPtr<'a>, - Self: Sized, - { - let ptr = func(ctx); - // Safety: The id given in `component_ids` is for `Self` - unsafe { ptr.read() } - } -} - -impl DynamicBundle for C { - type Effect = (); - #[inline] - fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) -> Self::Effect { - OwningPtr::make(self, |ptr| func(C::STORAGE_TYPE, ptr)); - } -} - -macro_rules! tuple_impl { - ($(#[$meta:meta])* $($name: ident),*) => { - #[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: - // - `Bundle::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 - // `StorageType` into the callback. - unsafe impl<$($name: Bundle),*> Bundle for ($($name,)*) { - fn component_ids(components: &mut ComponentsRegistrator, ids: &mut impl FnMut(ComponentId)){ - $(<$name as Bundle>::component_ids(components, ids);)* - } - - fn get_component_ids(components: &Components, ids: &mut impl FnMut(Option)){ - $(<$name as Bundle>::get_component_ids(components, ids);)* - } - } - - #[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: - // - `Bundle::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 - // `StorageType` into the callback. - unsafe impl<$($name: BundleFromComponents),*> BundleFromComponents for ($($name,)*) { - #[allow( - clippy::unused_unit, - reason = "Zero-length tuples will generate a function body equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case." - )] - unsafe fn from_components(ctx: &mut T, func: &mut F) -> Self - where - F: FnMut(&mut T) -> OwningPtr<'_> - { - #[allow( - unused_unsafe, - reason = "Zero-length tuples will not run anything in the unsafe block. Additionally, rewriting this to move the () outside of the unsafe would require putting the safety comment inside the tuple, hurting readability of the code." - )] - // SAFETY: Rust guarantees that tuple calls are evaluated 'left to right'. - // https://doc.rust-lang.org/reference/expressions.html#evaluation-order-of-operands - unsafe { ($(<$name as BundleFromComponents>::from_components(ctx, func),)*) } - } - } - - #[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])* - impl<$($name: Bundle),*> DynamicBundle for ($($name,)*) { - type Effect = ($($name::Effect,)*); - #[allow( - clippy::unused_unit, - reason = "Zero-length tuples will generate a function body equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case." - )] - #[inline(always)] - fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) -> Self::Effect { - #[allow( - non_snake_case, - reason = "The names of these variables are provided by the caller, not by us." - )] - let ($(mut $name,)*) = self; - ($( - $name.get_components(&mut *func), - )*) - } - } - } -} - -all_tuples!( - #[doc(fake_variadic)] - tuple_impl, - 0, - 15, - B -); - /// 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. pub trait NoBundleEffect {} -macro_rules! after_effect_impl { - ($($after_effect: ident),*) => { - #[expect( - clippy::allow_attributes, - reason = "This is a tuple-related macro; as such, the lints below may not always apply." - )] - impl<$($after_effect: BundleEffect),*> BundleEffect for ($($after_effect,)*) { - #[allow( - clippy::unused_unit, - reason = "Zero-length tuples will generate a function body equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case.") - ] - fn apply(self, _entity: &mut EntityWorldMut) { - #[allow( - non_snake_case, - reason = "The names of these variables are provided by the caller, not by us." - )] - let ($($after_effect,)*) = self; - $($after_effect.apply(_entity);)* - } - } - - impl<$($after_effect: NoBundleEffect),*> NoBundleEffect for ($($after_effect,)*) { } - } -} - -all_tuples!(after_effect_impl, 0, 15, P); - /// For a specific [`World`], this stores a unique value identifying a type of a registered [`Bundle`]. /// /// [`World`]: crate::world::World