Skip to content

Commit

Permalink
Required Components (#14791)
Browse files Browse the repository at this point in the history
## Introduction

This is the first step in my [Next Generation Scene / UI
Proposal](#14437).

Fixes #7272 #14800.

Bevy's current Bundles as the "unit of construction" hamstring the UI
user experience and have been a pain point in the Bevy ecosystem
generally when composing scenes:

* They are an additional _object defining_ concept, which must be
learned separately from components. Notably, Bundles _are not present at
runtime_, which is confusing and limiting.
* They can completely erase the _defining component_ during Bundle init.
For example, `ButtonBundle { style: Style::default(), ..default() }`
_makes no mention_ of the `Button` component symbol, which is what makes
the Entity a "button"!
* They are not capable of representing "dependency inheritance" without
completely non-viable / ergonomically crushing nested bundles. This
limitation is especially painful in UI scenarios, but it applies to
everything across the board.
* They introduce a bunch of additional nesting when defining scenes,
making them ugly to look at
* They introduce component name "stutter": `SomeBundle { component_name:
ComponentName::new() }`
* They require copious sprinklings of `..default()` when spawning them
in Rust code, due to the additional layer of nesting

**Required Components** solve this by allowing you to define which
components a given component needs, and how to construct those
components when they aren't explicitly provided.

This is what a `ButtonBundle` looks like with Bundles (the current
approach):

```rust
#[derive(Component, Default)]
struct Button;

#[derive(Bundle, Default)]
struct ButtonBundle {
    pub button: Button,
    pub node: Node,
    pub style: Style,
    pub interaction: Interaction,
    pub focus_policy: FocusPolicy,
    pub border_color: BorderColor,
    pub border_radius: BorderRadius,
    pub image: UiImage,
    pub transform: Transform,
    pub global_transform: GlobalTransform,
    pub visibility: Visibility,
    pub inherited_visibility: InheritedVisibility,
    pub view_visibility: ViewVisibility,
    pub z_index: ZIndex,
}

commands.spawn(ButtonBundle {
    style: Style {
        width: Val::Px(100.0),
        height: Val::Px(50.0),
        ..default()
    },
    focus_policy: FocusPolicy::Block,
    ..default()
})
```

And this is what it looks like with Required Components:

```rust
#[derive(Component)]
#[require(Node, UiImage)]
struct Button;

commands.spawn((
    Button,
    Style { 
        width: Val::Px(100.0),
        height: Val::Px(50.0),
        ..default()
    },
    FocusPolicy::Block,
));
```

With Required Components, we mention only the most relevant components.
Every component required by `Node` (ex: `Style`, `FocusPolicy`, etc) is
automatically brought in!

### Efficiency

1. At insertion/spawn time, Required Components (including recursive
required components) are initialized and inserted _as if they were
manually inserted alongside the given components_. This means that this
is maximally efficient: there are no archetype or table moves.
2. Required components are only initialized and inserted if they were
not manually provided by the developer. For the code example in the
previous section, because `Style` and `FocusPolicy` are inserted
manually, they _will not_ be initialized and inserted as part of the
required components system. Efficient!
3. The "missing required components _and_ constructors needed for an
insertion" are cached in the "archetype graph edge", meaning they aren't
computed per-insertion. When a component is inserted, the "missing
required components" list is iterated (and that graph edge (AddBundle)
is actually already looked up for us during insertion, because we need
that for "normal" insert logic too).

### IDE Integration

The `#[require(SomeComponent)]` macro has been written in such a way
that Rust Analyzer can provide type-inspection-on-hover and `F12` /
go-to-definition for required components.

### Custom Constructors

The `require` syntax expects a `Default` constructor by default, but it
can be overridden with a custom constructor:

```rust
#[derive(Component)]
#[require(
    Node,
    Style(button_style),
    UiImage
)]
struct Button;

fn button_style() -> Style {
    Style {
        width: Val::Px(100.0),
        ..default()
    }
}
```

### Multiple Inheritance

You may have noticed by now that this behaves a bit like "multiple
inheritance". One of the problems that this presents is that it is
possible to have duplicate requires for a given type at different levels
of the inheritance tree:

```rust
#[derive(Component)
struct X(usize);

#[derive(Component)]
#[require(X(x1))
struct Y;

fn x1() -> X {
    X(1)
}

#[derive(Component)]
#[require(
    Y,
    X(x2),
)]
struct Z;

fn x2() -> X {
    X(2)
}

// What version of X is inserted for Z?
commands.spawn(Z);
```

This is allowed (and encouraged), although this doesn't appear to occur
much in practice. First: only one version of `X` is initialized and
inserted for `Z`. In the case above, I think we can all probably agree
that it makes the most sense to use the `x2` constructor for `X`,
because `Y`'s `x1` constructor exists "beneath" `Z` in the inheritance
hierarchy; `Z`'s constructor is "more specific".

The algorithm is simple and predictable:

1. Use all of the constructors (including default constructors) directly
defined in the spawned component's require list
2. In the order the requires are defined in `#[require()]`, recursively
visit the require list of each of the components in the list (this is a
depth Depth First Search). When a constructor is found, it will only be
used if one has not already been found.

From a user perspective, just think about this as the following:

1. Specifying a required component constructor for `Foo` directly on a
spawned component `Bar` will result in that constructor being used (and
overriding existing constructors lower in the inheritance tree). This is
the classic "inheritance override" behavior people expect.
2. For cases where "multiple inheritance" results in constructor
clashes, Components should be listed in "importance order". List a
component earlier in the requirement list to initialize its inheritance
tree earlier.

Required Components _does_ generally result in a model where component
values are decoupled from each other at construction time. Notably, some
existing Bundle patterns use bundle constructors to initialize multiple
components with shared state. I think (in general) moving away from this
is necessary:

1. It allows Required Components (and the Scene system more generally)
to operate according to simple rules
2. The "do arbitrary init value sharing in Bundle constructors" approach
_already_ causes data consistency problems, and those problems would be
exacerbated in the context of a Scene/UI system. For cases where shared
state is truly necessary, I think we are better served by observers /
hooks.
3. If a situation _truly_ needs shared state constructors (which should
be rare / generally discouraged), Bundles are still there if they are
needed.

## Next Steps

* **Require Construct-ed Components**: I have already implemented this
(as defined in the [Next Generation Scene / UI
Proposal](#14437). However
I've removed `Construct` support from this PR, as that has not landed
yet. Adding this back in requires relatively minimal changes to the
current impl, and can be done as part of a future Construct pr.
* **Port Built-in Bundles to Required Components**: This isn't something
we should do right away. It will require rethinking our public
interfaces, which IMO should be done holistically after the rest of Next
Generation Scene / UI lands. I think we should merge this PR first and
let people experiment _inside their own code with their own Components_
while we wait for the rest of the new scene system to land.
* **_Consider_ Automatic Required Component Removal**: We should
evaluate _if_ automatic Required Component removal should be done. Ex:
if all components that explicitly require a component are removed,
automatically remove that component. This issue has been explicitly
deferred in this PR, as I consider the insertion behavior to be
desirable on its own (and viable on its own). I am also doubtful that we
can find a design that has behavior we actually want. Aka: can we
_really_ distinguish between a component that is "only there because it
was automatically inserted" and "a component that was necessary / should
be kept". See my [discussion response
here](#14437 (reply in thread))
for more details.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com>
Co-authored-by: Pascal Hertleif <killercup@gmail.com>
  • Loading branch information
4 people authored Aug 27, 2024
1 parent 5f061ea commit 9cdb915
Show file tree
Hide file tree
Showing 10 changed files with 987 additions and 113 deletions.
153 changes: 124 additions & 29 deletions crates/bevy_ecs/macros/src/component.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
use proc_macro::TokenStream;
use proc_macro2::{Span, TokenStream as TokenStream2};
use quote::quote;
use syn::{parse_macro_input, parse_quote, DeriveInput, ExprPath, Ident, LitStr, Path, Result};
use quote::{quote, ToTokens};
use std::collections::HashSet;
use syn::{
parenthesized,
parse::Parse,
parse_macro_input, parse_quote,
punctuated::Punctuated,
spanned::Spanned,
token::{Comma, Paren},
DeriveInput, ExprPath, Ident, LitStr, Path, Result,
};

pub fn derive_event(input: TokenStream) -> TokenStream {
let mut ast = parse_macro_input!(input as DeriveInput);
Expand Down Expand Up @@ -66,12 +75,55 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
.predicates
.push(parse_quote! { Self: Send + Sync + 'static });

let requires = &attrs.requires;
let mut register_required = Vec::with_capacity(attrs.requires.iter().len());
let mut register_recursive_requires = Vec::with_capacity(attrs.requires.iter().len());
if let Some(requires) = requires {
for require in requires {
let ident = &require.path;
register_recursive_requires.push(quote! {
<#ident as Component>::register_required_components(components, storages, required_components);
});
if let Some(func) = &require.func {
register_required.push(quote! {
required_components.register(components, storages, || { let x: #ident = #func().into(); x });
});
} else {
register_required.push(quote! {
required_components.register(components, storages, <#ident as Default>::default);
});
}
}
}
let struct_name = &ast.ident;
let (impl_generics, type_generics, where_clause) = &ast.generics.split_for_impl();

let required_component_docs = attrs.requires.map(|r| {
let paths = r
.iter()
.map(|r| format!("[`{}`]", r.path.to_token_stream()))
.collect::<Vec<_>>()
.join(", ");
let doc = format!("Required Components: {paths}. \n\n A component's Required Components are inserted whenever it is inserted. Note that this will also insert the required components _of_ the required components, recursively, in depth-first order.");
quote! {
#[doc = #doc]
}
});

// This puts `register_required` before `register_recursive_requires` to ensure that the constructors of _all_ top
// level components are initialized first, giving them precedence over recursively defined constructors for the same component type
TokenStream::from(quote! {
#required_component_docs
impl #impl_generics #bevy_ecs_path::component::Component for #struct_name #type_generics #where_clause {
const STORAGE_TYPE: #bevy_ecs_path::component::StorageType = #storage;
fn register_required_components(
components: &mut #bevy_ecs_path::component::Components,
storages: &mut #bevy_ecs_path::storage::Storages,
required_components: &mut #bevy_ecs_path::component::RequiredComponents
) {
#(#register_required)*
#(#register_recursive_requires)*
}

#[allow(unused_variables)]
fn register_component_hooks(hooks: &mut #bevy_ecs_path::component::ComponentHooks) {
Expand All @@ -86,13 +138,16 @@ pub fn derive_component(input: TokenStream) -> TokenStream {

pub const COMPONENT: &str = "component";
pub const STORAGE: &str = "storage";
pub const REQUIRE: &str = "require";

pub const ON_ADD: &str = "on_add";
pub const ON_INSERT: &str = "on_insert";
pub const ON_REPLACE: &str = "on_replace";
pub const ON_REMOVE: &str = "on_remove";

struct Attrs {
storage: StorageTy,
requires: Option<Punctuated<Require, Comma>>,
on_add: Option<ExprPath>,
on_insert: Option<ExprPath>,
on_replace: Option<ExprPath>,
Expand All @@ -105,6 +160,11 @@ enum StorageTy {
SparseSet,
}

struct Require {
path: Path,
func: Option<Path>,
}

// values for `storage` attribute
const TABLE: &str = "Table";
const SPARSE_SET: &str = "SparseSet";
Expand All @@ -116,42 +176,77 @@ fn parse_component_attr(ast: &DeriveInput) -> Result<Attrs> {
on_insert: None,
on_replace: None,
on_remove: None,
requires: None,
};

for meta in ast.attrs.iter().filter(|a| a.path().is_ident(COMPONENT)) {
meta.parse_nested_meta(|nested| {
if nested.path.is_ident(STORAGE) {
attrs.storage = match nested.value()?.parse::<LitStr>()?.value() {
s if s == TABLE => StorageTy::Table,
s if s == SPARSE_SET => StorageTy::SparseSet,
s => {
return Err(nested.error(format!(
"Invalid storage type `{s}`, expected '{TABLE}' or '{SPARSE_SET}'.",
)));
}
};
Ok(())
} else if nested.path.is_ident(ON_ADD) {
attrs.on_add = Some(nested.value()?.parse::<ExprPath>()?);
Ok(())
} else if nested.path.is_ident(ON_INSERT) {
attrs.on_insert = Some(nested.value()?.parse::<ExprPath>()?);
Ok(())
} else if nested.path.is_ident(ON_REPLACE) {
attrs.on_replace = Some(nested.value()?.parse::<ExprPath>()?);
Ok(())
} else if nested.path.is_ident(ON_REMOVE) {
attrs.on_remove = Some(nested.value()?.parse::<ExprPath>()?);
Ok(())
let mut require_paths = HashSet::new();
for attr in ast.attrs.iter() {
if attr.path().is_ident(COMPONENT) {
attr.parse_nested_meta(|nested| {
if nested.path.is_ident(STORAGE) {
attrs.storage = match nested.value()?.parse::<LitStr>()?.value() {
s if s == TABLE => StorageTy::Table,
s if s == SPARSE_SET => StorageTy::SparseSet,
s => {
return Err(nested.error(format!(
"Invalid storage type `{s}`, expected '{TABLE}' or '{SPARSE_SET}'.",
)));
}
};
Ok(())
} else if nested.path.is_ident(ON_ADD) {
attrs.on_add = Some(nested.value()?.parse::<ExprPath>()?);
Ok(())
} else if nested.path.is_ident(ON_INSERT) {
attrs.on_insert = Some(nested.value()?.parse::<ExprPath>()?);
Ok(())
} else if nested.path.is_ident(ON_REPLACE) {
attrs.on_replace = Some(nested.value()?.parse::<ExprPath>()?);
Ok(())
} else if nested.path.is_ident(ON_REMOVE) {
attrs.on_remove = Some(nested.value()?.parse::<ExprPath>()?);
Ok(())
} else {
Err(nested.error("Unsupported attribute"))
}
})?;
} else if attr.path().is_ident(REQUIRE) {
let punctuated =
attr.parse_args_with(Punctuated::<Require, Comma>::parse_terminated)?;
for require in punctuated.iter() {
if !require_paths.insert(require.path.to_token_stream().to_string()) {
return Err(syn::Error::new(
require.path.span(),
"Duplicate required components are not allowed.",
));
}
}
if let Some(current) = &mut attrs.requires {
current.extend(punctuated);
} else {
Err(nested.error("Unsupported attribute"))
attrs.requires = Some(punctuated);
}
})?;
}
}

Ok(attrs)
}

impl Parse for Require {
fn parse(input: syn::parse::ParseStream) -> Result<Self> {
let path = input.parse::<Path>()?;
let func = if input.peek(Paren) {
let content;
parenthesized!(content in input);
let func = content.parse::<Path>()?;
Some(func)
} else {
None
};
Ok(Require { path, func })
}
}

fn storage_path(bevy_ecs_path: &Path, ty: StorageTy) -> TokenStream2 {
let storage_type = match ty {
StorageTy::Table => Ident::new("Table", Span::call_site()),
Expand Down
14 changes: 13 additions & 1 deletion crates/bevy_ecs/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream {
let mut field_get_component_ids = Vec::new();
let mut field_get_components = 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()
Expand All @@ -88,6 +89,9 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream {
field_component_ids.push(quote! {
<#field_type as #ecs_path::bundle::Bundle>::component_ids(components, storages, &mut *ids);
});
field_required_components.push(quote! {
<#field_type as #ecs_path::bundle::Bundle>::register_required_components(components, storages, required_components);
});
field_get_component_ids.push(quote! {
<#field_type as #ecs_path::bundle::Bundle>::get_component_ids(components, &mut *ids);
});
Expand Down Expand Up @@ -153,6 +157,14 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream {
#(#field_from_components)*
}
}

fn register_required_components(
components: &mut #ecs_path::component::Components,
storages: &mut #ecs_path::storage::Storages,
required_components: &mut #ecs_path::component::RequiredComponents
){
#(#field_required_components)*
}
}

impl #impl_generics #ecs_path::bundle::DynamicBundle for #struct_name #ty_generics #where_clause {
Expand Down Expand Up @@ -527,7 +539,7 @@ pub fn derive_resource(input: TokenStream) -> TokenStream {
component::derive_resource(input)
}

#[proc_macro_derive(Component, attributes(component))]
#[proc_macro_derive(Component, attributes(component, require))]
pub fn derive_component(input: TokenStream) -> TokenStream {
component::derive_component(input)
}
Expand Down
25 changes: 24 additions & 1 deletion crates/bevy_ecs/src/archetype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

use crate::{
bundle::BundleId,
component::{ComponentId, Components, StorageType},
component::{ComponentId, Components, RequiredComponentConstructor, StorageType},
entity::{Entity, EntityLocation},
observer::Observers,
storage::{ImmutableSparseSet, SparseArray, SparseSet, SparseSetIndex, TableId, TableRow},
Expand Down Expand Up @@ -123,10 +123,31 @@ pub(crate) struct AddBundle {
/// For each component iterated in the same order as the source [`Bundle`](crate::bundle::Bundle),
/// indicate if the component is newly added to the target archetype or if it already existed
pub bundle_status: Vec<ComponentStatus>,
/// The set of additional required components that must be initialized immediately when adding this Bundle.
///
/// The initial values are determined based on the provided constructor, falling back to the `Default` trait if none is given.
pub required_components: Vec<RequiredComponentConstructor>,
/// The components added by this bundle. This includes any Required Components that are inserted when adding this bundle.
pub added: Vec<ComponentId>,
/// The components that were explicitly contributed by this bundle, but already existed in the archetype. This _does not_ include any
/// Required Components.
pub existing: Vec<ComponentId>,
}

impl AddBundle {
pub(crate) fn iter_inserted(&self) -> impl Iterator<Item = ComponentId> + '_ {
self.added.iter().chain(self.existing.iter()).copied()
}

pub(crate) fn iter_added(&self) -> impl Iterator<Item = ComponentId> + '_ {
self.added.iter().copied()
}

pub(crate) fn iter_existing(&self) -> impl Iterator<Item = ComponentId> + '_ {
self.existing.iter().copied()
}
}

/// This trait is used to report the status of [`Bundle`](crate::bundle::Bundle) components
/// being added to a given entity, relative to that entity's original archetype.
/// See [`crate::bundle::BundleInfo::write_components`] for more info.
Expand Down Expand Up @@ -208,6 +229,7 @@ impl Edges {
bundle_id: BundleId,
archetype_id: ArchetypeId,
bundle_status: Vec<ComponentStatus>,
required_components: Vec<RequiredComponentConstructor>,
added: Vec<ComponentId>,
existing: Vec<ComponentId>,
) {
Expand All @@ -216,6 +238,7 @@ impl Edges {
AddBundle {
archetype_id,
bundle_status,
required_components,
added,
existing,
},
Expand Down
Loading

0 comments on commit 9cdb915

Please sign in to comment.