# Objective
- There is a flaw in the implementation of `FogVolume`'s
`density_texture_offset` from #14868. Because of the way I am wrapping
the UVW coordinates in the volumetric fog shader, a seam is visible when
the 3d texture is wrapping around from one side to the other:
## Solution
- This PR fixes the issue by removing the wrapping from the shader and
instead leaving it to the user to configure the 3d noise texture to use
`ImageAddressMode::Repeat` if they want it to repeat. Using
`ImageAddressMode::Repeat` is the proper solution to avoid the obvious
seam:
- The sampler cannot be implicitly configured to use
`ImageAddressMode::Repeat` because that's not always desirable. For
example, the `fog_volumes` example wouldn't work properly because the
texture from the edges of the volume would overflow to the other sides,
which would be bad in this instance (but it's good in the case of the
`scrolling_fog` example). So leaving it to the user to decide on their
own whether they want the density texture to repeat seems to be the best
solution.
## Testing
- The `scrolling_fog` example still looks the same, it was just changed
to explicitly declare that the density texture should be repeating when
loading the asset. The `fog_volumes` example is unaffected.
<details>
<summary>Minimal reproduction example on current main</summary>
<pre>
use bevy::core_pipeline::experimental::taa::{TemporalAntiAliasBundle,
TemporalAntiAliasPlugin};
use bevy::pbr::{FogVolume, VolumetricFogSettings, VolumetricLight};
use bevy::prelude::*;<br>
fn main() {
App::new()
.add_plugins((DefaultPlugins, TemporalAntiAliasPlugin))
.add_systems(Startup, setup)
.run();
}<br>
fn setup(mut commands: Commands, assets: Res<AssetServer>) {
commands.spawn((
Camera3dBundle {
transform: Transform::from_xyz(3.5, -1.0, 0.4)
.looking_at(Vec3::new(0.0, 0.0, 0.4), Vec3::Y),
msaa: Msaa::Off,
..default()
},
TemporalAntiAliasBundle::default(),
VolumetricFogSettings {
ambient_intensity: 0.0,
jitter: 0.5,
..default()
},
));<br>
commands.spawn((
DirectionalLightBundle {
transform: Transform::from_xyz(-6.0, 5.0, -9.0)
.looking_at(Vec3::new(0.0, 0.0, 0.0), Vec3::Y),
directional_light: DirectionalLight {
illuminance: 32_000.0,
shadows_enabled: true,
..default()
},
..default()
},
VolumetricLight,
));<br>
commands.spawn((
SpatialBundle {
visibility: Visibility::Visible,
transform: Transform::from_xyz(0.0, 0.0,
0.0).with_scale(Vec3::splat(3.0)),
..default()
},
FogVolume {
density_texture: Some(assets.load("volumes/fog_noise.ktx2")),
density_texture_offset: Vec3::new(0.0, 0.0, 0.4),
scattering: 1.0,
..default()
},
));
}
</pre>
</details>
# Objective
- Fixes#14873, see that issue for a whole lot of context
## Solution
- Add a blessed system set for this stuff. See [this Discord
discussion](https://discord.com/channels/691052431525675048/749335865876021248/1276262931327094908).
Note that the gizmo systems,
[LWIM](https://github.com/Leafwing-Studios/leafwing-input-manager/pull/522/files#diff-9b59ee4899ad0a5d008889ea89a124a7291316532e42f9f3d6ae842b906fb095R154)
and now a new plugin I'm working on are all already ordering against
`run_fixed_main_schedule`, so having a dedicated system set should be
more robust and hopefully also more discoverable.
---
## ~~Showcase~~
~~I can add a little video of a smooth camera later if this gets merged
:)~~
Apparently a release note is not needed, so I'll leave it out. See the
changes in the fixed timestep example for usage showcase and the video
in #14873 for a more or less accurate video of the effect (it does not
use the same solution though, so it is not quite the same)
## Migration Guide
[run_fixed_main_schedule](https://docs.rs/bevy/latest/bevy/time/fn.run_fixed_main_schedule.html)
is no longer public. If you used to order against it, use the new
dedicated `RunFixedMainLoopSystem` system set instead. You can replace
your usage of `run_fixed_main_schedule` one for one by
`RunFixedMainLoopSystem::FixedMainLoop`, but it is now more idiomatic to
place your systems in either
`RunFixedMainLoopSystem::BeforeFixedMainLoop` or
`RunFixedMainLoopSystem::AfterFixedMainLoop`
Old:
```rust
app.add_systems(
RunFixedMainLoop,
some_system.before(run_fixed_main_schedule)
);
```
New:
```rust
app.add_systems(
RunFixedMainLoop,
some_system.in_set(RunFixedMainLoopSystem::BeforeFixedMainLoop)
);
```
---------
Co-authored-by: Tau Gärtli <git@tau.garden>
# Objective
When trying to test a gizmos change I ran `cargo test -p bevy_gizmos`
and the output had a lot of noise from warnings and failed doc errors.
This was because I didn't have all of the features enabled.
## Solution
I admit this might be pedantic, and am happy if the concensus is to
reject it. Although it does reduce the lines of code, testing noise, and
the amount of code compiled. I don't think it affects the complexity of
public code, and it doesn't change much to the complexity of internal
code.
I've removed un-needed `bevy_render` imports in all of the gizmos docs
examples, there's probably other unnecessary ones there too, but I
haven't looked exhaustively. It isn't needed for those docs, and isn't
available except in a subset of `cfg` combinations.
I've also made several of the `use` statements slightly more specific. I
shouldn't have changed the public interfaces, except that
`GizmoMeshConfig` requires either `bevy_sprite` or `bevy_pbr`, as it
does nothing without them.
I've also avoided adding some systems and plugins in situations where
they can't work. An example of this is where the `light` module depends
on `all(feature = "bevy_pbr", feature = "bevy_render")`, but it has
`use` statements that only require `bevy_render`.
## Testing
During development I ran:
```
cargo check -p bevy_gizmos && cargo check -p bevy_gizmos --features=bevy_pbr && cargo check -p bevy_gizmos --features=bevy_sprite && cargo check -p bevy_gizmos --features=bevy_render
```
Afterwards I ran this just to be sure:
```
cargo check && cargo check --features=bevy_pbr && cargo check --features=bevy_sprite && cargo check --features=bevy_render
```
Finally I ran:
```
cargo test -p bevy_gizmos && cargo test -p bevy_gizmos --features=bevy_pbr && test check -p bevy_gizmos --features=bevy_sprite && cargo test -p bevy_gizmos --features=bevy_render
```
## Migration Guide
There shouldn't be any reason to migrate, although if for some reason
you use `GizmoMeshConfig` and `bevy_render` but not `bevy_pbr` or
`bevy_sprite` (such that it does nothing), then you will get an error
that it no longer exists.
# Objective
It looks like `Gizmos::grid*` was missed in the colour migration.
## Solution
This updates the `grid` methods and implementation to use `Color`
instead of `LinearRgba`.
It looks like `ExtractedPointLight` and `ExtractedDirectionalLight` also
use `LinearRgba`, although I think in extracted structures it's probably
fine to make more assumptions about what you want?
## Testing
I ran `cargo test --all -- bevy_gizmos` and it passed.
## Migration Guide
This shouldn't be adding anything that isn't already in a migration
guide? I assume as it uses `impl Into<...>` in the public interfaces
that any users of these APIs shouldn't have to make any code changes.
# Objective
- Fixes#14844
## Solution
- implement reflect using the `impl_reflect_value` macro
## Testing
- I wrote a test locally to understand and learn how reflection worked
on a basic level and to confirm that yes indeed the bound struct could
use the reflection traits that have been implemented for it.
note: I did remove a line that asked for bound to not have reflect
implemented in a test, since that's the point of this PR and the test
worked without the line so I am not sure what that was about, not sure
if that uncovers a deeper issue or not.
# Objective
- The goal of this PR is to make it possible to move the density texture
of a `FogVolume` over time in order to create dynamic effects like fog
moving in the wind.
- You could theoretically move the `FogVolume` itself, but this is not
ideal, because the `FogVolume` AABB would eventually leave the area. If
you want an area to remain foggy while also creating the impression that
the fog is moving in the wind, a scrolling density texture is a better
solution.
## Solution
- The PR adds a `density_texture_offset` field to the `FogVolume`
component. This offset is in the UVW coordinates of the density texture,
meaning that a value of `(0.5, 0.0, 0.0)` moves the 3d texture by half
along the x-axis.
- Values above 1.0 are wrapped, a 1.5 offset is the same as a 0.5
offset. This makes it so that the density texture wraps around on the
other side, meaning that a repeating 3d noise texture can seamlessly
scroll forever. It also makes it easy to move the density texture over
time by simply increasing the offset every frame.
## Testing
- A `scrolling_fog` example has been added to demonstrate the feature.
It uses the offset to scroll a repeating 3d noise density texture to
create the impression of fog moving in the wind.
- The camera is looking at a pillar with the sun peaking behind it. This
highlights the effect the changing density has on the volumetric
lighting interactions.
- Temporal anti-aliasing combined with the `jitter` option of
`VolumetricFogSettings` is used to improve the quality of the effect.
---
## Showcase
https://github.com/user-attachments/assets/3aa50ebd-771c-4c99-ab5d-255c0c3be1a8
Closes#14836.
`filter_map_unchanged` optionally maps to an inner value by applying a
function to the contained reference. This is useful in a situation where
you need to convert a `Mut<T>` to a `Mut<U>`, but only if `T` contains
`U`.
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
- Fixes#14796
## Solution
- Copy docs for wrapper methods, make sure they are consistent with the
original docs except for the section on precision.
# Objective
`RenderLayers` was marketed as being unlimited in the Bevy 0.14 release
notes, but the most obvious constructor doesn't actually support
unlimited layers.
We should explicitly document this.
## Solution
Add some docs mentioning the limit and pointing the user to `with` or
`from_layers` if they need an arbitrary number of layers.
# Objective
Fixes#14782
## Solution
Enable the lint and fix all upcoming hints (`--fix`). Also tried to
figure out the false-positive (see review comment). Maybe split this PR
up into multiple parts where only the last one enables the lint, so some
can already be merged resulting in less many files touched / less
potential for merge conflicts?
Currently, there are some cases where it might be easier to read the
code with the qualifier, so perhaps remove the import of it and adapt
its cases? In the current stage it's just a plain adoption of the
suggestions in order to have a base to discuss.
## Testing
`cargo clippy` and `cargo run -p ci` are happy.
# Objective
add a quick shorthand for creating a sprite with an custom size. This is
often desired when working with custom units or scaled sprites and
allows for a cleaner syntax in those cases/
## Solution
Implemented a `sized` function on the Sprite struct which creates a
Sprite, sets the custom size and leaves the rest at their default values
---
## Changelog
- Added `Sprite::sized(custom_size: Vec2)`
# Objective
currently if we use an image with the wrong sampler type in a material,
wgpu panics with an invalid texture format. turn this into a warning and
fail more gracefully.
## Solution
the expected sampler type is specified in the AsBindGroup derive, so we
can just check the image sampler is what it should be.
i am not totally sure about the mapping of image sampler type to
#[sampler(type)], i assumed:
```
"filtering" => [ TextureSampleType::Float { filterable: true } ],
"non_filtering" => [
TextureSampleType::Float { filterable: false },
TextureSampleType::Sint,
TextureSampleType::Uint,
],
"comparison" => [ TextureSampleType::Depth ],
```
# Objective
- Fixes#14684
## Solution
- Added documentation to `all_tuples_with_size` based on existing
`all_tuples` documentation.
- Updated `all_tuples` documentation to match formatting of and link
back to `all_tuples_with_size`
## Testing
- Doctests ran locally.
## Notes
Formatting changes I have proposed make the documentation a little
cleaner in my opinion, but I am open to reverting them and amending
`all_tuples_with_size` to match if there are any reasonable objections.
This reverts commit e37bf18e2b, added in
#14784.
# Objective
The PR was fine, but the work was very poorly motivated and the
resulting API is not actually very nice. The actual user need is likely
better addressed by #14825.
## Solution
Revert the offending PR.
# 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
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
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
#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
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
- 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>
