-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Reduce internal system order ambiguities, and add an example explaini…
…ng them (#7383) # Objective - Bevy should not have any "internal" execution order ambiguities. These clutter the output of user-facing error reporting, and can result in nasty, nondetermistic, very difficult to solve bugs. - Verifying this currently involves repeated non-trivial manual work. ## Solution - [x] add an example to quickly check this - ~~[ ] ensure that this example panics if there are any unresolved ambiguities~~ - ~~[ ] run the example in CI 😈~~ There's one tricky ambiguity left, between UI and animation. I don't have the tools to fix this without system set configuration, so the remaining work is going to be left to #7267 or another PR after that. ``` 2023-01-27T18:38:42.989405Z INFO bevy_ecs::schedule::ambiguity_detection: Execution order ambiguities detected, you might want to add an explicit dependency relation between some of these systems: * Parallel systems: -- "bevy_animation::animation_player" and "bevy_ui::flex::flex_node_system" conflicts: ["bevy_transform::components::transform::Transform"] ``` ## Changelog Resolved internal execution order ambiguities for: 1. Transform propagation (ignored, we need smarter filter checking). 2. Gamepad processing (fixed). 3. bevy_winit's window handling (fixed). 4. Cascaded shadow maps and perspectives (fixed). Also fixed a desynchronized state bug that could occur when the `Window` component is removed and then added to the same entity in a single frame.
- Loading branch information
1 parent
cfc56cc
commit 5d514fb
Showing
8 changed files
with
134 additions
and
14 deletions.
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
//! By default, Bevy systems run in parallel with each other. | ||
//! Unless the order is explicitly specified, their relative order is nondeterministic. | ||
//! | ||
//! In many cases, this doesn't matter and is in fact desirable! | ||
//! Consider two systems, one which writes to resource A, and the other which writes to resource B. | ||
//! By allowing their order to be arbitrary, we can evaluate them greedily, based on the data that is free. | ||
//! Because their data accesses are **compatible**, there is no **observable** difference created based on the order they are run. | ||
//! | ||
//! But if instead we have two systems mutating the same data, or one reading it and the other mutating, | ||
//! then the actual observed value will vary based on the nondeterministic order of evaluation. | ||
//! These observable conflicts are called **system execution order ambiguities**. | ||
//! | ||
//! This example demonstrates how you might detect and resolve (or silence) these ambiguities. | ||
use bevy::{ecs::schedule::ReportExecutionOrderAmbiguities, prelude::*}; | ||
|
||
fn main() { | ||
App::new() | ||
// This resource controls the reporting strategy for system execution order ambiguities | ||
.insert_resource(ReportExecutionOrderAmbiguities) | ||
.init_resource::<A>() | ||
.init_resource::<B>() | ||
// This pair of systems has an ambiguous order, | ||
// as their data access conflicts, and there's no order between them. | ||
.add_system(reads_a) | ||
.add_system(writes_a) | ||
// This pair of systems has conflicting data access, | ||
// but it's resolved with an explicit ordering: | ||
// the .after relationship here means that we will always double after adding. | ||
.add_system(adds_one_to_b) | ||
.add_system(doubles_b.after(adds_one_to_b)) | ||
// This system isn't ambiguous with adds_one_to_b, | ||
// due to the transitive ordering created by our constraints: | ||
// if A is before B is before C, then A must be before C as well. | ||
.add_system(reads_b.after(doubles_b)) | ||
// This system will conflict with all of our writing systems | ||
// but we've silenced its ambiguity with adds_one_to_b. | ||
// This should only be done in the case of clear false positives: | ||
// leave a comment in your code justifying the decision! | ||
.add_system(reads_a_and_b.ambiguous_with(adds_one_to_b)) | ||
// Be mindful, internal ambiguities are reported too! | ||
// If there are any ambiguities due solely to DefaultPlugins, | ||
// or between DefaultPlugins and any of your third party plugins, | ||
// please file a bug with the repo responsible! | ||
// Only *you* can prevent nondeterministic bugs due to greedy parallelism. | ||
.add_plugins(DefaultPlugins) | ||
.run(); | ||
} | ||
|
||
#[derive(Resource, Debug, Default)] | ||
struct A(usize); | ||
|
||
#[derive(Resource, Debug, Default)] | ||
struct B(usize); | ||
|
||
// Data access is determined solely on the basis of the types of the system's parameters | ||
// Every implementation of the `SystemParam` and `WorldQuery` traits must declare which data is used | ||
// and whether or not it is mutably accessed. | ||
fn reads_a(_a: Res<A>) {} | ||
|
||
fn writes_a(mut a: ResMut<A>) { | ||
a.0 += 1; | ||
} | ||
|
||
fn adds_one_to_b(mut b: ResMut<B>) { | ||
b.0 = b.0.saturating_add(1); | ||
} | ||
|
||
fn doubles_b(mut b: ResMut<B>) { | ||
// This will overflow pretty rapidly otherwise | ||
b.0 = b.0.saturating_mul(2); | ||
} | ||
|
||
fn reads_b(b: Res<B>) { | ||
// This invariant is always true, | ||
// because we've fixed the system order so doubling always occurs after adding. | ||
assert!((b.0 % 2 == 0) || (b.0 == usize::MAX)); | ||
} | ||
|
||
fn reads_a_and_b(a: Res<A>, b: Res<B>) { | ||
// Only display the first few steps to avoid burying the ambiguities in the console | ||
if b.0 < 10 { | ||
info!("{}, {}", a.0, b.0); | ||
} | ||
} |