ca5712a7bf
23 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Lucas
|
ca5712a7bf |
feat: derive Serialize on Childof (#19336)
# Objective allow serialization / deserialization on the `ChildOf` entity, for example in network usage. my usage was for the bevy_replicon crate, to replicate `ChildOf`. ## Solution same implementation of serde as other types in the bevy repo --------- Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com> |
||
|
databasedav
|
289c51b547 |
fix .insert_related index bound (#19134)
# Objective resolves #19092 ## Solution - remove the `.saturating_sub` from the index transformation - add `.saturating_add` to the internal offset calculation ## Testing - added regression test, confirming 0 index order + testing max bound |
||
|
JaySpruce
|
f006f02f78 |
Add remove_children and remove_related to EntityWorldMut and EntityCommands (#18835)
Fixes #18834. `EntityWorldMut::remove_children` and `EntityCommands::remove_children` were removed in the relationships overhaul (#17398) and never got replaced. I don't *think* this was intentional (the methods were never mentioned in the PR or its comments), but I could've missed something. |
||
|
JaySpruce
|
fa480cded7 |
Fix wrong method call in relationship replacement command (#18824)
Fixes a small mix-up from #18058, which added bulk relationship replacement methods. `EntityCommands::replace_related_with_difference` calls `EntityWorldMut::replace_children_with_difference` instead of `EntityWorldMut::replace_related_with_difference`, which means it always operates on the `ChildOf` relationship instead of the `R: Relationship` generic it's provided. `EntityCommands::replace_children_with_difference` takes an `R: Relationship` generic that it shouldn't, but it accidentally works correctly on `main` because it calls the above method. |
||
|
Freyja-moth
|
772524b3a6 |
Change with_related to work with a Bundle and added with_relationships method (#18699)
# Objective Fixes #18678 ## Solution Moved the current `with_related` method to `with_relationships` and added a new `with_related` that uses a bundle. I'm not entirely sold on the name just yet, if anyone has any ideas let me know. ## Testing I wasn't able to test these changes because it crashed my computer every time I tried (fun). But there don't seem to be any tests that use the old `with_related` method so it should be fine, hopefully ## Showcase ```rust commands.spawn_empty() .with_related::<Relationship>(Name::new("Related thingy")) .with_relationships(|rel| { rel.spawn(Name::new("Second related thingy")); }); ``` --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com> |
||
|
JaySpruce
|
deba691fff |
Implement insert_children for EntityCommands (#18675)
Extension of #18409. I was updating a migration guide for hierarchy commands and realized `insert_children` wasn't added to `EntityCommands`, only `EntityWorldMut`. This adds that and `insert_related` (basically just some copy-and-pasting). |
||
|
Carter Anderson
|
1553ee98ff |
Switch ChildOf back to tuple struct (#18672)
# Objective In #17905 we swapped to a named field on `ChildOf` to help resolve variable naming ambiguity of child vs parent (ex: `child_of.parent` clearly reads as "I am accessing the parent of the child_of relationship", whereas `child_of.0` is less clear). Unfortunately this has the side effect of making initialization less ideal. `ChildOf { parent }` reads just as well as `ChildOf(parent)`, but `ChildOf { parent: root }` doesn't read nearly as well as `ChildOf(root)`. ## Solution Move back to `ChildOf(pub Entity)` but add a `child_of.parent()` function and use it for all accesses. The downside here is that users are no longer "forced" to access the parent field with `parent` nomenclature, but I think this strikes the right balance. Take a look at the diff. I think the results provide strong evidence for this change. Initialization has the benefit of reading much better _and_ of taking up significantly less space, as many lines go from 3 to 1, and we're cutting out a bunch of syntax in some cases. Sadly I do think this should land in 0.16 as the cost of doing this _after_ the relationships migration is high. |
||
|
Eagster
|
11ad3b6dde |
Finish #17558, re-adding insert_children (#18409)
fixes #17478 # Objective - Complete #17558. - the `insert_children` method was previously removed, and as #17478 points out, needs to be added back. ## Solution - Add a `OrderedRelationshipSourceCollection`, which allows sorting, ordering, rearranging, etc of a `RelationshipSourceCollection`. - Implement `insert_related` - Implement `insert_children` - Tidy up some docs while I'm here. ## Testing @bjoernp116 set up a unit test, and I added a doc test to `OrderedRelationshipSourceCollection`. --------- Co-authored-by: bjoernp116 <bjoernpollen@gmail.com> Co-authored-by: Dmytro Banin <banind@cs.washington.edu> Co-authored-by: Talin <viridia@gmail.com> |
||
|
Ida "Iyes
|
0f52a2816d |
Regression fix: Reintroduce sorting/reordering methods on Children (#18476)
# Objective Bevy 0.15 used to have methods on `Children` for sorting and reordering them. This is very important, because in certain situations, the order of children matters. For example, in the context of UI nodes. These methods are missing/omitted/forgotten in the current version, after the Relationships rework. Without them, it is impossible for me to upgrade `iyes_perf_ui` to Bevy 0.16. ## Solution Reintroduce the methods. This PR simply copy-pastes them from Bevy 0.15. |
||
|
Brezak
|
7cd729cf1a |
Add methods to bulk replace relationships on a entity (#18058)
# Objective Add a way to efficiently replace a set of specifically related entities with a new set. Closes #18041 ## Solution Add new `replace_(related/children)` to `EntityWorldMut` and friends. ## Testing Added a new test to `hierarchy.rs` that specifically check if `replace_children` actually correctly replaces the children on a entity while keeping the original one. --- ## Showcase `EntityWorldMut` and `EntityCommands` can now be used to efficiently replace the entities a entity is related to. ```rust /// `parent` has 2 children. `entity_a` and `entity_b`. assert_eq!([entity_a, entity_b], world.entity(parent).get::<Children>()); /// Replace `parent`s children with `entity_a` and `entity_c` world.entity_mut(parent).replace_related(&[entity_a, entity_c]); /// `parent` now has 2 children. `entity_a` and `entity_c`. /// /// `replace_children` has saved time by not removing and reading /// the relationship between `entity_a` and `parent` assert_eq!([entity_a, entity_c], world.entity(parent).get::<Children>()); --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Carter Anderson
|
6d6054116a
|
Support skipping Relationship on_replace hooks (#18378)
# Objective Fixes #18357 ## Solution Generalize `RelationshipInsertHookMode` to `RelationshipHookMode`, wire it up to on_replace execution, and use it in the `Relationship::on_replace` hook. |
||
|
Gino Valente
|
9b32e09551
|
bevy_reflect: Add clone registrations project-wide (#18307)
# Objective Now that #13432 has been merged, it's important we update our reflected types to properly opt into this feature. If we do not, then this could cause issues for users downstream who want to make use of reflection-based cloning. ## Solution This PR is broken into 4 commits: 1. Add `#[reflect(Clone)]` on all types marked `#[reflect(opaque)]` that are also `Clone`. This is mandatory as these types would otherwise cause the cloning operation to fail for any type that contains it at any depth. 2. Update the reflection example to suggest adding `#[reflect(Clone)]` on opaque types. 3. Add `#[reflect(clone)]` attributes on all fields marked `#[reflect(ignore)]` that are also `Clone`. This prevents the ignored field from causing the cloning operation to fail. Note that some of the types that contain these fields are also `Clone`, and thus can be marked `#[reflect(Clone)]`. This makes the `#[reflect(clone)]` attribute redundant. However, I think it's safer to keep it marked in the case that the `Clone` impl/derive is ever removed. I'm open to removing them, though, if people disagree. 4. Finally, I added `#[reflect(Clone)]` on all types that are also `Clone`. While not strictly necessary, it enables us to reduce the generated output since we can just call `Clone::clone` directly instead of calling `PartialReflect::reflect_clone` on each variant/field. It also means we benefit from any optimizations or customizations made in the `Clone` impl, including directly dereferencing `Copy` values and increasing reference counters. Along with that change I also took the liberty of adding any missing registrations that I saw could be applied to the type as well, such as `Default`, `PartialEq`, and `Hash`. There were hundreds of these to edit, though, so it's possible I missed quite a few. That last commit is **_massive_**. There were nearly 700 types to update. So it's recommended to review the first three before moving onto that last one. Additionally, I can break the last commit off into its own PR or into smaller PRs, but I figured this would be the easiest way of doing it (and in a timely manner since I unfortunately don't have as much time as I used to for code contributions). ## Testing You can test locally with a `cargo check`: ``` cargo check --workspace --all-features ``` |
||
|
Greeble
|
780f658f2c
|
Update ChildOf deprecation advice to match new layout (#18089)
https://github.com/bevyengine/bevy/pull/17905 replaced `ChildOf(entity)` with `ChildOf { parent: entity }`, but some deprecation advice was overlooked. Also corrected formatting in documentation. ## Testing Added a `set_parent` to a random example. Confirmed that the deprecation warning shows and the advice can be pasted in. |
||
|
Carter Anderson
|
b73811d40e
|
Remove ChildOf::get and Deref impl (#18080)
# Objective There are currently three ways to access the parent stored on a ChildOf relationship: 1. `child_of.parent` (field accessor) 2. `child_of.get()` (get function) 3. `**child_of` (Deref impl) I will assert that we should only have one (the field accessor), and that the existence of the other implementations causes confusion and legibility issues. The deref approach is heinous, and `child_of.get()` is significantly less clear than `child_of.parent`. ## Solution Remove `impl Deref for ChildOf` and `ChildOf::get`. The one "downside" I'm seeing is that: ```rust entity.get::<ChildOf>().map(ChildOf::get) ``` Becomes this: ```rust entity.get::<ChildOf>().map(|c| c.parent) ``` I strongly believe that this is worth the increased clarity and consistency. I'm also not really a huge fan of the "pass function pointer to map" syntax. I think most people don't think this way about maps. They think in terms of a function that takes the item in the Option and returns the result of some action on it. ## Migration Guide ```rust // Before **child_of // After child_of.parent // Before child_of.get() // After child_of.parent // Before entity.get::<ChildOf>().map(ChildOf::get) // After entity.get::<ChildOf>().map(|c| c.parent) ``` |
||
|
Tim Overbeek
|
ccb7069e7f
|
Change ChildOf to Childof { parent: Entity} and support deriving Relationship and RelationshipTarget with named structs (#17905)
# Objective fixes #17896 ## Solution Change ChildOf ( Entity ) to ChildOf { parent: Entity } by doing this we also allow users to use named structs for relationship derives, When you have more than 1 field in a struct with named fields the macro will look for a field with the attribute #[relationship] and all of the other fields should implement the Default trait. Unnamed fields are still supported. When u have a unnamed struct with more than one field the macro will fail. Do we want to support something like this ? ```rust #[derive(Component)] #[relationship_target(relationship = ChildOf)] pub struct Children (#[relationship] Entity, u8); ``` I could add this, it but doesn't seem nice. ## Testing crates/bevy_ecs - cargo test ## Showcase ```rust use bevy_ecs::component::Component; use bevy_ecs::entity::Entity; #[derive(Component)] #[relationship(relationship_target = Children)] pub struct ChildOf { #[relationship] pub parent: Entity, internal: u8, }; #[derive(Component)] #[relationship_target(relationship = ChildOf)] pub struct Children { children: Vec<Entity> }; ``` --------- Co-authored-by: Tim Overbeek <oorbecktim@Tims-MacBook-Pro.local> Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-042.client.nl.eduvpn.org> Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-059.client.nl.eduvpn.org> Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-054.client.nl.eduvpn.org> Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-027.client.nl.eduvpn.org> |
||
|
Alice Cecile
|
be3c6f7578
|
Improve the docs for ChildOf and Children (#17886)
# Context Renaming `Parent` to `ChildOf` in #17247 has been contentious. While those users concerns are valid (especially around legibility of code IMO!), @cart [has decided](https://discord.com/channels/691052431525675048/749335865876021248/1340434322833932430) to stick with the new name. > In general this conversation is unsurprising to me, as it played out essentially the same way when I asked for opinions in my PR. There are strong opinions on both sides. Everyone is right in their own way. > > I chose ChildOf for the following reasons: > > 1. I think it derives naturally from the system we have built, the concepts we have chosen, and how we generally name the types that implement a trait in Rust. This is the name of the type implementing Relationship. We are adding that Relationship component to a given entity (whether it "is" the relationship or "has" the relationship is kind of immaterial ... we are naming the relationship that it "is" or "has"). What is the name of the relationship that a child has to its parent? It is a "child" of the parent of course! > 2. In general the non-parent/child relationships I've seen in the wild generally benefit from (or need to) use the naming convention in (1) (aka calling the Relationship the name of the relationship the entity has). Many relationships don't have an equivalent to the Parent/Child name concept. > 3. I do think we could get away with using (1) for pretty much everything else and special casing Parent/Children. But by embracing the naming convention, we help establish that this is in fact a pattern, and we help prime people to think about these things in a consistent way. Consistency and predictability is a generally desirable property. And for something as divisive and polarizing as relationship naming, I think drawing a hard line in the sand is to the benefit of the community as a whole. > 4. I believe the fact that we dont see as much of the XOf naming style elsewhere is to our benefit. When people see things in that style, they are primed to think of them as relationships (after some exposure to Bevy and the ecosystem). I consider this a useful hint. > 5. Most of the practical confusion from using ChildOf seems to be from calling the value of the target field we read from the relationship child_of. The name of the target field should be parent (we could even consider renaming child_of.0 to child_of.parent for clarity). I suspect that existing Bevy users renaming their existing code will feel the most friction here, as this requires a reframing. Imo it is natural and expected to receive pushback from these users hitting this case. ## Objective The new documentation doesn't do a particularly good job at quickly explaining the meaning of each component or how to work with them; making a tricky migration more painful and slowing down new users as they learn about some of the most fundamental types in Bevy. ## Solution 1. Clearly explain what each component does in the very first line, assuming no background knowledge. This is the first relationships that 99% of users will encounter, so explaining that they are relationships is unhelpful as an introduction. 2. Add doc aliases for the rejected `IsParent`/`IsChild`/`Parent` names, to improve autocomplete and doc searching. 3. Do some assorted docs cleanup while we're here. --------- Co-authored-by: Eagster <79881080+ElliottjPierce@users.noreply.github.com> |
||
|
Carter Anderson
|
ea578415e1
|
Improved Spawn APIs and Bundle Effects (#17521)
## Objective
A major critique of Bevy at the moment is how boilerplatey it is to
compose (and read) entity hierarchies:
```rust
commands
.spawn(Foo)
.with_children(|p| {
p.spawn(Bar).with_children(|p| {
p.spawn(Baz);
});
p.spawn(Bar).with_children(|p| {
p.spawn(Baz);
});
});
```
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](https://github.com/bevyengine/bevy/discussions/16920)
_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 all
`RelationshipTarget` impls, which looks like this in practice:
```rust
world.spawn((
Foo,
Children::spawn((
Spawn((
Bar,
Children::spawn(Spawn(Baz)),
)),
Spawn((
Bar,
Children::spawn(Spawn(Baz)),
)),
))
))
```
`Children::spawn` returns `SpawnRelatedBundle<Children, L:
SpawnableList>`, which is a `Bundle` that inserts `Children`
(preallocated to the size of the `SpawnableList::size_hint()`).
`Spawn<B: Bundle>(pub B)` implements `SpawnableList` with a size of 1.
`SpawnableList` is also implemented for tuples of `SpawnableList` (same
general pattern as the Bundle impl).
There are currently three built-in `SpawnableList` implementations:
```rust
world.spawn((
Foo,
Children::spawn((
Spawn(Name::new("Child1")),
SpawnIter(["Child2", "Child3"].into_iter().map(Name::new),
SpawnWith(|parent: &mut ChildSpawner| {
parent.spawn(Name::new("Child4"));
parent.spawn(Name::new("Child5"));
})
)),
))
```
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:
1. By adding support for variadics to the Rust type system (removing the
need for nested bundles). This is out of scope for this PR :)
2. Using wrapper types to resolve the ambiguity (this is what I did in
this PR).
For the single-entity spawn cases, `Children::spawn_one` does also
exist, which removes the need for the wrapper:
```rust
world.spawn((
Foo,
Children::spawn_one(Bar),
))
```
## 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!
```rust
world.spawn((
Foo,
Observers::spawn((
Spawn(Observer::new(|trigger: Trigger<FuseLit>| {})),
Spawn(Observer::new(|trigger: Trigger<Exploded>| {})),
)),
OwnerOf::spawn(Spawn(Bar))
Children::spawn(Spawn(Baz))
))
```
## 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 new `children![X]` macro, which internally
produces `Children::spawn(Spawn(X))`:
```rust
world.spawn((
Foo,
children![
(
Bar,
children![Baz],
),
(
Bar,
children![Baz],
),
]
))
```
There is also a `related!` macro, which is a generic version of the
`children!` macro that supports any relationship type:
```rust
world.spawn((
Foo,
related!(Children[
(
Bar,
related!(Children[Baz]),
),
(
Bar,
related!(Children[Baz]),
),
])
))
```
## Returning Hierarchies from Functions
Thanks to these changes, the following pattern is now possible:
```rust
fn button(text: &str, color: Color) -> impl Bundle {
(
Node {
width: Val::Px(300.),
height: Val::Px(100.),
..default()
},
BackgroundColor(color),
children![
Text::new(text),
]
)
}
fn ui() -> impl Bundle {
(
Node {
width: Val::Percent(100.0),
height: Val::Percent(100.0),
..default(),
},
children![
button("hello", BLUE),
button("world", RED),
]
)
}
// spawn from a system
fn system(mut commands: Commands) {
commands.spawn(ui());
}
// spawn directly on World
world.spawn(ui());
```
## Additional Changes and Notes
* `Bundle::from_components` has been split out into
`BundleFromComponents::from_components`, enabling us to implement
`Bundle` for types that cannot be "taken" from the ECS (such as the new
`SpawnRelatedBundle`).
* The `NoBundleEffect` trait (which implements `BundleEffect`) 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.
* I've ported a few examples to illustrate real-world usage. I think in
a followup we should port all examples to the `children!` form whenever
possible (and for cases that require things like SpawnIter, use the raw
APIs).
* Some may ask "why not use the `Relationship` to spawn (ex:
`ChildOf::spawn(Foo)`) instead of the `RelationshipTarget` (ex:
`Children::spawn(Spawn(Foo))`)?". That _would_ allow us to remove the
`Spawn` wrapper. I've explicitly chosen to disallow this pattern.
`Bundle::Effect` has the ability to create _significant_ weirdness.
Things in `Bundle` position look like components. For example
`world.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 that `Bundle::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
* The macros are currently trivially implemented using macro_rules and
are currently limited to the max tuple length. They will require a
proc_macro implementation to work around the tuple length limit.
## Next Steps
* Port the remaining examples to use `children!` where possible and raw
`Spawn` / `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 `()`.
Additionally `Bundle::from_components` has been moved to the new
`BundleFromComponents` trait.
```rust
// Before
unsafe impl Bundle for X {
unsafe fn from_components<T, F>(ctx: &mut T, func: &mut F) -> Self {
}
/* remaining bundle impl here */
}
// After
unsafe impl Bundle for X {
type Effect = ();
/* remaining bundle impl here */
}
unsafe impl BundleFromComponents for X {
unsafe fn from_components<T, F>(ctx: &mut T, func: &mut F) -> Self {
}
}
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
Co-authored-by: Emerson Coskey <emerson@coskey.dev>
|
||
|
raldone01
|
1b7db895b7
|
Harden proc macro path resolution and add integration tests. (#17330)
This pr uses the `extern crate self as` trick to make proc macros behave the same way inside and outside bevy. # Objective - Removes noise introduced by `crate as` in the whole bevy repo. - Fixes #17004. - Hardens proc macro path resolution. ## TODO - [x] `BevyManifest` needs cleanup. - [x] Cleanup remaining `crate as`. - [x] Add proper integration tests to the ci. ## Notes - `cargo-manifest-proc-macros` is written by me and based/inspired by the old `BevyManifest` implementation and [`bkchr/proc-macro-crate`](https://github.com/bkchr/proc-macro-crate). - What do you think about the new integration test machinery I added to the `ci`? More and better integration tests can be added at a later stage. The goal of these integration tests is to simulate an actual separate crate that uses bevy. Ideally they would lightly touch all bevy crates. ## Testing - Needs RA test - Needs testing from other users - Others need to run at least `cargo run -p ci integration-test` and verify that they work. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Carter Anderson
|
3c8fae2390
|
Improved Entity Mapping and Cloning (#17687)
Fixes #17535 Bevy's approach to handling "entity mapping" during spawning and cloning needs some work. The addition of [Relations](https://github.com/bevyengine/bevy/pull/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](https://github.com/bevyengine/bevy/pull/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> |
||
|
Zachary Harrold
|
41e79ae826
|
Refactored ComponentHook Parameters into HookContext (#17503)
# Objective
- Make the function signature for `ComponentHook` less verbose
## Solution
- Refactored `Entity`, `ComponentId`, and `Option<&Location>` into a new
`HookContext` struct.
## Testing
- CI
---
## Migration Guide
Update the function signatures for your component hooks to only take 2
arguments, `world` and `context`. Note that because `HookContext` is
plain data with all members public, you can use de-structuring to
simplify migration.
```rust
// Before
fn my_hook(
mut world: DeferredWorld,
entity: Entity,
component_id: ComponentId,
) { ... }
// After
fn my_hook(
mut world: DeferredWorld,
HookContext { entity, component_id, caller }: HookContext,
) { ... }
```
Likewise, if you were discarding certain parameters, you can use `..` in
the de-structuring:
```rust
// Before
fn my_hook(
mut world: DeferredWorld,
entity: Entity,
_: ComponentId,
) { ... }
// After
fn my_hook(
mut world: DeferredWorld,
HookContext { entity, .. }: HookContext,
) { ... }
```
|
||
|
SpecificProtagonist
|
f32a6fb205
|
Track callsite for observers & hooks (#15607)
# Objective Fixes #14708 Also fixes some commands not updating tracked location. ## Solution `ObserverTrigger` has a new `caller` field with the `track_change_detection` feature; hooks take an additional caller parameter (which is `Some(…)` or `None` depending on the feature). ## Testing See the new tests in `src/observer/mod.rs` --- ## Showcase Observers now know from where they were triggered (if `track_change_detection` is enabled): ```rust world.observe(move |trigger: Trigger<OnAdd, Foo>| { println!("Added Foo from {}", trigger.caller()); }); ``` ## Migration - hooks now take an additional `Option<&'static Location>` argument --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Carter Anderson
|
ba5e71f53d
|
Parent -> ChildOf (#17427)
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`. |
||
|
Carter Anderson
|
21f1e3045c
|
Relationships (non-fragmenting, one-to-many) (#17398)
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](https://github.com/bevyengine/bevy/discussions/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](https://github.com/bevyengine/bevy/discussions/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 #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. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |