Commit Graph

8707 Commits

Author SHA1 Message Date
noxmore
18f543474d
Derive Clone for ImageLoaderSettings and ImageFormatSetting (#18335)
# Objective

I was setting up an asset loader that passes settings through to
`ImageLoader`, and i have to clone the settings to achieve this.

## Solution

Derive `Clone` for `ImageLoaderSettings` and `ImageFormatSetting`.

## Testing

Full CI passed.
2025-03-16 18:57:25 +00:00
François Mockers
70514229da
Build performance advice (#18339)
# Objective

- Fixes #18331 

## Solution

- Add some info on other tools that `cargo timings`
2025-03-16 11:37:41 +00:00
Patrick Walton
528e68f5bb
Pad out binding arrays to their full length if partially bound binding arrays aren't supported. (#18126)
I mistakenly thought that with the wgpu upgrade we'd have
`PARTIALLY_BOUND_BINDING_ARRAY` everywhere. Unfortunately this is not
the case. This PR adds a workaround back in.

Closes #18098.
2025-03-16 10:57:33 +00:00
JMS55
195a74afad
Add missing system ordering constraint to prepare_lights (#18308)
Fix https://github.com/bevyengine/bevy/issues/18094.
2025-03-15 22:57:52 +00:00
JMS55
d41de7f3df
Fix MainTransparentPass2dNode attachment ordering (#18306)
Fix https://github.com/bevyengine/bevy/issues/17763.

Attachment info needs to be created outside of the command encoding
task, as it needs to be part of the serial node runners in order to get
the ordering correct.
2025-03-15 17:31:52 +00:00
Robert Swain
bc7416aa22
Use 4-byte LightmapSlabIndex for batching instead of 16-byte AssetId<Image> (#18326)
Less data accessed and compared gives better batching performance.

# Objective

- Use a smaller id to represent the lightmap in batch data to enable a
faster implementation of draw streams.
- Improve batching performance for 3D sorted render phases.

## Solution

- 3D batching can use `LightmapSlabIndex` (a `NonMaxU32` which is 4
bytes) instead of the lightmap `AssetId<Image>` (an enum where the
largest variant is a 16-byte UUID) to discern the ability to batch.

## Testing

Tested main (yellow) vs this PR (red) on an M4 Max using the
`many_cubes` example with `WGPU_SETTINGS_PRIO=webgl2` to avoid
GPU-preprocessing, and modifying the materials in `many_cubes` to have
`AlphaMode::Blend` so that they would rely on the less efficient sorted
render phase batching.
<img width="1500" alt="Screenshot_2025-03-15_at_12 17 21"
src="https://github.com/user-attachments/assets/14709bd3-6d06-40fb-aa51-e1d2d606ebe3"
/>
A 44.75us or 7.5% reduction in median execution time of the batch and
prepare sorted render phase system for the `Transparent3d` phase
(handling 160k cubes).

---

## Migration Guide

- Changed: `RenderLightmap::new()` no longer takes an `AssetId<Image>`
argument for the asset id of the lightmap image.
2025-03-15 14:16:32 +00:00
Vic
b462f47864
add Entity default to the entity set wrappers (#18319)
# Objective

Installment of the #16547 series.

The vast majority of uses for these types will be the `Entity` case, so
it makes sense for it to be the default.

## Solution

`UniqueEntityVec`, `UniqueEntitySlice`, `UniqueEntityArray` and their
helper iterator aliases now have `Entity` as a default.

Unfortunately, this means the the `T` parameter for `UniqueEntityArray`
now has to be ordered after the `N` constant, which breaks the
consistency to `[T; N]`.
Same with about a dozen iterator aliases that take some `P`/`F`
predicate/function parameter.
This should however be an ergonomic improvement in most cases, so we'll
just have to live with this inconsistency.

## Migration Guide

Switch type parameter order for the relevant wrapper types/aliases.
2025-03-15 01:51:39 +00:00
JMS55
cb3e6a88dc
Fix shadow_biases example (#18303)
Fix moire artifacts in https://github.com/bevyengine/bevy/issues/16635.

Default directional light biases are overkill but it's fine.
2025-03-14 19:50:49 +00:00
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`
2025-03-14 19:33:57 +00:00
andriyDev
adbb53b87f
Return an error when direct-nested-loading a subasset (#18213)
# Objective

- Prevents #18291.
- Previously, attempting to direct-nested-load a subasset would return
the root of the nested-loaded asset. This is most problematic when doing
direct-nested-**untyped**-loads of subassets, where you may not even
realize you're dealing with the entirely wrong asset (at least with
typed loads, *most of the time* the root asset has a different type from
the subasset, and so at least you'd get an error that the types don't
match).

## Solution

- We now return an error when doing these kinds of loads.

Note an alternative would be to "solve" this problem, by just looking up
the appropriate subasset after doing the nested load. However there's
two problems: 1) we don't know which subassets of the root asset are
necessary for the subasset we are looking up (so any handles in that
subasset may never get registered), 2) a solution will likely hamper
attempts to resolve #18010. AFAICT, no one has complained about this
issue, so it doesn't seem critical to fix this for now.

## Testing

- Added a test to ensure this returns an error. I also checked that the
error before this was just a mismatched type error, meaning it was
trying to pass off the root asset type `CoolText` as the subasset type
`SubText` (which would have worked had I not been using typed loads).
2025-03-13 16:36:18 +00:00
Lynn
f245490e4a
implement Bounded2d for ConvexPolygon (#18286)
# Objective

- Implement `Bounded2d` for `ConvexPolygon`
2025-03-13 16:35:52 +00:00
krunchington
2c22bc12a0
Update computed_states example to use children macro (#18290)
# Objective

Contributes to #18238 
Updates the `computed_states`, example to use the `children!` macro.
Note that this example requires `--features bevy_dev_tools` to run

## Solution

Updates examples to use the Improved Spawning API merged in
https://github.com/bevyengine/bevy/pull/17521

## Testing

- Did you test these changes? If so, how?
- Opened the examples before and after and verified the same behavior
was observed. I did this on Ubuntu 24.04.2 LTS using `--features
wayland`.
- Are there any parts that need more testing?
- Other OS's and features can't hurt, but this is such a small change it
shouldn't be a problem.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
  - Run the examples yourself with and without these changes.
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
  - see above

---

## Showcase

n/a

## Migration Guide

n/a
2025-03-13 16:35:32 +00:00
Martín Maita
d70c469483
Update ui_test requirement from 0.23.0 to 0.29.1 (#18289)
# Objective

- Fixes #18223.

## Solution

- Updated ui_test requirement from 0.23.0 to 0.29.1.
- Updated code to use the new APIs.

## Testing

- Ran CI locally.

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2025-03-13 16:34:33 +00:00
Zachary Harrold
6299e3de3b
Add examples/helpers/* as library examples (#18288)
# Objective

Some of Bevy's examples contain boilerplate which is split out into the
`helpers` folder. This allows examples to have access to common
functionality without building into Bevy directly. However, these
helpers are themselves quite high-quality code, and we do intend for
users to read them and even use them. But, we don't list them in the
examples document, and they aren't explicitly checked in CI, only
transitively through examples which import them.

## Solution

- Added `camera_controller` and `widgets` as library examples.

## Testing

- CI

---

## Notes

- Library examples are identical to any other example, just with
`crate-type = ["lib"]` in the `Cargo.toml`. Since they are marked as
libraries, they don't require a `main` function but do require public
items to be documented.
- Library examples opens the possibility of creating examples which
don't need to be actual runnable applications. This may be more
appropriate for certain ECS examples, and allows for adding helpers
which (currently) don't have an example that needs them without them
going stale.
- I learned about this as a concept during research for `no_std`
examples, but believe it has value for Bevy outside that specific niche.

---------

Co-authored-by: mgi388 <135186256+mgi388@users.noreply.github.com>
Co-authored-by: Carter Weinberg <weinbergcarter@gmail.com>
2025-03-13 16:34:16 +00:00
krunchington
4d47de8ad8
Update custom_transitions and sub_states examples to use children macro (#18292)
# Objective

Contributes to #18238 
Updates the `custom_transitions` and `sub_states` examples to use the
`children!` macro.

## Solution

Updates examples to use the Improved Spawning API merged in
https://github.com/bevyengine/bevy/pull/17521

## Testing

- Did you test these changes? If so, how?
- Opened the examples before and after and verified the same behavior
was observed. I did this on Ubuntu 24.04.2 LTS using `--features
wayland`.
- Are there any parts that need more testing?
- Other OS's and features can't hurt, but this is such a small change it
shouldn't be a problem.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
  - Run the examples yourself with and without these changes.
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
  - see above

---

## Showcase

n/a

## Migration Guide

n/a
2025-03-13 03:11:13 +00:00
Alice Cecile
ab0e3f8714
Small cleanup for ECS error handling (#18280)
# Objective

While poking at https://github.com/bevyengine/bevy/issues/17272, I
noticed a few small things to clean up.

## Solution

- Improve the docs
- ~~move `SystemErrorContext` out of the `handler.rs` module: it's not
an error handler~~
2025-03-13 00:13:02 +00:00
krunchington
25103df301
Update gamepad_viewer to use children macro (#18282)
# Objective

Contributes to #18238 
Updates the `gamepad_viewer`, example to use the `children!` macro.  

## Solution

Updates examples to use the Improved Spawning API merged in
https://github.com/bevyengine/bevy/pull/17521

## Testing

- Did you test these changes? If so, how?
- Opened the examples before and after and verified the same behavior
was observed. I did this on Ubuntu 24.04.2 LTS using `--features
wayland`.
- Are there any parts that need more testing?
- Other OS's and features can't hurt, but this is such a small change it
shouldn't be a problem.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
  - Run the examples yourself with and without these changes.
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
  - see above

---

## Showcase

n/a

## Migration Guide

n/a
2025-03-12 23:52:39 +00:00
MevLyshkin
8f38ea352e
RPC Discover endpoint with basic informations (#18068)
# Objective

It does not resolves issue for full support for OpenRPC for
`bevy_remote`, but it is a first step in that direction. Connected to
the #16744 issue.

## Solution

- Adds `rpc.discover` endpoint to the bevy_remote which follows
https://spec.open-rpc.org/#openrpc-document For now in methods array
only the name, which is the endpoint address is populated.
- Moves json_schema structs into new module inside `bevy_remote`. 

## Testing

Tested the commands by running the BRP sample( cargo run --example
server --features="bevy_remote") and with these curl command:

```sh
curl -X POST -d '{ "jsonrpc": "2.0", "id": 1, "method": "rpc.discover"}' 127.0.0.1:15702 | jq .
```
The output is: 
```json
{
  "jsonrpc": "2.0",
  "id": 1,
  "result": {
    "info": {
      "title": "Bevy Remote Protocol",
      "version": "0.16.0-dev"
    },
    "methods": [
      {
        "name": "bevy/mutate_component",
        "params": []
      },
      {
        "name": "bevy/insert",
        "params": []
      },
      {
        "name": "bevy/get",
        "params": []
      },
      {
        "name": "bevy/spawn",
        "params": []
      },
      {
        "name": "bevy/get+watch",
        "params": []
      },
      {
        "name": "bevy/destroy",
        "params": []
      },
      {
        "name": "bevy/list",
        "params": []
      },
      {
        "name": "bevy/mutate_resource",
        "params": []
      },
      {
        "name": "bevy/reparent",
        "params": []
      },
      {
        "name": "bevy/registry/schema",
        "params": []
      },
      {
        "name": "bevy/get_resource",
        "params": []
      },
      {
        "name": "bevy/query",
        "params": []
      },
      {
        "name": "bevy/remove_resource",
        "params": []
      },
      {
        "name": "rpc.discover",
        "params": []
      },
      {
        "name": "bevy/insert_resource",
        "params": []
      },
      {
        "name": "bevy/list_resources",
        "params": []
      },
      {
        "name": "bevy/remove",
        "params": []
      },
      {
        "name": "bevy/list+watch",
        "params": []
      }
    ],
    "openrpc": "1.3.2",
    "servers": [
      {
        "name": "Server",
        "url": "127.0.0.1:15702"
      }
    ]
  }
}
```

---------

Co-authored-by: Viktor Gustavsson <villor94@gmail.com>
2025-03-12 23:32:06 +00:00
Matty Weatherley
84463e60e1
Implement Serialize/Deserialize/PartialEq for bounding primitives (#18281)
# Objective

Probably just because of an oversight, bounding primitives like `Aabb3d`
did not implement `Serialize`/`Deserialize` with the `serialize` feature
enabled, so the goal of this PR is to fill the gap.

## Solution

Derive it conditionally, just like we do for everything else. Also added
in `PartialEq`, just because I touched the files.

## Testing

Compiled with different feature combinations.
2025-03-12 21:38:29 +00:00
Zachary Harrold
dc7cb2a3e1
Switch to ImDocument in BevyManifest (#18272)
# Objective

When reviewing #18263, I noticed that `BevyManifest` internally stores a
`DocumentMut`, a mutable TOML document, instead of an `ImDocument`, an
immutable one. The process of creating a `DocumentMut` first involves
creating a `ImDocument` and then cloning all the referenced spans of
text into their own allocations (internally referred to as `despan` in
`toml_edit`). As such, using a `DocumentMut` without mutation is
strictly additional overhead.

In addition, I noticed that the filesystem operations associated with
reading a manifest and parsing it were written to be completed _while_ a
write-lock was held on `MANIFESTS`. This likely doesn't translate into a
performance or deadlock issue as the manifest files are generally small
and can be read quickly, but it is generally considered a bad practice.

## Solution

- Switched to `ImDocument<Box<str>>` instead of `DocumentMut`
- Re-ordered operations in `BevyManifest::shared` to minimise time spent
holding open the write-lock on `MANIFESTS`

## Testing

- CI

---

## Notes

I wasn't able to measure a meaningful performance difference with this
PR, so this is purely a code quality change and not one for performance.

---------

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2025-03-12 20:15:39 +00:00
Rob Parrett
81c0900cf2
Upgrade to cosmic-text 0.13 (#18239)
# Objective

Upgrade to `cosmic-text` 0.13

https://github.com/pop-os/cosmic-text/releases

This should include some performance improvements for layout and system
font loading.

## Solution

Bump version, fix the one changed API.

## Testing

Tested some examples locally, will invoke the example runner.

## Layout Perf

||main fps|cosmic-13 fps|
|-|-|-|
|many_buttons --recompute-text --no-borders|6.79|9.42 🟩 +38.7%|
|many_text2d --no-frustum-culling --recompute|3.19|4.28 🟩 +34.0%|
|many_glyphs --recompute-text|7.09|11.17 🟩 +57.6%|
|text_pipeline |140.15|139.90  -0.2%|

## System Font Loading Perf

I tested on macOS somewhat lazily by adding the following system to the
`system_fonts` example from #16365.

<details>
<summary>Expand code</summary>

```rust
fn exit_on_load(
    mut reader: EventReader<bevy::text::SystemFontsAvailable>,
    mut writer: EventWriter<AppExit>,
) {
    for _evt in reader.read() {
        writer.write(AppExit::Success);
    }
}
```
</details>

And running `hyperfine 'cargo run --release --example system_fonts
--features=system_font'`.

The results were nearly identical with and without this PR cherry-picked
there.

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2025-03-12 18:03:45 +00:00
NiseVoid
5d80ac3ded
Add derive Default to Disabled (#18275)
# Objective

- `#[require(Disabled)]` doesn't work as you'd expect

## Solution

- `#[derive(Default)]`
2025-03-12 17:57:20 +00:00
ickshonpe
26ea38e4a6
Remove the entity index from the UI phase's sort key (#18273)
# Objective

The sort key for the transparent UI phase is a (float32, u32) pair
consisting of the stack index and the render entity's index.
I guess the render entity index was intended to break ties but it's not
needed as the sort is stable. It also assumes the indices of the render
entities are generated sequentially, which isn't guaranteed.

Fixes the issues with the text wrap example seen in #18266

## Solution

Change the sort key to just use the stack index alone.
2025-03-12 17:11:02 +00:00
krunchington
ec822c8c3b
Update breakout example's stepping plugin to use children (#18271)
# Objective

Contributes to #18238 
Updates the `SteppingPlugin` of the `breakout` example to use the
`children!` macro. Note that in order to test this usage you must use
`--features bevy_debug_stepping` and hit the back-tick key to enable
stepping mode to see the proper text spans rendered.

## Solution

Updates examples to use the Improved Spawning API merged in
https://github.com/bevyengine/bevy/pull/17521

## Testing

- Did you test these changes? If so, how?
- Opened the examples before and after and verified the same behavior
was observed. I did this on Ubuntu 24.04.2 LTS using `--features
wayland`.
- Are there any parts that need more testing?
- Other OS's and features can't hurt, but this is such a small change it
shouldn't be a problem.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
  - Run the examples yourself with and without these changes.
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
  - see above

---

## Showcase

n/a

## Migration Guide

n/a
2025-03-12 04:51:59 +00:00
krunchington
a33161cf5b
Update render_primitives example for children! macro (#18268)
# Objective

Contributes to #18238 
Updates the `render_primitives` example to use the `children!` macro.  

## Solution

Updates examples to use the Improved Spawning API merged in
https://github.com/bevyengine/bevy/pull/17521

## Testing

- Did you test these changes? If so, how?
- Opened the examples before and after and verified the same behavior
was observed. I did this on Ubuntu 24.04.2 LTS using `--features
wayland`.
- Are there any parts that need more testing?
- Other OS's and features can't hurt, but this is such a small change it
shouldn't be a problem.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
  - Run the examples yourself with and without these changes.
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
  - see above

---

## Showcase

n/a

## Migration Guide

n/a
2025-03-12 04:49:07 +00:00
Carter Anderson
e21dfe81ce
Internalize BevyManifest logic. Switch to RwLock (#18263)
# Objective

Fixes #18103

#17330 introduced a significant compile time performance regression
(affects normal builds, clippy, and Rust Analyzer). While it did fix the
type-resolution bug (and the general approach there is still our best
known solution to the problem that doesn't involve [significant
maintenance
overhead](https://github.com/bevyengine/bevy/issues/18103#issuecomment-2702724676)),
the changes had a couple of issues:

1. It used a Mutex, which poses a significant threat to parallelization.
2. It externalized existing, relatively simple, performance critical
Bevy code to a crate outside of our control. I am not comfortable doing
that for cases like this. Going forward @bevyengine/maintainer-team
should be much stricter about this.
3. There were a number of other areas that introduced complexity and
overhead that I consider unnecessary for our use case. On a case by case
basis, if we encounter a need for more capabilities we can add them (and
weigh them against the cost of doing so).

## Solution

1. I moved us back to our original code as a baseline
2. I selectively ported over the minimal changes required to fix the
type resolution bug
3. I swapped `Mutex<BTreeMap<PathBuf, &'static Mutex<CargoManifest>>>`
for `RwLock<BTreeMap<PathBuf, CargoManifest>>`. Note that I used the
`parking_lot` RwLock because it has a mapping API that enables us to
return mapped guards.
2025-03-12 00:46:01 +00:00
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>
2025-03-12 00:12:30 +00:00
krunchington
f1331069e7
Implement SpawnableList for Vec<Bundle> (#18259)
# Objective

In updating examples to use the Improved Spawning API I got tripped up
on being able to spawn children with a Vec. I eventually figured out
that I could use `Children::spawn(SpawnIter(my_vec.into_iter()))` but
thought there might be a more ergonomic way to approach it. After
tinkering with it for a while I came up with the implementation here. It
allows authors to use `Children::spawn(my_vec)` as an equivalent
implementation.

## Solution

- Implements `<R: Relationship, B: Bundle SpawnableList<R> for Vec<B>`
- uses `alloc::vec::Vec` for compatibility with `no_std` (`std::Vec`
also inherits implementations against the `alloc::vec::Vec` because std
is a re-export of the alloc struct, thanks @bushrat011899 for the info
on this!)

## Testing

- Did you test these changes? If so, how?
- Opened the examples before and after and verified the same behavior
was observed. I did this on Ubuntu 24.04.2 LTS using `--features
wayland`.
- Are there any parts that need more testing?
- Other OS's and features can't hurt, but this is such a small change it
shouldn't be a problem.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
  - Run the examples yourself with and without these changes.
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
  - see above

## Showcase

n/a

## Migration Guide

- Optional: you may use the new API to spawn `Vec`s of `Bundle` instead
of using the `SpawnIter` approach.
2025-03-11 20:32:37 +00:00
Peepo-Juice
949d7811cd
Sync up the Derive of DragEntry to match the other events (#18220)
# Objective
Add `#[derive(Clone, PartialEq, Debug, Reflect)]` to DragEntry so it
matches the other picking events.

## Solution
Copy/paste (RIP Larry Tesler)

## Testing
Just ran cargo check. I don't believe this should break anything because
I did not remove any derives it had before.

---
2025-03-11 19:58:20 +00:00
Cyrill Schenkel
8570af1d96
Add print_stdout and print_stderr lints (#17446) (#18233)
# Objective

- Prevent usage of `println!`, `eprintln!` and the like because they
require `std`
- Fixes #17446

## Solution

- Enable the `print_stdout` and `print_stderr` clippy lints
- Replace all `println!` and `eprintln!` occurrences with `log::*` where
applicable or alternatively ignore the warnings

## Testing

- Run `cargo clippy --workspace` to ensure that there are no warnings
relating to printing to `stdout` or `stderr`
2025-03-11 19:35:48 +00:00
Brezak
4f6241178f
Remove superfluous spaces in Transform documentation (#18244)
# Objective

Closes #18228

## Solution

`hx
.\crates\bevy_transform\src\components\transform.rs<enter>40ggllldd:wq<enter>`

## Testing

`cargo +beta clippy --all-features`
2025-03-11 11:25:12 +00:00
Benjamin Brienen
c3ff6d4136
Fix non-crate typos (#18219)
# Objective

Correct spelling

## Solution

Fix typos, specifically ones that I found in folders other than /crates

## Testing

CI

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2025-03-11 06:17:48 +00:00
Gino Valente
f5210c54d2
bevy_reflect: Reflection-based cloning (#13432)
# Objective

Using `Reflect::clone_value` can be somewhat confusing to those
unfamiliar with how Bevy's reflection crate works. For example take the
following code:

```rust
let value: usize = 123;
let clone: Box<dyn Reflect> = value.clone_value();
```

What can we expect to be the underlying type of `clone`? If you guessed
`usize`, then you're correct! Let's try another:

```rust
#[derive(Reflect, Clone)]
struct Foo(usize);

let value: Foo = Foo(123);
let clone: Box<dyn Reflect> = value.clone_value();
```

What about this code? What is the underlying type of `clone`? If you
guessed `Foo`, unfortunately you'd be wrong. It's actually
`DynamicStruct`.

It's not obvious that the generated `Reflect` impl actually calls
`Struct::clone_dynamic` under the hood, which always returns
`DynamicStruct`.

There are already some efforts to make this a bit more apparent to the
end-user: #7207 changes the signature of `Reflect::clone_value` to
instead return `Box<dyn PartialReflect>`, signaling that we're
potentially returning a dynamic type.

But why _can't_ we return `Foo`?

`Foo` can obviously be cloned— in fact, we already derived `Clone` on
it. But even without the derive, this seems like something `Reflect`
should be able to handle. Almost all types that implement `Reflect`
either contain no data (trivially clonable), they contain a
`#[reflect_value]` type (which, by definition, must implement `Clone`),
or they contain another `Reflect` type (which recursively fall into one
of these three categories).

This PR aims to enable true reflection-based cloning where you get back
exactly the type that you think you do.

## Solution

Add a `Reflect::reflect_clone` method which returns `Result<Box<dyn
Reflect>, ReflectCloneError>`, where the `Box<dyn Reflect>` is
guaranteed to be the same type as `Self`.

```rust
#[derive(Reflect)]
struct Foo(usize);

let value: Foo = Foo(123);
let clone: Box<dyn Reflect> = value.reflect_clone().unwrap();
assert!(clone.is::<Foo>());
```

Notice that we didn't even need to derive `Clone` for this to work: it's
entirely powered via reflection!

Under the hood, the macro generates something like this:

```rust
fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> {
    Ok(Box::new(Self {
        // The `reflect_clone` impl for `usize` just makes use of its `Clone` impl
        0: Reflect::reflect_clone(&self.0)?.take().map_err(/* ... */)?,
    }))
}
```

If we did derive `Clone`, we can tell `Reflect` to rely on that instead:

```rust
#[derive(Reflect, Clone)]
#[reflect(Clone)]
struct Foo(usize);
```

<details>
<summary>Generated Code</summary>

```rust
fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> {
    Ok(Box::new(Clone::clone(self)))
}
```

</details>

Or, we can specify our own cloning function:

```rust
#[derive(Reflect)]
#[reflect(Clone(incremental_clone))]
struct Foo(usize);

fn incremental_clone(value: &usize) -> usize {
  *value + 1
}
```

<details>
<summary>Generated Code</summary>

```rust
fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> {
    Ok(Box::new(incremental_clone(self)))
}
```

</details>

Similarly, we can specify how fields should be cloned. This is important
for fields that are `#[reflect(ignore)]`'d as we otherwise have no way
to know how they should be cloned.

```rust
#[derive(Reflect)]
struct Foo {
 #[reflect(ignore, clone)]
  bar: usize,
  #[reflect(ignore, clone = "incremental_clone")]
  baz: usize,
}

fn incremental_clone(value: &usize) -> usize {
  *value + 1
}
```

<details>
<summary>Generated Code</summary>

```rust
fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> {
    Ok(Box::new(Self {
        bar: Clone::clone(&self.bar),
        baz: incremental_clone(&self.baz),
    }))
}
```

</details>

If we don't supply a `clone` attribute for an ignored field, then the
method will automatically return
`Err(ReflectCloneError::FieldNotClonable {/* ... */})`.

`Err` values "bubble up" to the caller. So if `Foo` contains `Bar` and
the `reflect_clone` method for `Bar` returns `Err`, then the
`reflect_clone` method for `Foo` also returns `Err`.

### Attribute Syntax

You might have noticed the differing syntax between the container
attribute and the field attribute.

This was purely done for consistency with the current attributes. There
are PRs aimed at improving this. #7317 aims at making the
"special-cased" attributes more in line with the field attributes
syntactically. And #9323 aims at moving away from the stringified paths
in favor of just raw function paths.

### Compatibility with Unique Reflect

This PR was designed with Unique Reflect (#7207) in mind. This method
actually wouldn't change that much (if at all) under Unique Reflect. It
would still exist on `Reflect` and it would still `Option<Box<dyn
Reflect>>`. In fact, Unique Reflect would only _improve_ the user's
understanding of what this method returns.

We may consider moving what's currently `Reflect::clone_value` to
`PartialReflect` and possibly renaming it to `partial_reflect_clone` or
`clone_dynamic` to better indicate how it differs from `reflect_clone`.

## Testing

You can test locally by running the following command:

```
cargo test --package bevy_reflect
```

---

## Changelog

- Added `Reflect::reflect_clone` method
- Added `ReflectCloneError` error enum
- Added `#[reflect(Clone)]` container attribute
- Added `#[reflect(clone)]` field attribute
2025-03-11 06:02:59 +00:00
Vic
32d53e7bd3
make various entity wrapper type modules public (#18248)
# Objective

Part of the #16547 series.

The entity wrapper types often have some associated types an aliases
with them that cannot be re-exported into an outer module together.
Some helper types are best used with part of their path:
`bevy::ecs::entity::index_set::Slice` as `index_set::Slice`.
This has already been done for `entity::hash_set` and
`entity::hash_map`.

## Solution

Publicize the `index_set`, `index_map`, `unique_vec`, `unique_slice`,
and `unique_array` modules.

## Migration Guide

Any mention or import of types in the affected modules have to add the
respective module name to the import path.
F.e.:
`bevy::ecs::entity::EntityIndexSet` ->
`bevy::ecs::entity::index_set::EntityIndexSet`
2025-03-11 05:48:31 +00:00
krunchington
f68ed878e5
Update text_input and virtual_time examples to use Improved Spawning API (#18249)
# Objective

Contributes to #18238 
Updates the `text_input` and `virtual_time` examples to use the
`children!` macro. I wanted to keep the PR small but chose to do two
examples since they both included only a single `with_children` call.

## Solution

Updates examples to use the Improved Spawning API merged in
https://github.com/bevyengine/bevy/pull/17521

## Testing

- Did you test these changes? If so, how?
- Opened the examples before and after and verified the same behavior
was observed. I did this on Ubuntu 24.04.2 LTS using `--features
wayland`.
- Are there any parts that need more testing?
- Other OS's and features can't hurt, but this is such a small change it
shouldn't be a problem.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
  - Run the examples yourself with and without these changes.
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
  - see above

---

## Showcase

n/a

## Migration Guide

n/a
2025-03-11 05:47:56 +00:00
krunchington
76ab6a9e8c
Update state/states example to use children! macro (#18250)
# Objective

Contributes to #18238 
Updates the `state/states` example to use the `children!` macro. Note
that this example also requires `--features bevy_dev_tools`

## Solution

Updates examples to use the Improved Spawning API merged in
https://github.com/bevyengine/bevy/pull/17521

## Testing

- Did you test these changes? If so, how?
- Opened the examples before and after and verified the same behavior
was observed. I did this on Ubuntu 24.04.2 LTS using `--features
wayland`.
- Are there any parts that need more testing?
- Other OS's and features can't hurt, but this is such a small change it
shouldn't be a problem.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
  - Run the examples yourself with and without these changes.
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
  - see above

---

## Showcase

n/a

## Migration Guide

n/a
2025-03-11 05:47:28 +00:00
jf908
bfe932d1f3
Rework WindowMode::Fullscreen API (#17525)
# Objective

- #16883 
- Improve the default behaviour of the exclusive fullscreen API.

## Solution

This PR changes the exclusive fullscreen window mode to require the type
`WindowMode::Fullscreen(MonitorSelection, VideoModeSelection)` and
removes `WindowMode::SizedFullscreen`. This API somewhat intentionally
more closely resembles Winit 0.31's upcoming fullscreen and video mode
API.

The new VideoModeSelection enum is specified as follows:

```rust
pub enum VideoModeSelection {
    /// Uses the video mode that the monitor is already in.
    Current,
    /// Uses a given [`crate::monitor::VideoMode`]. A list of video modes supported by the monitor
    /// is supplied by [`crate::monitor::Monitor::video_modes`].
    Specific(VideoMode),
}
```

### Changing default behaviour

This might be contentious because it removes the previous behaviour of
`WindowMode::Fullscreen` which selected the highest resolution possible.
While the previous behaviour would be quite easy to re-implement as
additional options, or as an impl method on Monitor, I would argue that
this isn't an implementation that should be encouraged.

From the perspective of a Windows user, I prefer what the majority of
modern games do when entering fullscreen which is to preserve the OS's
current resolution settings, which allows exclusive fullscreen to be
entered faster, and to only have it change if I manually select it in
either the options of the game or the OS. The highest resolution
available is not necessarily what the user prefers.

I am open to changing this if I have just missed a good use case for it.

Likewise, the only functionality that `WindowMode::SizedFullscreen`
provided was that it selected the resolution closest to the current size
of the window so it was removed since this behaviour can be replicated
via the new `VideoModeSelection::Specific` if necessary.

## Out of scope

WindowResolution and scale factor act strangely in exclusive fullscreen,
this PR doesn't address it or regress it.

## Testing

- Tested on Windows 11 and macOS 12.7
- Linux untested

## Migration Guide

`WindowMode::SizedFullscreen(MonitorSelection)` and
`WindowMode::Fullscreen(MonitorSelection)` has become
`WindowMode::Fullscreen(MonitorSelection, VideoModeSelection)`.
Previously, the VideoMode was selected based on the closest resolution
to the current window size for SizedFullscreen and the largest
resolution for Fullscreen. It is possible to replicate that behaviour by
searching `Monitor::video_modes` and selecting it with
`VideoModeSelection::Specific(VideoMode)` but it is recommended to use
`VideoModeSelection::Current` as the default video mode when entering
fullscreen.
2025-03-11 01:20:53 +00:00
Rob Parrett
4a41525cd0
Add more to the 2d testbed for text (#18240)
# Objective

`Text2d` testing hasn't been as thorough as text in the UI, and it
suffered a bunch of bugs / perf issues in recent cycles.

## Solution

Add some more `Text2d` scenarios to the 2d testbed to catch bugs,
testing bounded and unbounded text with various justification.

## Testing

`cargo run --example testbed_2d` (press space a few times)

<img width="1280" alt="Screenshot 2025-03-10 at 1 02 03 PM"
src="https://github.com/user-attachments/assets/1e4ee39c-809b-4cc6-81bd-68e67b9625b5"
/>

---------

Co-authored-by: Ben Frankel <ben.frankel7@gmail.com>
2025-03-10 22:04:14 +00:00
Brezak
906d788420
Expand the RelationshipSourceCollection to return more information (#18241)
# Objective

While redoing #18058 I needed `RelationshipSourceCollection` (henceforth
referred to as the **Trait**) to implement `clear` so I added it.

## Solution

Add the `clear` method to the **Trait**.
Also make `add` and `remove` report if they succeeded.

## Testing

Eyeballs

---

## Showcase

The `RelationshipSourceCollection` trait now reports if adding or
removing an entity from it was successful.
It also not contains the `clear` method so you can easily clear the
collection in generic contexts.

## Changes

EDITED by Alice: We should get this into 0.16, so no migration guide
needed.

The `RelationshipSourceCollection` methods `add` and `remove` now need
to return a boolean indicating if they were successful (adding a entity
to a set that already contains it counts as failure). Additionally the
`clear` method has been added to support clearing the collection in
generic contexts.
2025-03-10 22:04:08 +00:00
Emmet_v15
ed529a7cae
docs: Fix incorrect cycle_modes function documentation (#18237)
Update the documentation comment to correctly state that the function
responds to spacebar presses rather than mouse clicks. This aligns the
documentation with the actual implementation, which was already using
keyboard input.

# Objective

- Fix incorrect documentation comment for the cycle_modes function that
inaccurately described using mouse input when the implementation
actually uses spacebar.
- This ensures documentation accuracy and prevents confusion for new
learners.

## Solution

- Updated the documentation comment to correctly state that the function
responds to spacebar presses rather than mouse clicks.
- The code functionality remains unchanged.
2025-03-10 22:00:45 +00:00
BD103
e24191dd89
(Adoped) Remove panics and optimise mesh picking (#18232)
_Note from BD103: this PR was adopted from #16148. The majority of this
PR's description is copied from the original._

# Objective

Adds tests to cover various mesh picking cases and removes sources of
panics.

It should prevent users being able to trigger panics in `bevy_picking`
code via bad mesh data such as #15891, and is a follow up to my comments
in [#15800
(review)](https://github.com/bevyengine/bevy/pull/15800#pullrequestreview-2361694213).

This is motivated by #15979

## Testing

Adds 8 new tests to cover `ray_mesh_intersection` code.

## Changes from original PR

I reverted the changes to the benchmarks, since that was the largest
factor blocking it merging. I'll open a follow-up issue so that those
benchmark changes can be implemented.

---------

Co-authored-by: Trent <2771466+tbillington@users.noreply.github.com>
2025-03-10 21:55:40 +00:00
Benjamin Brienen
690858166c
Fix mikkitspace typos (#18218)
# Objective

Correct spelling

## Solution

Fix 2 minor typos in crates/bevy_mikkitspace

## Testing

Doesn't change functional code
2025-03-10 21:53:00 +00:00
Matt Thompson
4e2dc4b15f
Add Saturation trait to bevy_color (#18202)
# Objective

- Allow for convenient access and mutation of color saturation providing
following the `Hue`, `Luminance` traits.

- `with_saturation()` builder method
- `saturation()` to get the saturation of a `Color`
- `set_saturation()` to set the saturation of a mutable `Color`

## Solution

- Defined `Saturation` trait in `color_ops.rs`
- Implemented `Saturation` on `Hsla` and `Hsva`
- Implemented `Saturation` on `Color` which proxies to other color space
impls.
- In the case of colorspaces which don't have native saturation
components
  the color is converted to 'Hsla` internally

## Testing

- Empirically tested
---

## Showcase

```rust
fn next_golden(&mut self) -> Color {
    self.current
        .rotate_hue((1.0 / GOLDEN_RATIO) * 360.0)
        .with_saturation(self.rand_saturation())
        .with_luminance(self.rand_luminance())
        .into()
}
```

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2025-03-10 21:52:04 +00:00
Eagster
65a9e6fd9f
Address Entities::len inconsistency (#18190)
# Objective

I was recently exploreing `Entities` and stumbled on something strange.
`Entities::len` (the field) has the comment `Stores the number of free
entities for [`len`](Entities::len)`, refering to the method. But that
method says `The count of currently allocated entities.` Looking at the
code, the field's comment is wrong, and the public `len()` is correct.
Phew!

## Solution

So, I was just going to fix the comment, so it didn't confuse anyone
else, but as it turns out, we can just remove the field entirely. As a
bonus, this saves some book keeping work too. We can just calculate it
on the fly.

Also, add additional length methods and documentation for completeness.
These new length methods might be useful debug tools in the future.

---------

Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
2025-03-10 21:51:01 +00:00
Eagster
21f003f13f
Improve component registration performance (#18180)
# Objective

Make component registration faster. This is a tinny, almost petty PR,
but it led to roughly 10% faster registration, according to my
benchmarks in #17871.

Up to y'all if we do this or not. It is completely unnecessary, but its
such low hanging fruit that I wanted to put it out there.

## Solution

Instead of cloning a `HashSet`, collect it into a `SmallVec`. Since this
is empty for many components, this saves a lot of allocation and hashing
work.

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2025-03-10 21:49:07 +00:00
Florian Klink
0cf1dc12e0
bevy_reflect: bump petgraph dependency (#18174)
# Objective

petgraph 0.7 has been released a while ago, but bevy still pulls in
petgraph 0.6

## Solution

- Update the version in bevy_reflect's `Cargo.toml`.

## Testing

- I ran `cargo test --all-features`.
2025-03-10 21:47:48 +00:00
Eagster
246ce590e5
Queued component registration (#18173)
# Objective

This is an alternative to #17871 and #17701 for tracking issue #18155.
This thanks to @maniwani for help with this design.

The goal is to enable component ids to be reserved from multiple threads
concurrently and with only `&World`. This contributes to assets as
entities, read-only query and system parameter initialization, etc.

## What's wrong with #17871 ?

In #17871, I used my proposed staging utilities to allow *fully*
registering components from any thread concurrently with only
`&Components`. However, if we want to pursue components as entities
(which is desirable for a great many reasons. See
[here](https://discord.com/channels/691052431525675048/692572690833473578/1346499196655505534)
on discord), this staging isn't going to work. After all, if registering
a component requires spawning an entity, and spawning an entity requires
`&mut World`, it is impossible to register a component fully with only
`&World`.

## Solution

But what if we don't have to register it all the way? What if it's
enough to just know the `ComponentId` it will have once it is registered
and to queue it to be registered at a later time? Spoiler alert: That is
all we need for these features.

Here's the basic design:

Queue a registration:

1. Check if it has already been registered.
2. Check if it has already been queued.
3. Reserve a `ComponentId`.
4. Queue the registration at that id.

Direct (normal) registration:

1. Check if this registration has been queued.
2. If it has, use the queued registration instead.
3. Otherwise, proceed like normal.

Appllying the queue:

1. Pop queued items off one by one.
2. Register them directly.

One other change:

The whole point of this design over #17871 is to facilitate coupling
component registration with the World. To ensure that this would fully
work with that, I went ahead and moved the `ComponentId` generator onto
the world itself. That stemmed a couple of minor organizational changes
(see migration guide). As we do components as entities, we will replace
this generator with `Entities`, which lives on `World` too. Doing this
move early let me verify the design and will reduce migration headaches
in the future. If components as entities is as close as I think it is, I
don't think splitting this up into different PRs is worth it. If it is
not as close as it is, it might make sense to still do #17871 in the
meantime (see the risks section). I'll leave it up to y'all what we end
up doing though.

## Risks and Testing

The biggest downside of this compared to #17871 is that now we have to
deal with correct but invalid `ComponentId`s. They are invalid because
the component still isn't registered, but they are correct because, once
registered, the component will have exactly that id.

However, the only time this becomes a problem is if some code violates
safety rules by queuing a registration and using the returned id as if
it was valid. As this is a new feature though, nothing in Bevy does
this, so no new tests were added for it. When we do use it, I left
detailed docs to help mitigate issues here, and we can test those
usages. Ex: we will want some tests on using queries initialized from
queued registrations.

## Migration Guide

Component registration can now be queued with only `&World`. To
facilitate this, a few APIs needed to be moved around.

The following functions have moved from `Components` to
`ComponentsRegistrator`:

- `register_component`
- `register_component_with_descriptor`
- `register_resource_with_descriptor`
- `register_non_send`
- `register_resource`
- `register_required_components_manual`

Accordingly, functions in `Bundle` and `Component` now take
`ComponentsRegistrator` instead of `Components`.
You can obtain `ComponentsRegistrator` from the new
`World::components_registrator`.
You can obtain `ComponentsQueuedRegistrator` from the new
`World::components_queue`, and use it to stage component registration if
desired.

# Open Question

Can we verify that it is enough to queue registration with `&World`? I
don't think it would be too difficult to package this up into a
`Arc<MyComponentsManager>` type thing if we need to, but keeping this on
`&World` certainly simplifies things. If we do need the `Arc`, we'll
need to look into partitioning `Entities` for components as entities, so
we can keep most of the allocation fast on `World` and only keep a
smaller partition in the `Arc`. I'd love an SME on assets as entities to
shed some light on this.

---------

Co-authored-by: andriyDev <andriydzikh@gmail.com>
2025-03-10 21:46:27 +00:00
Patrick Walton
b46d9f0825
Make buffer binding arrays emit bind group layout entries and bindless resource descriptors again. (#18125)
PR #17965 mistakenly made the `AsBindGroup` macro no longer emit a bind
group layout entry and a resource descriptor for buffers. This commit
adds that functionality back, fixing the `shader_material_bindless`
example.

Closes #18124.
2025-03-10 21:40:24 +00:00
SpecificProtagonist
54247bcf86
Recursive run_system (#18076)
# Objective

Fixes #18030

## Solution

When running a one-shot system, requeue the system's command queue onto
the world's command queue, then execute the later.

If running the entire command queue of the world is undesired, I could
add a new method to `RawCommandQueue` to only apply part of it.

## Testing

See the new test.

---

## Showcase

```rust
#[derive(Resource)]
pub struct Test {
    id: SystemId,
    counter: u32,
}

let mut world = World::new();
let id = world.register_system(|mut commands: Commands, mut test: ResMut<Test>| {
    print!("{:?} ", test.counter);
    test.counter -= 1;
    if test.counter > 0 {
        commands.run_system(test.id);
    }
});
world.insert_resource(Test { id, counter: 5 });
world.run_system(id).unwrap();
```

```
5 4 3 2 1 
```
2025-03-10 21:38:36 +00:00
Eagster
79e7f8ae0c
Use register_dynamic for merging (#18028)
# Objective

I found a bug while working on #17871. When required components are
registered, ones that are more specific (smaller inheritance depth) are
preferred to others. So, if a ComponentA is already required, but it is
registered as required again, it will be updated if and only if the new
requirement has a smaller inheritance depth (is more specific). However,
this logic was not reflected in merging `RequriedComponents`s together.
Hence, for complicated requirement trees, the wrong initializer could be
used.

## Solution

Re-write merging to work by extending the collection via
`require_dynamic` instead of blindly combining the inner storage.

## Testing

I created this test to ensure this bug doesn't creep back in. This test
fails on main, but passes on this branch.

```rs
    #[test]
    fn required_components_inheritance_depth_bias() {
        #[derive(Component, PartialEq, Eq, Clone, Copy, Debug)]
        struct MyRequired(bool);

        #[derive(Component, Default)]
        #[require(MyRequired(|| MyRequired(false)))]
        struct MiddleMan;

        #[derive(Component, Default)]
        #[require(MiddleMan)]
        struct ConflictingRequire;

        #[derive(Component, Default)]
        #[require(MyRequired(|| MyRequired(true)))]
        struct MyComponent;

        let mut world = World::new();
        let order_a = world
            .spawn((ConflictingRequire, MyComponent))
            .get::<MyRequired>()
            .cloned();
        let order_b = world
            .spawn((MyComponent, ConflictingRequire))
            .get::<MyRequired>()
            .cloned();

        assert_eq!(order_a, Some(MyRequired(true)));
        assert_eq!(order_b, Some(MyRequired(true)));
    }
```
Note that when the inheritance depth is 0 (Like if there were no middle
man above), the order of the components in the bundle still matters.

## Migration Guide

`RequiredComponents::register_dynamic` has been changed to
`RequiredComponents::register_dynamic_with`.

Old:
```rust
required_components.register_dynamic(
      component_id,
      component_constructor.clone(),
      requirement_inheritance_depth,
);
```

New:
```rust
required_components.register_dynamic_with(
      component_id,
      requirement_inheritance_depth,
      || component_constructor.clone(),
);
```

This can prevent unnecessary cloning.

---------

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
2025-03-10 21:35:55 +00:00