# Objective
The trait method `with_entity` is used to add an `EntityCommand` to the
command queue. Currently this method returns `WithEntity<C>` which pairs
a command with an `Entity`. By replacing this explicit type with an
opaque type, implementors can override this default implementation by
returning a custom command or closure that does the same thing with a
lower memory footprint.
# Solution
Return an opaque type from the method. As a bonus this file is now
cleaner without the `WithEntity` boilerplate
# Objective
Numerous people have been confused that Bevy runs slowly, when the
reason is that the `llvmpipe` software rendered is being used.
## Solution
Printing a warning could reduce the confusion.
# Objective
- Fixes#13728
## Solution
- add a new feature `smaa_luts`. if enables, it also enables `ktx2` and
`zstd`. if not, it doesn't load the files but use placeholders instead
- adds all the resources needed in the same places that system that uses
them are added.
# Objective
Fixes https://github.com/bevyengine/bevy/issues/13993
PR inspired by https://github.com/bevyengine/bevy/pull/14007 to
accomplish the same thing, but maybe in a clearer fashion.
@Gingeh feel free to take my changes and add them to your PR, I don't
want to steal any credit
---------
Co-authored-by: Gingeh <39150378+Gingeh@users.noreply.github.com>
Co-authored-by: Bob Gardner <rgardner@inworld.ai>
Co-authored-by: Martín Maita <47983254+mnmaita@users.noreply.github.com>
# Objective
In Bevy 0.13, `BackgroundColor` simply tinted the image of any
`UiImage`. This was confusing: in every other case (e.g. Text), this
added a solid square behind the element. #11165 changed this, but
removed `BackgroundColor` from `ImageBundle` to avoid confusion, since
the semantic meaning had changed.
However, this resulted in a serious UX downgrade / inconsistency, as
this behavior was no longer part of the bundle (unlike for `TextBundle`
or `NodeBundle`), leaving users with a relatively frustrating upgrade
path.
Additionally, adding both `BackgroundColor` and `UiImage` resulted in a
bizarre effect, where the background color was seemingly ignored as it
was covered by a solid white placeholder image.
Fixes#13969.
## Solution
Per @viridia's design:
> - if you don't specify a background color, it's transparent.
> - if you don't specify an image color, it's white (because it's a
multiplier).
> - if you don't specify an image, no image is drawn.
> - if you specify both a background color and an image color, they are
independent.
> - the background color is drawn behind the image (in whatever pixels
are transparent)
As laid out by @benfrankel, this involves:
1. Changing the default `UiImage` to use a transparent texture but a
pure white tint.
2. Adding `UiImage::solid_color` to quickly set placeholder images.
3. Changing the default `BorderColor` and `BackgroundColor` to
transparent.
4. Removing the default overrides for these values in the other assorted
UI bundles.
5. Adding `BackgroundColor` back to `ImageBundle` and `ButtonBundle`.
6. Adding a 1x1 `Image::transparent`, which can be accessed from
`Assets<Image>` via the `TRANSPARENT_IMAGE_HANDLE` constant.
Huge thanks to everyone who helped out with the design in the linked
issue and [the Discord
thread](https://discord.com/channels/691052431525675048/1255209923890118697/1255209999278280844):
this was very much a joint design.
@cart helped me figure out how to set the UiImage's default texture to a
transparent 1x1 image, which is a much nicer fix.
## Testing
I've checked the examples modified by this PR, and the `ui` example as
well just to be sure.
## Migration Guide
- `BackgroundColor` no longer tints the color of images in `ImageBundle`
or `ButtonBundle`. Set `UiImage::color` to tint images instead.
- The default texture for `UiImage` is now a transparent white square.
Use `UiImage::solid_color` to quickly draw debug images.
- The default value for `BackgroundColor` and `BorderColor` is now
transparent. Set the color to white manually to return to previous
behavior.
# Objective
- Fixes#13811 (probably, I lost my test code...)
## Solution
- Turns out that Queue and PrepareAssets are _not_ ordered. We should
probably either rethink our system sets (again), or improve the
documentation here. For reference, I've included the current ordering
below.
- The `prepare_meshlet_meshes_X` systems need to run after
`prepare_assets::<PreparedMaterial<M>>`, and have also been moved to
QueueMeshes.
```rust
schedule.configure_sets(
(
ExtractCommands,
ManageViews,
Queue,
PhaseSort,
Prepare,
Render,
Cleanup,
)
.chain(),
);
schedule.configure_sets((ExtractCommands, PrepareAssets, Prepare).chain());
schedule.configure_sets(QueueMeshes.in_set(Queue).after(prepare_assets::<GpuMesh>));
schedule.configure_sets(
(PrepareResources, PrepareResourcesFlush, PrepareBindGroups)
.chain()
.in_set(Prepare),
);
```
## Testing
- Ambiguity checker to make sure I don't have ambiguous system ordering
# Objective
#12469 changed the `Debug` impl for `Entity`, making sure it's actually
accurate for debugging. To ensure that its can still be readily logged
in error messages and inspectors, this PR added a more concise and
human-friendly `Display` impl.
However, users found this form too verbose: the `to_bits` information
was unhelpful and too long. Fixes#13980.
## Solution
- Don't include `Entity::to_bits` in the `Display` implementation for
`Entity`. This information can readily be accessed and logged for users
who need it.
- Also clean up the implementation of `Display` for `DebugName`,
introduced in https://github.com/bevyengine/bevy/pull/13760, to simply
use the new `Display` impl (since this was the desired format there).
## Testing
I've updated an existing test to verify the output of `Entity::display`.
---------
Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com>
# Objective
- Fix issue #13821
## Solution
- Rewrote the test to ensure that it actually tests the functionality
correctly. Then made the par_read function correctly change the values
of self.reader.last_event_count.
## Testing
- Rewrote the test for par_read to run the system schedule twice,
checking the output each time
---------
Co-authored-by: Martín Maita <47983254+mnmaita@users.noreply.github.com>
# Objective
- Fixes https://github.com/bevyengine/bevy/issues/14003
## Solution
- Expose an API to perform updates for a specific sub-app, so we can
avoid mutable borrow the app twice.
## Testing
- I have tested the API by modifying the code in the `many_lights`
example with the following changes:
```rust
impl Plugin for LogVisibleLights {
fn build(&self, app: &mut App) {
let Some(render_app) = app.get_sub_app_mut(RenderApp) else {
return;
};
render_app.add_systems(Render, print_visible_light_count.in_set(RenderSet::Prepare));
}
fn finish(&self, app: &mut App) {
app.update_sub_app_by_label(RenderApp);
}
}
```
---
## Changelog
- add the `update_sub_app_by_label` API to `App` and `SubApps`.
---------
Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
# Objective
- When writing "in game" debugging tools, quite often you need the name
of an entity (for example an entity tree). DebugName is the usual way of
doing that.
- A recent change to Entity's Debug implementation meant it was no
longer a minimal {index}v{generation} but instead a more verbose auto
generated Debug.
- This made DebugName's Debug implementation also verbose
## Solution
- I changed DebugName to derive Debug automatically and added a new
(preferred) Display implementation for it which is the preferred name
for an entity. If the entity has a Name component its the contents of
that, otherwise it is {index}v{generation} (though this does not use
Display of the Entity as that is more verbose than this).
## Testing
- I've added a new test in name.rs which tests the Display
implementation for DebugName by using to_string.
---
## Migration Guide
- In code which uses DebugName you should now use the Display
implementation rather than the Debug implementation (ie {} instead of
{:?} if you were printing it out).
---------
Co-authored-by: John Payne <20407779+johngpayne@users.noreply.github.com>
Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
Co-authored-by: Andres O. Vela <andresovela@users.noreply.github.com>
fixes#13944
I literally just added `Did you forget to call SystemState::apply?` to
the error message. I tested it with the code snipped from the Issue and
yeah it works
# Objective
Fixes#13982
## Solution
~~Adds a new field to `bevy_log::LogPlugin`: `ansi: bool`~~
Documents the use of `std::env::set_var("NO_COLOR", "1");` to disable
colour output in terminals.
## Testing
Yes, all tests passed when I ran `cargo run -p ci -- test` and `cargo
run -p ci -- lints`
I have only tested the code on my Mac, though I doubt this change would
have any affect on other platforms.
---
# Objective
Closes#5943. Seems like Assets v2 solved this one.
## Solution
Added a test to confirm that using `Reflect::clone_value` and then
`FromReflect::from_reflect` on a `Handle<T>` both increment the strong
count.
## Testing
A new test was added to confirm behavior.
---------
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
# Objective
Tight, in-frame generation, re-parenting, despawning, etc., UI
operations could sometime lead taffy to panic (invalid SlotMap key used)
when an entity with an invalid state later despawned.
Fixes#12403
## Solution
Move the `remove_entities` call after children updates.
## Testing
`sickle_ui` had a case that always caused the panic. Tested before this
change, after this change, and before the change again to make sure the
error is there without the fix. The fix worked. Test steps and used
commit described in issue #12403.
I have also ran every bevy UI example, though none of them deal with
entity re-parenting or removal. No regression detected on them.
Tested on Windows only.
* Fixes https://github.com/bevyengine/bevy/issues/13813
* Fixes https://github.com/bevyengine/bevy/issues/13810
Tested a combined scene with both regular meshes and meshlet meshes
with:
* Regular forward setup
* Forward + normal/motion vector prepasses
* Deferred (with depth prepass since that's required)
* Deferred + depth/normal/motion vector prepasses
Still broken:
* Using meshlet meshes rendering in deferred and regular meshes
rendering in forward + depth/normal prepass. I don't know how to fix
this at the moment, so for now I've just add instructions to not mix
them.
# Objective
`with_event` will result in unsafe casting of event data of the given
type to the type expected by the Observer system. This is inherently
unsafe.
## Solution
Flag `Observer::with_event` and `ObserverDescriptor::with_events` as
unsafe. This will not affect normal workflows as `with_event` is
intended for very specific (largely internal) use cases.
This _should_ be backported to 0.14 before release.
---
## Changelog
- `Observer::with_event` is now unsafe.
- Rename `ObserverDescriptor::with_triggers` to
`ObserverDescriptor::with_events` and make it unsafe.
# Objective
- Fixes#13702
- When creating a new window, its scale was changed to match the one
returned by winit, but its size was not which resulted in an incorrect
size until the event with the correct size was received, at least 1
frame later
## Solution
- Apply the window scale to its size when creating it
I updated my 'main' branch, which accidentally closed the original PR
#13747. I'm reopening the this from an actual branch on my repo like I
should have done in the first place. Here's the original info from the
first PR:
* * *
# Problem
The `file_watcher` feature panics if the file_watcher's path "assets" is
not present. I stumbled upon this behavior when I was actually testing
against `embedded_watcher`. I had no "assets" directory and didn't need
one for [my project](https://github.com/shanecelis/bevy_plane_cut).
```text
$ cargo run --example simple; # Runs fine.
$ cargo run --example simple --feature embedded_watcher; # Panics
thread 'main' panicked at /Users/shane/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_asset-0.14.0-rc.2/src/io/source.rs:503:21:
Failed to create file watcher from path "assets", Error { kind: PathNotFound, paths: ["/Users/shane/Projects/bevy_plane_cut/assets"] }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
# Opinion
If a project runs without panicing, then adding the `file_watcher`
feature shouldn't cause it to panic.
# Suggested Solution
This PR suggests if the "assets" path does not exist, emit a warning
stating that the file watcher could not be created and why. All other
errors will be treated as before with a panic and a message.
```text
$ cargo run --example simple --feature embedded_watcher; # Panics
2024-06-08T08:55:11.385249Z WARN bevy_asset::io::source: Skip creating file watcher because path "assets" does not exist.
2024-06-08T08:55:11.385291Z WARN bevy_asset::io::source: AssetSourceId::Default does not have an AssetWatcher configured. Consider enabling the `file_watcher` feature.
```
The second warning is new and I'd prefer it didn't emit under this
condition, but I'll wait to see whether this is actually regarded as a
bug.
# Testing
No tests added. Compiled against my project and it demonstrated the
suggested behavior.
* * *
I changed the second warning to the following when the `file_watcher`
feature is present. When it's not present, it uses the same warning as
before.
```
024-06-09T01:22:16.880619Z WARN bevy_asset::io::source: Skip creating file watcher because path "assets" does not exist.
2024-06-09T01:22:16.880660Z WARN bevy_asset::io::source: AssetSourceId::Default does not have an AssetWatcher configured. Consider adding an "assets" directory.
```
This is an attempt to address issue #13725, which was about the
geometric primitives in the bevy_math crate lacking some detail in the
docs.
# Objective
Fixes#13725
## Solution
Added details to the docstrings. Mostly this consisted of specifying
that the primitives are centered on the origin, or describing how
they're defined (e.g., a circle is the set of all points some distance
from the origin).
## Testing
No testing, since the only changes were to docs.
Fixes#13701
After `winit` upgrade to `0.31`, windows were no longer correctly
resizing. This appears to just have been a simple mistake, where the new
physical size was being sourced from the `winit` window rather than on
the incoming `Window` component.
## Testing
Tested on macOS, but I'm curious whether this was also broken on other
platforms.
# Objective
Fixes#13866
## Solution
Add `insert_before` in **FixedMainScheduleOrder** and
**MainScheduleOrder**, add `insert_startup_before` in
**MainScheduleOrder**, applying the same logic as `insert_after`, except
for parameters naming and insertion index.
This is a followup to https://github.com/bevyengine/bevy/pull/13904
based on the discussion there, and switches two HashMaps that used
meshlet ids as keys to Vec.
In addition to a small further performance boost for `from_mesh` (1.66s
=> 1.60s), this makes processing deterministic modulo threading issues
wrt CRT rand described in the linked PR. This is valuable for debugging,
as you can visually or programmatically inspect the meshlet distribution
before/after making changes that should not change the output, whereas
previously every asset rebuild would change the meshlet structure.
Tested with https://github.com/bevyengine/bevy/pull/13431; after this
change, the visual output of meshlets is consistent between asset
rebuilds, and the MD5 of the output GLB file does not change either,
which was not the case before.
# Objective
- Make primitive meshing behavior consisten across platforms
- Avoid using sizes bigger than `u32` since these aren't even supported
for meshes
## Solution
- Use `u32` instead of `usize` for resolution/subdivisions/segments/etc
fields
---
## Changelog
- Change resolutions in primitive mesh builders from `usize` to `u32`
## Migration Guide
- All primitive mesh builders now take `u32` instead of `usize` for
their resolution/subdivision/segment counts
# Objective
Fixes#13920
## Solution
As described in the issue.
## Testing
Moved a custom transition plugin in example before any of the app-state
methods.
# Objective
Fixes#13933.
## Solution
Changed the return type.
## Testing
Fixed and reused the pre-existing tests for `inspect_entity`.
---
## Migration Guide
- `World::inspect_entity` now returns an `Iterator` instead of a `Vec`.
If you need a `Vec`, immediately collect the iterator:
`world.inspect_entity(entity).collect<Vec<_>>()`
# Objective
Fixes#13845
## Solution
Fix inline docs links inside `init_state` and `insert_state`.
## Testing
- Did you test these changes? If so, how?
Manually checked on `cargo doc` and `rust-analyzer lsp`.
# Objective
Fix a 9-slice asymmetric border issue that
[QueenOfSquiggles](https://blobfox.coffee/@queenofsquiggles/112639035165575222)
found. Here's the behavior before:
<img width="340" alt="the-bug"
src="https://github.com/bevyengine/bevy/assets/54390/81ff1847-b2ea-4578-9fd0-af6ee96c5438">
## Solution
Here's the behavior with the fix.
<img width="327" alt="the-fix"
src="https://github.com/bevyengine/bevy/assets/54390/33a4e3f0-b6a8-448e-9654-1197218ea11d">
## Testing
I used QueenOfSquiggles
[repo](https://github.com/QueenOfSquiggles/my-bevy-learning-project) to
exercise the code. I manually went through a number of variations of the
border and caught a few other issues after the first pass. I added some
code to create random borders and though they often looked funny there
weren't any gaps like before.
### Unit Tests
I did add some tests to `slicer.rs` mostly as an exploratory programming
exercise. So they currently act as a limited, incomplete,
"golden-file"-ish approach. Perhaps they're not worth keeping.
In order to write the tests, I did add a `PartialEq` derive for
`TextureSlice`.
I only tested these changes on macOS.
---
## Changelog
Make 9-slice textures work with asymmetric borders.
# Objective
- Make gizmos behavior consistent across platforms
## Solution
- Use `u32` instead of `usize` for resolution/subdivisions/segments/etc
fields
---
## Changelog
- Change resolutions in gizmos from `usize` to `u32`
## Migration Guide
- All gizmos now take `u32` instead of `usize` for their
resolution/subdivision/segment counts
# Objective
- Primitives should not use poorly defined types like `usize`,
especially since they are serializable
## Solution
- Use `u32` instead of `usize`
- The generic array types do not need to be changed because this size is
not actually stored or serialized anywhere
---
## Migration Guide
- `RegularPolygon` now uses `u32` instead of `usize` for the number of
sides
# Objective
This PR aims to improve error handling for log filters.
Closes#13850
## Solution
I changed the parsing of LogPlugin its filter to lossy, so that it
prints the directives with an error but does not skip them. I decided on
letting it gracefully handle the error instead of panicking to be
consistent with the parsing from an environment variable that it tries
to do before parsing it from the LogPlugin filter.
If the user decides to specify the filter by an environment variable, it
would silently fail and default to the LogPlugin filter value. It now
prints an error before defaulting to the LogPlugin filter value.
Unfortunately, I could not try and loosely set the filter from the
environment variable because the `tracing-subscriber` module does not
expose the function it uses to get the environment variable, and I would
rather not copy its code. We may want to check if the maintainers are
open to exposing the method.
## Testing
Consider the following bevy app, where the second of the 3 filters is
invalid:
```
use bevy::{log::LogPlugin, prelude::*};
fn main() {
App::new().add_plugins(DefaultPlugins
.set(LogPlugin {
filter: "wgpu=error,my_package=invalid_log_level,naga=warn".into(),
..default()
})
).run();
}
```
In the previous situation, it would panic with a non-descriptive error:
"called `Result::unwrap()` on an `Err` value: ParseError { kind:
Other(None) }", while only 1 of the 3 filters is invalid. When running
`cargo run`, it will now use the two valid filters and print an error on
the invalid filter.
> ignoring `my_package=invalid_log_level`: invalid filter directive
This error comes from `tracing-subscriber` and cannot be altered as far
as I can see.
To test setting the log filter through an environment variable, you can
use `RUST_LOG="wgpu=error,my_package=invalid_log_level,naga=warn" cargo
run` to run your app. In the previous situation it would silently fail
and use the LogPlugin filter. It will now print an error before using
the LogPlugin filter.
> LogPlugin failed to parse filter from env: invalid filter directive
## Changelog
- Added warning when using invalid filter in the RUST_LOG environment
variable
- Prevent the app from panicking when setting an invalid LogPlugin
filter
---------
Co-authored-by: Luc Drenth <luc.drenth@ing.com>
# Objective
Fixes#13917
## Solution
Changed `IntoSystemSetConfigs::chain_ignore_deferred`'s return type from
`SystemConfigs` to `SystemSetConfigs`
## Testing
Tried to run the `ecs_guide` example, where `chain` method is replaced
by `chain_ignore_deferred` method
---
# Objective
- Fix a typo in documentation for `Query::single_mut`
## Solution
- Change `item` to `items`
## Testing
- I built the documentation and it looked fine.
- Since this only affects a doc comment, no further testing should be
necessary.
---
## Changelog
> This section is optional. If this was a trivial fix, or has no
externally-visible impact, you can delete this section.
- Fixed a typo in the documentation for Query.
# Objective
- Fixes#11933.
- Related: #12280.
## Solution
- Specify that, after applying `AmbientLight`, the resulting units are
in cd/m^2.
- This is based on [@fintelia's
comment](https://github.com/bevyengine/bevy/issues/11933#issuecomment-1995427587),
and will need to be verified.
---
## Changelog
- Specified units for `AmbientLight`'s `brightness` field.
This change reworks `find_connected_meshlets` to scale more linearly
with the mesh size, which significantly reduces the cost of building
meshlet representations. As a small extra complexity reduction, it moves
`simplify_scale` call out of the loop so that it's called once (it only
depends on the vertex data => is safe to cache).
The new implementation of connectivity analysis builds edge=>meshlet
list data structure, which allows us to only iterate through
`tuple_combinations` of a (usually) small list. There is still some
redundancy as if two meshlets share two edges, they will be represented
in the meshlet lists twice, but it's overall much faster.
Since the hash traversal is non-deterministic, to keep this part of the
algorithm deterministic for reproducible results we sort the output
adjacency lists.
Overall this reduces the time to process bunny mesh from ~4.2s to ~1.7s
when using release; in unoptimized builds the delta is even more
significant.
This was tested by using https://github.com/bevyengine/bevy/pull/13431
and:
a) comparing the result of `find_connected_meshlets` using old and new
code; they are equal in all steps of the clustering process
b) comparing the rendered result of the old code vs new code *after*
making the rest of the algorithm deterministic: right now the loop that
iterates through the result of `group_meshlets()` call executes in
different order between program runs. This is orthogonal to this change
and can be fixed separately.
Note: a future change can shrink the processing time further from ~1.7s
to ~0.4s with a small diff but that requires an update to meshopt crate
which is pending in https://github.com/gwihlidal/meshopt-rs/pull/42.
This change is independent.
# Objective
- first part of #13900
## Solution
- split `check_light_mesh_visibility `into
`check_dir_light_mesh_visibility `and
`check_point_light_mesh_visibility` for better review
# Objective
- After #12582 , Bevy split visibleEntities into a TypeIdMap for
different types of entities, but the behavior in
`check_light_mesh_visibility `simply calls HashMap::clear(), which will
reallocate memory every frame.
## Testing
cargo run --release --example many_cubes --features bevy/trace_tracy --
--shadows
~10% win in `check_light_mesh_visibilty`
![image](https://github.com/bevyengine/bevy/assets/45868716/1bf4deef-bab2-4e5f-9f60-bea8b7e33e3e)
# Objective
The error messages that appear when a value cannot be serialized or
deserialized via reflection could be slightly improved.
When one of these operations fails, some users are confused about how to
resolve the issue. I've spoken with a few who didn't know they could
register `ReflectSerialize` themselves. We should try to clarify this to
some degree in the error messages.
## Solution
Add some more detail to the error messages.
For example, replacing this:
```
Type 'core::ops::RangeInclusive<f32>' did not register ReflectSerialize
```
with this:
```
Type `core::ops::RangeInclusive<f32>` did not register the `ReflectSerialize` type data. For certain types, this may need to be registered manually using `register_type_data`
```
I also added a separate error message if the type was not registered in
the type registry at all:
```
Type `core::ops::RangeInclusive<f32>` is not registered in the type registry
```
## Testing
You can test locally by running:
```
cargo test --package bevy_reflect
```
---
## Changelog
- Added error message for missing type registration when serializing
reflect data
- Changed error message for missing `ReflectSerialize` registration when
serializing reflect data
- Changed error message for missing `ReflectDeserialize` registration
when deserializing reflect data
# Objective
I got little confused by the document of `all_tuples!` because type
names of the parameter `T` and extracted names `Pn` are difference.
## Solution
I fixed type names of the document.
# Objective
While writing code for the `bevy_ecs` I noticed we were using a
unnecessarily stable sort to sort component ids
## Solution
- Sort component ids with a unstable sort
- Comb the bevy_ecs crate for any other obvious inefficiencies.
- Don't clone component vectors when inserting an archetype.
## Testing
I ran `cargo test -p bevy_ecs`. Everything else I leave to CI.
## Profiling
I measured about a 1% speed increase when spawning entities directly
into a world. Since the difference is so small (and might just be noise)
I didn't bother to figure out which of change if any made the biggest
difference.
<details>
<summary> Tracy data </summary>
Yellow is this PR. Red is the commit I branched from.
![image](https://github.com/bevyengine/bevy/assets/59848927/f1a5c95d-a882-4dfb-ac07-dd2922273b91)
</details>
<details>
<summary>Methodology</summary>
I created a system that spawn a 1000 entities each with the same 30
components each frame, and then I measured it's run time. The unusually
high number of components was chosen because the standard library [will
use a insertion sort for slices under 20
elements](0de24a5177/library/core/src/slice/sort.rs (L1048-L1049)).
This holds for both stable and unstable sorts.
</details>
# Objective
Fixes#13299
On Linux/X11, changing focus into a winit window will produce winit
KeyboardInput events with a "is_synthetic=true" flag that are not
intended to be used. Bevy erroneously passes them on to the user,
resulting in phantom key presses.
## Solution
This patch properly filters out winit KeyboardInput events with
"is_synthetic=true".
For example, pressing Alt+Tab to focus a bevy winit window results in a
permanently stuck Tab key until the user presses Tab once again to
produce a winit KeyboardInput release event. The Tab key press event
that causes this problem is "synthetic", should not be used according to
the winit devs, and simply ignoring it fixes this problem.
Synthetic key **releases** are still evaluated though, as they are
essential for correct release key handling. For example, if the user
binds the key combination Alt+1 to the action "move the window to
workspace 1", places the bevy game in workspace 2, focuses the game and
presses Alt+1, then the key release event for the "1" key will be
synthetic. If we would filter out all synthetic keys, the bevy game
would think that the 1 key remains pressed forever, until the user
manually presses+releases the key again inside bevy.
Reference:
https://docs.rs/winit/0.30.0/winit/event/enum.WindowEvent.html#variant.KeyboardInput.field.is_synthetic
Relevant discussion: https://github.com/rust-windowing/winit/issues/3543
## Testing
Tested with the "keyboard_input_events" example. Entering/exiting the
window with various keys, as well as changing its workspace, produces
the correct press/release events.
# Objective
- Fixes#13874
## Solution
- Confirm that the `StatesPlugin` is installed when trying to add
states.
- Skipped for state scoped entities, since those will warn about missing
states.
# Objective
Fixes#13854
## Solution
Removed the inaccurate warning. This was done for a few reasons:
- States not existing is now a valid "state" (for lack of a better term)
- Other run conditions don't provide an equivalent warning
# Objective
Allow the use of color definitions from Bevy in other contexts than pure
Bevy apps, e.g. outside ECS use.
In those cases it's nice to not have more dependencies than you need.
## Solution
Hide use of reflection behind a feature flag.
Defaults to on.
## Points to consider
1. This was straightforward _except_ for the
`crates/bevy_color/src/lib.rs` change where I removed `Reflect` as a
bound. That is awkward to have feature gated since features should be
additive. If the bound was added as part of the feature flag, the result
would be _more_ restrictive, and _disable_ impls which did not have the
impl. On the other hand having the reflect bound there unconditionally
would defeat the purpose of the PR. I opted to remove the bound since it
seems overly restrictive anyway.
2. It's possible to hide `encase` and `bytemuck` behind the new feature
flag too (or a separate one). I'm thinking if `bevy-support` is not
desired then it's unlikely that the user has need of those.
---------
Signed-off-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
Co-authored-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
# Objective
- #13846 introduced a bug where text not bound was not displayed
## Solution
- bounds are infinite
- use computed size instead, that already should be using the available
bounds
# Objective
This is the first of a series of PRs intended to begin the upstreaming
process for `bevy_mod_picking`. The purpose of this PR is to:
+ Create the new `bevy_picking` crate
+ Upstream `CorePlugin` as `PickingPlugin`
+ Upstream the core pointer and backend abstractions.
This code has been ported verbatim from the corresponding files in
[bevy_picking_core](https://github.com/aevyrie/bevy_mod_picking/tree/main/crates/bevy_picking_core/src)
with a few tiny naming and docs tweaks.
The work here is only an initial foothold to get the up-streaming
process started in earnest. We can do refactoring and improvements once
this is in-tree.
---------
Co-authored-by: Aevyrie <aevyrie@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- Provide an expressive way to register dynamic behavior in response to
ECS changes that is consistent with existing bevy types and traits as to
provide a smooth user experience.
- Provide a mechanism for immediate changes in response to events during
command application in order to facilitate improved query caching on the
path to relations.
## Solution
- A new fundamental ECS construct, the `Observer`; inspired by flec's
observers but adapted to better fit bevy's access patterns and rust's
type system.
---
## Examples
There are 3 main ways to register observers. The first is a "component
observer" that looks like this:
```rust
world.observe(|trigger: Trigger<OnAdd, Transform>, query: Query<&Transform>| {
let transform = query.get(trigger.entity()).unwrap();
});
```
The above code will spawn a new entity representing the observer that
will run it's callback whenever the `Transform` component is added to an
entity. This is a system-like function that supports dependency
injection for all the standard bevy types: `Query`, `Res`, `Commands`
etc. It also has a `Trigger` parameter that provides information about
the trigger such as the target entity, and the event being triggered.
Importantly these systems run during command application which is key
for their future use to keep ECS internals up to date. There are similar
events for `OnInsert` and `OnRemove`, and this will be expanded with
things such as `ArchetypeCreated`, `TableEmpty` etc. in follow up PRs.
Another way to register an observer is an "entity observer" that looks
like this:
```rust
world.entity_mut(entity).observe(|trigger: Trigger<Resize>| {
// ...
});
```
Entity observers run whenever an event of their type is triggered
targeting that specific entity. This type of observer will de-spawn
itself if the entity (or entities) it is observing is ever de-spawned so
as to not leave dangling observers.
Entity observers can also be spawned from deferred contexts such as
other observers, systems, or hooks using commands:
```rust
commands.entity(entity).observe(|trigger: Trigger<Resize>| {
// ...
});
```
Observers are not limited to in built event types, they can be used with
any type that implements `Event` (which has been extended to implement
Component). This means events can also carry data:
```rust
#[derive(Event)]
struct Resize { x: u32, y: u32 }
commands.entity(entity).observe(|trigger: Trigger<Resize>, query: Query<&mut Size>| {
let event = trigger.event();
// ...
});
// Will trigger the observer when commands are applied.
commands.trigger_targets(Resize { x: 10, y: 10 }, entity);
```
You can also trigger events that target more than one entity at a time:
```rust
commands.trigger_targets(Resize { x: 10, y: 10 }, [e1, e2]);
```
Additionally, Observers don't _need_ entity targets:
```rust
app.observe(|trigger: Trigger<Quit>| {
})
commands.trigger(Quit);
```
In these cases, `trigger.entity()` will be a placeholder.
Observers are actually just normal entities with an `ObserverState` and
`Observer` component! The `observe()` functions above are just shorthand
for:
```rust
world.spawn(Observer::new(|trigger: Trigger<Resize>| {});
```
This will spawn the `Observer` system and use an `on_add` hook to add
the `ObserverState` component.
Dynamic components and trigger types are also fully supported allowing
for runtime defined trigger types.
## Possible Follow-ups
1. Deprecate `RemovedComponents`, observers should fulfill all use cases
while being more flexible and performant.
2. Queries as entities: Swap queries to entities and begin using
observers listening to archetype creation triggers to keep their caches
in sync, this allows unification of `ObserverState` and `QueryState` as
well as unlocking several API improvements for `Query` and the
management of `QueryState`.
3. Trigger bubbling: For some UI use cases in particular users are
likely to want some form of bubbling for entity observers, this is
trivial to implement naively but ideally this includes an acceleration
structure to cache hierarchy traversals.
4. All kinds of other in-built trigger types.
5. Optimization; in order to not bloat the complexity of the PR I have
kept the implementation straightforward, there are several areas where
performance can be improved. The focus for this PR is to get the
behavior implemented and not incur a performance cost for users who
don't use observers.
I am leaving each of these to follow up PR's in order to keep each of
them reviewable as this already includes significant changes.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: MiniaczQ <xnetroidpl@gmail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
- Fixes#13844
- Warn user when initializing state multiple times
## Solution
- `insert_state` will overwrite previously initialized state value,
reset transition events and re-insert it's own transition event.
- `init_state`, `add_sub_state`, `add_computed_state` are idempotent, so
calling them multiple times will emit a warning.
## Testing
- 2 tests confirming overwrite works.
- Given the example from #13844
```rs
use bevy::prelude::*;
fn main() {
App::new()
.add_plugins(DefaultPlugins)
.insert_state(AppState::A)
.insert_state(AppState::B)
.add_systems(OnEnter(AppState::A), setup_a)
.add_systems(OnEnter(AppState::B), setup_b)
.add_systems(OnExit(AppState::A), cleanup_a)
.add_systems(OnExit(AppState::B), cleanup_b)
.run();
}
#[derive(States, Debug, Clone, PartialEq, Eq, Hash)]
enum AppState {
A,
B,
}
fn setup_a() {
info!("setting up A");
}
fn setup_b() {
info!("setting up B");
}
fn cleanup_a() {
info!("cleaning up A");
}
fn cleanup_b() {
info!("cleaning up B");
}
```
We get the following result:
```
INFO states: setting up B
```
which matches our expectations.
# Objective
Fixes#13815
## Solution
Move insertion of the plugin name to after build is called.
## Testing
I added a regression test
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: François Mockers <mockersf@gmail.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
# Objective
when a parent container is auto-sized, text alignments `Center` and
`Right` don't align to the center and right properly. fix it
## Solution
ab_glyph positions return +/- values from an anchor point. we currently
transform them to positive values from the min-x of the glyphs, and then
offset from the left of the bounds. instead, we can keep the negative
values as ab_glyph intended and offset from the left/middle/right of the
bounds as appropriate.
## Testing
texts with align left, center, right, all contained in the purple boxes:
before (0.14.0-rc.2):
![Screenshot 2024-06-14
165456](https://github.com/bevyengine/bevy/assets/50659922/90fb73b0-d8bd-4ae8-abf3-7106eafc93ba)
after:
![Screenshot 2024-06-14
164449](https://github.com/bevyengine/bevy/assets/50659922/0a75ff09-b51d-4fbe-a491-b655a145c08b)
code:
```rs
use bevy::prelude::*;
fn main() {
App::new()
.add_plugins(DefaultPlugins)
.add_systems(Startup, setup)
.run();
}
fn setup(mut commands: Commands) {
commands.spawn(Camera2dBundle::default());
for (left, justify) in [
(100.0, JustifyText::Left),
(500.0, JustifyText::Center),
(900.0, JustifyText::Right),
] {
commands
// container
.spawn(NodeBundle {
style: Style {
flex_direction: FlexDirection::Column,
position_type: PositionType::Absolute,
left: Val::Px(left),
top: Val::Px(100.0),
width: Val::Px(300.0),
..Default::default()
},
..Default::default()
})
.with_children(|commands| {
commands.spawn(NodeBundle{
style: Style {
flex_direction: FlexDirection::Row,
height: Val::Px(75.0),
..Default::default()
},
background_color: Color::srgb(1.0, 0.0, 1.0).into(),
..Default::default()
}).with_children(|commands| {
// a div that reduces the available size
commands.spawn(NodeBundle {
style: Style {
width: Val::Px(75.0),
..Default::default()
},
background_color: Color::srgb(0.0, 1.0, 0.0).into(),
..Default::default()
});
// text with width=auto, but actual size will not be what it expcets due to the sibling div above
commands.spawn(TextBundle {
text: Text::from_section("Some text that wraps onto a second line", Default::default()).with_justify(justify),
style: Style {
align_self: AlignSelf::Center,
..Default::default()
},
..Default::default()
});
});
});
}
}
```
Currently blocked on https://github.com/gfx-rs/wgpu/issues/5774
# Objective
Update to wgpu 0.20
## Solution
Update to wgpu 0.20 and naga_oil 0.14.
## Testing
Tested a few different examples on linux (vulkan, webgl2, webgpu) and
windows (dx12 + vulkan) and they worked.
---
## Changelog
- Updated to wgpu 0.20. Note that we don't currently support wgpu's new
pipeline overridable constants, as they don't work on web currently and
need some more changes to naga_oil (and are somewhat redundant with
naga_oil's shader defs). See wgpu's changelog for more
https://github.com/gfx-rs/wgpu/blob/trunk/CHANGELOG.md#v0200-2024-04-28
## Migration Guide
TODO
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: François Mockers <mockersf@gmail.com>
# Objective
- Fixes#13807
## Solution
- Before this pr we antialiased between 0.5 and -0.5. This pr changes
things to antialias between 0.25 and -0.25. I tried slightly larger
ranges, but the edge between the boxes still showed. I'm not 100% sure
this is the correct solution, but from what I could find the range you
use is more art than science.
## Testing
- Ran rounded_borders example, the code in the linked issue, and the
testing example from #12702.
---
## Changelog
- reduce antialiasing in ui shader.
StatesPlugin and GizmoPlugin were missing from the doc comment of
DefaultPlugins. I am not sure whether this was for a reason, but i just
stumbled over it and it seemed off...
## Testing
I'm not sure how to test these changes?
# Objective
As discovered in
https://github.com/Leafwing-Studios/leafwing-input-manager/issues/538,
there appears to be some real weirdness going on in how event updates
are processed between Bevy 0.13 and Bevy 0.14.
To identify the cause and prevent regression, I've added tests to
validate the intended behavior.
My initial suspicion was that this would be fixed by
https://github.com/bevyengine/bevy/pull/13762, but that doesn't seem to
be the case.
Instead, events appear to never be updated at all when using `bevy_app`
by itself. This is part of the problem resolved by
https://github.com/bevyengine/bevy/pull/11528, and introduced by
https://github.com/bevyengine/bevy/pull/10077.
After some investigation, it appears that `signal_event_update_system`
is never added using a bare-bones `App`, and so event updates are always
skipped.
This can be worked around by adding your own copy to a
later-in-the-frame schedule, but that's not a very good fix.
## Solution
Ensure that if we're not using a `FixedUpdate` schedule, events are
always updated every frame.
To do this, I've modified the logic of `event_update_condition` and
`event_update_system` to clearly and correctly differentiate between the
two cases: where we're waiting for a "you should update now" signal and
where we simply don't care.
To encode this, I've added the `ShouldUpdateEvents` enum, replacing a
simple `bool` in `EventRegistry`'s `needs_update` field.
Now, both tests pass as expected, without having to manually add a
system!
## Testing
I've written two parallel unit tests to cover the intended behavior:
1. Test that `iter_current_update_events` works as expected in
`bevy_ecs`.
2. Test that `iter_current_update_events` works as expected in
`bevy_app`
I've also added a test to verify that event updating works correctly in
the presence of a fixed main schedule, and a second test to verify that
fixed updating works at all to help future authors narrow down failures.
## Outstanding
- [x] figure out why the `bevy_app` version of this test fails but the
`bevy_ecs` version does not
- [x] figure out why `EventRegistry::run_updates` isn't working properly
- [x] figure out why `EventRegistry::run_updates` is never getting
called
- [x] figure out why `event_update_condition` is always returning false
- [x] figure out why `EventRegistry::needs_update` is always false
- [x] verify that the problem is a missing `signal_events_update_system`
---------
Co-authored-by: Mike <mike.hsu@gmail.com>
…izer (#13442)"
This reverts commit 5cfb063d4a.
- This PR broke bevy-trait-query, which needs to be able to write a
resource in init_state. See #13798 for more details.
- Note this doesn't fix everything as transmutes for bevy-trait-query
will still be broken,. But the current usage in that crate is UB, so we
need to find another solution.
# Objective
- Split the bevy_ecs::events module so it's easier to work with
## Solution
- Split the event.rs file across multiple files, made sure all tests
passed, and exports from the module were the same as previous
## Testing
- All automated tests pass.
# Objective
Closes#13738
## Solution
Added `from_color` to materials that would support it. Didn't add
`from_color` to `WireframeMaterial` as it doesn't seem we expect users
to be constructing them themselves.
## Testing
None
---
## Changelog
### Added
- `from_color` to `StandardMaterial` so you can construct this material
from any color type.
- `from_color` to `ColorMaterial` so you can construct this material
from any color type.
# Objective
- Mikktspace requires that we normalize world normals/tangents _before_
interpolation across vertices, and then do _not_ normalize after. I had
it backwards.
- We do not (am not supposed to?) need a second set of barycentrics for
motion vectors. If you think about the typical raster pipeline, in the
vertex shader we calculate previous_world_position, and then it gets
interpolated using the current triangle's barycentrics.
## Solution
- Fix normal/tangent processing
- Reuse barycentrics for motion vector calculations
- Not implementing this for 0.14, but long term I aim to remove explicit
vertex tangents and calculate them in the shader on the fly.
## Testing
- I tested out some of the normal maps we have in repo. Didn't seem to
make a difference, but mikktspace is all about correctness across
various baking tools. I probably just didn't have any of the ones that
would cause it to break.
- Didn't test motion vectors as there's a known bug with the depth
buffer and meshlets that I'm waiting on the render graph rewrite to fix.
# Objective
Reading system information severely slows down the update loop.
Fixes#12848.
## Solution
Read system info in a separate thread.
## Testing
- Open the scene 3d example
- Add `FrameTimeDiagnosticsPlugin`, `SystemInformationDiagnosticsPlugin`
and `LogDiagnosticsPlugin` to the app.
- Add this system to the update schedule to disable Vsync on the main
window
```rust
fn change_window_mode(mut windows: Query<&mut Window, Added<Window>>) {
for mut window in &mut windows {
window.present_mode = PresentMode::AutoNoVsync;
}
}
```
- Read the fps values in the console before and after this PR.
On my PC I went from around 50 fps to around 1150 fps.
---
## Changelog
### Changed
- The `SystemInformationDiagnosticsPlugin` now reads system data
separate of the update cycle.
### Added
- The `EXPECTED_SYSTEM_INFORMATION_INTERVAL` constant which defines how
often we read system diagnostic data.
---------
Co-authored-by: IceSentry <IceSentry@users.noreply.github.com>
Fixes#13758.
# Objective
Calling `update` on the main app already calls `clear_trackers`. Calling
it again in `SubApps::update` caused RemovedCompenet Events to be
cleared earlier than they should be.
## Solution
- Don't call clear_trackers an extra time.
## Testing
I manually tested the fix with this unit test:
```
#[cfg(test)]
mod test {
use crate::core::{FrameCount, FrameCountPlugin};
use crate::prelude::*;
#[test]
fn test_next_frame_removal() {
#[derive(Component)]
struct Foo;
#[derive(Resource)]
struct RemovedCount(usize);
let mut app = App::new();
app.add_plugins(FrameCountPlugin);
app.add_systems(Startup, |mut commands: Commands| {
for _ in 0..100 {
commands.spawn(Foo);
}
commands.insert_resource(RemovedCount(0));
});
app.add_systems(First, |counter: Res<FrameCount>| {
println!("Frame {}:", counter.0)
});
fn detector_system(
mut removals: RemovedComponents<Foo>,
foos: Query<Entity, With<Foo>>,
mut removed_c: ResMut<RemovedCount>,
) {
for e in removals.read() {
println!(" Detected removed Foo component for {e:?}");
removed_c.0 += 1;
}
let c = foos.iter().count();
println!(" Total Foos: {}", c);
assert_eq!(c + removed_c.0, 100);
}
fn deleter_system(foos: Query<Entity, With<Foo>>, mut commands: Commands) {
foos.iter().next().map(|e| {
commands.entity(e).remove::<Foo>();
});
}
app.add_systems(Update, (detector_system, deleter_system).chain());
app.update();
app.update();
app.update();
app.update();
}
}
```
i based the design on @mgi388 in the discussion about the issue.
i added the illustration in such a way that it shows up when you hover
your mouse over the type, i hope this is what was meant by the issue
no unit tests were added bc obviously
Fixes#13664
# Objective
There were some issues with the `serialize` feature:
- `bevy_app` had a `serialize` feature and a dependency on `serde` even
there is no usage of serde at all inside `bevy_app`
- the `bevy_app/serialize` feature enabled `bevy_ecs/serde`, which is
strange
- `bevy_internal/serialize` did not enable `bevy_app/serialize` so there
was no way of serializing an Entity in bevy 0.14
## Solution
- Remove `serde` and `bevy_app/serialize`
- Add a `serialize` flag on `bevy_ecs` that enables `serde`
- ` bevy_internal/serialize` now enables `bevy_ecs/serialize`
# Objective
- My attempt at fulfilling #13629.
## Solution
Renames the `and_then` / `or_else` run condition methods to `and` /
`or`, respectively.
Extends the run conditions API to include a suite of binary logical
operators:
- `and`
- `or`
- `nand`
- `nor`
- `xor`
- `xnor`
## Testing
- Did you test these changes? If so, how?
- The test **run_condition_combinators** was extended to include the
added run condition combinators. A **double_counter** system was added
to test for combinators running on even count cycles.
- Are there any parts that need more testing?
- I'm not too sure how I feel about the "counter" style of testing but I
wanted to keep it consistent. If it's just a unit test I would prefer
simply to just assert `true` == _combinator output_ or `false` ==
_combinator output_ .
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- Nothing too specific. The added methods should be equivalent to the
logical operators they are analogous to (`&&` , `||`, `^`, `!`).
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
- Should not be relevant, I'm using Windows.
## Changelog
- What changed as a result of this PR?
- The run conditions API.
- If applicable, organize changes under "Added", "Changed", or "Fixed"
sub-headings
- Changed:
- `and_then` run condition combinator renamed to simply `and`
- `or_else` run condition combinator renamed to simply `or`
- Added:
- `nand` run condition combinator.
- `nor` run condition combinator.
- `xor` run condition combinator.
- `xnor` run condition combinator.
## Migration Guide
- The `and_then` run condition method has been replaced with the `and`
run condition method.
- The `or_else` run condition method has been replaced with the `or` run
condition method.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Andres O. Vela <andresovela@users.noreply.github.com>
# Objective
- If the fog is disabled it still generates a useless branch which can
hurt performance
## Solution
- Make the flag a shader_def instead
## Testing
- I tested enabling/disabling fog works as expected per-material in the
fog example
- I also tested that scenes that don't add the FogSettings resource
still work correctly
## Review notes
I'm not sure how to handle the removed material flag. Right now I just
commented it out and added a not to reuse it instead of creating a new
one.
The documentation for the `State` resource still referenced the use of
`apply_state_transition` to manually force a state transition to occur,
and the question around how to force transitions had come up a few times
on discord.
This is a docs-only change, that does the following:
- Properly references `StateTransition` in the `MainSchedule` docs
- replace the explanations for applying `NextState` with ones that
explain the `StateTransition` schedule, and mentions the possibility of
calling it manually
- Add an example of calling `StateTransition` manually in the docs for
the state transition schedule itself.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
The error printed-out due to a missing shader file was confusing; This
PR changes the error message.
Fixes#13644
## Solution
I replaced the confusing wording (`... shader is not loaded yet`) with a
clear explanation (`... shader could not be loaded`)
## Testing
> Did you test these changes? If so, how?
removing `assets/shaders/game_of_life.wgsl` & running its associated
example now produces the following error:
```
thread '<unnamed>' panicked at examples/shader/compute_shader_game_of_life.rs:233:25:
Initializing assets/shaders/game_of_life.wgsl:
Pipeline could not be compiled because the following shader could not be loaded: AssetId<bevy_render::render_resource::shader::Shader>{ index: 0, generation: 0}
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Encountered a panic in system `bevy_render::renderer::render_system`!
```
I don't think there are any tests expecting the previous error message,
so this change should not break anything.
> Are there any parts that need more testing?
If there was an intent behind the original message, this might need more
attention.
> How can other people (reviewers) test your changes? Is there anything
specific they need to know?
One should be able to preview the changes by running any example after
deleting/renaming their associated shader(s).
> If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
N/A
# Objective
Fixes#13711
## Solution
Introduce smaller, generic system sets for each schedule variant, which
are ordered against other generic variants:
- `ExitSchedules<S>` - For `OnExit` schedules, runs from leaf states to
root states.
- `TransitionSchedules<S>` - For `OnTransition` schedules, runs in
arbitrary order.
- `EnterSchedules<S>` - For `OnEnter` schedules, runs from root states
to leaf states.
Also unified `ApplyStateTransition<S>` schedule which works in basically
the same way, just for internals.
## Testing
- One test that tests schedule execution order
---------
Co-authored-by: Lee-Orr <lee-orr@users.noreply.github.com>
# Objective
One thing missing from the new Color implementation in 0.14 is the
ability to easily convert to a u8 representation of the rgb color.
(note this is a redo of PR https://github.com/bevyengine/bevy/pull/13739
as I needed to move the source branch
## Solution
I have added to_u8_array and to_u8_array_no_alpha to a new trait called
ColorToPacked to mirror the f32 conversions in ColorToComponents and
implemented the new trait for Srgba and LinearRgba.
To go with those I also added matching from_u8... functions and
converted a couple of cases that used ad-hoc implementations of that
conversion to use these.
After discussion on Discord of the experience of using the API I renamed
Color::linear to Color::to_linear, as without that it looks like a
constructor (like Color::rgb).
I also added to_srgba which is the other commonly converted to type of
color (for UI and 2D) to match to_linear.
Removed a redundant extra implementation of to_f32_array for LinearColor
as it is also supplied in ColorToComponents (I'm surprised that's
allowed?)
## Testing
Ran all tests and manually tested.
Added to_and_from_u8 to linear_rgba::tests
## Changelog
visible change is Color::linear becomes Color::to_linear.
---------
Co-authored-by: John Payne <20407779+johngpayne@users.noreply.github.com>
# Objective
The method `AssetServer::add_async` (added in
https://github.com/bevyengine/bevy/pull/13700) requires a future that
returns an `AssetLoadError` error, which was a bit of an oversight on my
part, as that type of error only really makes sense in the context of
bevy's own asset loader -- returning it from user-defined futures isn't
very useful.
## Solution
Allow passing custom error types to `add_async`, which get cast into a
trait object matching the form of `AssetLoader::load`. If merged before
the next release this will not be a breaking change
# Objective
Partially address #13408
Rework of #13613
Unify the very nice forms of interpolation specifically present in
`bevy_math` under a shared trait upon which further behavior can be
based.
The ideas in this PR were prompted by [Lerp smoothing is broken by Freya
Holmer](https://www.youtube.com/watch?v=LSNQuFEDOyQ).
## Solution
There is a new trait `StableInterpolate` in `bevy_math::common_traits`
which enshrines a quite-specific notion of interpolation with a lot of
guarantees:
```rust
/// A type with a natural interpolation that provides strong subdivision guarantees.
///
/// Although the only required method is `interpolate_stable`, many things are expected of it:
///
/// 1. The notion of interpolation should follow naturally from the semantics of the type, so
/// that inferring the interpolation mode from the type alone is sensible.
///
/// 2. The interpolation recovers something equivalent to the starting value at `t = 0.0`
/// and likewise with the ending value at `t = 1.0`.
///
/// 3. Importantly, the interpolation must be *subdivision-stable*: for any interpolation curve
/// between two (unnamed) values and any parameter-value pairs `(t0, p)` and `(t1, q)`, the
/// interpolation curve between `p` and `q` must be the *linear* reparametrization of the original
/// interpolation curve restricted to the interval `[t0, t1]`.
///
/// The last of these conditions is very strong and indicates something like constant speed. It
/// is called "subdivision stability" because it guarantees that breaking up the interpolation
/// into segments and joining them back together has no effect.
///
/// Here is a diagram depicting it:
/// ```text
/// top curve = u.interpolate_stable(v, t)
///
/// t0 => p t1 => q
/// |-------------|---------|-------------|
/// 0 => u / \ 1 => v
/// / \
/// / \
/// / linear \
/// / reparametrization \
/// / t = t0 * (1 - s) + t1 * s \
/// / \
/// |-------------------------------------|
/// 0 => p 1 => q
///
/// bottom curve = p.interpolate_stable(q, s)
/// ```
///
/// Note that some common forms of interpolation do not satisfy this criterion. For example,
/// [`Quat::lerp`] and [`Rot2::nlerp`] are not subdivision-stable.
///
/// Furthermore, this is not to be used as a general trait for abstract interpolation.
/// Consumers rely on the strong guarantees in order for behavior based on this trait to be
/// well-behaved.
///
/// [`Quat::lerp`]: crate::Quat::lerp
/// [`Rot2::nlerp`]: crate::Rot2::nlerp
pub trait StableInterpolate: Clone {
/// Interpolate between this value and the `other` given value using the parameter `t`.
/// Note that the parameter `t` is not necessarily clamped to lie between `0` and `1`.
/// When `t = 0.0`, `self` is recovered, while `other` is recovered at `t = 1.0`,
/// with intermediate values lying between the two.
fn interpolate_stable(&self, other: &Self, t: f32) -> Self;
}
```
This trait has a blanket implementation over `NormedVectorSpace`, where
`lerp` is used, along with implementations for `Rot2`, `Quat`, and the
direction types using variants of `slerp`. Other areas may choose to
implement this trait in order to hook into its functionality, but the
stringent requirements must actually be met.
This trait bears no direct relationship with `bevy_animation`'s
`Animatable` trait, although they may choose to use `interpolate_stable`
in their trait implementations if they wish, as both traits involve
type-inferred interpolations of the same kind. `StableInterpolate` is
not a supertrait of `Animatable` for a couple reasons:
1. Notions of interpolation in animation are generally going to be much
more general than those allowed under these constraints.
2. Laying out these generalized interpolation notions is the domain of
`bevy_animation` rather than of `bevy_math`. (Consider also that
inferring interpolation from types is not universally desirable.)
Similarly, this is not implemented on `bevy_color`'s color types,
although their current mixing behavior does meet the conditions of the
trait.
As an aside, the subdivision-stability condition is of interest
specifically for the [Curve
RFC](https://github.com/bevyengine/rfcs/pull/80), where it also ensures
a kind of stability for subsampling.
Importantly, this trait ensures that the "smooth following" behavior
defined in this PR behaves predictably:
```rust
/// Smoothly nudge this value towards the `target` at a given decay rate. The `decay_rate`
/// parameter controls how fast the distance between `self` and `target` decays relative to
/// the units of `delta`; the intended usage is for `decay_rate` to generally remain fixed,
/// while `delta` is something like `delta_time` from an updating system. This produces a
/// smooth following of the target that is independent of framerate.
///
/// More specifically, when this is called repeatedly, the result is that the distance between
/// `self` and a fixed `target` attenuates exponentially, with the rate of this exponential
/// decay given by `decay_rate`.
///
/// For example, at `decay_rate = 0.0`, this has no effect.
/// At `decay_rate = f32::INFINITY`, `self` immediately snaps to `target`.
/// In general, higher rates mean that `self` moves more quickly towards `target`.
///
/// # Example
/// ```
/// # use bevy_math::{Vec3, StableInterpolate};
/// # let delta_time: f32 = 1.0 / 60.0;
/// let mut object_position: Vec3 = Vec3::ZERO;
/// let target_position: Vec3 = Vec3::new(2.0, 3.0, 5.0);
/// // Decay rate of ln(10) => after 1 second, remaining distance is 1/10th
/// let decay_rate = f32::ln(10.0);
/// // Calling this repeatedly will move `object_position` towards `target_position`:
/// object_position.smooth_nudge(&target_position, decay_rate, delta_time);
/// ```
fn smooth_nudge(&mut self, target: &Self, decay_rate: f32, delta: f32) {
self.interpolate_stable_assign(target, 1.0 - f32::exp(-decay_rate * delta));
}
```
As the documentation indicates, the intention is for this to be called
in game update systems, and `delta` would be something like
`Time::delta_seconds` in Bevy, allowing positions, orientations, and so
on to smoothly follow a target. A new example, `smooth_follow`,
demonstrates a basic implementation of this, with a sphere smoothly
following a sharply moving target:
https://github.com/bevyengine/bevy/assets/2975848/7124b28b-6361-47e3-acf7-d1578ebd0347
## Testing
Tested by running the example with various parameters.
# Objective
- Add support for `segments` for extrusion-meshes, akin to what is
possible with cylinders
## Solution
- Added a `.segments(segments: usize)` function to `ExtrusionBuilder`.
- Implemented support for segments in the meshing algorithm.
- If you set `.segments(0)`, the meshing will fail, just like it does
with cylinders.
## Additional information
Here is a wireframe of some extrusions with 1, 2, 3, etc. segments:
![image_2024-06-06_233205114](https://github.com/bevyengine/bevy/assets/62256001/358081e2-172d-407b-8bdb-9cda88eb4664)
---------
Co-authored-by: Lynn Büttgenbach <62256001+solis-lumine-vorago@users.noreply.github.com>
# Objective
`Scene` and `DynamicScene` work with `InstanceInfo` at different levels
of abstraction
- `Scene::write_to_world_with` returns an `InstanceInfo` whereas
`DynamicScene::write_to_world_with` returns `()`. Instances are created
one level higher at the `SceneSpawner` API level.
- `DynamicScene::write_to_world_with` takes the `entity_map` as an
argument whereas the `Scene` version is less flexible and creates a new
one for you. No reason this needs to be the case.
## Solution
I propose changing `Scene::write_to_world_with` to match the API we have
for `DynamicScene`. Returning the `InstanceInfo` as we do today just
seems like a leaky abstraction - it's only used in
`spawn_sync_internal`. Being able to pass in an entity_map gives you
more flexibility with how you write entities to a world.
This also moves `InstanceInfo` out of `Scene` which is cleaner
conceptually. If someone wants to work with instances then they should
work with `SceneSpawner` - I see `write_to_world_with` as a lower-level
API to be used with exclusive world access.
## Testing
Code is just shifting things around.
## Changelog
Changed `Scene::write_to_world_with` to take `entity_map` as an argument
and no longer return an `InstanceInfo`
## Migration Guide
`Scene::write_to_world_with` no longer returns an `InstanceInfo`.
Before
```rust
scene.write_to_world_with(world, ®istry)
```
After
```rust
let mut entity_map = EntityHashMap::default();
scene.write_to_world_with(world, &mut entity_map, ®istry)
```
# Objective
- Let `init_non_send_resource` take `FromWorld` values again, not only
`Default`
- This reverts an unintended breaking change introduced in #9202
## Solution
- The resource initialized with `init_non_send_resource` requires
`FromWorld` again
# Objective
The `EntityCommands::despawn` method was previously changed from
panicking behavior to a warning, but the docs continue to state that it
panics.
## Solution
- Removed panic section, copied warning blurb from `World::despawn`
- Adds a similar warning blurb to
`DespawnRecursiveExt::despawn_recursive` and
`DespawnRecursiveExt::despawn_descendants`
# Objective
- In #13649 additional method had been added to AppExitStates, but there
feature gate left for method in implementation for App at refactoring
stage.
- Fixes#13733 .
## Solution
- Removed the feature gate.
## Testing
- Ran reproducing example from #13733 with no compilation errors
# Objective
- Fixes#13703
## Solution
- Added `mappings` to the `EntityMapper` trait, which returns an
iterator over currently tracked `Entity` to `Entity` mappings.
- Added `DynEntityMapper` as an [object
safe](https://doc.rust-lang.org/reference/items/traits.html#object-safety)
alternative to `EntityMapper`.
- Added `assert_object_safe` as a helper for ensuring traits are object
safe.
## Testing
- Added new unit test `entity_mapper_iteration` which tests the
`SceneEntityMapper` implementation of `EntityMapper::mappings`.
- Added unit tests to ensure `DynEntityMapper`, `DynEq` and `DynHash`
are object safe.
- Passed CI on my Windows 10 development environment
---
## Changelog
- Added `mappings` to `EntityMapper` trait.
## Migration Guide
- If you are implementing `EntityMapper` yourself, you can use the below
as a stub implementation:
```rust
fn mappings(&self) -> impl Iterator<Item = (Entity, Entity)> {
unimplemented!()
}
```
- If you were using `EntityMapper` as a trait object (`dyn
EntityMapper`), instead use `dyn DynEntityMapper` and its associated
methods.
## Notes
- The original issue proposed returning a `Vec` from `EntityMapper`
instead of an `impl Iterator` to preserve its object safety. This is a
simpler option, but also forces an allocation where it isn't strictly
needed. I've opted for this split into `DynEntityMapper` and
`EntityMapper` as it's been done several times across Bevy already, and
provides maximum flexibility to users.
- `assert_object_safe` is an empty function, since the assertion
actually happens once you try to use a `dyn T` for some trait `T`. I
have still added this function to clearly document what object safety is
within Bevy, and to create a standard way to communicate that a given
trait must be object safe.
- Other traits should have tests added to ensure object safety, but I've
left those off to avoid cluttering this PR further.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Remove some unnecessary coupling between `DynamicTextureAtlasBuilder`
and `bevy_asset`.
## Solution
Remove the dependency of `DynamicTextureAtlasBuilder::add_texture` to
`bevy_asset`, by directly passing the `Image` of the atlas to mutate,
instead of passing separate `Assets<Image>` and `Handle<Image>` for the
function to do the lookup by itself. The lookup can be done from the
caller, and this allows using the builder in contexts where the `Image`
is not stored inside `Assets`.
Clean-up a bit the font atlas files by introducing a `PlacedGlyph` type
storing the `GlyphId` and its `SubpixelOffset`, which were otherwise
always both passed as function parameters and the pair used as key in
hash maps.
## Testing
There's no change in behavior.
---
## Changelog
- Added a `PlacedGlyph` type aggregating a `GlyphId` and a
`SubpixelOffset`. That type is now used as parameter in a few text atlas
APIs, instead of passing individual values.
## Migration Guide
- Replace the `glyph_id` and `subpixel_offset` of a few text atlas APIs
by a single `place_glyph: PlacedGlyph` parameter trivially combining the
two.
# Objective
Some use cases might require holding onto the previous state of the
animation player for change detection.
## Solution
Added `clone` and `copy` implementation to most animation types.
Added optimized `clone_from` implementations for the specific use case
of holding a `PreviousAnimationPlayer` component.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
previously I worked on fixing issue #13646, back when the error message
did not include the type at all.
But that error message had room for improvement, so I included the
feedback of @alice-i-cecile and @MrGVSV.
The error message will now read `the given key (of type
bevy_reflect::tests::Foo) does not support hashing` or 'the given key
(of type bevy_reflect::DynamicStruct) does not support hashing' in case
of a dynamic struct that represents a hashable struct
i also added a new unit test for this new behaviour
(`reflect_map_no_hash_dynamic`).
Fixes#13646 (again)
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>