From a173abb98bebeab0b192e63658a87a1f157ea3f1 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Mon, 3 Oct 2022 16:07:25 -0400 Subject: [PATCH 1/4] Add methods for silencing ambiguities based on `SystemLabel`s --- .../src/schedule/ambiguity_detection.rs | 25 +++++++- .../bevy_ecs/src/schedule/system_container.rs | 6 +- .../src/schedule/system_descriptor.rs | 57 +++++++++++++++++++ 3 files changed, 85 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index ce4a3360dec06..4fc9dac55280d 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -2,9 +2,11 @@ use bevy_utils::tracing::info; use fixedbitset::FixedBitSet; use crate::component::ComponentId; -use crate::schedule::{SystemContainer, SystemStage}; +use crate::schedule::{AmbiguityDetection, GraphNode, SystemContainer, SystemStage}; use crate::world::World; +use super::SystemLabelId; + #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] struct SystemOrderAmbiguity { segment: SystemStageSegment, @@ -194,6 +196,24 @@ impl SystemStage { /// along with specific components that have triggered the warning. /// Systems must be topologically sorted beforehand. fn find_ambiguities(systems: &[SystemContainer]) -> Vec<(usize, usize, Vec)> { + // Check if we should ignore ambiguities between `system_a` and `system_b`. + fn should_ignore(system_a: &SystemContainer, system_b: &SystemContainer) -> bool { + fn should_ignore_inner( + system_a_detection: &AmbiguityDetection, + system_b_labels: &[SystemLabelId], + ) -> bool { + match system_a_detection { + AmbiguityDetection::Check => false, + AmbiguityDetection::IgnoreAll => true, + AmbiguityDetection::IgnoreWithLabel(labels) => { + labels.iter().any(|l| system_b_labels.contains(l)) + } + } + } + should_ignore_inner(&system_a.ambiguity_detection, system_b.labels()) + || should_ignore_inner(&system_b.ambiguity_detection, system_a.labels()) + } + let mut all_dependencies = Vec::::with_capacity(systems.len()); let mut all_dependants = Vec::::with_capacity(systems.len()); for (index, container) in systems.iter().enumerate() { @@ -235,7 +255,8 @@ fn find_ambiguities(systems: &[SystemContainer]) -> Vec<(usize, usize, Vec, before: Vec, after: Vec, + pub(crate) ambiguity_detection: AmbiguityDetection, } impl SystemContainer { @@ -29,6 +32,7 @@ impl SystemContainer { labels: descriptor.labels, before: descriptor.before, after: descriptor.after, + ambiguity_detection: descriptor.ambiguity_detection, is_exclusive: descriptor.exclusive_insertion_point.is_some(), } } diff --git a/crates/bevy_ecs/src/schedule/system_descriptor.rs b/crates/bevy_ecs/src/schedule/system_descriptor.rs index e673f12cc58ab..8d0d87b9d9a47 100644 --- a/crates/bevy_ecs/src/schedule/system_descriptor.rs +++ b/crates/bevy_ecs/src/schedule/system_descriptor.rs @@ -3,6 +3,16 @@ use crate::{ system::{AsSystemLabel, BoxedSystem, IntoSystem}, }; +/// Configures ambiguity detection for a single system. +#[derive(Default)] +pub(crate) enum AmbiguityDetection { + #[default] + Check, + IgnoreAll, + /// Ignore systems with any of these labels. + IgnoreWithLabel(Vec), +} + /// Encapsulates a system and information on when it run in a `SystemStage`. /// /// Systems can be inserted into 4 different groups within the stage: @@ -38,6 +48,7 @@ pub struct SystemDescriptor { pub(crate) labels: Vec, pub(crate) before: Vec, pub(crate) after: Vec, + pub(crate) ambiguity_detection: AmbiguityDetection, } impl SystemDescriptor { @@ -53,6 +64,7 @@ impl SystemDescriptor { run_criteria: None, before: Vec::new(), after: Vec::new(), + ambiguity_detection: Default::default(), } } } @@ -75,6 +87,15 @@ pub trait IntoSystemDescriptor { /// Specifies that the system should run after systems with the given label. fn after(self, label: impl AsSystemLabel) -> SystemDescriptor; + /// Marks this system as ambiguous with any system with the specified label. + /// This means that execution order between these systems does not matter, + /// which allows [some warnings](crate::schedule::ReportExecutionOrderAmbiguities) to be silenced. + fn ambiguous_with(self, label: impl AsSystemLabel) -> SystemDescriptor; + + /// Specifies that this system should opt out of + /// [execution order ambiguity detection](crate::schedule::ReportExecutionOrderAmbiguities). + fn ignore_all_ambiguities(self) -> SystemDescriptor; + /// Specifies that the system should run with other exclusive systems at the start of stage. fn at_start(self) -> SystemDescriptor; @@ -110,6 +131,26 @@ impl IntoSystemDescriptor<()> for SystemDescriptor { self } + fn ambiguous_with(mut self, label: impl AsSystemLabel) -> SystemDescriptor { + match &mut self.ambiguity_detection { + detection @ AmbiguityDetection::Check => { + *detection = + AmbiguityDetection::IgnoreWithLabel(vec![label.as_system_label().as_label()]); + } + AmbiguityDetection::IgnoreWithLabel(labels) => { + labels.push(label.as_system_label().as_label()); + } + // This descriptor is already ambiguous with everything. + AmbiguityDetection::IgnoreAll => {} + } + self + } + + fn ignore_all_ambiguities(mut self) -> SystemDescriptor { + self.ambiguity_detection = AmbiguityDetection::IgnoreAll; + self + } + fn at_start(mut self) -> SystemDescriptor { self.exclusive_insertion_point = Some(ExclusiveInsertionPoint::AtStart); self @@ -154,6 +195,14 @@ where SystemDescriptor::new(Box::new(IntoSystem::into_system(self))).after(label) } + fn ambiguous_with(self, label: impl AsSystemLabel) -> SystemDescriptor { + SystemDescriptor::new(Box::new(IntoSystem::into_system(self))).ambiguous_with(label) + } + + fn ignore_all_ambiguities(self) -> SystemDescriptor { + SystemDescriptor::new(Box::new(IntoSystem::into_system(self))).ignore_all_ambiguities() + } + fn at_start(self) -> SystemDescriptor { SystemDescriptor::new(Box::new(IntoSystem::into_system(self))).at_start() } @@ -191,6 +240,14 @@ impl IntoSystemDescriptor<()> for BoxedSystem<(), ()> { SystemDescriptor::new(self).after(label) } + fn ambiguous_with(self, label: impl AsSystemLabel) -> SystemDescriptor { + SystemDescriptor::new(self).ambiguous_with(label) + } + + fn ignore_all_ambiguities(self) -> SystemDescriptor { + SystemDescriptor::new(self).ignore_all_ambiguities() + } + fn at_start(self) -> SystemDescriptor { SystemDescriptor::new(self).at_start() } From bbd5dfba56cf615df663d47df4b5b2bd8ddc1504 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Mon, 3 Oct 2022 16:17:50 -0400 Subject: [PATCH 2/4] add tests for silencing ambiguities --- .../src/schedule/ambiguity_detection.rs | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index 4fc9dac55280d..770c15d343af9 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -492,4 +492,52 @@ mod tests { assert_eq!(test_stage.ambiguity_count(&world), 0); } + + #[test] + fn ignore_all_ambiguities() { + let mut world = World::new(); + world.insert_resource(R); + + let mut test_stage = SystemStage::parallel(); + test_stage + .add_system(resmut_system.ignore_all_ambiguities()) + .add_system(res_system); + + test_stage.run(&mut world); + + assert_eq!(test_stage.ambiguity_count(&world), 0); + } + + #[test] + fn ambiguous_with_label() { + let mut world = World::new(); + world.insert_resource(R); + + #[derive(SystemLabel)] + struct IgnoreMe; + + let mut test_stage = SystemStage::parallel(); + test_stage + .add_system(resmut_system.ambiguous_with(IgnoreMe)) + .add_system(res_system.label(IgnoreMe)) + .add_system(res_system.label(IgnoreMe)); + + test_stage.run(&mut world); + + assert_eq!(test_stage.ambiguity_count(&world), 0); + } + + #[test] + fn ambiguous_with_system() { + let mut world = World::new(); + + let mut test_stage = SystemStage::parallel(); + test_stage + .add_system(write_component_system.ambiguous_with(read_component_system)) + .add_system(read_component_system); + + test_stage.run(&mut world); + + assert_eq!(test_stage.ambiguity_count(&world), 0); + } } From 101d277e7ff0fa3e9cc22d955e920eaeba2da9f0 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Mon, 3 Oct 2022 16:21:55 -0400 Subject: [PATCH 3/4] make some tests nicer --- crates/bevy_ecs/src/schedule/ambiguity_detection.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index 770c15d343af9..cb63497ee7919 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -501,7 +501,8 @@ mod tests { let mut test_stage = SystemStage::parallel(); test_stage .add_system(resmut_system.ignore_all_ambiguities()) - .add_system(res_system); + .add_system(res_system) + .add_system(nonsend_system); test_stage.run(&mut world); @@ -520,7 +521,7 @@ mod tests { test_stage .add_system(resmut_system.ambiguous_with(IgnoreMe)) .add_system(res_system.label(IgnoreMe)) - .add_system(res_system.label(IgnoreMe)); + .add_system(nonsend_system.label(IgnoreMe)); test_stage.run(&mut world); From f978367370f0564579e1731a4c0c136a62058f0b Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Sun, 9 Oct 2022 12:28:05 -0400 Subject: [PATCH 4/4] Add a test case for deterministic ambiguities --- .../src/schedule/ambiguity_detection.rs | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index cb63497ee7919..001f39530b7d3 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -541,4 +541,68 @@ mod tests { assert_eq!(test_stage.ambiguity_count(&world), 0); } + + fn system_a(_res: ResMut) {} + fn system_b(_res: ResMut) {} + fn system_c(_res: ResMut) {} + fn system_d(_res: ResMut) {} + fn system_e(_res: ResMut) {} + + // Tests that the correct ambiguities were reported in the correct order. + #[test] + fn correct_ambiguities() { + use super::*; + + let mut world = World::new(); + world.insert_resource(R); + + let mut test_stage = SystemStage::parallel(); + test_stage + .add_system(system_a) + .add_system(system_b) + .add_system(system_c.ignore_all_ambiguities()) + .add_system(system_d.ambiguous_with(system_b)) + .add_system(system_e.after(system_a)); + + test_stage.run(&mut world); + + let ambiguities = test_stage.ambiguities(&world); + assert_eq!( + ambiguities, + vec![ + SystemOrderAmbiguity { + system_names: [ + "bevy_ecs::schedule::ambiguity_detection::tests::system_a".to_string(), + "bevy_ecs::schedule::ambiguity_detection::tests::system_b".to_string() + ], + conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()], + segment: SystemStageSegment::Parallel, + }, + SystemOrderAmbiguity { + system_names: [ + "bevy_ecs::schedule::ambiguity_detection::tests::system_a".to_string(), + "bevy_ecs::schedule::ambiguity_detection::tests::system_d".to_string() + ], + conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()], + segment: SystemStageSegment::Parallel, + }, + SystemOrderAmbiguity { + system_names: [ + "bevy_ecs::schedule::ambiguity_detection::tests::system_b".to_string(), + "bevy_ecs::schedule::ambiguity_detection::tests::system_e".to_string() + ], + conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()], + segment: SystemStageSegment::Parallel, + }, + SystemOrderAmbiguity { + system_names: [ + "bevy_ecs::schedule::ambiguity_detection::tests::system_d".to_string(), + "bevy_ecs::schedule::ambiguity_detection::tests::system_e".to_string() + ], + conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()], + segment: SystemStageSegment::Parallel, + }, + ] + ); + } }