Skip to content
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

Relationships (non-fragmenting, one-to-many) #17398

Merged
merged 40 commits into from
Jan 18, 2025
Merged

Conversation

cart
Copy link
Member

@cart cart commented Jan 16, 2025

This adds support for one-to-many non-fragmenting relationships (with planned paths for fragmenting and non-fragmenting many-to-many relationships). "Non-fragmenting" means that entities with the same relationship type, but different relationship targets, are not forced into separate tables (which would cause "table fragmentation").

Functionally, this fills a similar niche as the current Parent/Children system. The biggest differences are:

  1. Relationships have simpler internals and significantly improved performance and UX. Commands and specialized APIs are no longer necessary to keep everything in sync. Just spawn entities with the relationship components you want and everything "just works".
  2. Relationships are generalized. Bevy can provide additional built in relationships, and users can define their own.

REQUEST TO REVIEWERS: please don't leave top level comments and instead comment on specific lines of code. That way we can take advantage of threaded discussions. Also dont leave comments simply pointing out CI failures as I can read those just fine.

Built on top of what we have

Relationships are implemented on top of the Bevy ECS features we already have: components, immutability, and hooks. This makes them immediately compatible with all of our existing (and future) APIs for querying, spawning, removing, scenes, reflection, etc. The fewer specialized APIs we need to build, maintain, and teach, the better.

Why focus on one-to-many non-fragmenting first?

  1. This allows us to improve Parent/Children relationships immediately, in a way that is reasonably uncontroversial. Switching our hierarchy to fragmenting relationships would have significant performance implications. Flecs is heavily considering a switch to non-fragmenting relations after careful considerations of the performance tradeoffs. (Correction from @SanderMertens: Flecs is implementing non-fragmenting storage specialized for asset hierarchies, where asset hierarchies are many instances of small trees that have a well defined structure)
  2. Adding generalized one-to-many relationships is currently a priority for the Next Generation Scene / UI effort. Specifically, we're interested in building reactions and observers on top.

The changes

This PR does the following:

  1. Adds a generic one-to-many Relationship system
  2. Ports the existing Parent/Children system to Relationships, which now lives in bevy_ecs::hierarchy. The old bevy_hierarchy crate has been removed.
  3. Adds on_despawn component hooks
  4. Relationships can opt-in to "despawn descendants" behavior, meaning that the entire relationship hierarchy is despawned when entity.despawn() is called. The built in Parent/Children hierarchies enable this behavior, and entity.despawn_recursive() has been removed.
  5. world.spawn now applies commands after spawning. This ensures that relationship bookkeeping happens immediately and removes the need to manually flush. This is in line with the equivalent behaviors recently added to the other APIs (ex: insert).
  6. Removes the ValidParentCheckPlugin (system-driven / poll based) in favor of a validate_parent_has_component hook.

Using Relationships

The Relationship trait looks like this:

pub trait Relationship: Component + Sized {
    type RelationshipTarget: RelationshipTarget<Relationship = Self>;
    fn get(&self) -> Entity;
    fn from(entity: Entity) -> Self;
}

A relationship is a component that:

  1. Is a simple wrapper over a "target" Entity.
  2. Has a corresponding RelationshipTarget component, which is a simple wrapper over a collection of entities. Every "target entity" targeted by a "source entity" with a Relationship has a RelationshipTarget component, which contains every "source entity" that targets it.

For example, the Parent component (as it currently exists in Bevy) is the Relationship component and the entity containing the Parent is the "source entity". The entity inside the Parent(Entity) component is the "target entity". And that target entity has a Children component (which implements RelationshipTarget).

In practice, the Parent/Children relationship looks like this:

#[derive(Relationship)]
#[relationship(relationship_sources = Children)]
pub struct Parent(pub Entity);

#[derive(RelationshipTarget)]
#[relationship_sources(relationship = Parent)]
pub struct Children(Vec<Entity>);

The Relationship and RelationshipTarget derives automatically implement Component with the relevant configuration (namely, the hooks necessary to keep everything in sync).

The most direct way to add relationships is to spawn entities with relationship components:

let a = world.spawn_empty().id();
let b = world.spawn(Parent(a)).id();

assert_eq!(world.entity(a).get::<Children>().unwrap(), &[b]);

There are also convenience APIs for spawning more than one entity with the same relationship:

world.spawn_empty().with_related::<Children>(|s| {
    s.spawn_empty();
    s.spawn_empty();
})

The existing with_children API is now a simpler wrapper over with_related. This makes this change largely non-breaking for existing spawn patterns.

