dependabot/cargo/accesskit-0.20
320 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
charlotte 🌸
|
92e65d5eb1
|
Upgrade to Rust 1.88 (#19825) | ||
|
dependabot[bot]
|
c7b5bc93c3
|
Update derive_more requirement from 1 to 2 (#19671)
Updates the requirements on [derive_more](https://github.com/JelteF/derive_more) to permit the latest version. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/JelteF/derive_more/releases">derive_more's releases</a>.</em></p> <blockquote> <h2>2.0.1</h2> <p><a href="https://docs.rs/derive_more/2.0.1">API docs</a> <a href="https://github.com/JelteF/derive_more/blob/v2.0.1/CHANGELOG.md#201---2025-02-03">Changelog</a></p> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/JelteF/derive_more/blob/master/CHANGELOG.md">derive_more's changelog</a>.</em></p> <blockquote> <h2>2.0.1 - 2025-02-03</h2> <h3>Added</h3> <ul> <li>Add crate metadata for the Rust Playground. This makes sure that the Rust Playground will have all <code>derive_more</code> features available once <a href="https://docs.rs/selectors/latest/selectors"><code>selectors</code></a> crate updates its <code>derive_more</code> version. (<a href="https://redirect.github.com/JelteF/derive_more/pull/445">#445</a>)</li> </ul> <h2>2.0.0 - 2025-02-03</h2> <h3>Breaking changes</h3> <ul> <li><code>use derive_more::SomeTrait</code> now imports macro only. Importing macro with its trait along is possible now via <code>use derive_more::with_trait::SomeTrait</code>. (<a href="https://redirect.github.com/JelteF/derive_more/pull/406">#406</a>)</li> <li>Top-level <code>#[display("...")]</code> attribute on an enum now has defaulting behavior instead of replacing when no wrapping is possible (no <code>_variant</code> placeholder). (<a href="https://redirect.github.com/JelteF/derive_more/pull/395">#395</a>)</li> </ul> <h3>Fixed</h3> <ul> <li>Associated types of type parameters not being treated as generics in <code>Debug</code> and <code>Display</code> expansions. (<a href="https://redirect.github.com/JelteF/derive_more/pull/399">#399</a>)</li> <li><code>unreachable_code</code> warnings on generated code when <code>!</code> (never type) is used. (<a href="https://redirect.github.com/JelteF/derive_more/pull/404">#404</a>)</li> <li>Ambiguous associated item error when deriving <code>TryFrom</code>, <code>TryInto</code> or <code>FromStr</code> with an associated item called <code>Error</code> or <code>Err</code> respectively. (<a href="https://redirect.github.com/JelteF/derive_more/pull/410">#410</a>)</li> <li>Top-level <code>#[display("...")]</code> attribute on an enum being incorrectly treated as transparent or wrapping. (<a href="https://redirect.github.com/JelteF/derive_more/pull/395">#395</a>)</li> <li>Omitted raw identifiers in <code>Debug</code> and <code>Display</code> expansions. (<a href="https://redirect.github.com/JelteF/derive_more/pull/431">#431</a>)</li> <li>Incorrect rendering of raw identifiers as field names in <code>Debug</code> expansions. (<a href="https://redirect.github.com/JelteF/derive_more/pull/431">#431</a>)</li> <li>Top-level <code>#[display("...")]</code> attribute on an enum not working transparently for directly specified fields. (<a href="https://redirect.github.com/JelteF/derive_more/pull/438">#438</a>)</li> <li>Incorrect dereferencing of unsized fields in <code>Debug</code> and <code>Display</code> expansions. (<a href="https://redirect.github.com/JelteF/derive_more/pull/440">#440</a>)</li> </ul> <h2>0.99.19 - 2025-02-03</h2> <ul> <li>Add crate metadata for the Rust Playground.</li> </ul> <h2>1.0.0 - 2024-08-07</h2> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href=" |
||
|
Jan Hohenheim
|
7aed3e411d
|
Implement serializable mesh (#19743)
# Objective - Alternative to and closes #19545 - Resolves #9790 by providing an alternative - `Mesh` is meant as format optimized for the renderer. There are no guarantees about how it looks, and breaking changes are expected - This makes it not feasible to implement `Reflect` for all its fields or `Serialize` it. - However, (de)serializing a mesh has an important use case: send a mesh over BRP to another process, like an editor! - In my case, I'm making a navmesh editor and need to copy the level that is running in the game into the editor process - Assets don't solve this because - They don't work over BRP #19709 and - The meshes may be procedural - So, we need a way to (de)serialize a mesh for short-term transmissions. ## Solution - Like `SerializedAnimationGraph` before, let's make a `SerializedMesh`! - This type's fields are all `private` because we want to keep the internals of `Mesh` hidden, and exposing them through this secondary struct would be counter-productive to that - All this struct can do is be serialized, be deserialized, and be converted to and from a mesh - It's not a lossless transmission: the handle for morph targets is ignored, and things like the render usages make no sense to be transmitted imo ## Future Work The same song and dance needs to happen for `Image`, but I can live with completely white meshes for the moment lol ## Testing - Added a simple test --------- Co-authored-by: atlv <email@atlasdostal.com> |
||
|
github-actions[bot]
|
a466084167
|
Bump Version after Release (#19774)
Bump version after release This PR has been auto-generated Fixes #19766 --------- Co-authored-by: Bevy Auto Releaser <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: François Mockers <francois.mockers@vleue.com> Co-authored-by: François Mockers <mockersf@gmail.com> |
||
|
François Mockers
|
4e694aea53
|
ECS: put strings only used for debug behind a feature (#19558)
# Objective - Many strings in bevy_ecs are created but only used for debug: system name, component name, ... - Those strings make a significant part of the final binary and are no use in a released game ## Solution - Use [`strings`](https://linux.die.net/man/1/strings) to find ... strings in a binary - Try to find where they come from - Many are made from `type_name::<T>()` and only used in error / debug messages - Add a new structure `DebugName` that holds no value if `debug` feature is disabled - Replace `core::any::type_name::<T>()` by `DebugName::type_name::<T>()` ## Testing Measurements were taken without the new feature being enabled by default, to help with commands ### File Size I tried building the `breakout` example with `cargo run --release --example breakout` |`debug` enabled|`debug` disabled| |-|-| |81621776 B|77735728B| |77.84MB|74.13MB| ### Compilation time `hyperfine --min-runs 15 --prepare "cargo clean && sleep 5" 'RUSTC_WRAPPER="" cargo build --release --example breakout' 'RUSTC_WRAPPER="" cargo build --release --example breakout --features debug'` ``` breakout' 'RUSTC_WRAPPER="" cargo build --release --example breakout --features debug' Benchmark 1: RUSTC_WRAPPER="" cargo build --release --example breakout Time (mean ± σ): 84.856 s ± 3.565 s [User: 1093.817 s, System: 32.547 s] Range (min … max): 78.038 s … 89.214 s 15 runs Benchmark 2: RUSTC_WRAPPER="" cargo build --release --example breakout --features debug Time (mean ± σ): 92.303 s ± 2.466 s [User: 1193.443 s, System: 33.803 s] Range (min … max): 90.619 s … 99.684 s 15 runs Summary RUSTC_WRAPPER="" cargo build --release --example breakout ran 1.09 ± 0.05 times faster than RUSTC_WRAPPER="" cargo build --release --example breakout --features debug ``` |
||
|
Joona Aalto
|
38c3423693
|
Event Split: Event, EntityEvent, and BufferedEvent (#19647)
# Objective Closes #19564. The current `Event` trait looks like this: ```rust pub trait Event: Send + Sync + 'static { type Traversal: Traversal<Self>; const AUTO_PROPAGATE: bool = false; fn register_component_id(world: &mut World) -> ComponentId { ... } fn component_id(world: &World) -> Option<ComponentId> { ... } } ``` The `Event` trait is used by both buffered events (`EventReader`/`EventWriter`) and observer events. If they are observer events, they can optionally be targeted at specific `Entity`s or `ComponentId`s, and can even be propagated to other entities. However, there has long been a desire to split the trait semantically for a variety of reasons, see #14843, #14272, and #16031 for discussion. Some reasons include: - It's very uncommon to use a single event type as both a buffered event and targeted observer event. They are used differently and tend to have distinct semantics. - A common footgun is using buffered events with observers or event readers with observer events, as there is no type-level error that prevents this kind of misuse. - #19440 made `Trigger::target` return an `Option<Entity>`. This *seriously* hurts ergonomics for the general case of entity observers, as you need to `.unwrap()` each time. If we could statically determine whether the event is expected to have an entity target, this would be unnecessary. There's really two main ways that we can categorize events: push vs. pull (i.e. "observer event" vs. "buffered event") and global vs. targeted: | | Push | Pull | | ------------ | --------------- | --------------------------- | | **Global** | Global observer | `EventReader`/`EventWriter` | | **Targeted** | Entity observer | - | There are many ways to approach this, each with their tradeoffs. Ultimately, we kind of want to split events both ways: - A type-level distinction between observer events and buffered events, to prevent people from using the wrong kind of event in APIs - A statically designated entity target for observer events to avoid accidentally using untargeted events for targeted APIs This PR achieves these goals by splitting event traits into `Event`, `EntityEvent`, and `BufferedEvent`, with `Event` being the shared trait implemented by all events. ## `Event`, `EntityEvent`, and `BufferedEvent` `Event` is now a very simple trait shared by all events. ```rust pub trait Event: Send + Sync + 'static { // Required for observer APIs fn register_component_id(world: &mut World) -> ComponentId { ... } fn component_id(world: &World) -> Option<ComponentId> { ... } } ``` You can call `trigger` for *any* event, and use a global observer for listening to the event. ```rust #[derive(Event)] struct Speak { message: String, } // ... app.add_observer(|trigger: On<Speak>| { println!("{}", trigger.message); }); // ... commands.trigger(Speak { message: "Y'all like these reworked events?".to_string(), }); ``` To allow an event to be targeted at entities and even propagated further, you can additionally implement the `EntityEvent` trait: ```rust pub trait EntityEvent: Event { type Traversal: Traversal<Self>; const AUTO_PROPAGATE: bool = false; } ``` This lets you call `trigger_targets`, and to use targeted observer APIs like `EntityCommands::observe`: ```rust #[derive(Event, EntityEvent)] #[entity_event(traversal = &'static ChildOf, auto_propagate)] struct Damage { amount: f32, } // ... let enemy = commands.spawn((Enemy, Health(100.0))).id(); // Spawn some armor as a child of the enemy entity. // When the armor takes damage, it will bubble the event up to the enemy. let armor_piece = commands .spawn((ArmorPiece, Health(25.0), ChildOf(enemy))) .observe(|trigger: On<Damage>, mut query: Query<&mut Health>| { // Note: `On::target` only exists because this is an `EntityEvent`. let mut health = query.get(trigger.target()).unwrap(); health.0 -= trigger.amount(); }); commands.trigger_targets(Damage { amount: 10.0 }, armor_piece); ``` > [!NOTE] > You *can* still also trigger an `EntityEvent` without targets using `trigger`. We probably *could* make this an either-or thing, but I'm not sure that's actually desirable. To allow an event to be used with the buffered API, you can implement `BufferedEvent`: ```rust pub trait BufferedEvent: Event {} ``` The event can then be used with `EventReader`/`EventWriter`: ```rust #[derive(Event, BufferedEvent)] struct Message(String); fn write_hello(mut writer: EventWriter<Message>) { writer.write(Message("I hope these examples are alright".to_string())); } fn read_messages(mut reader: EventReader<Message>) { // Process all buffered events of type `Message`. for Message(message) in reader.read() { println!("{message}"); } } ``` In summary: - Need a basic event you can trigger and observe? Derive `Event`! - Need the event to be targeted at an entity? Derive `EntityEvent`! - Need the event to be buffered and support the `EventReader`/`EventWriter` API? Derive `BufferedEvent`! ## Alternatives I'll now cover some of the alternative approaches I have considered and briefly explored. I made this section collapsible since it ended up being quite long :P <details> <summary>Expand this to see alternatives</summary> ### 1. Unified `Event` Trait One option is not to have *three* separate traits (`Event`, `EntityEvent`, `BufferedEvent`), and to instead just use associated constants on `Event` to determine whether an event supports targeting and buffering or not: ```rust pub trait Event: Send + Sync + 'static { type Traversal: Traversal<Self>; const AUTO_PROPAGATE: bool = false; const TARGETED: bool = false; const BUFFERED: bool = false; fn register_component_id(world: &mut World) -> ComponentId { ... } fn component_id(world: &World) -> Option<ComponentId> { ... } } ``` Methods can then use bounds like `where E: Event<TARGETED = true>` or `where E: Event<BUFFERED = true>` to limit APIs to specific kinds of events. This would keep everything under one `Event` trait, but I don't think it's necessarily a good idea. It makes APIs harder to read, and docs can't easily refer to specific types of events. You can also create weird invariants: what if you specify `TARGETED = false`, but have `Traversal` and/or `AUTO_PROPAGATE` enabled? ### 2. `Event` and `Trigger` Another option is to only split the traits between buffered events and observer events, since that is the main thing people have been asking for, and they have the largest API difference. If we did this, I think we would need to make the terms *clearly* separate. We can't really use `Event` and `BufferedEvent` as the names, since it would be strange that `BufferedEvent` doesn't implement `Event`. Something like `ObserverEvent` and `BufferedEvent` could work, but it'd be more verbose. For this approach, I would instead keep `Event` for the current `EventReader`/`EventWriter` API, and call the observer event a `Trigger`, since the "trigger" terminology is already used in the observer context within Bevy (both as a noun and a verb). This is also what a long [bikeshed on Discord](https://discord.com/channels/691052431525675048/749335865876021248/1298057661878898791) seemed to land on at the end of last year. ```rust // For `EventReader`/`EventWriter` pub trait Event: Send + Sync + 'static {} // For observers pub trait Trigger: Send + Sync + 'static { type Traversal: Traversal<Self>; const AUTO_PROPAGATE: bool = false; const TARGETED: bool = false; fn register_component_id(world: &mut World) -> ComponentId { ... } fn component_id(world: &World) -> Option<ComponentId> { ... } } ``` The problem is that "event" is just a really good term for something that "happens". Observers are rapidly becoming the more prominent API, so it'd be weird to give them the `Trigger` name and leave the good `Event` name for the less common API. So, even though a split like this seems neat on the surface, I think it ultimately wouldn't really work. We want to keep the `Event` name for observer events, and there is no good alternative for the buffered variant. (`Message` was suggested, but saying stuff like "sends a collision message" is weird.) ### 3. `GlobalEvent` + `TargetedEvent` What if instead of focusing on the buffered vs. observed split, we *only* make a distinction between global and targeted events? ```rust // A shared event trait to allow global observers to work pub trait Event: Send + Sync + 'static { fn register_component_id(world: &mut World) -> ComponentId { ... } fn component_id(world: &World) -> Option<ComponentId> { ... } } // For buffered events and non-targeted observer events pub trait GlobalEvent: Event {} // For targeted observer events pub trait TargetedEvent: Event { type Traversal: Traversal<Self>; const AUTO_PROPAGATE: bool = false; } ``` This is actually the first approach I implemented, and it has the neat characteristic that you can only use non-targeted APIs like `trigger` with a `GlobalEvent` and targeted APIs like `trigger_targets` with a `TargetedEvent`. You have full control over whether the entity should or should not have a target, as they are fully distinct at the type-level. However, there's a few problems: - There is no type-level indication of whether a `GlobalEvent` supports buffered events or just non-targeted observer events - An `Event` on its own does literally nothing, it's just a shared trait required to make global observers accept both non-targeted and targeted events - If an event is both a `GlobalEvent` and `TargetedEvent`, global observers again have ambiguity on whether an event has a target or not, undermining some of the benefits - The names are not ideal ### 4. `Event` and `EntityEvent` We can fix some of the problems of Alternative 3 by accepting that targeted events can also be used in non-targeted contexts, and simply having the `Event` and `EntityEvent` traits: ```rust // For buffered events and non-targeted observer events pub trait Event: Send + Sync + 'static { fn register_component_id(world: &mut World) -> ComponentId { ... } fn component_id(world: &World) -> Option<ComponentId> { ... } } // For targeted observer events pub trait EntityEvent: Event { type Traversal: Traversal<Self>; const AUTO_PROPAGATE: bool = false; } ``` This is essentially identical to this PR, just without a dedicated `BufferedEvent`. The remaining major "problem" is that there is still zero type-level indication of whether an `Event` event *actually* supports the buffered API. This leads us to the solution proposed in this PR, using `Event`, `EntityEvent`, and `BufferedEvent`. </details> ## Conclusion The `Event` + `EntityEvent` + `BufferedEvent` split proposed in this PR aims to solve all the common problems with Bevy's current event model while keeping the "weirdness" factor minimal. It splits in terms of both the push vs. pull *and* global vs. targeted aspects, while maintaining a shared concept for an "event". ### Why I Like This - The term "event" remains as a single concept for all the different kinds of events in Bevy. - Despite all event types being "events", they use fundamentally different APIs. Instead of assuming that you can use an event type with any pattern (when only one is typically supported), you explicitly opt in to each one with dedicated traits. - Using separate traits for each type of event helps with documentation and clearer function signatures. - I can safely make assumptions on expected usage. - If I see that an event is an `EntityEvent`, I can assume that I can use `observe` on it and get targeted events. - If I see that an event is a `BufferedEvent`, I can assume that I can use `EventReader` to read events. - If I see both `EntityEvent` and `BufferedEvent`, I can assume that both APIs are supported. In summary: This allows for a unified concept for events, while limiting the different ways to use them with opt-in traits. No more guess-work involved when using APIs. ### Problems? - Because `BufferedEvent` implements `Event` (for more consistent semantics etc.), you can still use all buffered events for non-targeted observers. I think this is fine/good. The important part is that if you see that an event implements `BufferedEvent`, you know that the `EventReader`/`EventWriter` API should be supported. Whether it *also* supports other APIs is secondary. - I currently only support `trigger_targets` for an `EntityEvent`. However, you can technically target components too, without targeting any entities. I consider that such a niche and advanced use case that it's not a huge problem to only support it for `EntityEvent`s, but we could also split `trigger_targets` into `trigger_entities` and `trigger_components` if we wanted to (or implement components as entities :P). - You can still trigger an `EntityEvent` *without* targets. I consider this correct, since `Event` implements the non-targeted behavior, and it'd be weird if implementing another trait *removed* behavior. However, it does mean that global observers for entity events can technically return `Entity::PLACEHOLDER` again (since I got rid of the `Option<Entity>` added in #19440 for ergonomics). I think that's enough of an edge case that it's not a huge problem, but it is worth keeping in mind. - ~~Deriving both `EntityEvent` and `BufferedEvent` for the same type currently duplicates the `Event` implementation, so you instead need to manually implement one of them.~~ Changed to always requiring `Event` to be derived. ## Related Work There are plans to implement multi-event support for observers, especially for UI contexts. [Cart's example](https://github.com/bevyengine/bevy/issues/14649#issuecomment-2960402508) API looked like this: ```rust // Truncated for brevity trigger: Trigger<( OnAdd<Pressed>, OnRemove<Pressed>, OnAdd<InteractionDisabled>, OnRemove<InteractionDisabled>, OnInsert<Hovered>, )>, ``` I believe this shouldn't be in conflict with this PR. If anything, this PR might *help* achieve the multi-event pattern for entity observers with fewer footguns: by statically enforcing that all of these events are `EntityEvent`s in the context of `EntityCommands::observe`, we can avoid misuse or weird cases where *some* events inside the trigger are targeted while others are not. |
||
|
Joona Aalto
|
e5dc177b4b
|
Rename Trigger to On (#19596)
# Objective
Currently, the observer API looks like this:
```rust
app.add_observer(|trigger: Trigger<Explode>| {
info!("Entity {} exploded!", trigger.target());
});
```
Future plans for observers also include "multi-event observers" with a
trigger that looks like this (see [Cart's
example](https://github.com/bevyengine/bevy/issues/14649#issuecomment-2960402508)):
```rust
trigger: Trigger<(
OnAdd<Pressed>,
OnRemove<Pressed>,
OnAdd<InteractionDisabled>,
OnRemove<InteractionDisabled>,
OnInsert<Hovered>,
)>,
```
In scenarios like this, there is a lot of repetition of `On`. These are
expected to be very high-traffic APIs especially in UI contexts, so
ergonomics and readability are critical.
By renaming `Trigger` to `On`, we can make these APIs read more cleanly
and get rid of the repetition:
```rust
app.add_observer(|trigger: On<Explode>| {
info!("Entity {} exploded!", trigger.target());
});
```
```rust
trigger: On<(
Add<Pressed>,
Remove<Pressed>,
Add<InteractionDisabled>,
Remove<InteractionDisabled>,
Insert<Hovered>,
)>,
```
Names like `On<Add<Pressed>>` emphasize the actual event listener nature
more than `Trigger<OnAdd<Pressed>>`, and look cleaner. This *also* frees
up the `Trigger` name if we want to use it for the observer event type,
splitting them out from buffered events (bikeshedding this is out of
scope for this PR though).
For prior art:
[`bevy_eventlistener`](https://github.com/aevyrie/bevy_eventlistener)
used
[`On`](https://docs.rs/bevy_eventlistener/latest/bevy_eventlistener/event_listener/struct.On.html)
for its event listener type. Though in our case, the observer is the
event listener, and `On` is just a type containing information about the
triggered event.
## Solution
Steal from `bevy_event_listener` by @aevyrie and use `On`.
- Rename `Trigger` to `On`
- Rename `OnAdd` to `Add`
- Rename `OnInsert` to `Insert`
- Rename `OnReplace` to `Replace`
- Rename `OnRemove` to `Remove`
- Rename `OnDespawn` to `Despawn`
## Discussion
### Naming Conflicts??
Using a name like `Add` might initially feel like a very bad idea, since
it risks conflict with `core::ops::Add`. However, I don't expect this to
be a big problem in practice.
- You rarely need to actually implement the `Add` trait, especially in
modules that would use the Bevy ECS.
- In the rare cases where you *do* get a conflict, it is very easy to
fix by just disambiguating, for example using `ops::Add`.
- The `Add` event is a struct while the `Add` trait is a trait (duh), so
the compiler error should be very obvious.
For the record, renaming `OnAdd` to `Add`, I got exactly *zero* errors
or conflicts within Bevy itself. But this is of course not entirely
representative of actual projects *using* Bevy.
You might then wonder, why not use `Added`? This would conflict with the
`Added` query filter, so it wouldn't work. Additionally, the current
naming convention for observer events does not use past tense.
### Documentation
This does make documentation slightly more awkward when referring to
`On` or its methods. Previous docs often referred to `Trigger::target`
or "sends a `Trigger`" (which is... a bit strange anyway), which would
now be `On::target` and "sends an observer `Event`".
You can see the diff in this PR to see some of the effects. I think it
should be fine though, we may just need to reword more documentation to
read better.
|
||
|
Eagster
|
064e5e48b4
|
Remove entity placeholder from observers (#19440)
# Objective `Entity::PLACEHOLDER` acts as a magic number that will *probably* never really exist, but it certainly could. And, `Entity` has a niche, so the only reason to use `PLACEHOLDER` is as an alternative to `MaybeUninit` that trades safety risks for logic risks. As a result, bevy has generally advised against using `PLACEHOLDER`, but we still use if for a lot internally. This pr starts removing internal uses of it, starting from observers. ## Solution Change all trigger target related types from `Entity` to `Option<Entity>` Small migration guide to come. ## Testing CI ## Future Work This turned a lot of code from ```rust trigger.target() ``` to ```rust trigger.target().unwrap() ``` The extra panic is no worse than before; it's just earlier than panicking after passing the placeholder to something else. But this is kinda annoying. I would like to add a `TriggerMode` or something to `Event` that would restrict what kinds of targets can be used for that event. Many events like `Removed` etc, are always triggered with a target. We can make those have a way to assume Some, etc. But I wanted to save that for a future pr. |
||
|
Eagster
|
8ad7118443
|
Only get valid component ids (#19510)
# Objective - #19504 showed a 11x regression in getting component values for unregistered components. This pr should fix that and improve others a little too. - This is some cleanup work from #18173 . ## Solution - Whenever we expect a component value to exist, we only care about fully registered components, not queued to be registered components since, for the value to exist, it must be registered. - So we can use the faster `get_valid_*` instead of `get_*` in a lot of places. - Also found a bug where `valid_*` did not forward to `get_valid_*` properly. That's fixed. ## Testing CI |
||
|
Carter Anderson
|
7e9d6d852b
|
bevyengine.org -> bevy.org (#19503)
We have acquired [bevy.org](https://bevy.org) and the migration has finished! Meaning we can now update all of the references in this repo. |
||
|
Manuel Brea Carreras
|
3902804114
|
Fix #19219 by moving observer triggers out of resource_scope (#19221)
# Objective Fixes #19219 ## Solution Instead of calling `world.commands().trigger` and `world.commands().trigger_targets` whenever each scene is spawned, save the `instance_id` and optional parent entity to perform all such calls at the end. This prevents the potential flush of the world command queue that can happen if `add_child` is called from causing the crash. ## Testing - Did you test these changes? If so, how? - Verified that I can no longer reproduce the bug with the instructions at #19219. - Ran `bevy_scene` tests - Visually verified that the following examples still run as expected `many_foxes`, `scene` . (should I test any more?) - Are there any parts that need more testing? - Pending to run `cargo test` at the root to test that all examples still build; I will update the PR when that's done - How can other people (reviewers) test your changes? Is there anything specific they need to know? - Run bevy as usual - If relevant, what platforms did you test these changes on, and are there any important ones you can't test? - N/a (tested on Linux/wayland but it shouldn't be relevant) --- |
||
|
Eagster
|
12aba64900
|
Make entity generation a new type and remove identifier (#19121)
# Objective This is a followup to #18704 . There's lots more followup work, but this is the minimum to unblock #18670, etc. This direction has been given the green light by Alice [here](https://github.com/bevyengine/bevy/pull/18704#issuecomment-2853368129). ## Solution I could have split this over multiple PRs, but I figured skipping straight here would be easiest for everyone and would unblock things the quickest. This removes the now no longer needed `identifier` module and makes `Entity::generation` go from `NonZeroU32` to `struct EntityGeneration(u32)`. ## Testing CI --------- Co-authored-by: Mark Nokalt <marknokalt@live.com> |
||
|
Eagster
|
0b4858726c
|
Make entity::index non max (#18704)
# Objective There are two problems this aims to solve. First, `Entity::index` is currently a `u32`. That means there are `u32::MAX + 1` possible entities. Not only is that awkward, but it also make `Entity` allocation more difficult. I discovered this while working on remote entity reservation, but even on main, `Entities` doesn't handle the `u32::MAX + 1` entity very well. It can not be batch reserved because that iterator uses exclusive ranges, which has a maximum upper bound of `u32::MAX - 1`. In other words, having `u32::MAX` as a valid index can be thought of as a bug right now. We either need to make that invalid (this PR), which makes Entity allocation cleaner and makes remote reservation easier (because the length only needs to be u32 instead of u64, which, in atomics is a big deal), or we need to take another pass at `Entities` to make it handle the `u32::MAX` index properly. Second, `TableRow`, `ArchetypeRow` and `EntityIndex` (a type alias for u32) all have `u32` as the underlying type. That means using these as the index type in a `SparseSet` uses 64 bits for the sparse list because it stores `Option<IndexType>`. By using `NonMaxU32` here, we cut the memory of that list in half. To my knowledge, `EntityIndex` is the only thing that would really benefit from this niche. `TableRow` and `ArchetypeRow` I think are not stored in an `Option` in bulk. But if they ever are, this would help. Additionally this ensures `TableRow::INVALID` and `ArchetypeRow::INVALID` never conflict with an actual row, which in a nice bonus. As a related note, if we do components as entities where `ComponentId` becomes `Entity`, the the `SparseSet<ComponentId>` will see a similar memory improvement too. ## Solution Create a new type `EntityRow` that wraps `NonMaxU32`, similar to `TableRow` and `ArchetypeRow`. Change `Entity::index` to this type. ## Downsides `NonMax` is implemented as a `NonZero` with a binary inversion. That means accessing and storing the value takes one more instruction. I don't think that's a big deal, but it's worth mentioning. As a consequence, `to_bits` uses `transmute` to skip the inversion which keeps it a nop. But that also means that ordering has now flipped. In other words, higher indices are considered less than lower indices. I don't think that's a problem, but it's also worth mentioning. ## Alternatives We could keep the index as a u32 type and just document that `u32::MAX` is invalid, modifying `Entities` to ensure it never gets handed out. (But that's not enforced by the type system.) We could still take advantage of the niche here in `ComponentSparseSet`. We'd just need some unsafe manual conversions, which is probably fine, but opens up the possibility for correctness problems later. We could change `Entities` to fully support the `u32::MAX` index. (But that makes `Entities` more complex and potentially slightly slower.) ## Testing - CI - A few tests were changed because they depend on different ordering and `to_bits` values. ## Future Work - It might be worth removing the niche on `Entity::generation` since there is now a different niche. - We could move `Entity::generation` into it's own type too for clarity. - We should change `ComponentSparseSet` to take advantage of the new niche. (This PR doesn't change that yet.) - Consider removing or updating `Identifier`. This is only used for `Entity`, so it might be worth combining since `Entity` is now more unique. --------- Co-authored-by: atlv <email@atlasdostal.com> Co-authored-by: Zachary Harrold <zac@harrold.com.au> |
||
|
Carter Anderson
|
e9a0ef49f9
|
Rename bevy_platform_support to bevy_platform (#18813)
# Objective The goal of `bevy_platform_support` is to provide a set of platform agnostic APIs, alongside platform-specific functionality. This is a high traffic crate (providing things like HashMap and Instant). Especially in light of https://github.com/bevyengine/bevy/discussions/18799, it deserves a friendlier / shorter name. Given that it hasn't had a full release yet, getting this change in before Bevy 0.16 makes sense. ## Solution - Rename `bevy_platform_support` to `bevy_platform`. |
||
|
Zachary Harrold
|
7892f9e3af
|
Newtype hashbrown (#18694)
# Objective - Fixes #18690 - Closes [#2065](https://github.com/bevyengine/bevy-website/pull/2065) - Alternative to #18691 The changes to the Hash made in #15801 to the [BuildHasher](https://doc.rust-lang.org/std/hash/trait.BuildHasher.html) resulted in serious migration problems and downgraded UX for users of Bevy's re-exported hashmaps. Once merged, we need to go in and remove the migration guide added as part of #15801. ## Solution - Newtype `HashMap` and `HashSet` instead of type aliases - Added `Deref/Mut` to allow accessing future `hashbrown` methods without maintenance from Bevy - Added bidirectional `From` implementations to provide escape hatch for API incompatibility - Added inlinable re-exports of all methods directly to Bevy's types. This ensures `HashMap::new()` works (since the `Deref` implementation wont cover these kinds of invocations). ## Testing - CI --- ## Migration Guide - If you relied on Bevy's `HashMap` and/or `HashSet` types to be identical to `hashbrown`, consider using `From` and `Into` to convert between the `hashbrown` and Bevy types as required. - If you relied on `hashbrown/serde` or `hashbrown/rayon` features, you may need to enable `bevy_platform_support/serialize` and/or `bevy_platform_support/rayon` respectively. --- ## Notes - Did not replicate the Rayon traits, users will need to rely on the `Deref/Mut` or `From` implementations for those methods. - Did not re-expose the `unsafe` methods from `hashbrown`. In most cases users will still have access via `Deref/Mut` anyway. - I have added `inline` to all methods as they are trivial wrappings of existing methods. - I chose to make `HashMap::new` and `HashSet::new` const, which is different to `hashbrown`. We can do this because we default to a fixed-state build-hasher. Mild ergonomic win over using `HashMap::with_hasher(FixedHasher)`. |
||
|
Carter Anderson
|
d8fa57bd7b
|
Switch ChildOf back to tuple struct (#18672)
# Objective In #17905 we swapped to a named field on `ChildOf` to help resolve variable naming ambiguity of child vs parent (ex: `child_of.parent` clearly reads as "I am accessing the parent of the child_of relationship", whereas `child_of.0` is less clear). Unfortunately this has the side effect of making initialization less ideal. `ChildOf { parent }` reads just as well as `ChildOf(parent)`, but `ChildOf { parent: root }` doesn't read nearly as well as `ChildOf(root)`. ## Solution Move back to `ChildOf(pub Entity)` but add a `child_of.parent()` function and use it for all accesses. The downside here is that users are no longer "forced" to access the parent field with `parent` nomenclature, but I think this strikes the right balance. Take a look at the diff. I think the results provide strong evidence for this change. Initialization has the benefit of reading much better _and_ of taking up significantly less space, as many lines go from 3 to 1, and we're cutting out a bunch of syntax in some cases. Sadly I do think this should land in 0.16 as the cost of doing this _after_ the relationships migration is high. |
||
|
Vic
|
f57c7a43c4
|
reexport entity set collections in entity module (#18413)
# Objective Unlike for their helper typers, the import paths for `unique_array::UniqueEntityArray`, `unique_slice::UniqueEntitySlice`, `unique_vec::UniqueEntityVec`, `hash_set::EntityHashSet`, `hash_map::EntityHashMap`, `index_set::EntityIndexSet`, `index_map::EntityIndexMap` are quite redundant. When looking at the structure of `hashbrown`, we can also see that while both `HashSet` and `HashMap` have their own modules, the main types themselves are re-exported to the crate level. ## Solution Re-export the types in their shared `entity` parent module, and simplify the imports where they're used. |
||
|
Kristoffer Søholm
|
7a37c4a109
|
Update bincode to 2.0 (#18396)
# Objective Update bincode ## Solution Fix compilation for #18352 by reading the [migration guide](https://github.com/bincode-org/bincode/blob/trunk/docs/migration_guide.md) Also fixes an unused import warning I got when running the tests for bevy_reflect. |
||
|
Zachary Harrold
|
e3968e2963
|
Properly gate imports in bevy_scene (#18482)
# Objective - Some imports are only used with certain features. ## Solution - Gate them! ## Testing - CI |
||
|
Carter Anderson
|
a033f1b206
|
Replace VisitEntities with MapEntities (#18432)
# Objective There are currently too many disparate ways to handle entity mapping, especially after #17687. We now have MapEntities, VisitEntities, VisitEntitiesMut, Component::visit_entities, Component::visit_entities_mut. Our only known use case at the moment for these is entity mapping. This means we have significant consolidation potential. Additionally, VisitEntitiesMut cannot be implemented for map-style collections like HashSets, as you cant "just" mutate a `&mut Entity`. Our current approach to Component mapping requires VisitEntitiesMut, meaning this category of entity collection isn't mappable. `MapEntities` is more generally applicable. Additionally, the _existence_ of the blanket From impl on VisitEntitiesMut blocks us from implementing MapEntities for HashSets (or any types we don't own), because the owner could always add a conflicting impl in the future. ## Solution Use `MapEntities` everywhere and remove all "visit entities" usages. * Add `Component::map_entities` * Remove `Component::visit_entities`, `Component::visit_entities_mut`, `VisitEntities`, and `VisitEntitiesMut` * Support deriving `Component::map_entities` in `#[derive(Coomponent)]` * Add `#[derive(MapEntities)]`, and share logic with the `Component::map_entities` derive. * Add `ComponentCloneCtx::queue_deferred`, which is command-like logic that runs immediately after normal clones. Reframe `FromWorld` fallback logic in the "reflect clone" impl to use it. This cuts out a lot of unnecessary work and I think justifies the existence of a pseudo-command interface (given how niche, yet performance sensitive this is). Note that we no longer auto-impl entity mapping for ` IntoIterator<Item = &'a Entity>` types, as this would block our ability to implement cases like `HashMap`. This means the onus is on us (or type authors) to add explicit support for types that should be mappable. Also note that the Component-related changes do not require a migration guide as there hasn't been a release with them yet. ## Migration Guide If you were previously implementing `VisitEntities` or `VisitEntitiesMut` (likely via a derive), instead use `MapEntities`. Those were almost certainly used in the context of Bevy Scenes or reflection via `ReflectMapEntities`. If you have a case that uses `VisitEntities` or `VisitEntitiesMut` directly, where `MapEntities` is not a viable replacement, please let us know! ```rust // before #[derive(VisitEntities, VisitEntitiesMut)] struct Inventory { items: Vec<Entity>, #[visit_entities(ignore)] label: String, } // after #[derive(MapEntities)] struct Inventory { #[entities] items: Vec<Entity>, label: String, } ``` |
||
|
Eagster
|
339914b0af
|
Fix dynamic scene resources not being entity mapped (#18395)
# Objective The resources were converted via `clone_reflect_value` and the cloned value was mapped. But the value that is inserted is the source of the clone, which was not mapped. I ran into this issue while working on #18380. Having non consecutive entity allocations has caught a lot of bugs. ## Solution Use the cloned value for insertion if it exists. |
||
|
Carter Anderson
|
6d6054116a
|
Support skipping Relationship on_replace hooks (#18378)
# Objective Fixes #18357 ## Solution Generalize `RelationshipInsertHookMode` to `RelationshipHookMode`, wire it up to on_replace execution, and use it in the `Relationship::on_replace` hook. |
||
|
andristarr
|
2b21b6cc13
|
FilteredResource returns a Result instead of a simple Option (#18265)
# Objective FilteredResource::get should return a Result instead of Option Fixes #17480 --- ## Migration Guide Users will need to handle the different return type on FilteredResource::get, FilteredResource::get_id, FilteredResource::get_mut as it is now a Result not an Option. |
||
|
Gino Valente
|
9b32e09551
|
bevy_reflect: Add clone registrations project-wide (#18307)
# Objective Now that #13432 has been merged, it's important we update our reflected types to properly opt into this feature. If we do not, then this could cause issues for users downstream who want to make use of reflection-based cloning. ## Solution This PR is broken into 4 commits: 1. Add `#[reflect(Clone)]` on all types marked `#[reflect(opaque)]` that are also `Clone`. This is mandatory as these types would otherwise cause the cloning operation to fail for any type that contains it at any depth. 2. Update the reflection example to suggest adding `#[reflect(Clone)]` on opaque types. 3. Add `#[reflect(clone)]` attributes on all fields marked `#[reflect(ignore)]` that are also `Clone`. This prevents the ignored field from causing the cloning operation to fail. Note that some of the types that contain these fields are also `Clone`, and thus can be marked `#[reflect(Clone)]`. This makes the `#[reflect(clone)]` attribute redundant. However, I think it's safer to keep it marked in the case that the `Clone` impl/derive is ever removed. I'm open to removing them, though, if people disagree. 4. Finally, I added `#[reflect(Clone)]` on all types that are also `Clone`. While not strictly necessary, it enables us to reduce the generated output since we can just call `Clone::clone` directly instead of calling `PartialReflect::reflect_clone` on each variant/field. It also means we benefit from any optimizations or customizations made in the `Clone` impl, including directly dereferencing `Copy` values and increasing reference counters. Along with that change I also took the liberty of adding any missing registrations that I saw could be applied to the type as well, such as `Default`, `PartialEq`, and `Hash`. There were hundreds of these to edit, though, so it's possible I missed quite a few. That last commit is **_massive_**. There were nearly 700 types to update. So it's recommended to review the first three before moving onto that last one. Additionally, I can break the last commit off into its own PR or into smaller PRs, but I figured this would be the easiest way of doing it (and in a timely manner since I unfortunately don't have as much time as I used to for code contributions). ## Testing You can test locally with a `cargo check`: ``` cargo check --workspace --all-features ``` |
||
|
Gino Valente
|
c2854a2a05
|
bevy_reflect: Deprecate PartialReflect::clone_value (#18284)
# Objective #13432 added proper reflection-based cloning. This is a better method than cloning via `clone_value` for reasons detailed in the description of that PR. However, it may not be immediately apparent to users why one should be used over the other, and what the gotchas of `clone_value` are. ## Solution This PR marks `PartialReflect::clone_value` as deprecated, with the deprecation notice pointing users to `PartialReflect::reflect_clone`. However, it also suggests using a new method introduced in this PR: `PartialReflect::to_dynamic`. `PartialReflect::to_dynamic` is essentially a renaming of `PartialReflect::clone_value`. By naming it `to_dynamic`, we make it very obvious that what's returned is a dynamic type. The one caveat to this is that opaque types still use `reflect_clone` as they have no corresponding dynamic type. Along with changing the name, the method is now optional, and comes with a default implementation that calls out to the respective reflection subtrait method. This was done because there was really no reason to require manual implementors provide a method that almost always calls out to a known set of methods. Lastly, to make this default implementation work, this PR also did a similar thing with the `clone_dynamic ` methods on the reflection subtraits. For example, `Struct::clone_dynamic` has been marked deprecated and is superseded by `Struct::to_dynamic_struct`. This was necessary to avoid the "multiple names in scope" issue. ### Open Questions This PR maintains the original signature of `clone_value` on `to_dynamic`. That is, it takes `&self` and returns `Box<dyn PartialReflect>`. However, in order for this to work, it introduces a panic if the value is opaque and doesn't override the default `reflect_clone` implementation. One thing we could do to avoid the panic would be to make the conversion fallible, either returning `Option<Box<dyn PartialReflect>>` or `Result<Box<dyn PartialReflect>, ReflectCloneError>`. This makes using the method a little more involved (i.e. users have to either unwrap or handle the rare possibility of an error), but it would set us up for a world where opaque types don't strictly need to be `Clone`. Right now this bound is sort of implied by the fact that `clone_value` is a required trait method, and the default behavior of the macro is to use `Clone` for opaque types. Alternatively, we could keep the signature but make the method required. This maintains that implied bound where manual implementors must provide some way of cloning the value (or YOLO it and just panic), but also makes the API simpler to use. Finally, we could just leave it with the panic. It's unlikely this would occur in practice since our macro still requires `Clone` for opaque types, and thus this would only ever be an issue if someone were to manually implement `PartialReflect` without a valid `to_dynamic` or `reflect_clone` method. ## Testing You can test locally using the following command: ``` cargo test --package bevy_reflect --all-features ``` --- ## Migration Guide `PartialReflect::clone_value` is being deprecated. Instead, use `PartialReflect::to_dynamic` if wanting to create a new dynamic instance of the reflected value. Alternatively, use `PartialReflect::reflect_clone` to attempt to create a true clone of the underlying value. Similarly, the following methods have been deprecated and should be replaced with these alternatives: - `Array::clone_dynamic` → `Array::to_dynamic_array` - `Enum::clone_dynamic` → `Enum::to_dynamic_enum` - `List::clone_dynamic` → `List::to_dynamic_list` - `Map::clone_dynamic` → `Map::to_dynamic_map` - `Set::clone_dynamic` → `Set::to_dynamic_set` - `Struct::clone_dynamic` → `Struct::to_dynamic_struct` - `Tuple::clone_dynamic` → `Tuple::to_dynamic_tuple` - `TupleStruct::clone_dynamic` → `TupleStruct::to_dynamic_tuple_struct` |
||
|
newclarityex
|
ecccd57417
|
Generic system config (#17962)
# Objective Prevents duplicate implementation between IntoSystemConfigs and IntoSystemSetConfigs using a generic, adds a NodeType trait for more config flexibility (opening the door to implement https://github.com/bevyengine/bevy/issues/14195?). ## Solution Followed writeup by @ItsDoot: https://hackmd.io/@doot/rJeefFHc1x Removes IntoSystemConfigs and IntoSystemSetConfigs, instead using IntoNodeConfigs with generics. ## Testing Pending --- ## Showcase N/A ## Migration Guide SystemSetConfigs -> NodeConfigs<InternedSystemSet> SystemConfigs -> NodeConfigs<ScheduleSystem> IntoSystemSetConfigs -> IntoNodeConfigs<InternedSystemSet, M> IntoSystemConfigs -> IntoNodeConfigs<ScheduleSystem, M> --------- Co-authored-by: Christian Hughes <9044780+ItsDoot@users.noreply.github.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Zachary Harrold
|
c6204279eb
|
Support for non-browser wasm (#17499)
# Objective - Contributes to #15460 - Supersedes #8520 - Fixes #4906 ## Solution - Added a new `web` feature to `bevy`, and several of its crates. - Enabled new `web` feature automatically within crates without `no_std` support. ## Testing - `cargo build --no-default-features --target wasm32v1-none` --- ## Migration Guide When using Bevy crates which _don't_ automatically enable the `web` feature, please enable it when building for the browser. ## Notes - I added [`cfg_if`](https://crates.io/crates/cfg-if) to help manage some of the feature gate gore that this extra feature introduces. It's still pretty ugly, but I think much easier to read. - Certain `wasm` targets (e.g., [wasm32-wasip1](https://doc.rust-lang.org/nightly/rustc/platform-support/wasm32-wasip1.html#wasm32-wasip1)) provide an incomplete implementation for `std`. I have not tested these platforms, but I suspect Bevy's liberal use of usually unsupported features (e.g., threading) will cause these targets to fail. As such, consider `wasm32-unknown-unknown` as the only `wasm` platform with support from Bevy for `std`. All others likely will need to be treated as `no_std` platforms. |
||
|
Zachary Harrold
|
cc69fdd0c6
|
Add no_std support to bevy (#17955)
# Objective - Fixes #15460 (will open new issues for further `no_std` efforts) - Supersedes #17715 ## Solution - Threaded in new features as required - Made certain crates optional but default enabled - Removed `compile-check-no-std` from internal `ci` tool since GitHub CI can now simply check `bevy` itself now - Added CI task to check `bevy` on `thumbv6m-none-eabi` to ensure `portable-atomic` support is still valid [^1] [^1]: This may be controversial, since it could be interpreted as implying Bevy will maintain support for `thumbv6m-none-eabi` going forward. In reality, just like `x86_64-unknown-none`, this is a [canary](https://en.wiktionary.org/wiki/canary_in_a_coal_mine) target to make it clear when `portable-atomic` no longer works as intended (fixing atomic support on atomically challenged platforms). If a PR comes through and makes supporting this class of platforms impossible, then this CI task can be removed. I however wager this won't be a problem. ## Testing - CI --- ## Release Notes Bevy now has support for `no_std` directly from the `bevy` crate. Users can disable default features and enable a new `default_no_std` feature instead, allowing `bevy` to be used in `no_std` applications and libraries. ```toml # Bevy for `no_std` platforms bevy = { version = "0.16", default-features = false, features = ["default_no_std"] } ``` `default_no_std` enables certain required features, such as `libm` and `critical-section`, and as many optional crates as possible (currently just `bevy_state`). For atomically-challenged platforms such as the Raspberry Pi Pico, `portable-atomic` will be used automatically. For library authors, we recommend depending on `bevy` with `default-features = false` to allow `std` and `no_std` users to both depend on your crate. Here are some recommended features a library crate may want to expose: ```toml [features] # Most users will be on a platform which has `std` and can use the more-powerful `async_executor`. default = ["std", "async_executor"] # Features for typical platforms. std = ["bevy/std"] async_executor = ["bevy/async_executor"] # Features for `no_std` platforms. libm = ["bevy/libm"] critical-section = ["bevy/critical-section"] [dependencies] # We disable default features to ensure we don't accidentally enable `std` on `no_std` targets, for example. bevy = { version = "0.16", default-features = false } ``` While this is verbose, it gives the maximum control to end-users to decide how they wish to use Bevy on their platform. We encourage library authors to experiment with `no_std` support. For libraries relying exclusively on `bevy` and no other dependencies, it may be as simple as adding `#![no_std]` to your `lib.rs` and exposing features as above! Bevy can also provide many `std` types, such as `HashMap`, `Mutex`, and `Instant` on all platforms. See `bevy::platform_support` for details on what's available out of the box! ## Migration Guide - If you were previously relying on `bevy` with default features disabled, you may need to enable the `std` and `async_executor` features. - `bevy_reflect` has had its `bevy` feature removed. If you were relying on this feature, simply enable `smallvec` and `smol_str` instead. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Carter Anderson
|
06cb5c5fd9
|
Fix Component require() IDE integration (#18165)
# Objective Component `require()` IDE integration is fully broken, as of #16575. ## Solution This reverts us back to the previous "put the docs on Component trait" impl. This _does_ reduce the accessibility of the required components in rust docs, but the complete erasure of "required component IDE experience" is not worth the price of slightly increased prominence of requires in docs. Additionally, Rust Analyzer has recently started including derive attributes in suggestions, so we aren't losing that benefit of the proc_macro attribute impl. |
||
|
Carter Anderson
|
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> |
||
|
Alice Cecile
|
2ad5908e58
|
Make Query::single (and friends) return a Result (#18082)
# Objective As discussed in #14275, Bevy is currently too prone to panic, and makes the easy / beginner-friendly way to do a large number of operations just to panic on failure. This is seriously frustrating in library code, but also slows down development, as many of the `Query::single` panics can actually safely be an early return (these panics are often due to a small ordering issue or a change in game state. More critically, in most "finished" products, panics are unacceptable: any unexpected failures should be handled elsewhere. That's where the new With the advent of good system error handling, we can now remove this. Note: I was instrumental in a) introducing this idea in the first place and b) pushing to make the panicking variant the default. The introduction of both `let else` statements in Rust and the fancy system error handling work in 0.16 have changed my mind on the right balance here. ## Solution 1. Make `Query::single` and `Query::single_mut` (and other random related methods) return a `Result`. 2. Handle all of Bevy's internal usage of these APIs. 3. Deprecate `Query::get_single` and friends, since we've moved their functionality to the nice names. 4. Add detailed advice on how to best handle these errors. Generally I like the diff here, although `get_single().unwrap()` in tests is a bit of a downgrade. ## Testing I've done a global search for `.single` to track down any missed deprecated usages. As to whether or not all the migrations were successful, that's what CI is for :) ## Future work ~~Rename `Query::get_single` and friends to `Query::single`!~~ ~~I've opted not to do this in this PR, and smear it across two releases in order to ease the migration. Successive deprecations are much easier to manage than the semantics and types shifting under your feet.~~ Cart has convinced me to change my mind on this; see https://github.com/bevyengine/bevy/pull/18082#discussion_r1974536085. ## Migration guide `Query::single`, `Query::single_mut` and their `QueryState` equivalents now return a `Result`. Generally, you'll want to: 1. Use Bevy 0.16's system error handling to return a `Result` using the `?` operator. 2. Use a `let else Ok(data)` block to early return if it's an expected failure. 3. Use `unwrap()` or `Ok` destructuring inside of tests. The old `Query::get_single` (etc) methods which did this have been deprecated. |
||
|
Carter Anderson
|
b73811d40e
|
Remove ChildOf::get and Deref impl (#18080)
# Objective There are currently three ways to access the parent stored on a ChildOf relationship: 1. `child_of.parent` (field accessor) 2. `child_of.get()` (get function) 3. `**child_of` (Deref impl) I will assert that we should only have one (the field accessor), and that the existence of the other implementations causes confusion and legibility issues. The deref approach is heinous, and `child_of.get()` is significantly less clear than `child_of.parent`. ## Solution Remove `impl Deref for ChildOf` and `ChildOf::get`. The one "downside" I'm seeing is that: ```rust entity.get::<ChildOf>().map(ChildOf::get) ``` Becomes this: ```rust entity.get::<ChildOf>().map(|c| c.parent) ``` I strongly believe that this is worth the increased clarity and consistency. I'm also not really a huge fan of the "pass function pointer to map" syntax. I think most people don't think this way about maps. They think in terms of a function that takes the item in the Option and returns the result of some action on it. ## Migration Guide ```rust // Before **child_of // After child_of.parent // Before child_of.get() // After child_of.parent // Before entity.get::<ChildOf>().map(ChildOf::get) // After entity.get::<ChildOf>().map(|c| c.parent) ``` |
||
|
Zachary Harrold
|
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> |
||
|
Carter Anderson
|
f3b2139e92
|
Only despawn scene entities still in the hierarchy (#17938)
Fixes #17883 # Objective + Solution When doing normal scene root entity despawns (which are notably now recursive), do not despawn instanced entities that are no longer in the hierarchy. (I would not classify this as a bug, but rather a behavior change) ## Migration Guide If you previously relied on scene entities no longer in the hierarchy being despawned when the scene root is despawned , use `SceneSpawner::despawn_instance()` instead. |
||
|
Alice Cecile
|
fcc77fe3d6
|
Allow users to register their own disabling components / default query filters (#17768)
# Objective Currently, default query filters, as added in #13120 / #17514 are hardcoded to only use a single query filter. This is limiting, as multiple distinct disabling components can serve important distinct roles. I ran into this limitation when experimenting with a workflow for prefabs, which don't represent the same state as "an entity which is temporarily nonfunctional". ## Solution 1. Change `DefaultQueryFilters` to store a SmallVec of ComponentId, rather than an Option. 2. Expose methods on `DefaultQueryFilters`, `World` and `App` to actually configure this. 3. While we're here, improve the docs, write some tests, make use of FromWorld and make some method names more descriptive. ## Follow-up I'm not convinced that supporting sparse set disabling components is useful, given the hit to iteration performance and runtime checks incurred. That's disjoint from this PR though, so I'm not doing it here. The existing warnings are fine for now. ## Testing I've added both a doc test and an mid-level unit test to verify that this works! |
||
|
François Mockers
|
7400e7adfd
|
Cleanup publish process (#17728)
# Objective - publish script copy the license files to all subcrates, meaning that all publish are dirty. this breaks git verification of crates - the order and list of crates to publish is manually maintained, leading to error. cargo 1.84 is more strict and the list is currently wrong ## Solution - duplicate all the licenses to all crates and remove the `--allow-dirty` flag - instead of a manual list of crates, get it from `cargo package --workspace` - remove the `--no-verify` flag to... verify more things? |
||
|
Carter Anderson
|
3c8fae2390
|
Improved Entity Mapping and Cloning (#17687)
Fixes #17535 Bevy's approach to handling "entity mapping" during spawning and cloning needs some work. The addition of [Relations](https://github.com/bevyengine/bevy/pull/17398) both [introduced a new "duplicate entities" bug when spawning scenes in the scene system](#17535) and made the weaknesses of the current mapping system exceedingly clear: 1. Entity mapping requires _a ton_ of boilerplate (implement or derive VisitEntities and VisitEntitesMut, then register / reflect MapEntities). Knowing the incantation is challenging and if you forget to do it in part or in whole, spawning subtly breaks. 2. Entity mapping a spawned component in scenes incurs unnecessary overhead: look up ReflectMapEntities, create a _brand new temporary instance_ of the component using FromReflect, map the entities in that instance, and then apply that on top of the actual component using reflection. We can do much better. Additionally, while our new [Entity cloning system](https://github.com/bevyengine/bevy/pull/16132) is already pretty great, it has some areas we can make better: * It doesn't expose semantic info about the clone (ex: ignore or "clone empty"), meaning we can't key off of that in places where it would be useful, such as scene spawning. Rather than duplicating this info across contexts, I think it makes more sense to add that info to the clone system, especially given that we'd like to use cloning code in some of our spawning scenarios. * EntityCloner is currently built in a way that prioritizes a single entity clone * EntityCloner's recursive cloning is built to be done "inside out" in a parallel context (queue commands that each have a clone of EntityCloner). By making EntityCloner the orchestrator of the clone we can remove internal arcs, improve the clarity of the code, make EntityCloner mutable again, and simplify the builder code. * EntityCloner does not currently take into account entity mapping. This is necessary to do true "bullet proof" cloning, would allow us to unify the per-component scene spawning and cloning UX, and ultimately would allow us to use EntityCloner in place of raw reflection for scenes like `Scene(World)` (which would give us a nice performance boost: fewer archetype moves, less reflection overhead). ## Solution ### Improved Entity Mapping First, components now have first-class "entity visiting and mapping" behavior: ```rust #[derive(Component, Reflect)] #[reflect(Component)] struct Inventory { size: usize, #[entities] items: Vec<Entity>, } ``` Any field with the `#[entities]` annotation will be viewable and mappable when cloning and spawning scenes. Compare that to what was required before! ```rust #[derive(Component, Reflect, VisitEntities, VisitEntitiesMut)] #[reflect(Component, MapEntities)] struct Inventory { #[visit_entities(ignore)] size: usize, items: Vec<Entity>, } ``` Additionally, for relationships `#[entities]` is implied, meaning this "just works" in scenes and cloning: ```rust #[derive(Component, Reflect)] #[relationship(relationship_target = Children)] #[reflect(Component)] struct ChildOf(pub Entity); ``` Note that Component _does not_ implement `VisitEntities` directly. Instead, it has `Component::visit_entities` and `Component::visit_entities_mut` methods. This is for a few reasons: 1. We cannot implement `VisitEntities for C: Component` because that would conflict with our impl of VisitEntities for anything that implements `IntoIterator<Item=Entity>`. Preserving that impl is more important from a UX perspective. 2. We should not implement `Component: VisitEntities` VisitEntities in the Component derive, as that would increase the burden of manual Component trait implementors. 3. Making VisitEntitiesMut directly callable for components would make it easy to invalidate invariants defined by a component author. By putting it in the `Component` impl, we can make it harder to call naturally / unavailable to autocomplete using `fn visit_entities_mut(this: &mut Self, ...)`. `ReflectComponent::apply_or_insert` is now `ReflectComponent::apply_or_insert_mapped`. By moving mapping inside this impl, we remove the need to go through the reflection system to do entity mapping, meaning we no longer need to create a clone of the target component, map the entities in that component, and patch those values on top. This will make spawning mapped entities _much_ faster (The default `Component::visit_entities_mut` impl is an inlined empty function, so it will incur no overhead for unmapped entities). ### The Bug Fix To solve #17535, spawning code now skips entities with the new `ComponentCloneBehavior::Ignore` and `ComponentCloneBehavior::RelationshipTarget` variants (note RelationshipTarget is a temporary "workaround" variant that allows scenes to skip these components. This is a temporary workaround that can be removed as these cases should _really_ be using EntityCloner logic, which should be done in a followup PR. When that is done, `ComponentCloneBehavior::RelationshipTarget` can be merged into the normal `ComponentCloneBehavior::Custom`). ### Improved Cloning * `Option<ComponentCloneHandler>` has been replaced by `ComponentCloneBehavior`, which encodes additional intent and context (ex: `Default`, `Ignore`, `Custom`, `RelationshipTarget` (this last one is temporary)). * Global per-world entity cloning configuration has been removed. This felt overly complicated, increased our API surface, and felt too generic. Each clone context can have different requirements (ex: what a user wants in a specific system, what a scene spawner wants, etc). I'd prefer to see how far context-specific EntityCloners get us first. * EntityCloner's internals have been reworked to remove Arcs and make it mutable. * EntityCloner is now directly stored on EntityClonerBuilder, simplifying the code somewhat * EntityCloner's "bundle scratch" pattern has been moved into the new BundleScratch type, improving its usability and making it usable in other contexts (such as future cross-world cloning code). Currently this is still private, but with some higher level safe APIs it could be used externally for making dynamic bundles * EntityCloner's recursive cloning behavior has been "externalized". It is now responsible for orchestrating recursive clones, meaning it no longer needs to be sharable/clone-able across threads / read-only. * EntityCloner now does entity mapping during clones, like scenes do. This gives behavior parity and also makes it more generically useful. * `RelatonshipTarget::RECURSIVE_SPAWN` is now `RelationshipTarget::LINKED_SPAWN`, and this field is used when cloning relationship targets to determine if cloning should happen recursively. The new `LINKED_SPAWN` term was picked to make it more generically applicable across spawning and cloning scenarios. ## Next Steps * I think we should adapt EntityCloner to support cross world cloning. I think this PR helps set the stage for that by making the internals slightly more generalized. We could have a CrossWorldEntityCloner that reuses a lot of this infrastructure. * Once we support cross world cloning, we should use EntityCloner to spawn `Scene(World)` scenes. This would yield significant performance benefits (no archetype moves, less reflection overhead). --------- Co-authored-by: eugineerd <70062110+eugineerd@users.noreply.github.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Zachary Harrold
|
642e016aef
|
Bump to uuid 1.13.1 and enable js on wasm targets (#17689)
# Objective - Fixes CI failure due to `uuid` 1.13 using the new version of `getrandom` which requires using a new API to work on Wasm. ## Solution - Based on [`uuid` 1.13 release notes](https://github.com/uuid-rs/uuid/releases/tag/1.13.0) I've enabled the `js` feature on `wasm32`. This will need to be revisited once #17499 is up for review - Updated minimum `uuid` version to 1.13.1, which fixes a separate issue with `target_feature = atomics` on `wasm`. ## Testing - `cargo check --target wasm32-unknown-unknown` |
||
|
NiseVoid
|
62285a47ba
|
Add simple Disabled marker (#17514)
# Objective We have default query filters now, but there is no first-party marker for entity disabling yet Fixes #17458 ## Solution Add the marker, cool recursive features and/or potential hook changes should be follow up work ## Testing Added a unit test to check that the new marker is enabled by default |
||
|
Zachary Harrold
|
9bc0ae33c3
|
Move hashbrown and foldhash out of bevy_utils (#17460)
# Objective - Contributes to #16877 ## Solution - Moved `hashbrown`, `foldhash`, and related types out of `bevy_utils` and into `bevy_platform_support` - Refactored the above to match the layout of these types in `std`. - Updated crates as required. ## Testing - CI --- ## Migration Guide - The following items were moved out of `bevy_utils` and into `bevy_platform_support::hash`: - `FixedState` - `DefaultHasher` - `RandomState` - `FixedHasher` - `Hashed` - `PassHash` - `PassHasher` - `NoOpHash` - The following items were moved out of `bevy_utils` and into `bevy_platform_support::collections`: - `HashMap` - `HashSet` - `bevy_utils::hashbrown` has been removed. Instead, import from `bevy_platform_support::collections` _or_ take a dependency on `hashbrown` directly. - `bevy_utils::Entry` has been removed. Instead, import from `bevy_platform_support::collections::hash_map` or `bevy_platform_support::collections::hash_set` as appropriate. - All of the above equally apply to `bevy::utils` and `bevy::platform_support`. ## Notes - I left `PreHashMap`, `PreHashMapExt`, and `TypeIdMap` in `bevy_utils` as they might be candidates for micro-crating. They can always be moved into `bevy_platform_support` at a later date if desired. |
||
|
Zachary Harrold
|
41e79ae826
|
Refactored ComponentHook Parameters into HookContext (#17503)
# Objective
- Make the function signature for `ComponentHook` less verbose
## Solution
- Refactored `Entity`, `ComponentId`, and `Option<&Location>` into a new
`HookContext` struct.
## Testing
- CI
---
## Migration Guide
Update the function signatures for your component hooks to only take 2
arguments, `world` and `context`. Note that because `HookContext` is
plain data with all members public, you can use de-structuring to
simplify migration.
```rust
// Before
fn my_hook(
mut world: DeferredWorld,
entity: Entity,
component_id: ComponentId,
) { ... }
// After
fn my_hook(
mut world: DeferredWorld,
HookContext { entity, component_id, caller }: HookContext,
) { ... }
```
Likewise, if you were discarding certain parameters, you can use `..` in
the de-structuring:
```rust
// Before
fn my_hook(
mut world: DeferredWorld,
entity: Entity,
_: ComponentId,
) { ... }
// After
fn my_hook(
mut world: DeferredWorld,
HookContext { entity, .. }: HookContext,
) { ... }
```
|
||
|
SpecificProtagonist
|
f32a6fb205
|
Track callsite for observers & hooks (#15607)
# Objective Fixes #14708 Also fixes some commands not updating tracked location. ## Solution `ObserverTrigger` has a new `caller` field with the `track_change_detection` feature; hooks take an additional caller parameter (which is `Some(…)` or `None` depending on the feature). ## Testing See the new tests in `src/observer/mod.rs` --- ## Showcase Observers now know from where they were triggered (if `track_change_detection` is enabled): ```rust world.observe(move |trigger: Trigger<OnAdd, Foo>| { println!("Added Foo from {}", trigger.caller()); }); ``` ## Migration - hooks now take an additional `Option<&'static Location>` argument --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Alice Cecile
|
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`. |
||
|
Carter Anderson
|
ba5e71f53d
|
Parent -> ChildOf (#17427)
Fixes #17412 ## Objective `Parent` uses the "has a X" naming convention. There is increasing sentiment that we should use the "is a X" naming convention for relationships (following #17398). This leaves `Children` as-is because there is prevailing sentiment that `Children` is clearer than `ParentOf` in many cases (especially when treating it like a collection). This renames `Parent` to `ChildOf`. This is just the implementation PR. To discuss the path forward, do so in #17412. ## Migration Guide - The `Parent` component has been renamed to `ChildOf`. |
||
|
Alice Cecile
|
5a9bc28502
|
Support non-Vec data structures in relations (#17447)
# Objective
The existing `RelationshipSourceCollection` uses `Vec` as the only
possible backing for our relationships. While a reasonable choice,
benchmarking use cases might reveal that a different data type is better
or faster.
For example:
- Not all relationships require a stable ordering between the
relationship sources (i.e. children). In cases where we a) have many
such relations and b) don't care about the ordering between them, a hash
set is likely a better datastructure than a `Vec`.
- The number of children-like entities may be small on average, and a
`smallvec` may be faster
## Solution
- Implement `RelationshipSourceCollection` for `EntityHashSet`, our
custom entity-optimized `HashSet`.
-~~Implement `DoubleEndedIterator` for `EntityHashSet` to make things
compile.~~
- This implementation was cursed and very surprising.
- Instead, by moving the iterator type on `RelationshipSourceCollection`
from an erased RPTIT to an explicit associated type we can add a trait
bound on the offending methods!
- Implement `RelationshipSourceCollection` for `SmallVec`
## Testing
I've added a pair of new tests to make sure this pattern compiles
successfully in practice!
## Migration Guide
`EntityHashSet` and `EntityHashMap` are no longer re-exported in
`bevy_ecs::entity` directly. If you were not using `bevy_ecs` / `bevy`'s
`prelude`, you can access them through their now-public modules,
`hash_set` and `hash_map` instead.
## Notes to reviewers
The `EntityHashSet::Iter` type needs to be public for this impl to be
allowed. I initially renamed it to something that wasn't ambiguous and
re-exported it, but as @Victoronz pointed out, that was somewhat
unidiomatic.
In
|
||
|
Carter Anderson
|
21f1e3045c
|
Relationships (non-fragmenting, one-to-many) (#17398)
This adds support for one-to-many non-fragmenting relationships (with planned paths for fragmenting and non-fragmenting many-to-many relationships). "Non-fragmenting" means that entities with the same relationship type, but different relationship targets, are not forced into separate tables (which would cause "table fragmentation"). Functionally, this fills a similar niche as the current Parent/Children system. The biggest differences are: 1. Relationships have simpler internals and significantly improved performance and UX. Commands and specialized APIs are no longer necessary to keep everything in sync. Just spawn entities with the relationship components you want and everything "just works". 2. Relationships are generalized. Bevy can provide additional built in relationships, and users can define their own. **REQUEST TO REVIEWERS**: _please don't leave top level comments and instead comment on specific lines of code. That way we can take advantage of threaded discussions. Also dont leave comments simply pointing out CI failures as I can read those just fine._ ## Built on top of what we have Relationships are implemented on top of the Bevy ECS features we already have: components, immutability, and hooks. This makes them immediately compatible with all of our existing (and future) APIs for querying, spawning, removing, scenes, reflection, etc. The fewer specialized APIs we need to build, maintain, and teach, the better. ## Why focus on one-to-many non-fragmenting first? 1. This allows us to improve Parent/Children relationships immediately, in a way that is reasonably uncontroversial. Switching our hierarchy to fragmenting relationships would have significant performance implications. ~~Flecs is heavily considering a switch to non-fragmenting relations after careful considerations of the performance tradeoffs.~~ _(Correction from @SanderMertens: Flecs is implementing non-fragmenting storage specialized for asset hierarchies, where asset hierarchies are many instances of small trees that have a well defined structure)_ 2. Adding generalized one-to-many relationships is currently a priority for the [Next Generation Scene / UI effort](https://github.com/bevyengine/bevy/discussions/14437). Specifically, we're interested in building reactions and observers on top. ## The changes This PR does the following: 1. Adds a generic one-to-many Relationship system 3. Ports the existing Parent/Children system to Relationships, which now lives in `bevy_ecs::hierarchy`. The old `bevy_hierarchy` crate has been removed. 4. Adds on_despawn component hooks 5. Relationships can opt-in to "despawn descendants" behavior, meaning that the entire relationship hierarchy is despawned when `entity.despawn()` is called. The built in Parent/Children hierarchies enable this behavior, and `entity.despawn_recursive()` has been removed. 6. `world.spawn` now applies commands after spawning. This ensures that relationship bookkeeping happens immediately and removes the need to manually flush. This is in line with the equivalent behaviors recently added to the other APIs (ex: insert). 7. Removes the ValidParentCheckPlugin (system-driven / poll based) in favor of a `validate_parent_has_component` hook. ## Using Relationships The `Relationship` trait looks like this: ```rust pub trait Relationship: Component + Sized { type RelationshipSources: RelationshipSources<Relationship = Self>; fn get(&self) -> Entity; fn from(entity: Entity) -> Self; } ``` A relationship is a component that: 1. Is a simple wrapper over a "target" Entity. 2. Has a corresponding `RelationshipSources` component, which is a simple wrapper over a collection of entities. Every "target entity" targeted by a "source entity" with a `Relationship` has a `RelationshipSources` component, which contains every "source entity" that targets it. For example, the `Parent` component (as it currently exists in Bevy) is the `Relationship` component and the entity containing the Parent is the "source entity". The entity _inside_ the `Parent(Entity)` component is the "target entity". And that target entity has a `Children` component (which implements `RelationshipSources`). In practice, the Parent/Children relationship looks like this: ```rust #[derive(Relationship)] #[relationship(relationship_sources = Children)] pub struct Parent(pub Entity); #[derive(RelationshipSources)] #[relationship_sources(relationship = Parent)] pub struct Children(Vec<Entity>); ``` The Relationship and RelationshipSources derives automatically implement Component with the relevant configuration (namely, the hooks necessary to keep everything in sync). The most direct way to add relationships is to spawn entities with relationship components: ```rust let a = world.spawn_empty().id(); let b = world.spawn(Parent(a)).id(); assert_eq!(world.entity(a).get::<Children>().unwrap(), &[b]); ``` There are also convenience APIs for spawning more than one entity with the same relationship: ```rust world.spawn_empty().with_related::<Children>(|s| { s.spawn_empty(); s.spawn_empty(); }) ``` The existing `with_children` API is now a simpler wrapper over `with_related`. This makes this change largely non-breaking for existing spawn patterns. ```rust world.spawn_empty().with_children(|s| { s.spawn_empty(); s.spawn_empty(); }) ``` There are also other relationship APIs, such as `add_related` and `despawn_related`. ## Automatic recursive despawn via the new on_despawn hook `RelationshipSources` can opt-in to "despawn descendants" behavior, which will despawn all related entities in the relationship hierarchy: ```rust #[derive(RelationshipSources)] #[relationship_sources(relationship = Parent, despawn_descendants)] pub struct Children(Vec<Entity>); ``` This means that `entity.despawn_recursive()` is no longer required. Instead, just use `entity.despawn()` and the relevant related entities will also be despawned. To despawn an entity _without_ despawning its parent/child descendants, you should remove the `Children` component first, which will also remove the related `Parent` components: ```rust entity .remove::<Children>() .despawn() ``` This builds on the on_despawn hook introduced in this PR, which is fired when an entity is despawned (before other hooks). ## Relationships are the source of truth `Relationship` is the _single_ source of truth component. `RelationshipSources` is merely a reflection of what all the `Relationship` components say. By embracing this, we are able to significantly improve the performance of the system as a whole. We can rely on component lifecycles to protect us against duplicates, rather than needing to scan at runtime to ensure entities don't already exist (which results in quadratic runtime). A single source of truth gives us constant-time inserts. This does mean that we cannot directly spawn populated `Children` components (or directly add or remove entities from those components). I personally think this is a worthwhile tradeoff, both because it makes the performance much better _and_ because it means theres exactly one way to do things (which is a philosophy we try to employ for Bevy APIs). As an aside: treating both sides of the relationship as "equivalent source of truth relations" does enable building simple and flexible many-to-many relationships. But this introduces an _inherent_ need to scan (or hash) to protect against duplicates. [`evergreen_relations`](https://github.com/EvergreenNest/evergreen_relations) has a very nice implementation of the "symmetrical many-to-many" approach. Unfortunately I think the performance issues inherent to that approach make it a poor choice for Bevy's default relationship system. ## Followup Work * Discuss renaming `Parent` to `ChildOf`. I refrained from doing that in this PR to keep the diff reasonable, but I'm personally biased toward this change (and using that naming pattern generally for relationships). * [Improved spawning ergonomics](https://github.com/bevyengine/bevy/discussions/16920) * Consider adding relationship observers/triggers for "relationship targets" whenever a source is added or removed. This would replace the current "hierarchy events" system, which is unused upstream but may have existing users downstream. I think triggers are the better fit for this than a buffered event queue, and would prefer not to add that back. * Fragmenting relations: My current idea hinges on the introduction of "value components" (aka: components whose type _and_ value determines their ComponentId, via something like Hashing / PartialEq). By labeling a Relationship component such as `ChildOf(Entity)` as a "value component", `ChildOf(e1)` and `ChildOf(e2)` would be considered "different components". This makes the transition between fragmenting and non-fragmenting a single flag, and everything else continues to work as expected. * Many-to-many support * Non-fragmenting: We can expand Relationship to be a list of entities instead of a single entity. I have largely already written the code for this. * Fragmenting: With the "value component" impl mentioned above, we get many-to-many support "for free", as it would allow inserting multiple copies of a Relationship component with different target entities. Fixes #3742 (If this PR is merged, I think we should open more targeted followup issues for the work above, with a fresh tracking issue free of the large amount of less-directed historical context) Fixes #17301 Fixes #12235 Fixes #15299 Fixes #15308 ## Migration Guide * Replace `ChildBuilder` with `ChildSpawnerCommands`. * Replace calls to `.set_parent(parent_id)` with `.insert(Parent(parent_id))`. * Replace calls to `.replace_children()` with `.remove::<Children>()` followed by `.add_children()`. Note that you'll need to manually despawn any children that are not carried over. * Replace calls to `.despawn_recursive()` with `.despawn()`. * Replace calls to `.despawn_descendants()` with `.despawn_related::<Children>()`. * If you have any calls to `.despawn()` which depend on the children being preserved, you'll need to remove the `Children` component first. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
MichiRecRoom
|
26bb0b40d2
|
Move #![warn(clippy::allow_attributes, clippy::allow_attributes_without_reason)] to the workspace Cargo.toml (#17374)
# Objective Fixes https://github.com/bevyengine/bevy/issues/17111 ## Solution Move `#![warn(clippy::allow_attributes, clippy::allow_attributes_without_reason)]` to the workspace `Cargo.toml` ## Testing Lots of CI testing, and local testing too. --------- Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com> |
||
|
MichiRecRoom
|
447108b2a4
|
Downgrade clippy::allow_attributes and clippy::allow_attributes_without_reason to warn (#17320)
# Objective I realized that setting these to `deny` may have been a little aggressive - especially since we upgrade warnings to denies in CI. ## Solution Downgrades these lints to `warn`, so that compiles can work locally. CI will still treat these as denies. |
||
|
Rob Parrett
|
b77e3ef33a
|
Fix a few typos (#17292)
# Objective Stumbled upon a `from <-> form` transposition while reviewing a PR, thought it was interesting, and went down a bit of a rabbit hole. ## Solution Fix em |
||
|
MichiRecRoom
|
6650931adf
|
bevy_scene: Apply #![deny(clippy::allow_attributes, clippy::allow_attributes_without_reason)] (#17278)
# Objective - https://github.com/bevyengine/bevy/issues/17111 ## Solution Set the `clippy::allow_attributes` and `clippy::allow_attributes_without_reason` lints to `deny`, and bring `bevy_scene` in line with the new restrictions. ## Testing `cargo clippy --tests --all-features --package bevy_scene` was run, and no errors were encountered. |