From 7b6f8659f8e85056273ccc41388ef0620aef35cd Mon Sep 17 00:00:00 2001 From: Mike Date: Sun, 27 Aug 2023 10:54:59 -0700 Subject: [PATCH] Refactor build_schedule and related errors (#9579) # Objective - break up large build_schedule system to make it easier to read - Clean up related error messages. - I have a follow up PR that adds the schedule name to the error messages, but wanted to break this up from that. ## Changelog - refactor `build_schedule` to be easier to read ## Sample Error Messages Dependency Cycle ```text thread 'main' panicked at 'System dependencies contain cycle(s). schedule has 1 before/after cycle(s): cycle 1: system set 'A' must run before itself system set 'A' ... which must run before system set 'B' ... which must run before system set 'A' ', crates\bevy_ecs\src\schedule\schedule.rs:228:13 ``` ```text thread 'main' panicked at 'System dependencies contain cycle(s). schedule has 1 before/after cycle(s): cycle 1: system 'foo' must run before itself system 'foo' ... which must run before system 'bar' ... which must run before system 'foo' ', crates\bevy_ecs\src\schedule\schedule.rs:228:13 ``` Hierarchy Cycle ```text thread 'main' panicked at 'System set hierarchy contains cycle(s). schedule has 1 in_set cycle(s): cycle 1: set 'A' contains itself set 'A' ... which contains set 'B' ... which contains set 'A' ', crates\bevy_ecs\src\schedule\schedule.rs:230:13 ``` System Type Set ```text thread 'main' panicked at 'Tried to order against `SystemTypeSet(fn foo())` in a schedule that has more than one `SystemTypeSet(fn foo())` instance. `SystemTypeSet(fn foo())` is a `SystemTypeSet` and cannot be used for ordering if ambiguous. Use a different set without this restriction.', crates\bevy_ecs\src\schedule\schedule.rs:230:13 ``` Hierarchy Redundancy ```text thread 'main' panicked at 'System set hierarchy contains redundant edges. hierarchy contains redundant edge(s) -- system set 'X' cannot be child of set 'A', longer path exists ', crates\bevy_ecs\src\schedule\schedule.rs:230:13 ``` Systems have ordering but interset ```text thread 'main' panicked at '`A` and `C` have a `before`-`after` relationship (which may be transitive) but share systems.', crates\bevy_ecs\src\schedule\schedule.rs:227:51 ``` Cross Dependency ```text thread 'main' panicked at '`A` and `B` have both `in_set` and `before`-`after` relationships (these might be transitive). This combination is unsolvable as a system cannot run before or after a set it belongs to.', crates\bevy_ecs\src\schedule\schedule.rs:230:13 ``` Ambiguity ```text thread 'main' panicked at 'Systems with conflicting access have indeterminate run order. 1 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these: -- res_mut and res_ref conflict on: ["bevymark::ambiguity::X"] ', crates\bevy_ecs\src\schedule\schedule.rs:230:13 ``` --- crates/bevy_ecs/src/schedule/mod.rs | 16 +- crates/bevy_ecs/src/schedule/schedule.rs | 386 ++++++++++++++--------- crates/bevy_ecs/src/schedule/set.rs | 2 +- 3 files changed, 244 insertions(+), 160 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index c585fb508fb72..1982ce03dae3d 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -541,7 +541,10 @@ mod tests { schedule.configure_set(TestSet::B.after(TestSet::A)); let result = schedule.initialize(&mut world); - assert!(matches!(result, Err(ScheduleBuildError::DependencyCycle))); + assert!(matches!( + result, + Err(ScheduleBuildError::DependencyCycle(_)) + )); fn foo() {} fn bar() {} @@ -551,7 +554,10 @@ mod tests { schedule.add_systems((foo.after(bar), bar.after(foo))); let result = schedule.initialize(&mut world); - assert!(matches!(result, Err(ScheduleBuildError::DependencyCycle))); + assert!(matches!( + result, + Err(ScheduleBuildError::DependencyCycle(_)) + )); } #[test] @@ -570,7 +576,7 @@ mod tests { schedule.configure_set(TestSet::B.in_set(TestSet::A)); let result = schedule.initialize(&mut world); - assert!(matches!(result, Err(ScheduleBuildError::HierarchyCycle))); + assert!(matches!(result, Err(ScheduleBuildError::HierarchyCycle(_)))); } #[test] @@ -645,7 +651,7 @@ mod tests { let result = schedule.initialize(&mut world); assert!(matches!( result, - Err(ScheduleBuildError::HierarchyRedundancy) + Err(ScheduleBuildError::HierarchyRedundancy(_)) )); } @@ -707,7 +713,7 @@ mod tests { schedule.add_systems((res_ref, res_mut)); let result = schedule.initialize(&mut world); - assert!(matches!(result, Err(ScheduleBuildError::Ambiguity))); + assert!(matches!(result, Err(ScheduleBuildError::Ambiguity(_)))); } } } diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 18e230114e69d..2db71e2dc19e2 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -382,9 +382,7 @@ pub struct ScheduleGraph { uninit: Vec<(NodeId, usize)>, hierarchy: Dag, dependency: Dag, - dependency_flattened: Dag, ambiguous_with: UnGraphMap, - ambiguous_with_flattened: UnGraphMap, ambiguous_with_all: HashSet, conflicting_systems: Vec<(NodeId, NodeId, Vec)>, changed: bool, @@ -403,9 +401,7 @@ impl ScheduleGraph { uninit: Vec::new(), hierarchy: Dag::new(), dependency: Dag::new(), - dependency_flattened: Dag::new(), ambiguous_with: UnGraphMap::new(), - ambiguous_with_flattened: UnGraphMap::new(), ambiguous_with_all: HashSet::new(), conflicting_systems: Vec::new(), changed: false, @@ -863,45 +859,70 @@ impl ScheduleGraph { components: &Components, ) -> Result { // check hierarchy for cycles - self.hierarchy.topsort = self - .topsort_graph(&self.hierarchy.graph, ReportCycles::Hierarchy) - .map_err(|_| ScheduleBuildError::HierarchyCycle)?; + self.hierarchy.topsort = + self.topsort_graph(&self.hierarchy.graph, ReportCycles::Hierarchy)?; let hier_results = check_graph(&self.hierarchy.graph, &self.hierarchy.topsort); - if self.settings.hierarchy_detection != LogLevel::Ignore - && self.contains_hierarchy_conflicts(&hier_results.transitive_edges) - { - self.report_hierarchy_conflicts(&hier_results.transitive_edges); - if matches!(self.settings.hierarchy_detection, LogLevel::Error) { - return Err(ScheduleBuildError::HierarchyRedundancy); - } - } + self.optionally_check_hierarchy_conflicts(&hier_results.transitive_edges)?; // remove redundant edges self.hierarchy.graph = hier_results.transitive_reduction; // check dependencies for cycles - self.dependency.topsort = self - .topsort_graph(&self.dependency.graph, ReportCycles::Dependency) - .map_err(|_| ScheduleBuildError::DependencyCycle)?; + self.dependency.topsort = + self.topsort_graph(&self.dependency.graph, ReportCycles::Dependency)?; // check for systems or system sets depending on sets they belong to let dep_results = check_graph(&self.dependency.graph, &self.dependency.topsort); - for &(a, b) in dep_results.connected.iter() { - if hier_results.connected.contains(&(a, b)) || hier_results.connected.contains(&(b, a)) - { - let name_a = self.get_node_name(&a); - let name_b = self.get_node_name(&b); - return Err(ScheduleBuildError::CrossDependency(name_a, name_b)); - } - } + self.check_for_cross_dependencies(&dep_results, &hier_results.connected)?; // map all system sets to their systems // go in reverse topological order (bottom-up) for efficiency + let (set_systems, set_system_bitsets) = + self.map_sets_to_systems(&self.hierarchy.topsort, &self.hierarchy.graph); + self.check_order_but_intersect(&dep_results.connected, &set_system_bitsets)?; + + // check that there are no edges to system-type sets that have multiple instances + self.check_system_type_set_ambiguity(&set_systems)?; + + let dependency_flattened = self.get_dependency_flattened(&set_systems); + + // topsort + let mut dependency_flattened_dag = Dag { + topsort: self.topsort_graph(&dependency_flattened, ReportCycles::Dependency)?, + graph: dependency_flattened, + }; + + let flat_results = check_graph( + &dependency_flattened_dag.graph, + &dependency_flattened_dag.topsort, + ); + + // remove redundant edges + dependency_flattened_dag.graph = flat_results.transitive_reduction; + + // flatten: combine `in_set` with `ambiguous_with` information + let ambiguous_with_flattened = self.get_ambiguous_with_flattened(&set_systems); + + // check for conflicts + let conflicting_systems = + self.get_conflicting_systems(&flat_results.disconnected, &ambiguous_with_flattened); + self.optionally_check_conflicts(&conflicting_systems, components)?; + self.conflicting_systems = conflicting_systems; + + // build the schedule + Ok(self.build_schedule_inner(dependency_flattened_dag, hier_results.reachable)) + } + + fn map_sets_to_systems( + &self, + hierarchy_topsort: &[NodeId], + hierarchy_graph: &GraphMap, + ) -> (HashMap>, HashMap) { let mut set_systems: HashMap> = HashMap::with_capacity(self.system_sets.len()); let mut set_system_bitsets = HashMap::with_capacity(self.system_sets.len()); - for &id in self.hierarchy.topsort.iter().rev() { + for &id in hierarchy_topsort.iter().rev() { if id.is_system() { continue; } @@ -909,11 +930,7 @@ impl ScheduleGraph { let mut systems = Vec::new(); let mut system_bitset = FixedBitSet::with_capacity(self.systems.len()); - for child in self - .hierarchy - .graph - .neighbors_directed(id, Direction::Outgoing) - { + for child in hierarchy_graph.neighbors_directed(id, Direction::Outgoing) { match child { NodeId::System(_) => { systems.push(child); @@ -931,47 +948,13 @@ impl ScheduleGraph { set_systems.insert(id, systems); set_system_bitsets.insert(id, system_bitset); } + (set_systems, set_system_bitsets) + } - // check that there is no ordering between system sets that intersect - for (a, b) in dep_results.connected.iter() { - if !(a.is_set() && b.is_set()) { - continue; - } - - let a_systems = set_system_bitsets.get(a).unwrap(); - let b_systems = set_system_bitsets.get(b).unwrap(); - - if !(a_systems.is_disjoint(b_systems)) { - return Err(ScheduleBuildError::SetsHaveOrderButIntersect( - self.get_node_name(a), - self.get_node_name(b), - )); - } - } - - // check that there are no edges to system-type sets that have multiple instances - for (&id, systems) in set_systems.iter() { - let set = &self.system_sets[id.index()]; - if set.is_system_type() { - let instances = systems.len(); - let ambiguous_with = self.ambiguous_with.edges(id); - let before = self - .dependency - .graph - .edges_directed(id, Direction::Incoming); - let after = self - .dependency - .graph - .edges_directed(id, Direction::Outgoing); - let relations = before.count() + after.count() + ambiguous_with.count(); - if instances > 1 && relations > 0 { - return Err(ScheduleBuildError::SystemTypeSetAmbiguity( - self.get_node_name(&id), - )); - } - } - } - + fn get_dependency_flattened( + &self, + set_systems: &HashMap>, + ) -> GraphMap { // flatten: combine `in_set` with `before` and `after` information // have to do it like this to preserve transitivity let mut dependency_flattened = self.dependency.graph.clone(); @@ -1003,21 +986,13 @@ impl ScheduleGraph { } } - // topsort - self.dependency_flattened.topsort = self - .topsort_graph(&dependency_flattened, ReportCycles::Dependency) - .map_err(|_| ScheduleBuildError::DependencyCycle)?; - self.dependency_flattened.graph = dependency_flattened; - - let flat_results = check_graph( - &self.dependency_flattened.graph, - &self.dependency_flattened.topsort, - ); - - // remove redundant edges - self.dependency_flattened.graph = flat_results.transitive_reduction; + dependency_flattened + } - // flatten: combine `in_set` with `ambiguous_with` information + fn get_ambiguous_with_flattened( + &self, + set_systems: &HashMap>, + ) -> GraphMap { let mut ambiguous_with_flattened = UnGraphMap::new(); for (lhs, rhs, _) in self.ambiguous_with.all_edges() { match (lhs, rhs) { @@ -1044,12 +1019,17 @@ impl ScheduleGraph { } } - self.ambiguous_with_flattened = ambiguous_with_flattened; + ambiguous_with_flattened + } - // check for conflicts + fn get_conflicting_systems( + &self, + flat_results_disconnected: &Vec<(NodeId, NodeId)>, + ambiguous_with_flattened: &GraphMap, + ) -> Vec<(NodeId, NodeId, Vec)> { let mut conflicting_systems = Vec::new(); - for &(a, b) in &flat_results.disconnected { - if self.ambiguous_with_flattened.contains_edge(a, b) + for &(a, b) in flat_results_disconnected { + if ambiguous_with_flattened.contains_edge(a, b) || self.ambiguous_with_all.contains(&a) || self.ambiguous_with_all.contains(&b) { @@ -1070,18 +1050,15 @@ impl ScheduleGraph { } } - if self.settings.ambiguity_detection != LogLevel::Ignore - && self.contains_conflicts(&conflicting_systems) - { - self.report_conflicts(&conflicting_systems, components); - if matches!(self.settings.ambiguity_detection, LogLevel::Error) { - return Err(ScheduleBuildError::Ambiguity); - } - } - self.conflicting_systems = conflicting_systems; + conflicting_systems + } - // build the schedule - let dg_system_ids = self.dependency_flattened.topsort.clone(); + fn build_schedule_inner( + &self, + dependency_flattened_dag: Dag, + hier_results_reachable: FixedBitSet, + ) -> SystemSchedule { + let dg_system_ids = dependency_flattened_dag.topsort.clone(); let dg_system_idx_map = dg_system_ids .iter() .cloned() @@ -1120,14 +1097,12 @@ impl ScheduleGraph { let mut system_dependencies = Vec::with_capacity(sys_count); let mut system_dependents = Vec::with_capacity(sys_count); for &sys_id in &dg_system_ids { - let num_dependencies = self - .dependency_flattened + let num_dependencies = dependency_flattened_dag .graph .neighbors_directed(sys_id, Direction::Incoming) .count(); - let dependents = self - .dependency_flattened + let dependents = dependency_flattened_dag .graph .neighbors_directed(sys_id, Direction::Outgoing) .map(|dep_id| dg_system_idx_map[&dep_id]) @@ -1145,7 +1120,7 @@ impl ScheduleGraph { let bitset = &mut systems_in_sets_with_conditions[i]; for &(col, sys_id) in &hg_systems { let idx = dg_system_idx_map[&sys_id]; - let is_descendant = hier_results.reachable[index(row, col, hg_node_count)]; + let is_descendant = hier_results_reachable[index(row, col, hg_node_count)]; bitset.set(idx, is_descendant); } } @@ -1160,12 +1135,12 @@ impl ScheduleGraph { .enumerate() .take_while(|&(_idx, &row)| row < col) { - let is_ancestor = hier_results.reachable[index(row, col, hg_node_count)]; + let is_ancestor = hier_results_reachable[index(row, col, hg_node_count)]; bitset.set(idx, is_ancestor); } } - Ok(SystemSchedule { + SystemSchedule { systems: Vec::with_capacity(sys_count), system_conditions: Vec::with_capacity(sys_count), set_conditions: Vec::with_capacity(set_with_conditions_count), @@ -1175,7 +1150,7 @@ impl ScheduleGraph { system_dependents, sets_with_conditions_of_systems, systems_in_sets_with_conditions, - }) + } } fn update_schedule( @@ -1292,20 +1267,36 @@ impl ScheduleGraph { } } - fn contains_hierarchy_conflicts(&self, transitive_edges: &[(NodeId, NodeId)]) -> bool { - if transitive_edges.is_empty() { - return false; + /// If [`ScheduleBuildSettings::hierarchy_detection`] is [`LogLevel::Ignore`] this check + /// is skipped. + fn optionally_check_hierarchy_conflicts( + &self, + transitive_edges: &[(NodeId, NodeId)], + ) -> Result<(), ScheduleBuildError> { + if self.settings.hierarchy_detection == LogLevel::Ignore || transitive_edges.is_empty() { + return Ok(()); } - true + let message = self.get_hierarchy_conflicts_error_message(transitive_edges); + match self.settings.hierarchy_detection { + LogLevel::Ignore => unreachable!(), + LogLevel::Warn => { + error!("{}", message); + Ok(()) + } + LogLevel::Error => Err(ScheduleBuildError::HierarchyRedundancy(message)), + } } - fn report_hierarchy_conflicts(&self, transitive_edges: &[(NodeId, NodeId)]) { + fn get_hierarchy_conflicts_error_message( + &self, + transitive_edges: &[(NodeId, NodeId)], + ) -> String { let mut message = String::from("hierarchy contains redundant edge(s)"); for (parent, child) in transitive_edges { writeln!( message, - " -- {:?} '{:?}' cannot be child of set '{:?}', longer path exists", + " -- {} `{}` cannot be child of set `{}`, longer path exists", self.get_node_kind(child), self.get_node_name(child), self.get_node_name(parent), @@ -1313,7 +1304,7 @@ impl ScheduleGraph { .unwrap(); } - error!("{}", message); + message } /// Tries to topologically sort `graph`. @@ -1329,7 +1320,7 @@ impl ScheduleGraph { &self, graph: &DiGraphMap, report: ReportCycles, - ) -> Result, Vec>> { + ) -> Result, ScheduleBuildError> { // Tarjan's SCC algorithm returns elements in *reverse* topological order. let mut tarjan_scc = TarjanScc::new(); let mut top_sorted_nodes = Vec::with_capacity(graph.node_count()); @@ -1355,39 +1346,43 @@ impl ScheduleGraph { cycles.append(&mut simple_cycles_in_component(graph, scc)); } - match report { - ReportCycles::Hierarchy => self.report_hierarchy_cycles(&cycles), - ReportCycles::Dependency => self.report_dependency_cycles(&cycles), - } + let error = match report { + ReportCycles::Hierarchy => ScheduleBuildError::HierarchyCycle( + self.get_hierarchy_cycles_error_message(&cycles), + ), + ReportCycles::Dependency => ScheduleBuildError::DependencyCycle( + self.get_dependency_cycles_error_message(&cycles), + ), + }; - Err(sccs_with_cycles) + Err(error) } } /// Logs details of cycles in the hierarchy graph. - fn report_hierarchy_cycles(&self, cycles: &[Vec]) { + fn get_hierarchy_cycles_error_message(&self, cycles: &[Vec]) -> String { let mut message = format!("schedule has {} in_set cycle(s):\n", cycles.len()); for (i, cycle) in cycles.iter().enumerate() { let mut names = cycle.iter().map(|id| self.get_node_name(id)); let first_name = names.next().unwrap(); writeln!( message, - "cycle {}: set '{first_name}' contains itself", + "cycle {}: set `{first_name}` contains itself", i + 1, ) .unwrap(); - writeln!(message, "set '{first_name}'").unwrap(); + writeln!(message, "set `{first_name}`").unwrap(); for name in names.chain(std::iter::once(first_name)) { - writeln!(message, " ... which contains set '{name}'").unwrap(); + writeln!(message, " ... which contains set `{name}`").unwrap(); } writeln!(message).unwrap(); } - error!("{}", message); + message } /// Logs details of cycles in the dependency graph. - fn report_dependency_cycles(&self, cycles: &[Vec]) { + fn get_dependency_cycles_error_message(&self, cycles: &[Vec]) -> String { let mut message = format!("schedule has {} before/after cycle(s):\n", cycles.len()); for (i, cycle) in cycles.iter().enumerate() { let mut names = cycle @@ -1396,36 +1391,119 @@ impl ScheduleGraph { let (first_kind, first_name) = names.next().unwrap(); writeln!( message, - "cycle {}: {first_kind} '{first_name}' must run before itself", + "cycle {}: {first_kind} `{first_name}` must run before itself", i + 1, ) .unwrap(); - writeln!(message, "{first_kind} '{first_name}'").unwrap(); + writeln!(message, "{first_kind} `{first_name}`").unwrap(); for (kind, name) in names.chain(std::iter::once((first_kind, first_name))) { - writeln!(message, " ... which must run before {kind} '{name}'").unwrap(); + writeln!(message, " ... which must run before {kind} `{name}`").unwrap(); } writeln!(message).unwrap(); } - error!("{}", message); + message } - fn contains_conflicts(&self, conflicts: &[(NodeId, NodeId, Vec)]) -> bool { - if conflicts.is_empty() { - return false; + fn check_for_cross_dependencies( + &self, + dep_results: &CheckGraphResults, + hier_results_connected: &HashSet<(NodeId, NodeId)>, + ) -> Result<(), ScheduleBuildError> { + for &(a, b) in dep_results.connected.iter() { + if hier_results_connected.contains(&(a, b)) || hier_results_connected.contains(&(b, a)) + { + let name_a = self.get_node_name(&a); + let name_b = self.get_node_name(&b); + return Err(ScheduleBuildError::CrossDependency(name_a, name_b)); + } } - true + Ok(()) + } + + fn check_order_but_intersect( + &self, + dep_results_connected: &HashSet<(NodeId, NodeId)>, + set_system_bitsets: &HashMap, + ) -> Result<(), ScheduleBuildError> { + // check that there is no ordering between system sets that intersect + for (a, b) in dep_results_connected.iter() { + if !(a.is_set() && b.is_set()) { + continue; + } + + let a_systems = set_system_bitsets.get(a).unwrap(); + let b_systems = set_system_bitsets.get(b).unwrap(); + + if !(a_systems.is_disjoint(b_systems)) { + return Err(ScheduleBuildError::SetsHaveOrderButIntersect( + self.get_node_name(a), + self.get_node_name(b), + )); + } + } + + Ok(()) + } + + fn check_system_type_set_ambiguity( + &self, + set_systems: &HashMap>, + ) -> Result<(), ScheduleBuildError> { + for (&id, systems) in set_systems.iter() { + let set = &self.system_sets[id.index()]; + if set.is_system_type() { + let instances = systems.len(); + let ambiguous_with = self.ambiguous_with.edges(id); + let before = self + .dependency + .graph + .edges_directed(id, Direction::Incoming); + let after = self + .dependency + .graph + .edges_directed(id, Direction::Outgoing); + let relations = before.count() + after.count() + ambiguous_with.count(); + if instances > 1 && relations > 0 { + return Err(ScheduleBuildError::SystemTypeSetAmbiguity( + self.get_node_name(&id), + )); + } + } + } + Ok(()) + } + + /// if [`ScheduleBuildSettings::ambiguity_detection`] is [`LogLevel::Ignore`], this check is skipped + fn optionally_check_conflicts( + &self, + conflicts: &[(NodeId, NodeId, Vec)], + components: &Components, + ) -> Result<(), ScheduleBuildError> { + if self.settings.ambiguity_detection == LogLevel::Ignore || conflicts.is_empty() { + return Ok(()); + } + + let message = self.get_conflicts_error_message(conflicts, components); + match self.settings.ambiguity_detection { + LogLevel::Ignore => Ok(()), + LogLevel::Warn => { + warn!("{}", message); + Ok(()) + } + LogLevel::Error => Err(ScheduleBuildError::Ambiguity(message)), + } } - fn report_conflicts( + fn get_conflicts_error_message( &self, ambiguities: &[(NodeId, NodeId, Vec)], components: &Components, - ) { + ) -> String { let n_ambiguities = ambiguities.len(); - let mut string = format!( + let mut message = format!( "{n_ambiguities} pairs of systems with conflicting data access have indeterminate execution order. \ Consider adding `before`, `after`, or `ambiguous_with` relationships between these:\n", ); @@ -1437,22 +1515,22 @@ impl ScheduleGraph { debug_assert!(system_a.is_system(), "{name_a} is not a system."); debug_assert!(system_b.is_system(), "{name_b} is not a system."); - writeln!(string, " -- {name_a} and {name_b}").unwrap(); + 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!(string, " conflict on: {conflict_names:?}").unwrap(); + writeln!(message, " conflict on: {conflict_names:?}").unwrap(); } else { // one or both systems must be exclusive let world = std::any::type_name::(); - writeln!(string, " conflict on: {world}").unwrap(); + writeln!(message, " conflict on: {world}").unwrap(); } } - warn!("{}", string); + message } fn traverse_sets_containing_node(&self, id: NodeId, f: &mut impl FnMut(NodeId) -> bool) { @@ -1485,33 +1563,33 @@ pub enum ScheduleBuildError { #[error("`{0:?}` contains itself.")] HierarchyLoop(String), /// The hierarchy of system sets contains a cycle. - #[error("System set hierarchy contains cycle(s).")] - HierarchyCycle, + #[error("System set hierarchy contains cycle(s).\n{0}")] + HierarchyCycle(String), /// The hierarchy of system sets contains redundant edges. /// /// This error is disabled by default, but can be opted-in using [`ScheduleBuildSettings`]. - #[error("System set hierarchy contains redundant edges.")] - HierarchyRedundancy, + #[error("System set hierarchy contains redundant edges.\n{0}")] + HierarchyRedundancy(String), /// A system (set) has been told to run before itself. #[error("`{0:?}` depends on itself.")] DependencyLoop(String), /// The dependency graph contains a cycle. - #[error("System dependencies contain cycle(s).")] - DependencyCycle, + #[error("System dependencies contain cycle(s).\n{0}")] + DependencyCycle(String), /// Tried to order a system (set) relative to a system set it belongs to. - #[error("`{0:?}` and `{1:?}` have both `in_set` and `before`-`after` relationships (these might be transitive). This combination is unsolvable as a system cannot run before or after a set it belongs to.")] + #[error("`{0}` and `{1}` have both `in_set` and `before`-`after` relationships (these might be transitive). This combination is unsolvable as a system cannot run before or after a set it belongs to.")] CrossDependency(String, String), /// Tried to order system sets that share systems. - #[error("`{0:?}` and `{1:?}` have a `before`-`after` relationship (which may be transitive) but share systems.")] + #[error("`{0}` and `{1}` have a `before`-`after` relationship (which may be transitive) but share systems.")] SetsHaveOrderButIntersect(String, String), /// Tried to order a system (set) relative to all instances of some system function. - #[error("Tried to order against `fn {0:?}` in a schedule that has more than one `{0:?}` instance. `fn {0:?}` is a `SystemTypeSet` and cannot be used for ordering if ambiguous. Use a different set without this restriction.")] + #[error("Tried to order against `{0}` in a schedule that has more than one `{0}` instance. `{0}` is a `SystemTypeSet` and cannot be used for ordering if ambiguous. Use a different set without this restriction.")] SystemTypeSetAmbiguity(String), /// Systems with conflicting access have indeterminate run order. /// /// This error is disabled by default, but can be opted-in using [`ScheduleBuildSettings`]. - #[error("Systems with conflicting access have indeterminate run order.")] - Ambiguity, + #[error("Systems with conflicting access have indeterminate run order.\n{0}")] + Ambiguity(String), /// Tried to run a schedule before all of its systems have been initialized. #[error("Systems in schedule have not been initialized.")] Uninitialized, diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index 6e0caaba0937c..1277f4b54dc2b 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -71,7 +71,7 @@ impl SystemTypeSet { impl Debug for SystemTypeSet { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_tuple("SystemTypeSet") - .field(&std::any::type_name::()) + .field(&format_args!("fn {}()", &std::any::type_name::())) .finish() } }