world.spawn_empty().with_children(|s| {
    s.spawn_empty();
    s.spawn_empty();
})

There are also other relationship APIs, such as add_related and despawn_related.

Automatic recursive despawn via the new on_despawn hook

RelationshipTarget can opt-in to "despawn descendants" behavior, which will despawn all related entities in the relationship hierarchy:

#[derive(RelationshipTarget)]
#[relationship_sources(relationship = Parent, despawn_descendants)]
pub struct Children(Vec<Entity>);

This means that entity.despawn_recursive() is no longer required. Instead, just use entity.despawn() and the relevant related entities will also be despawned.

To despawn an entity without despawning its parent/child descendants, you should remove the Children component first, which will also remove the related Parent components:

entity
    .remove::<Children>()
    .despawn()

This builds on the on_despawn hook introduced in this PR, which is fired when an entity is despawned (before other hooks).

Relationships are the source of truth

Relationship is the single source of truth component. RelationshipTarget is merely a reflection of what all the Relationship components say. By embracing this, we are able to significantly improve the performance of the system as a whole. We can rely on component lifecycles to protect us against duplicates, rather than needing to scan at runtime to ensure entities don't already exist (which results in quadratic runtime). A single source of truth gives us constant-time inserts. This does mean that we cannot directly spawn populated Children components (or directly add or remove entities from those components). I personally think this is a worthwhile tradeoff, both because it makes the performance much better and because it means theres exactly one way to do things (which is a philosophy we try to employ for Bevy APIs).

As an aside: treating both sides of the relationship as "equivalent source of truth relations" does enable building simple and flexible many-to-many relationships. But this introduces an inherent need to scan (or hash) to protect against duplicates. evergreen_relations has a very nice implementation of the "symmetrical many-to-many" approach. Unfortunately I think the performance issues inherent to that approach make it a poor choice for Bevy's default relationship system.

Followup Work

  • Discuss renaming Parent to ChildOf. I refrained from doing that in this PR to keep the diff reasonable, but I'm personally biased toward this change (and using that naming pattern generally for relationships).
  • Improved spawning ergonomics
  • Consider adding relationship observers/triggers for "relationship targets" whenever a source is added or removed. This would replace the current "hierarchy events" system, which is unused upstream but may have existing users downstream. I think triggers are the better fit for this than a buffered event queue, and would prefer not to add that back.
  • Fragmenting relations: My current idea hinges on the introduction of "value components" (aka: components whose type and value determines their ComponentId, via something like Hashing / PartialEq). By labeling a Relationship component such as ChildOf(Entity) as a "value component", ChildOf(e1) and ChildOf(e2) would be considered "different components". This makes the transition between fragmenting and non-fragmenting a single flag, and everything else continues to work as expected.
  • Many-to-many support
    • Non-fragmenting: We can expand Relationship to be a list of entities instead of a single entity. I have largely already written the code for this.
    • Fragmenting: With the "value component" impl mentioned above, we get many-to-many support "for free", as it would allow inserting multiple copies of a Relationship component with different target entities.

Fixes #3742 (If this PR is merged, I think we should open more targeted followup issues for the work above, with a fresh tracking issue free of the large amount of less-directed historical context)
Fixes #17301
Fixes #12235
Fixes #15299
Fixes #15308

Migration Guide

  • Replace ChildBuilder with ChildSpawnerCommands.
  • Replace calls to .set_parent(parent_id) with .insert(Parent(parent_id)).
  • Replace calls to .replace_children() with .remove::<Children>() followed by .add_children(). Note that you'll need to manually despawn any children that are not carried over.
  • Replace calls to .despawn_recursive() with .despawn().
  • Replace calls to .despawn_descendants() with .despawn_related::<Children>().
  • If you have any calls to .despawn() which depend on the children being preserved, you'll need to remove the Children component first.

@cart cart added A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jan 16, 2025
@cart cart added this to the 0.16 milestone Jan 16, 2025
Copy link
Contributor

@Carter0 Carter0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg its happening

@alice-i-cecile alice-i-cecile added M-Needs-Release-Note Work that should be called out in the blog due to impact D-Complex Quite challenging from either a design or technical perspective. Ask for help! X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 16, 2025
Copy link
Contributor

@bushrat011899 bushrat011899 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent to see this landing! Mostly comments, but I do have one question around the need for Component<Mutability = Mutable> on RelationshipSource.

