diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index faba4851e95c9..5f0ae88a8f015 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -716,4 +716,334 @@ mod tests { assert!(matches!(result, Err(ScheduleBuildError::Ambiguity(_)))); } } + + mod system_ambiguity { + // Required to make the derive macro behave + use crate as bevy_ecs; + use crate::event::Events; + use crate::prelude::*; + + #[derive(Resource)] + struct R; + + #[derive(Component)] + struct A; + + #[derive(Component)] + struct B; + + // An event type + #[derive(Event)] + struct E; + + fn empty_system() {} + fn res_system(_res: Res) {} + fn resmut_system(_res: ResMut) {} + fn nonsend_system(_ns: NonSend) {} + fn nonsendmut_system(_ns: NonSendMut) {} + fn read_component_system(_query: Query<&A>) {} + fn write_component_system(_query: Query<&mut A>) {} + fn with_filtered_component_system(_query: Query<&mut A, With>) {} + fn without_filtered_component_system(_query: Query<&mut A, Without>) {} + fn event_reader_system(_reader: EventReader) {} + fn event_writer_system(_writer: EventWriter) {} + fn event_resource_system(_events: ResMut>) {} + fn read_world_system(_world: &World) {} + fn write_world_system(_world: &mut World) {} + + // Tests for conflict detection + + #[test] + fn one_of_everything() { + let mut world = World::new(); + world.insert_resource(R); + world.spawn(A); + world.init_resource::>(); + + let mut schedule = Schedule::default(); + schedule + // nonsendmut system deliberately conflicts with resmut system + .add_systems((resmut_system, write_component_system, event_writer_system)); + + let _ = schedule.initialize(&mut world); + + assert_eq!(schedule.graph().conflicting_systems().len(), 0); + } + + #[test] + fn read_only() { + let mut world = World::new(); + world.insert_resource(R); + world.spawn(A); + world.init_resource::>(); + + let mut schedule = Schedule::default(); + schedule.add_systems(( + empty_system, + empty_system, + res_system, + res_system, + nonsend_system, + nonsend_system, + read_component_system, + read_component_system, + event_reader_system, + event_reader_system, + read_world_system, + read_world_system, + )); + + let _ = schedule.initialize(&mut world); + + assert_eq!(schedule.graph().conflicting_systems().len(), 0); + } + + #[test] + fn read_world() { + let mut world = World::new(); + world.insert_resource(R); + world.spawn(A); + world.init_resource::>(); + + let mut schedule = Schedule::default(); + schedule.add_systems(( + resmut_system, + write_component_system, + event_writer_system, + read_world_system, + )); + + let _ = schedule.initialize(&mut world); + + assert_eq!(schedule.graph().conflicting_systems().len(), 3); + } + + #[test] + fn resources() { + let mut world = World::new(); + world.insert_resource(R); + + let mut schedule = Schedule::default(); + schedule.add_systems((resmut_system, res_system)); + + let _ = schedule.initialize(&mut world); + + assert_eq!(schedule.graph().conflicting_systems().len(), 1); + } + + #[test] + fn nonsend() { + let mut world = World::new(); + world.insert_resource(R); + + let mut schedule = Schedule::default(); + schedule.add_systems((nonsendmut_system, nonsend_system)); + + let _ = schedule.initialize(&mut world); + + assert_eq!(schedule.graph().conflicting_systems().len(), 1); + } + + #[test] + fn components() { + let mut world = World::new(); + world.spawn(A); + + let mut schedule = Schedule::default(); + schedule.add_systems((read_component_system, write_component_system)); + + let _ = schedule.initialize(&mut world); + + assert_eq!(schedule.graph().conflicting_systems().len(), 1); + } + + #[test] + #[ignore = "Known failing but fix is non-trivial: https://github.com/bevyengine/bevy/issues/4381"] + fn filtered_components() { + let mut world = World::new(); + world.spawn(A); + + let mut schedule = Schedule::default(); + schedule.add_systems(( + with_filtered_component_system, + without_filtered_component_system, + )); + + let _ = schedule.initialize(&mut world); + + assert_eq!(schedule.graph().conflicting_systems().len(), 0); + } + + #[test] + fn events() { + let mut world = World::new(); + world.init_resource::>(); + + let mut schedule = Schedule::default(); + schedule.add_systems(( + // All of these systems clash + event_reader_system, + event_writer_system, + event_resource_system, + )); + + let _ = schedule.initialize(&mut world); + + assert_eq!(schedule.graph().conflicting_systems().len(), 3); + } + + #[test] + fn exclusive() { + let mut world = World::new(); + world.insert_resource(R); + world.spawn(A); + world.init_resource::>(); + + let mut schedule = Schedule::default(); + schedule.add_systems(( + // All 3 of these conflict with each other + write_world_system, + write_world_system, + res_system, + )); + + let _ = schedule.initialize(&mut world); + + assert_eq!(schedule.graph().conflicting_systems().len(), 3); + } + + // Tests for silencing and resolving ambiguities + #[test] + fn before_and_after() { + let mut world = World::new(); + world.init_resource::>(); + + let mut schedule = Schedule::default(); + schedule.add_systems(( + event_reader_system.before(event_writer_system), + event_writer_system, + event_resource_system.after(event_writer_system), + )); + + let _ = schedule.initialize(&mut world); + + assert_eq!(schedule.graph().conflicting_systems().len(), 0); + } + + #[test] + fn ignore_all_ambiguities() { + let mut world = World::new(); + world.insert_resource(R); + + let mut schedule = Schedule::default(); + schedule.add_systems(( + resmut_system.ambiguous_with_all(), + res_system, + nonsend_system, + )); + + let _ = schedule.initialize(&mut world); + + assert_eq!(schedule.graph().conflicting_systems().len(), 0); + } + + #[test] + fn ambiguous_with_label() { + let mut world = World::new(); + world.insert_resource(R); + + #[derive(SystemSet, Hash, PartialEq, Eq, Debug, Clone)] + struct IgnoreMe; + + let mut schedule = Schedule::default(); + schedule.add_systems(( + resmut_system.ambiguous_with(IgnoreMe), + res_system.in_set(IgnoreMe), + nonsend_system.in_set(IgnoreMe), + )); + + let _ = schedule.initialize(&mut world); + + assert_eq!(schedule.graph().conflicting_systems().len(), 0); + } + + #[test] + fn ambiguous_with_system() { + let mut world = World::new(); + + let mut schedule = Schedule::default(); + schedule.add_systems(( + write_component_system.ambiguous_with(read_component_system), + read_component_system, + )); + let _ = schedule.initialize(&mut world); + + assert_eq!(schedule.graph().conflicting_systems().len(), 0); + } + + // Tests that the correct ambiguities were reported in the correct order. + #[test] + fn correct_ambiguities() { + use super::*; + + #[derive(ScheduleLabel, Hash, PartialEq, Eq, Debug, Clone)] + struct TestSchedule; + + 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) {} + + let mut world = World::new(); + world.insert_resource(R); + + let mut schedule = Schedule::new(TestSchedule); + schedule.add_systems(( + system_a, + system_b, + system_c.ambiguous_with_all(), + system_d.ambiguous_with(system_b), + system_e.after(system_a), + )); + + schedule.graph_mut().initialize(&mut world); + let _ = schedule + .graph_mut() + .build_schedule(world.components(), &TestSchedule.dyn_clone()); + + let ambiguities: Vec<_> = schedule + .graph() + .conflicts_to_string(world.components()) + .collect(); + + let expected = &[ + ( + "system_d".to_string(), + "system_a".to_string(), + vec!["bevy_ecs::schedule::tests::system_ambiguity::R"], + ), + ( + "system_d".to_string(), + "system_e".to_string(), + vec!["bevy_ecs::schedule::tests::system_ambiguity::R"], + ), + ( + "system_b".to_string(), + "system_a".to_string(), + vec!["bevy_ecs::schedule::tests::system_ambiguity::R"], + ), + ( + "system_b".to_string(), + "system_e".to_string(), + vec!["bevy_ecs::schedule::tests::system_ambiguity::R"], + ), + ]; + + // ordering isn't stable so do this + for entry in expected { + assert!(ambiguities.contains(entry)); + } + } + } } diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 33d5c5647f936..0024360158a20 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -1553,21 +1553,11 @@ impl ScheduleGraph { Consider adding `before`, `after`, or `ambiguous_with` relationships between these:\n", ); - for (system_a, system_b, conflicts) in ambiguities { - let name_a = self.get_node_name(system_a); - let name_b = self.get_node_name(system_b); - - debug_assert!(system_a.is_system(), "{name_a} is not a system."); - debug_assert!(system_b.is_system(), "{name_b} is not a system."); - + for (name_a, name_b, conflicts) in self.conflicts_to_string(components) { writeln!(message, " -- {name_a} and {name_b}").unwrap(); - if !conflicts.is_empty() { - let conflict_names: Vec<_> = conflicts - .iter() - .map(|id| components.get_name(*id).unwrap()) - .collect(); - writeln!(message, " conflict on: {conflict_names:?}").unwrap(); + if !conflicts.is_empty() { + writeln!(message, " conflict on: {conflicts:?}").unwrap(); } else { // one or both systems must be exclusive let world = std::any::type_name::(); @@ -1578,6 +1568,29 @@ impl ScheduleGraph { message } + /// convert conflics to human readable format + pub fn conflicts_to_string<'a>( + &'a self, + components: &'a Components, + ) -> impl Iterator)> + 'a { + self.conflicting_systems + .iter() + .map(move |(system_a, system_b, conflicts)| { + let name_a = self.get_node_name(system_a); + let name_b = self.get_node_name(system_b); + + debug_assert!(system_a.is_system(), "{name_a} is not a system."); + debug_assert!(system_b.is_system(), "{name_b} is not a system."); + + let conflict_names: Vec<_> = conflicts + .iter() + .map(|id| components.get_name(*id).unwrap()) + .collect(); + + (name_a, name_b, conflict_names) + }) + } + fn traverse_sets_containing_node(&self, id: NodeId, f: &mut impl FnMut(NodeId) -> bool) { for (set_id, _, _) in self.hierarchy.graph.edges_directed(id, Direction::Incoming) { if f(set_id) {