From f28707e74bd7225a5e69f5d1fd1bb11dec3a2dfa Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 27 Jan 2023 11:35:18 -0500 Subject: [PATCH 01/21] Basic internal_ambiguities example --- examples/README.md | 12 ++++++++---- examples/tools/internal_ambiguities.rs | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) create mode 100644 examples/tools/internal_ambiguities.rs diff --git a/examples/README.md b/examples/README.md index f202483fd5dbb..cf7d3afcc5ef0 100644 --- a/examples/README.md +++ b/examples/README.md @@ -56,19 +56,22 @@ git checkout v0.4.0 - [Transforms](#transforms) - [UI (User Interface)](#ui-user-interface) - [Window](#window) - - [Tests](#tests) - [Platform-Specific Examples](#platform-specific-examples) - [Android](#android) - [Setup](#setup) - - [Build & Run](#build--run) + - [Build \& Run](#build--run) + - [Debugging](#debugging) - [Old phones](#old-phones) - [iOS](#ios) - [Setup](#setup-1) - - [Build & Run](#build--run-1) + - [Build \& Run](#build--run-1) - [WASM](#wasm) - [Setup](#setup-2) - - [Build & Run](#build--run-2) + - [Build \& Run](#build--run-2) + - [Optimizing](#optimizing) + - [1. Tweak your `Cargo.toml`](#1-tweak-your-cargotoml) + - [2. Use `wasm-opt` from the binaryen package](#2-use-wasm-opt-from-the-binaryen-package) - [Loading Assets](#loading-assets) # The Bare Minimum @@ -299,6 +302,7 @@ Example | Description Example | Description --- | --- [Gamepad Viewer](../examples/tools/gamepad_viewer.rs) | Shows a visualization of gamepad buttons, sticks, and triggers +[Internal Ambiguities](../examples/tools/internal_ambiguities.rs) | Verify that `DefaultPlugins` works, and that the schedule is free of problematic execution order ambiguities. [Scene Viewer](../examples/tools/scene_viewer/main.rs) | A simple way to view glTF models with Bevy. Just run `cargo run --release --example scene_viewer /path/to/model.gltf#Scene0`, replacing the path as appropriate. With no arguments it will load the FieldHelmet glTF model from the repository assets subdirectory ## Transforms diff --git a/examples/tools/internal_ambiguities.rs b/examples/tools/internal_ambiguities.rs new file mode 100644 index 0000000000000..82fd12d820299 --- /dev/null +++ b/examples/tools/internal_ambiguities.rs @@ -0,0 +1,15 @@ +//! Checks that a schedule with all default plugins runs, and no internal system execution order ambiguities exist. +//! +//! Note that execution order ambiguities can and are deliberately ignored. +//! If any of these are causing issues to the deterministic execution of your game, please open an issue! +//! +//! This is mostly used for testing that Bevy works as expected, both on your device and on CI. +//! Consider it an advanced "hello world". You should see an empty window open. + +use bevy::prelude::*; + +fn main() { + app.add_plugins(DefaultPlugins) + .insert_resource(ReportExecutionOrderAmbiguities) + .run(); +} From 87b8c6bdcc3818dd817d7f66a23e532014861c2a Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 27 Jan 2023 13:20:42 -0500 Subject: [PATCH 02/21] Add example to Cargo.toml --- Cargo.toml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index cf1fafbeafabf..f6a41cdaaef54 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1425,6 +1425,16 @@ description = "Shows a visualization of gamepad buttons, sticks, and triggers" category = "Tools" wasm = false +[[example]] +name = "internal_ambiguities" +path = "examples/tools/internal_ambiguities.rs" + +[package.metadata.example.internal_ambiguities] +name = "Internal Ambiguities" +description = "Verify that `DefaultPlugins` works, and that the schedule is free of problematic execution order ambiguities." +category = "Tools" +wasm = false + [[example]] name = "3d_rotation" path = "examples/transforms/3d_rotation.rs" From 18dd504213044c0789f4ee42901394293e266625 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 27 Jan 2023 13:22:50 -0500 Subject: [PATCH 03/21] Ensure example runs --- examples/tools/internal_ambiguities.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/examples/tools/internal_ambiguities.rs b/examples/tools/internal_ambiguities.rs index 82fd12d820299..4a963051af8e2 100644 --- a/examples/tools/internal_ambiguities.rs +++ b/examples/tools/internal_ambiguities.rs @@ -6,10 +6,11 @@ //! This is mostly used for testing that Bevy works as expected, both on your device and on CI. //! Consider it an advanced "hello world". You should see an empty window open. -use bevy::prelude::*; +use bevy::{ecs::schedule::ReportExecutionOrderAmbiguities, prelude::*}; fn main() { - app.add_plugins(DefaultPlugins) + App::new() + .add_plugins(DefaultPlugins) .insert_resource(ReportExecutionOrderAmbiguities) .run(); } From 50fc12248153aca9a93afd8efed95161abf75e9f Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 27 Jan 2023 13:27:28 -0500 Subject: [PATCH 04/21] Ignore ambiguities in TransformPlugin --- crates/bevy_transform/src/lib.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/bevy_transform/src/lib.rs b/crates/bevy_transform/src/lib.rs index c68f01b2e0b5b..ddd5e961ee297 100644 --- a/crates/bevy_transform/src/lib.rs +++ b/crates/bevy_transform/src/lib.rs @@ -19,6 +19,7 @@ use bevy_app::prelude::*; use bevy_ecs::prelude::*; use bevy_hierarchy::ValidParentCheckPlugin; use prelude::{GlobalTransform, Transform}; +use systems::{propagate_transforms, sync_simple_transforms}; /// A [`Bundle`] of the [`Transform`] and [`GlobalTransform`] /// [`Component`](bevy_ecs::component::Component)s, which describe the position of an entity. @@ -101,19 +102,25 @@ impl Plugin for TransformPlugin { // add transform systems to startup so the first update is "correct" .add_startup_system_to_stage( StartupStage::PostStartup, - systems::sync_simple_transforms.label(TransformSystem::TransformPropagate), + sync_simple_transforms + .label(TransformSystem::TransformPropagate) + // These systems cannot access the same entities, + // due to subtle query filtering that is not yet correctly computed in the ambiguity detector + .ambiguous_with(propagate_transforms), ) .add_startup_system_to_stage( StartupStage::PostStartup, - systems::propagate_transforms.label(TransformSystem::TransformPropagate), + propagate_transforms.label(TransformSystem::TransformPropagate), ) .add_system_to_stage( CoreStage::PostUpdate, - systems::sync_simple_transforms.label(TransformSystem::TransformPropagate), + sync_simple_transforms + .label(TransformSystem::TransformPropagate) + .ambiguous_with(propagate_transforms), ) .add_system_to_stage( CoreStage::PostUpdate, - systems::propagate_transforms.label(TransformSystem::TransformPropagate), + propagate_transforms.label(TransformSystem::TransformPropagate), ); } } From dfaa0ff69be96479fa3bf3ea00fd736db1a85a2f Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 27 Jan 2023 13:31:21 -0500 Subject: [PATCH 05/21] Resolve ambiguities between gamepad systems --- crates/bevy_input/src/lib.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/bevy_input/src/lib.rs b/crates/bevy_input/src/lib.rs index fccc2027f56b4..46c7e51127adb 100644 --- a/crates/bevy_input/src/lib.rs +++ b/crates/bevy_input/src/lib.rs @@ -83,9 +83,17 @@ impl Plugin for InputPlugin { CoreStage::PreUpdate, SystemSet::new() .with_system(gamepad_event_system) - .with_system(gamepad_button_event_system.after(gamepad_event_system)) - .with_system(gamepad_axis_event_system.after(gamepad_event_system)) .with_system(gamepad_connection_system.after(gamepad_event_system)) + .with_system( + gamepad_button_event_system + .after(gamepad_event_system) + .after(gamepad_connection_system), + ) + .with_system( + gamepad_axis_event_system + .after(gamepad_event_system) + .after(gamepad_connection_system), + ) .label(InputSystem), ) // touch From 99fcd894dce7e33e11f6e1eb947cde2694b56a2d Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 27 Jan 2023 13:35:47 -0500 Subject: [PATCH 06/21] Resolved ambiguities with bevy_winit --- crates/bevy_winit/src/lib.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/bevy_winit/src/lib.rs b/crates/bevy_winit/src/lib.rs index 84303caaa1c99..5681f367d2a93 100644 --- a/crates/bevy_winit/src/lib.rs +++ b/crates/bevy_winit/src/lib.rs @@ -25,9 +25,10 @@ use bevy_utils::{ Instant, }; use bevy_window::{ - CursorEntered, CursorLeft, CursorMoved, FileDragAndDrop, ModifiesWindows, ReceivedCharacter, - RequestRedraw, Window, WindowBackendScaleFactorChanged, WindowCloseRequested, WindowCreated, - WindowFocused, WindowMoved, WindowResized, WindowScaleFactorChanged, + exit_on_all_closed, CursorEntered, CursorLeft, CursorMoved, FileDragAndDrop, ModifiesWindows, + ReceivedCharacter, RequestRedraw, Window, WindowBackendScaleFactorChanged, + WindowCloseRequested, WindowCreated, WindowFocused, WindowMoved, WindowResized, + WindowScaleFactorChanged, }; use winit::{ @@ -54,8 +55,8 @@ impl Plugin for WinitPlugin { CoreStage::PostUpdate, SystemSet::new() .label(ModifiesWindows) - .with_system(changed_window) - .with_system(despawn_window), + .with_system(changed_window.before(exit_on_all_closed)) + .with_system(despawn_window.after(changed_window)), ); #[cfg(target_arch = "wasm32")] From 521270f43f38e68cf671593048abfffb4938c410 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 27 Jan 2023 13:39:00 -0500 Subject: [PATCH 07/21] Resolve cascaded shadow map ambiguity --- crates/bevy_pbr/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/bevy_pbr/src/lib.rs b/crates/bevy_pbr/src/lib.rs index 1ee943b19ee97..3496fad8c302d 100644 --- a/crates/bevy_pbr/src/lib.rs +++ b/crates/bevy_pbr/src/lib.rs @@ -190,7 +190,8 @@ impl Plugin for PbrPlugin { CoreStage::PostUpdate, update_directional_light_cascades .label(SimulationLightSystems::UpdateDirectionalLightCascades) - .after(TransformSystem::TransformPropagate), + .after(TransformSystem::TransformPropagate) + .after(CameraUpdateSystem), ) .add_system_to_stage( CoreStage::PostUpdate, From 62e9dd8ba8acb4dab715c5488c17198dfb45b17e Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 27 Jan 2023 13:48:52 -0500 Subject: [PATCH 08/21] Update example README --- examples/README.md | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/examples/README.md b/examples/README.md index cf7d3afcc5ef0..6e7bb76460532 100644 --- a/examples/README.md +++ b/examples/README.md @@ -56,22 +56,19 @@ git checkout v0.4.0 - [Transforms](#transforms) - [UI (User Interface)](#ui-user-interface) - [Window](#window) + - [Tests](#tests) - [Platform-Specific Examples](#platform-specific-examples) - [Android](#android) - [Setup](#setup) - - [Build \& Run](#build--run) - - [Debugging](#debugging) + - [Build & Run](#build--run) - [Old phones](#old-phones) - [iOS](#ios) - [Setup](#setup-1) - - [Build \& Run](#build--run-1) + - [Build & Run](#build--run-1) - [WASM](#wasm) - [Setup](#setup-2) - - [Build \& Run](#build--run-2) - - [Optimizing](#optimizing) - - [1. Tweak your `Cargo.toml`](#1-tweak-your-cargotoml) - - [2. Use `wasm-opt` from the binaryen package](#2-use-wasm-opt-from-the-binaryen-package) + - [Build & Run](#build--run-2) - [Loading Assets](#loading-assets) # The Bare Minimum From 5f415e61457323da39f3a8b4c37146dbd39e773f Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 27 Jan 2023 14:21:02 -0500 Subject: [PATCH 09/21] Add FIXME --- crates/bevy_transform/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_transform/src/lib.rs b/crates/bevy_transform/src/lib.rs index ddd5e961ee297..4aab52830c0c5 100644 --- a/crates/bevy_transform/src/lib.rs +++ b/crates/bevy_transform/src/lib.rs @@ -104,6 +104,7 @@ impl Plugin for TransformPlugin { StartupStage::PostStartup, sync_simple_transforms .label(TransformSystem::TransformPropagate) + // FIXME: https://github.com/bevyengine/bevy/issues/4381 // These systems cannot access the same entities, // due to subtle query filtering that is not yet correctly computed in the ambiguity detector .ambiguous_with(propagate_transforms), From 1f72ad51b9d30cae31f105b22d4fbe5e017cbfe8 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 27 Jan 2023 15:20:41 -0500 Subject: [PATCH 10/21] Add rationale to bevy_winit system orderings --- crates/bevy_winit/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/bevy_winit/src/lib.rs b/crates/bevy_winit/src/lib.rs index 5681f367d2a93..096b35e89ae79 100644 --- a/crates/bevy_winit/src/lib.rs +++ b/crates/bevy_winit/src/lib.rs @@ -55,7 +55,9 @@ impl Plugin for WinitPlugin { CoreStage::PostUpdate, SystemSet::new() .label(ModifiesWindows) + // Update the state of the window before checking if they have been closed to avoid delays and spurious exits .with_system(changed_window.before(exit_on_all_closed)) + // Update the state of the window before attempting to despawn to avoid desynchronized state .with_system(despawn_window.after(changed_window)), ); From c330ec08a26afe6dcc774ce667c34faad2e30265 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 27 Jan 2023 16:33:50 -0500 Subject: [PATCH 11/21] Allow exit_on_all_closed ambiguity --- crates/bevy_winit/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/bevy_winit/src/lib.rs b/crates/bevy_winit/src/lib.rs index 096b35e89ae79..41713f70347cd 100644 --- a/crates/bevy_winit/src/lib.rs +++ b/crates/bevy_winit/src/lib.rs @@ -55,8 +55,9 @@ impl Plugin for WinitPlugin { CoreStage::PostUpdate, SystemSet::new() .label(ModifiesWindows) - // Update the state of the window before checking if they have been closed to avoid delays and spurious exits - .with_system(changed_window.before(exit_on_all_closed)) + // exit_on_all_closed only uses the query to determine if the query is empty, + // and so doesn't care about ordering relative to changed_window + .with_system(changed_window.ambiguous_with(exit_on_all_closed)) // Update the state of the window before attempting to despawn to avoid desynchronized state .with_system(despawn_window.after(changed_window)), ); From 8b0952721f730ebd5003021b7ca4daa952ab4874 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 27 Jan 2023 16:39:30 -0500 Subject: [PATCH 12/21] Guard against a tricky edge case window desync --- crates/bevy_winit/src/system.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/bevy_winit/src/system.rs b/crates/bevy_winit/src/system.rs index 2c1fe1875cf95..82164036635e8 100644 --- a/crates/bevy_winit/src/system.rs +++ b/crates/bevy_winit/src/system.rs @@ -87,14 +87,18 @@ pub struct WindowTitleCache(HashMap); pub(crate) fn despawn_window( closed: RemovedComponents, + window_entities: Query<&Window>, mut close_events: EventWriter, mut winit_windows: NonSendMut, ) { for window in closed.iter() { info!("Closing window {:?}", window); - - winit_windows.remove_window(window); - close_events.send(WindowClosed { window }); + // Guard to verify that the window is in fact actually gone, + // rather than having the component added and removed in the same frame. + if !window_entities.contains(window) { + winit_windows.remove_window(window); + close_events.send(WindowClosed { window }); + } } } From 0bf8d8f2a67dd63934e91dea06f1dd1be7d70e57 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 27 Jan 2023 16:40:46 -0500 Subject: [PATCH 13/21] Change rationale for resolving winit ambiguities --- crates/bevy_winit/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_winit/src/lib.rs b/crates/bevy_winit/src/lib.rs index 41713f70347cd..260c6c8345e04 100644 --- a/crates/bevy_winit/src/lib.rs +++ b/crates/bevy_winit/src/lib.rs @@ -58,7 +58,7 @@ impl Plugin for WinitPlugin { // exit_on_all_closed only uses the query to determine if the query is empty, // and so doesn't care about ordering relative to changed_window .with_system(changed_window.ambiguous_with(exit_on_all_closed)) - // Update the state of the window before attempting to despawn to avoid desynchronized state + // Update the state of the window before attempting to despawn to ensure consistent event ordering .with_system(despawn_window.after(changed_window)), ); From c5548d11c0c2bfa84b922ce935973374ce2917f7 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 29 Jan 2023 15:07:40 -0500 Subject: [PATCH 14/21] Swap example to a lesson on ambiguities --- Cargo.toml | 12 +-- examples/ecs/nondeterministic_system_order.rs | 83 +++++++++++++++++++ examples/tools/internal_ambiguities.rs | 16 ---- 3 files changed, 89 insertions(+), 22 deletions(-) create mode 100644 examples/ecs/nondeterministic_system_order.rs delete mode 100644 examples/tools/internal_ambiguities.rs diff --git a/Cargo.toml b/Cargo.toml index f6a41cdaaef54..5ac394543fcb2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1426,13 +1426,13 @@ category = "Tools" wasm = false [[example]] -name = "internal_ambiguities" -path = "examples/tools/internal_ambiguities.rs" +name = "nondeterministic_system_order" +path = "examples/ecs/nondeterministic_system_order.rs" -[package.metadata.example.internal_ambiguities] -name = "Internal Ambiguities" -description = "Verify that `DefaultPlugins` works, and that the schedule is free of problematic execution order ambiguities." -category = "Tools" +[package.metadata.example.nondeterministic_system_order] +name = "Nondeterministic System Order" +description = "Systems run in paralell, but their order isn't always deteriministic. Here's how to detect and fix this." +category = "ECS" wasm = false [[example]] diff --git a/examples/ecs/nondeterministic_system_order.rs b/examples/ecs/nondeterministic_system_order.rs new file mode 100644 index 0000000000000..4a366b6af588b --- /dev/null +++ b/examples/ecs/nondeterministic_system_order.rs @@ -0,0 +1,83 @@ +//! 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, +//! than 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::() + .init_resource::() + // 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 trio of systems has conflicting data access, + // but it's resolved with an explicit ordering + .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. + .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) + // We're only going to run one frame of this app, + // to make sure we can see the warnings at the start. + .update(); +} + +#[derive(Resource, Debug, Default)] +struct A(usize); + +#[derive(Resource, Debug, Default)] +struct B(usize); + +// Data access is determined solely +fn reads_a(a: Res) { + dbg!(a); +} + +fn writes_a(mut a: ResMut) { + a.0 += 1; +} + +fn adds_one_to_b(mut b: ResMut) { + b.0 = b.0.saturating_add(1); +} + +fn doubles_b(mut b: ResMut) { + // This will overflow pretty rapidly otherwise + b.0 = b.0.saturating_mul(2); +} + +fn reads_b(b: Res) { + // 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)); + dbg!(b); +} + +fn reads_a_and_b(a: Res, b: Res) { + dbg!(a, b); +} diff --git a/examples/tools/internal_ambiguities.rs b/examples/tools/internal_ambiguities.rs deleted file mode 100644 index 4a963051af8e2..0000000000000 --- a/examples/tools/internal_ambiguities.rs +++ /dev/null @@ -1,16 +0,0 @@ -//! Checks that a schedule with all default plugins runs, and no internal system execution order ambiguities exist. -//! -//! Note that execution order ambiguities can and are deliberately ignored. -//! If any of these are causing issues to the deterministic execution of your game, please open an issue! -//! -//! This is mostly used for testing that Bevy works as expected, both on your device and on CI. -//! Consider it an advanced "hello world". You should see an empty window open. - -use bevy::{ecs::schedule::ReportExecutionOrderAmbiguities, prelude::*}; - -fn main() { - App::new() - .add_plugins(DefaultPlugins) - .insert_resource(ReportExecutionOrderAmbiguities) - .run(); -} From 156e149fccb2c813cb6fa19c9a3c18aa5f702d98 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 29 Jan 2023 15:38:42 -0500 Subject: [PATCH 15/21] Update README --- Cargo.toml | 2 +- examples/README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5ac394543fcb2..36ddddf5fca23 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1432,7 +1432,7 @@ path = "examples/ecs/nondeterministic_system_order.rs" [package.metadata.example.nondeterministic_system_order] name = "Nondeterministic System Order" description = "Systems run in paralell, but their order isn't always deteriministic. Here's how to detect and fix this." -category = "ECS" +category = "ECS (Entity Component System)" wasm = false [[example]] diff --git a/examples/README.md b/examples/README.md index 6e7bb76460532..1200fca870d39 100644 --- a/examples/README.md +++ b/examples/README.md @@ -200,6 +200,7 @@ Example | Description [Generic System](../examples/ecs/generic_system.rs) | Shows how to create systems that can be reused with different types [Hierarchy](../examples/ecs/hierarchy.rs) | Creates a hierarchy of parents and children entities [Iter Combinations](../examples/ecs/iter_combinations.rs) | Shows how to iterate over combinations of query results +[Nondeterministic System Order](../examples/ecs/nondeterministic_system_order.rs) | Systems run in paralell, but their order isn't always deteriministic. Here's how to detect and fix this. [Parallel Query](../examples/ecs/parallel_query.rs) | Illustrates parallel queries with `ParallelIterator` [Removal Detection](../examples/ecs/removal_detection.rs) | Query for entities that had a specific component removed in a previous stage during the current frame [Startup System](../examples/ecs/startup_system.rs) | Demonstrates a startup system (one that runs once when the app starts up) @@ -299,7 +300,6 @@ Example | Description Example | Description --- | --- [Gamepad Viewer](../examples/tools/gamepad_viewer.rs) | Shows a visualization of gamepad buttons, sticks, and triggers -[Internal Ambiguities](../examples/tools/internal_ambiguities.rs) | Verify that `DefaultPlugins` works, and that the schedule is free of problematic execution order ambiguities. [Scene Viewer](../examples/tools/scene_viewer/main.rs) | A simple way to view glTF models with Bevy. Just run `cargo run --release --example scene_viewer /path/to/model.gltf#Scene0`, replacing the path as appropriate. With no arguments it will load the FieldHelmet glTF model from the repository assets subdirectory ## Transforms From df94eb642dccddae40e208d148c1159162ee56c3 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 29 Jan 2023 17:34:52 -0500 Subject: [PATCH 16/21] Shortcircuiting OR Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com> --- examples/ecs/nondeterministic_system_order.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/ecs/nondeterministic_system_order.rs b/examples/ecs/nondeterministic_system_order.rs index 4a366b6af588b..9ea8a64900fbb 100644 --- a/examples/ecs/nondeterministic_system_order.rs +++ b/examples/ecs/nondeterministic_system_order.rs @@ -74,7 +74,7 @@ fn doubles_b(mut b: ResMut) { fn reads_b(b: Res) { // 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)); + assert!((b.0 % 2 == 0) || (b.0 == usize::MAX)); dbg!(b); } From ae2419b153662b2be6868e64824c2a48394225b0 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 29 Jan 2023 17:37:13 -0500 Subject: [PATCH 17/21] Complete comment --- examples/ecs/nondeterministic_system_order.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/examples/ecs/nondeterministic_system_order.rs b/examples/ecs/nondeterministic_system_order.rs index 9ea8a64900fbb..7b87cf6a06d0e 100644 --- a/examples/ecs/nondeterministic_system_order.rs +++ b/examples/ecs/nondeterministic_system_order.rs @@ -53,7 +53,9 @@ struct A(usize); #[derive(Resource, Debug, Default)] struct B(usize); -// Data access is determined solely +// 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) { dbg!(a); } From c5f6e3bcec6c5b69f7249cb08702765e9c550309 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 29 Jan 2023 17:41:38 -0500 Subject: [PATCH 18/21] Early exit + cleaner debug output --- examples/ecs/nondeterministic_system_order.rs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/examples/ecs/nondeterministic_system_order.rs b/examples/ecs/nondeterministic_system_order.rs index 7b87cf6a06d0e..c47d14676d001 100644 --- a/examples/ecs/nondeterministic_system_order.rs +++ b/examples/ecs/nondeterministic_system_order.rs @@ -12,7 +12,7 @@ //! //! This example demonstrates how you might detect and resolve (or silence) these ambiguities. -use bevy::{ecs::schedule::ReportExecutionOrderAmbiguities, prelude::*}; +use bevy::{app::AppExit, ecs::schedule::ReportExecutionOrderAmbiguities, prelude::*}; fn main() { App::new() @@ -42,9 +42,7 @@ fn main() { // please file a bug with the repo responsible! // Only *you* can prevent nondeterministic bugs due to greedy parallelism. .add_plugins(DefaultPlugins) - // We're only going to run one frame of this app, - // to make sure we can see the warnings at the start. - .update(); + .run(); } #[derive(Resource, Debug, Default)] @@ -56,9 +54,7 @@ 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) { - dbg!(a); -} +fn reads_a(_a: Res) {} fn writes_a(mut a: ResMut) { a.0 += 1; @@ -77,9 +73,13 @@ fn reads_b(b: Res) { // 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)); - dbg!(b); } -fn reads_a_and_b(a: Res, b: Res) { - dbg!(a, b); +fn reads_a_and_b(a: Res, b: Res, mut app_exit: EventWriter) { + dbg!(a.0, b.0); + + // Break early to make sure the debug output is easily seen + if b.0 > 10 { + app_exit.send_default(); + } } From d0aed2b8e2fa72b1b16c6dbb9357b2b7744fc456 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 29 Jan 2023 17:44:09 -0500 Subject: [PATCH 19/21] Format merge conflict T_T --- crates/bevy_winit/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_winit/src/lib.rs b/crates/bevy_winit/src/lib.rs index 7fdcfde877387..544db1b6f268c 100644 --- a/crates/bevy_winit/src/lib.rs +++ b/crates/bevy_winit/src/lib.rs @@ -25,8 +25,8 @@ use bevy_utils::{ Instant, }; use bevy_window::{ - exit_on_all_closed, CursorEntered, CursorLeft, CursorMoved, FileDragAndDrop, Ime, ModifiesWindows, - ReceivedCharacter, RequestRedraw, Window, WindowBackendScaleFactorChanged, + exit_on_all_closed, CursorEntered, CursorLeft, CursorMoved, FileDragAndDrop, Ime, + ModifiesWindows, ReceivedCharacter, RequestRedraw, Window, WindowBackendScaleFactorChanged, WindowCloseRequested, WindowCreated, WindowFocused, WindowMoved, WindowResized, WindowScaleFactorChanged, }; From f2c76703cd4485a8d65d0abf6bcc0e2ee7b397b9 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 30 Jan 2023 03:45:18 -0500 Subject: [PATCH 20/21] Typo fix Co-authored-by: Austreelis --- examples/ecs/nondeterministic_system_order.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/ecs/nondeterministic_system_order.rs b/examples/ecs/nondeterministic_system_order.rs index c47d14676d001..fa2f9ada296c2 100644 --- a/examples/ecs/nondeterministic_system_order.rs +++ b/examples/ecs/nondeterministic_system_order.rs @@ -7,7 +7,7 @@ //! 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, -//! than the actual observed value will vary based on the nondeterministic order of evaluation. +//! 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. From bfe199b377b044882f4d2a8e4fbd66099bc29689 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 30 Jan 2023 19:48:11 -0500 Subject: [PATCH 21/21] Improve example to address review feedback --- examples/ecs/nondeterministic_system_order.rs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/examples/ecs/nondeterministic_system_order.rs b/examples/ecs/nondeterministic_system_order.rs index c47d14676d001..d5099e12c99ee 100644 --- a/examples/ecs/nondeterministic_system_order.rs +++ b/examples/ecs/nondeterministic_system_order.rs @@ -12,7 +12,7 @@ //! //! This example demonstrates how you might detect and resolve (or silence) these ambiguities. -use bevy::{app::AppExit, ecs::schedule::ReportExecutionOrderAmbiguities, prelude::*}; +use bevy::{ecs::schedule::ReportExecutionOrderAmbiguities, prelude::*}; fn main() { App::new() @@ -24,12 +24,14 @@ fn main() { // as their data access conflicts, and there's no order between them. .add_system(reads_a) .add_system(writes_a) - // This trio of systems has conflicting data access, - // but it's resolved with an explicit ordering + // 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. + // 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. @@ -75,11 +77,9 @@ fn reads_b(b: Res) { assert!((b.0 % 2 == 0) || (b.0 == usize::MAX)); } -fn reads_a_and_b(a: Res, b: Res, mut app_exit: EventWriter) { - dbg!(a.0, b.0); - - // Break early to make sure the debug output is easily seen - if b.0 > 10 { - app_exit.send_default(); +fn reads_a_and_b(a: Res, b: Res) { + // Only display the first few steps to avoid burying the ambiguities in the console + if b.0 < 10 { + info!("{}, {}", a.0, b.0); } }