# Objective
- In the latest released version (15.3) I am able to obtain this
information by getting the actual `EventLoop` via `non_send_resource`.
Now that this object has (probably rightfully so) been replaced by the
`EventLoopProxy`, I can no longer maintain my custom render backend:
https://github.com/HugoPeters1024/bevy_vulkan. I also need the display
handle for a custom winit integration, for which I've made patches to
bevy before: XREF: https://github.com/bevyengine/bevy/pull/15884
## Solution
- Luckily, all that is required is exposing the `OwnedDisplayHandle` in
its own wrapper resource.
## Testing
- Aforementioned custom rendering backend works on this commit.
---------
Co-authored-by: HugoPeters1024 <hugopeters1024@gmail.com>
# Objective
- Cleanup
## Solution
- Remove completely unused weak_handle
(`MESH_PREPROCESS_TYPES_SHADER_HANDLE`). This value is not used
directly, and is never populated.
- Delete multiple loads of `BUILD_INDIRECT_PARAMS_SHADER_HANDLE`. We
load it three times right after one another. This looks to be a
copy-paste error.
## Testing
- None.
# Objective
Improve error messages for missing resources.
The default error handler currently prints the `Debug` representation of
the error type instead of `Display`. Most error types use
`#[derive(Debug)]`, resulting in a dump of the structure, but will have
a user-friendly message for `Display`.
Follow-up to #18593
## Solution
Change the default error handler to use `Display` instead of `Debug`.
Change `BevyError` to include the backtrace in the `Display` format in
addition to `Debug` so that it is still included.
## Showcase
Before:
```
Encountered an error in system `system_name`: SystemParamValidationError { skipped: false, message: "Resource does not exist", param: "bevy_ecs::change_detection::Res<app_name::ResourceType>" }
Encountered an error in system `other_system_name`: "String message with\nmultiple lines."
```
After
```
Encountered an error in system `system_name`: Parameter `Res<ResourceType>` failed validation: Resource does not exist
Encountered an error in system `other_system_name`: String message with
multiple lines.
```
# Objective
My ecosystem crate, bevy_mod_outline, currently uses `SetMeshBindGroup`
as part of its custom rendering pipeline. I would like to allow for
possibility that, due to changes in 0.16, I need to customise the
behaviour of `SetMeshBindGroup` in order to make it work. However, not
all of the symbol needed to implement this render command are public
outside of Bevy.
## Solution
- Include `MorphIndices` in re-export list. I feel this is morally
equivalent to `SkinUniforms` already being exported.
- Change `MorphIndex::index` field to be public. I feel this is morally
equivalent to the `SkinByteOffset::byte_offset` field already being
public.
- Change `RenderMeshIntances::mesh_asset_id()` to be public (although
since all the fields of `RenderMeshInstances` are public it's possible
to work around this one by reimplementing).
These changes exclude:
- Making any change to the `RenderLightmaps` type as I don't need to
bind the light-maps for my use-case and I wanted to keep these changes
minimal. It has a private field which would need to be public or have
access methods.
- The changes already included in #18612.
## Testing
Confirmed that a copy of `SetMeshBindGroup` can be compiled outside of
Bevy with these changes, provided that the light-map code is removed.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Fixes#17872
## Solution
This should have basically no impact on static scenes. We can optimize
more later if anything comes up. Needing to iterate the two level bin is
a bit unfortunate but shouldn't matter for apps that use a single
camera.
# Objective
- There's been several changes to `Query` for this release cycle, and
`Query`'s top-level documentation has gotten slightly out-of-date.
- Alternative to #18615.
## Solution
- Edit `Query`'s docs for consistency, clarity, and correctness.
- Make sure to group `get()` and `get_many()` together instead of
`single()` and `get_many()`, to enforce the distinction from
https://github.com/bevyengine/bevy/pull/18615#issuecomment-2764355672.
- Reformat doc tests so they would be readable if extracted into their
own file. (Which mainly involves adding more spacing.)
- Move link definitions to be nearer where they are used.
- Fix the tables so they are up-to-date and correctly escape square
brackets `\[ \]`.
## Testing
I ran `cargo doc -p bevy_ecs --no-deps` to view the docs and `cargo test
-p bevy_ecs --doc` to test the doc comments.
## Reviewing
The diff is difficult to read, so I don't recommend _just_ looking at
that. Instead, run `cargo doc -p bevy_ecs --no-deps` locally and read
through the new version. It should theoretically read smoother with less
super-technical jargon. :)
## Follow-up
I want to go through some of `Query`'s methods, such as `single()`,
`get()`, and `get_many()`, but I'll leave that for another PR.
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
Fixes#17986Fixes#18608
## Solution
Guard against situations where an extracted mesh does not have an
associated material. The way that mesh is dependent on the material api
(although decoupled) here is a bit unfortunate and we might consider
ways in the future to support these material features without this
indirect dependency.
- the bevy workspace fails to publish
```
Packaging bevy_asset v0.16.0-dev (/home/runner/work/bevy-releasability/bevy-releasability/crates/bevy_asset)
Updating crates.io index
Updating `kellnr` index
error: failed to prepare local package for uploading
Caused by:
no matching package named `bevy_log` found
location searched: `kellnr` index
required by package `bevy_asset v0.16.0-dev (/home/runner/work/bevy-releasability/bevy-releasability/crates/bevy_asset)`
```
-
https://github.com/TheBevyFlock/bevy-releasability/actions/runs/14153238476/job/39649160443
- Remove bevy_log dev-dependency from bevy_asset
- Not sure of why this is a problem, but the dev-dependency is not
really needed so... 🤷
# Objective
Fixes#9367.
Yet another follow-up to #16547.
These traits were initially based on `Borrow<Entity>` because that trait
was what they were replacing, and it felt close enough in meaning.
However, they ultimately don't quite match: `borrow` always returns
references, whereas `EntityBorrow` always returns a plain `Entity`.
Additionally, `EntityBorrow` can imply that we are borrowing an `Entity`
from the ECS, which is not what it does.
Due to its safety contract, `TrustedEntityBorrow` is important an
important and widely used trait for `EntitySet` functionality.
In contrast, the safe `EntityBorrow` does not see much use, because even
outside of `EntitySet`-related functionality, it is a better idea to
accept `TrustedEntityBorrow` over `EntityBorrow`.
Furthermore, as #9367 points out, abstracting over returning `Entity`
from pointers/structs that contain it can skip some ergonomic friction.
On top of that, there are aspects of #18319 and #18408 that are relevant
to naming:
We've run into the issue that relying on a type default can switch
generic order. This is livable in some contexts, but unacceptable in
others.
To remedy that, we'd need to switch to a type alias approach:
The "defaulted" `Entity` case becomes a
`UniqueEntity*`/`Entity*Map`/`Entity*Set` alias, and the base type
receives a more general name. `TrustedEntityBorrow` does not mesh
clearly with sensible base type names.
## Solution
Replace any `EntityBorrow` bounds with `TrustedEntityBorrow`.
+
Rename them as such:
`EntityBorrow` -> `ContainsEntity`
`TrustedEntityBorrow` -> `EntityEquivalent`
For `EntityBorrow` we produce a change in meaning; We designate it for
types that aren't necessarily strict wrappers around `Entity` or some
pointer to `Entity`, but rather any of the myriad of types that contain
a single associated `Entity`.
This pattern can already be seen in the common `entity`/`id` methods
across the engine.
We do not mean for `ContainsEntity` to be a trait that abstracts input
API (like how `AsRef<T>` is often used, f.e.), because eliding
`entity()` would be too implicit in the general case.
We prefix "Contains" to match the intuition of a struct with an `Entity`
field, like some contain a `length` or `capacity`.
It gives the impression of structure, which avoids the implication of a
relationship to the `ECS`.
`HasEntity` f.e. could be interpreted as "a currently live entity",
As an input trait for APIs like #9367 envisioned, `TrustedEntityBorrow`
is a better fit, because it *does* restrict itself to strict wrappers
and pointers. Which is why we replace any
`EntityBorrow`/`ContainsEntity` bounds with
`TrustedEntityBorrow`/`EntityEquivalent`.
Here, the name `EntityEquivalent` is a lot closer to its actual meaning,
which is "A type that is both equivalent to an `Entity`, and forms the
same total order when compared".
Prior art for this is the
[`Equivalent`](https://docs.rs/hashbrown/latest/hashbrown/trait.Equivalent.html)
trait in `hashbrown`, which utilizes both `Borrow` and `Eq` for its one
blanket impl!
Given that we lose the `Borrow` moniker, and `Equivalent` can carry
various meanings, we expand on the safety comment of `EntityEquivalent`
somewhat. That should help prevent the confusion we saw in
[#18408](https://github.com/bevyengine/bevy/pull/18408#issuecomment-2742094176).
The new name meshes a lot better with the type aliasing approach in
#18408, by aligning with the base name `EntityEquivalentHashMap`.
For a consistent scheme among all set types, we can use this scheme for
the `UniqueEntity*` wrapper types as well!
This allows us to undo the switched generic order that was introduced to
`UniqueEntityArray` by its `Entity` default.
Even without the type aliases, I think these renames are worth doing!
## Migration Guide
Any use of `EntityBorrow` becomes `ContainsEntity`.
Any use of `TrustedEntityBorrow` becomes `EntityEquivalent`.
# 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.
- bevy_image fails to build without default features:
```
error[E0277]: `image::Image` does not implement `TypePath` so cannot provide static type path information
--> crates/bevy_image/src/image.rs:341:12
|
341 | pub struct Image {
| ^^^^^ the trait `bevy_reflect::type_path::TypePath` is not implemented for `image::Image`
|
= note: consider annotating `image::Image` with `#[derive(Reflect)]` or `#[derive(TypePath)]`
= help: the following other types implement trait `bevy_reflect::type_path::TypePath`:
&'static Location<'static>
&'static T
&'static mut T
()
(P,)
(P1, P0)
(P1, P2, P0)
(P1, P2, P3, P0)
and 146 others
note: required by a bound in `Asset`
--> /home/runner/work/bevy-releasability/bevy-releasability/crates/bevy_asset/src/lib.rs:415:43
|
415 | pub trait Asset: VisitAssetDependencies + TypePath + Send + Sync + 'static {}
| ^^^^^^^^ required by this bound in `Asset`
= note: `Asset` is a "sealed trait", because to implement it you also need to implement `bevy_reflect::type_path::TypePath`, which is not accessible; this is usually done to force you to use one of the provided types that already implement it
= help: the following types implement the trait:
bevy_asset::AssetIndex
bevy_asset::LoadedUntypedAsset
bevy_asset::AssetEvent<A>
bevy_asset::LoadedFolder
bevy_asset::StrongHandle
bevy_asset::Handle<A>
bevy_asset::AssetId<A>
bevy_asset::AssetPath<'a>
and 148 others
error[E0277]: `image::Image` does not implement `TypePath` so cannot provide static type path information
--> crates/bevy_image/src/image_loader.rs:121:18
|
121 | type Asset = Image;
| ^^^^^ the trait `bevy_reflect::type_path::TypePath` is not implemented for `image::Image`
|
= note: consider annotating `image::Image` with `#[derive(Reflect)]` or `#[derive(TypePath)]`
= help: the following other types implement trait `bevy_reflect::type_path::TypePath`:
&'static Location<'static>
&'static T
&'static mut T
()
(P,)
(P1, P0)
(P1, P2, P0)
(P1, P2, P3, P0)
and 146 others
= note: required for `<ImageLoader as AssetLoader>::Asset` to implement `Asset`
note: required by a bound in `bevy_asset::AssetLoader::Asset`
--> /home/runner/work/bevy-releasability/bevy-releasability/crates/bevy_asset/src/loader.rs:33:17
|
33 | type Asset: Asset;
| ^^^^^ required by this bound in `AssetLoader::Asset`
error[E0277]: `texture_atlas::TextureAtlasLayout` does not implement `TypePath` so cannot provide static type path information
--> crates/bevy_image/src/texture_atlas.rs💯12
|
100 | pub struct TextureAtlasLayout {
| ^^^^^^^^^^^^^^^^^^ the trait `bevy_reflect::type_path::TypePath` is not implemented for `texture_atlas::TextureAtlasLayout`
|
= note: consider annotating `texture_atlas::TextureAtlasLayout` with `#[derive(Reflect)]` or `#[derive(TypePath)]`
= help: the following other types implement trait `bevy_reflect::type_path::TypePath`:
&'static Location<'static>
&'static T
&'static mut T
()
(P,)
(P1, P0)
(P1, P2, P0)
(P1, P2, P3, P0)
and 146 others
note: required by a bound in `Asset`
--> /home/runner/work/bevy-releasability/bevy-releasability/crates/bevy_asset/src/lib.rs:415:43
|
415 | pub trait Asset: VisitAssetDependencies + TypePath + Send + Sync + 'static {}
| ^^^^^^^^ required by this bound in `Asset`
= note: `Asset` is a "sealed trait", because to implement it you also need to implement `bevy_reflect::type_path::TypePath`, which is not accessible; this is usually done to force you to use one of the provided types that already implement it
= help: the following types implement the trait:
bevy_asset::AssetIndex
bevy_asset::LoadedUntypedAsset
bevy_asset::AssetEvent<A>
bevy_asset::LoadedFolder
bevy_asset::StrongHandle
bevy_asset::Handle<A>
bevy_asset::AssetId<A>
bevy_asset::AssetPath<'a>
and 148 others
```
- `Asset` trait depends on `TypePath` which is in bevy_reflect. it's
usually implemented by the `Reflect` derive
- make bevy_reflect not an optional dependency
- when feature `bevy_reflect` is not enabled, derive `TypePath` directly
# Objective
- Optimize static scene performance by marking unchanged subtrees.
-
[bef0209](bef0209de1)
fixes#18255 and #18363.
- Closes#18365
- Includes change from #18321
## Solution
- Mark hierarchy subtrees with dirty bits to avoid transform propagation
where not needed
- This causes a performance regression when spawning many entities, or
when the scene is entirely dynamic.
- This results in massive speedups for largely static scenes.
- In the future we could allow the user to change this behavior, or add
some threshold based on how dynamic the scene is?
## Testing
- Caldera Hotel scene
# Objective
Fixes#18515
After the recent changes to system param validation, the panic message
for a missing resource is currently:
```
Encountered an error in system `missing_resource_error::res_system`: SystemParamValidationError { skipped: false }
```
Add the parameter type name and a descriptive message, improving the
panic message to:
```
Encountered an error in system `missing_resource_error::res_system`: SystemParamValidationError { skipped: false, message: "Resource does not exist", param: "bevy_ecs::change_detection::Res<missing_resource_error::MissingResource>" }
```
## Solution
Add fields to `SystemParamValidationError` for error context. Include
the `type_name` of the param and a message.
Store them as `Cow<'static, str>` and only format them into a friendly
string in the `Display` impl. This lets us create errors using a
`&'static str` with no allocation or formatting, while still supporting
runtime `String` values if necessary.
Add a unit test that verifies the panic message.
## Future Work
If we change the default error handling to use `Display` instead of
`Debug`, and to use `ShortName` for the system name, the panic message
could be further improved to:
```
Encountered an error in system `res_system`: Parameter `Res<MissingResource>` failed validation: Resource does not exist
```
However, `BevyError` currently includes the backtrace in `Debug` but not
`Display`, and I didn't want to try to change that in this PR.
# Objective
This fixes `NonMesh` draw commands not receiving render-world entities
since
- https://github.com/bevyengine/bevy/pull/17698
This unbreaks item queries for queued non-mesh entities:
```rust
struct MyDrawCommand {
type ItemQuery = Read<DynamicUniformIndex<SomeUniform>>;
// ...
}
```
### Solution
Pass render entity to `NonMesh` draw commands instead of
`Entity::PLACEHOLDER`. This PR also introduces sorting of the `NonMesh`
bin keys like other types, which I assume is the intended behavior.
@pcwalton
## Testing
- Tested on a local project that extensively uses `NonMesh` items.
# Objective
As of bevy 0.16-dev, the pre-existing public function
`bevy::pbr::setup_morph_and_skinning_defs()` is now passed a boolean
flag called `skins_use_uniform_buffers`. The value of this boolean is
computed by the function
`bevy_pbr::render::skin::skins_use_uniform_buffers()`, but it is not
exported publicly.
Found while porting
[bevy_mod_outline](https://github.com/komadori/bevy_mod_outline) to
0.16.
## Solution
Add `skin::skins_use_uniform_buffers` to the re-export list of
`bevy_pbr::render`.
## Testing
Confirmed test program can access public API.
# Objective
The flags are referenced later outside of the VERTEX_UVS ifdef/endif
block. The current behavior causes the pre-pass shader to fail to
compile when UVs are not present in the mesh, such as when using a
`LineStrip` to render a grid.
Fixes#18600
## Solution
Move the definition of the `flags` outside of the ifdef/endif block.
## Testing
Ran a modified `3d_example` that used a mesh and material with
alpha_mode blend, `LineStrip` topology, and no UVs.
# Objective
Fixes#18606
When a type implements `Add` for `String`, the compiler can get confused
when attempting to add a `&String` to a `String`.
Unfortunately, this seems to be [expected
behavior](https://github.com/rust-lang/rust/issues/77143#issuecomment-698369286)
which causes problems for generic types since the current `TypePath`
derive generates code that appends strings in this manner.
## Solution
Explicitly use the `Add<&str>` implementation in the `TypePath` derive
macro.
## Testing
You can test locally by running:
```
cargo check -p bevy_reflect --tests
```
# Objective
For the LogDiagnosticsPlugin, the log target is "bevy diagnostic" with a
space; I think it may (?) be a typo intended to be "bevy_diagnostic"
with an underline.
I couldn't get filtering INFO level logs with work with this plugin,
changing this seems to produce the expected behavior.
# Objective
- feature `shader_format_wesl` doesn't compile in Wasm
- once fixed, example `shader_material_wesl` doesn't work in WebGL2
## Solution
- remove special path handling when loading shaders. this seems like a
way to escape the asset folder which we don't want to allow, and can't
compile on android or wasm, and can't work on iOS (filesystem is rooted
there)
- pad material so that it's 16 bits. I couldn't get conditional
compilation to work in wesl for type declaration, it fails to parse
- the shader renders the color `(0.0, 0.0, 0.0, 0.0)` when it's not a
polka dot. this renders as black on WebGPU/metal/..., and white on
WebGL2. change it to `(0.0, 0.0, 0.0, 1.0)` so that it's black
everywhere
* `submit_graph_commands` was incorrectly timing the command buffer
generation tasks as well, and not only the queue submission. Moved the
span to fix that.
* Added a new `command_buffer_generation_tasks` span as a parent for all
the individual command buffer generation tasks that don't run as part of
the Core3d span.
# Objective
Fixes#18562.
## Solution
- Specified that `StateTransition` is actually run before `PreStartup`.
- Specified consequences of this and how to actually run systems before
any game logic regardless of state.
- Updated docs of `StateTransition` to reflect that it is run before
`PreStartup` in addition to being run after `PreUpdate`.
## Testing
- `cargo doc`
- `cargo test --doc`
# Objective
Fixes https://github.com/bevyengine/bevy/issues/16586.
## Solution
- Free meshes before allocating new ones (so hopefully the existing
allocation is used, but it's not guaranteed since it might end up
getting used by a smaller mesh first).
- Keep track of modified render assets, and have the mesh allocator free
their allocations.
- Cleaned up some render asset code to make it more understandable,
since it took me several minutes to reverse engineer/remember how it was
supposed to work.
Long term we'll probably want to explicitly reusing allocations for
modified meshes that haven't grown in size, or do delta uploads using a
compute shader or something, but this is an easy fix for the near term.
## Testing
Ran the example provided in the issue. No crash after a few minutes, and
memory usage remains steady.
## Objective
Fix#18557.
## Solution
As described in the bug, `remaining_weight` should have been inside the
loop.
## Testing
Locally changed the `animated_mesh_control` example to spawn multiple
meshes and play different transitions.
- Fixes#18010.
- Revert the offending PRs! These are #15481 and #18013. We now no
longer get an error if there are duplicate subassets.
- In theory we could untangle #18013 from #15481, but that may be
tricky, and may still introduce regressions. To avoid this worry (since
we're already in RC mode), I am just reverting both.
- This is just a revert.
---
<Remove the migration guides for #15481 and #18013>
I will make a PR to the bevy_website repo after this is merged.
# Objective
#18555 added improved require syntax, but inline structs didn't support
`..Default::default()` syntax (for technical reasons we can't parse the
struct directly, so there is manual logic that missed this case).
## Solution
When a `{}` or `()` section is encountered for a required component,
rather than trying to parse the fields directly, just pass _all_ of the
tokens through. This ensures no tokens are dropped, protects us against
any future syntax changes, and optimizes our parsing logic (as we're
dropping the field parsing logic entirely).
# Objective
Requires are currently more verbose than they need to be. People would
like to define inline component values. Additionally, the current
`#[require(Foo(custom_constructor))]` and `#[require(Foo(|| Foo(10))]`
syntax doesn't really make sense within the context of the Rust type
system. #18309 was an attempt to improve ergonomics for some cases, but
it came at the cost of even more weirdness / unintuitive behavior. Our
approach as a whole needs a rethink.
## Solution
Rework the `#[require()]` syntax to make more sense. This is a breaking
change, but I think it will make the system easier to learn, while also
improving ergonomics substantially:
```rust
#[derive(Component)]
#[require(
A, // this will use A::default()
B(1), // inline tuple-struct value
C { value: 1 }, // inline named-struct value
D::Variant, // inline enum variant
E::SOME_CONST, // inline associated const
F::new(1), // inline constructor
G = returns_g(), // an expression that returns G
H = SomethingElse::new(), // expression returns SomethingElse, where SomethingElse: Into<H>
)]
struct Foo;
```
## Migration Guide
Custom-constructor requires should use the new expression-style syntax:
```rust
// before
#[derive(Component)]
#[require(A(returns_a))]
struct Foo;
// after
#[derive(Component)]
#[require(A = returns_a())]
struct Foo;
```
Inline-closure-constructor requires should use the inline value syntax
where possible:
```rust
// before
#[derive(Component)]
#[require(A(|| A(10))]
struct Foo;
// after
#[derive(Component)]
#[require(A(10)]
struct Foo;
```
In cases where that is not possible, use the expression-style syntax:
```rust
// before
#[derive(Component)]
#[require(A(|| A(10))]
struct Foo;
// after
#[derive(Component)]
#[require(A = A(10)]
struct Foo;
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: François Mockers <mockersf@gmail.com>
## Objective
- Remove the second to last `bevy_animation` dependency on
`bevy_render`.
- Update some older documentation to reflect later changes to the crate.
## Narrative
I'm trying to make `bevy_animation` independent of `bevy_render`. The
documentation for `bevy_animation::AnimatableProperty` is one of the
last few dependencies. It uses `bevy_render::Projection` to demonstrate
animating an arbitrary value, but I thought that could be easily swapped
for something else.
I then realised that the rest of the documentation was a bit out of
date. Originally `AnimatableProperty` was the only way to animate a
property and so the documentation was quite detailed. But over time the
crate has gained more documentation and other ways to hook up
properties, leaving parts of the docs stale or covered elsewhere. So
I've slimmed down the `AnimatableProperty` docs and added a link to the
main alternative (`animated_field`).
I've probably swung too far towards brevity, so I can build them back up
if preferred. Also the example is kinda contrived and doesn't show the
range of `AnimatableProperty`, like being able to choose different
components. And finally the memes might be a bit stale?
## Showcase
## Testing
```
cargo doc -p bevy_animation --no-deps --all-features
cargo test -p bevy_animation --doc --all-features
```
# Objective
Fix panic in `run_system` when running an exclusive system wrapped in a
`PipeSystem` or `AdapterSystem`.
#18076 introduced a `System::run_without_applying_deferred` method. It
normally calls `System::run_unsafe`, but
`ExclusiveFunctionSystem::run_unsafe` panics, so it was overridden for
that type. Unfortunately, `PipeSystem::run_without_applying_deferred`
still calls `PipeSystem::run_unsafe`, which can then call
`ExclusiveFunctionSystem::run_unsafe` and panic.
## Solution
Make `ExclusiveFunctionSystem::run_unsafe` work instead of panicking.
Clarify the safety requirements that make this sound.
The alternative is to override `run_without_applying_deferred` in
`PipeSystem`, `CombinatorSystem`, `AdapterSystem`,
`InfallibleSystemWrapper`, and `InfallibleObserverWrapper`. That seems
like a lot of extra code just to preserve a confusing special case!
Remove some implementations of `System::run` that are no longer
necessary with this change. This slightly changes the behavior of
`PipeSystem` and `CombinatorSystem`: Currently `run` will call
`apply_deferred` on the first system before running the second, but
after this change it will only call it after *both* systems have run.
The new behavior is consistent with `run_unsafe` and
`run_without_applying_deferred`, and restores the behavior prior to
#11823.
The panic was originally necessary because [`run_unsafe` took
`&World`](https://github.com/bevyengine/bevy/pull/6083/files#diff-708dfc60ec5eef432b20a6f471357a7ea9bfb254dc2f918d5ed4a66deb0e85baR90).
Now that it takes `UnsafeWorldCell`, it is possible to make it work. See
also Cart's concerns at
https://github.com/bevyengine/bevy/pull/4166#discussion_r979140356,
although those also predate `UnsafeWorldCell`.
And see #6698 for a previous bug caused by this panic.
# Objective
Make it easier to short-circuit system parameter validation.
Simplify the API surface by combining `ValidationOutcome` with
`SystemParamValidationError`.
## Solution
Replace `ValidationOutcome` with `Result<(),
SystemParamValidationError>`. Move the docs from `ValidationOutcome` to
`SystemParamValidationError`.
Add a `skipped` field to `SystemParamValidationError` to distinguish the
`Skipped` and `Invalid` variants.
Use the `?` operator to short-circuit validation in tuples of system
params.
# Objective
- Fixes#16861
## Solution
- Added:
- `UnsafeEntityCell::get_mut_assume_mutable_by_id`
- `EntityMut::get_mut_assume_mutable_by_id`
- `EntityMut::get_mut_assume_mutable_by_id_unchecked`
- `EntityWorldMut::into_mut_assume_mutable_by_id`
- `EntityWorldMut::into_mut_assume_mutable`
- `EntityWorldMut::get_mut_assume_mutable_by_id`
- `EntityWorldMut::into_mut_assume_mutable_by_id`
- `EntityWorldMut::modify_component_by_id`
- `World::modify_component_by_id`
- `DeferredWorld::modify_component_by_id`
- Added `fetch_mut_assume_mutable` to `DynamicComponentFetch` trait
(this is a breaking change)
## Testing
- CI
---
## Migration Guide
If you had previously implemented `DynamicComponentFetch` you must now
include a definition for `fetch_mut_assume_mutable`. In general this
will be identical to `fetch_mut` using the relevant alternatives for
actually getting a component.
---
## Notes
All of the added methods are minor variations on existing functions and
should therefore be of low risk for inclusion during the RC process.
# Objective
The fix in #17488 forced Windows to always behave as if it were in
`UpdateMode::Continuous`.
CC https://github.com/bevyengine/bevy/pull/17991
## Solution
Removed the unconditional `redraw_requested = true` and added a check
for `Reactive` in `about_to_wait`.
## Testing
- Verified that the `low_power` example worked as expected with all
`UpdateMode` options.
- Verified that animation continued in both `eased_motion ` and
`low_power` examples when in `Continuous` update mode while:
- Resizing the Window
- Moving the window via clicking and dragging the title bar
- Verified that `window_settings` example still worked as expected.
- Verified that `monitor_info` example still worked as expected.
# Objective
fixes#18452.
## Solution
Spawning used to flush commands only, but those commands can reserve
entities. Now, spawning flushes everything, including reserved entities.
I checked, and this was the only place where `flush_commands` is used
instead of `flush` by mistake.
## Testing
I simplified the MRE from #18452 into its own test, which fails on main,
but passes on this branch.
Migration guide:
# Objective
Currently there seems to be no way to enable picking through
render-to-texture cameras
## Solution
This PR allows casting rays from the game code quite easily.
## Testing
- I've tested these in my game and it seems to work
- I haven't tested edge cases
---
## Showcase
<details>
<summary>Click to view showcase</summary>
```rust
fn cast_rays_from_additional_camera(
cameras: Query<(&GlobalTransform, &Camera, Entity), With<RenderToTextureCamera>>,
mut rays: ResMut<RayMap>,
pointers: Query<(&PointerId, &PointerLocation)>,
) {
for (camera_global_transform, camera, camera_entity) in &cameras {
for (pointer_id, pointer_loc) in &pointers {
let Some(viewport_pos) = pointer_loc.location() else {
continue;
};
// if camera result is transformed in any way, the reverse transformation
// should be applied somewhere here
let ray = camera
.viewport_to_world(camera_global_transform, viewport_pos.position)
.ok();
if let Some(r) = ray {
rays.map.insert(RayId::new(camera_entity, *pointer_id), r);
}
}
}
}
```
</details>
## Migration Guide
The `bevy_picking::backend::ray::RayMap::map` method is removed as
redundant,
In systems using `Res<RayMap>` replace `ray_map.map()` with
`&ray_map.map`
# Objective
- The prepass pipeline has a generic bound on the specialize function
but 95% of it doesn't need it
## Solution
- Move most of the fields to an internal struct and use a separate
specialize function for those fields
## Testing
- Ran the 3d_scene and it worked like before
---
## Migration Guide
If you were using a field of the `PrepassPipeline`, most of them have
now been move to `PrepassPipeline::internal`.
## Notes
Here's the cargo bloat size comparison (from this tool
https://github.com/bevyengine/bevy/discussions/14864):
```
before:
(
"<bevy_pbr::prepass::PrepassPipeline<M> as bevy_render::render_resource::pipeline_specializer::SpecializedMeshPipeline>::specialize",
25416,
0.05582993,
),
after:
(
"<bevy_pbr::prepass::PrepassPipeline<M> as bevy_render::render_resource::pipeline_specializer::SpecializedMeshPipeline>::specialize",
2496,
0.005490916,
),
(
"bevy_pbr::prepass::PrepassPipelineInternal::specialize",
11444,
0.025175499,
),
```
The size for the specialize function that is generic is now much
smaller, so users won't need to recompile it for every material.
# Objective
- #18495
## Solution
- The code in the PR #18232 accidentally used a vertex index as a
triangle index, causing the wrong triangle to be used for normal
computation and if the triangle went out of bounds, it would skip the
ray-hit.
- Don't do that.
## Testing
- Run `cargo run --example mesh_picking`
# Objective
Enabling `serialize`, `critical-section`, or `async-executor` would
improperly enable `bevy_math`, `bevy_input`, and/or `bevy_transform`.
This was caused by those crates previously being required but are now
optional (gated behind `std` and/or `libm`).
## Solution
- Added `?` to features not intended to enable those crates
## Testing
- CI
# Objective
- Fixes https://github.com/bevyengine/bevy/issues/11682
## Solution
- https://github.com/bevyengine/bevy/pull/4086 introduced an
optimization to not do redundant calculations, but did not take into
account changes to the resource `global_lights`. I believe that my patch
includes the optimization benefit but adds the required nuance to fix
said bug.
## Testing
The example originally given by
[@kirillsurkov](https://github.com/kirillsurkov) and then updated by me
to bevy 15.3 here:
https://github.com/bevyengine/bevy/issues/11682#issuecomment-2746287416
will not have shadows without this patch:
```rust
use bevy::prelude::*;
#[derive(Resource)]
struct State {
x: f32,
}
fn main() {
App::new()
.add_plugins(DefaultPlugins)
.add_systems(Startup, setup)
.add_systems(Update, update)
.insert_resource(State { x: -40.0 })
.run();
}
fn setup(
mut commands: Commands,
mut meshes: ResMut<Assets<Mesh>>,
mut materials: ResMut<Assets<StandardMaterial>>,
) {
commands.spawn((
Mesh3d(meshes.add(Circle::new(4.0))),
MeshMaterial3d(materials.add(Color::WHITE)),
));
commands.spawn((
Mesh3d(meshes.add(Cuboid::new(1.0, 1.0, 1.0))),
MeshMaterial3d(materials.add(Color::linear_rgb(0.0, 1.0, 0.0))),
));
commands.spawn((
PointLight {
shadows_enabled: true,
..default()
},
Transform::from_xyz(4.0, 8.0, 4.0),
));
commands.spawn(Camera3d::default());
}
fn update(mut state: ResMut<State>, mut camera: Query<&mut Transform, With<Camera3d>>) {
let mut camera = camera.single_mut().unwrap();
let t = Vec3::new(state.x, 0.0, 10.0);
camera.translation = t;
camera.look_at(t - Vec3::Z, Vec3::Y);
state.x = 0.0;
}
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
# Objective
When introduced, `Single` was intended to simply be silently skipped,
allowing for graceful and efficient handling of systems during invalid
game states (such as when the player is dead).
However, this also caused missing resources to *also* be silently
skipped, leading to confusing and very hard to debug failures. In
0.15.1, this behavior was reverted to a panic, making missing resources
easier to debug, but largely making `Single` (and `Populated`)
worthless, as they would panic during expected game states.
Ultimately, the consensus is that this behavior should differ on a
per-system-param basis. However, there was no sensible way to *do* that
before this PR.
## Solution
Swap `SystemParam::validate_param` from a `bool` to:
```rust
/// The outcome of system / system param validation,
/// used by system executors to determine what to do with a system.
pub enum ValidationOutcome {
/// All system parameters were validated successfully and the system can be run.
Valid,
/// At least one system parameter failed validation, and an error must be handled.
/// By default, this will result in1 a panic. See [crate::error] for more information.
///
/// This is the default behavior, and is suitable for system params that should *always* be valid,
/// either because sensible fallback behavior exists (like [`Query`] or because
/// failures in validation should be considered a bug in the user's logic that must be immediately addressed (like [`Res`]).
Invalid,
/// At least one system parameter failed validation, but the system should be skipped due to [`ValidationBehavior::Skip`].
/// This is suitable for system params that are intended to only operate in certain application states, such as [`Single`].
Skipped,
}
```
Then, inside of each `SystemParam` implementation, return either Valid,
Invalid or Skipped.
Currently, only `Single`, `Option<Single>` and `Populated` use the
`Skipped` behavior. Other params (like resources) retain their current
failing
## Testing
Messed around with the fallible_params example. Added a pair of tests:
one for panicking when resources are missing, and another for properly
skipping `Single` and `Populated` system params.
## To do
- [x] get https://github.com/bevyengine/bevy/pull/18454 merged
- [x] fix the todo!() in the macro-powered tuple implementation (please
help 🥺)
- [x] test
- [x] write a migration guide
- [x] update the example comments
## Migration Guide
Various system and system parameter validation methods
(`SystemParam::validate_param`, `System::validate_param` and
`System::validate_param_unsafe`) now return and accept a
`ValidationOutcome` enum, rather than a `bool`. The previous `true`
values map to `ValidationOutcome::Valid`, while `false` maps to
`ValidationOutcome::Invalid`.
However, if you wrote a custom schedule executor, you should now respect
the new `ValidationOutcome::Skipped` parameter, skipping any systems
whose validation was skipped. By contrast, `ValidationOutcome::Invalid`
systems should also be skipped, but you should call the
`default_error_handler` on them first, which by default will result in a
panic.
If you are implementing a custom `SystemParam`, you should consider
whether failing system param validation is an error or an expected
state, and choose between `Invalid` and `Skipped` accordingly. In Bevy
itself, `Single` and `Populated` now once again skip the system when
their conditions are not met. This is the 0.15.0 behavior, but stands in
contrast to the 0.15.1 behavior, where they would panic.
---------
Co-authored-by: MiniaczQ <xnetroidpl@gmail.com>
Co-authored-by: Dmytro Banin <banind@cs.washington.edu>
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
There are two related problems here:
1. Users should be able to change the fallback behavior of *all*
ECS-based errors in their application by setting the
`GLOBAL_ERROR_HANDLER`. See #18351 for earlier work in this vein.
2. The existing solution (#15500) for customizing this behavior is high
on boilerplate, not global and adds a great deal of complexity.
The consensus is that the default behavior when a parameter fails
validation should be set based on the kind of system parameter in
question: `Single` / `Populated` should silently skip the system, but
`Res` should panic. Setting this behavior at the system level is a
bandaid that makes getting to that ideal behavior more painful, and can
mask real failures (if a resource is missing but you've ignored a system
to make the Single stop panicking you're going to have a bad day).
I've removed the existing `ParamWarnPolicy`-based configuration, and
wired up the `GLOBAL_ERROR_HANDLER`/`default_error_handler` to the
various schedule executors to properly plumb through errors .
Additionally, I've done a small cleanup pass on the corresponding
example.
I've run the `fallible_params` example, with both the default and a
custom global error handler. The former panics (as expected), and the
latter spams the error console with warnings 🥲
1. Currently, failed system param validation will result in endless
console spam. Do you want me to implement a solution for warn_once-style
debouncing somehow?
2. Currently, the error reporting for failed system param validation is
very limited: all we get is that a system param failed validation and
the name of the system. Do you want me to implement improved error
reporting by bubbling up errors in this PR?
3. There is broad consensus that the default behavior for failed system
param validation should be set on a per-system param basis. Would you
like me to implement that in this PR?
My gut instinct is that we absolutely want to solve 2 and 3, but it will
be much easier to do that work (and review it) if we split the PRs
apart.
`ParamWarnPolicy` and the `WithParamWarnPolicy` have been removed
completely. Failures during system param validation are now handled via
the `GLOBAL_ERROR_HANDLER`: please see the `bevy_ecs::error` module docs
for more information.
---------
Co-authored-by: MiniaczQ <xnetroidpl@gmail.com>
# Objective
Fixes#18461
Apparently `RustRover` creates a temporary file with a tilde like
`load_scene_example.scn.ron~` and at the moment of calling
`.canonicalize()` the file does not exists anymore.
## Solution
Not call `.unwrap()` and return `None` fixes the issue.
## Testing
- `cargo ci`: OK
- Tested the `scene` example with `file_watcher` feature and it works as
expected.
Co-authored-by: François Mockers <mockersf@gmail.com>
Reduce dependencies on `bevy_render` by preferring `bevy_mesh` imports
over `bevy_render` re-exports.
```diff
- use bevy_render::mesh::Mesh;
+ use bevy_mesh::Mesh;
```
This is intended to help with #18423 (render crate restructure). Affects
`bevy_gltf`, `bevy_animation` and `bevy_picking`.
As part of #18423, I'm assuming there'll be a push to make crates less
dependent on the big render crates. This PR seemed like a small and safe
step along that path - it only changes imports and makes the `bevy_mesh`
crate dependency explicit in `Cargo.toml`. Any remaining dependencies on
`bevy_render` are true dependencies.
```
cargo run --example testbed_3d
cargo run --example mesh_picking
```
# Objective
- Fixes#18225
## Solution
- Updated `accesskit` version requirement from 0.17 to 0.18
- Updated `accesskit_winit` version requirement from 0.23 to 0.25
## Testing
- Ran CI checks locally.
---------
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Instead of extracting an individual sprite per glyph of a text spawn or
slice of a nine-patched sprite, add a buffer to store the extracted
slice geometry.
Fixes#16972
* New struct `ExtractedSlice` to hold sprite slice size, position and
atlas info (for text each glyph is a slice).
* New resource `ExtractedSlices` that wraps the `ExtractedSlice` buffer.
This is a separate resource so it can be used without sprites (with a
text material, for example).
* New enum `ExtractedSpriteKind` with variants `Single` and `Slices`.
`Single` represents a single sprite, `Slices` contains a range into the
`ExtractedSlice` buffer.
* Only queue a single `ExtractedSprite` for sets of glyphs or slices and
push the geometry for each individual slice or glyph into the
`ExtractedSlice` buffer.
* Modify `ComputedTextureSlices` to return an `ExtractedSlice` iterator
instead of `ExtractedSprites`.
* Modify `extract_text2d_sprite` to only queue new `ExtractedSprite`s on
font changes and otherwise push slices.
I don't like the name `ExtractedSpriteKind` much, it's a bit redundant
and too haskellish. But although it's exported, it's not something users
will interact with most of the time so don't want to overthink it.
yellow = this pr, red = main
```cargo run --example many_glyphs --release --features "trace_tracy" -- --no-ui```
<img width="454" alt="many-glyphs" src="https://github.com/user-attachments/assets/711b52c9-2d4d-43c7-b154-e81a69c94dce" />
```cargo run --example many_text2d --release --features "trace_tracy"```
<img width="415" alt="many-text2d"
src="https://github.com/user-attachments/assets/5ea2480a-52e0-4cd0-9f12-07405cf6b8fa"
/>
* `ExtractedSprite` has a new `kind: ExtractedSpriteKind` field with
variants `Single` and `Slices`.
- `Single` represents a single sprite. `ExtractedSprite`'s `anchor`,
`rect`, `scaling_mode` and `custom_size` fields have been moved into
`Single`.
- `Slices` contains a range that indexes into a new resource
`ExtractedSlices`. Slices are used to draw elements composed from
multiple sprites such as text or nine-patched borders.
* `ComputedTextureSlices::extract_sprites` has been renamed to
`extract_slices`. Its `transform` and `original_entity` parameters have
been removed.
---------
Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com>
Remove `critical-section` from required dependencies, allowing linking
without any features.
- Switched from `OnceCell` to `LazyLock`
- Removed `std` feature from `bevy_dylib` (proof that it works)
- CI
- building bevy_winit on linux fails with
```
error: The platform you're compiling for is not supported by winit
--> /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/winit-0.30.9/src/platform_impl/mod.rs:78:1
|
78 | compile_error!("The platform you're compiling for is not supported by winit");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```
- this blocks publishing Bevy from linux during the verification step
- Enable `x11` by default when building bevy_winit, and disable default
features of bevy_winit in bevy_internal
- This doesn't change anything when depending on Bevy
# Objective
- Some crates don't compile or have clippy warnings when building for
wasm32
## Solution
- bevy_asset: unused lifetime
- bevy_gltf: the error is not too large in wasm32
- bevy_remote: fails to compile as feature http is also gated on wasm32
- bevy_winit: unused import `error`
# Objective
`cargo clippy -p bevy_asset` warns on a pair of lints on my Windows 10
development machine (return from let binding).
## Solution
Addressed them!
## Testing
- CI
# Objective
Bevy 0.15 used to have methods on `Children` for sorting and reordering
them. This is very important, because in certain situations, the order
of children matters. For example, in the context of UI nodes.
These methods are missing/omitted/forgotten in the current version,
after the Relationships rework.
Without them, it is impossible for me to upgrade `iyes_perf_ui` to Bevy
0.16.
## Solution
Reintroduce the methods. This PR simply copy-pastes them from Bevy 0.15.
# Objective
- Compiling `bevy_time` without the `std`-feature results in a
`clippy::unnecessary-literal-unwrap`.
## Solution
- Fix lint error
## Testing
- CI
---
- Compiling `bevy_a11y` without default features fails because you need
to select a floating point backed. But you actually don't need it, this
requirement is from an unused linkage to `bevy_input_focus`
- Remove link
- CI
---------
Co-authored-by: François Mockers <mockersf@gmail.com>
- Compile failure with `bevy_anti_aliasing` due to `dds` feature not
enabling `bevy_core_pipeline/dds`, causing a public API desync.
- Ensured feature is enabled
- CI
# Objective
Noticed that `bevy_remote` fails to compile without default features.
## Solution
Adjusted offending method to avoid reliance on `http` module when it is
disabled.
## Testing
- CI
- `cargo clippy -p bevy_remote --no-default-features`
# Objective
Every time I run `cargo clippy -p bevy_ecs` it pops up and it's
distracting.
## Solution
Removed unnecessary returns. The blocks themselves are necessary or the
`#[cfg(...)]` doesn't apply properly
## Testing
`cargo clippy -p bevy_ecs` + ci build tests
# 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,
}
```
# Objective
Const values should be more ergonomic to insert, since this is too
verbose
``` rust
#[derive(Component)]
#[require(
LockedAxes(||LockedAxes::ROTATION_LOCKED),
)]
pub struct CharacterController;
```
instead, users can now abbreviate that nonsense like this
``` rust
#[derive(Component)]
#[require(
LockedAxes = ROTATION_LOCKED),
)]
pub struct CharacterController;
```
it also works for enum labels.
I chose to omit the type, since were trying to reduce typing here. The
alternative would have been this:
```rust
#[require(
LockedAxes = LockedAxes::ROTATION_LOCKED),
)]
```
This of course has its disadvantages, since the const has to be
associated, but the old closure method is still possible, so I dont
think its a problem.
- Fixes#16720
## Testing
I added one new test in the docs, which also explain the new change. I
also saw that the docs for the required components on line 165 was
missing an assertion, so I added it back in
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
While working on #18058 I realized I could use
`RelationshipTargetCollection::new`, so I added it.
## Solution
- Add `RelationshipTargetCollection::new`
- Add `RelationshipTargetCollection::reserve`. Could generally be useful
when doing micro-optimizations.
- Add `RelationshipTargetCollection::shrink_to_fit`. Rust collections
generally don't shrink when removing elements. Might be a good idea to
call this once in a while.
## Testing
`cargo clippy`
---
## Showcase
`RelationshipSourceCollection` now implements `new`, `reserve` and
`shrink_to_fit` to give greater control over how much memory it
consumes.
## Migration Guide
Any type implementing `RelationshipSourceCollection` now needs to also
implement `new`, `reserve` and `shrink_to_fit`. `reserve` and
`shrink_to_fit` can be made no-ops if they conceptually mean nothing to
a collection.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
correctly load gltfs without explicit bindposes
## Solution
use identity matrices if bindposes are not found.
note: currently does nothing, as gltfs without explicit bindposes fail
to load, see <https://github.com/gltf-rs/gltf/pull/449>
---------
Co-authored-by: François Mockers <francois.mockers@vleue.com>
# Objective
Add a way to efficiently replace a set of specifically related entities
with a new set.
Closes#18041
## Solution
Add new `replace_(related/children)` to `EntityWorldMut` and friends.
## Testing
Added a new test to `hierarchy.rs` that specifically check if
`replace_children` actually correctly replaces the children on a entity
while keeping the original one.
---
## Showcase
`EntityWorldMut` and `EntityCommands` can now be used to efficiently
replace the entities a entity is related to.
```rust
/// `parent` has 2 children. `entity_a` and `entity_b`.
assert_eq!([entity_a, entity_b], world.entity(parent).get::<Children>());
/// Replace `parent`s children with `entity_a` and `entity_c`
world.entity_mut(parent).replace_related(&[entity_a, entity_c]);
/// `parent` now has 2 children. `entity_a` and `entity_c`.
///
/// `replace_children` has saved time by not removing and reading
/// the relationship between `entity_a` and `parent`
assert_eq!([entity_a, entity_c], world.entity(parent).get::<Children>());
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
## Objective
Fix `bevy_ecs` doc tests failing when used with `--all-features`.
```
---- crates\bevy_ecs\src\error\handler.rs - error::handler::GLOBAL_ERROR_HANDLER (line 87) stdout ----
error[E0425]: cannot find function `default_error_handler` in this scope
--> crates\bevy_ecs\src\error\handler.rs:92:24
|
8 | let error_handler = default_error_handler();
| ^^^^^^^^^^^^^^^^^^^^^ not found in this scope
```
I happened to come across this while testing #12207. I'm not sure it
actually needs fixing but seemed worth a go
## Testing
```
cargo test --doc -p bevy_ecs --all-features
```
## Side Notes
The CI misses this error as it doesn't use `--all-features`. Perhaps it
should?
I tried adding `--all-features` to `ci/src/commands/doc_tests.rs` but
this triggered a linker error:
```
Compiling bevy_dylib v0.16.0-dev (C:\Projects\bevy\crates\bevy_dylib)
error: linking with `link.exe` failed: exit code: 1189
= note: LINK : fatal error LNK1189: library limit of 65535 objects exceeded␍
```
# Objective
- Fixes https://github.com/bevyengine/bevy/issues/18332
## Solution
- Move specialize_shadows to ManageViews so that it can run after
prepare_lights, so that shadow views exist for specialization.
- Unfortunately this means that specialize_shadows is no longer in
PrepareMeshes like the rest of the specialization systems.
## Testing
- Ran anti_aliasing example, switched between the different AA options,
observed no glitches.
# Objective
For materials that aren't being used or a visible entity doesn't have an
instance of, we were unnecessarily constantly checking whether they
needed specialization, saying yes (because the material had never been
specialized for that entity), and failing to look up the material
instance.
## Solution
If an entity doesn't have an instance of the material, it can't possibly
need specialization, so exit early before spending time doing the check.
Fixes#18388.
- bevy_dylib fails to build:
```
Compiling bevy_dylib v0.16.0-rc.1 (/bevy/crates/bevy_dylib)
error: linking with `cc` failed: exit status: 1
|
= note: some arguments are omitted. use `--verbose` to show all linker arguments
= note: Undefined symbols for architecture arm64:
"__critical_section_1_0_acquire", referenced from:
critical_section::with::h00cfbe529dea9dc9 in libbevy_tasks-53c9db6a3865f250.rlib[58](bevy_tasks-53c9db6a3865f250.evom2xwveqp508omiiqb25xig.rcgu.o)
"__critical_section_1_0_release", referenced from:
core::ptr::drop_in_place$LT$critical_section..with..Guard$GT$::hfa034e0208e1a49d in libbevy_tasks-53c9db6a3865f250.rlib[48](bevy_tasks-53c9db6a3865f250.d9dwgpd0156zfn2h5z5ff94zn.rcgu.o)
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```
- enable `std` when building bevy_dylib
---------
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
# Objective
- `collect_path` is only declared when feature `bevy_animation` is
enabled
- it is imported without checking for the feature, not compiling when
not enabled
## Solution
- Gate the import
# Objective
@cart noticed some issues with my work in
https://github.com/bevyengine/bevy/pull/17348#discussion_r2001815637,
which I somehow missed before merging the PR.
## Solution
- feature gate the UiPickingPlugin correctly
- don't manually add the picking plugins
## Testing
Ran the debug_picking and sprite_picking examples (for UI and sprites
respectively): both seem to work fine.
# 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.
# Objective
- ECS error handling is a lovely flagship feature for Bevy 0.16, all in
the name of reducing panics and encouraging better error handling
(#14275).
- Currently though, command and system error handling are completely
disjoint and use different mechanisms.
- Additionally, there's a number of distinct ways to set the
default/fallback/global error handler that have limited value. As far as
I can tell, this will be cfg flagged to toggle between dev and
production builds in 99.9% of cases, with no real value in more granular
settings or helpers.
- Fixes#17272
## Solution
- Standardize error handling on the OnceLock global error mechanisms
ironed out in https://github.com/bevyengine/bevy/pull/17215
- As discussed there, there are serious performance concerns there,
especially for commands
- I also think this is a better fit for the use cases, as it's truly
global
- Move from `SystemErrorContext` to a more general purpose
`ErrorContext`, which can handle observers and commands more clearly
- Cut the superfluous setter methods on `App` and `SubApp`
- Rename the limited (and unhelpful) `fallible_systems` example to
`error_handling`, and add an example of command error handling
## Testing
Ran the `error_handling` example.
## Notes for reviewers
- Do you see a clear way to allow commands to retain &mut World access
in the per-command custom error handlers? IMO that's a key feature here
(allowing the ad-hoc creation of custom commands), but I'm not sure how
to get there without exploding complexity.
- I've removed the feature gate on the default_error_handler: contrary
to @cart's opinion in #17215 I think that virtually all apps will want
to use this. Can you think of a category of app that a) is extremely
performance sensitive b) is fine with shipping to production with the
panic error handler? If so, I can try to gather performance numbers
and/or reintroduce the feature flag. UPDATE: see benches at the end of
this message.
- ~~`OnceLock` is in `std`: @bushrat011899 what should we do here?~~
- Do you have ideas for more automated tests for this collection of
features?
## Benchmarks
I checked the impact of the feature flag introduced: benchmarks might
show regressions. This bears more investigation. I'm still skeptical
that there are users who are well-served by a fast always panicking
approach, but I'm going to re-add the feature flag here to avoid
stalling this out.
---------
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
# Objective
Currently, our picking backends are inconsistent:
- Mesh picking and sprite picking both have configurable opt in/out
behavior. UI picking does not.
- Sprite picking uses `SpritePickingCamera` and `Pickable` for control,
but mesh picking uses `RayCastPickable`.
- `MeshPickingPlugin` is not a part of `DefaultPlugins`.
`SpritePickingPlugin` and `UiPickingPlugin` are.
## Solution
- Add configurable opt in/out behavior to UI picking (defaults to opt
out).
- Replace `RayCastPickable` with `MeshPickingCamera` and `Pickable`.
- Remove `SpritePickingPlugin` and `UiPickingPlugin` from
`DefaultPlugins`.
## Testing
Ran some examples.
## Migration Guide
`UiPickingPlugin` and `SpritePickingPlugin` are no longer included in
`DefaultPlugins`. They must be explicitly added.
`RayCastPickable` has been replaced in favor of the `MeshPickingCamera`
and `Pickable` components. You should add them to cameras and entities,
respectively, if you have `MeshPickingSettings::require_markers` set to
`true`.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Fixes https://github.com/bevyengine/bevy/issues/18366 which seems to
have a similar underlying cause than the already closed (but not fixed)
https://github.com/bevyengine/bevy/issues/16185.
## Solution
For Windows with the AMD vulkan driver, there was already a hack to
force serial command encoding, which prevented these issues. The Linux
version of the AMD vulkan driver seems to have similar issues than its
Windows counterpart, so I extended the hack to also cover AMD on Linux.
I also removed the mention of `wgpu` since it was already outdated, and
doesn't seem to be relevant to the core issue (the AMD driver being
buggy).
## Testing
- Did you test these changes? If so, how?
- I ran the `3d_scene` example, which on `main` produced the flickering
shadows on Linux with the amdvlk driver, while it no longer does with
the workaround applied.
- Are there any parts that need more testing?
- Not sure.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- Requires a Linux system with an AMD card and the AMDVLK driver.
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
- My change should only affect Linux, where I did test it.
# Objective
- #17219 introduced a circular dependency between bevy_image and
bevy_sprite for documentation
## Solution
- Remove the circular dependency
- Simplify the doc example
# Objective
Fixes#18357
## Solution
Generalize `RelationshipInsertHookMode` to `RelationshipHookMode`, wire
it up to on_replace execution, and use it in the
`Relationship::on_replace` hook.
# Objective
- https://github.com/bevyengine/bevy/pull/17887 introduced a circular
dependency between bevy_image and bevy_core_pipeline
- This makes it impossible to publish Bevy
## Solution
- Remove the circular dependency, reintroduce the compilation failure
- This failure shouldn't be an issue for users of Bevy, only for users
of subcrates, and can be workaround
- Proper fix should be done with
https://github.com/bevyengine/bevy/issues/17891
- Limited compilation failure is better than publish failure
# Objective
Extract sprites into a `Vec` instead of a `HashMap`.
## Solution
Extract UI nodes into a `Vec` instead of an `EntityHashMap`.
Add an index into the `Vec` to `Transparent2d`.
Compare both the index and render entity in prepare so there aren't any
collisions.
## Showcase
yellow this PR, red main
```
cargo run --example many_sprites --release --features "trace_tracy"
```
`extract_sprites`
<img width="452" alt="extract_sprites"
src="https://github.com/user-attachments/assets/66c60406-7c2b-4367-907d-4a71d3630296"
/>
`queue_sprites`
<img width="463" alt="queue_sprites"
src="https://github.com/user-attachments/assets/54b903bd-4137-4772-9f87-e10e1e050d69"
/>
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- Fixes#17506
- Fixes#16258
## Solution
- Added a new folder of examples, `no_std`, similar to the `mobile`
folder.
- Added a single example, `no_std_library`, which demonstrates how to
make a `no_std` compatible Bevy library.
- Added a new CI task, `check-compiles-no-std-examples`, which checks
that `no_std` examples compile on `no_std` targets.
- Added `bevy_platform_support::prelude` to `bevy::prelude`.
## Testing
- CI
---
## Notes
- I've structured the folders here to permit further `no_std` examples
(e.g., GameBoy Games, ESP32 firmware, etc.), but I am starting with the
simplest and least controversial example.
- I've tried to be as clear as possible with the documentation for this
example, catering to an audience who may not have even heard of `no_std`
before.
---------
Co-authored-by: Greeble <166992735+greeble-dev@users.noreply.github.com>
# Objective
I experienced an issue where `HashMap::new` was not returning a value
typed appropriately for a `HashMap<K,V>` declaration that omitted the
Hasher- e.g. the Default Hasher for the type is different than what the
`new` method produces.
After discussion on discord, this appears to be an issue in `hashbrown`,
and working around it would be very nontrivial, requiring a newtype on
top of the `hashbrown` implementation. Rather than doing that, it was
suggested that we add docs to make the issue more visible and provide a
clear workaround.
## Solution
Updated the docs for `bevy_platform_support::collections`. I couldn't
update Struct docs because they're re-exports, so I had to settle for
the module.
Note that the `[HashMap::new]` link wasn't generating properly- I'm not
sure why. I see the method in the docs.rs site,
https://docs.rs/hashbrown/0.15.1/hashbrown/struct.HashMap.html#method.new,
but not on the generated internal documentation. I wonder if `hashbrown`
isn't actually implementing the new or something?
## Testing
n/a although I did generate and open the docs on my Ubuntu machine.
---
## Showcase
before:
after:
---------
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
# Objective
WESL was broken on windows.
## Solution
- Upgrade to `wesl_rs` 1.2.
- Fix path handling on windows.
- Improve example for khronos demo this week.
# Objective
- #17581 broke gizmos
- Fixes#18325
## Solution
- Revert #17581
- Add gizmos to testbed
## Testing
- Run any example with gizmos, it renders correctly
# Objective
On iOS:
- Allow `std` to do its runtime initialization.
- Avoid requiring the user to specify linked libraries and framework in
Xcode.
- Reduce the amount of work that `#[bevy_main]` does
- In the future we may also be able to eliminate the need for it on
Android, cc @daxpedda.
## Solution
We previously:
- Exposed an `extern "C" fn main_rs` entry point.
- Ran Cargo in a separate Xcode target as an external build system.
- Imported that as a dependency of `bevy_mobile_example.app`.
- Compiled a trampoline C file with Xcode that called `main_rs`.
- Linked that via. Xcode.
All of this is unnecessary; `rustc` is well capable of creating iOS
executables, the trick is just to place it at the correct location for
Xcode to understand it, namely `$TARGET_BUILD_DIR/$EXECUTABLE_PATH`
(places it in `bevy_mobile_example.app/bevy_mobile_example`).
Note: We might want to wait with the changes to `#[bevy_main]` until the
problem is resolved on Android too, to make the migration easier.
## Testing
Open the Xcode project, and build for an iOS target.
---
## Migration Guide
**If you have been building your application for iOS:**
Previously, the `#[bevy_main]` attribute created a `main_rs` entry point
that most Xcode templates were using to run your Rust code from C. This
was found to be unnecessary, as you can simply let Rust build your
application as a binary, and run that directly.
You have two options for dealing with this:
If you've added further C code and Xcode customizations, or it makes
sense for your use-case to continue link with Xcode, you can revert to
the old behaviour by adding `#[no_mangle] extern "C" main_rs() { main()
}` to your `main.rs`. Note that the old approach of linking a static
library prevents the Rust standard library from doing runtime
initialization, so certain functionality provided by `std` might be
unavailable (stack overflow handlers, stdout/stderr flushing and other
such functionality provided by the initialization routines).
The other, preferred option is to remove your "compile" and "link" build
phases, and instead replace it with a "run script" phase that invokes
`cargo build --bin ...`, and moves the built binary to the Xcode path
`$TARGET_BUILD_DIR/$EXECUTABLE_PATH`. An example of how to do this can
be viewed at [INSERT LINK TO UPDATED EXAMPLE PROJECT].
To make the debugging experience in Xcode nicer after this, you might
also want to consider either enabling `panic = "abort"` or to set a
breakpoint on the `rust_panic` symbol.
---------
Co-authored-by: François Mockers <mockersf@gmail.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
# Objective
As pointed out by @cart on
[Discord](https://discord.com/channels/691052431525675048/1002362493634629796/1351279139872571462),
we should be careful when using tuple shorthand to register types. Doing
so incurs some unnecessary penalties such as memory/compile/performance
cost to generate registrations for a tuple type that will never be used.
A better solution would be to create a custom lint for this, but for now
we can at least remove the existing usages of this pattern.
> [!note]
> This pattern of using tuples to register multiple types at once isn't
inherently bad. Users should feel free to use this pattern, knowing the
side effects it may have. What this problem really is about is using
this in _library_ code, where users of Bevy have no choice in whether a
tuple is unnecessarily registered in an internal plugin or not.
## Solution
Replace tuple registrations with single-type registrations.
Note that I left the tuple registrations in test code since I feel like
brevity is more important in those cases. But let me know if I should
change them or leave a comment above them!
## Testing
You can test locally by running:
```
cargo check --workspace --all-features
```
# 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.
# Objective
Continuation of #17449.
#17449 implemented the wrapper types around `IndexMap`/`Set` and co.,
however punted on the slice types.
They are needed to support creating `EntitySetIterator`s from their
slices, not just the base maps and sets.
## Solution
Add the wrappers, in the same vein as #17449 and #17589 before.
The `Index`/`IndexMut` implementations take up a lot of space, however
they cannot be merged because we'd then get overlaps.
They are simply named `Slice` to match the `indexmap` naming scheme, but
this means they cannot be differentiated properly until their modules
are made public, which is already a follow-up mentioned in #17954.
# 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
```
# Objective
Add a `UiRect::AUTO` const which is a `UiRect` with all its edge values
set to `Val::Auto`.
IIRC `UiRect`'s default for its fields a few versions ago was
`Val::Auto` because positions were represented using a `UiRect` and they
required `Val::Auto` as a default. Then when position was split up and
the `UiRect` default was changed, we forgot add a `UiRect::AUTO` const.
Extracted from my DLSS branch.
## Changelog
* Added `source_texture` and `destination_texture` to
`PostProcessWrite`, in addition to the existing texture views.
obtaining a reference to an empty enum is not possible in Rust, so I
just replaced any match on self with an `unreachable!()`
I checked if an enum is empty by checking if the `variant_patterns` vec
of the `EnumVariantOutputData` struct is empty
Fixes#18277
## Testing
I added one new unit test.
``` rust
#[test]
fn should_allow_empty_enums() {
#[derive(Reflect)]
enum Empty {}
assert_impl_all!(Empty: Reflect);
}
```
# Objective
In its existing form, the clamping that's done in `camera_system`
doesn't work well when the `physical_position` of the associated
viewport is nonzero. In such cases, it may produce invalid viewport
rectangles (i.e. not lying inside the render target), which may result
in crashes during the render pass.
The goal of this PR is to eliminate this possibility by making the
clamping behavior always result in a valid viewport rectangle when
possible.
## Solution
Incorporate the `physical_position` information into the clamping
behavior. In particular, always cut off enough so that it's contained in
the render target rather than clamping it to the same dimensions as the
target itself. In weirder situations, still try to produce a valid
viewport rectangle to avoid crashes.
## Testing
Tested these changes on my work branch where I encountered the crash.
# Objective
Currently experimenting with manually implementing
`Relationship`/`RelationshipTarget` to support associated edge data,
which means I need to replace the default hook implementations provided
by those traits. However, copying them over for editing revealed that
`UnsafeWorldCell::get_raw_command_queue` is `pub(crate)`, and I would
like to not have to clone the source collection, like the default impl.
So instead, I've taken to providing a safe abstraction for being able to
access entities and queue commands simultaneously.
## Solution
Added `World::entities_and_commands` and
`DeferredWorld::entities_and_commands`, which can be used like so:
```rust
let eid: Entity = /* ... */;
let (mut fetcher, mut commands) = world.entities_and_commands();
let emut = fetcher.get_mut(eid).unwrap();
commands.entity(eid).despawn();
```
## Testing
- Added a new test for each of the added functions.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Continuation to #16547 and #17954.
The `get_many` family are the last methods on `Query`/`QueryState` for
which we're still missing a `unique` version.
## Solution
Offer `get_many_unique`/`get_many_unique_mut` and
`get_many_unique_inner`!
Their implementation is the same as `get_many`, the difference lies in
their guaranteed-to-be unique inputs, meaning we never do any aliasing
checks.
To reduce confusion, we also rename `get_many_readonly` into
`get_many_inner` and the current `get_many_inner` into
`get_many_mut_inner` to clarify their purposes.
## Testing
Doc examples.
## Migration Guide
`get_many_inner` is now called `get_many_mut_inner`.
`get_many_readonly` is now called `get_many_inner`.
# Objective
- Fixes#18342.
## Solution
- Canonicalize the root path so that when we try to strip the prefix, it
matches the canonicalized asset path.
- This is basically just a followup to #18023.
## Testing
- Ran the hot_asset_reloading example and it no longer panics.
# Objective
The `ViewportConversionError` error type does not implement `Error`,
making it incompatible with `BevyError`.
## Solution
Derive `Error` for `ViewportConversionError`.
I chose to use `thiserror` since it's already a dependency, but do let
me know if we should be preferring `derive_more`.
## Testing
You can test this by trying to compile the following:
```rust
let error: BevyError = ViewportConversionError::InvalidData.into();
```
Registering dynamic bundles was not possible for the user yet.
It is alone not very useful though as there are no methods to clone,
move or remove components via a `BundleId`. This could be a follow-up
work if this PR is approved and such a third (besides `T: Bundle` and
`ComponentId`(s)) API for structural operation is desired. I certainly
would use a hypothetical `EntityClonerBuilder::allow_by_bundle_id`.
I personally still would like this register method because I have a
`Bundles`-like custom data structure and I would like to not reinvent
the wheel. Then instead of having boxed `ComponentId` slices in my
collection I could look up explicit and required components there.
For reference scroll up to the typed version right above the new one.
# 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.
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.
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.
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.
# 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.
# 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`
# 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).
# 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~~
# 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.
# 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>
# 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>
# 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.
# 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.
# 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>
# 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.
# 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.
---
# 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`
# 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
# 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`
# 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.
# 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.
_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>
# 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>
# 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>
# 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>
