2c22bc12a0
62 Commits
Author | SHA1 | Message | Date | |
---|---|---|---|---|
![]() |
a530c07bc5
|
Preserve spawned RelationshipTarget order and other improvements (#17858)
Fixes #17720 ## Objective Spawning RelationshipTargets from scenes currently fails to preserve RelationshipTarget ordering (ex: `Children` has an arbitrary order). This is because it uses the normal hook flow to set up the collection, which means we are pushing onto the collection in _spawn order_ (which is currently in archetype order, which will often produce mismatched orderings). We need to preserve the ordering in the original RelationshipTarget collection. Ideally without expensive checking / fixups. ## Solution One solution would be to spawn in hierarchy-order. However this gets complicated as there can be multiple hierarchies, and it also means we can't spawn in more cache-friendly orders (ex: the current per-archetype spawning, or future even-smarter per-table spawning). Additionally, same-world cloning has _slightly_ more nuanced needs (ex: recursively clone linked relationships, while maintaining _original_ relationships outside of the tree via normal hooks). The preferred approach is to directly spawn the remapped RelationshipTarget collection, as this trivially preserves the ordering. Unfortunately we can't _just_ do that, as when we spawn the children with their Relationships (ex: `ChildOf`), that will insert a duplicate. We could "fixup" the collection retroactively by just removing the back half of duplicates, but this requires another pass / more lookups / allocating twice as much space. Additionally, it becomes complicated because observers could insert additional children, making it harder (aka more expensive) to determine which children are dupes and which are not. The path I chose is to support "opting out" of the relationship target hook in the contexts that need that, as this allows us to just cheaply clone the mapped collection. The relationship hook can look for this configuration when it runs and skip its logic when that happens. A "simple" / small-amount-of-code way to do this would be to add a "skip relationship spawn" flag to World. Sadly, any hook / observer that runs _as the result of an insert_ would also read this flag. We really need a way to scope this setting to a _specific_ insert. Therefore I opted to add a new `RelationshipInsertHookMode` enum and an `entity.insert_with_relationship_insert_hook_mode` variant. Obviously this is verbose and ugly. And nobody wants _more_ insert variants. But sadly this was the best I could come up with from a performance and capability perspective. If you have alternatives let me know! There are three variants: 1. `RelationshipInsertHookMode::Run`: always run relationship insert hooks (this is the default) 2. `RelationshipInsertHookMode::Skip`: do not run any relationship insert hooks for this insert (this is used by spawner code) 3. `RelationshipInsertHookMode::RunIfNotLinked`: only run hooks for _unlinked_ relationships (this is used in same-world recursive entity cloning to preserve relationships outside of the deep-cloned tree) Note that I have intentionally only added "insert with relationship hook mode" variants to the cases we absolutely need (everything else uses the default `Run` mode), just to keep the code size in check. I do not think we should add more without real _very necessary_ use cases. I also made some other minor tweaks: 1. I split out `SourceComponent` from `ComponentCloneCtx`. Reading the source component no longer needlessly blocks mutable access to `ComponentCloneCtx`. 2. Thanks to (1), I've removed the `RefCell` wrapper over the cloned component queue. 3. (1) also allowed me to write to the EntityMapper while queuing up clones, meaning we can reserve entities during the component clone and write them to the mapper _before_ inserting the component, meaning cloned collections can be mapped on insert. 4. I've removed the closure from `write_target_component_ptr` to simplify the API / make it compatible with the split `SourceComponent` approach. 5. I've renamed `EntityCloner::recursive` to `EntityCloner::linked_cloning` to connect that feature more directly with `RelationshipTarget::LINKED_SPAWN` 6. I've removed `EntityCloneBehavior::RelationshipTarget`. This was always intended to be temporary, and this new behavior removes the need for it. --------- Co-authored-by: Viktor Gustavsson <villor94@gmail.com> |
||
![]() |
5241e09671
|
Upgrade to Rust Edition 2024 (#17967)
# Objective - Fixes #17960 ## Solution - Followed the [edition upgrade guide](https://doc.rust-lang.org/edition-guide/editions/transitioning-an-existing-project-to-a-new-edition.html) ## Testing - CI --- ## Summary of Changes ### Documentation Indentation When using lists in documentation, proper indentation is now linted for. This means subsequent lines within the same list item must start at the same indentation level as the item. ```rust /* Valid */ /// - Item 1 /// Run-on sentence. /// - Item 2 struct Foo; /* Invalid */ /// - Item 1 /// Run-on sentence. /// - Item 2 struct Foo; ``` ### Implicit `!` to `()` Conversion `!` (the never return type, returned by `panic!`, etc.) no longer implicitly converts to `()`. This is particularly painful for systems with `todo!` or `panic!` statements, as they will no longer be functions returning `()` (or `Result<()>`), making them invalid systems for functions like `add_systems`. The ideal fix would be to accept functions returning `!` (or rather, _not_ returning), but this is blocked on the [stabilisation of the `!` type itself](https://doc.rust-lang.org/std/primitive.never.html), which is not done. The "simple" fix would be to add an explicit `-> ()` to system signatures (e.g., `|| { todo!() }` becomes `|| -> () { todo!() }`). However, this is _also_ banned, as there is an existing lint which (IMO, incorrectly) marks this as an unnecessary annotation. So, the "fix" (read: workaround) is to put these kinds of `|| -> ! { ... }` closuers into variables and give the variable an explicit type (e.g., `fn()`). ```rust // Valid let system: fn() = || todo!("Not implemented yet!"); app.add_systems(..., system); // Invalid app.add_systems(..., || todo!("Not implemented yet!")); ``` ### Temporary Variable Lifetimes The order in which temporary variables are dropped has changed. The simple fix here is _usually_ to just assign temporaries to a named variable before use. ### `gen` is a keyword We can no longer use the name `gen` as it is reserved for a future generator syntax. This involved replacing uses of the name `gen` with `r#gen` (the raw-identifier syntax). ### Formatting has changed Use statements have had the order of imports changed, causing a substantial +/-3,000 diff when applied. For now, I have opted-out of this change by amending `rustfmt.toml` ```toml style_edition = "2021" ``` This preserves the original formatting for now, reducing the size of this PR. It would be a simple followup to update this to 2024 and run `cargo fmt`. ### New `use<>` Opt-Out Syntax Lifetimes are now implicitly included in RPIT types. There was a handful of instances where it needed to be added to satisfy the borrow checker, but there may be more cases where it _should_ be added to avoid breakages in user code. ### `MyUnitStruct { .. }` is an invalid pattern Previously, you could match against unit structs (and unit enum variants) with a `{ .. }` destructuring. This is no longer valid. ### Pretty much every use of `ref` and `mut` are gone Pattern binding has changed to the point where these terms are largely unused now. They still serve a purpose, but it is far more niche now. ### `iter::repeat(...).take(...)` is bad New lint recommends using the more explicit `iter::repeat_n(..., ...)` instead. ## Migration Guide The lifetimes of functions using return-position impl-trait (RPIT) are likely _more_ conservative than they had been previously. If you encounter lifetime issues with such a function, please create an issue to investigate the addition of `+ use<...>`. ## Notes - Check the individual commits for a clearer breakdown for what _actually_ changed. --------- Co-authored-by: François Mockers <francois.mockers@vleue.com> |
||
![]() |
0a32450715
|
Support using FilteredResources with ReflectResource. (#15624)
# Objective Support accessing resources using reflection when using `FilteredResources` in a dynamic system. This is similar to how components can be queried using reflection when using `FilteredEntityRef|Mut`. ## Solution Change `ReflectResource` from taking `&World` and `&mut World` to taking `impl Into<FilteredResources>` and `impl Into<FilteredResourcesMut>`, similar to how `ReflectComponent` takes `impl Into<FilteredEntityRef>` and `impl Into<FilteredEntityMut>`. There are `From` impls that ensure code passing `&World` and `&mut World` continues to work as before. ## Migration Guide If you are manually creating a `ReflectComponentFns` struct, the `reflect` function now takes `FilteredResources` instead `&World`, and there is a new `reflect_mut` function that takes `FilteredResourcesMut`. |
||
![]() |
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> |
||
![]() |
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> |
||
![]() |
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> |
||
![]() |
44ad3bf62b
|
Move Resource trait to its own file (#17469)
# Objective `bevy_ecs`'s `system` module is something of a grab bag, and *very* large. This is particularly true for the `system_param` module, which is more than 2k lines long! While it could be defensible to put `Res` and `ResMut` there (lol no they're in change_detection.rs, obviously), it doesn't make any sense to put the `Resource` trait there. This is confusing to navigate (and painful to work on and review). ## Solution - Create a root level `bevy_ecs/resource.rs` module to mirror `bevy_ecs/component.rs` - move the `Resource` trait to that module - move the `Resource` derive macro to that module as well (Rust really likes when you pun on the names of the derive macro and trait and put them in the same path) - fix all of the imports ## Notes to reviewers - We could probably move more stuff into here, but I wanted to keep this PR as small as possible given the absurd level of import changes. - This PR is ground work for my upcoming attempts to store resource data on components (resources-as-entities). Splitting this code out will make the work and review a bit easier, and is the sort of overdue refactor that's good to do as part of more meaningful work. ## Testing cargo build works! ## Migration Guide `bevy_ecs::system::Resource` has been moved to `bevy_ecs::resource::Resource`. |
||
![]() |
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> |
||
![]() |
f5d38f30cc
|
Fix entity does not exist message on index reuse (#17264)
# Objective With the `track_location` feature, the error message of trying to acquire an entity that was despawned pointed to the wrong line if the entity index has been reused. ## Showcase ```rust use bevy_ecs::prelude::*; fn main() { let mut world = World::new(); let e = world.spawn_empty().id(); world.despawn(e); world.flush(); let _ = world.spawn_empty(); world.entity(e); } ``` Old message: ``` Entity 0v1 was despawned by src/main.rs:8:19 ``` New message: ``` Entity 0v1 does not exist (its index has been reused) ``` |
||
![]() |
4bca7f1b6d
|
Improved Command Errors (#17215)
# Objective Rework / build on #17043 to simplify the implementation. #17043 should be merged first, and the diff from this PR will get much nicer after it is merged (this PR is net negative LOC). ## Solution 1. Command and EntityCommand have been vastly simplified. No more marker components. Just one function. 2. Command and EntityCommand are now generic on the return type. This enables result-less commands to exist, and allows us to statically distinguish between fallible and infallible commands, which allows us to skip the "error handling overhead" for cases that don't need it. 3. There are now only two command queue variants: `queue` and `queue_fallible`. `queue` accepts commands with no return type. `queue_fallible` accepts commands that return a Result (specifically, one that returns an error that can convert to `bevy_ecs::result::Error`). 4. I've added the concept of the "default error handler", which is used by `queue_fallible`. This is a simple direct call to the `panic()` error handler by default. Users that want to override this can enable the `configurable_error_handler` cargo feature, then initialize the GLOBAL_ERROR_HANDLER value on startup. This is behind a flag because there might be minor overhead with `OnceLock` and I'm guessing this will be a niche feature. We can also do perf testing with OnceLock if someone really wants it to be used unconditionally, but I don't personally feel the need to do that. 5. I removed the "temporary error handler" on Commands (and all code associated with it). It added more branching, made Commands bigger / more expensive to initialize (note that we construct it at high frequencies / treat it like a pointer type), made the code harder to follow, and introduced a bunch of additional functions. We instead rely on the new default error handler used in `queue_fallible` for most things. In the event that a custom handler is required, `handle_error_with` can be used. 6. EntityCommand now _only_ supports functions that take `EntityWorldMut` (and all existing entity commands have been ported). Removing the marker component from EntityCommand hinged on this change, but I strongly believe this is for the best anyway, as this sets the stage for more efficient batched entity commands. 7. I added `EntityWorldMut::resource` and the other variants for more ergonomic resource access on `EntityWorldMut` (removes the need for entity.world_scope, which also incurs entity-lookup overhead). ## Open Questions 1. I believe we could merge `queue` and `queue_fallible` into a single `queue` which accepts both fallible and infallible commands (via the introduction of a `QueueCommand` trait). Is this desirable? |
||
![]() |
0403948aa2
|
Remove Implicit std Prelude from no_std Crates (#17086)
# Background In `no_std` compatible crates, there is often an `std` feature which will allow access to the standard library. Currently, with the `std` feature _enabled_, the [`std::prelude`](https://doc.rust-lang.org/std/prelude/index.html) is implicitly imported in all modules. With the feature _disabled_, instead the [`core::prelude`](https://doc.rust-lang.org/core/prelude/index.html) is implicitly imported. This creates a subtle and pervasive issue where `alloc` items _may_ be implicitly included (if `std` is enabled), or must be explicitly included (if `std` is not enabled). # Objective - Make the implicit imports for `no_std` crates consistent regardless of what features are/not enabled. ## Solution - Replace the `cfg_attr` "double negative" `no_std` attribute with conditional compilation to _include_ `std` as an external crate. ```rust // Before #![cfg_attr(not(feature = "std"), no_std)] // After #![no_std] #[cfg(feature = "std")] extern crate std; ``` - Fix imports that are currently broken but are only now visible with the above fix. ## Testing - CI ## Notes I had previously used the "double negative" version of `no_std` based on general consensus that it was "cleaner" within the Rust embedded community. However, this implicit prelude issue likely was considered when forming this consensus. I believe the reason why is the items most affected by this issue are provided by the `alloc` crate, which is rarely used within embedded but extensively used within Bevy. |
||
![]() |
0f2b2de333
|
Move some structs that impl Command to methods on World and EntityWorldMut (#16999)
## Objective Commands were previously limited to structs that implemented `Command`. Now there are blanket implementations for closures, which (in my opinion) are generally preferable. Internal commands within `commands/mod.rs` have been switched from structs to closures, but there are a number of internal commands in other areas of the engine that still use structs. I'd like to tidy these up by moving their implementations to methods on `World`/`EntityWorldMut` and changing `Commands` to use those methods through closures. This PR handles the following: - `TriggerEvent` and `EmitDynamicTrigger` double as commands and helper structs, and can just be moved to `World` methods. - Four structs that enabled insertion/removal of components via reflection. This functionality shouldn't be exclusive to commands, and can be added to `EntityWorldMut`. - Five structs that mostly just wrapped `World` methods, and can be replaced with closures that do the same thing. ## Solution - __Observer Triggers__ (`observer/trigger_event.rs` and `observer/mod.rs`) - Moved the internals of `TriggerEvent` to the `World` methods that used it. - Replaced `EmitDynamicTrigger` with two `World` methods: - `trigger_targets_dynamic` - `trigger_targets_dynamic_ref` - `TriggerTargets` was now the only thing in `observer/trigger_event.rs`, so it's been moved to `observer/mod.rs` and `trigger_event.rs` was deleted. - __Reflection Insert/Remove__ (`reflect/entity_commands.rs`) - Replaced the following `Command` impls with equivalent methods on `EntityWorldMut`: - `InsertReflect` -> `insert_reflect` - `InsertReflectWithRegistry` -> `insert_reflect_with_registry` - `RemoveReflect` -> `remove_reflect` - `RemoveReflectWithRegistry` -> `remove_reflect_with_registry` - __System Registration__ (`system/system_registry.rs`) - The following `Command` impls just wrapped a `World` method and have been replaced with closures: - `RunSystemWith` - `UnregisterSystem` - `RunSystemCachedWith` - `UnregisterSystemCached` - `RegisterSystem` called a helper function that basically worked as a constructor for `RegisteredSystem` and made sure it came with a marker component. That helper function has been replaced with `RegisteredSystem::new` and a `#[require]`. ## Possible Addition The extension trait that adds the reflection commands, `ReflectCommandExt`, isn't strictly necessary; we could just `impl EntityCommands`. We could even move them to the same files as the main impls and put it behind a `#[cfg]`. The PR that added it [had a similar conversation](https://github.com/bevyengine/bevy/pull/8895#discussion_r1234713671) and decided to stick with the trait, but we could revisit it here if so desired. |
||
![]() |
1f2d0e6308
|
Add no_std support to bevy_ecs (#16758)
# Objective - Contributes to #15460 ## Solution - Added the following features: - `std` (default) - `async_executor` (default) - `edge_executor` - `critical-section` - `portable-atomic` - Gated `tracing` in `bevy_utils` to allow compilation on certain platforms - Switched from `tracing` to `log` for simple message logging within `bevy_ecs`. Note that `tracing` supports capturing from `log` so this should be an uncontroversial change. - Fixed imports and added feature gates as required - Made `bevy_tasks` optional within `bevy_ecs`. Turns out it's only needed for parallel operations which are already gated behind `multi_threaded` anyway. ## Testing - Added to `compile-check-no-std` CI command - `cargo check -p bevy_ecs --no-default-features --features edge_executor,critical-section,portable-atomic --target thumbv6m-none-eabi` - `cargo check -p bevy_ecs --no-default-features --features edge_executor,critical-section` - `cargo check -p bevy_ecs --no-default-features` ## Draft Release Notes Bevy's core ECS now supports `no_std` platforms. In prior versions of Bevy, it was not possible to work with embedded or niche platforms due to our reliance on the standard library, `std`. This has blocked a number of novel use-cases for Bevy, such as an embedded database for IoT devices, or for creating games on retro consoles. With this release, `bevy_ecs` no longer requires `std`. To use Bevy on a `no_std` platform, you must disable default features and enable the new `edge_executor` and `critical-section` features. You may also need to enable `portable-atomic` and `critical-section` if your platform does not natively support all atomic types and operations used by Bevy. ```toml [dependencies] bevy_ecs = { version = "0.16", default-features = false, features = [ # Required for platforms with incomplete atomics (e.g., Raspberry Pi Pico) "portable-atomic", "critical-section", # Optional "bevy_reflect", "serialize", "bevy_debug_stepping", "edge_executor" ] } ``` Currently, this has been tested on bare-metal x86 and the Raspberry Pi Pico. If you have trouble using `bevy_ecs` on a particular platform, please reach out either through a GitHub issue or in the `no_std` working group on the Bevy Discord server. Keep an eye out for future `no_std` updates as we continue to improve the parity between `std` and `no_std`. We look forward to seeing what kinds of applications are now possible with Bevy! ## Notes - Creating PR in draft to ensure CI is passing before requesting reviews. - This implementation has no support for multithreading in `no_std`, especially due to `NonSend` being unsound if allowed in multithreading. The reason is we cannot check the `ThreadId` in `no_std`, so we have no mechanism to at-runtime determine if access is sound. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Vic <59878206+Victoronz@users.noreply.github.com> |
||
![]() |
21195a75e6
|
track_change_detection: Also track spawns/despawns (#16047)
# Objective Expand `track_change_detection` feature to also track entity spawns and despawns. Use this to create better error messages. # Solution Adds `Entities::entity_get_spawned_or_despawned_by` as well as `{all entity reference types}::spawned_by`. This also removes the deprecated `get_many_entities_mut` & co (and therefore can't land in 0.15) because we don't yet have no Polonius. ## Testing Added a test that checks that the locations get updated and these updates are ordered correctly vs hooks & observers. --- ## Showcase Access location: ```rust let mut world = World::new(); let entity = world.spawn_empty().id(); println!("spawned by: {}", world.entity(entity).spawned_by()); ``` ``` spawned by: src/main.rs:5:24 ``` Error message (with `track_change_detection`): ```rust world.despawn(entity); world.entity(entity); ``` ``` thread 'main' panicked at src/main.rs:11:11: Entity 0v1#4294967296 was despawned by src/main.rs:10:11 ``` and without: ``` thread 'main' panicked at src/main.rs:11:11: Entity 0v1#4294967296 does not exist (enable `track_change_detection` feature for more details) ``` Similar error messages now also exists for `Query::get`, `World::entity_mut`, `EntityCommands` creation and everything that causes `B0003`, e.g. ``` error[B0003]: Could not insert a bundle (of type `MaterialMeshBundle<StandardMaterial>`) for entity Entity { index: 7, generation: 1 }, which was despawned by src/main.rs:10:11. See: https://bevyengine.org/learn/errors/#b0003 ``` --------- Co-authored-by: kurk070ff <108901106+kurk070ff@users.noreply.github.com> Co-authored-by: Freya Pines <freya@MacBookAir.lan> Co-authored-by: Freya Pines <freya@Freyas-MacBook-Air.local> Co-authored-by: Matty Weatherley <weatherleymatthew@gmail.com> |
||
![]() |
a35811d088
|
Add Immutable Component Support (#16372)
# Objective - Fixes #16208 ## Solution - Added an associated type to `Component`, `Mutability`, which flags whether a component is mutable, or immutable. If `Mutability= Mutable`, the component is mutable. If `Mutability= Immutable`, the component is immutable. - Updated `derive_component` to default to mutable unless an `#[component(immutable)]` attribute is added. - Updated `ReflectComponent` to check if a component is mutable and, if not, panic when attempting to mutate. ## Testing - CI - `immutable_components` example. --- ## Showcase Users can now mark a component as `#[component(immutable)]` to prevent safe mutation of a component while it is attached to an entity: ```rust #[derive(Component)] #[component(immutable)] struct Foo { // ... } ``` This prevents creating an exclusive reference to the component while it is attached to an entity. This is particularly powerful when combined with component hooks, as you can now fully track a component's value, ensuring whatever invariants you desire are upheld. Before this would be done my making a component private, and manually creating a `QueryData` implementation which only permitted read access. <details> <summary>Using immutable components as an index</summary> ```rust /// This is an example of a component like [`Name`](bevy::prelude::Name), but immutable. #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Component)] #[component( immutable, on_insert = on_insert_name, on_replace = on_replace_name, )] pub struct Name(pub &'static str); /// This index allows for O(1) lookups of an [`Entity`] by its [`Name`]. #[derive(Resource, Default)] struct NameIndex { name_to_entity: HashMap<Name, Entity>, } impl NameIndex { fn get_entity(&self, name: &'static str) -> Option<Entity> { self.name_to_entity.get(&Name(name)).copied() } } fn on_insert_name(mut world: DeferredWorld<'_>, entity: Entity, _component: ComponentId) { let Some(&name) = world.entity(entity).get::<Name>() else { unreachable!() }; let Some(mut index) = world.get_resource_mut::<NameIndex>() else { return; }; index.name_to_entity.insert(name, entity); } fn on_replace_name(mut world: DeferredWorld<'_>, entity: Entity, _component: ComponentId) { let Some(&name) = world.entity(entity).get::<Name>() else { unreachable!() }; let Some(mut index) = world.get_resource_mut::<NameIndex>() else { return; }; index.name_to_entity.remove(&name); } // Setup our name index world.init_resource::<NameIndex>(); // Spawn some entities! let alyssa = world.spawn(Name("Alyssa")).id(); let javier = world.spawn(Name("Javier")).id(); // Check our index let index = world.resource::<NameIndex>(); assert_eq!(index.get_entity("Alyssa"), Some(alyssa)); assert_eq!(index.get_entity("Javier"), Some(javier)); // Changing the name of an entity is also fully capture by our index world.entity_mut(javier).insert(Name("Steven")); // Javier changed their name to Steven let steven = javier; // Check our index let index = world.resource::<NameIndex>(); assert_eq!(index.get_entity("Javier"), None); assert_eq!(index.get_entity("Steven"), Some(steven)); ``` </details> Additionally, users can use `Component<Mutability = ...>` in trait bounds to enforce that a component _is_ mutable or _is_ immutable. When using `Component` as a trait bound without specifying `Mutability`, any component is applicable. However, methods which only work on mutable or immutable components are unavailable, since the compiler must be pessimistic about the type. ## Migration Guide - When implementing `Component` manually, you must now provide a type for `Mutability`. The type `Mutable` provides equivalent behaviour to earlier versions of `Component`: ```rust impl Component for Foo { type Mutability = Mutable; // ... } ``` - When working with generic components, you may need to specify that your generic parameter implements `Component<Mutability = Mutable>` rather than `Component` if you require mutable access to said component. - The entity entry API has had to have some changes made to minimise friction when working with immutable components. Methods which previously returned a `Mut<T>` will now typically return an `OccupiedEntry<T>` instead, requiring you to add an `into_mut()` to get the `Mut<T>` item again. ## Draft Release Notes Components can now be made immutable while stored within the ECS. Components are the fundamental unit of data within an ECS, and Bevy provides a number of ways to work with them that align with Rust's rules around ownership and borrowing. One part of this is hooks, which allow for defining custom behavior at key points in a component's lifecycle, such as addition and removal. However, there is currently no way to respond to _mutation_ of a component using hooks. The reasons for this are quite technical, but to summarize, their addition poses a significant challenge to Bevy's core promises around performance. Without mutation hooks, it's relatively trivial to modify a component in such a way that breaks invariants it intends to uphold. For example, you can use `core::mem::swap` to swap the components of two entities, bypassing the insertion and removal hooks. This means the only way to react to this modification is via change detection in a system, which then begs the question of what happens _between_ that alteration and the next run of that system? Alternatively, you could make your component private to prevent mutation, but now you need to provide commands and a custom `QueryData` implementation to allow users to interact with your component at all. Immutable components solve this problem by preventing the creation of an exclusive reference to the component entirely. Without an exclusive reference, the only way to modify an immutable component is via removal or replacement, which is fully captured by component hooks. To make a component immutable, simply add `#[component(immutable)]`: ```rust #[derive(Component)] #[component(immutable)] struct Foo { // ... } ``` When implementing `Component` manually, there is an associated type `Mutability` which controls this behavior: ```rust impl Component for Foo { type Mutability = Mutable; // ... } ``` Note that this means when working with generic components, you may need to specify that a component is mutable to gain access to certain methods: ```rust // Before fn bar<C: Component>() { // ... } // After fn bar<C: Component<Mutability = Mutable>>() { // ... } ``` With this new tool, creating index components, or caching data on an entity should be more user friendly, allowing libraries to provide APIs relying on components and hooks to uphold their invariants. ## Notes - ~~I've done my best to implement this feature, but I'm not happy with how reflection has turned out. If any reflection SMEs know a way to improve this situation I'd greatly appreciate it.~~ There is an outstanding issue around the fallibility of mutable methods on `ReflectComponent`, but the DX is largely unchanged from `main` now. - I've attempted to prevent all safe mutable access to a component that does not implement `Component<Mutability = Mutable>`, but there may still be some methods I have missed. Please indicate so and I will address them, as they are bugs. - Unsafe is an escape hatch I am _not_ attempting to prevent. Whatever you do with unsafe is between you and your compiler. - I am marking this PR as ready, but I suspect it will undergo fairly major revisions based on SME feedback. - I've marked this PR as _Uncontroversial_ based on the feature, not the implementation. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com> Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Co-authored-by: Nuutti Kotivuori <naked@iki.fi> |
||
![]() |
f005a96dd4
|
ReflectBundle::remove improvement (#16139)
# Objective Fixes #15676 ## Solution `remove` returns the removed item Add `take` ## Testing None yet ## Migration Guide If you don't need the returned value from `remove`, discard it. |
||
![]() |
da5d2fccf5
|
Fix some duplicate words in docs/comments (#15980)
# Objective Stumbled upon one of these, and set off in search of more, armed with my trusty `\b(\w+)\s+\1\b`. ## Solution Remove ~one~ one of them. |
||
![]() |
584d14808a
|
Allow World::entity family of functions to take multiple entities and get multiple references back (#15614)
# Objective Following the pattern established in #15593, we can reduce the API surface of `World` by providing a single function to grab both a singular entity reference, or multiple entity references. ## Solution The following functions can now also take multiple entity IDs and will return multiple entity references back: - `World::entity` - `World::get_entity` - `World::entity_mut` - `World::get_entity_mut` - `DeferredWorld::entity_mut` - `DeferredWorld::get_entity_mut` If you pass in X, you receive Y: - give a single `Entity`, receive a single `EntityRef`/`EntityWorldMut` (matches current behavior) - give a `[Entity; N]`/`&[Entity; N]` (array), receive an equally-sized `[EntityRef; N]`/`[EntityMut; N]` - give a `&[Entity]` (slice), receive a `Vec<EntityRef>`/`Vec<EntityMut>` - give a `&EntityHashSet`, receive a `EntityHashMap<EntityRef>`/`EntityHashMap<EntityMut>` Note that `EntityWorldMut` is only returned in the single-entity case, because having multiple at the same time would lead to UB. Also, `DeferredWorld` receives an `EntityMut` in the single-entity case because it does not allow structural access. ## Testing - Added doc-tests on `World::entity`, `World::entity_mut`, and `DeferredWorld::entity_mut` - Added tests for aliased mutability and entity existence --- ## Showcase <details> <summary>Click to view showcase</summary> The APIs for fetching `EntityRef`s and `EntityMut`s from the `World` have been unified. ```rust // This code will be referred to by subsequent code blocks. let world = World::new(); let e1 = world.spawn_empty().id(); let e2 = world.spawn_empty().id(); let e3 = world.spawn_empty().id(); ``` Querying for a single entity remains mostly the same: ```rust // 0.14 let eref: EntityRef = world.entity(e1); let emut: EntityWorldMut = world.entity_mut(e1); let eref: Option<EntityRef> = world.get_entity(e1); let emut: Option<EntityWorldMut> = world.get_entity_mut(e1); // 0.15 let eref: EntityRef = world.entity(e1); let emut: EntityWorldMut = world.entity_mut(e1); let eref: Result<EntityRef, Entity> = world.get_entity(e1); let emut: Result<EntityWorldMut, Entity> = world.get_entity_mut(e1); ``` Querying for multiple entities with an array has changed: ```rust // 0.14 let erefs: [EntityRef; 2] = world.many_entities([e1, e2]); let emuts: [EntityMut; 2] = world.many_entities_mut([e1, e2]); let erefs: Result<[EntityRef; 2], Entity> = world.get_many_entities([e1, e2]); let emuts: Result<[EntityMut; 2], QueryEntityError> = world.get_many_entities_mut([e1, e2]); // 0.15 let erefs: [EntityRef; 2] = world.entity([e1, e2]); let emuts: [EntityMut; 2] = world.entity_mut([e1, e2]); let erefs: Result<[EntityRef; 2], Entity> = world.get_entity([e1, e2]); let emuts: Result<[EntityMut; 2], EntityFetchError> = world.get_entity_mut([e1, e2]); ``` Querying for multiple entities with a slice has changed: ```rust let ids = vec![e1, e2, e3]); // 0.14 let erefs: Result<Vec<EntityRef>, Entity> = world.get_many_entities_dynamic(&ids[..]); let emuts: Result<Vec<EntityMut>, QueryEntityError> = world.get_many_entities_dynamic_mut(&ids[..]); // 0.15 let erefs: Result<Vec<EntityRef>, Entity> = world.get_entity(&ids[..]); let emuts: Result<Vec<EntityMut>, EntityFetchError> = world.get_entity_mut(&ids[..]); let erefs: Vec<EntityRef> = world.entity(&ids[..]); // Newly possible! let emuts: Vec<EntityMut> = world.entity_mut(&ids[..]); // Newly possible! ``` Querying for multiple entities with an `EntityHashSet` has changed: ```rust let set = EntityHashSet::from_iter([e1, e2, e3]); // 0.14 let emuts: Result<Vec<EntityMut>, QueryEntityError> = world.get_many_entities_from_set_mut(&set); // 0.15 let emuts: Result<EntityHashMap<EntityMut>, EntityFetchError> = world.get_entity_mut(&set); let erefs: Result<EntityHashMap<EntityRef>, EntityFetchError> = world.get_entity(&set); // Newly possible! let emuts: EntityHashMap<EntityMut> = world.entity_mut(&set); // Newly possible! let erefs: EntityHashMap<EntityRef> = world.entity(&set); // Newly possible! ``` </details> ## Migration Guide - `World::get_entity` now returns `Result<_, Entity>` instead of `Option<_>`. - Use `world.get_entity(..).ok()` to return to the previous behavior. - `World::get_entity_mut` and `DeferredWorld::get_entity_mut` now return `Result<_, EntityFetchError>` instead of `Option<_>`. - Use `world.get_entity_mut(..).ok()` to return to the previous behavior. - Type inference for `World::entity`, `World::entity_mut`, `World::get_entity`, `World::get_entity_mut`, `DeferredWorld::entity_mut`, and `DeferredWorld::get_entity_mut` has changed, and might now require the input argument's type to be explicitly written when inside closures. - The following functions have been deprecated, and should be replaced as such: - `World::many_entities` -> `World::entity::<[Entity; N]>` - `World::many_entities_mut` -> `World::entity_mut::<[Entity; N]>` - `World::get_many_entities` -> `World::get_entity::<[Entity; N]>` - `World::get_many_entities_dynamic` -> `World::get_entity::<&[Entity]>` - `World::get_many_entities_mut` -> `World::get_entity_mut::<[Entity; N]>` - The equivalent return type has changed from `Result<_, QueryEntityError>` to `Result<_, EntityFetchError>` - `World::get_many_entities_dynamic_mut` -> `World::get_entity_mut::<&[Entity]>1 - The equivalent return type has changed from `Result<_, QueryEntityError>` to `Result<_, EntityFetchError>` - `World::get_many_entities_from_set_mut` -> `World::get_entity_mut::<&EntityHashSet>` - The equivalent return type has changed from `Result<Vec<EntityMut>, QueryEntityError>` to `Result<EntityHashMap<EntityMut>, EntityFetchError>`. If necessary, you can still convert the `EntityHashMap` into a `Vec`. |
||
![]() |
40e88dceff
|
Change ReflectMapEntities to operate on components before insertion (#15422)
Previous PR https://github.com/bevyengine/bevy/pull/14549 was closed in error and couldn't be reopened since I had updated the branch 😿 # Objective Fixes #14465 ## Solution `ReflectMapEntities` now works similarly to `MapEntities` in that it works on the reflected value itself rather than the component in the world after insertion. This makes it so that observers see the remapped entities on insertion rather than the entity IDs from the scene. `ReflectMapEntities` now works for both components and resources, so we only need the one. ## Testing * New unit test for `Observer`s + `DynamicScene`s * New unit test for `Observer`s + `Scene`s * Open to suggestions for other tests! --- ## Migration Guide - Consumers of `ReflectMapEntities` will need to call `map_entities` on values prior to inserting them into the world. - Implementors of `MapEntities` will need to remove the `mappings` method, which is no longer needed for `ReflectMapEntities` and has been removed from the trait. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com> |
||
![]() |
f97eba2082
|
Add VisitEntities for generic and reflectable Entity iteration (#15425)
# Objective - Provide a generic and _reflectable_ way to iterate over contained entities ## Solution Adds two new traits: * `VisitEntities`: Reflectable iteration, accepts a closure rather than producing an iterator. Implemented by default for `IntoIterator` implementing types. A proc macro is also provided. * A `Mut` variant of the above. Its derive macro uses the same field attribute to avoid repetition. ## Testing Added a test for `VisitEntities` that also transitively tests its derive macro as well as the default `MapEntities` impl. |
||
![]() |
c32e0b9ec2
|
Allow registering of resources via ReflectResource / ReflectComponent (#15496)
# Objective - Resolves #15453 ## Solution - Added new `World::resource_id` and `World::register_resource` methods to support this feature - Added new `ReflectResource::register_resource` method, and new pointer to this new function - Added new `ReflectComponent::register_component` ## Testing - Tested this locally, but couldn't test the entire crate locally, just this new feature, expect that CI will do the rest of the work. --- ## Showcase ```rs #[derive(Component, Reflect)] #[reflect(Component)] struct MyComp; let mut world = World::new(); let mut registry = TypeRegistration::of::<MyComp>(); registry.insert::<ReflectComponent>(FromType::<MyComp>::from_type()); let data = registry.data::<ReflectComponent>().unwrap(); // Its now possible to register the Component in the world this way let component_id = data.register_component(&mut world); // They will be the same assert_eq!(component_id, world.component_id::<MyComp>().unwrap()); ``` ```rs #[derive(Resource, Reflect)] #[reflect(Resource)] struct MyResource; let mut world = World::new(); let mut registry = TypeRegistration::of::<MyResource>(); registry.insert::<ReflectResource>(FromType::<MyResource>::from_type()); let data = registry.data::<ReflectResource>().unwrap(); // Same with resources let component_id = data.register_resource(&mut world); // They match assert_eq!(component_id, world.resource_id::<MyResource>().unwrap()); ``` |
||
![]() |
d70595b667
|
Add core and alloc over std Lints (#15281)
# Objective - Fixes #6370 - Closes #6581 ## Solution - Added the following lints to the workspace: - `std_instead_of_core` - `std_instead_of_alloc` - `alloc_instead_of_core` - Used `cargo +nightly fmt` with [item level use formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Item%5C%3A) to split all `use` statements into single items. - Used `cargo clippy --workspace --all-targets --all-features --fix --allow-dirty` to _attempt_ to resolve the new linting issues, and intervened where the lint was unable to resolve the issue automatically (usually due to needing an `extern crate alloc;` statement in a crate root). - Manually removed certain uses of `std` where negative feature gating prevented `--all-features` from finding the offending uses. - Used `cargo +nightly fmt` with [crate level use formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Crate%5C%3A) to re-merge all `use` statements matching Bevy's previous styling. - Manually fixed cases where the `fmt` tool could not re-merge `use` statements due to conditional compilation attributes. ## Testing - Ran CI locally ## Migration Guide The MSRV is now 1.81. Please update to this version or higher. ## Notes - This is a _massive_ change to try and push through, which is why I've outlined the semi-automatic steps I used to create this PR, in case this fails and someone else tries again in the future. - Making this change has no impact on user code, but does mean Bevy contributors will be warned to use `core` and `alloc` instead of `std` where possible. - This lint is a critical first step towards investigating `no_std` options for Bevy. --------- Co-authored-by: François Mockers <francois.mockers@vleue.com> |
||
![]() |
efda7f3f9c
|
Simpler lint fixes: makes ci lints work but disables a lint for now (#15376)
Takes the first two commits from #15375 and adds suggestions from this comment: https://github.com/bevyengine/bevy/pull/15375#issuecomment-2366968300 See #15375 for more reasoning/motivation. ## Rebasing (rerunning) ```rust git switch simpler-lint-fixes git reset --hard main cargo fmt --all -- --unstable-features --config normalize_comments=true,imports_granularity=Crate cargo fmt --all git add --update git commit --message "rustfmt" cargo clippy --workspace --all-targets --all-features --fix cargo fmt --all -- --unstable-features --config normalize_comments=true,imports_granularity=Crate cargo fmt --all git add --update git commit --message "clippy" git cherry-pick e6c0b94f6795222310fb812fa5c4512661fc7887 ``` |
||
![]() |
b45d83ebda
|
Rename Add to Queue for methods with deferred semantics (#15234)
# Objective - Fixes #15106 ## Solution - Trivial refactor to rename the method. The duplicate method `push` was removed as well. This will simpify the API and make the semantics more clear. `Add` implies that the action happens immediately, whereas in reality, the command is queued to be run eventually. - `ChildBuilder::add_command` has similarly been renamed to `queue_command`. ## Testing Unit tests should suffice for this simple refactor. --- ## Migration Guide - `Commands::add` and `Commands::push` have been replaced with `Commnads::queue`. - `ChildBuilder::add_command` has been renamed to `ChildBuilder::queue_command`. |
||
![]() |
8bfe635c3e
|
Finish enhancing ReflectCommandExt to work with Bundles (#15152)
# Objective - Finish resolving https://github.com/bevyengine/bevy/issues/15125 - Inserting bundles was implemented in https://github.com/bevyengine/bevy/pull/15128 but removing bundles still needed to be implemented. ## Solution - Modified `bevy_ecs::reflect::entity_commands::remove_reflect` to handle both components and bundles - Modified documentation of `ReflectCommandExt` methods to reflect that one can now use bundles with these commands. ## Testing - Three tests were added to match the ones for inserting components. |
||
![]() |
0cf276f239
|
Enhance ReflectCommandExt (#15128)
# Objective - Enhance #15125 ## Solution - Modified `ReflectCommandExt::insert_reflect` to accept and handle both components and bundles. --------- Co-authored-by: Gonçalo Rica Pais da Silva <bluefinger@gmail.com> Co-authored-by: Lixou <82600264+DasLixou@users.noreply.github.com> Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com> |
||
![]() |
0ea46663b0
|
Use map_unchanged in reflection instead of creating a Mut manually. (#14692)
# Objective The code to create `ReflectComponent` and `ReflectResource` instances manually constructs a `Mut<dyn Reflect>` by copying everything but `value`. That can be done more concisely and better respecting encapsulation by calling the `map_unchanged()` method. ## Solution Use `map_unchanged` instead of creating a `Mut` manually. --------- Co-authored-by: radiish <cb.setho@gmail.com> |
||
![]() |
6ab8767d3b
|
reflect: implement the unique reflect rfc (#7207)
# Objective
- Implements the [Unique Reflect
RFC](https://github.com/nicopap/rfcs/blob/bevy-reflect-api/rfcs/56-better-reflect.md).
## Solution
- Implements the RFC.
- This implementation differs in some ways from the RFC:
- In the RFC, it was suggested `Reflect: Any` but `PartialReflect:
?Any`. During initial implementation I tried this, but we assume the
`PartialReflect: 'static` in a lot of places and the changes required
crept out of the scope of this PR.
- `PartialReflect::try_into_reflect` originally returned `Option<Box<dyn
Reflect>>` but i changed this to `Result<Box<dyn Reflect>, Box<dyn
PartialReflect>>` since the method takes by value and otherwise there
would be no way to recover the type. `as_full` and `as_full_mut` both
still return `Option<&(mut) dyn Reflect>`.
---
## Changelog
- Added `PartialReflect`.
- `Reflect` is now a subtrait of `PartialReflect`.
- Moved most methods on `Reflect` to the new `PartialReflect`.
- Added `PartialReflect::{as_partial_reflect, as_partial_reflect_mut,
into_partial_reflect}`.
- Added `PartialReflect::{try_as_reflect, try_as_reflect_mut,
try_into_reflect}`.
- Added `<dyn PartialReflect>::{try_downcast_ref, try_downcast_mut,
try_downcast, try_take}` supplementing the methods on `dyn Reflect`.
## Migration Guide
- Most instances of `dyn Reflect` should be changed to `dyn
PartialReflect` which is less restrictive, however trait bounds should
generally stay as `T: Reflect`.
- The new `PartialReflect::{as_partial_reflect, as_partial_reflect_mut,
into_partial_reflect, try_as_reflect, try_as_reflect_mut,
try_into_reflect}` methods as well as `Reflect::{as_reflect,
as_reflect_mut, into_reflect}` will need to be implemented for manual
implementors of `Reflect`.
## Future Work
- This PR is designed to be followed up by another "Unique Reflect Phase
2" that addresses the following points:
- Investigate making serialization revolve around `Reflect` instead of
`PartialReflect`.
- [Remove the `try_*` methods on `dyn PartialReflect` since they are
stop
gaps](https://github.com/bevyengine/bevy/pull/7207#discussion_r1083476050).
- Investigate usages like `ReflectComponent`. In the places they
currently use `PartialReflect`, should they be changed to use `Reflect`?
- Merging this opens the door to lots of reflection features we haven't
been able to implement.
- We could re-add [the `Reflectable`
trait](
|
||
![]() |
8c2e70b744
|
Adding Reflect data types for States and FreelyMutableState . (#14643)
# Objective - While developing a debug tool I saw the gap where it was not possible to get all existing states from a World using reflection. - This PR allows to iterate over all `States` types that exist in a world, and modify them in case they implement `FreelyMutableState`. - Two new methods are available on `App` and `SubApp` as helper to register the data types: - `register_state_reflect` and `register_mutable_state_reflect` ## Solution - Two new data types are added: - `ReflectState`: Allows to extract the current value of a state from the World. - `ReflectFreelyMutableState`: Allows to set the next state in a world, similar to call `NextState::set`. - There is no distinction between `States`, `SubStates` and `ComputedStates`: - `States` can register both `ReflectState` and `ReflectFreelyMutableState`. - `SubStates` can register both `ReflectState` and `ReflectFreelyMutableState`. - `ComputedStates` can register only `ReflectState` . ## Testing - Added tests inside the `bevy_state` crate. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Co-authored-by: Jan Hohenheim <jan@hohenheim.ch> |
||
![]() |
df61117850
|
bevy_reflect: Function registry (#14098)
# Objective #13152 added support for reflecting functions. Now, we need a way to register those functions such that they may be accessed anywhere within the ECS. ## Solution Added a `FunctionRegistry` type similar to `TypeRegistry`. This allows a function to be registered and retrieved by name. ```rust fn foo() -> i32 { 123 } let mut registry = FunctionRegistry::default(); registry.register("my_function", foo); let function = registry.get_mut("my_function").unwrap(); let value = function.call(ArgList::new()).unwrap().unwrap_owned(); assert_eq!(value.downcast_ref::<i32>(), Some(&123)); ``` Additionally, I added an `AppFunctionRegistry` resource which wraps a `FunctionRegistryArc`. Functions can be registered into this resource using `App::register_function` or by getting a mutable reference to the resource itself. ### Limitations #### `Send + Sync` In order to get this registry to work across threads, it needs to be `Send + Sync`. This means that `DynamicFunction` needs to be `Send + Sync`, which means that its internal function also needs to be `Send + Sync`. In most cases, this won't be an issue because standard Rust functions (the type most likely to be registered) are always `Send + Sync`. Additionally, closures tend to be `Send + Sync` as well, granted they don't capture any `!Send` or `!Sync` variables. This PR adds this `Send + Sync` requirement, but as mentioned above, it hopefully shouldn't be too big of an issue. #### Closures Unfortunately, closures can't be registered yet. This will likely be explored and added in a followup PR. ### Future Work Besides addressing the limitations listed above, another thing we could look into is improving the lookup of registered functions. One aspect is in the performance of hashing strings. The other is in the developer experience of having to call `std::any::type_name_of_val` to get the name of their function (assuming they didn't give it a custom name). ## Testing You can run the tests locally with: ``` cargo test --package bevy_reflect ``` --- ## Changelog - Added `FunctionRegistry` - Added `AppFunctionRegistry` (a `Resource` available from `bevy_ecs`) - Added `FunctionRegistryArc` - Added `FunctionRegistrationError` - Added `reflect_functions` feature to `bevy_ecs` and `bevy_app` - `FunctionInfo` is no longer `Default` - `DynamicFunction` now requires its wrapped function be `Send + Sync` ## Internal Migration Guide > [!important] > Function reflection was introduced as part of the 0.15 dev cycle. This migration guide was written for developers relying on `main` during this cycle, and is not a breaking change coming from 0.14. `DynamicFunction` (both those created manually and those created with `IntoFunction`), now require `Send + Sync`. All standard Rust functions should meet that requirement. Closures, on the other hand, may not if they capture any `!Send` or `!Sync` variables from its environment. |
||
![]() |
9575b20d31
|
Track source location in change detection (#14034)
# Objective - Make it possible to know *what* changed your component or resource. - Common need when debugging, when you want to know the last code location that mutated a value in the ECS. - This feature would be very useful for the editor alongside system stepping. ## Solution - Adds the caller location to column data. - Mutations now `track_caller` all the way up to the public API. - Commands that invoke these functions immediately call `Location::caller`, and pass this into the functions, instead of the functions themselves attempting to get the caller. This would not work for commands which are deferred, as the commands are executed by the scheduler, not the user's code. ## Testing - The `component_change_detection` example now shows where the component was mutated: ``` 2024-07-28T06:57:48.946022Z INFO component_change_detection: Entity { index: 1, generation: 1 }: New value: MyComponent(0.0) 2024-07-28T06:57:49.004371Z INFO component_change_detection: Entity { index: 1, generation: 1 }: New value: MyComponent(1.0) 2024-07-28T06:57:49.012738Z WARN component_change_detection: Change detected! -> value: Ref(MyComponent(1.0)) -> added: false -> changed: true -> changed by: examples/ecs/component_change_detection.rs:36:23 ``` - It's also possible to inspect change location from a debugger: <img width="608" alt="image" src="https://github.com/user-attachments/assets/c90ecc7a-0462-457a-80ae-42e7f5d346b4"> --- ## Changelog - Added source locations to ECS change detection behind the `track_change_detection` flag. ## Migration Guide - Added `changed_by` field to many internal ECS functions used with change detection when the `track_change_detection` feature flag is enabled. Use Location::caller() to provide the source of the function call. --------- Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com> Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> |
||
![]() |
2414311079
|
Fixed #14248 and other URL issues (#14276)
# Objective Fixes #14248 and other URL issues. ## Solution - Describe the solution used to achieve the objective above. Removed the random #s in the URL. Led users to the wrong page. For example, https://bevyengine.org/learn/errors/#b0003 takes users to https://bevyengine.org/learn/errors/introduction, which is not the right page. Removing the #s fixes it. ## Testing - Did you test these changes? If so, how? I pasted the URL into my address bar and it took me to the right place. - Are there any parts that need more testing? No |
||
![]() |
7570c9f3d2
|
Map entities from a resource when written to the world. (#13650)
# Objective - Fix #10958 by performing entity mapping on the entities inside of resources. ## Solution - Resources can reflect(MapEntitiesResource) and impl MapEntities to get access to the mapper during the world insert of the scene. ## Testing - A test resource_entity_map_maps_entities confirms the desired behavior. ## Changelog - Added reflect(MapEntitiesResource) for mapping entities on Resources in a DynamicScene. fixes 10958 |
||
![]() |
05288ffa32
|
Generalize component reflection to operate on FilteredEntityRef and FilteredEntityMut , not EntityRef and EntityMut . (#13549)
Currently, either an `EntityRef` or `EntityMut` is required in order to reflect a component on an entity. This can, however, be generalized to `FilteredEntityRef` and `FilteredEntityMut`, which are versions of `EntityRef` and `EntityMut` that restrict the components that can be accessed. This is useful because dynamic queries yield `FilteredEntityRef` and `FilteredEntityMut` rows when iterated over. This commit changes `ReflectComponent::contains()`, `ReflectComponent::reflect()`, and `ReflectComponent::reflect_mut()` to take an `Into<FilteredEntityRef>` (in the case of `contains()` and `reflect()`) and `Into<FilteredEntityMut>` (in the case of `reflect_mut()`). Fortunately, `EntityRef` and `EntityMut` already implement the corresponding trait, so nothing else has to be done to the public API. Note that in order to implement `ReflectComponent::reflect_mut()` properly, an additional method `FilteredEntityMut::into_mut()` was required, to match the one on `EntityMut`. I ran into this when attempting to implement `QUERY` in the Bevy Remote Protocol when trying to iterate over rows of dynamic queries and fetch the associated components without unsafe code. There were other potential ways to work around this problem, but they required either reimplementing the query logic myself instead of using regular Bevy queries or storing entity IDs and then issuing another query to fetch the associated `EntityRef`. Both of these seemed worse than just improving the `reflect()` function. ## Migration Guide * `ReflectComponent::contains`, `ReflectComponent::reflect`, and `ReflectComponent::reflect_mut` now take `FilteredEntityRef` (in the case of `contains()` and `reflect()`) and `FilteredEntityMut` (in the case of `reflect_mut()`) parameters. `FilteredEntityRef` and `FilteredEntityMut` have very similar APIs to `EntityRef` and `EntityMut` respectively, but optionally restrict the components that can be accessed. |
||
![]() |
d9b69731de
|
Make from_reflect_or_world also try ReflectDefault and improve some comments and panic messages (#12499)
# Objective - `from_reflect_or_world` is an internal utilty used in the implementations of `ReflectComponent` and `ReflectBundle` to create a `T` given a `&dyn Reflect` by trying to use `FromReflect`, and if that fails it falls back to `ReflectFromWorld` - reflecting `FromWorld` is not intuitive though: often it is implicitly implemented by deriving `Default` so people might not even be aware of it. - the panic messages mentioning `ReflectFromWorld` are not directly correlated to what the user would have to do (reflect `FromWorld`) ## Solution - Also check for `ReflectDefault` in addition to `ReflectFromWorld`. - Change the panic messages to mention the reflected trait rather than the `Reflect*` types. --- ## Changelog - `ReflectComponent` and `ReflectBundle` no longer require `T: FromReflect` but instead only `T: Reflect`. - `ReflectComponent` and `ReflectBundle` will also work with types that only reflected `Default` and not `FromWorld`. ## Migration Guide - `ReflectBundle::insert` now requires an additional `&TypeRegistry` parameter. |
||
![]() |
74f52076a3
|
Make some ReflectComponent /ReflectBundle methods work with EntityMut too (#12895)
# Objective - Make `ReflectComponent::apply`, `ReflectComponent::reflect_mut` and `ReflectBundle::apply` work with `EntityMut` too (currently they only work with the more restricting `EntityWorldMut`); - Note: support for the `Filtered*` variants has been left out since the conversion in that case is more expensive. Let me know if I should add support for them too. ## Solution - Make `ReflectComponent::apply`, `ReflectComponent::reflect_mut` and `ReflectBundle::apply` take an `impl Into<EntityMut<'a>>`; - Make the corresponding `*Fns` function pointers take a `EntityMut`. --- ## Changelog - `ReflectComponent::apply`, `ReflectComponent::reflect_mut` and `ReflectBundle::apply` now accept `EntityMut` as well ## Migration Guide - `ReflectComponentFns`'s `apply` and `reflect_mut` fields now take `EntityMut` instead of `&mut EntityWorldMut` - `ReflectBundleFns`'s `apply` field now takes `EntityMut` instead of `&mut EntityWorldMut` |
||
![]() |
13cbb9cf10
|
Move commands module into bevy::ecs::world (#12234)
# Objective Fixes https://github.com/bevyengine/bevy/issues/11628 ## Migration Guide `Command` and `CommandQueue` have migrated from `bevy_ecs::system` to `bevy_ecs::world`, so `use bevy_ecs::world::{Command, CommandQueue};` when necessary. |
||
![]() |
cbfeb9438a
|
include links in error messages (#12165)
# Objective Fixes #12118 ## Solution did a grip for any "B000" in the crates directory, and added the links note: ignored the test functions in this file https://github.com/bevyengine/bevy/blob/main/crates/bevy_ecs/src/system/mod.rs#L537 |
||
![]() |
309c3876bf
|
Replace FromWorld requirement on ReflectResource and reflect Resource for State<S> (#12136)
# Objective - In #9623 I forgot to change the `FromWorld` requirement for `ReflectResource`, fix that; - Fix #12129 ## Solution - Use the same approach as in #9623 to try using `FromReflect` and falling back to the `ReflectFromWorld` contained in the `TypeRegistry` provided - Just reflect `Resource` on `State<S>` since now that's possible without introducing new bounds. --- ## Changelog - `ReflectResource`'s `FromType<T>` implementation no longer requires `T: FromWorld`, but instead now requires `FromReflect`. - `ReflectResource::insert`, `ReflectResource::apply_or_insert` and `ReflectResource::copy` now take an extra `&TypeRegistry` parameter. ## Migration Guide - Users of `#[reflect(Resource)]` will need to also implement/derive `FromReflect` (should already be the default). - Users of `#[reflect(Resource)]` may now want to also add `FromWorld` to the list of reflected traits in case their `FromReflect` implementation may fail. - Users of `ReflectResource` will now need to pass a `&TypeRegistry` to its `insert`, `apply_or_insert` and `copy` methods. |
||
![]() |
9d67edc3a6
|
fix some typos (#12038)
# Objective Split - containing only the fixed typos - https://github.com/bevyengine/bevy/pull/12036#pullrequestreview-1894738751 # Migration Guide In `crates/bevy_mikktspace/src/generated.rs` ```rs // before pub struct SGroup { pub iVertexRepresentitive: i32, .. } // after pub struct SGroup { pub iVertexRepresentative: i32, .. } ``` In `crates/bevy_core_pipeline/src/core_2d/mod.rs` ```rs // before Node2D::ConstrastAdaptiveSharpening // after Node2D::ContrastAdaptiveSharpening ``` --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: James Liu <contact@jamessliu.com> Co-authored-by: François <mockersf@gmail.com> |
||
![]() |
33c7a2251e
|
bevy_ecs address trivial cases of unsafe_op_in_unsafe_fn (#11861)
# Objective - Part of #11590 - Fix `unsafe_op_in_unsafe_fn` for trivial cases in bevy_ecs ## Solution Fix `unsafe_op_in_unsafe_fn` in bevy_ecs for trivial cases, i.e., add an `unsafe` block when the safety comment already exists or add a comment like "The invariants are uphold by the caller". --------- Co-authored-by: James Liu <contact@jamessliu.com> |
||
![]() |
1c67e020f7
|
Move EntityHash related types into bevy_ecs (#11498)
# Objective Reduce the size of `bevy_utils` (https://github.com/bevyengine/bevy/issues/11478) ## Solution Move `EntityHash` related types into `bevy_ecs`. This also allows us access to `Entity`, which means we no longer need `EntityHashMap`'s first generic argument. --- ## Changelog - Moved `bevy::utils::{EntityHash, EntityHasher, EntityHashMap, EntityHashSet}` into `bevy::ecs::entity::hash` . - Removed `EntityHashMap`'s first generic argument. It is now hardcoded to always be `Entity`. ## Migration Guide - Uses of `bevy::utils::{EntityHash, EntityHasher, EntityHashMap, EntityHashSet}` now have to be imported from `bevy::ecs::entity::hash`. - Uses of `EntityHashMap` no longer have to specify the first generic parameter. It is now hardcoded to always be `Entity`. |
||
![]() |
9223201d54
|
Make the MapEntities trait generic over Mappers, and add a simpler EntityMapper (#11428)
# Objective My motivation are to resolve some of the issues I describe in this [PR](https://github.com/bevyengine/bevy/issues/11415): - not being able to easily mapping entities because the current EntityMapper requires `&mut World` access - not being able to create my own `EntityMapper` because some components (`Parent` or `Children`) do not provide any public way of modifying the inner entities This PR makes the `MapEntities` trait accept a generic type that implements `Mapper` to perform the mapping. This means we don't need to use `EntityMapper` to perform our mapping, we can use any type that implements `Mapper`. Basically this change is very similar to what `serde` does. Instead of specifying directly how to map entities for a given type, we have 2 distinct steps: - the user implements `MapEntities` to define how the type will be traversed and which `Entity`s will be mapped - the `Mapper` defines how the mapping is actually done This is similar to the distinction between `Serialize` (`MapEntities`) and `Serializer` (`Mapper`). This allows networking library to map entities without having to use the existing `EntityMapper` (which requires `&mut World` access and the use of `world_scope()`) ## Migration Guide - The existing `EntityMapper` (notably used to replicate `Scenes` across different `World`s) has been renamed to `SceneEntityMapper` - The `MapEntities` trait now works with a generic `EntityMapper` instead of the specific struct `EntityMapper`. Calls to `fn map_entities(&mut self, entity_mapper: &mut EntityMapper)` need to be updated to `fn map_entities<M: EntityMapper>(&mut self, entity_mapper: &mut M)` - The new trait `EntityMapper` has been added to the prelude --------- Co-authored-by: Charles Bournhonesque <cbournhonesque@snapchat.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com> |
||
![]() |
7d69d3195f
|
refactor: Simplify lifetimes for Commands and related types (#11445)
# Objective It would be convenient to be able to call functions with `Commands` as a parameter without having to move your own instance of `Commands`. Since this struct is composed entirely of references, we can easily get an owned instance of `Commands` by shortening the lifetime. ## Solution Add `Commands::reborrow`, `EntiyCommands::reborrow`, and `Deferred::reborrow`, which returns an owned version of themselves with a shorter lifetime. Remove unnecessary lifetimes from `EntityCommands`. The `'w` and `'s` lifetimes only have to be separate for `Commands` because it's used as a `SystemParam` -- this is not the case for `EntityCommands`. --- ## Changelog Added `Commands::reborrow`. This is useful if you have `&mut Commands` but need `Commands`. Also added `EntityCommands::reborrow` and `Deferred:reborrow` which serve the same purpose. ## Migration Guide The lifetimes for `EntityCommands` have been simplified. ```rust // Before (Bevy 0.12) struct MyStruct<'w, 's, 'a> { commands: EntityCommands<'w, 's, 'a>, } // After (Bevy 0.13) struct MyStruct<'a> { commands: EntityCommands<'a>, } ``` The method `EntityCommands::commands` now returns `Commands` rather than `&mut Commands`. ```rust // Before (Bevy 0.12) let commands = entity_commands.commands(); commands.spawn(...); // After (Bevy 0.13) let mut commands = entity_commands.commands(); commands.spawn(...); ``` |
||
![]() |
eff96e20a0
|
Add ReflectFromWorld and replace the FromWorld requirement on ReflectComponent and ReflectBundle with FromReflect (#9623)
# Objective - `FromType<T>` for `ReflectComponent` and `ReflectBundle` currently require `T: FromWorld` for two reasons: - they include a `from_world` method; - they create dummy `T`s using `FromWorld` and then `apply` a `&dyn Reflect` to it to simulate `FromReflect`. - However `FromWorld`/`Default` may be difficult/weird/impractical to implement, while `FromReflect` is easier and also more natural for the job. - See also https://discord.com/channels/691052431525675048/1146022009554337792 ## Solution - Split `from_world` from `ReflectComponent` and `ReflectBundle` into its own `ReflectFromWorld` struct. - Replace the requirement on `FromWorld` in `ReflectComponent` and `ReflectBundle` with `FromReflect` --- ## Changelog - `ReflectComponent` and `ReflectBundle` no longer offer a `from_world` method. - `ReflectComponent` and `ReflectBundle`'s `FromType<T>` implementation no longer requires `T: FromWorld`, but now requires `FromReflect`. - `ReflectComponent::insert`, `ReflectComponent::apply_or_insert` and `ReflectComponent::copy` now take an extra `&TypeRegistry` parameter. - There is now a new `ReflectFromWorld` struct. ## Migration Guide - Existing uses of `ReflectComponent::from_world` and `ReflectBundle::from_world` will have to be changed to `ReflectFromWorld::from_world`. - Users of `#[reflect(Component)]` and `#[reflect(Bundle)]` will need to also implement/derive `FromReflect`. - Users of `#[reflect(Component)]` and `#[reflect(Bundle)]` may now want to also add `FromWorld` to the list of reflected traits in case their `FromReflect` implementation may fail. - Users of `ReflectComponent` will now need to pass a `&TypeRegistry` to its `insert`, `apply_or_insert` and `copy` methods. |
||
![]() |
189ceaf0d3
|
Replace or document ignored doctests (#11040)
# Objective There are a lot of doctests that are `ignore`d for no documented reason. And that should be fixed. ## Solution I searched the bevy repo with the regex ` ```[a-z,]*ignore ` in order to find all `ignore`d doctests. For each one of the `ignore`d doctests, I did the following steps: 1. Attempt to remove the `ignored` attribute while still passing the test. I did this by adding hidden dummy structs and imports. 2. If step 1 doesn't work, attempt to replace the `ignored` attribute with the `no_run` attribute while still passing the test. 3. If step 2 doesn't work, keep the `ignored` attribute but add documentation for why the `ignored` attribute was added. --------- Co-authored-by: François <mockersf@gmail.com> |
||
![]() |
fd308571c4
|
Remove unnecessary path prefixes (#10749)
# Objective - Shorten paths by removing unnecessary prefixes ## Solution - Remove the prefixes from many paths which do not need them. Finding the paths was done automatically using built-in refactoring tools in Jetbrains RustRover. |
||
![]() |
879893c30a
|
fix insert_reflect panic caused by clone_value (#10627)
# Objective - `insert_reflect` relies on `reflect_type_path`, which doesn't gives the actual type path for object created by `clone_value`, leading to an unexpected panic. This is a workaround for it. - Fix #10590 ## Solution - Tries to get type path from `get_represented_type_info` if get failed from `reflect_type_path`. --- ## Defect remaining - `get_represented_type_info` implies a shortage on performance than using `TypeRegistry`. |
||
![]() |
11b1b3a24f
|
delete methods deprecated in 0.12 (#10693)
## Changelog - delete methods deprecated in 0.12 |
||
![]() |
04ceb46fe0
|
Use EntityHashMap for EntityMapper (#10415)
# Objective - There is a specialized hasher for entities: [`EntityHashMap`](https://docs.rs/bevy/latest/bevy/utils/type.EntityHashMap.html) - [`EntityMapper`] currently uses a normal `HashMap<Entity, Entity>` - Fixes #10391 ## Solution - Replace the normal `HashMap` with the more performant `EntityHashMap` ## Questions - This does change public API. Should a system be implemented to help migrate code? - Perhaps an `impl From<HashMap<K, V, S>> for EntityHashMap<K, V>` - I updated to docs for each function that I changed, but I may have missed something --- ## Changelog - Changed `EntityMapper` to use `EntityHashMap` instead of normal `HashMap` ## Migration Guide If you are using the following types, update their listed methods to use the new `EntityHashMap`. `EntityHashMap` has the same methods as the normal `HashMap`, so you just need to replace the name. ### `EntityMapper` - `get_map` - `get_mut_map` - `new` - `world_scope` ### `ReflectMapEntities` - `map_all_entities` - `map_entities` - `write_to_world` ### `InstanceInfo` - `entity_map` - This is a property, not a method. --- This is my first time contributing in a while, and I'm not familiar with the usage of `EntityMapper`. I changed the type definition and fixed all errors, but there may have been things I've missed. Please keep an eye out for me! |