Skip to content

Commit

Permalink
Implement Bundle for Component. Use Bundle tuples for insertion (
Browse files Browse the repository at this point in the history 
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]>
  • Loading branch information
@DJMcNab @cart @james7132
2 people authored and james7132 committed Oct 28, 2022
1 parent d057794 commit 3fa4d69
Show file tree
Hide file tree
Showing 6 changed files with 178 additions and 114 deletions.
63 changes: 18 additions & 45 deletions crates/bevy_ecs/macros/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),*);
}
Expand All @@ -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();
Expand All @@ -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::<Vec<bool>>();
let field = named_fields
.iter()
.map(|field| field.ident.as_ref().unwrap())
Expand All @@ -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;
Expand All @@ -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<'_>
{
Expand All @@ -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)*
}
}
Expand Down
207 changes: 146 additions & 61 deletions crates/bevy_ecs/src/bundle.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,124 +14,208 @@ 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 &mdash; 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<ComponentId>;
#[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
///
/// # Safety
/// Caller must return data for each component in the bundle, in the order of this bundle's
/// [`Component`]s
unsafe fn from_components<T, F>(ctx: &mut T, func: F) -> Self
#[doc(hidden)]
unsafe fn from_components<T, F>(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<C: Component> Bundle for C {
fn component_ids(
components: &mut Components,
storages: &mut Storages,
ids: &mut impl FnMut(ComponentId),
) {
ids(components.init_component::<C>(storages));
}

unsafe fn from_components<T, F>(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<ComponentId> {
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<T, F>(ctx: &mut T, mut func: F) -> Self
unsafe fn from_components<T, F>(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);
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -601,7 +685,8 @@ impl Bundles {
) -> &'a BundleInfo {
let bundle_infos = &mut self.bundle_infos;
let id = self.bundle_ids.entry(TypeId::of::<T>()).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 {
Expand Down
Loading

0 comments on commit 3fa4d69

Please sign in to comment.