Skip to content

Commit eda8509

Browse files
DJMcNabcart
authored andcommitted
Implement Bundle for Component. Use Bundle tuples for insertion (bevyengine#2975)
@BoxyUwU this is your fault. Also cart didn't arrive in time to tell us not to do this. # Objective - Fix bevyengine#2974 ## Solution - The first commit just does the actual change - Follow up commits do steps to prove that this method works to unify as required, but this does not remove `insert_bundle`. ## Changelog ### Changed Nested bundles now collapse automatically, and every `Component` now implements `Bundle`. This means that you can combine bundles and components arbitrarily, for example: ```rust // before: .insert(A).insert_bundle(MyBBundle{..}) // after: .insert_bundle((A, MyBBundle {..})) ``` Note that there will be a follow up PR that removes the current `insert` impl and renames `insert_bundle` to `insert`. ### Removed The `bundle` attribute in `derive(Bundle)`. ## Migration guide In `derive(Bundle)`, the `bundle` attribute has been removed. Nested bundles are not collapsed automatically. You should remove `#[bundle]` attributes. Co-authored-by: Carter Anderson <[email protected]>
1 parent cfbf036 commit eda8509

File tree

6 files changed

+178
-114
lines changed

6 files changed

+178
-114
lines changed

crates/bevy_ecs/macros/src/lib.rs

+18-45
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ pub fn all_tuples(input: TokenStream) -> TokenStream {
6868

6969
let macro_ident = &input.macro_ident;
7070
let invocations = (input.start..=input.end).map(|i| {
71-
let ident_tuples = &ident_tuples[0..i - input.start];
71+
let ident_tuples = &ident_tuples[..i];
7272
quote! {
7373
#macro_ident!(#(#ident_tuples),*);
7474
}
@@ -80,9 +80,7 @@ pub fn all_tuples(input: TokenStream) -> TokenStream {
8080
})
8181
}
8282

83-
static BUNDLE_ATTRIBUTE_NAME: &str = "bundle";
84-
85-
#[proc_macro_derive(Bundle, attributes(bundle))]
83+
#[proc_macro_derive(Bundle)]
8684
pub fn derive_bundle(input: TokenStream) -> TokenStream {
8785
let ast = parse_macro_input!(input as DeriveInput);
8886
let ecs_path = bevy_ecs_path();
@@ -92,15 +90,6 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream {
9290
Err(e) => return e.into_compile_error().into(),
9391
};
9492

95-
let is_bundle = named_fields
96-
.iter()
97-
.map(|field| {
98-
field
99-
.attrs
100-
.iter()
101-
.any(|a| *a.path.get_ident().as_ref().unwrap() == BUNDLE_ATTRIBUTE_NAME)
102-
})
103-
.collect::<Vec<bool>>();
10493
let field = named_fields
10594
.iter()
10695
.map(|field| field.ident.as_ref().unwrap())
@@ -113,32 +102,17 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream {
113102
let mut field_component_ids = Vec::new();
114103
let mut field_get_components = Vec::new();
115104
let mut field_from_components = Vec::new();
116-
for ((field_type, is_bundle), field) in
117-
field_type.iter().zip(is_bundle.iter()).zip(field.iter())
118-
{
119-
if *is_bundle {
120-
field_component_ids.push(quote! {
121-
component_ids.extend(<#field_type as #ecs_path::bundle::Bundle>::component_ids(components, storages));
122-
});
123-
field_get_components.push(quote! {
124-
self.#field.get_components(&mut func);
125-
});
126-
field_from_components.push(quote! {
127-
#field: <#field_type as #ecs_path::bundle::Bundle>::from_components(ctx, &mut func),
128-
});
129-
} else {
130-
field_component_ids.push(quote! {
131-
component_ids.push(components.init_component::<#field_type>(storages));
132-
});
133-
field_get_components.push(quote! {
134-
#ecs_path::ptr::OwningPtr::make(self.#field, &mut func);
135-
});
136-
field_from_components.push(quote! {
137-
#field: func(ctx).read::<#field_type>(),
138-
});
139-
}
105+
for (field_type, field) in field_type.iter().zip(field.iter()) {
106+
field_component_ids.push(quote! {
107+
<#field_type as #ecs_path::bundle::Bundle>::component_ids(components, storages, &mut *ids);
108+
});
109+
field_get_components.push(quote! {
110+
self.#field.get_components(&mut *func);
111+
});
112+
field_from_components.push(quote! {
113+
#field: <#field_type as #ecs_path::bundle::Bundle>::from_components(ctx, &mut *func),
114+
});
140115
}
141-
let field_len = field.len();
142116
let generics = ast.generics;
143117
let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();
144118
let struct_name = &ast.ident;
@@ -149,14 +123,13 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream {
149123
fn component_ids(
150124
components: &mut #ecs_path::component::Components,
151125
storages: &mut #ecs_path::storage::Storages,
152-
) -> ::std::vec::Vec<#ecs_path::component::ComponentId> {
153-
let mut component_ids = ::std::vec::Vec::with_capacity(#field_len);
126+
ids: &mut impl FnMut(#ecs_path::component::ComponentId)
127+
){
154128
#(#field_component_ids)*
155-
component_ids
156129
}
157130

158-
#[allow(unused_variables, unused_mut, non_snake_case)]
159-
unsafe fn from_components<__T, __F>(ctx: &mut __T, mut func: __F) -> Self
131+
#[allow(unused_variables, non_snake_case)]
132+
unsafe fn from_components<__T, __F>(ctx: &mut __T, func: &mut __F) -> Self
160133
where
161134
__F: FnMut(&mut __T) -> #ecs_path::ptr::OwningPtr<'_>
162135
{
@@ -165,8 +138,8 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream {
165138
}
166139
}
167140

168-
#[allow(unused_variables, unused_mut, forget_copy, forget_ref)]
169-
fn get_components(self, mut func: impl FnMut(#ecs_path::ptr::OwningPtr<'_>)) {
141+
#[allow(unused_variables)]
142+
fn get_components(self, func: &mut impl FnMut(#ecs_path::ptr::OwningPtr<'_>)) {
170143
#(#field_get_components)*
171144
}
172145
}

crates/bevy_ecs/src/bundle.rs

+146-61
Original file line numberDiff line numberDiff line change
@@ -14,124 +14,208 @@ use bevy_ecs_macros::all_tuples;
1414
use bevy_ptr::OwningPtr;
1515
use std::{any::TypeId, collections::HashMap};
1616

17-
/// An ordered collection of [`Component`]s.
17+
/// The `Bundle` trait enables insertion and removal of [`Component`]s from an entity.
1818
///
19-
/// Commonly used for spawning entities and adding and removing components in bulk. This
20-
/// trait is automatically implemented for tuples of components: `(ComponentA, ComponentB)`
21-
/// is a very convenient shorthand when working with one-off collections of components. Note
22-
/// that both the unit type `()` and `(ComponentA, )` are valid bundles. The unit bundle is
23-
/// particularly useful for spawning multiple empty entities by using
24-
/// [`Commands::spawn_batch`](crate::system::Commands::spawn_batch).
19+
/// Implementors of the `Bundle` trait are called 'bundles'.
2520
///
26-
/// # Examples
21+
/// Each bundle represents a static set of [`Component`] types.
22+
/// Currently, bundles can only contain one of each [`Component`], and will
23+
/// panic once initialised if this is not met.
2724
///
28-
/// Typically, you will simply use `#[derive(Bundle)]` when creating your own `Bundle`. Each
29-
/// struct field is a component:
25+
/// ## Insertion
3026
///
31-
/// ```
32-
/// # use bevy_ecs::prelude::*;
33-
/// # #[derive(Component)]
34-
/// # struct ComponentA;
35-
/// # #[derive(Component)]
36-
/// # struct ComponentB;
37-
/// # #[derive(Component)]
38-
/// # struct ComponentC;
39-
/// #
40-
/// #[derive(Bundle)]
41-
/// struct MyBundle {
42-
/// a: ComponentA,
43-
/// b: ComponentB,
44-
/// c: ComponentC,
45-
/// }
46-
/// ```
27+
/// The primary use for bundles is to add a useful collection of components to an entity.
28+
///
29+
/// Adding a value of bundle to an entity will add the components from the set it
30+
/// represents to the entity.
31+
/// The values of these components are taken from the bundle.
32+
/// If an entity already had one of these components, the entity's original component value
33+
/// will be overwritten.
34+
///
35+
/// Importantly, bundles are only their constituent set of components.
36+
/// You **should not** use bundles as a unit of behaviour.
37+
/// The behaviour of your app can only be considered in terms of components, as systems,
38+
/// which drive the behaviour of a `bevy` application, operate on combinations of
39+
/// components.
40+
///
41+
/// This rule is also important because multiple bundles may contain the same component type,
42+
/// calculated in different ways &mdash; adding both of these bundles to one entity
43+
/// would create incoherent behaviour.
44+
/// This would be unexpected if bundles were treated as an abstraction boundary, as
45+
/// the abstraction would be unmaintainable for these cases.
46+
/// For example, both `Camera3dBundle` and `Camera2dBundle` contain the `CameraRenderGraph`
47+
/// component, but specifying different render graphs to use.
48+
/// If the bundles were both added to the same entity, only one of these two bundles would work.
49+
///
50+
/// For this reason, There is intentionally no [`Query`] to match whether an entity
51+
/// contains the components of a bundle.
52+
/// Queries should instead only select the components they logically operate on.
53+
///
54+
/// ## Removal
55+
///
56+
/// Bundles are also used when removing components from an entity.
57+
///
58+
/// Removing a bundle from an entity will remove any of its components attached
59+
/// to the entity from the entity.
60+
/// That is, if the entity does not have all the components of the bundle, those
61+
/// which are present will be removed.
62+
///
63+
/// # Implementors
64+
///
65+
/// Every type which implements [`Component`] also implements `Bundle`, since
66+
/// [`Component`] types can be added to or removed from an entity.
67+
///
68+
/// Additionally, [Tuples](`tuple`) of bundles are also [`Bundle`] (with up to 15 bundles).
69+
/// These bundles contain the items of the 'inner' bundles.
70+
/// This is a convenient shorthand which is primarily used when spawning entities.
71+
/// For example, spawning an entity using the bundle `(SpriteBundle {...}, PlayerMarker)`
72+
/// will spawn an entity with components required for a 2d sprite, and the `PlayerMarker` component.
73+
///
74+
/// [`unit`], otherwise known as [`()`](`unit`), is a [`Bundle`] containing no components (since it
75+
/// can also be considered as the empty tuple).
76+
/// This can be useful for spawning large numbers of empty entities using
77+
/// [`World::spawn_batch`](crate::world::World::spawn_batch).
78+
///
79+
/// Tuple bundles can be nested, which can be used to create an anonymous bundle with more than
80+
/// 15 items.
81+
/// However, in most cases where this is required, the derive macro [`derive@Bundle`] should be
82+
/// used instead.
83+
/// The derived `Bundle` implementation contains the items of its fields, which all must
84+
/// implement `Bundle`.
85+
/// As explained above, this includes any [`Component`] type, and other derived bundles:
4786
///
48-
/// You can nest bundles using the `#[bundle]` attribute:
4987
/// ```
5088
/// # use bevy_ecs::{component::Component, bundle::Bundle};
5189
///
5290
/// #[derive(Component)]
53-
/// struct X(i32);
91+
/// struct XPosition(i32);
5492
/// #[derive(Component)]
55-
/// struct Y(u64);
56-
/// #[derive(Component)]
57-
/// struct Z(String);
93+
/// struct YPosition(i32);
5894
///
5995
/// #[derive(Bundle)]
60-
/// struct A {
61-
/// x: X,
62-
/// y: Y,
96+
/// struct PositionBundle {
97+
/// // A bundle can contain components
98+
/// x: XPosition,
99+
/// y: YPosition,
63100
/// }
64101
///
65102
/// #[derive(Bundle)]
66-
/// struct B {
67-
/// #[bundle]
68-
/// a: A,
69-
/// z: Z,
103+
/// struct NamedPointBundle {
104+
/// // Or other bundles
105+
/// a: PositionBundle,
106+
/// // In addition to more components
107+
/// z: PointName,
70108
/// }
109+
///
110+
/// #[derive(Component)]
111+
/// struct PointName(String);
71112
/// ```
72113
///
73114
/// # Safety
74115
///
75-
/// - [`Bundle::component_ids`] must return the [`ComponentId`] for each component type in the
76-
/// bundle, in the _exact_ order that [`Bundle::get_components`] is called.
77-
/// - [`Bundle::from_components`] must call `func` exactly once for each [`ComponentId`] returned by
78-
/// [`Bundle::component_ids`].
116+
/// Manual implementations of this trait are unsupported.
117+
/// That is, there is no safe way to implement this trait, and you must not do so.
118+
/// If you want a type to implement [`Bundle`], you must use [`derive@Bundle`](derive@Bundle).
119+
///
120+
///
121+
/// [`Query`]: crate::system::Query
122+
// Some safety points:
123+
// - [`Bundle::component_ids`] must return the [`ComponentId`] for each component type in the
124+
// bundle, in the _exact_ order that [`Bundle::get_components`] is called.
125+
// - [`Bundle::from_components`] must call `func` exactly once for each [`ComponentId`] returned by
126+
// [`Bundle::component_ids`].
79127
pub unsafe trait Bundle: Send + Sync + 'static {
80128
/// Gets this [`Bundle`]'s component ids, in the order of this bundle's [`Component`]s
81-
fn component_ids(components: &mut Components, storages: &mut Storages) -> Vec<ComponentId>;
129+
#[doc(hidden)]
130+
fn component_ids(
131+
components: &mut Components,
132+
storages: &mut Storages,
133+
ids: &mut impl FnMut(ComponentId),
134+
);
82135

83136
/// Calls `func`, which should return data for each component in the bundle, in the order of
84137
/// this bundle's [`Component`]s
85138
///
86139
/// # Safety
87140
/// Caller must return data for each component in the bundle, in the order of this bundle's
88141
/// [`Component`]s
89-
unsafe fn from_components<T, F>(ctx: &mut T, func: F) -> Self
142+
#[doc(hidden)]
143+
unsafe fn from_components<T, F>(ctx: &mut T, func: &mut F) -> Self
90144
where
91-
F: FnMut(&mut T) -> OwningPtr<'_>,
145+
// Ensure that the `OwningPtr` is used correctly
146+
F: for<'a> FnMut(&'a mut T) -> OwningPtr<'a>,
92147
Self: Sized;
93148

94-
/// Calls `func` on each value, in the order of this bundle's [`Component`]s. This will
95-
/// [`std::mem::forget`] the bundle fields, so callers are responsible for dropping the fields
96-
/// if that is desirable.
97-
fn get_components(self, func: impl FnMut(OwningPtr<'_>));
149+
/// Calls `func` on each value, in the order of this bundle's [`Component`]s. This passes
150+
/// ownership of the component values to `func`.
151+
#[doc(hidden)]
152+
fn get_components(self, func: &mut impl FnMut(OwningPtr<'_>));
153+
}
154+
155+
// SAFETY:
156+
// - `Bundle::component_ids` calls `ids` for C's component id (and nothing else)
157+
// - `Bundle::get_components` is called exactly once for C.
158+
// - `Bundle::from_components` calls `func` exactly once for C, which is the exact value returned by `Bundle::component_ids`.
159+
unsafe impl<C: Component> Bundle for C {
160+
fn component_ids(
161+
components: &mut Components,
162+
storages: &mut Storages,
163+
ids: &mut impl FnMut(ComponentId),
164+
) {
165+
ids(components.init_component::<C>(storages));
166+
}
167+
168+
unsafe fn from_components<T, F>(ctx: &mut T, func: &mut F) -> Self
169+
where
170+
// Ensure that the `OwningPtr` is used correctly
171+
F: for<'a> FnMut(&'a mut T) -> OwningPtr<'a>,
172+
Self: Sized,
173+
{
174+
// Safety: The id given in `component_ids` is for `Self`
175+
func(ctx).read()
176+
}
177+
178+
fn get_components(self, func: &mut impl FnMut(OwningPtr<'_>)) {
179+
OwningPtr::make(self, func);
180+
}
98181
}
99182

100183
macro_rules! tuple_impl {
101184
($($name: ident),*) => {
102185
// SAFETY:
103-
// - `Bundle::component_ids` returns the `ComponentId`s for each component type in the
186+
// - `Bundle::component_ids` calls `ids` for each component type in the
104187
// bundle, in the exact order that `Bundle::get_components` is called.
105188
// - `Bundle::from_components` calls `func` exactly once for each `ComponentId` returned by `Bundle::component_ids`.
106-
unsafe impl<$($name: Component),*> Bundle for ($($name,)*) {
189+
unsafe impl<$($name: Bundle),*> Bundle for ($($name,)*) {
107190
#[allow(unused_variables)]
108-
fn component_ids(components: &mut Components, storages: &mut Storages) -> Vec<ComponentId> {
109-
vec![$(components.init_component::<$name>(storages)),*]
191+
fn component_ids(components: &mut Components, storages: &mut Storages, ids: &mut impl FnMut(ComponentId)){
192+
$(<$name as Bundle>::component_ids(components, storages, ids);)*
110193
}
111194

112195
#[allow(unused_variables, unused_mut)]
113196
#[allow(clippy::unused_unit)]
114-
unsafe fn from_components<T, F>(ctx: &mut T, mut func: F) -> Self
197+
unsafe fn from_components<T, F>(ctx: &mut T, func: &mut F) -> Self
115198
where
116199
F: FnMut(&mut T) -> OwningPtr<'_>
117200
{
118-
#[allow(non_snake_case)]
119-
($(func(ctx).read::<$name>(),)*)
201+
// Rust guarantees that tuple calls are evaluated 'left to right'.
202+
// https://doc.rust-lang.org/reference/expressions.html#evaluation-order-of-operands
203+
($(<$name as Bundle>::from_components(ctx, func),)*)
120204
}
121205

122206
#[allow(unused_variables, unused_mut)]
123-
fn get_components(self, mut func: impl FnMut(OwningPtr<'_>)) {
207+
fn get_components(self, func: &mut impl FnMut(OwningPtr<'_>)) {
124208
#[allow(non_snake_case)]
125209
let ($(mut $name,)*) = self;
126210
$(
127-
OwningPtr::make($name, &mut func);
211+
$name.get_components(&mut *func);
128212
)*
129213
}
130214
}
131215
}
132216
}
133217

134-
all_tuples!(tuple_impl, 0, 15, C);
218+
all_tuples!(tuple_impl, 0, 15, B);
135219

136220
#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
137221
pub struct BundleId(usize);
@@ -279,7 +363,7 @@ impl BundleInfo {
279363
// NOTE: get_components calls this closure on each component in "bundle order".
280364
// bundle_info.component_ids are also in "bundle order"
281365
let mut bundle_component = 0;
282-
bundle.get_components(|component_ptr| {
366+
bundle.get_components(&mut |component_ptr| {
283367
let component_id = *self.component_ids.get_unchecked(bundle_component);
284368
match self.storage_types[bundle_component] {
285369
StorageType::Table => {
@@ -601,7 +685,8 @@ impl Bundles {
601685
) -> &'a BundleInfo {
602686
let bundle_infos = &mut self.bundle_infos;
603687
let id = self.bundle_ids.entry(TypeId::of::<T>()).or_insert_with(|| {
604-
let component_ids = T::component_ids(components, storages);
688+
let mut component_ids = Vec::new();
689+
T::component_ids(components, storages, &mut |id| component_ids.push(id));
605690
let id = BundleId(bundle_infos.len());
606691
// SAFETY: T::component_id ensures info was created
607692
let bundle_info = unsafe {

0 commit comments

Comments
 (0)