ExposureSettings rebase + improvements#32
Merged
superdump merged 515 commits intosuperdump:exposure-settingsfrom Sep 1, 2023
Merged
ExposureSettings rebase + improvements#32superdump merged 515 commits intosuperdump:exposure-settingsfrom
superdump merged 515 commits intosuperdump:exposure-settingsfrom
Conversation
# Objective Title. This is necessary in order to update [`bevy-trait-query`](https://crates.io/crates/bevy-trait-query) to Bevy 0.11. --- ## Changelog Added the unsafe function `UnsafeWorldCell::storages`, which provides unchecked access to the internal data stores of a `World`.
# Objective **This implementation is based on bevyengine/rfcs#59 --- Resolves bevyengine#4597 Full details and motivation can be found in the RFC, but here's a brief summary. `FromReflect` is a very powerful and important trait within the reflection API. It allows Dynamic types (e.g., `DynamicList`, etc.) to be formed into Real ones (e.g., `Vec<i32>`, etc.). This mainly comes into play concerning deserialization, where the reflection deserializers both return a `Box<dyn Reflect>` that almost always contain one of these Dynamic representations of a Real type. To convert this to our Real type, we need to use `FromReflect`. It also sneaks up in other ways. For example, it's a required bound for `T` in `Vec<T>` so that `Vec<T>` as a whole can be made `FromReflect`. It's also required by all fields of an enum as it's used as part of the `Reflect::apply` implementation. So in other words, much like `GetTypeRegistration` and `Typed`, it is very much a core reflection trait. The problem is that it is not currently treated like a core trait and is not automatically derived alongside `Reflect`. This makes using it a bit cumbersome and easy to forget. ## Solution Automatically derive `FromReflect` when deriving `Reflect`. Users can then choose to opt-out if needed using the `#[reflect(from_reflect = false)]` attribute. ```rust #[derive(Reflect)] struct Foo; #[derive(Reflect)] #[reflect(from_reflect = false)] struct Bar; fn test<T: FromReflect>(value: T) {} test(Foo); // <-- OK test(Bar); // <-- Panic! Bar does not implement trait `FromReflect` ``` #### `ReflectFromReflect` This PR also automatically adds the `ReflectFromReflect` (introduced in bevyengine#6245) registration to the derived `GetTypeRegistration` impl— if the type hasn't opted out of `FromReflect` of course. <details> <summary><h4>Improved Deserialization</h4></summary> > **Warning** > This section includes changes that have since been descoped from this PR. They will likely be implemented again in a followup PR. I am mainly leaving these details in for archival purposes, as well as for reference when implementing this logic again. And since we can do all the above, we might as well improve deserialization. We can now choose to deserialize into a Dynamic type or automatically convert it using `FromReflect` under the hood. `[Un]TypedReflectDeserializer::new` will now perform the conversion and return the `Box`'d Real type. `[Un]TypedReflectDeserializer::new_dynamic` will work like what we have now and simply return the `Box`'d Dynamic type. ```rust // Returns the Real type let reflect_deserializer = UntypedReflectDeserializer::new(®istry); let mut deserializer = ron::de::Deserializer::from_str(input)?; let output: SomeStruct = reflect_deserializer.deserialize(&mut deserializer)?.take()?; // Returns the Dynamic type let reflect_deserializer = UntypedReflectDeserializer::new_dynamic(®istry); let mut deserializer = ron::de::Deserializer::from_str(input)?; let output: DynamicStruct = reflect_deserializer.deserialize(&mut deserializer)?.take()?; ``` </details> --- ## Changelog * `FromReflect` is now automatically derived within the `Reflect` derive macro * This includes auto-registering `ReflectFromReflect` in the derived `GetTypeRegistration` impl * ~~Renamed `TypedReflectDeserializer::new` and `UntypedReflectDeserializer::new` to `TypedReflectDeserializer::new_dynamic` and `UntypedReflectDeserializer::new_dynamic`, respectively~~ **Descoped** * ~~Changed `TypedReflectDeserializer::new` and `UntypedReflectDeserializer::new` to automatically convert the deserialized output using `FromReflect`~~ **Descoped** ## Migration Guide * `FromReflect` is now automatically derived within the `Reflect` derive macro. Items with both derives will need to remove the `FromReflect` one. ```rust // OLD #[derive(Reflect, FromReflect)] struct Foo; // NEW #[derive(Reflect)] struct Foo; ``` If using a manual implementation of `FromReflect` and the `Reflect` derive, users will need to opt-out of the automatic implementation. ```rust // OLD #[derive(Reflect)] struct Foo; impl FromReflect for Foo {/* ... */} // NEW #[derive(Reflect)] #[reflect(from_reflect = false)] struct Foo; impl FromReflect for Foo {/* ... */} ``` <details> <summary><h4>Removed Migrations</h4></summary> > **Warning** > This section includes changes that have since been descoped from this PR. They will likely be implemented again in a followup PR. I am mainly leaving these details in for archival purposes, as well as for reference when implementing this logic again. * The reflect deserializers now perform a `FromReflect` conversion internally. The expected output of `TypedReflectDeserializer::new` and `UntypedReflectDeserializer::new` is no longer a Dynamic (e.g., `DynamicList`), but its Real counterpart (e.g., `Vec<i32>`). ```rust let reflect_deserializer = UntypedReflectDeserializer::new_dynamic(®istry); let mut deserializer = ron::de::Deserializer::from_str(input)?; // OLD let output: DynamicStruct = reflect_deserializer.deserialize(&mut deserializer)?.take()?; // NEW let output: SomeStruct = reflect_deserializer.deserialize(&mut deserializer)?.take()?; ``` Alternatively, if this behavior isn't desired, use the `TypedReflectDeserializer::new_dynamic` and `UntypedReflectDeserializer::new_dynamic` methods instead: ```rust // OLD let reflect_deserializer = UntypedReflectDeserializer::new(®istry); // NEW let reflect_deserializer = UntypedReflectDeserializer::new_dynamic(®istry); ``` </details> --------- Co-authored-by: Carter Anderson <[email protected]>
# Objective - This fixes a crash when loading shaders, when running an Adreno GPU and using WebGL mode. - Fixes bevyengine#8506 - Fixes bevyengine#8047 ## Solution - The shader pbr_functions.wgsl, will fail in apply_fog function, trying to access values that are null on Adreno chipsets using WebGL, these devices are commonly found in android handheld devices. --------- Co-authored-by: Carter Anderson <[email protected]> Co-authored-by: François <[email protected]>
…e#8993) # Objective Followup bugfix for bevyengine#5703. Without this we get the following error when CAS (Contrast Adaptive Sharpening) is enabled: ``` 2023-06-29T01:31:23.829331Z ERROR bevy_render::render_resource::pipeline_cache: failed to process shader: error: unknown type: 'FullscreenVertexOutput' ┌─ crates/bevy_core_pipeline/src/contrast_adaptive_sharpening/robust_contrast_adaptive_sharpening.wgsl:63:17 │ 63 │ fn fragment(in: FullscreenVertexOutput) -> @location(0) vec4<f32> { │ ^^^^^^^^^^^^^^^^^^^^^^ unknown type │ = unknown type: 'FullscreenVertexOutput' ``` @robtfm I wouldn't expect this to fail. I was under the impression the `#import bevy_core_pipeline::fullscreen_vertex_shader` would pull "everything" from that file into this one?
# Objective The setup code in `animated_fox` uses a `done` boolean to avoid running the `play` logic repetitively. It is a common pattern, but it just work with exactly one fox, and misses an even more common pattern. When a user modifies the code to try it with several foxes, they are confused as to why it doesn't work (bevyengine#8996). ## Solution The more common pattern is to use `Added<AnimationPlayer>` as a query filter. This both reduces complexity and naturally extend the setup code to handle several foxes, added at any time.
# Objective Since 10f5c92, parallax mapping was broken. When bevyengine#5703 was merged, the change from `in.uv` to `uv` in the pbr shader was reverted. So the shader would use the wrong coordinate to sample the various textures. ## Solution We revert to using the correct uv.
…e#8926) # Objective Fixes bevyengine#8925 ## Solution ~~Clamp the bad values.~~ Normalize the prepass normals when we get them in the `prepass_normal()` function. ## More Info The issue is that NdotV is sometimes very slightly greater than 1 (maybe FP rounding issues?), which caused `F_Schlick()` to return NANs in `pow(1.0 - NdotV, 5.0)` (call stack looked like`pbr()` -> `directional_light()` -> `Fd_Burley()` -> `F_Schlick()`)
# Objective Since 10f5c92, shadows were broken for models with morph target. When bevyengine#5703 was merged, the morph target code in `render/mesh.wgsl` was correctly updated to use the new import syntax. However, similar code exists in `prepass/prepass.wgsl`, but it was never update. (the reason code is duplicated is that the `Vertex` struct is different for both files). ## Solution Update the code, so that shadows render correctly with morph targets.
# Objective - Remove all references to base sets following schedule-first rework ## Solution - Remove last references to base sets in `GraphInfo`
# Objective The bounding box colors are from bevy_gizmo are randomized between app runs. This can get confusing for users. ## Solution Use a fixed seed with `RandomState::with_seeds` rather than initializing a `AHash`. The random number was chose so that the first few colors are clearly distinct. According to the `RandomState::hash_one` documentation, it's also faster.  --- ## Changelog * bevy_gizmo: Keep a consistent color for AABBs of identical entities between runs
# Objective - Fix bevyengine#8984 ### Solution - Address compilation errors I admit: I did sneak it an unrelated mini-refactor. of the `measurment.rs` module. it seemed to me that directly importing `taffy` types helped reduce a lot of boilerplate, so I did it.
# Objective - Remove need to call `.get()` on two ticks to compare them for equality. ## Solution - Derive `Eq` and `PartialEq`. --- ## Changelog > `Tick` now implements `Eq` and `PartialEq`
# Objective bevy_render currently has a dependency on a random older version of once_cell which is not used anywhere. ## Solution Remove the dependency ## Changelog N/A ## Migration Guide N/A
# Objective `bevy_animation::PlayingAnimation` derives `Reflect` but is not registered. ## Solution Register `bevy_animation::PlayingAnimation`.
# Objective I'm creating an iOS game and had to find a way to persist game state when the application is terminated. This required listening to the [`applicationWillTerminate()` method](https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1623111-applicationwillterminate), but I cannot do so myself anymore since `winit` already set up a delegate to listen for it, and there can be only one delegate. So I had to move up the stack and try to respond to one of the events from `winit` instead. It appears `winit` fires two events that could serve my purpose: `WindowEvent::Destroyed` and `Event::LoopDestroyed`. It seemed to me the former might be slightly more generally useful, and I also found a past discussion that suggested it would be appropriate for Bevy to have a `WindowDestroyed` event: bevyengine#5589 (comment) ## Solution - I've added the `WindowDestroyed` event, which fires when `winit` fires `WindowEvent::Destroyed`. --- ## Changelog ### Added - Introduced a new `WindowDestroyed` event type. It is used to indicate a window has been destroyed by the windowing system.
# Objective Adds a border to each button. The borders can be disabled with the "no-borders" command line argument.
…bevyengine#9027) # Objective - Fixes bevyengine#8989 ## Solution - Renamed Interaction::Clicked -> Interaction::Pressed - Minor changes to comments to keep clarity of terms ## Migration Guide - Rename all instances of Interaction::Clicked -> Interaction::Pressed
…ngine#9024) # Objective - Fixes bevyengine#8630. ## Solution Since a camera's view and projection matrices are modified during `PostUpdate` in `camera_system` and `propagate_transforms`, it is fine to move `update_previous_view_projections` from `Update` to `PreUpdate`. Doing so adds consistence with `update_mesh_previous_global_transforms` and allows systems in `Update` to use `PreviousViewProjection` correctly without explicit ordering.
# Objective The current mobile example produces an APK of 1.5 Gb. - Running the example on a real device takes significant time (around one minute just to copy the file over USB to my phone). - Default virtual devices in Android studio run out of space after the first install. This can of course be solved/configured, but it causes unnecessary friction. - One impression could be, that Bevy produces bloated APKs. 1.5Gb is even double the size of debug builds for desktop examples. ## Solution - Strip the debug symbols of the shared libraries before they are copied to the APK APK size after this change: 200Mb Copy time on my machine: ~8s ## Considered alternative APKs built in release mode are only 50Mb in size, but require setting up signing for the profile and compile longer.
# Objective In `extract_uinodes` it's not neccessary to clone the `DEFAULT_IMAGE_HANDLE.typed()` handle.
…ogical (bevyengine#8720) # Objective After the UI layout is computed when the coordinates are converted back from physical coordinates to logical coordinates the `UiScale` is ignored. This results in a confusing situation where we have two different systems of logical coordinates. Example: ```rust use bevy::prelude::*; fn main() { App::new() .add_plugins(DefaultPlugins) .add_systems(Startup, setup) .add_systems(Update, update) .run(); } fn setup(mut commands: Commands, mut ui_scale: ResMut<UiScale>) { ui_scale.scale = 4.; commands.spawn(Camera2dBundle::default()); commands.spawn(NodeBundle { style: Style { align_items: AlignItems::Center, justify_content: JustifyContent::Center, width: Val::Percent(100.), ..Default::default() }, ..Default::default() }) .with_children(|builder| { builder.spawn(NodeBundle { style: Style { width: Val::Px(100.), height: Val::Px(100.), ..Default::default() }, background_color: Color::MAROON.into(), ..Default::default() }).with_children(|builder| { builder.spawn(TextBundle::from_section("", TextStyle::default()); }); }); } fn update( mut text_query: Query<(&mut Text, &Parent)>, node_query: Query<Ref<Node>>, ) { for (mut text, parent) in text_query.iter_mut() { let node = node_query.get(parent.get()).unwrap(); if node.is_changed() { text.sections[0].value = format!("size: {}", node.size()); } } } ``` result:  We asked for a 100x100 UI node but the Node's size is multiplied by the value of `UiScale` to give a logical size of 400x400. ## Solution Divide the output physical coordinates by `UiScale` in `ui_layout_system` and multiply the logical viewport size by `UiScale` when creating the projection matrix for the UI's `ExtractedView` in `extract_default_ui_camera_view`. --- ## Changelog * The UI layout's physical coordinates are divided by both the window scale factor and `UiScale` when converting them back to logical coordinates. The logical size of Ui nodes now matches the values given to their size constraints. * Multiply the logical viewport size by `UiScale` before creating the projection matrix for the UI's `ExtractedView` in `extract_default_ui_camera_view`. * In `ui_focus_system` the cursor position returned from `Window` is divided by `UiScale`. * Added a scale factor parameter to `Node::physical_size` and `Node::physical_rect`. * The example `viewport_debug` now uses a `UiScale` of 2. to ensure that viewport coordinates are working correctly with a non-unit `UiScale`. ## Migration Guide Physical UI coordinates are now divided by both the `UiScale` and the window's scale factor to compute the logical sizes and positions of UI nodes. This ensures that UI Node size and position values, held by the `Node` and `GlobalTransform` components, conform to the same logical coordinate system as the style constraints from which they are derived, irrespective of the current `scale_factor` and `UiScale`. --------- Co-authored-by: Carter Anderson <[email protected]>
# Objective
Currently, `DynamicScene`s extract all components listed in the given
(or the world's) type registry. This acts as a quasi-filter of sorts.
However, it can be troublesome to use effectively and lacks decent
control.
For example, say you need to serialize only the following component over
the network:
```rust
#[derive(Reflect, Component, Default)]
#[reflect(Component)]
struct NPC {
name: Option<String>
}
```
To do this, you'd need to:
1. Create a new `AppTypeRegistry`
2. Register `NPC`
3. Register `Option<String>`
If we skip Step 3, then the entire scene might fail to serialize as
`Option<String>` requires registration.
Not only is this annoying and easy to forget, but it can leave users
with an impossible task: serializing a third-party type that contains
private types.
Generally, the third-party crate will register their private types
within a plugin so the user doesn't need to do it themselves. However,
this means we are now unable to serialize _just_ that type— we're forced
to allow everything!
## Solution
Add the `SceneFilter` enum for filtering components to extract.
This filter can be used to optionally allow or deny entire sets of
components/resources. With the `DynamicSceneBuilder`, users have more
control over how their `DynamicScene`s are built.
To only serialize a subset of components, use the `allow` method:
```rust
let scene = builder
.allow::<ComponentA>()
.allow::<ComponentB>()
.extract_entity(entity)
.build();
```
To serialize everything _but_ a subset of components, use the `deny`
method:
```rust
let scene = builder
.deny::<ComponentA>()
.deny::<ComponentB>()
.extract_entity(entity)
.build();
```
Or create a custom filter:
```rust
let components = HashSet::from([type_id]);
let filter = SceneFilter::Allowlist(components);
// let filter = SceneFilter::Denylist(components);
let scene = builder
.with_filter(Some(filter))
.extract_entity(entity)
.build();
```
Similar operations exist for resources:
<details>
<summary>View Resource Methods</summary>
To only serialize a subset of resources, use the `allow_resource`
method:
```rust
let scene = builder
.allow_resource::<ResourceA>()
.extract_resources()
.build();
```
To serialize everything _but_ a subset of resources, use the
`deny_resource` method:
```rust
let scene = builder
.deny_resource::<ResourceA>()
.extract_resources()
.build();
```
Or create a custom filter:
```rust
let resources = HashSet::from([type_id]);
let filter = SceneFilter::Allowlist(resources);
// let filter = SceneFilter::Denylist(resources);
let scene = builder
.with_resource_filter(Some(filter))
.extract_resources()
.build();
```
</details>
### Open Questions
- [x] ~~`allow` and `deny` are mutually exclusive. Currently, they
overwrite each other. Should this instead be a panic?~~ Took @soqb's
suggestion and made it so that the opposing method simply removes that
type from the list.
- [x] ~~`DynamicSceneBuilder` extracts entity data as soon as
`extract_entity`/`extract_entities` is called. Should this behavior
instead be moved to the `build` method to prevent ordering mixups (e.g.
`.allow::<Foo>().extract_entity(entity)` vs
`.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be
iterating over the given entities twice: once at extraction and again at
build.~~ Based on the feedback from @Testare it sounds like it might be
better to just keep the current functionality (if anything we can open a
separate PR that adds deferred methods for extraction, so the
choice/performance hit is up to the user).
- [ ] An alternative might be to remove the filter from
`DynamicSceneBuilder` and have it as a separate parameter to the
extraction methods (either in the existing ones or as added
`extract_entity_with_filter`-type methods). Is this preferable?
- [x] ~~Should we include constructors that include common types to
allow/deny? For example, a `SceneFilter::standard_allowlist` that
includes things like `Parent` and `Children`?~~ Consensus suggests we
should. I may split this out into a followup PR, though.
- [x] ~~Should we add the ability to remove types from the filter
regardless of whether an allowlist or denylist (e.g.
`filter.remove::<Foo>()`)?~~ See the first list item
- [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a
struct that contains an `is_denylist` boolean?~~ With the added
`SceneFilter::None` state (replacing the need to wrap in an `Option` or
rely on an empty `Denylist`), it seems an enum is better suited now
- [x] ~~Bikeshed: Do we like the naming convention? Should we instead
use `include`/`exclude` terminology?~~ Sounds like we're sticking with
`allow`/`deny`!
- [x] ~~Does this feature need a new example? Do we simply include it in
the existing one (maybe even as a comment?)? Should this be done in a
followup PR instead?~~ Example will be added in a followup PR
### Followup Tasks
- [ ] Add a dedicated `SceneFilter` example
- [ ] Possibly add default types to the filter (e.g. deny things like
`ComputedVisibility`, allow `Parent`, etc)
---
## Changelog
- Added the `SceneFilter` enum for filtering components and resources
when building a `DynamicScene`
- Added methods:
- `DynamicSceneBuilder::with_filter`
- `DynamicSceneBuilder::allow`
- `DynamicSceneBuilder::deny`
- `DynamicSceneBuilder::allow_all`
- `DynamicSceneBuilder::deny_all`
- `DynamicSceneBuilder::with_resource_filter`
- `DynamicSceneBuilder::allow_resource`
- `DynamicSceneBuilder::deny_resource`
- `DynamicSceneBuilder::allow_all_resources`
- `DynamicSceneBuilder::deny_all_resources`
- Removed methods:
- `DynamicSceneBuilder::from_world_with_type_registry`
- `DynamicScene::from_scene` and `DynamicScene::from_world` no longer
require an `AppTypeRegistry` reference
## Migration Guide
- `DynamicScene::from_scene` and `DynamicScene::from_world` no longer
require an `AppTypeRegistry` reference:
```rust
// OLD
let registry = world.resource::<AppTypeRegistry>();
let dynamic_scene = DynamicScene::from_world(&world, registry);
// let dynamic_scene = DynamicScene::from_scene(&scene, registry);
// NEW
let dynamic_scene = DynamicScene::from_world(&world);
// let dynamic_scene = DynamicScene::from_scene(&scene);
```
- Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the
registry is automatically taken from the given world:
```rust
// OLD
let registry = world.resource::<AppTypeRegistry>();
let builder = DynamicSceneBuilder::from_world_with_type_registry(&world,
registry);
// NEW
let builder = DynamicSceneBuilder::from_world(&world);
```
# Objective `accesskit` and `accesskit_winit` need to be upgraded. ## Solution Upgrade `accesskit` and `accesskit_winit`. --- ## Changelog ### Changed * Upgrade accesskit to v0.11. * Upgrade accesskit_winit to v0.14.
# Objective
Improve the `bevy_audio` API to make it more user-friendly and
ECS-idiomatic. This PR is a first-pass at addressing some of the most
obvious (to me) problems. In the interest of keeping the scope small,
further improvements can be done in future PRs.
The current `bevy_audio` API is very clunky to work with, due to how it
(ab)uses bevy assets to represent audio sinks.
The user needs to write a lot of boilerplate (accessing
`Res<Assets<AudioSink>>`) and deal with a lot of cognitive overhead
(worry about strong vs. weak handles, etc.) in order to control audio
playback.
Audio playback is initiated via a centralized `Audio` resource, which
makes it difficult to keep track of many different sounds playing in a
typical game.
Further, everything carries a generic type parameter for the sound
source type, making it difficult to mix custom sound sources (such as
procedurally generated audio or unofficial formats) with regular audio
assets.
Let's fix these issues.
## Solution
Refactor `bevy_audio` to a more idiomatic ECS API. Remove the `Audio`
resource. Do everything via entities and components instead.
Audio playback data is now stored in components:
- `PlaybackSettings`, `SpatialSettings`, `Handle<AudioSource>` are now
components. The user inserts them to tell Bevy to play a sound and
configure the initial playback parameters.
- `AudioSink`, `SpatialAudioSink` are now components instead of special
magical "asset" types. They are inserted by Bevy when it actually begins
playing the sound, and can be queried for by the user in order to
control the sound during playback.
Bundles: `AudioBundle` and `SpatialAudioBundle` are available to make it
easy for users to play sounds. Spawn an entity with one of these bundles
(or insert them to a complex entity alongside other stuff) to play a
sound.
Each entity represents a sound to be played.
There is also a new "auto-despawn" feature (activated using
`PlaybackSettings`), which, if enabled, tells Bevy to despawn entities
when the sink playback finishes. This allows for "fire-and-forget" sound
playback. Users can simply
spawn entities whenever they want to play sounds and not have to worry
about leaking memory.
## Unsolved Questions
I think the current design is *fine*. I'd be happy for it to be merged.
It has some possibly-surprising usability pitfalls, but I think it is
still much better than the old `bevy_audio`. Here are some discussion
questions for things that we could further improve. I'm undecided on
these questions, which is why I didn't implement them. We should decide
which of these should be addressed in this PR, and what should be left
for future PRs. Or if they should be addressed at all.
### What happens when sounds start playing?
Currently, the audio sink components are inserted and the bundle
components are kept. Should Bevy remove the bundle components? Something
else?
The current design allows an entity to be reused for playing the same
sound with the same parameters repeatedly. This is a niche use case I'd
like to be supported, but if we have to give it up for a simpler design,
I'd be fine with that.
### What happens if users remove any of the components themselves?
As described above, currently, entities can be reused. Removing the
audio sink causes it to be "detached" (I kept the old `Drop` impl), so
the sound keeps playing. However, if the audio bundle components are not
removed, Bevy will detect this entity as a "queued" sound entity again
(has the bundle compoenents, without a sink component), just like before
playing the sound the first time, and start playing the sound again.
This behavior might be surprising? Should we do something different?
### Should mutations to `PlaybackSettings` be applied to the audio sink?
We currently do not do that. `PlaybackSettings` is just for the initial
settings when the sound starts playing. This is clearly documented.
Do we want to keep this behavior, or do we want to allow users to use
`PlaybackSettings` instead of `AudioSink`/`SpatialAudioSink` to control
sounds during playback too?
I think I prefer for them to be kept separate. It is not a bad mental
model once you understand it, and it is documented.
### Should `AudioSink` and `SpatialAudioSink` be unified into a single
component type?
They provide a similar API (via the `AudioSinkPlayback` trait) and it
might be annoying for users to have to deal with both of them. The
unification could be done using an enum that is matched on internally by
the methods. Spatial audio has extra features, so this might make it
harder to access. I think we shouldn't.
### Automatic synchronization of spatial sound properties from
Transforms?
Should Bevy automatically apply changes to Transforms to spatial audio
entities? How do we distinguish between listener and emitter? Which one
does the transform represent? Where should the other one come from?
Alternatively, leave this problem for now, and address it in a future
PR. Or do nothing, and let users deal with it, as shown in the
`spatial_audio_2d` and `spatial_audio_3d` examples.
---
## Changelog
Added:
- `AudioBundle`/`SpatialAudioBundle`, add them to entities to play
sounds.
Removed:
- The `Audio` resource.
- `AudioOutput` is no longer `pub`.
Changed:
- `AudioSink`, `SpatialAudioSink` are now components instead of assets.
## Migration Guide
// TODO: write a more detailed migration guide, after the "unsolved
questions" are answered and this PR is finalized.
Before:
```rust
/// Need to store handles somewhere
#[derive(Resource)]
struct MyMusic {
sink: Handle<AudioSink>,
}
fn play_music(
asset_server: Res<AssetServer>,
audio: Res<Audio>,
audio_sinks: Res<Assets<AudioSink>>,
mut commands: Commands,
) {
let weak_handle = audio.play_with_settings(
asset_server.load("music.ogg"),
PlaybackSettings::LOOP.with_volume(0.5),
);
// upgrade to strong handle and store it
commands.insert_resource(MyMusic {
sink: audio_sinks.get_handle(weak_handle),
});
}
fn toggle_pause_music(
audio_sinks: Res<Assets<AudioSink>>,
mymusic: Option<Res<MyMusic>>,
) {
if let Some(mymusic) = &mymusic {
if let Some(sink) = audio_sinks.get(&mymusic.sink) {
sink.toggle();
}
}
}
```
Now:
```rust
/// Marker component for our music entity
#[derive(Component)]
struct MyMusic;
fn play_music(
mut commands: Commands,
asset_server: Res<AssetServer>,
) {
commands.spawn((
AudioBundle::from_audio_source(asset_server.load("music.ogg"))
.with_settings(PlaybackSettings::LOOP.with_volume(0.5)),
MyMusic,
));
}
fn toggle_pause_music(
// `AudioSink` will be inserted by Bevy when the audio starts playing
query_music: Query<&AudioSink, With<MyMusic>>,
) {
if let Ok(sink) = query.get_single() {
sink.toggle();
}
}
```
# Objective - accesskit_unix is not optional anymore ## Solution - Enable `async-io` feature of `accesskit_winit` only when `accesskit_unix` is enabled
…ne#6690) # Objective Fixes bevyengine#6689. ## Solution Add `single-threaded` as an optional non-default feature to `bevy_ecs` and `bevy_tasks` that: - disable the `ParallelExecutor` as a default runner - disables the multi-threaded `TaskPool` - internally replace `QueryParIter::for_each` calls with `Query::for_each`. Removed the `Mutex` and `Arc` usage in the single-threaded task pool.  ## Future Work/TODO Create type aliases for `Mutex`, `Arc` that change to single-threaaded equivalents where possible. --- ## Changelog Added: Optional default feature `multi-theaded` to that enables multithreaded parallelism in the engine. Disabling it disables all multithreading in exchange for higher single threaded performance. Does nothing on WASM targets. --------- Co-authored-by: Carter Anderson <[email protected]>
…ugin` (bevyengine#9054) This pull request is mutually exclusive with bevyengine#9066. # Objective Complete the initialization of the plugin in `ScheduleRunnerPlugin`. ## Solution Wait for asynchronous tasks to complete, then `App::finish` and `App::cleanup` in the runner function.
# Objective fixes bevyengine#8911, bevyengine#7712 ## Solution Rounding was added to Taffy which fixed issue bevyengine#7712. The implementation uses the f32 `round` method which rounds ties (fractional part is a half) away from zero. Issue bevyengine#8911 occurs when a node's min and max bounds on either axis are "ties" and zero is between them. Then the bounds are rounded away from each other, and the node grows by a pixel. This alone shouldn't cause the node to expand continuously, but I think there is some interaction with the way Taffy recomputes a layout from its cached data that I didn't identify. This PR fixes bevyengine#8911 by first disabling Taffy's internal rounding and using an alternative rounding function that rounds ties up. Then, instead of rounding the values of the internal layout tree as Taffy's built-in rounding does, we leave those values unmodified and only the values stored in the components are rounded. This requires walking the tree for the UI node geometry update rather than iterating through a query. Because the component values are regenerated each update, that should mean that UI updates are idempotent (ish) now and make the growing node behaviour seen in issue bevyengine#8911 impossible. I expected a performance regression, but it's an improvement on main: ``` cargo run --profile stress-test --features trace_tracy --example many_buttons ``` <img width="461" alt="ui-rounding-fix-compare" src="https://github.com/bevyengine/bevy/assets/27962798/914bfd50-e18a-4642-b262-fafa69005432"> I guess it makes sense to do the rounding together with the node size and position updates. --- ## Changelog `bevy_ui::layout`: * Taffy's built-in rounding is disabled and rounding is now performed by `ui_layout_system`. * Instead of rounding the values of the internal layout tree as Taffy's built-in rounding does, we leave those values unmodified and only the values stored in the components are rounded. This requires walking the tree for the UI node geometry update rather than iterating through a query. Because the component values are regenerated each update, that should mean that UI updates are idempotent now and make the growing node behaviour seen in issue bevyengine#8911 impossible. * Added two helper functions `round_ties_up` and `round_layout_coordinates`. --------- Co-authored-by: Carter Anderson <[email protected]>
This release I'm experimenting with a new changelog format that doesn't sap hours of extra time to put together. Basically: 1. Sort by "development area" instead of added / changed / fixed. These distinctions are often unclear anyway and we have no automated way of assigning them currently. "Development area" sorting feels equally useful (if not more useful). Our changelog generator also already does "development area" categorization so this is efficient. 2. Sort by "importance" and trim out irrelevant changes. This is laborious, but I already do this as part of my blog post construction methodology, so it isn't "extra" work.
# Objective Remove `Val`'s `try_*` arithmetic methods. fixes bevyengine#9571 ## Changelog Removed these methods from `bevy_ui::ui_node::Val`: - `try_add` - `try_sub` - `try_add_assign_with_size` - `try_sub_assign_with_size` - `try_add_assign` - `try_sub_assign` - `try_add_assign_with_size` - `try_sub_assign_with_size` ## Migration Guide `Val`'s `try_*` arithmetic methods have been removed. To perform arithmetic on `Val`s deconstruct them using pattern matching.
# Objective - Fixes [bevyengine#8835](bevyengine#8835) ## Solution - Added a note to the `set_volume` docstring which explains how volume is interpreted. --------- Co-authored-by: Alice Cecile <[email protected]> Co-authored-by: GitGhillie <[email protected]> Co-authored-by: François <[email protected]>
# Objective To enable non exclusive system usage of reflected components and make reflection more ergonomic to use by making it more in line with standard entity commands. ## Solution - Implements a new `EntityCommands` extension trait for reflection related functions in the reflect module of bevy_ecs. - Implements 4 new commands, `insert_reflect`, `insert_reflect_with_registry`, `remove_reflect`, and `remove_reflect_with_registry`. Both insert commands take a `Box<dyn Reflect>` component while the remove commands take the component type name. - Made `EntityCommands` fields pub(crate) to allow access in the reflect module. (Might be worth making these just public to enable user end custom entity commands in a different pr) - Added basic tests to ensure the commands are actually working. - Documentation of functions. --- ## Changelog Added: - Implements 4 new commands on the new entity commands extension. - `insert_reflect` - `remove_reflect` - `insert_reflect_with_registry` - `remove_reflect_with_registry` The commands operate the same except the with_registry commands take a generic parameter for a resource that implements `AsRef<TypeRegistry>`. Otherwise the default commands use the `AppTypeRegistry` for reflection data. Changed: - Made `EntityCommands` fields pub(crate) to allow access in the reflect module. > Hopefully this time it works. Please don't make me rebase again ☹
…ettings` to all schedules (bevyengine#9514) # Objective - Fixes: bevyengine#9508 - Fixes: bevyengine#9526 ## Solution - Adds ```rust fn configure_schedules(&mut self, schedule_build_settings: ScheduleBuildSettings) ``` to `Schedules`, and `App` to simplify applying `ScheduleBuildSettings` to all schedules. --- ## Migration Guide - No breaking changes. - Adds `Schedule::get_build_settings()` getter for the schedule's `ScheduleBuildSettings`. - Can replaced manual configuration of all schedules: ```rust // Old for (_, schedule) in app.world.resource_mut::<Schedules>().iter_mut() { schedule.set_build_settings(build_settings); } // New app.configure_schedules(build_settings); ```
# Objective Doc comment for the `global_transform` field in `NodeBundle` says: ``` /// This field is automatically managed by the UI layout system. ``` The `GlobalTransform` component is the thing being managed, not the `global_transform` field, and the `TransformPropagate` systems do the managing, not the UI layout system.
…vyengine#9542) # Objective Every frame, `Events::update` gets called, which clears out any old events from the buffer. There should be a way of taking ownership of these old events instead of throwing them away. My use-case is dumping old events into a debug menu so they can be inspected later. One potential workaround is to just have a system that clones any incoming events and stores them in a list -- however, this requires the events to implement `Clone`. ## Solution Add `Events::update_drain`, which returns an iterator of the events that were removed from the buffer.
# Objective - Move schedule name into `Schedule` to allow the schedule name to be used for errors and tracing in Schedule methods - Fixes bevyengine#9510 ## Solution - Move label onto `Schedule` and adjust api's on `World` and `Schedule` to not pass explicit label where it makes sense to. - add name to errors and tracing. - `Schedule::new` now takes a label so either add the label or use `Schedule::default` which uses a default label. `default` is mostly used in doc examples and tests. --- ## Changelog - move label onto `Schedule` to improve error message and logging for schedules. ## Migration Guide `Schedule::new` and `App::add_schedule` ```rust // old let schedule = Schedule::new(); app.add_schedule(MyLabel, schedule); // new let schedule = Schedule::new(MyLabel); app.add_schedule(schedule); ``` if you aren't using a label and are using the schedule struct directly you can use the default constructor. ```rust // old let schedule = Schedule::new(); schedule.run(world); // new let schedule = Schedule::default(); schedule.run(world); ``` `Schedules:insert` ```rust // old let schedule = Schedule::new(); schedules.insert(MyLabel, schedule); // new let schedule = Schedule::new(MyLabel); schedules.insert(schedule); ``` `World::add_schedule` ```rust // old let schedule = Schedule::new(); world.add_schedule(MyLabel, schedule); // new let schedule = Schedule::new(MyLabel); world.add_schedule(schedule); ```
# Objective Fix bevyengine#4278 Fix bevyengine#5504 Fix bevyengine#9422 Provide safe ways to borrow an entire entity, while allowing disjoint mutable access. `EntityRef` and `EntityMut` are not suitable for this, since they provide access to the entire world -- they are just helper types for working with `&World`/`&mut World`. This has potential uses for reflection and serialization ## Solution Remove `EntityRef::world`, which allows it to soundly be used within queries. `EntityMut` no longer supports structural world mutations, which allows multiple instances of it to exist for different entities at once. Structural world mutations are performed using the new type `EntityWorldMut`. ```rust fn disjoint_system( q2: Query<&mut A>, q1: Query<EntityMut, Without<A>>, ) { ... } let [entity1, entity2] = world.many_entities_mut([id1, id2]); *entity1.get_mut::<T>().unwrap() = *entity2.get().unwrap(); for entity in world.iter_entities_mut() { ... } ``` --- ## Changelog - Removed `EntityRef::world`, to fix a soundness issue with queries. + Removed the ability to structurally mutate the world using `EntityMut`, which allows it to be used in queries. + Added `EntityWorldMut`, which is used to perform structural mutations that are no longer allowed using `EntityMut`. ## Migration Guide **Note for maintainers: ensure that the guide for bevyengine#9604 is updated accordingly.** Removed the method `EntityRef::world`, to fix a soundness issue with queries. If you need access to `&World` while using an `EntityRef`, consider passing the world as a separate parameter. `EntityMut` can no longer perform 'structural' world mutations, such as adding or removing components, or despawning the entity. Additionally, `EntityMut::world`, `EntityMut::world_mut` , and `EntityMut::world_scope` have been removed. Instead, use the newly-added type `EntityWorldMut`, which is a helper type for working with `&mut World`. --------- Co-authored-by: Alice Cecile <[email protected]>
Legitimately, bevy emits a WARN when encountering entities in UI trees without NodeBunlde components. Bevy pretty much always panics when such a thing happens, due to the update_clipping system. However, sometimes, it's perfectly legitimate to have a child without UI nodes in a UI tree. For example, as a "seed" entity that is consumed by a 3rd party plugin, which will later spawn a valid UI tree. In loading scenarios, you are pretty much guaranteed to have incomplete children. The presence of the WARN hints that bevy does not intend to panic on such occasion (otherwise the warn! would be a panic!) so I assume panic is an unintended behavior, aka a bug. ## Solution Early-return instead of panicking. I did only test that it indeed fixed the panic, not checked for UI inconsistencies. Though on a logical level, it can only have changed code that would otherwise panic. ## Alternatives Instead of early-returning on invalid entity in `update_clipping`, do not call it with invalid entity in its recursive call. --- ## Changelog - Do not panic on non-UI child of UI entity
…upport (bevyengine#9568) # Objective Rename `Val`'s `evaluate` method to `resolve`. Implement `resolve` support for `Val`'s viewport variants. fixes bevyengine#9535 --- ## Changelog `bevy_ui::ui_node::Val`: * Renamed the following methods and added a `viewport_size` parameter: - `evaluate` to `resolve` - `try_add_with_size` to `try_add_with_context` - `try_add_assign_with_size` to `try_add_assign_with_context` - `try_sub_with_size` to `try_sub_with_context` - `try_sub_assign_with_size` to `try_sub_assign_with_context` * Implemented `resolve` support for `Val`'s viewport coordinate types ## Migration Guide * Renamed the following `Val` methods and added a `viewport_size` parameter: - `evaluate` to `resolve` - `try_add_with_size` to `try_add_with_context` - `try_add_assign_with_size` to `try_add_assign_with_context` - `try_sub_with_size` to `try_sub_with_context` - `try_sub_assign_with_size` to `try_sub_assign_with_context`
# Objective As far as I can tell, this is no longer needed since the switch to fancier shader imports via `naga_oil`. This shouldn't have any affect on compile times because it's in our tree from `naga_oil`, `tracing-subscriber`, and `rodio`.
…mple (bevyengine#9619) # Objective Change two places where `entity.insert` is used to add components individually to spawn a tuple bundle instead.
# Objective - Some of the old ambiguity tests didn't get ported over during schedule v3. ## Solution - Port over tests from https://github.com/bevyengine/bevy/blob/15ee98db8d1c6705111e0f11a8fc240ceaf9f2db/crates/bevy_ecs/src/schedule/ambiguity_detection.rs#L279-L612 with minimal changes - Make a method to convert the ambiguity conflicts to a string for easier verification of correct results.
# Objective The latest `clippy` release has a much more aggressive application of the [`explicit_iter_loop`](https://rust-lang.github.io/rust-clippy/master/index.html#/explicit_into_iter_loop?groups=pedantic) pedantic lint. As a result, clippy now suggests the following: ```diff -for event in events.iter() { +for event in &mut events { ``` I'm generally in favor of this lint. Using `for mut item in &mut query` is also recommended over `for mut item in query.iter_mut()` for good reasons IMO. But, it is my personal belief that `&mut events` is much less clear than `events.iter()`. Why? The reason is that the events from `EventReader` **are not mutable**, they are immutable references to each event in the event reader. `&mut events` suggests we are getting mutable access to events — similarly to `&mut query` — which is not the case. Using `&mut events` is therefore misleading. `IntoIterator` requires a mutable `EventReader` because it updates the internal `last_event_count`, not because it let you mutate it. So clippy's suggested improvement is a downgrade. ## Solution Do not implement `IntoIterator` for `&mut events`. Without the impl, clippy won't suggest its "fix". This also prevents generally people from using `&mut events` for iterating `EventReader`s, which makes the ecosystem every-so-slightly better. --- ## Changelog - Removed `IntoIterator` impl for `&mut EventReader` ## Migration Guide - `&mut EventReader` does not implement `IntoIterator` anymore. replace `for foo in &mut events` by `for foo in events.iter()`
# Objective - Since bevyengine#9387, there is no need to import VecSwizzles separately, it is already included in the prelude. ## Solution - Remove the imports.
# Objective - Avoid using bevy_internal imports in examples. ## Solution - Add CI to check for bevy_internal imports like suggested in bevyengine#9547 (comment) - Fix another import I don't know much about CI so I don't know if this is the better approach, but I think is better than doing a pull request every time I found this lol, any suggestion is welcome. --------- Co-authored-by: Rob Parrett <[email protected]>
# Objective - Make the default docs more useful like suggested in bevyengine#9600 (comment) ## Solution - Move the documentation to the `fn default()` method instead of the `impl Default`. Allows you to view the docs directly on the function without having to go to the implementation. ### Before  ### After 
# Objective - The current `EventReader::iter` has been determined to cause confusion among new Bevy users. It was suggested by @JoJoJet to rename the method to better clarify its usage. - Solves bevyengine#9624 ## Solution - Rename `EventReader::iter` to `EventReader::read`. - Rename `EventReader::iter_with_id` to `EventReader::read_with_id`. - Rename `ManualEventReader::iter` to `ManualEventReader::read`. - Rename `ManualEventReader::iter_with_id` to `ManualEventReader::read_with_id`. --- ## Changelog - `EventReader::iter` has been renamed to `EventReader::read`. - `EventReader::iter_with_id` has been renamed to `EventReader::read_with_id`. - `ManualEventReader::iter` has been renamed to `ManualEventReader::read`. - `ManualEventReader::iter_with_id` has been renamed to `ManualEventReader::read_with_id`. - Deprecated `EventReader::iter` - Deprecated `EventReader::iter_with_id` - Deprecated `ManualEventReader::iter` - Deprecated `ManualEventReader::iter_with_id` ## Migration Guide - Existing usages of `EventReader::iter` and `EventReader::iter_with_id` will have to be changed to `EventReader::read` and `EventReader::read_with_id` respectively. - Existing usages of `ManualEventReader::iter` and `ManualEventReader::iter_with_id` will have to be changed to `ManualEventReader::read` and `ManualEventReader::read_with_id` respectively.
# Objective The two functions [`Urect::height`](https://docs.rs/bevy_math/latest/bevy_math/struct.URect.html#method.height), [`Urect::width`](https://docs.rs/bevy_math/latest/bevy_math/struct.URect.html#method.width) are currently not const. Since the methods are very unlikely to change (ever) and are useful to be const for some games, they should be. Co-authored-by: Emi <[email protected]>
…yengine#9630) # Objective Make it easier to create bounding boxes in user code by providing a constructor that computes a box surrounding an arbitrary number of points. ## Solution Add `Aabb::enclosing`, which accepts iterators, slices, or arrays. --------- Co-authored-by: Tristan Guichaoua <[email protected]>
plural "windows" in message where it should be singular "window"
# Objective - Fix compilation issue with wrongly specified glam version - bevy uses `Vec2::INFINITY`, depends on `0.24` (equivalent to `0.24.0`) yet it was only introduced in version `0.24.1` Context: https://discord.com/channels/691052431525675048/692572690833473578/1146586570787397794 ## Solution - Bump glam version.
… the window (bevyengine#9657) # Objective `Window::physical_cursor_position` checks to see if the cursor's position is inside the window but it constructs the bounding rect for the window using its logical size and then checks to see if it contains the cursor's physical position. When the physical size is smaller than the logical size, this leaves a dead zone where the cursor is over the window but its position is unreported. fixes: bevyengine#9656 ## Solution Use the physical size of the window.
# Objective I've been collecting some mistakes in the documentation and fixed them --------- Co-authored-by: Emi <[email protected]> Co-authored-by: Nicola Papale <[email protected]>
# Objective - I broke ambiguity reporting in one of my refactors. `conflicts_to_string` should have been using the passed in parameter rather than the one stored on self.
# Objective - Fixes bevyengine#9641 - Anonymous sets are named by their system members. When `ScheduleBuildSettings::report_sets` is on, systems are named by their sets. So when getting the anonymous set name this would cause an infinite recursion. ## Solution - When getting the anonymous system set name, don't get their system's names with the sets the systems belong to. ## Other Possible solutions - An alternate solution might be to skip anonymous sets when getting the system's name for an anonymous set's name.
Author
|
@superdump I'd like for you to review and merge this PR, and then I'll do a followup PR (or maybe someone else will) to fix all the example exposures. I'd rather not do that as part of this PR. |
superdump
approved these changes
Sep 1, 2023
Owner
superdump
left a comment
superdump
left a comment
There was a problem hiding this comment.
After the merge of main I can’t see what the actual changes are anymore. In future when making PRs against my branches, please don’t merge main in the same PR as other changes as it makes it near impossible to review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.