From 02025eff0b25bb8c18103dbafc33a764e1989ad4 Mon Sep 17 00:00:00 2001 From: Mike Date: Thu, 31 Aug 2023 15:52:59 -0700 Subject: [PATCH] Fix anonymous set name stack overflow (#9650) # Objective - Fixes #9641 - Anonymous sets are named by their system members. When `ScheduleBuildSettings::report_sets` is on, systems are named by their sets. So when getting the anonymous set name this would cause an infinite recursion. ## Solution - When getting the anonymous system set name, don't get their system's names with the sets the systems belong to. ## Other Possible solutions - An alternate solution might be to skip anonymous sets when getting the system's name for an anonymous set's name. --- crates/bevy_ecs/src/schedule/mod.rs | 33 ++++++++++++++++++++++++ crates/bevy_ecs/src/schedule/schedule.rs | 10 +++++-- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index ad5bc617a6a2e..88671df5c3575 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -1045,5 +1045,38 @@ mod tests { assert!(ambiguities.contains(entry)); } } + + // Test that anonymous set names work properly + // Related issue https://github.com/bevyengine/bevy/issues/9641 + #[test] + fn anonymous_set_name() { + use super::*; + + #[derive(ScheduleLabel, Hash, PartialEq, Eq, Debug, Clone)] + struct TestSchedule; + + let mut schedule = Schedule::new(TestSchedule); + schedule.add_systems((resmut_system, resmut_system).run_if(|| true)); + + let mut world = World::new(); + 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(schedule.graph().conflicting_systems(), world.components()) + .collect(); + + assert_eq!( + ambiguities[0], + ( + "resmut_system (in set (resmut_system, resmut_system))".to_string(), + "resmut_system (in set (resmut_system, resmut_system))".to_string(), + vec!["bevy_ecs::schedule::tests::system_ambiguity::R"], + ) + ); + } } } diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 8b174abdcc7ff..f012cef7ffd3c 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -1257,10 +1257,15 @@ enum ReportCycles { // methods for reporting errors impl ScheduleGraph { fn get_node_name(&self, id: &NodeId) -> String { + self.get_node_name_inner(id, self.settings.report_sets) + } + + #[inline] + fn get_node_name_inner(&self, id: &NodeId, report_sets: bool) -> String { let mut name = match id { NodeId::System(_) => { let name = self.systems[id.index()].get().unwrap().name().to_string(); - if self.settings.report_sets { + if report_sets { let sets = self.names_of_sets_containing_node(id); if sets.is_empty() { name @@ -1294,7 +1299,8 @@ impl ScheduleGraph { self.hierarchy .graph .edges_directed(*id, Direction::Outgoing) - .map(|(_, member_id, _)| self.get_node_name(&member_id)) + // never get the sets of the members or this will infinite recurse when the report_sets setting is on. + .map(|(_, member_id, _)| self.get_node_name_inner(&member_id, false)) .reduce(|a, b| format!("{a}, {b}")) .unwrap_or_default() )