Skip to content

Commit 3321d68

Browse files
committed
Add methods for silencing system-order ambiguity warnings (#6158)
# Background Incremental implementation of #4299. The code is heavily borrowed from that PR. # Objective The execution order ambiguity checker often emits false positives, since bevy is not aware of invariants upheld by the user. ## Solution Title --- ## Changelog + Added methods `SystemDescriptor::ignore_all_ambiguities` and `::ambiguous_with`. These allow you to silence warnings for specific system-order ambiguities. ## Migration Guide ***Note for maintainers**: This should replace the migration guide for #5916* Ambiguity sets have been replaced with a simpler API. ```rust // These systems technically conflict, but we don't care which order they run in. fn jump_on_click(mouse: Res<Input<MouseButton>>, mut transforms: Query<&mut Transform>) { ... } fn jump_on_spacebar(keys: Res<Input<KeyCode>>, mut transforms: Query<&mut Transform>) { ... } // // Before #[derive(AmbiguitySetLabel)] struct JumpSystems; app .add_system(jump_on_click.in_ambiguity_set(JumpSystems)) .add_system(jump_on_spacebar.in_ambiguity_set(JumpSystems)); // // After app .add_system(jump_on_click.ambiguous_with(jump_on_spacebar)) .add_system(jump_on_spacebar); ```
1 parent f5322cd commit 3321d68

File tree

3 files changed

+198
-3
lines changed

3 files changed

+198
-3
lines changed

crates/bevy_ecs/src/schedule/ambiguity_detection.rs

+136-2
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@ use bevy_utils::tracing::info;
22
use fixedbitset::FixedBitSet;
33

44
use crate::component::ComponentId;
5-
use crate::schedule::{SystemContainer, SystemStage};
5+
use crate::schedule::{AmbiguityDetection, GraphNode, SystemContainer, SystemStage};
66
use crate::world::World;
77

8+
use super::SystemLabelId;
9+
810
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
911
struct SystemOrderAmbiguity {
1012
segment: SystemStageSegment,
@@ -194,6 +196,24 @@ impl SystemStage {
194196
/// along with specific components that have triggered the warning.
195197
/// Systems must be topologically sorted beforehand.
196198
fn find_ambiguities(systems: &[SystemContainer]) -> Vec<(usize, usize, Vec<ComponentId>)> {
199+
// Check if we should ignore ambiguities between `system_a` and `system_b`.
200+
fn should_ignore(system_a: &SystemContainer, system_b: &SystemContainer) -> bool {
201+
fn should_ignore_inner(
202+
system_a_detection: &AmbiguityDetection,
203+
system_b_labels: &[SystemLabelId],
204+
) -> bool {
205+
match system_a_detection {
206+
AmbiguityDetection::Check => false,
207+
AmbiguityDetection::IgnoreAll => true,
208+
AmbiguityDetection::IgnoreWithLabel(labels) => {
209+
labels.iter().any(|l| system_b_labels.contains(l))
210+
}
211+
}
212+
}
213+
should_ignore_inner(&system_a.ambiguity_detection, system_b.labels())
214+
|| should_ignore_inner(&system_b.ambiguity_detection, system_a.labels())
215+
}
216+
197217
let mut all_dependencies = Vec::<FixedBitSet>::with_capacity(systems.len());
198218
let mut all_dependants = Vec::<FixedBitSet>::with_capacity(systems.len());
199219
for (index, container) in systems.iter().enumerate() {
@@ -235,7 +255,8 @@ fn find_ambiguities(systems: &[SystemContainer]) -> Vec<(usize, usize, Vec<Compo
235255
for index_b in full_bitset.difference(&relations)
236256
// .take(index_a)
237257
{
238-
if !processed.contains(index_b) {
258+
if !processed.contains(index_b) && !should_ignore(&systems[index_a], &systems[index_b])
259+
{
239260
let system_a = &systems[index_a];
240261
let system_b = &systems[index_b];
241262
if system_a.is_exclusive() || system_b.is_exclusive() {
@@ -471,4 +492,117 @@ mod tests {
471492

472493
assert_eq!(test_stage.ambiguity_count(&world), 0);
473494
}
495+
496+
#[test]
497+
fn ignore_all_ambiguities() {
498+
let mut world = World::new();
499+
world.insert_resource(R);
500+
501+
let mut test_stage = SystemStage::parallel();
502+
test_stage
503+
.add_system(resmut_system.ignore_all_ambiguities())
504+
.add_system(res_system)
505+
.add_system(nonsend_system);
506+
507+
test_stage.run(&mut world);
508+
509+
assert_eq!(test_stage.ambiguity_count(&world), 0);
510+
}
511+
512+
#[test]
513+
fn ambiguous_with_label() {
514+
let mut world = World::new();
515+
world.insert_resource(R);
516+
517+
#[derive(SystemLabel)]
518+
struct IgnoreMe;
519+
520+
let mut test_stage = SystemStage::parallel();
521+
test_stage
522+
.add_system(resmut_system.ambiguous_with(IgnoreMe))
523+
.add_system(res_system.label(IgnoreMe))
524+
.add_system(nonsend_system.label(IgnoreMe));
525+
526+
test_stage.run(&mut world);
527+
528+
assert_eq!(test_stage.ambiguity_count(&world), 0);
529+
}
530+
531+
#[test]
532+
fn ambiguous_with_system() {
533+
let mut world = World::new();
534+
535+
let mut test_stage = SystemStage::parallel();
536+
test_stage
537+
.add_system(write_component_system.ambiguous_with(read_component_system))
538+
.add_system(read_component_system);
539+
540+
test_stage.run(&mut world);
541+
542+
assert_eq!(test_stage.ambiguity_count(&world), 0);
543+
}
544+
545+
fn system_a(_res: ResMut<R>) {}
546+
fn system_b(_res: ResMut<R>) {}
547+
fn system_c(_res: ResMut<R>) {}
548+
fn system_d(_res: ResMut<R>) {}
549+
fn system_e(_res: ResMut<R>) {}
550+
551+
// Tests that the correct ambiguities were reported in the correct order.
552+
#[test]
553+
fn correct_ambiguities() {
554+
use super::*;
555+
556+
let mut world = World::new();
557+
world.insert_resource(R);
558+
559+
let mut test_stage = SystemStage::parallel();
560+
test_stage
561+
.add_system(system_a)
562+
.add_system(system_b)
563+
.add_system(system_c.ignore_all_ambiguities())
564+
.add_system(system_d.ambiguous_with(system_b))
565+
.add_system(system_e.after(system_a));
566+
567+
test_stage.run(&mut world);
568+
569+
let ambiguities = test_stage.ambiguities(&world);
570+
assert_eq!(
571+
ambiguities,
572+
vec![
573+
SystemOrderAmbiguity {
574+
system_names: [
575+
"bevy_ecs::schedule::ambiguity_detection::tests::system_a".to_string(),
576+
"bevy_ecs::schedule::ambiguity_detection::tests::system_b".to_string()
577+
],
578+
conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()],
579+
segment: SystemStageSegment::Parallel,
580+
},
581+
SystemOrderAmbiguity {
582+
system_names: [
583+
"bevy_ecs::schedule::ambiguity_detection::tests::system_a".to_string(),
584+
"bevy_ecs::schedule::ambiguity_detection::tests::system_d".to_string()
585+
],
586+
conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()],
587+
segment: SystemStageSegment::Parallel,
588+
},
589+
SystemOrderAmbiguity {
590+
system_names: [
591+
"bevy_ecs::schedule::ambiguity_detection::tests::system_b".to_string(),
592+
"bevy_ecs::schedule::ambiguity_detection::tests::system_e".to_string()
593+
],
594+
conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()],
595+
segment: SystemStageSegment::Parallel,
596+
},
597+
SystemOrderAmbiguity {
598+
system_names: [
599+
"bevy_ecs::schedule::ambiguity_detection::tests::system_d".to_string(),
600+
"bevy_ecs::schedule::ambiguity_detection::tests::system_e".to_string()
601+
],
602+
conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()],
603+
segment: SystemStageSegment::Parallel,
604+
},
605+
]
606+
);
607+
}
474608
}

crates/bevy_ecs/src/schedule/system_container.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
use crate::{
22
component::ComponentId,
33
query::Access,
4-
schedule::{GraphNode, RunCriteriaLabelId, SystemDescriptor, SystemLabelId},
4+
schedule::{
5+
AmbiguityDetection, GraphNode, RunCriteriaLabelId, SystemDescriptor, SystemLabelId,
6+
},
57
system::System,
68
};
79
use std::borrow::Cow;
@@ -16,6 +18,7 @@ pub struct SystemContainer {
1618
labels: Vec<SystemLabelId>,
1719
before: Vec<SystemLabelId>,
1820
after: Vec<SystemLabelId>,
21+
pub(crate) ambiguity_detection: AmbiguityDetection,
1922
}
2023

2124
impl SystemContainer {
@@ -29,6 +32,7 @@ impl SystemContainer {
2932
labels: descriptor.labels,
3033
before: descriptor.before,
3134
after: descriptor.after,
35+
ambiguity_detection: descriptor.ambiguity_detection,
3236
is_exclusive: descriptor.exclusive_insertion_point.is_some(),
3337
}
3438
}

crates/bevy_ecs/src/schedule/system_descriptor.rs

+57
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,16 @@ use crate::{
33
system::{AsSystemLabel, BoxedSystem, IntoSystem},
44
};
55

6+
/// Configures ambiguity detection for a single system.
7+
#[derive(Default)]
8+
pub(crate) enum AmbiguityDetection {
9+
#[default]
10+
Check,
11+
IgnoreAll,
12+
/// Ignore systems with any of these labels.
13+
IgnoreWithLabel(Vec<SystemLabelId>),
14+
}
15+
616
/// Encapsulates a system and information on when it run in a `SystemStage`.
717
///
818
/// Systems can be inserted into 4 different groups within the stage:
@@ -38,6 +48,7 @@ pub struct SystemDescriptor {
3848
pub(crate) labels: Vec<SystemLabelId>,
3949
pub(crate) before: Vec<SystemLabelId>,
4050
pub(crate) after: Vec<SystemLabelId>,
51+
pub(crate) ambiguity_detection: AmbiguityDetection,
4152
}
4253

4354
impl SystemDescriptor {
@@ -53,6 +64,7 @@ impl SystemDescriptor {
5364
run_criteria: None,
5465
before: Vec::new(),
5566
after: Vec::new(),
67+
ambiguity_detection: Default::default(),
5668
}
5769
}
5870
}
@@ -75,6 +87,15 @@ pub trait IntoSystemDescriptor<Params> {
7587
/// Specifies that the system should run after systems with the given label.
7688
fn after<Marker>(self, label: impl AsSystemLabel<Marker>) -> SystemDescriptor;
7789

90+
/// Marks this system as ambiguous with any system with the specified label.
91+
/// This means that execution order between these systems does not matter,
92+
/// which allows [some warnings](crate::schedule::ReportExecutionOrderAmbiguities) to be silenced.
93+
fn ambiguous_with<Marker>(self, label: impl AsSystemLabel<Marker>) -> SystemDescriptor;
94+
95+
/// Specifies that this system should opt out of
96+
/// [execution order ambiguity detection](crate::schedule::ReportExecutionOrderAmbiguities).
97+
fn ignore_all_ambiguities(self) -> SystemDescriptor;
98+
7899
/// Specifies that the system should run with other exclusive systems at the start of stage.
79100
fn at_start(self) -> SystemDescriptor;
80101

@@ -110,6 +131,26 @@ impl IntoSystemDescriptor<()> for SystemDescriptor {
110131
self
111132
}
112133

134+
fn ambiguous_with<Marker>(mut self, label: impl AsSystemLabel<Marker>) -> SystemDescriptor {
135+
match &mut self.ambiguity_detection {
136+
detection @ AmbiguityDetection::Check => {
137+
*detection =
138+
AmbiguityDetection::IgnoreWithLabel(vec![label.as_system_label().as_label()]);
139+
}
140+
AmbiguityDetection::IgnoreWithLabel(labels) => {
141+
labels.push(label.as_system_label().as_label());
142+
}
143+
// This descriptor is already ambiguous with everything.
144+
AmbiguityDetection::IgnoreAll => {}
145+
}
146+
self
147+
}
148+
149+
fn ignore_all_ambiguities(mut self) -> SystemDescriptor {
150+
self.ambiguity_detection = AmbiguityDetection::IgnoreAll;
151+
self
152+
}
153+
113154
fn at_start(mut self) -> SystemDescriptor {
114155
self.exclusive_insertion_point = Some(ExclusiveInsertionPoint::AtStart);
115156
self
@@ -154,6 +195,14 @@ where
154195
SystemDescriptor::new(Box::new(IntoSystem::into_system(self))).after(label)
155196
}
156197

198+
fn ambiguous_with<Marker>(self, label: impl AsSystemLabel<Marker>) -> SystemDescriptor {
199+
SystemDescriptor::new(Box::new(IntoSystem::into_system(self))).ambiguous_with(label)
200+
}
201+
202+
fn ignore_all_ambiguities(self) -> SystemDescriptor {
203+
SystemDescriptor::new(Box::new(IntoSystem::into_system(self))).ignore_all_ambiguities()
204+
}
205+
157206
fn at_start(self) -> SystemDescriptor {
158207
SystemDescriptor::new(Box::new(IntoSystem::into_system(self))).at_start()
159208
}
@@ -191,6 +240,14 @@ impl IntoSystemDescriptor<()> for BoxedSystem<(), ()> {
191240
SystemDescriptor::new(self).after(label)
192241
}
193242

243+
fn ambiguous_with<Marker>(self, label: impl AsSystemLabel<Marker>) -> SystemDescriptor {
244+
SystemDescriptor::new(self).ambiguous_with(label)
245+
}
246+
247+
fn ignore_all_ambiguities(self) -> SystemDescriptor {
248+
SystemDescriptor::new(self).ignore_all_ambiguities()
249+
}
250+
194251
fn at_start(self) -> SystemDescriptor {
195252
SystemDescriptor::new(self).at_start()
196253
}

0 commit comments

Comments
 (0)