-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improved Spawn APIs and Bundle Effects #17521
base: main
Are you sure you want to change the base?
Conversation
We probably want to roll this up into / right after the relations release note, but I want to make sure this doesn't get missed when writing them. It's also notable enough that folks skimming the milestone + "Needs-Release-Notes" will be interested. |
IMO this is a step in the right direction, making entity spawning constructs more reusable / composable (having only read the PR description). One thing I feel will still be missing after this PR is argument passing ergonomics, since |
Agreed: the key thing there is "dependency injection-flavored access to asset collections". The |
} | ||
|
||
/// The parts from [`Bundle`] that don't require statically knowing the components of the bundle. | ||
pub trait DynamicBundle { | ||
/// An operation on the entity that happens _after_ inserting this bundle. | ||
type Effect: BundleEffect; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will partially break Box<dyn DynamicBundle>
, since you'll need to specify an Effect associated type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the existing and intended use cases for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this. The end result is great: like a generic, flexible version of the WithChild
component I hacked together, but without the terrible performance and no bizarre type-system gotchas. I prefer the non-macro API, but I don't mind the macro form, and I'm happy to standardize that in the learning material.
Implementation is solid: lots of moving parts, but I don't see any immediate way to simplify them, and the docs are exceptionally clear. Updating the rest of the example should go in follow-up.
This makes impl Bundle
the blessed API for spawning entity collections, which is super exciting because it unblocks widgets over in bevy_ui
. I'm a little nervous about the lack of boxed trait objects hampering our the flexibility of designs there, but the recently added default query filters + entity cloning should make an entity zoo (prefabs?) approach extremely comfortable.
I think there's a third option here - breaking the concept of nested bundles into a set of new traits. The ambiguity arises because we have the following impl: impl<B: Bundle> Bundle for (B1, B2, ..., Bn) {} I believe we could remove this impl, and introduce a new trait: trait BundleList {}
impl<B: Bundle> BundleList for (B1, B2, ..., Bn) {}
impl<B: Bundle> BundleList for B {} This allows us to use the bundles in
We'll also need to adjust the definition of /// An `Effect` or a `Component`, something that can be "spawned" on a given entity
///
/// Currently this will either be:
/// - a `Component`
/// - The `SpawnRelated<T>` struct, returned by `RelationshipTarget::spawn`.
/// `SpawnRelated<T>` has an Effect that spawns the related entities
trait Spawn {}
/// A bundle is a list of `Spawn`, things that can be spawned on an entity
trait Bundle {}
impl<S: Spawn> Bundle for (S1, S2, ..., Sn) {}
impl<S: Spawn> Bundle for S {} Finally, in cases where we want to support nested bundles, we make use of the /// World::spawn creates a new entity with all of the bundles in the BundleList
impl World {
fn spawn(bundle: impl BundleList);
}
/// `Children::spawn` returns a type that implements `Spawn` that isn't a component, but an Effect
/// The returned `impl Spawn` has an Effect that spawns a new child entity for each Bundle in the BundleList
impl Children {
fn spawn(entities: impl BundleList) -> impl Spawn {}
} This set of traits should allow the following to work: world.spawn((
Foo,
(Bar, Baz),
Children::spawn((
(Bar, Children::spawn(Baz)),
(Bar, Children::spawn(Baz))
)),
Observers::spawn((Observer1, Observer2)),
)); |
Co-authored-by: Alice Cecile <[email protected]> Co-authored-by: Gino Valente <[email protected]> Co-authored-by: Emerson Coskey <[email protected]>
When we hit the |
For spawning with The only place I'd see this being an issue is For the rare cases where you need more than 16, we could introduce a |
#[macro_export] | ||
macro_rules! children { | ||
[$($child:expr),*$(,)?] => { | ||
Children::spawn(($($crate::spawn::Spawn($child)),*)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still needs a $crate
path:
Children::spawn(($($crate::spawn::Spawn($child)),*)) | |
$crate::hierarchy::Children::spawn(($($crate::spawn::Spawn($child)),*)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm very glad this will be a thing soon! Leaving a minor suggestion.
Would also be interesting to give the option presented by vultix
some more thought, as it looks like it'd simplify a few things. No macros/wrappers to build the related entities but having to use macros/wrappers when we're exceeding some tuple limit feels much more approachable. In my experience this is more of a corner case so it seems appropriate to add the "burden" there.
/// Children::spawn_one(Name::new("Child")), | ||
/// )); | ||
/// ``` | ||
fn spawn_one<B: Bundle>(bundle: B) -> SpawnOneRelated<Self::Relationship, B>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's a longer name but IMHO spawn_single
sounds better since it "spawns a single entity containing Bundle
" as per the docs. I think it's also easier to reason about it since we already have Single
and single methods to get entities from queries.
Now that I have had a chance to work with this extensively, I have a bunch of feedback items. I've discussed some of this already on Discord, but I want to gather it all in one place, especially given that this is going to be a long post. TL;DR: For constructing individual entities, I'm able do pretty much all of the things I want. However, I've come to the conclusion that I can't build a complete templating solution within the restrictions of the API. Checkbox ExampleLet's look at a concrete example. The let mut checkbox = builder.spawn((
Node { ..default() },
Hovering::default(),
Name::new("Checkbox"),
Styles((style_checkbox, self.style.clone())),
TabIndex(self.tab_index),
CoreCheckbox {
checked: false,
on_change: self.on_change,
},
MutateDyn::new(
move |world: DeferredWorld| checked.get(&world),
|checked, ent| {
let mut checkbox = ent.get_mut::<CoreCheckbox>().unwrap();
checkbox.checked = checked;
},
),
InsertWhen::new(
move |world: DeferredWorld| disabled.get(&world),
|| InteractionDisabled,
),
));
let checkbox_id = checkbox.id(); Dynamic Mutation ComponentsWithin this example, there are three components that use the new API:
Both The satellite entity is linked to the primary entity using an In this example, the creation of the satellite entity and the relationship is done within the after effect, meaning that this is hidden from the user. Now, let's look at a slightly different example: let mut checkbox = builder.spawn((
Node { ..default() },
Hovering::default(),
Name::new("Checkbox"),
Styles((style_checkbox, self.style.clone())),
TabIndex(self.tab_index),
CoreCheckbox {
checked: false,
on_change: self.on_change,
},
owned![
MutateDyn::new(
move |world: DeferredWorld| checked.get(&world),
|checked, ent| {
let mut checkbox = ent.get_mut::<CoreCheckbox>().unwrap();
checkbox.checked = checked;
},
),
InsertWhen::new(
move |world: DeferredWorld| disabled.get(&world),
|| InteractionDisabled,
),
]
));
let checkbox_id = checkbox.id(); In this example, we have changed how This approach is more in line with @cart 's vision: it explicitly shows the user what the actual hierarchy is going to look like, instead of hiding things under the hood. It's also more efficient: the However, it also has some drawbacks:
In general the pros and cons of the two approaches really depend on what the user cares about. Here's a metaphor: the smooth flowing skin of a sports car hides all the nasty details of engines, crankshafts and grease. Most drivers would prefer not to think about the mechanics of what's going on under the hood, and don't want to see all that stuff. However, some car enthusiasts are very interested in engines and such, and do care about what's happening. By the same token, many users don't care about how observers work, they just want to call Checkbox Example Part 2I only showed the first part of the checkbox construction. The next part looks like this (many details omitted): checkbox.insert(
StyleDyn::new(
move |world: DeferredWorld| world.is_focus_visible(checkbox_id),
|is_focused, ec| {
if is_focused {
ec.insert(Outline {
color: colors::FOCUS.into(),
width: ui::Val::Px(2.0),
offset: ui::Val::Px(2.0),
});
} else {
ec.remove::<Outline>();
};
},
)), This code is responsible for placing a focus rectangle (using The important thing to note here is the use of The checkbox id is captured by the closure. There's no way to inject it as a SystemParam. This means that the checkbox widget cannot be created all in one shot: it requires a two-phase initialization. This will be important later. Template FrictionThe PR allows returning bundles from functions. However, this is not sufficient for templating.
In my previous frameworks, the Button::new()
.variant(ButtonVariant::Primary)
.labeled("Primary"), For example, the It would be possible to adapt this struct-based approach by adding an explicit An alternate approach would be for the template to use an after-effect. This at least would let us get around the two-phase construction problem, but it doesn't address the issue of multiple roots, since an after-effect can't control how many entities are spawned, it can only control what goes into the entities after they are spawned. Also, we can't make a blanket impl for all templates (at least, I tried and wasn't able to). This means that each template will have to derive |
What's the reason to not use I do realize one might want to keep the hierarchy "clean". If it is a UI hierarchy - its all Actually the same thing for
|
@villor As several have mentioned, in React the problem is solved via "Fragment" nodes. These are essentially the same as ghost nodes, except for lifecycle: in a fragment, the "flattening" is done as a conversion from the VDOM to the DOM, whereas with ghost nodes the flattening is performed continuously, each time the tree is traversed. Note BTW that ghost nodes as they exist today won't solve our problem, or rather they only solve the UI case, but don't provide a general solution. Ghost nodes are currently only interpreted by the UI subsystems (layout and picking). 3D scenes ignore ghost nodes because 3D scenes don't care if there are intermediate nodes (SpatialBundle and friends). However, for BSN there are other use cases: for example, suppose we want to represent actor goal nodes as an entity graph, such that an actor's behavior is a composition of multiple BSN resources, so that the various prioritized goals are merged together. If you used a ghost node inside a resource like this, it wouldn't work unless the goal tree walker used the ghost-aware traversal methods (which you wrote). So a different approach would be to get rid of ghost nodes entirely and replace them with fragments. This would require having two different "children" relationships (which, fortunately, is easy to do now). That is, your template would construct a node with a "source children" which can be a mix of normal entities and fragments (a fragment being exactly the same as a ghost node - an entity with a marker, and some children). Then at some point the source children is transformed into a list of "flat children", that is, a flat list of children with all of the fragments flattened into a normal All of the normal systems that traverse children relationships would now look at the "flat children", and would not need any special logic for ghost nodes / fragments. And it works across all types of hierarchies, not just UI or 3D scenes. Fragment root nodes could also work: if a fragment has no parent, then its children are never flattened, meaning that they are not part of any flat children list. So even though they are part of a "source children" relationship, they behave as roots in all other ways. Templates could return either regular nodes or fragments, since a fragment is just an entity. So this solves the problem of templates returning more than one root entity. However, this idea does come with some downsides:
Under this approach, all existing users of ghost nodes (Cond, ForEach and so on) would instead generate fragments. Note that this means, however, that if you try and use these control-flow nodes within regular (flat) children, they won't work. |
@viridia It seems like we are on the same track here, but from different angles. Correct me if I'm wrong, but it seems like you want the output of the template to be the "flat children" (e.g. the DOM), while I think that the template should output "source children". In other words "source children" is A clarification on derived hierarchiesWhat I am proposing here would be a built-in way to define derived relationships/hierarchies. I have to admit I haven't thought this through completely. But I imagine something like this, for the UI/ #[derive(Component)]
#[relationship(relationship_source = NodeChildren)]
#[derive_hierarchy(derive_from = Children, marker = Node)]
struct NodeChildOf(Entity); A derived relationship would be a filtered version of the This would completely replace the current The same could possibly be done for This allows the developer/scene designer to group entities (implicit fragments) in any way they like in their scenes/templates, while also getting the expected UI layout/spatial propagation that they are used to. Reactive "auxilary" entities like Ideally, derived relationships would also allow:
And of course the future scene inspector would need to have a nice way to switch between "Scene hierarchy", "UI hierarchy (DOM)", "Spatial (3D/2D) hierarchy" etc. |
There's another aspect that I want to dive deeper into here: the choice of "spawning requires a world" as opposed to "spawning requires commands". Specifically, the My earlier frameworks (quill and bevy_reactor) used a similar approach, but my most recent framework, thorium, uses a commands-based spawning protocol. The reason for this is that I wanted as much as possible for the developer experience of spawning hierarchies to look and feel like the canonical bevy spawning pattern. Making everything commands-compatible was a considerable amount of work on my part, and I'm curious to know whether that is going to be a dead end or not. Using commands for spawning does have some limitations. The most obvious is that any world dereferences must be lazy. That is, the template only has direct access to the parameters that are passed in to it. If it needs any additional data to populate the entity graph, that data can only be obtained asynchronously. However, because of the automatic flushing of commands, the delay in fetching the data is minimal in most cases. For example, the Of course, the advantage of the commands-based approach is that it doesn't require exclusive access. However, this advantage is highly situational: how many other threads are we blocking? This is hard to evaluate as a general principle. Right now, all of my |
I've coded up another experiment to wrestle with the issues around multi-root and two-phase templates. The code looks like this: commands.spawn((
Node::default(),
BorderColor(css::ALICE_BLUE.into()),
Children::spawn((
Hello,
World,
Spawn((
Node {
border: ui::UiRect::all(ui::Val::Px(3.)),
..default()
},
BorderColor(css::LIME.into()),
)),
)),
)); In this example struct Hello;
impl BundleTemplate for Hello {
fn build(&self, builder: &mut ChildSpawner) {
builder.spawn(Text::new("Hello, "));
}
}
impl_bundle_template!(Hello); Note that I'll be the first to admit that this looks a bit odd. Here's the justification: This somewhat echoes the situation in JSX: "native" elements like Unfortunately, this means that templates can only be used with the explicit Also, the reason for the Note that |
This looks great! I do like the implementation, but the user-facing API does look a little clunky and stuttery. It would be great if we could make @vultix‘s suggestion work, as it does look like a cleaner API, even if it introduces some extra boilerplate for the (I believe) non-standard path. |
Objective
A major critique of Bevy at the moment is how boilerplatey it is to compose (and read) entity hierarchies:
There is also currently no good way to statically define and return an entity hierarchy from a function. Instead, people often do this "internally" with a Commands function that returns nothing, making it impossible to spawn the hierarchy in other cases (direct World spawns, ChildSpawner, etc).
Additionally, because this style of API results in creating the hierarchy bits after the initial spawn of a bundle, it causes ECS archetype changes (and often expensive table moves).
Because children are initialized after the fact, we also can't count them to pre-allocate space. This means each time a child inserts itself, it has a high chance of overflowing the currently allocated capacity in the
RelationshipTarget
collection, causing literal worst-case reallocations.We can do better!
Solution
The Bundle trait has been extended to support an optional
BundleEffect
. This is applied directly to World immediately after the Bundle has fully inserted. Note that this is intentionally not done via a deferred Command, which would require repeatedly copying each remaining subtree of the hierarchy to a new command as we walk down the tree (not good performance).This allows us to implement the new
SpawnRelated
trait for allRelationshipTarget
impls, which looks like this in practice:Children::spawn
returnsSpawnRelatedBundle<Children, L: SpawnableList>
, which is aBundle
that insertsChildren
(preallocated to the size of theSpawnableList::size_hint()
).Spawn<B: Bundle>(pub B)
implementsSpawnableList
with a size of 1.SpawnableList
is also implemented for tuples ofSpawnableList
(same general pattern as the Bundle impl).There are currently three built-in
SpawnableList
implementations:We get the benefits of "structured init", but we have nice flexibility where it is required!
Some readers' first instinct might be to try to remove the need for the
Spawn
wrapper. This is impossible in the Rust type system, as a tuple of "child Bundles to be spawned" and a "tuple of Components to be added via a single Bundle" is ambiguous in the Rust type system. There are two ways to resolve that ambiguity:For the single-entity spawn cases,
Children::spawn_one
does also exist, which removes the need for the wrapper:This works for all Relationships
This API isn't just for
Children
/ChildOf
relationships. It works for any relationship type, and they can be mixed and matched!Macros
While
Spawn
is necessary to satisfy the type system, we can remove the need to express it via macros. The example above can be expressed more succinctly using the newchildren![X]
macro, which internally producesChildren::spawn(Spawn(X))
:There is also a
related!
macro, which is a generic version of thechildren!
macro that supports any relationship type:Returning Hierarchies from Functions
Thanks to these changes, the following pattern is now possible:
Additional Changes and Notes
Bundle::from_components
has been split out intoBundleFromComponents::from_components
, enabling us to implementBundle
for types that cannot be "taken" from the ECS (such as the newSpawnRelatedBundle
).NoBundleEffect
trait (which implementsBundleEffect
) is implemented for empty tuples (and tuples of empty tuples), which allows us to constrain APIs to only accept bundles that do not have effects. This is critical because the current batch spawn APIs cannot efficiently apply BundleEffects in their current form (as doing so in-place could invalidate the cached raw pointers). We could consider allocating a buffer of the effects to be applied later, but that does have performance implications that could offset the balance and value of the batched APIs (and would likely require some refactors to the underlying code). I've decided to be conservative here. We can consider relaxing that requirement on those APIs later, but that should be done in a followup imo.children!
form whenever possible (and for cases that require things like SpawnIter, use the raw APIs).Relationship
to spawn (ex:ChildOf::spawn(Foo)
) instead of theRelationshipTarget
(ex:Children::spawn(Spawn(Foo))
)?". That would allow us to remove theSpawn
wrapper. I've explicitly chosen to disallow this pattern.Bundle::Effect
has the ability to create significant weirdness. Things inBundle
position look like components. For exampleworld.spawn((Foo, ChildOf::spawn(Bar)))
looks and reads like Foo is a child of Bar.ChildOf
is in Foo's "component position" but it is not a component on Foo. This is a huge problem. Now thatBundle::Effect
exists, we should be very principled about keeping the "weird and unintuitive behavior" to a minimum. Things that read like components _should be the components they appear to be".Remaining Work
Next Steps
children!
where possible and rawSpawn
/SpawnIter
/SpawnWith
where the flexibility of the raw API is required.Migration Guide
Existing spawn patterns will continue to work as expected.
Manual Bundle implementations now require a
BundleEffect
associated type. Exisiting bundles would have no bundle effect, so use()
. AdditionallyBundle::from_components
has been moved to the newBundleFromComponents
trait.