# Objective
- #6402 changed `World::fetch_table` (now `UnsafeWorldCell::fetch_table`) to access the archetype in order to get the `table_id` and `table_row` of the entity involved. However this is useless since those were already present in the `EntityLocation`
- Moreover it's useless for `UnsafeWorldCell::fetch_table` to return the `TableRow` too, since the caller must already have access to the `EntityLocation` which contains the `TableRow`.
- The result is that `UnsafeWorldCell::fetch_table` now only does 2 memory fetches instead of 4.
## Solution
- Revert the changes to the implementation of `UnsafeWorldCell::fetch_table` made in #6402
…u64, so hash safety is not a concern
# Objective
- While reading the code, just noticed the BundleInfo's HashMap is std::collections::HashMap, which uses a slow but safe hasher.
## Solution
- Use bevy_utils::HashMap instead
benchmark diff (I run several times in a linux box, the perf improvement is consistent, though numbers varies from time to time, I paste my last run result here):
``` bash
cargo bench -- spawn
Compiling bevy_ecs v0.9.0 (/home/lishuo/developer/pr/bevy/crates/bevy_ecs)
Compiling bevy_app v0.9.0 (/home/lishuo/developer/pr/bevy/crates/bevy_app)
Compiling benches v0.1.0 (/home/lishuo/developer/pr/bevy/benches)
Finished bench [optimized] target(s) in 1m 17s
Running benches/bevy_ecs/change_detection.rs (/home/lishuo/developer/pr/bevy/benches/target/release/deps/change_detection-86c5445d0dc34529)
Gnuplot not found, using plotters backend
Running benches/bevy_ecs/benches.rs (/home/lishuo/developer/pr/bevy/benches/target/release/deps/ecs-e49b3abe80bfd8c0)
Gnuplot not found, using plotters backend
spawn_commands/2000_entities
time: [153.94 µs 159.19 µs 164.37 µs]
change: [-14.706% -11.050% -6.9633%] (p = 0.00 < 0.05)
Performance has improved.
spawn_commands/4000_entities
time: [328.77 µs 339.11 µs 349.11 µs]
change: [-7.6331% -3.9932% +0.0487%] (p = 0.06 > 0.05)
No change in performance detected.
spawn_commands/6000_entities
time: [445.01 µs 461.29 µs 477.36 µs]
change: [-16.639% -13.358% -10.006%] (p = 0.00 < 0.05)
Performance has improved.
spawn_commands/8000_entities
time: [657.94 µs 677.71 µs 696.95 µs]
change: [-8.8708% -5.2591% -1.6847%] (p = 0.01 < 0.05)
Performance has improved.
get_or_spawn/individual time: [452.02 µs 466.70 µs 482.07 µs]
change: [-17.218% -14.041% -10.728%] (p = 0.00 < 0.05)
Performance has improved.
get_or_spawn/batched time: [291.12 µs 301.12 µs 311.31 µs]
change: [-12.281% -8.9163% -5.3660%] (p = 0.00 < 0.05)
Performance has improved.
spawn_world/1_entities time: [81.668 ns 84.284 ns 86.860 ns]
change: [-12.251% -6.7872% -1.5402%] (p = 0.02 < 0.05)
Performance has improved.
spawn_world/10_entities time: [789.78 ns 821.96 ns 851.95 ns]
change: [-19.738% -14.186% -8.0733%] (p = 0.00 < 0.05)
Performance has improved.
spawn_world/100_entities
time: [7.9906 µs 8.2449 µs 8.5013 µs]
change: [-12.417% -6.6837% -0.8766%] (p = 0.02 < 0.05)
Change within noise threshold.
spawn_world/1000_entities
time: [81.602 µs 84.161 µs 86.833 µs]
change: [-13.656% -8.6520% -3.0491%] (p = 0.00 < 0.05)
Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild
Benchmarking spawn_world/10000_entities: Warming up for 500.00 ms
Warning: Unable to complete 100 samples in 4.0s. You may wish to increase target time to 4.0s, enable flat sampling, or reduce sample count to 70.
spawn_world/10000_entities
time: [813.02 µs 839.76 µs 865.41 µs]
change: [-12.133% -6.1970% -0.2302%] (p = 0.05 < 0.05)
Change within noise threshold.
```
---
## Changelog
> This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.
- use bevy_utils::HashMap for Bundles::bundle_ids
## Migration Guide
> This section is optional. If there are no breaking changes, you can delete this section.
- Not a breaking change, hashmap is internal impl.
# Objective
We have a few old system labels that are now system sets but are still named or documented as labels. Documentation also generally mentioned system labels in some places.
## Solution
- Clean up naming and documentation regarding system sets
## Migration Guide
`PrepareAssetLabel` is now called `PrepareAssetSet`
# Objective
- Improve readability of the run condition for systems only running in a certain state
## Solution
- Rename `state_equals` to `in_state` (see [comment by cart](https://github.com/bevyengine/bevy/pull/7634#issuecomment-1428740311) in #7634 )
- `.run_if(state_equals(variant))` now is `.run_if(in_state(variant))`
This breaks the naming pattern a bit with the related conditions `state_exists` and `state_exists_and_equals` but I could not think of better names for those and think the improved readability of `in_state` is worth it.
# Objective
- Fixes#7612
- Since #7493, windows started as unfocused
## Solution
- Creating the window at the end of the event loop after the resume event instead of at the beginning of the loop of the next event fixes the focus
# Objective
The text contained by a text node is only recomputed when its `Style` or `Text` components change, or when the scale factor changes. Not when the geometry of the text node is modified.
Make it so that any change in text node size triggers a text recomputation.
## Solution
Change `text_system` so that it queries for text nodes with changed `Node` components and recomputes their text.
---
Most users won't notice any difference but it should fix some confusing edge cases in more complicated and interactive layouts.
## Changelog
* Added `Changed<Node>` to the change detection query of `text_system`. This ensures that any change in the size of a text node will cause any text it contains to be recomputed.
# Objective
Standard material defaults are currently strange, and the docs are wrong re: metallic.
## Solution
Change the defaults to be similar to [Godot](https://github.com/godotengine/godot/pull/62756).
---
## Changelog
#### Changed
- `StandardMaterial` now defaults to a dielectric material (0.0 `metallic`) with 0.5 `perceptual_roughness`.
## Migration Guide
`StandardMaterial`'s default have now changed to be a fully dielectric material with medium roughness. If you want to use the old defaults, you can set `perceptual_roughness = 0.089` and `metallic = 0.01` (though metallic should generally only be set to 0.0 or 1.0).
# Objective
Fixes#7632.
As discussed in #7634, it can be quite challenging for users to intuit the mental model of how states now work.
## Solution
Rather than change the behavior of the `OnUpdate` system set, instead work on making sure it's easy to understand what's going on.
Two things have been done:
1. Remove the `.on_update` method from our bevy of system building traits. This was special-cased and made states feel much more magical than they need to.
2. Improve the docs for the `OnUpdate` system set.
# Objective
Resolves#7121
## Solution
Decouples `List` and `Array` by removing `Array` as a supertrait of `List`. Additionally, similar methods from `Array` have been added to `List` so that their usages can remain largely unchanged.
#### Possible Alternatives
##### `Sequence`
My guess for why we originally made `List` a subtrait of `Array` is that they share a lot of common operations. We could potentially move these overlapping methods to a `Sequence` (name taken from #7059) trait and make that a supertrait of both. This would allow functions to contain logic that simply operates on a sequence rather than "list vs array".
However, this means that we'd need to add methods for converting to a `dyn Sequence`. It also might be confusing since we wouldn't add a `ReflectRef::Sequence` or anything like that. Is such a trait worth adding (either in this PR or a followup one)?
---
## Changelog
- Removed `Array` as supertrait of `List`
- Added methods to `List` that were previously provided by `Array`
## Migration Guide
The `List` trait is no longer dependent on `Array`. Implementors of `List` can remove the `Array` impl and move its methods into the `List` impl (with only a couple tweaks).
```rust
// BEFORE
impl Array for Foo {
fn get(&self, index: usize) -> Option<&dyn Reflect> {/* ... */}
fn get_mut(&mut self, index: usize) -> Option<&mut dyn Reflect> {/* ... */}
fn len(&self) -> usize {/* ... */}
fn is_empty(&self) -> bool {/* ... */}
fn iter(&self) -> ArrayIter {/* ... */}
fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {/* ... */}
fn clone_dynamic(&self) -> DynamicArray {/* ... */}
}
impl List for Foo {
fn insert(&mut self, index: usize, element: Box<dyn Reflect>) {/* ... */}
fn remove(&mut self, index: usize) -> Box<dyn Reflect> {/* ... */}
fn push(&mut self, value: Box<dyn Reflect>) {/* ... */}
fn pop(&mut self) -> Option<Box<dyn Reflect>> {/* ... */}
fn clone_dynamic(&self) -> DynamicList {/* ... */}
}
// AFTER
impl List for Foo {
fn get(&self, index: usize) -> Option<&dyn Reflect> {/* ... */}
fn get_mut(&mut self, index: usize) -> Option<&mut dyn Reflect> {/* ... */}
fn insert(&mut self, index: usize, element: Box<dyn Reflect>) {/* ... */}
fn remove(&mut self, index: usize) -> Box<dyn Reflect> {/* ... */}
fn push(&mut self, value: Box<dyn Reflect>) {/* ... */}
fn pop(&mut self) -> Option<Box<dyn Reflect>> {/* ... */}
fn len(&self) -> usize {/* ... */}
fn is_empty(&self) -> bool {/* ... */}
fn iter(&self) -> ListIter {/* ... */}
fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {/* ... */}
fn clone_dynamic(&self) -> DynamicList {/* ... */}
}
```
Some other small tweaks that will need to be made include:
- Use `ListIter` for `List::iter` instead of `ArrayIter` (the return type from `Array::iter`)
- Replace `array_hash` with `list_hash` in `Reflect::reflect_hash` for implementors of `List`
# Objective
- rebased version of #6155
The `MaterialPipeline` cannot be integrated into other pipelines like the `MeshPipeline`.
## Solution
Implement `Clone` for `MaterialPipeline`. Expose systems and resources part of the `MaterialPlugin` to allow custom assembly - especially combining existing systems and resources with a custom `queue_material_meshes` system.
# Changelog
## Added
- Clone impl for MaterialPipeline
## Changed
- ExtractedMaterials, extract_materials and prepare_materials are now public
# Objective
- Fixes: #7187
Since avoiding the `SRes::into_inner` call does not seem to be possible, this PR tries to at least document its usage.
I am not sure if I explained the lifetime issue correctly, please let me know if something is incorrect.
## Solution
- Add information about the `SRes::into_inner` usage on both `RenderCommand` and `Res`
# Objective
Add doc tests for the `Size` constructor functions.
Also changed the function descriptions so they explicitly state the values that elided values are given.
## Changelog
* Added doc tests for the `Size` constructor functions.
# Objective
The `size` field of `CalculatedSize` shouldn't be a `Size` as it only ever stores (unscaled) pixel values. By default its fields are `Val::Auto` but these are converted to `0`s before being sent to Taffy.
## Solution
Change the `size` field of `CalculatedSize` to a Vec2.
## Changelog
* Changed the `size` field of `CalculatedSize` to a Vec2.
* Removed the `Val` <-> `f32` conversion code for `CalculatedSize`.
## Migration Guide
* The size field of `CalculatedSize` has been changed to a `Vec2`.
Profiles show that in extremely hot loops, like the draw loops in the renderer, invoking the trace! macro has noticeable overhead, even if the trace log level is not enabled.
Solve this by introduce a 'wrapper' detailed_trace macro around trace, that wraps the trace! log statement in a trivially false if statement unless a cargo feature is enabled
# Objective
- Eliminate significant overhead observed with trace-level logging in render hot loops, even when trace log level is not enabled.
- This is an alternative solution to the one proposed in #7223
## Solution
- Introduce a wrapper around the `trace!` macro called `detailed_trace!`. This macro wraps the `trace!` macro with an if statement that is conditional on a new cargo feature, `detailed_trace`. When the feature is not enabled (the default), then the if statement is trivially false and should be optimized away at compile time.
- Convert the observed hot occurrences of trace logging in `TrackedRenderPass` with this new macro.
Testing the results of
```
cargo run --profile stress-test --features bevy/trace_tracy --example many_cubes -- spheres
```
![image](https://user-images.githubusercontent.com/1222141/218298552-38551717-b062-4c64-afdc-a60267ac984d.png)
shows significant improvement of the `main_opaque_pass_3d` of the renderer, a median time decrease from 6.0ms to 3.5ms.
---
## Changelog
- For performance reasons, some detailed renderer trace logs now require the use of cargo feature `detailed_trace` in addition to setting the log level to `TRACE` in order to be shown.
## Migration Guide
- Some detailed bevy trace events now require the use of the cargo feature `detailed_trace` in addition to enabling `TRACE` level logging to view. Should you wish to see these logs, please compile your code with the bevy feature `detailed_trace`. Currently, the only logs that are affected are the renderer logs pertaining to `TrackedRenderPass` functions
# Objective
- Required features were added to some examples in #7051 even though those features aren't the main focus of the examples
- Don't require features on examples that are useful without them
## Solution
- Remove required features on examples `load_gltf` and `scene_viewer`, but log a warning when they are not enabled
# Objective
Related to #7530.
`EventReader` iterators currently use the default impl for `.count()`, which unnecessarily loops over all unread events.
# Solution
Add specialized impls that mark the `EventReader` as consumed and return the number of unread events.
# Objective
There was issue #191 requesting subdivisions on the shape::Plane.
I also could have used this recently. I then write the solution.
Fixes #191
## Solution
I changed the shape::Plane to include subdivisions field and the code to create the subdivisions. I don't know how people are counting subdivisions so as I put in the doc comments 0 subdivisions results in the original geometry of the Plane.
Greater then 0 results in the number of lines dividing the plane.
I didn't know if it would be better to create a new struct that implemented this feature, say SubdivisionPlane or change Plane. I decided on changing Plane as that was what the original issue was.
It would be trivial to alter this to use another struct instead of altering Plane.
The issues of migration, although small, would be eliminated if a new struct was implemented.
## Changelog
### Added
Added subdivisions field to shape::Plane
## Migration Guide
All the examples needed to be updated to initalize the subdivisions field.
Also there were two tests in tests/window that need to be updated.
A user would have to update all their uses of shape::Plane to initalize the subdivisions field.
# Objective
- While working on scope recently, I ran into a missing invariant for the transmutes in scope. The references passed into Scope are active for the rest of the scope function, but rust doesn't know this so it allows using the owned `spawned` and `scope` after `f` returns.
## Solution
- Update the safety comment
- Shadow the owned values so they can't be used.
# Objective
In our project we parse `GltfNode` from `*.gltf` file, and we need extra properties information from Blender. Right now there is no way to get this properties from GltfNode (only through query when you spawn scene), so objective of this PR is to add extra properties to `GltfNode`
## Solution
Store extra properties inside `Gltf` structs
---
## Changelog
- Add pub field `extras` to `GltfNode`/`GltfMesh`/`GltfPrimitive` which store extras
- Add pub field `material_extras` to `GltfPrimitive` which store material extras
# Objective
`Size::width` sets the `height` field to `Val::DEFAULT` which is `Val::Undefined`, but the default for `Size` `height` is `Val::Auto`.
`Size::height` has the same problem, but with the `width` field.
The UI examples specify numeric values in many places where they could either be elided or replaced by composition of the Flex enum properties.
related: https://github.com/bevyengine/bevy/pull/7468
fixes: https://github.com/bevyengine/bevy/issues/6498
## Solution
Change `Size::width` so it sets `height` to `Val::AUTO` and change `Size::height` so it sets `width` to `Val::AUTO`.
Added some tests so this doesn't happen again.
## Changelog
Changed `Size::width` so it sets the `height` to `Val::AUTO`.
Changed `Size::height` so it sets the `width` to `Val::AUTO`.
Added tests to `geometry.rs` for `Size` and `UiRect` to ensure correct behaviour.
Simplified the UI examples. Replaced numeric values with the Flex property enums or elided them where possible, and removed the remaining use of auto margins.
## Migration Guide
The `Size::width` constructor function now sets the `height` to `Val::Auto` instead of `Val::Undefined`.
The `Size::height` constructor function now sets the `width` to `Val::Auto` instead of `Val::Undefined`.
# Objective
Make the name less verbose without sacrificing clarity.
---
## Migration Guide
*Note for maintainers:* This PR has no breaking changes relative to bevy 0.9. Instead of this PR having its own migration guide, we should just edit the changelog for #6404.
The type `UnsafeWorldCellEntityRef` has been renamed to `UnsafeEntityCell`.
# Objective
Continuation of #7560.
`MutUntyped::last_changed` and `set_last_changed` do not behave as described in their docs.
## Solution
Fix them using the same approach that was used for `Mut<>` in #7560.
fixes#6799
# Objective
We should be able to reuse the `Globals` or `View` shader struct definitions from anywhere (including third party plugins) without needing to worry about defining unrelated shader defs.
Also we'd like to refactor these structs to not be repeatedly defined.
## Solution
Refactor both `Globals` and `View` into separate importable shaders.
Use the imports throughout.
Co-authored-by: Torstein Grindvik <52322338+torsteingrindvik@users.noreply.github.com>
# Objective
- This makes code a little more readable now.
## Solution
- Use `position` provided by `Iter` instead of `enumerating` indices and `map`ping to the index.
This was missed in #7205.
Should be fixed now. 😄
## Migration Guide
- `SpecializedComputePipelines::specialize` now takes a `&PipelineCache` instead of a `&mut PipelineCache`
# Objective
The current `AlignSelf` doc comments:
```rust
pub enum AlignSelf {
/// Use the value of [`AlignItems`]
Auto,
/// If the parent has [`AlignItems::Center`] only this item will be at the start
FlexStart,
/// If the parent has [`AlignItems::Center`] only this item will be at the end
FlexEnd,
/// If the parent has [`AlignItems::FlexStart`] only this item will be at the center
Center,
/// If the parent has [`AlignItems::Center`] only this item will be at the baseline
Baseline,
/// If the parent has [`AlignItems::Center`] only this item will stretch along the whole cross axis
Stretch,
}
```
Actual behaviour of `AlignSelf` in Bevy main:
<img width="642" alt="align_self" src="https://user-images.githubusercontent.com/27962798/217795178-7a82638f-118d-4474-b7f9-ca27f204731d.PNG">
The white label at the top of each column is the parent node's `AlignItems` setting.
`AlignSelf` is always applied, not (as the documentation states) only when the parent has `AlignItems::Center` or `AlignItems::FlexStart` set.
```rust
use bevy::prelude::*;
fn main() {
App::new()
.add_plugins(DefaultPlugins)
.add_startup_system(setup)
.run();
}
fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
commands.spawn(Camera2dBundle::default());
commands.spawn(NodeBundle {
style: Style {
justify_content: JustifyContent::SpaceAround,
align_items: AlignItems::Center,
size: Size::new(Val::Percent(100.), Val::Percent(100.)),
..Default::default()
},
background_color: BackgroundColor(Color::NAVY),
..Default::default()
}).with_children(|builder| {
for align_items in [
AlignItems::Baseline,
AlignItems::FlexStart,
AlignItems::Center,
AlignItems::FlexEnd,
AlignItems::Stretch,
] {
builder.spawn(NodeBundle {
style: Style {
align_items,
flex_direction: FlexDirection::Column,
justify_content: JustifyContent::SpaceBetween,
size: Size::new(Val::Px(150.), Val::Px(500.)),
..Default::default()
},
background_color: BackgroundColor(Color::DARK_GRAY),
..Default::default()
}).with_children(|builder| {
builder.spawn((
TextBundle {
text: Text::from_section(
format!("AlignItems::{align_items:?}"),
TextStyle {
font: asset_server.load("fonts/FiraSans-Regular.ttf"),
font_size: 16.0,
color: Color::BLACK,
},
),
style: Style {
align_self: AlignSelf::Stretch,
..Default::default()
},
..Default::default()
},
BackgroundColor(Color::WHITE),
));
for align_self in [
AlignSelf::Auto,
AlignSelf::FlexStart,
AlignSelf::Center,
AlignSelf::FlexEnd,
AlignSelf::Baseline,
AlignSelf::Stretch,
] {
builder.spawn((
TextBundle {
text: Text::from_section(
format!("AlignSelf::{align_self:?}"),
TextStyle {
font: asset_server.load("fonts/FiraSans-Regular.ttf"),
font_size: 16.0,
color: Color::WHITE,
},
),
style: Style {
align_self,
..Default::default()
},
..Default::default()
},
BackgroundColor(Color::BLACK),
));
}
});
}
});
}
```
# Objective
Make `last_changed` behave as described in its docs.
## Solution
- Return `changed` instead of `last_change_tick`. `last_change_tick` is the system's previous tick and is just used for comparison.
- Update the docs of the similarly named `set_last_changed` (which does correctly interact with `last_change_tick`) to clarify that the two functions touch different data. (I advocate for renaming one or the other if anyone has any good suggestions).
It also might make sense to return a cloned `Tick` instead of `u32`.
---
## Changelog
- Fixed `DetectChanges::last_changed` returning the wrong value.
- Fixed `DetectChangesMut::set_last_changed` not actually updating the `changed` tick.
## Migration Guide
- The incorrect value that was being returned by `DetectChanges::last_changed` was the previous run tick of the system checking for changed values. If you depended on this value, you can get it from the `SystemChangeTick` `SystemParam` instead.
(Before)
![image](https://user-images.githubusercontent.com/47158642/213946111-15ec758f-1f1d-443c-b196-1fdcd4ae49da.png)
(After)
![image](https://user-images.githubusercontent.com/47158642/217051179-67381e73-dd44-461b-a2c7-87b0440ef8de.png)
![image](https://user-images.githubusercontent.com/47158642/212492404-524e4ad3-7837-4ed4-8b20-2abc276aa8e8.png)
# Objective
- Improve lighting; especially reflections.
- Closes https://github.com/bevyengine/bevy/issues/4581.
## Solution
- Implement environment maps, providing better ambient light.
- Add microfacet multibounce approximation for specular highlights from Filament.
- Occlusion is no longer incorrectly applied to direct lighting. It now only applies to diffuse indirect light. Unsure if it's also supposed to apply to specular indirect light - the glTF specification just says "indirect light". In the case of ambient occlusion, for instance, that's usually only calculated as diffuse though. For now, I'm choosing to apply this just to indirect diffuse light, and not specular.
- Modified the PBR example to use an environment map, and have labels.
- Added `FallbackImageCubemap`.
## Implementation
- IBL technique references can be found in environment_map.wgsl.
- It's more accurate to use a LUT for the scale/bias. Filament has a good reference on generating this LUT. For now, I just used an analytic approximation.
- For now, environment maps must first be prefiltered outside of bevy using a 3rd party tool. See the `EnvironmentMap` documentation.
- Eventually, we should have our own prefiltering code, so that we can have dynamically changing environment maps, as well as let users drop in an HDR image and use asset preprocessing to create the needed textures using only bevy.
---
## Changelog
- Added an `EnvironmentMapLight` camera component that adds additional ambient light to a scene.
- StandardMaterials will now appear brighter and more saturated at high roughness, due to internal material changes. This is more physically correct.
- Fixed StandardMaterial occlusion being incorrectly applied to direct lighting.
- Added `FallbackImageCubemap`.
Co-authored-by: IceSentry <c.giguere42@gmail.com>
Co-authored-by: James Liu <contact@jamessliu.com>
Co-authored-by: Rob Parrett <robparrett@gmail.com>
# Objective
Clarify what the function is actually calculating.
The `Tick::is_older_than` function is actually calculating whether the tick is newer than the system's `last_change_tick`, not older. As far as I can tell, the engine was using it correctly everywhere already.
## Solution
- Rename the function.
---
## Changelog
- `Tick::is_older_than` was renamed to `Tick::is_newer_than`. This is not a functional change, since that was what was always being calculated, despite the wrong name.
## Migration Guide
- Replace usages of `Tick::is_older_than` with `Tick::is_newer_than`.
# Objective
Closes#7202
## Solution
~~Introduce a `not` helper to pipe conditions. Opened mostly for discussion. Maybe create an extension trait with `not` method? Please, advice.~~
Introduce `not(condition)` condition that inverses the result of the passed.
---
## Changelog
### Added
- `not` condition.
# Objective
Fixes#7565
`ecs_guide` example is currently panicking. This seems to just be a problem with the example only caused by the base sets PR.
## Solution
First, changed a few `in_set` to `in_base_set` to fix a few of
```
thread 'main' panicked at 'Systems cannot be added to 'base' system sets using 'in_set'. Use 'in_base_set' instead.', examples/ecs/ecs_guide.rs:301:45
```
And then added an `in_base_set` to fix the resulting (confusing) cycle error
```
2023-02-06T13:54:29.213843Z ERROR bevy_ecs::schedule_v3::schedule: schedule contains at least 1 cycle(s) -- cycle(s) found within:
---- 0: ["ecs_guide::game_over_system", "ecs_guide::score_check_system"]
```
I also changed this `add_system` call so the comment above and below make sense:
```diff
// add_system(system) adds systems to the Update system set by default
// However we can manually specify the set if we want to. The following is equivalent to
// add_system(score_system)
- .add_system(score_system)
+ .add_system(score_system.in_base_set(CoreSet::Update))
// There are other `CoreSets`, such as `Last` which runs at the very end of each run.
```
## Notes
- Does `MySet` even need to be a base set? Seems like yes.
- Is that cycle error when there is no explicit `in_base_set` actually expected?
# Objective
- Terminology used in field names and docs aren't accurate
- `window_origin` doesn't have any effect when `scaling_mode` is `ScalingMode::None`
- `left`, `right`, `bottom`, and `top` are set automatically unless `scaling_mode` is `None`. Fields that only sometimes give feedback are confusing.
- `ScalingMode::WindowSize` has no arguments, which is inconsistent with other `ScalingMode`s. 1 pixel = 1 world unit is also typically way too wide.
- `OrthographicProjection` feels generally less streamlined than its `PerspectiveProjection` counterpart
- Fixes#5818
- Fixes#6190
## Solution
- Improve consistency in `OrthographicProjection`'s public fields (they should either always give feedback or never give feedback).
- Improve consistency in `ScalingMode`'s arguments
- General usability improvements
- Improve accuracy of terminology:
- "Window" should refer to the physical window on the desktop
- "Viewport" should refer to the component in the window that images are drawn on (typically all of it)
- "View frustum" should refer to the volume captured by the projection
---
## Changelog
### Added
- Added argument to `ScalingMode::WindowSize` that specifies the number of pixels that equals one world unit.
- Added documentation for fields and enums
### Changed
- Renamed `window_origin` to `viewport_origin`, which now:
- Affects all `ScalingMode`s
- Takes a fraction of the viewport's width and height instead of an enum
- Removed `WindowOrigin` enum as it's obsolete
- Renamed `ScalingMode::None` to `ScalingMode::Fixed`, which now:
- Takes arguments to specify the projection size
- Replaced `left`, `right`, `bottom`, and `top` fields with a single `area: Rect`
- `scale` is now applied before updating `area`. Reading from it will take `scale` into account.
- Documentation changes to make terminology more accurate and consistent
## Migration Guide
- Change `window_origin` to `viewport_origin`; replace `WindowOrigin::Center` with `Vec2::new(0.5, 0.5)` and `WindowOrigin::BottomLeft` with `Vec2::new(0.0, 0.0)`
- For shadow projections and such, replace `left`, `right`, `bottom`, and `top` with `area: Rect::new(left, bottom, right, top)`
- For camera projections, remove l/r/b/t values from `OrthographicProjection` instantiations, as they no longer have any effect in any `ScalingMode`
- Change `ScalingMode::None` to `ScalingMode::Fixed`
- Replace manual changes of l/r/b/t with:
- Arguments in `ScalingMode::Fixed` to specify size
- `viewport_origin` to specify offset
- Change `ScalingMode::WindowSize` to `ScalingMode::WindowSize(1.0)`
# Objective
Fix#7571
## Solution
* Removed the offending line.
***
## Changelog
* Removed
* * The line: ``\\ [`apply_system_buffers`]: bevy_ecs::prelude::apply_system_buffers`` from `bevy_app` crate, which overrides the link in that specific comment block.
Co-authored-by: lupan <kallll5@hotmail.com>
Small commit to remove an unused resource scoped within a single bevy_ecs unit test. Also rearranged the initialization to follow initialization conventions of surrounding tests. World/Schedule initialization followed by resource initialization.
This change was tested locally with `cargo test`, and `cargo fmt` was run.
Risk should be tiny as change is scoped to a single unit test and very tiny, and I can't see any way that this resource is being used in the test.
Thank you so much!
# Objective
One of the most important usages of plugin names is currently not mentioned in its documentation: the uniqueness check
## Solution
- Mention that the plugin name is used to check for uniqueness
# Objective
Run conditions are a special type of system that do not modify the world, and which return a bool. Due to the way they are currently implemented, you can *only* use bare function systems as a run condition. Among other things, this prevents the use of system piping with run conditions. This make very basic constructs impossible, such as `my_system.run_if(my_condition.pipe(not))`.
Unblocks a basic solution for #7202.
## Solution
Add the trait `ReadOnlySystem`, which is implemented for any system whose parameters all implement `ReadOnlySystemParam`. Allow any `-> bool` system implementing this trait to be used as a run condition.
---
## Changelog
+ Added the trait `ReadOnlySystem`, which is implemented for any `System` type whose parameters all implement `ReadOnlySystemParam`.
+ Added the function `bevy::ecs::system::assert_is_read_only_system`.