diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 2d1bab83b2882..ed4391d7e0041 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -68,7 +68,7 @@ pub fn all_tuples(input: TokenStream) -> TokenStream { let macro_ident = &input.macro_ident; let invocations = (input.start..=input.end).map(|i| { - let ident_tuples = &ident_tuples[0..i - input.start]; + let ident_tuples = &ident_tuples[..i]; quote! { #macro_ident!(#(#ident_tuples),*); } @@ -80,9 +80,7 @@ pub fn all_tuples(input: TokenStream) -> TokenStream { }) } -static BUNDLE_ATTRIBUTE_NAME: &str = "bundle"; - -#[proc_macro_derive(Bundle, attributes(bundle))] +#[proc_macro_derive(Bundle)] pub fn derive_bundle(input: TokenStream) -> TokenStream { let ast = parse_macro_input!(input as DeriveInput); let ecs_path = bevy_ecs_path(); @@ -92,15 +90,6 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { Err(e) => return e.into_compile_error().into(), }; - let is_bundle = named_fields - .iter() - .map(|field| { - field - .attrs - .iter() - .any(|a| *a.path.get_ident().as_ref().unwrap() == BUNDLE_ATTRIBUTE_NAME) - }) - .collect::>(); let field = named_fields .iter() .map(|field| field.ident.as_ref().unwrap()) @@ -113,32 +102,17 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { let mut field_component_ids = Vec::new(); let mut field_get_components = Vec::new(); let mut field_from_components = Vec::new(); - for ((field_type, is_bundle), field) in - field_type.iter().zip(is_bundle.iter()).zip(field.iter()) - { - if *is_bundle { - field_component_ids.push(quote! { - component_ids.extend(<#field_type as #ecs_path::bundle::Bundle>::component_ids(components, storages)); - }); - field_get_components.push(quote! { - self.#field.get_components(&mut func); - }); - field_from_components.push(quote! { - #field: <#field_type as #ecs_path::bundle::Bundle>::from_components(ctx, &mut func), - }); - } else { - field_component_ids.push(quote! { - component_ids.push(components.init_component::<#field_type>(storages)); - }); - field_get_components.push(quote! { - #ecs_path::ptr::OwningPtr::make(self.#field, &mut func); - }); - field_from_components.push(quote! { - #field: func(ctx).read::<#field_type>(), - }); - } + for (field_type, field) in field_type.iter().zip(field.iter()) { + field_component_ids.push(quote! { + <#field_type as #ecs_path::bundle::Bundle>::component_ids(components, storages, &mut *ids); + }); + field_get_components.push(quote! { + self.#field.get_components(&mut *func); + }); + field_from_components.push(quote! { + #field: <#field_type as #ecs_path::bundle::Bundle>::from_components(ctx, &mut *func), + }); } - let field_len = field.len(); let generics = ast.generics; let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); let struct_name = &ast.ident; @@ -149,14 +123,13 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { fn component_ids( components: &mut #ecs_path::component::Components, storages: &mut #ecs_path::storage::Storages, - ) -> ::std::vec::Vec<#ecs_path::component::ComponentId> { - let mut component_ids = ::std::vec::Vec::with_capacity(#field_len); + ids: &mut impl FnMut(#ecs_path::component::ComponentId) + ){ #(#field_component_ids)* - component_ids } - #[allow(unused_variables, unused_mut, non_snake_case)] - unsafe fn from_components<__T, __F>(ctx: &mut __T, mut func: __F) -> Self + #[allow(unused_variables, non_snake_case)] + unsafe fn from_components<__T, __F>(ctx: &mut __T, func: &mut __F) -> Self where __F: FnMut(&mut __T) -> #ecs_path::ptr::OwningPtr<'_> { @@ -165,8 +138,8 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { } } - #[allow(unused_variables, unused_mut, forget_copy, forget_ref)] - fn get_components(self, mut func: impl FnMut(#ecs_path::ptr::OwningPtr<'_>)) { + #[allow(unused_variables)] + fn get_components(self, func: &mut impl FnMut(#ecs_path::ptr::OwningPtr<'_>)) { #(#field_get_components)* } } diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 8eb7730289f49..d9124f2493b84 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -14,71 +14,124 @@ use bevy_ecs_macros::all_tuples; use bevy_ptr::OwningPtr; use std::{any::TypeId, collections::HashMap}; -/// An ordered collection of [`Component`]s. +/// The `Bundle` trait enables insertion and removal of [`Component`]s from an entity. /// -/// Commonly used for spawning entities and adding and removing components in bulk. This -/// trait is automatically implemented for tuples of components: `(ComponentA, ComponentB)` -/// is a very convenient shorthand when working with one-off collections of components. Note -/// that both the unit type `()` and `(ComponentA, )` are valid bundles. The unit bundle is -/// particularly useful for spawning multiple empty entities by using -/// [`Commands::spawn_batch`](crate::system::Commands::spawn_batch). +/// Implementors of the `Bundle` trait are called 'bundles'. /// -/// # Examples +/// Each bundle represents a static set of [`Component`] types. +/// Currently, bundles can only contain one of each [`Component`], and will +/// panic once initialised if this is not met. /// -/// Typically, you will simply use `#[derive(Bundle)]` when creating your own `Bundle`. Each -/// struct field is a component: +/// ## Insertion /// -/// ``` -/// # use bevy_ecs::prelude::*; -/// # #[derive(Component)] -/// # struct ComponentA; -/// # #[derive(Component)] -/// # struct ComponentB; -/// # #[derive(Component)] -/// # struct ComponentC; -/// # -/// #[derive(Bundle)] -/// struct MyBundle { -/// a: ComponentA, -/// b: ComponentB, -/// c: ComponentC, -/// } -/// ``` +/// The primary use for bundles is to add a useful collection of components to an entity. +/// +/// Adding a value of bundle to an entity will add the components from the set it +/// represents to the entity. +/// The values of these components are taken from the bundle. +/// If an entity already had one of these components, the entity's original component value +/// will be overwritten. +/// +/// Importantly, bundles are only their constituent set of components. +/// You **should not** use bundles as a unit of behaviour. +/// The behaviour of your app can only be considered in terms of components, as systems, +/// which drive the behaviour of a `bevy` application, operate on combinations of +/// components. +/// +/// This rule is also important because multiple bundles may contain the same component type, +/// calculated in different ways — adding both of these bundles to one entity +/// would create incoherent behaviour. +/// This would be unexpected if bundles were treated as an abstraction boundary, as +/// the abstraction would be unmaintainable for these cases. +/// For example, both `Camera3dBundle` and `Camera2dBundle` contain the `CameraRenderGraph` +/// component, but specifying different render graphs to use. +/// If the bundles were both added to the same entity, only one of these two bundles would work. +/// +/// For this reason, There is intentionally no [`Query`] to match whether an entity +/// 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. +/// +/// # Implementors +/// +/// Every type which implements [`Component`] also implements `Bundle`, since +/// [`Component`] types can be added to or removed from an entity. +/// +/// Additionally, [Tuples](`tuple`) of bundles are also [`Bundle`] (with up to 15 bundles). +/// These bundles contain the items of the 'inner' bundles. +/// This is a convenient shorthand which is primarily used when spawning entities. +/// For example, spawning an entity using the bundle `(SpriteBundle {...}, PlayerMarker)` +/// will spawn an entity with components required for a 2d sprite, and the `PlayerMarker` component. +/// +/// [`unit`], otherwise known as [`()`](`unit`), is a [`Bundle`] containing no components (since it +/// can also be considered as the empty tuple). +/// This can be useful for spawning large numbers of empty entities using +/// [`World::spawn_batch`](crate::world::World::spawn_batch). +/// +/// Tuple bundles can be nested, which can be used to create an anonymous bundle with more than +/// 15 items. +/// However, in most cases where this is required, the derive macro [`derive@Bundle`] should be +/// used instead. +/// The derived `Bundle` implementation contains the items of its fields, which all must +/// implement `Bundle`. +/// As explained above, this includes any [`Component`] type, and other derived bundles: /// -/// You can nest bundles using the `#[bundle]` attribute: /// ``` /// # use bevy_ecs::{component::Component, bundle::Bundle}; /// /// #[derive(Component)] -/// struct X(i32); +/// struct XPosition(i32); /// #[derive(Component)] -/// struct Y(u64); -/// #[derive(Component)] -/// struct Z(String); +/// struct YPosition(i32); /// /// #[derive(Bundle)] -/// struct A { -/// x: X, -/// y: Y, +/// struct PositionBundle { +/// // A bundle can contain components +/// x: XPosition, +/// y: YPosition, /// } /// /// #[derive(Bundle)] -/// struct B { -/// #[bundle] -/// a: A, -/// z: Z, +/// struct NamedPointBundle { +/// // Or other bundles +/// a: PositionBundle, +/// // In addition to more components +/// z: PointName, /// } +/// +/// #[derive(Component)] +/// struct PointName(String); /// ``` /// /// # Safety /// -/// - [`Bundle::component_ids`] must return the [`ComponentId`] for each component type in the -/// bundle, in the _exact_ order that [`Bundle::get_components`] is called. -/// - [`Bundle::from_components`] must call `func` exactly once for each [`ComponentId`] returned by -/// [`Bundle::component_ids`]. +/// 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 [`Bundle::get_components`] is called. +// - [`Bundle::from_components`] must call `func` exactly once for each [`ComponentId`] returned by +// [`Bundle::component_ids`]. pub unsafe trait Bundle: Send + Sync + 'static { /// Gets this [`Bundle`]'s component ids, in the order of this bundle's [`Component`]s - fn component_ids(components: &mut Components, storages: &mut Storages) -> Vec; + #[doc(hidden)] + fn component_ids( + components: &mut Components, + storages: &mut Storages, + ids: &mut impl FnMut(ComponentId), + ); /// Calls `func`, which should return data for each component in the bundle, in the order of /// this bundle's [`Component`]s @@ -86,52 +139,83 @@ pub unsafe trait Bundle: Send + Sync + 'static { /// # Safety /// Caller must return data for each component in the bundle, in the order of this bundle's /// [`Component`]s - unsafe fn from_components(ctx: &mut T, func: F) -> Self + #[doc(hidden)] + unsafe fn from_components(ctx: &mut T, func: &mut F) -> Self where - F: FnMut(&mut T) -> OwningPtr<'_>, + // Ensure that the `OwningPtr` is used correctly + F: for<'a> FnMut(&'a mut T) -> OwningPtr<'a>, Self: Sized; - /// Calls `func` on each value, in the order of this bundle's [`Component`]s. This will - /// [`std::mem::forget`] the bundle fields, so callers are responsible for dropping the fields - /// if that is desirable. - fn get_components(self, func: impl FnMut(OwningPtr<'_>)); + /// 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(OwningPtr<'_>)); +} + +// SAFETY: +// - `Bundle::component_ids` calls `ids` for C's component id (and nothing else) +// - `Bundle::get_components` is called exactly once for C. +// - `Bundle::from_components` calls `func` exactly once for C, which is the exact value returned by `Bundle::component_ids`. +unsafe impl Bundle for C { + fn component_ids( + components: &mut Components, + storages: &mut Storages, + ids: &mut impl FnMut(ComponentId), + ) { + ids(components.init_component::(storages)); + } + + 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, + { + // Safety: The id given in `component_ids` is for `Self` + func(ctx).read() + } + + fn get_components(self, func: &mut impl FnMut(OwningPtr<'_>)) { + OwningPtr::make(self, func); + } } macro_rules! tuple_impl { ($($name: ident),*) => { // SAFETY: - // - `Bundle::component_ids` returns the `ComponentId`s for each component type in the + // - `Bundle::component_ids` calls `ids` for each component type in the // bundle, in the exact order that `Bundle::get_components` is called. // - `Bundle::from_components` calls `func` exactly once for each `ComponentId` returned by `Bundle::component_ids`. - unsafe impl<$($name: Component),*> Bundle for ($($name,)*) { + unsafe impl<$($name: Bundle),*> Bundle for ($($name,)*) { #[allow(unused_variables)] - fn component_ids(components: &mut Components, storages: &mut Storages) -> Vec { - vec![$(components.init_component::<$name>(storages)),*] + fn component_ids(components: &mut Components, storages: &mut Storages, ids: &mut impl FnMut(ComponentId)){ + $(<$name as Bundle>::component_ids(components, storages, ids);)* } #[allow(unused_variables, unused_mut)] #[allow(clippy::unused_unit)] - unsafe fn from_components(ctx: &mut T, mut func: F) -> Self + unsafe fn from_components(ctx: &mut T, func: &mut F) -> Self where F: FnMut(&mut T) -> OwningPtr<'_> { - #[allow(non_snake_case)] - ($(func(ctx).read::<$name>(),)*) + // Rust guarantees that tuple calls are evaluated 'left to right'. + // https://doc.rust-lang.org/reference/expressions.html#evaluation-order-of-operands + ($(<$name as Bundle>::from_components(ctx, func),)*) } #[allow(unused_variables, unused_mut)] - fn get_components(self, mut func: impl FnMut(OwningPtr<'_>)) { + fn get_components(self, func: &mut impl FnMut(OwningPtr<'_>)) { #[allow(non_snake_case)] let ($(mut $name,)*) = self; $( - OwningPtr::make($name, &mut func); + $name.get_components(&mut *func); )* } } } } -all_tuples!(tuple_impl, 0, 15, C); +all_tuples!(tuple_impl, 0, 15, B); #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] pub struct BundleId(usize); @@ -279,7 +363,7 @@ impl BundleInfo { // NOTE: get_components calls this closure on each component in "bundle order". // bundle_info.component_ids are also in "bundle order" let mut bundle_component = 0; - bundle.get_components(|component_ptr| { + bundle.get_components(&mut |component_ptr| { let component_id = *self.component_ids.get_unchecked(bundle_component); match self.storage_types[bundle_component] { StorageType::Table => { @@ -601,7 +685,8 @@ impl Bundles { ) -> &'a BundleInfo { let bundle_infos = &mut self.bundle_infos; let id = self.bundle_ids.entry(TypeId::of::()).or_insert_with(|| { - let component_ids = T::component_ids(components, storages); + let mut component_ids = Vec::new(); + T::component_ids(components, storages, &mut |id| component_ids.push(id)); let id = BundleId(bundle_infos.len()); // SAFETY: T::component_id ensures info was created let bundle_info = unsafe { diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index f2328fdf0de7a..87affbaaecb5c 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -140,10 +140,14 @@ mod tests { x: TableStored, y: SparseStored, } + let mut ids = Vec::new(); + ::component_ids(&mut world.components, &mut world.storages, &mut |id| { + ids.push(id); + }); assert_eq!( - ::component_ids(&mut world.components, &mut world.storages), - vec![ + ids, + &[ world.init_component::(), world.init_component::(), ] @@ -184,14 +188,18 @@ mod tests { #[derive(Bundle, PartialEq, Debug)] struct Nested { a: A, - #[bundle] foo: Foo, b: B, } + let mut ids = Vec::new(); + ::component_ids(&mut world.components, &mut world.storages, &mut |id| { + ids.push(id); + }); + assert_eq!( - ::component_ids(&mut world.components, &mut world.storages), - vec![ + ids, + &[ world.init_component::(), world.init_component::(), world.init_component::(), diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 02d5372ed9f1f..b59a5c2c84d57 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -293,7 +293,7 @@ impl<'w> EntityMut<'w> { // SAFETY: bundle components are iterated in order, which guarantees that the component type // matches let result = unsafe { - T::from_components(storages, |storages| { + T::from_components(storages, &mut |storages| { let component_id = bundle_components.next().unwrap(); // SAFETY: entity location is valid and table row is removed below take_component( diff --git a/examples/ecs/iter_combinations.rs b/examples/ecs/iter_combinations.rs index 21dcac8b58c6d..48d3f0f6ab971 100644 --- a/examples/ecs/iter_combinations.rs +++ b/examples/ecs/iter_combinations.rs @@ -43,7 +43,6 @@ struct Star; #[derive(Bundle, Default)] struct BodyBundle { - #[bundle] pbr: PbrBundle, mass: Mass, last_pos: LastPos, diff --git a/examples/games/breakout.rs b/examples/games/breakout.rs index c2d592b4c7e3c..178a282c60d20 100644 --- a/examples/games/breakout.rs +++ b/examples/games/breakout.rs @@ -97,7 +97,6 @@ struct CollisionSound(Handle); struct WallBundle { // You can nest bundles inside of other bundles like this // Allowing you to compose their functionality - #[bundle] sprite_bundle: SpriteBundle, collider: Collider, }