benches/benches/bevy_ecs/entity_cloning.rs Show resolved Hide resolved
@@ -207,6 +210,7 @@ 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";
pub const ON_DESPAWN: &str = "on_despawn";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An ON_DESPAWN hook is also a great outcome form this PR in of itself. I assume there's negligible performance impacts from the addition of this hook?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can benchmark, but I don't expect significant degradation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't expect any problems either, agreed.

crates/bevy_ecs/macros/src/relationship.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/macros/src/relationship.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@cBournhonesque cBournhonesque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't read everything, added a couple comments.
Is there a test for the commands.remove::<Parent>().despawn() doesn't despawn descendants? I think that would be good to have

crates/bevy_ecs/src/hierarchy.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/hierarchy.rs Show resolved Hide resolved
@alice-i-cecile
Copy link
Member

@cart I have no concerns about this at this stage and we have plenty of community review; feel free to merge when you feel it's ready.

@cart cart added this pull request to the merge queue Jan 18, 2025
Merged via the queue into bevyengine:main with commit 21f1e30 Jan 18, 2025
32 checks passed
@cart cart mentioned this pull request Jan 19, 2025
/// #[relationship_target(relationship = Parent, despawn_descendants)]
/// pub struct Children(Vec<Entity>);
/// ```
pub trait Relationship: Component + Sized {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason this is called Relationship and not Relation? I like the latter more since it's shorter

Copy link
Contributor

@mrchantey mrchantey Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose relationship is more accurate when describing specific instances of relations, though I'm also a fan of short names.
https://www.britannica.com/dictionary/eb/qa/relations-and-relationship

github-merge-queue bot pushed a commit that referenced this pull request Jan 20, 2025
Fixes #17412

## Objective

`Parent` uses the "has a X" naming convention. There is increasing
sentiment that we should use the "is a X" naming convention for
relationships (following #17398). This leaves `Children` as-is because
there is prevailing sentiment that `Children` is clearer than `ParentOf`
in many cases (especially when treating it like a collection).

This renames `Parent` to `ChildOf`.

This is just the implementation PR. To discuss the path forward, do so
in #17412.

## Migration Guide

- The `Parent` component has been renamed to `ChildOf`.
github-merge-queue bot pushed a commit that referenced this pull request Jan 20, 2025
# Objective

After #17398, Bevy now has relations! We don't teach users how to make /
work with these in the examples yet though, but we definitely should.

## Solution

- Add a simple abstract example that goes over defining, spawning,
traversing and removing a custom relations.
- ~~Add `Relationship` and `RelationshipTarget` to the prelude: the
trait methods are really helpful here.~~
- this causes subtle ambiguities with method names and weird compiler
errors. Not doing it here!
- Clean up related documentation that I referenced when writing this
example.

## Testing

`cargo run --example relationships`

## Notes to reviewers

1. Yes, I know that the cycle detection code could be more efficient. I
decided to reduce the caching to avoid distracting from the broader
point of "here's how you traverse relationships".
2. Instead of using an `App`, I've decide to use
`World::run_system_once` + system functions defined inside of `main` to
do something closer to literate programming.

---------

Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>
Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com>
mrchantey pushed a commit to mrchantey/bevy that referenced this pull request Feb 4, 2025
This adds support for one-to-many non-fragmenting relationships (with
planned paths for fragmenting and non-fragmenting many-to-many
relationships). "Non-fragmenting" means that entities with the same
relationship type, but different relationship targets, are not forced
into separate tables (which would cause "table fragmentation").

Functionally, this fills a similar niche as the current Parent/Children
system. The biggest differences are:

1. Relationships have simpler internals and significantly improved
performance and UX. Commands and specialized APIs are no longer
necessary to keep everything in sync. Just spawn entities with the
relationship components you want and everything "just works".
2. Relationships are generalized. Bevy can provide additional built in
relationships, and users can define their own.

**REQUEST TO REVIEWERS**: _please don't leave top level comments and
instead comment on specific lines of code. That way we can take
advantage of threaded discussions. Also dont leave comments simply
pointing out CI failures as I can read those just fine._

## Built on top of what we have

Relationships are implemented on top of the Bevy ECS features we already
have: components, immutability, and hooks. This makes them immediately
compatible with all of our existing (and future) APIs for querying,
spawning, removing, scenes, reflection, etc. The fewer specialized APIs
we need to build, maintain, and teach, the better.

## Why focus on one-to-many non-fragmenting first?

1. This allows us to improve Parent/Children relationships immediately,
in a way that is reasonably uncontroversial. Switching our hierarchy to
fragmenting relationships would have significant performance
implications. ~~Flecs is heavily considering a switch to non-fragmenting
relations after careful considerations of the performance tradeoffs.~~
_(Correction from @SanderMertens: Flecs is implementing non-fragmenting
storage specialized for asset hierarchies, where asset hierarchies are
many instances of small trees that have a well defined structure)_
2. Adding generalized one-to-many relationships is currently a priority
for the [Next Generation Scene / UI
effort](bevyengine#14437).
Specifically, we're interested in building reactions and observers on
top.

## The changes

This PR does the following:

1. Adds a generic one-to-many Relationship system
3. Ports the existing Parent/Children system to Relationships, which now
lives in `bevy_ecs::hierarchy`. The old `bevy_hierarchy` crate has been
removed.
4. Adds on_despawn component hooks
5. Relationships can opt-in to "despawn descendants" behavior, meaning
that the entire relationship hierarchy is despawned when
`entity.despawn()` is called. The built in Parent/Children hierarchies
enable this behavior, and `entity.despawn_recursive()` has been removed.
6. `world.spawn` now applies commands after spawning. This ensures that
relationship bookkeeping happens immediately and removes the need to
manually flush. This is in line with the equivalent behaviors recently
added to the other APIs (ex: insert).
7. Removes the ValidParentCheckPlugin (system-driven / poll based) in
favor of a `validate_parent_has_component` hook.

## Using Relationships

The `Relationship` trait looks like this:

```rust
pub trait Relationship: Component + Sized {
    type RelationshipSources: RelationshipSources<Relationship = Self>;
    fn get(&self) -> Entity;
    fn from(entity: Entity) -> Self;
}
```

A relationship is a component that:

1. Is a simple wrapper over a "target" Entity.
2. Has a corresponding `RelationshipSources` component, which is a
simple wrapper over a collection of entities. Every "target entity"
targeted by a "source entity" with a `Relationship` has a
`RelationshipSources` component, which contains every "source entity"
that targets it.

For example, the `Parent` component (as it currently exists in Bevy) is
the `Relationship` component and the entity containing the Parent is the
"source entity". The entity _inside_ the `Parent(Entity)` component is
the "target entity". And that target entity has a `Children` component
(which implements `RelationshipSources`).

In practice, the Parent/Children relationship looks like this:

```rust
#[derive(Relationship)]
#[relationship(relationship_sources = Children)]
pub struct Parent(pub Entity);

#[derive(RelationshipSources)]
#[relationship_sources(relationship = Parent)]
pub struct Children(Vec<Entity>);
```

The Relationship and RelationshipSources derives automatically implement
Component with the relevant configuration (namely, the hooks necessary
to keep everything in sync).

The most direct way to add relationships is to spawn entities with
relationship components:

```rust
let a = world.spawn_empty().id();
let b = world.spawn(Parent(a)).id();

assert_eq!(world.entity(a).get::<Children>().unwrap(), &[b]);
```

There are also convenience APIs for spawning more than one entity with
the same relationship:

```rust
world.spawn_empty().with_related::<Children>(|s| {
    s.spawn_empty();
    s.spawn_empty();
})
```

The existing `with_children` API is now a simpler wrapper over
`with_related`. This makes this change largely non-breaking for existing
spawn patterns.

```rust
world.spawn_empty().with_children(|s| {
    s.spawn_empty();
    s.spawn_empty();
})
```

There are also other relationship APIs, such as `add_related` and
`despawn_related`.

## Automatic recursive despawn via the new on_despawn hook

`RelationshipSources` can opt-in to "despawn descendants" behavior,
which will despawn all related entities in the relationship hierarchy:

```rust
#[derive(RelationshipSources)]
#[relationship_sources(relationship = Parent, despawn_descendants)]
pub struct Children(Vec<Entity>);
```

This means that `entity.despawn_recursive()` is no longer required.
Instead, just use `entity.despawn()` and the relevant related entities
will also be despawned.

To despawn an entity _without_ despawning its parent/child descendants,
you should remove the `Children` component first, which will also remove
the related `Parent` components:

```rust
entity
    .remove::<Children>()
    .despawn()
```

This builds on the on_despawn hook introduced in this PR, which is fired
when an entity is despawned (before other hooks).

## Relationships are the source of truth

`Relationship` is the _single_ source of truth component.
`RelationshipSources` is merely a reflection of what all the
`Relationship` components say. By embracing this, we are able to
significantly improve the performance of the system as a whole. We can
rely on component lifecycles to protect us against duplicates, rather
than needing to scan at runtime to ensure entities don't already exist
(which results in quadratic runtime). A single source of truth gives us
constant-time inserts. This does mean that we cannot directly spawn
populated `Children` components (or directly add or remove entities from
those components). I personally think this is a worthwhile tradeoff,
both because it makes the performance much better _and_ because it means
theres exactly one way to do things (which is a philosophy we try to
employ for Bevy APIs).

As an aside: treating both sides of the relationship as "equivalent
source of truth relations" does enable building simple and flexible
many-to-many relationships. But this introduces an _inherent_ need to
scan (or hash) to protect against duplicates.
[`evergreen_relations`](https://github.com/EvergreenNest/evergreen_relations)
has a very nice implementation of the "symmetrical many-to-many"
approach. Unfortunately I think the performance issues inherent to that
approach make it a poor choice for Bevy's default relationship system.

## Followup Work

* Discuss renaming `Parent` to `ChildOf`. I refrained from doing that in
this PR to keep the diff reasonable, but I'm personally biased toward
this change (and using that naming pattern generally for relationships).
* [Improved spawning
ergonomics](bevyengine#16920)
* Consider adding relationship observers/triggers for "relationship
targets" whenever a source is added or removed. This would replace the
current "hierarchy events" system, which is unused upstream but may have
existing users downstream. I think triggers are the better fit for this
than a buffered event queue, and would prefer not to add that back.
* Fragmenting relations: My current idea hinges on the introduction of
"value components" (aka: components whose type _and_ value determines
their ComponentId, via something like Hashing / PartialEq). By labeling
a Relationship component such as `ChildOf(Entity)` as a "value
component", `ChildOf(e1)` and `ChildOf(e2)` would be considered
"different components". This makes the transition between fragmenting
and non-fragmenting a single flag, and everything else continues to work
as expected.
* Many-to-many support
* Non-fragmenting: We can expand Relationship to be a list of entities
instead of a single entity. I have largely already written the code for
this.
* Fragmenting: With the "value component" impl mentioned above, we get
many-to-many support "for free", as it would allow inserting multiple
copies of a Relationship component with different target entities.

Fixes bevyengine#3742 (If this PR is merged, I think we should open more targeted
followup issues for the work above, with a fresh tracking issue free of
the large amount of less-directed historical context)
Fixes bevyengine#17301
Fixes bevyengine#12235 
Fixes bevyengine#15299
Fixes bevyengine#15308 

## Migration Guide

* Replace `ChildBuilder` with `ChildSpawnerCommands`.
* Replace calls to `.set_parent(parent_id)` with
`.insert(Parent(parent_id))`.
* Replace calls to `.replace_children()` with `.remove::<Children>()`
followed by `.add_children()`. Note that you'll need to manually despawn
any children that are not carried over.
* Replace calls to `.despawn_recursive()` with `.despawn()`.
* Replace calls to `.despawn_descendants()` with
`.despawn_related::<Children>()`.
* If you have any calls to `.despawn()` which depend on the children
being preserved, you'll need to remove the `Children` component first.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
mrchantey pushed a commit to mrchantey/bevy that referenced this pull request Feb 4, 2025
Fixes bevyengine#17412

## Objective

`Parent` uses the "has a X" naming convention. There is increasing
sentiment that we should use the "is a X" naming convention for
relationships (following bevyengine#17398). This leaves `Children` as-is because
there is prevailing sentiment that `Children` is clearer than `ParentOf`
in many cases (especially when treating it like a collection).

This renames `Parent` to `ChildOf`.

This is just the implementation PR. To discuss the path forward, do so
in bevyengine#17412.

## Migration Guide

- The `Parent` component has been renamed to `ChildOf`.
mrchantey pushed a commit to mrchantey/bevy that referenced this pull request Feb 4, 2025
…17443)

# Objective

After bevyengine#17398, Bevy now has relations! We don't teach users how to make /
work with these in the examples yet though, but we definitely should.

## Solution

- Add a simple abstract example that goes over defining, spawning,
traversing and removing a custom relations.
- ~~Add `Relationship` and `RelationshipTarget` to the prelude: the
trait methods are really helpful here.~~
- this causes subtle ambiguities with method names and weird compiler
errors. Not doing it here!
- Clean up related documentation that I referenced when writing this
example.

## Testing

`cargo run --example relationships`

## Notes to reviewers

1. Yes, I know that the cycle detection code could be more efficient. I
decided to reduce the caching to avoid distracting from the broader
point of "here's how you traverse relationships".
2. Instead of using an `App`, I've decide to use
`World::run_system_once` + system functions defined inside of `main` to
do something closer to literate programming.

---------

Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>
Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com>
github-merge-queue bot pushed a commit that referenced this pull request Feb 6, 2025
Fixes #17535

Bevy's approach to handling "entity mapping" during spawning and cloning
needs some work. The addition of
[Relations](#17398) both
[introduced a new "duplicate entities" bug when spawning scenes in the
scene system](#17535) and made the weaknesses of the current mapping
system exceedingly clear:

1. Entity mapping requires _a ton_ of boilerplate (implement or derive
VisitEntities and VisitEntitesMut, then register / reflect MapEntities).
Knowing the incantation is challenging and if you forget to do it in
part or in whole, spawning subtly breaks.
2. Entity mapping a spawned component in scenes incurs unnecessary
overhead: look up ReflectMapEntities, create a _brand new temporary
instance_ of the component using FromReflect, map the entities in that
instance, and then apply that on top of the actual component using
reflection. We can do much better.

Additionally, while our new [Entity cloning
system](#16132) is already pretty
great, it has some areas we can make better:

* It doesn't expose semantic info about the clone (ex: ignore or "clone
empty"), meaning we can't key off of that in places where it would be
useful, such as scene spawning. Rather than duplicating this info across
contexts, I think it makes more sense to add that info to the clone
system, especially given that we'd like to use cloning code in some of
our spawning scenarios.
* EntityCloner is currently built in a way that prioritizes a single
entity clone
* EntityCloner's recursive cloning is built to be done "inside out" in a
parallel context (queue commands that each have a clone of
EntityCloner). By making EntityCloner the orchestrator of the clone we
can remove internal arcs, improve the clarity of the code, make
EntityCloner mutable again, and simplify the builder code.
* EntityCloner does not currently take into account entity mapping. This
is necessary to do true "bullet proof" cloning, would allow us to unify
the per-component scene spawning and cloning UX, and ultimately would
allow us to use EntityCloner in place of raw reflection for scenes like
`Scene(World)` (which would give us a nice performance boost: fewer
archetype moves, less reflection overhead).

## Solution

### Improved Entity Mapping

First, components now have first-class "entity visiting and mapping"
behavior:

```rust
#[derive(Component, Reflect)]
#[reflect(Component)]
struct Inventory {
    size: usize,
    #[entities]
    items: Vec<Entity>,
}
```

Any field with the `#[entities]` annotation will be viewable and
mappable when cloning and spawning scenes.

Compare that to what was required before!

```rust
#[derive(Component, Reflect, VisitEntities, VisitEntitiesMut)]
#[reflect(Component, MapEntities)]
struct Inventory {
    #[visit_entities(ignore)]
    size: usize,
    items: Vec<Entity>,
}
```

Additionally, for relationships `#[entities]` is implied, meaning this
"just works" in scenes and cloning:

```rust
#[derive(Component, Reflect)]
#[relationship(relationship_target = Children)]
#[reflect(Component)]
struct ChildOf(pub Entity);
```

Note that Component _does not_ implement `VisitEntities` directly.
Instead, it has `Component::visit_entities` and
`Component::visit_entities_mut` methods. This is for a few reasons:

1. We cannot implement `VisitEntities for C: Component` because that
would conflict with our impl of VisitEntities for anything that
implements `IntoIterator<Item=Entity>`. Preserving that impl is more
important from a UX perspective.
2. We should not implement `Component: VisitEntities` VisitEntities in
the Component derive, as that would increase the burden of manual
Component trait implementors.
3. Making VisitEntitiesMut directly callable for components would make
it easy to invalidate invariants defined by a component author. By
putting it in the `Component` impl, we can make it harder to call
naturally / unavailable to autocomplete using `fn
visit_entities_mut(this: &mut Self, ...)`.

`ReflectComponent::apply_or_insert` is now
`ReflectComponent::apply_or_insert_mapped`. By moving mapping inside
this impl, we remove the need to go through the reflection system to do
entity mapping, meaning we no longer need to create a clone of the
target component, map the entities in that component, and patch those
values on top. This will make spawning mapped entities _much_ faster
(The default `Component::visit_entities_mut` impl is an inlined empty
function, so it will incur no overhead for unmapped entities).

### The Bug Fix

To solve #17535, spawning code now skips entities with the new
`ComponentCloneBehavior::Ignore` and
`ComponentCloneBehavior::RelationshipTarget` variants (note
RelationshipTarget is a temporary "workaround" variant that allows
scenes to skip these components. This is a temporary workaround that can
be removed as these cases should _really_ be using EntityCloner logic,
which should be done in a followup PR. When that is done,
`ComponentCloneBehavior::RelationshipTarget` can be merged into the
normal `ComponentCloneBehavior::Custom`).

### Improved Cloning

* `Option<ComponentCloneHandler>` has been replaced by
`ComponentCloneBehavior`, which encodes additional intent and context
(ex: `Default`, `Ignore`, `Custom`, `RelationshipTarget` (this last one
is temporary)).
* Global per-world entity cloning configuration has been removed. This
felt overly complicated, increased our API surface, and felt too
generic. Each clone context can have different requirements (ex: what a
user wants in a specific system, what a scene spawner wants, etc). I'd
prefer to see how far context-specific EntityCloners get us first.
* EntityCloner's internals have been reworked to remove Arcs and make it
mutable.
* EntityCloner is now directly stored on EntityClonerBuilder,
simplifying the code somewhat
* EntityCloner's "bundle scratch" pattern has been moved into the new
BundleScratch type, improving its usability and making it usable in
other contexts (such as future cross-world cloning code). Currently this
is still private, but with some higher level safe APIs it could be used
externally for making dynamic bundles
* EntityCloner's recursive cloning behavior has been "externalized". It
is now responsible for orchestrating recursive clones, meaning it no
longer needs to be sharable/clone-able across threads / read-only.
* EntityCloner now does entity mapping during clones, like scenes do.
This gives behavior parity and also makes it more generically useful.
* `RelatonshipTarget::RECURSIVE_SPAWN` is now
`RelationshipTarget::LINKED_SPAWN`, and this field is used when cloning
relationship targets to determine if cloning should happen recursively.
The new `LINKED_SPAWN` term was picked to make it more generically
applicable across spawning and cloning scenarios.

## Next Steps

* I think we should adapt EntityCloner to support cross world cloning. I
think this PR helps set the stage for that by making the internals
slightly more generalized. We could have a CrossWorldEntityCloner that
reuses a lot of this infrastructure.
* Once we support cross world cloning, we should use EntityCloner to
spawn `Scene(World)` scenes. This would yield significant performance
benefits (no archetype moves, less reflection overhead).

---------

Co-authored-by: eugineerd <70062110+eugineerd@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
@rparrett
Copy link
Contributor

rparrett commented Feb 7, 2025

Could we offer users some advice on migrating the removed HierarchyEvent?

mrchantey pushed a commit to mrchantey/bevy that referenced this pull request Feb 17, 2025
Fixes bevyengine#17535

Bevy's approach to handling "entity mapping" during spawning and cloning
needs some work. The addition of
[Relations](bevyengine#17398) both
[introduced a new "duplicate entities" bug when spawning scenes in the
scene system](bevyengine#17535) and made the weaknesses of the current mapping
system exceedingly clear:

1. Entity mapping requires _a ton_ of boilerplate (implement or derive
VisitEntities and VisitEntitesMut, then register / reflect MapEntities).
Knowing the incantation is challenging and if you forget to do it in
part or in whole, spawning subtly breaks.
2. Entity mapping a spawned component in scenes incurs unnecessary
overhead: look up ReflectMapEntities, create a _brand new temporary
instance_ of the component using FromReflect, map the entities in that
instance, and then apply that on top of the actual component using
reflection. We can do much better.

Additionally, while our new [Entity cloning
system](bevyengine#16132) is already pretty
great, it has some areas we can make better:

* It doesn't expose semantic info about the clone (ex: ignore or "clone
empty"), meaning we can't key off of that in places where it would be
useful, such as scene spawning. Rather than duplicating this info across
contexts, I think it makes more sense to add that info to the clone
system, especially given that we'd like to use cloning code in some of
our spawning scenarios.
* EntityCloner is currently built in a way that prioritizes a single
entity clone
* EntityCloner's recursive cloning is built to be done "inside out" in a
parallel context (queue commands that each have a clone of
EntityCloner). By making EntityCloner the orchestrator of the clone we
can remove internal arcs, improve the clarity of the code, make
EntityCloner mutable again, and simplify the builder code.
* EntityCloner does not currently take into account entity mapping. This
is necessary to do true "bullet proof" cloning, would allow us to unify
the per-component scene spawning and cloning UX, and ultimately would
allow us to use EntityCloner in place of raw reflection for scenes like
`Scene(World)` (which would give us a nice performance boost: fewer
archetype moves, less reflection overhead).

## Solution

### Improved Entity Mapping

First, components now have first-class "entity visiting and mapping"
behavior:

```rust
#[derive(Component, Reflect)]
#[reflect(Component)]
struct Inventory {
    size: usize,
    #[entities]
    items: Vec<Entity>,
}
```

Any field with the `#[entities]` annotation will be viewable and
mappable when cloning and spawning scenes.

Compare that to what was required before!

```rust
#[derive(Component, Reflect, VisitEntities, VisitEntitiesMut)]
#[reflect(Component, MapEntities)]
struct Inventory {
    #[visit_entities(ignore)]
    size: usize,
    items: Vec<Entity>,
}
```

Additionally, for relationships `#[entities]` is implied, meaning this
"just works" in scenes and cloning:

```rust
#[derive(Component, Reflect)]
#[relationship(relationship_target = Children)]
#[reflect(Component)]
struct ChildOf(pub Entity);
```

Note that Component _does not_ implement `VisitEntities` directly.
Instead, it has `Component::visit_entities` and
`Component::visit_entities_mut` methods. This is for a few reasons:

1. We cannot implement `VisitEntities for C: Component` because that
would conflict with our impl of VisitEntities for anything that
implements `IntoIterator<Item=Entity>`. Preserving that impl is more
important from a UX perspective.
2. We should not implement `Component: VisitEntities` VisitEntities in
the Component derive, as that would increase the burden of manual
Component trait implementors.
3. Making VisitEntitiesMut directly callable for components would make
it easy to invalidate invariants defined by a component author. By
putting it in the `Component` impl, we can make it harder to call
naturally / unavailable to autocomplete using `fn
visit_entities_mut(this: &mut Self, ...)`.

`ReflectComponent::apply_or_insert` is now
`ReflectComponent::apply_or_insert_mapped`. By moving mapping inside
this impl, we remove the need to go through the reflection system to do
entity mapping, meaning we no longer need to create a clone of the
target component, map the entities in that component, and patch those
values on top. This will make spawning mapped entities _much_ faster
(The default `Component::visit_entities_mut` impl is an inlined empty
function, so it will incur no overhead for unmapped entities).

### The Bug Fix

To solve bevyengine#17535, spawning code now skips entities with the new
`ComponentCloneBehavior::Ignore` and
`ComponentCloneBehavior::RelationshipTarget` variants (note
RelationshipTarget is a temporary "workaround" variant that allows
scenes to skip these components. This is a temporary workaround that can
be removed as these cases should _really_ be using EntityCloner logic,
which should be done in a followup PR. When that is done,
`ComponentCloneBehavior::RelationshipTarget` can be merged into the
normal `ComponentCloneBehavior::Custom`).

### Improved Cloning

* `Option<ComponentCloneHandler>` has been replaced by
`ComponentCloneBehavior`, which encodes additional intent and context
(ex: `Default`, `Ignore`, `Custom`, `RelationshipTarget` (this last one
is temporary)).
* Global per-world entity cloning configuration has been removed. This
felt overly complicated, increased our API surface, and felt too
generic. Each clone context can have different requirements (ex: what a
user wants in a specific system, what a scene spawner wants, etc). I'd
prefer to see how far context-specific EntityCloners get us first.
* EntityCloner's internals have been reworked to remove Arcs and make it
mutable.
* EntityCloner is now directly stored on EntityClonerBuilder,
simplifying the code somewhat
* EntityCloner's "bundle scratch" pattern has been moved into the new
BundleScratch type, improving its usability and making it usable in
other contexts (such as future cross-world cloning code). Currently this
is still private, but with some higher level safe APIs it could be used
externally for making dynamic bundles
* EntityCloner's recursive cloning behavior has been "externalized". It
is now responsible for orchestrating recursive clones, meaning it no
longer needs to be sharable/clone-able across threads / read-only.
* EntityCloner now does entity mapping during clones, like scenes do.
This gives behavior parity and also makes it more generically useful.
* `RelatonshipTarget::RECURSIVE_SPAWN` is now
`RelationshipTarget::LINKED_SPAWN`, and this field is used when cloning
relationship targets to determine if cloning should happen recursively.
The new `LINKED_SPAWN` term was picked to make it more generically
applicable across spawning and cloning scenarios.

## Next Steps

* I think we should adapt EntityCloner to support cross world cloning. I
think this PR helps set the stage for that by making the internals
slightly more generalized. We could have a CrossWorldEntityCloner that
reuses a lot of this infrastructure.
* Once we support cross world cloning, we should use EntityCloner to
spawn `Scene(World)` scenes. This would yield significant performance
benefits (no archetype moves, less reflection overhead).

---------

Co-authored-by: eugineerd <70062110+eugineerd@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Complex Quite challenging from either a design or technical perspective. Ask for help! M-Needs-Release-Note Work that should be called out in the blog due to impact S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers
Projects
None yet