From 1078761a2a204d9da11bcd87df304674ca53d73d Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 10 Feb 2021 18:35:23 -0500 Subject: [PATCH 1/7] More in-depth ambiguity checker docs --- crates/bevy_ecs/src/schedule/stage.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index 1bcee3a201b75..e1367705be3a7 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -21,11 +21,21 @@ pub trait Stage: Downcast + Send + Sync { impl_downcast!(Stage); -/// When this resource is present in `Resources`, `SystemStage` will log a report containing -/// pairs of systems with ambiguous execution order - i.e., those systems might induce different -/// results depending on the order they're executed in, yet don't have an explicit execution order -/// constraint between them. -/// This is not necessarily a bad thing - you have to make that judgement yourself. +/// When this resource is present in the `AppBuilder`'s `Resources`, +/// each `SystemStage` will log a report containing +/// pairs of systems with ambiguous execution order. +/// +/// Systems that access the same Component or Resource within the same stage +/// risk an ambiguous order that could result in logic bugs, as, unless they have an +/// explicit execution ordering constraint between them. +/// +/// This occurs because, in the absence of explicit constraints, systems are executed in +/// an unstable, arbitrary order within each stage that may vary between runs and frames. +/// +/// Some ambiguities reported by the ambiguity checker may be warranted (to allow two systems to run without blocking each other) +/// or spurious, as the exact combination of archetypes used may prevent them from ever conflicting during actual gameplay. +/// You can resolve the warnings produced by the ambiguity checker by adding `.before` or `.after` to one of the conflicting systems +/// referencing the other system to force a specific ordering. pub struct ReportExecutionOrderAmbiguities; struct VirtualSystemSet { From b0b982180c9daf4f1296b9f1ba93466bd68f1203 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 10 Feb 2021 18:41:50 -0500 Subject: [PATCH 2/7] Explicitly note methods to specify ordering --- crates/bevy_ecs/src/schedule/system_descriptor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/schedule/system_descriptor.rs b/crates/bevy_ecs/src/schedule/system_descriptor.rs index 60d65508e8d78..dd368f9f453fe 100644 --- a/crates/bevy_ecs/src/schedule/system_descriptor.rs +++ b/crates/bevy_ecs/src/schedule/system_descriptor.rs @@ -12,7 +12,7 @@ use std::borrow::Cow; /// been applied. /// /// All systems can have a label attached to them; other systems in the same group can then specify -/// that they have to run before or after the system with that label. +/// that they have to run before or after the system with that label using the `before` and `after` methods. /// /// # Example /// ```rust From 4db8d75370d05a604661b0cd415eee99c5e22cec Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 10 Feb 2021 19:02:14 -0500 Subject: [PATCH 3/7] Updated ecs_guide example with explicit dependencies --- examples/ecs/ecs_guide.rs | 43 ++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/examples/ecs/ecs_guide.rs b/examples/ecs/ecs_guide.rs index 4697b994d7702..167080b10a66d 100644 --- a/examples/ecs/ecs_guide.rs +++ b/examples/ecs/ecs_guide.rs @@ -261,19 +261,25 @@ fn main() { // // SYSTEM EXECUTION ORDER // - // By default, all systems run in parallel. This is efficient, but sometimes order matters. + // Each system belongs to a `Stage`, which controls the execution strategy and broad order of the systems within each tick. + // Startup stages (which startup systems are registered in) will always complete before ordinary stages begin, + // and every system in a stage must complete before the next stage advances. + // Once every stage has concluded, the main loop is complete and begins again. + // + // By default, all systems run in parallel, except when they require mutable access to a piece of data. + // This is efficient, but sometimes order matters. // For example, we want our "game over" system to execute after all other systems to ensure we don't // accidentally run the game for an extra round. // - // First, if a system writes a component or resource (ComMut / ResMut), it will force a synchronization. - // Any systems that access the data type and were registered BEFORE the system will need to finish first. - // Any systems that were registered _after_ the system will need to wait for it to finish. This is a great - // default that makes everything "just work" as fast as possible without us needing to think about it ... provided - // we don't care about execution order. If we do care, one option would be to use the rules above to force a synchronization - // at the right time. But that is complicated and error prone! + // Rather than splitting each of your systems into separate stages, you should force an explicit ordering between them + // by giving the relevant systems a label with `.label`, then using the `.before` or `.after` methods. + // Systems will not be scheduled until all of the systems that they have an "ordering dependency" on have completed. + // + // Doing so will help make sure your systems can run safely and quickly in parallel, as changing `Stages` require a "hard sync", + // where every system is concluded in order to give exclusive access to both `World` and `Resources`. + // While this is typically a drawback, it is necessary to allow `Commands` to be processed, allowing you to + // perform heavy operations such as adding or removing entities or components. // - // This is where "stages" come in. A "stage" is a group of systems that execute (in parallel). Stages are executed in order, - // and the next stage won't start until all systems in the current stage have finished. // add_system(system) adds systems to the UPDATE stage by default // However we can manually specify the stage if we want to. The following is equivalent to add_system(score_system) .add_system_to_stage(stage::UPDATE, score_system.system()) @@ -285,11 +291,20 @@ fn main() { .add_stage_after(stage::UPDATE, "after_round", SystemStage::parallel()) .add_system_to_stage("before_round", new_round_system.system()) .add_system_to_stage("before_round", new_player_system.system()) - .add_system_to_stage("after_round", score_check_system.system()) - .add_system_to_stage("after_round", game_over_system.system()) - // score_check_system will run before game_over_system because score_check_system modifies GameState and game_over_system - // reads GameState. This works, but it's a bit confusing. In practice, it would be clearer to create a new stage that runs - // before "after_round" + // We can ensure that game_over system runs after score_check_system using explicit ordering constraints + // First, we label the system we want to refer to using `.label` + // Then, we use either `.before` or `.after` to describe the order we want the relationship + .add_system_to_stage( + "after_round", + score_check_system.system().label("score_check"), + ) + .add_system_to_stage( + "after_round", + game_over_system.system().after("score_check"), + ) + // We can check our systems for execution order ambiguities by examining the output produced in the console + // by adding the following Resource to our App :) + .insert_resource(ReportExecutionOrderAmbiguities {}) // This call to run() starts the app we just built! .run(); } From 19859ea605503e25d4eec0a44fc0909e02c8a0e4 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 11 Feb 2021 12:52:15 -0500 Subject: [PATCH 4/7] Update examples/ecs/ecs_guide.rs Co-authored-by: Alexander Sepity --- examples/ecs/ecs_guide.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/examples/ecs/ecs_guide.rs b/examples/ecs/ecs_guide.rs index 167080b10a66d..2b546bb080a54 100644 --- a/examples/ecs/ecs_guide.rs +++ b/examples/ecs/ecs_guide.rs @@ -275,10 +275,14 @@ fn main() { // by giving the relevant systems a label with `.label`, then using the `.before` or `.after` methods. // Systems will not be scheduled until all of the systems that they have an "ordering dependency" on have completed. // - // Doing so will help make sure your systems can run safely and quickly in parallel, as changing `Stages` require a "hard sync", - // where every system is concluded in order to give exclusive access to both `World` and `Resources`. - // While this is typically a drawback, it is necessary to allow `Commands` to be processed, allowing you to - // perform heavy operations such as adding or removing entities or components. + // Doing that will, in just about all cases, lead to better performance compared to + // splitting systems between stages, because it gives the scheduling algorithm more + // opportunities to run systems in parallel. + // Stages are still necessary, however: end of a stage is a hard sync point + // (meaning, no systems are running) where `Commands` issued by systems are processed. + // This is required because commands can perform operations that are incompatible with + // having systems in flight, such as spawning or deleting entities, + // adding or removing resources, etc. // // add_system(system) adds systems to the UPDATE stage by default // However we can manually specify the stage if we want to. The following is equivalent to add_system(score_system) From 0f574978ebac54d6799ab6ec2597406af2059601 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 11 Feb 2021 12:52:24 -0500 Subject: [PATCH 5/7] Update crates/bevy_ecs/src/schedule/stage.rs Co-authored-by: Alexander Sepity --- crates/bevy_ecs/src/schedule/stage.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index e1367705be3a7..adfaf04cea2c1 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -36,6 +36,9 @@ impl_downcast!(Stage); /// or spurious, as the exact combination of archetypes used may prevent them from ever conflicting during actual gameplay. /// You can resolve the warnings produced by the ambiguity checker by adding `.before` or `.after` to one of the conflicting systems /// referencing the other system to force a specific ordering. +/// +/// The checker may report a system more times than the amount of constraints it would actually need to have +/// unambiguous order with regards to a group of already-constrained systems. pub struct ReportExecutionOrderAmbiguities; struct VirtualSystemSet { From 246221818300bbf8169c20e61c6c9c476629d122 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 11 Feb 2021 12:53:24 -0500 Subject: [PATCH 6/7] Update examples/ecs/ecs_guide.rs Co-authored-by: Alexander Sepity --- examples/ecs/ecs_guide.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/examples/ecs/ecs_guide.rs b/examples/ecs/ecs_guide.rs index 2b546bb080a54..677cc0f9b75f0 100644 --- a/examples/ecs/ecs_guide.rs +++ b/examples/ecs/ecs_guide.rs @@ -308,7 +308,9 @@ fn main() { ) // We can check our systems for execution order ambiguities by examining the output produced in the console // by adding the following Resource to our App :) - .insert_resource(ReportExecutionOrderAmbiguities {}) + // Be aware that not everything reported by this checker is a potential problem, you'll have to make + // that judgement yourself. + .insert_resource(ReportExecutionOrderAmbiguities) // This call to run() starts the app we just built! .run(); } From e5bd671123e7c74bc66fc8ec954a1baf0364efe9 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 11 Feb 2021 12:56:03 -0500 Subject: [PATCH 7/7] Typo correction --- crates/bevy_ecs/src/schedule/stage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index adfaf04cea2c1..f22a30c0dafd8 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -26,7 +26,7 @@ impl_downcast!(Stage); /// pairs of systems with ambiguous execution order. /// /// Systems that access the same Component or Resource within the same stage -/// risk an ambiguous order that could result in logic bugs, as, unless they have an +/// risk an ambiguous order that could result in logic bugs, unless they have an /// explicit execution ordering constraint between them. /// /// This occurs because, in the absence of explicit constraints, systems are executed in