# Objective
- Fixes#10478
## Solution
Generalised `From/Into` implementations over `&str` and `Option<&str>`
for `AssetSourceId` and `AssetPath` across all lifetimes, not just
static. To maintain access to the `'static`-only specialisation, these
types (and `CowArc`) now include an `as_static` method which will apply
the specialisation.
```rust
// Snipped from `AssetApp`
fn register_asset_source(
&mut self,
id: impl Into<AssetSourceId<'static>>,
// ^^^^^^^
// | as_static is only available for 'static lifetimes
source: AssetSourceBuilder,
) -> &mut Self {
let id = id.into().as_static();
// ^^^^^^ ^^^^^^^^^
// | | Specialized (internally storing CowArc::Static)
// | Generic Into (internally storing CowArc::Borrowed)
// ...
}
```
This post-fix specialisation is available here because the actual
specialisation performed is only a marker for if/when modification or
ownership is required, making the transform a very cheap operation. For
cleanliness, I've also added `from_static`, which wraps this behaviour
in a clean shorthand designed to replace `from` calls.
---
## Changelog
- Generalised the following implementations over a generic lifetime:
- `From<&'static str> for AssetSourceId<'static>`
- `From<Option<&'static str>> for AssetSourceId<'static>`
- `From<&'static str> for AssetPath<'static>`
- `From<&'static Path> for AssetPath<'static>`
- Added `as_static` specialisation to:
- `CowArc`
- `AssetSourceId`
- `AssetPath`
- Added `from_static` specialised constructor to:
- `AssetSourceId`
- `AssetPath`
## Migration Guide
In areas where these implementations where being used, you can now add
`from_static` in order to get the original specialised implementation
which avoids creating an `Arc` internally.
```rust
// Before
let asset_path = AssetPath::from("my/path/to/an/asset.ext");
// After
let asset_path = AssetPath::from_static("my/path/to/an/asset.ext");
```
To be clear, this is only required if you wish to maintain the
performance benefit that came with the specialisation. Existing code is
_not_ broken by this change.
# Objective
The `target` directory in `/tools/compile_fail_utils` was not being
ignored by Git.
## Solution
Added `/tools/compile_fail_utils/target` to the root `.gitignore` in the
same spirit as the other top-level, workspace-excluded crate: `benches`.
# Objective
One of the changes in #14704 made `DynamicFunction` effectively the same
as `DynamicClosure<'static>`. This change meant that the de facto
function type would likely be `DynamicClosure<'static>` instead of the
intended `DynamicFunction`, since the former is much more flexible.
We _could_ explore ways of making `DynamicFunction` implement `Copy`
using some unsafe code, but it likely wouldn't be worth it. And users
would likely still reach for the convenience of
`DynamicClosure<'static>` over the copy-ability of `DynamicFunction`.
The goal of this PR is to fix this confusion between the two types.
## Solution
Firstly, the `DynamicFunction` type was removed. Again, it was no
different than `DynamicClosure<'static>` so it wasn't a huge deal to
remove.
Secondly, `DynamicClosure<'env>` and `DynamicClosureMut<'env>` were
renamed to `DynamicFunction<'env>` and `DynamicFunctionMut<'env>`,
respectively.
Yes, we still ultimately kept the naming of `DynamicFunction`, but
changed its behavior to that of `DynamicClosure<'env>`. We need a term
to refer to both functions and closures, and "function" was the best
option.
[Originally](https://discord.com/channels/691052431525675048/1002362493634629796/1274091992162242710),
I was going to go with "callable" as the replacement term to encompass
both functions and closures (e.g. `DynamciCallable<'env>`). However, it
was
[suggested](https://discord.com/channels/691052431525675048/1002362493634629796/1274653581777047625)
by @SkiFire13 that the simpler "function" term could be used instead.
While "callable" is perhaps the better umbrella term—being truly
ambiguous over functions and closures— "function" is more familiar, used
more often, easier to discover, and is subjectively just
"better-sounding".
## Testing
Most changes are purely swapping type names or updating documentation,
but you can verify everything still works by running the following
command:
```
cargo test --package bevy_reflect
```
# Objective
currently if an asset loader panics, the asset is left in a perpetual
`Loading` state. this can occur with external crates (eg the image crate
panics on bad data). catch this panic and resolve the asset to `Failed`
## Solution
`AssertUnwindSafe(loader.load).catch_unwind()` and map the panic to an
`AssetLoadError`
separated out from #13170
# Objective
- Fix error when closing window in 2d_viewport_to_world example.
Before
```
2024-08-17T22:51:47.690252Z INFO bevy_winit::system: Creating new window "App" (0v1#4294967296)
2024-08-17T22:52:22.062959Z INFO bevy_window::system: No windows are open, exiting
2024-08-17T22:52:22.064045Z INFO bevy_winit::system: Closing window 0v1#4294967296
thread 'Compute Task Pool (5)' panicked at examples/2d/2d_viewport_to_world.rs:20:41:
called `Result::unwrap()` on an `Err` value: NoEntities("bevy_ecs::query::state::QueryState<&bevy_window:🪟:Window>")
```
After
```
2024-08-17T22:57:31.623499Z INFO bevy_winit::system: Creating new window "App" (0v1#4294967296)
2024-08-17T22:57:32.990058Z INFO bevy_window::system: No windows are open, exiting
2024-08-17T22:57:32.991152Z INFO bevy_winit::system: Closing window 0v1#4294967296
2024-08-17T22:57:32.994426Z INFO bevy_window::system: No windows are open, exiting
* Terminal will be reused by tasks, press any key to close it.
```
## Solution
- Check if the window still exists before drawing the cursor
# Objective
While looking through the changes #14782 will create I noticed this.
## Solution
Reuse the existing thread_rng. As this is a code change I would like to
not include it in a pure lint enable PR.
## Testing
I did not test this change (other than the automated CI with this PR). I
think it should be a fairly simple change that can be reviewed only by
the code.
# Objective
Delete some code that isn't actually doing anything. This was actually
discovered way back in this obsolete PR: #5513.
Also Fixes#6286
## Solution
Delete it
## Alternatives
Make `Direction` do things. But it's not totally clear to me if it's
possible to override cosmic-text's unicode bidi stuff.
## Migration Guide
`Style` no longer has a `direction` field, and `Direction` has been
deleted. They didn't do anything, so you can delete any references to
them as well.
# Objective
Fixes#14521.
## Solution
Added to methods to the VIsibility.
```rs
is_visible() -> Result<bool, String>
```
and
```rs
visbility_from_bool(bool) -> Visibility
```
## Testing
Ran
* `cargo run -p ci -- lints`
* `cargo run -p ci -- test`
* `cargo run -p ci -- compile`
it seems to be working.
However I got few error messages :`ERROR bevy_log: could not set global
logger and tracing subscriber as they are already set. Consider
disabling LogPlugin` in `cargo run -p ci -- test`, even though all test
passed. I'm not sure if that's expected behaviour
Ps. I'm new to contributing, please correct me if anything is wrong
# Objective
Some algorithms don't really work well or are not efficient in 3D space.
When we know we have points on an `InfinitePlane3d` it would be helpful
to have some utility methods to reversibly transform points on the plane
to 2D space to apply some algorithms there.
## Solution
This PR adds a few of methods to project 3D points on a plane to 2D
points and inject them back. Additionally there are some other small
common helper methods.
## Testing
- added some tests that cover the new methods
---------
Co-authored-by: Matty <weatherleymatthew@gmail.com>
# Objective
When reading the ECS code it is sometimes confusing to understand why we
have 2 accesses, one of ComponentId and one of ArchetypeComponentId
## Solution
Make the usage of these 2 accesses more explicit
---------
Co-authored-by: Pascal Hertleif <killercup@gmail.com>
# Objective
Fixes Commands not being `Send` or `Sync` anymore in 0.14 by
implementing `Send` and `Sync` for `RawCommandQueue`.
## Solution
Reference discussion in
[discord](https://discord.com/channels/691052431525675048/691052431974465548/1259464518539411570).
It seems that in https://github.com/bevyengine/bevy/pull/13249, when
adding a `RawCommandQueue` variant to the `InternalQueue`, the `Send /
Sync` traits were not implemented for it, which bubbled up all the way
to `Commands` not being `Send / Sync` anymore.
I am not very familiar with the ECS internals so I can't say whether the
`RawCommandQueue` is safe to be shared between threads, but I know for
sure that before the linked PR `Commands` were indeed `Send` and `Sync`
so that PR broke "some workflows" (mandatory
[xkcd](https://xkcd.com/1172/)).
## Testing
This PR itself includes a compile test to make sure `Commands` will
implement `Send` and `Sync`. The test itself fails without the
implementation and succeeds with it.
Furthermore, if I cherry pick the test to a previous release (i.e. 0.13)
it indeed succeeds, showing that this is a regression specific to 0.14.
---------
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
# Objective
`MeshVertexAttributeId` is currently a wrapper type around a `usize`.
Application developers are exposed to the `usize` whenever they need to
define a new custom vertex attribute, which requires them to generate a
random `usize` ID to avoid clashes with any other custom vertex
attributes in the same application. As the range of a `usize` is
platform dependent, developers on 64-bit machines may inadvertently
generate random values which will fail to compile for a 32-bit target.
The use of a `usize` here encourages non-portable behaviour and should
be replaced with a fixed width type.
## Solution
In this PR I have changed the ID type from `usize` to `u64`, but equally
a `u32` could be used at the risk of breaking some extant non-portable
programs and increasing the chance of an ID collision.
# Objective
Fixes#14811
## Solution
- Switch `D` to `T`: `T` for "on top of"
- Switch `A` to `B`: `B` in "AABB", or "boxes"
## Testing
- Ran the example locally
- Checked the key bindings that the camera controller uses and made sure
we're not using them in the 3d_gizmos example anymore
After:
<img width="1278" alt="image"
src="https://github.com/user-attachments/assets/4f558d09-5acf-4eb8-8ece-6d4297e62c9f">
# Objective
#14098 added the `FunctionRegistry` for registering functions such that
they can be retrieved by name and used dynamically. One thing we chose
to leave out in that initial PR is support for closures.
Why support closures? Mainly, we don't want to prohibit users from
injecting environmental data into their registered functions. This
allows these functions to not leak their internals to the public API.
For example, let's say we're writing a library crate that allows users
to register callbacks for certain actions. We want to perform some
actions before invoking the user's callback so we can't just call it
directly. We need a closure for this:
```rust
registry.register("my_lib::onclick", move |event: ClickEvent| {
// ...other work...
user_onclick.call(event); // <-- Captured variable
});
```
We could have made our callback take a reference to the user's callback.
This would remove the need for the closure, but it would change our
desired API to place the burden of fetching the correct callback on the
caller.
## Solution
Modify the `FunctionRegistry` to store registered functions as
`DynamicClosure<'static>` instead of `DynamicFunction` (now using
`IntoClosure` instead of `IntoFunction`).
Due to limitations in Rust and how function reflection works,
`DynamicClosure<'static>` is functionally equivalent to
`DynamicFunction`. And a normal function is considered a subset of
closures (it's a closure that doesn't capture anything), so there
shouldn't be any difference in usage: all functions that satisfy
`IntoFunction` should satisfy `IntoClosure`.
This means that the registration API introduced in #14098 should require
little-to-no changes on anyone following `main`.
### Closures vs Functions
One consideration here is whether we should keep closures and functions
separate.
This PR unifies them into `DynamicClosure<'static>`, but we can consider
splitting them up. The reasons we might want to do so are:
- Simplifies mental model and terminology (users don't have to
understand that functions turn into closures)
- If Rust ever improves its function model, we may be able to add
additional guarantees to `DynamicFunction` that make it useful to
separate the two
- Adding support for generic functions may be less confusing for users
since closures in Rust technically can't be generic
The reasons behind this PR's unification approach are:
- Reduces the number of methods needed on `FunctionRegistry`
- Reduces the number of lookups a user may have to perform (i.e.
"`get_function` or else `get_closure`")
- Establishes `DynamicClosure<'static>` as the de facto dynamic callable
(similar to how most APIs in Rust code tend to prefer `impl Fn() ->
String` over `fn() -> String`)
I'd love to hear feedback on this matter, and whether we should continue
with this PR's approach or switch to a split model.
## Testing
You can test locally by running:
```
cargo test --package bevy_reflect
```
---
## Showcase
Closures can now be registered into the `FunctionRegistry`:
```rust
let punct = String::from("!!!");
registry.register_with_name("my_crate::punctuate", move |text: String| {
format!("{}{}", text, punct)
});
```
# Objective
The `prepare_view_upscaling_pipelines` system has dozens of ambiguities,
which makes it harder to spot and prevent new ambiguities.
Closes https://github.com/bevyengine/bevy/issues/14770.
## Solution
Just exclude the system from ambiguity detection. See the linked issue
for more context on why this resolution was chosen.
## Testing
Running the `ambiguity_detection` example now reports dozens fewer
`Render` app ambiguities.
# Objective
- Add "Available on crate feature <image format> only." for docs of
image format related types/functions
- Add warning "WARN bevy_render::texture::image: feature "<image
format>" is not enabled" on load attempt
- Fixes#13468 .
## Solution
- Added #[cfg(feature = "<image format>")] for types and warn!("feature
\"<image format>\" is not enabled"); for ImageFormat enum conversions
## Testing
ran reproducing example from issue #13468 and saw in logs
`WARN bevy_render::texture::image: feature "exr" is not enabled`
generated docs with command `RUSTDOCFLAGS="-Zunstable-options
--cfg=docsrs" cargo +nightly doc --workspace --all-features --no-deps
--document-private-items --open` and saw
that docs contain `Available on crate feature <image format> only.`
marks
## Migration Guide
Image format related entities are feature gated, if there are
compilation errors about unknown names there are some of features in
list (`exr`, `hdr`, `basis-universal`, `png`, `dds`, `tga`, `jpeg`,
`bmp`, `ktx2`, `webp` and `pnm`) should be added.
# Objective
- Fixes#14655
## Solution
Rotation should happen first as this is more easier to conceptualize in
the mind: We rotate around the coordinate origin `Vec3::ZERO` and then
we just shift the geometry so that its center is exactly on the
specified position
## Testing && Showcase
Code:
```rust
gizmos.grid(
Vec3::ONE * 10.0,
Quat::from_rotation_x(PI / 3. * 2.),
UVec2::splat(20),
Vec2::new(2., 2.),
PURPLE,
);
gizmos.sphere(Vec3::ONE * 10.0, Quat::default(), 1.0, PURPLE);
```
Before picture:
After picture:
## Migration Guide
- Users might have to double check their already existing calls to all
the `grid` methods. It should be more intuitive now though.
# Objective
Fix#14771 by adding a `try_insert_if_new` method to the
`EntityCommands`
## Solution
This simply calls the `try_insert` function with `InsertMode::Keep`
## Testing
I did not add any test because `EntityCommands::try_insert` does not
seem to be tested either. I can add some if needed.
# Objective
Fixes#14736
## Solution
Enables feature `bevy_render` for dependency `bevy_gizmos` in
`bevy_dev_tools` cargo.
`bevy_dev_tools` has `bevy_render` as a required dependency, whereas it
is optional for `bevy_gizmos`. When building with no default features,
this causes gizmos to not compile with `bevy_render` features, meaning
some fields and code are not available. Since these features are
required in dev tools, it makes sense to ensure they are enabled. Making
`bevy_render` optional would introduce additional and potentially
unwanted change wake. in dev tools.
## Testing
Reproed and tested on Windows 10, issue originally reported on Ubuntu
and MacOS.
- Original issue command completed without error: `cargo c -p bevy
--no-default-features -F bevy_dev_tools`
- Ran full ci validations with `cargo run -p ci`
# Objective
Finish what we started in #14630. The Curve RFC is
[here](https://github.com/bevyengine/rfcs/blob/main/rfcs/80-curve-trait.md).
## Solution
This contains the rest of the library from my branch. The main things
added here are:
- Bulk sampling / resampling methods on `Curve` itself
- Data structures supporting the above
- The `cores` submodule that those data structures use to encapsulate
sample interpolation
The weirdest thing in here is probably `ChunkedUnevenCore` in `cores`,
which is not used by anything in the Curve library itself but which is
required for efficient storage of glTF animation curves. (See #13105.)
We can move it into a different PR if we want to; I don't have strong
feelings either way.
## Testing
New tests related to resampling are included. As I write this, I realize
we could use some tests in `cores` itself, so I will add some on this
branch before too long.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Robert Walter <26892280+RobWalt@users.noreply.github.com>
# Objective
- The [developer docs](https://dev-docs.bevyengine.org) are primarily
used by developers, who often deal with internal and private functions.
- @mweatherley suggested [on
Discord](https://discord.com/channels/691052431525675048/743559241461399582/1273658470134186112)
that CI passes `--document-private-items` to the dev-docs, so
contributors can easily browse these internal details.
## Solution
- Add `--document-private-items` to the CI job that builds the dev-docs.
## Testing
- It should run in CI and generate documentation for private code.
## Drawbacks
- Users that track the main branch may use the dev-docs instead of
<https://docs.rs/bevy>, which may now show a lot of unwanted internal
details.
- Searching may be slower / bloated with internal details.
# Objective
fixes#14569
## Solution
added an example to the diagnostic examples and linked the code to the
docs of the diagnostic library itself.
## Testing
I tested locally on my laptop in a web browser. Looked fine. You are
able to collapse the whole "intro" part of the doc to get to the links
sooner (for those who may think that including the example code here is
annoying to scroll through)
I would like people to run ```cargo doc``` and go the bevy_diagnostic
page to see if they have any issues or suggestions.
---
## Showcase
<img width="1067" alt="Screenshot 2024-08-14 at 12 52 16"
src="https://github.com/user-attachments/assets/70b6c18a-0bb9-4656-ba53-c416f62c6116">
---------
Co-authored-by: dpeke <dpekelis@funstage.com>
# Objective
This `apply_deferred` doesn't seem to have any effect, pointlessly
restricts parallelism and is responsible for a large number of system
order ambiguities. Spotted as part of #7386.
## Solution
Remove it.
This is the *only* manual apply_deferred in the code base currently.
## Testing
I've checked various UI examples and `split_screen`, and couldn't
discern any difference.
This looks like a remnant of a `(a, apply_deferred, b).chain()` pattern
where `b` got removed, leaving us with a weird vestige.
# Objective
Often there are reasons to insert some components (e.g. Transform)
separately from the rest of a bundle (e.g. PbrBundle). However `insert`
overwrites existing components, making this difficult.
See also issue #14397Fixes#2054.
## Solution
This PR adds the method `insert_if_new` to EntityMut and Commands, which
is the same as `insert` except that the old component is kept in case of
conflicts.
It also renames some internal enums (from `ComponentStatus::Mutated` to
`Existing`), to reflect the possible change in meaning.
## Testing
*Did you test these changes? If so, how?*
Added basic unit tests; used the new behavior in my project.
*Are there any parts that need more testing?*
There should be a test that the change time isn't set if a component is
not overwritten; I wasn't sure how to write a test for that case.
*How can other people (reviewers) test your changes? Is there anything
specific they need to know?*
`cargo test` in the bevy_ecs project.
*If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?*
Only tested on Windows, but it doesn't touch anything platform-specific.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Giacomo Stevanato <giaco.stevanato@gmail.com>
# Objective
`World::clear_entities` is ambiguous with all of the other systems in
`RenderSet::Cleanup` because it access `&mut World`.
## Solution
I've added another system set variant, and made sure that this runs
after everything else.
## Testing
The `ambiguity_detection` example
## Migration Guide
`World::clear_entities` is now part of `RenderSet::PostCleanup` rather
than `RenderSet::Cleanup`. Your cleanup systems should likely stay in
`RenderSet::Cleanup`.
## Additional context
Spotted when working on #7386: this was responsible for a large number
of ambiguities.
This should be removed if / when #14449 is merged: there's no need to
call `clear_entities` at all if the rendering world is retained!
# Objective
When an item in the transparent 2d phase fails to render, bevy crashes
with _"PassSpanScope::end was never called"_ instead of outputting the
actual error to the console. This PR fixes this so that phase errors are
output to the console. It also makes bevy not crash.
```
thread '<unnamed>' panicked at /Users/brianreavis/Repositories/project/bevy/crates/bevy_render/src/diagnostic/mod.rs:157:9:
PassSpanScope::end was never called
stack backtrace:
0: rust_begin_unwind
at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/panicking.rs:652:5
1: core::panicking::panic_fmt
at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/panicking.rs:72:14
2: <bevy_render::diagnostic::PassSpanGuard<R,P> as core::ops::drop::Drop>::drop
at /Users/brianreavis/Repositories/project/bevy/crates/bevy_render/src/diagnostic/mod.rs:157:9
3: core::ptr::drop_in_place<bevy_render::diagnostic::PassSpanGuard<core::option::Option<alloc::sync::Arc<bevy_render::diagnostic::internal::DiagnosticsRecorder>>,bevy_render::render_phase::draw_state::TrackedRenderPass>>
at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/ptr/mod.rs:514:1
4: <bevy_core_pipeline::core_2d::main_transparent_pass_2d_node::MainTransparentPass2dNode as bevy_render::render_graph::node::ViewNode>::run
at /Users/brianreavis/Repositories/project/bevy/crates/bevy_core_pipeline/src/core_2d/main_transparent_pass_2d_node.rs:75:9
5: <bevy_render::render_graph::node::ViewNodeRunner<T> as bevy_render::render_graph::node::Node>::run
at /Users/brianreavis/Repositories/project/bevy/crates/bevy_render/src/render_graph/node.rs:406:9
6: bevy_render::renderer::graph_runner::RenderGraphRunner::run_graph
at /Users/brianreavis/Repositories/project/bevy/crates/bevy_render/src/renderer/graph_runner.rs:226:21
7: bevy_render::renderer::graph_runner::RenderGraphRunner::run_graph
at /Users/brianreavis/Repositories/project/bevy/crates/bevy_render/src/renderer/graph_runner.rs:233:21
8: bevy_render::renderer::graph_runner::RenderGraphRunner::run
at /Users/brianreavis/Repositories/project/bevy/crates/bevy_render/src/renderer/graph_runner.rs:81:9
9: bevy_render::renderer::render_system
at /Users/brianreavis/Repositories/project/bevy/crates/bevy_render/src/renderer/mod.rs:40:15
10: core::ops::function::FnMut::call_mut
at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/ops/function.rs:166:5
11: core::ops::function::impls::<impl core::ops::function::FnMut<A> for &mut F>::call_mut
at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/ops/function.rs:294:13
12: <Func as bevy_ecs::system::exclusive_function_system::ExclusiveSystemParamFunction<fn(F0) .> Out>>::run::call_inner
at /Users/brianreavis/Repositories/project/bevy/crates/bevy_ecs/src/system/exclusive_function_system.rs:229:21
13: <Func as bevy_ecs::system::exclusive_function_system::ExclusiveSystemParamFunction<fn(F0) .> Out>>::run
at /Users/brianreavis/Repositories/project/bevy/crates/bevy_ecs/src/system/exclusive_function_system.rs:232:17
14: <bevy_ecs::system::exclusive_function_system::ExclusiveFunctionSystem<Marker,F> as bevy_ecs::system::system::System>::run::{{closure}}
at /Users/brianreavis/Repositories/project/bevy/crates/bevy_ecs/src/system/exclusive_function_system.rs:124:23
15: bevy_ecs::world::World::last_change_tick_scope
at /Users/brianreavis/Repositories/project/bevy/crates/bevy_ecs/src/world/mod.rs:2383:9
16: <bevy_ecs::system::exclusive_function_system::ExclusiveFunctionSystem<Marker,F> as bevy_ecs::system::system::System>::run
at /Users/brianreavis/Repositories/project/bevy/crates/bevy_ecs/src/system/exclusive_function_system.rs:116:9
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
```
## Solution
Matched the behavior of the other render phases ([like
here](9ca5540b75/crates/bevy_core_pipeline/src/core_3d/main_opaque_pass_3d_node.rs (L98-L101)))
# Objective
While tackling https://github.com/bevyengine/bevy/issues/7386, I noticed
a few nits / frustrations with the existing testing infrastructure.
Rather than mixing those changes in with much more challenging to review
changes to reduce ambiguities, I've split this work into its own PR.
## Solution
Substantially simplify the `ambiguity_detection` test code, and reduce
the verbosity of logging.
## Testing
When run locally, the test functions as expected, with somewhat cleaner
logging.
---------
Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
# Objective
- Sometimes some method or function takes an owned `Query`, but we don't
want to give up ours;
- transmuting it technically a solution, but it more costly than
necessary.
- Make query iterators more flexible
- this would allow the equivalent of
`slice::split_first`/`slice::split_first_mut` for query iterators
- helps with requests like #14685
## Solution
- Add a way for reborrowing queries, that is going from a `&'a mut
Query<'w, 's, D, F>` to a `Query<'a, 's, D, F>`:
- this is safe because the original query will be borrowed while the new
query exists and thus no aliased access can happen;
- it's basically the equivalent of going from `&'short mut &'long mut T`
to `&'short mut T` the the compiler automatically implements.
- Add a way for getting the remainder of a query iterator:
- this is interesting also because the original iterator keeps its
position, which was not possible before;
- this in turn requires a way to reborrow query fetches, which I had to
add to `WorldQuery`.
## Showcase
- You can now reborrow a `Query`, getting an equivalent `Query` with a
shorter lifetime. Previously this was possible for read-only queries by
using `Query::to_readonly`, now it's possible for mutable queries too;
- You can now separately iterate over the remainder of `QueryIter`.
## Migration Guide
- `WorldQuery` now has an additional `shrink_fetch` method you have to
implement if you were implementing `WorldQuery` manually.
# Objective
`Res` and `ResMut` perform redundant lookups of the resource storage,
first to initialize the `ArchetypeComponentId` and then to retrieve it.
## Solution
Use the `archetype_component_id` returned from
`initialize_resource_internal` to avoid an extra lookup and `unwrap()`.
# Objective
fix#14742
## Solution
the issue arises because "finished" animations (where current time >=
last keyframe time) are not applied at all.
when transitioning from a finished animation to another later-indexed
anim, the transition kind-of works because the finished anim is skipped,
then the new anim is applied with a lower weight (weight / total_weight)
when transitioning from a finished animation to another earlier-indexed
anim, the transition is instant as the new anim is applied with 1.0 (as
weight == total_weight for the first applied), then the finished
animation is skipped.
to fix this we can always apply every animation based on the nearest 2
keyframes, and clamp the interpolation between them to [0,1].
pros:
- finished animations can be transitioned out of correctly
- blended animations where some curves have a last-keyframe before the
end of the animation will blend properly
- animations will actually finish on their last keyframe, rather than a
fraction of a render-frame before the end
cons:
- we have to re-apply finished animations every frame whether it's
necessary or not. i can't see a way to avoid this.
Makes the newly merged picking usable for UI elements.
currently it both triggers the events, as well as sends them as throught
commands.trigger_targets. We should probably figure out if this is
needed for them all.
# Objective
Hooks up obserers and picking for a very simple example
## Solution
upstreamed the UI picking backend from bevy_mod_picking
## Testing
tested with the new example picking/simple_picking.rs
---
---------
Co-authored-by: Lixou <82600264+DasLixou@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com>
# Objective
The code to create `ReflectComponent` and `ReflectResource` instances
manually constructs a `Mut<dyn Reflect>` by copying everything but
`value`. That can be done more concisely and better respecting
encapsulation by calling the `map_unchanged()` method.
## Solution
Use `map_unchanged` instead of creating a `Mut` manually.
---------
Co-authored-by: radiish <cb.setho@gmail.com>
# Objective
- Right now `DynamicEnum::is_dynamic()` is returning `false`. I don't
think this was expected, since the rest of `Dynamic*` types return
`true`.
## Solution
- Making `DynamicEnum::is_dynamic()` return true
## Testing
- Added an extra unit test to verify that `.is_dynamic()` returns
`true`.
# Objective
- Bevy now supports an opaque phase for mesh2d, but it's very common for
2d textures to have a transparent alpha channel.
## Solution
- Add an alpha mask phase identical to the one in 3d. It will do the
alpha masking in the shader before drawing the mesh.
- Uses the BinnedRenderPhase
- Since it's an opaque draw it also correctly writes to depth
## Testing
- Tested the mes2d_alpha_mode example and the bevymark example with
alpha mask mode enabled
---
## Showcase
The white logo on the right is rendered with alpha mask enabled.
Running the bevymark example I can get 65fps for 120k mesh2d all using
alpha mask.
## Notes
This is one more step for mesh2d improvements
https://github.com/bevyengine/bevy/issues/13265
---------
Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com>
# Objective
- CI broke after #14284 because of the `cursor_options` rename
## Solution
- Update the broken patch
## Testing
- Patch tested with `git apply`.
# Objective
As is, calling
[`DeferredWorld::query`](https://docs.rs/bevy/latest/bevy/ecs/world/struct.DeferredWorld.html#method.query)
requires you to first `reborrow()` the world in order to use it at all.
Simple reproduction:
```rust
fn test<'w>(mut world: DeferredWorld<'w>, mut state: QueryState<(), ()>) {
let query = world.query(&mut state);
// let query = world.reborrow().query(&mut state); // << Required
}
```
Error message:
```
error[E0597]: `world` does not live long enough
|
444 | fn test<'w>(mut world: DeferredWorld<'w>, mut state: QueryState<(), ()>) {
| -- --------- binding `world` declared here
| |
| lifetime `'w` defined here
445 | let query = world.query(&mut state);
| ^^^^^------------------
| |
| borrowed value does not live long enough
| argument requires that `world` is borrowed for `'w`
446 | }
| - `world` dropped here while still borrowed
```
## Solution
Fix the world borrow lifetime on the `query` method, which now correctly
allows the above usage.
# Objective
This PR is a follow-up to #14703. I forgot to also add the flags from
`package.metadata.docs.rs` to the docs build.
## Solution
As [discussed on
Discord](https://discord.com/channels/691052431525675048/1272629407659589663/1272643734260940940),
I added the missing flags to `docs.yml`.
I also updated `rustc-args` to properly make use of the array (I think
the value has to be either a space-separated string *or* an array of
strings but not an array of space-separated strings 😆)
# Objective
- Wireframe plugins have inconsistencies between 3D and 2D versions.
This PR addresses the following
- 2d version uses `Srgba` for colors, 3d version uses `Color`.
## Solution
- This PR brings consistency by doing the following change
- `Wireframe2d` now uses `Color` instead of `Srgba`
## Testing
- `wireframe_2d` and `wireframe` examples were verified and they work as
before.
---
## Migration Guide
- `Wireframe2dConfig`.`default_color` type is now `Color` instead of
`Srgba`. Use `.into()` to convert between them.
- `Wireframe2dColor`.`color` type is now `Color` instead of `Srgba`. Use
`.into()` to convert between them.
# Objective
By default, Bevy's bloom effect shows square artifacts on small bright
particles due to a low max mip resolution. This PR makes this
configurable via BloomSettings so users can customize these parameters
instead of having them in private module constants.
## Solution
Expose max_mip_dimension and uv_offset in BloomSettings.
## Testing
I tested these changes by running the Bloom 2D / 3D examples.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
The goal with this PR is to allow the use of types that don't implement
`Reflect` within the reflection API.
Rust's [orphan
rule](https://doc.rust-lang.org/book/ch10-02-traits.html#implementing-a-trait-on-a-type)
prevents implementing a trait on an external type when neither type nor
trait are owned by the implementor. This means that if a crate,
`cool_rust_lib`, defines a type, `Foo`, then a user cannot use it with
reflection. What this means is that we have to ignore it most of the
time:
```rust
#[derive(Reflect)]
struct SomeStruct {
#[reflect(ignore)]
data: cool_rust_lib::Foo
}
```
Obviously, it's impossible to implement `Reflect` on `Foo`. But does it
*have* to be?
Most of reflection doesn't deal with concrete types— it's almost all
using `dyn Reflect`. And being very metadata-driven, it should
theoretically be possible. I mean,
[`serde`](https://serde.rs/remote-derive.html) does it.
## Solution
> Special thanks to @danielhenrymantilla for their help reviewing this
PR and offering wisdom wrt safety.
Taking a page out of `serde`'s book, this PR adds the ability to easily
use "remote types" with reflection. In this context, a "remote type" is
the external type for which we have no ability to implement `Reflect`.
This adds the `#[reflect_remote(...)]` attribute macro, which is used to
generate "remote type wrappers". All you have to do is define the
wrapper exactly the same as the remote type's definition:
```rust
// Pretend this is our external crate
mod cool_rust_lib {
#[derive(Default)]
struct Foo {
pub value: String
}
}
#[reflect_remote(cool_rust_lib::Foo)]
struct FooWrapper {
pub value: String
}
```
> **Note:** All fields in the external type *must* be public. This could
be addressed with a separate getter/setter attribute either in this PR
or in another one.
The macro takes this user-defined item and transforms it into a newtype
wrapper around the external type, marking it as `#[repr(transparent)]`.
The fields/variants defined by the user are simply used to build out the
reflection impls.
Additionally, it generates an implementation of the new trait,
`ReflectRemote`, which helps prevent accidental misuses of this API.
Therefore, the output generated by the macro would look something like:
```rust
#[repr(transparent)]
struct FooWrapper(pub cool_rust_lib::Foo);
impl ReflectRemote for FooWrapper {
type Remote = cool_rust_lib::Foo;
// transmutation methods...
}
// reflection impls...
// these will acknowledge and make use of the `value` field
```
Internally, the reflection API will pass around the `FooWrapper` and
[transmute](https://doc.rust-lang.org/std/mem/fn.transmute.html) it
where necessary. All we have to do is then tell `Reflect` to do that. So
rather than ignoring the field, we tell `Reflect` to use our wrapper
using the `#[reflect(remote = ...)]` field attribute:
```rust
#[derive(Reflect)]
struct SomeStruct {
#[reflect(remote = FooWrapper)]
data: cool_rust_lib::Foo
}
```
#### Other Macros & Type Data
Because this macro consumes the defined item and generates a new one, we
can't just put our macros anywhere. All macros that should be passed to
the generated struct need to come *below* this macro. For example, to
derive `Default` and register its associated type data:
```rust
// ✅ GOOD
#[reflect_remote(cool_rust_lib::Foo)]
#[derive(Default)]
#[reflect(Default)]
struct FooWrapper {
pub value: String
}
// ❌ BAD
#[derive(Default)]
#[reflect_remote(cool_rust_lib::Foo)]
#[reflect(Default)]
struct FooWrapper {
pub value: String
}
```
#### Generics
Generics are forwarded to the generated struct as well. They should also
be defined in the same order:
```rust
#[reflect_remote(RemoteGeneric<'a, T1, T2>)]
struct GenericWrapper<'a, T1, T2> {
pub foo: &'a T1,
pub bar: &'a T2,
}
```
> Naming does *not* need to match the original definition's. Only order
matters here.
> Also note that the code above is just a demonstration and doesn't
actually compile since we'd need to enforce certain bounds (e.g. `T1:
Reflect`, `'a: 'static`, etc.)
#### Nesting
And, yes, you can nest remote types:
```rust
#[reflect_remote(RemoteOuter)]
struct OuterWrapper {
#[reflect(remote = InnerWrapper)]
pub inner: RemoteInner
}
#[reflect_remote(RemoteInner)]
struct InnerWrapper(usize);
```
#### Assertions
This macro will also generate some compile-time assertions to ensure
that the correct types are used. It's important we catch this early so
users don't have to wait for something to panic. And it also helps keep
our `unsafe` a little safer.
For example, a wrapper definition that does not match its corresponding
remote type will result in an error:
```rust
mod external_crate {
pub struct TheirStruct(pub u32);
}
#[reflect_remote(external_crate::TheirStruct)]
struct MyStruct(pub String); // ERROR: expected type `u32` but found `String`
```
<details>
<summary>Generated Assertion</summary>
```rust
const _: () = {
#[allow(non_snake_case)]
#[allow(unused_variables)]
#[allow(unused_assignments)]
#[allow(unreachable_patterns)]
#[allow(clippy::multiple_bound_locations)]
fn assert_wrapper_definition_matches_remote_type(
mut __remote__: external_crate::TheirStruct,
) {
__remote__.0 = (|| -> ::core::option::Option<String> { None })().unwrap();
}
};
```
</details>
Additionally, using the incorrect type in a `#[reflect(remote = ...)]`
attribute should result in an error:
```rust
mod external_crate {
pub struct TheirFoo(pub u32);
pub struct TheirBar(pub i32);
}
#[reflect_remote(external_crate::TheirFoo)]
struct MyFoo(pub u32);
#[reflect_remote(external_crate::TheirBar)]
struct MyBar(pub i32);
#[derive(Reflect)]
struct MyStruct {
#[reflect(remote = MyBar)] // ERROR: expected type `TheirFoo` but found struct `TheirBar`
foo: external_crate::TheirFoo
}
```
<details>
<summary>Generated Assertion</summary>
```rust
const _: () = {
struct RemoteFieldAssertions;
impl RemoteFieldAssertions {
#[allow(non_snake_case)]
#[allow(clippy::multiple_bound_locations)]
fn assert__foo__is_valid_remote() {
let _: <MyBar as bevy_reflect::ReflectRemote>::Remote = (|| -> ::core::option::Option<external_crate::TheirFoo> {
None
})().unwrap();
}
}
};
```
</details>
### Discussion
There are a couple points that I think still need discussion or
validation.
- [x] 1. `Any` shenanigans
~~If we wanted to downcast our remote type from a `dyn Reflect`, we'd
have to first downcast to the wrapper then extract the inner type. This
PR has a [commit](b840db9f74cb6d357f951cb11b150d46bac89ee2) that
addresses this by making all the `Reflect::*any` methods return the
inner type rather than the wrapper type. This allows us to downcast
directly to our remote type.~~
~~However, I'm not sure if this is something we want to do. For
unknowing users, it could be confusing and seemingly inconsistent. Is it
worth keeping? Or should this behavior be removed?~~
I think this should be fine. The remote wrapper is an implementation
detail and users should not need to downcast to the wrapper type. Feel
free to let me know if there are other opinions on this though!
- [x] 2. Implementing `Deref/DerefMut` and `From`
~~We don't currently do this, but should we implement other traits on
the generated transparent struct? We could implement `Deref`/`DerefMut`
to easily access the inner type. And we could implement `From` for
easier conversion between the two types (e.g. `T: Into<Foo>`).~~ As
mentioned in the comments, we probably don't need to do this. Again, the
remote wrapper is an implementation detail, and should generally not be
used directly.
- [x] 3. ~~Should we define a getter/setter field attribute in this PR
as well or leave it for a future one?~~ I think this should be saved for
a future PR
- [ ] 4. Any foreseeable issues with this implementation?
#### Alternatives
One alternative to defining our own `ReflectRemote` would be to use
[bytemuck's
`TransparentWrapper`](https://docs.rs/bytemuck/1.13.1/bytemuck/trait.TransparentWrapper.html)
(as suggested by @danielhenrymantilla).
This is definitely a viable option, as `ReflectRemote` is pretty much
the same thing as `TransparentWrapper`. However, the cost would be
bringing in a new crate— though, it is already in use in a few other
sub-crates like bevy_render.
I think we're okay just defining `ReflectRemote` ourselves, but we can
go the bytemuck route if we'd prefer offloading that work to another
crate.
---
## Changelog
* Added the `#[reflect_remote(...)]` attribute macro to allow `Reflect`
to be used on remote types
* Added `ReflectRemote` trait for ensuring proper remote wrapper usage
# Objective
- Fixes#14697
## Solution
This PR modifies the existing `all_tuples!` macro to optionally accept a
`#[doc(fake_variadic)]` attribute in its input. If the attribute is
present, each invocation of the impl macro gets the correct attributes
(i.e. the first impl receives `#[doc(fake_variadic)]` while the other
impls are hidden using `#[doc(hidden)]`.
Impls for the empty tuple (unit type) are left untouched (that's what
the [standard
library](https://doc.rust-lang.org/std/cmp/trait.PartialEq.html#impl-PartialEq-for-())
and
[serde](https://docs.rs/serde/latest/serde/trait.Serialize.html#impl-Serialize-for-())
do).
To work around https://github.com/rust-lang/cargo/issues/8811 and to get
impls on re-exports to correctly show up as variadic, `--cfg docsrs_dep`
is passed when building the docs for the toplevel `bevy` crate.
`#[doc(fake_variadic)]` only works on tuples and fn pointers, so impls
for structs like `AnyOf<(T1, T2, ..., Tn)>` are unchanged.
## Testing
I built the docs locally using `RUSTDOCFLAGS='--cfg docsrs'
RUSTFLAGS='--cfg docsrs_dep' cargo +nightly doc --no-deps --workspace`
and checked the documentation page of a trait both in its original crate
and the re-exported version in `bevy`.
The description should correctly mention for how many tuple items the
trait is implemented.
I added `rustc-args` for docs.rs to the `bevy` crate, I hope there
aren't any other notable crates that re-export `#[doc(fake_variadic)]`
traits.
---
## Showcase
`bevy_ecs::query::QueryData`:
<img width="1015" alt="Screenshot 2024-08-12 at 16 41 28"
src="https://github.com/user-attachments/assets/d40136ed-6731-475f-91a0-9df255cd24e3">
`bevy::ecs::query::QueryData` (re-export):
<img width="1005" alt="Screenshot 2024-08-12 at 16 42 57"
src="https://github.com/user-attachments/assets/71d44cf0-0ab0-48b0-9a51-5ce332594e12">
## Original Description
<details>
Resolves#14697
Submitting as a draft for now, very WIP.
Unfortunately, the docs don't show the variadics nicely when looking at
reexported items.
For example:
`bevy_ecs::bundle::Bundle` correctly shows the variadic impl:
while `bevy::ecs::bundle::Bundle` (the reexport) shows all the impls
(not good):
Built using `RUSTDOCFLAGS='--cfg docsrs' cargo +nightly doc --workspace
--no-deps` (`--no-deps` because of wgpu-core).
Maybe I missed something or this is a limitation in the *totally not
private* `#[doc(fake_variadic)]` thingy. In any case I desperately need
some sleep now :))
</details>
