From cbdac43a6b98345dd1f437718ff257e05b1a58df Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Thu, 24 Aug 2023 21:37:56 -0700 Subject: [PATCH 1/3] add track caller to configure sets and configure set --- crates/bevy_app/src/app.rs | 2 ++ crates/bevy_ecs/src/schedule/config.rs | 2 ++ crates/bevy_ecs/src/schedule/schedule.rs | 5 +++++ 3 files changed, 9 insertions(+) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 39ed373a785e2..2bc076d205f12 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -393,6 +393,7 @@ impl App { } /// Configures a system set in the default schedule, adding the set if it does not exist. + #[track_caller] pub fn configure_set( &mut self, schedule: impl ScheduleLabel, @@ -410,6 +411,7 @@ impl App { } /// Configures a collection of system sets in the default schedule, adding any sets that do not exist. + #[track_caller] pub fn configure_sets( &mut self, schedule: impl ScheduleLabel, diff --git a/crates/bevy_ecs/src/schedule/config.rs b/crates/bevy_ecs/src/schedule/config.rs index 708661035ed55..43750967ddabd 100644 --- a/crates/bevy_ecs/src/schedule/config.rs +++ b/crates/bevy_ecs/src/schedule/config.rs @@ -388,6 +388,7 @@ pub struct SystemSetConfig { } impl SystemSetConfig { + #[track_caller] fn new(set: BoxedSystemSet) -> Self { // system type sets are automatically populated // to avoid unintentionally broad changes, they cannot be configured @@ -444,6 +445,7 @@ pub trait IntoSystemSetConfig: Sized { } impl IntoSystemSetConfig for S { + #[track_caller] fn into_config(self) -> SystemSetConfig { SystemSetConfig::new(Box::new(self)) } diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index cb9ab0a978335..cb64160c6792e 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -181,12 +181,14 @@ impl Schedule { } /// Configures a system set in this schedule, adding it if it does not exist. + #[track_caller] pub fn configure_set(&mut self, set: impl IntoSystemSetConfig) -> &mut Self { self.graph.configure_set(set); self } /// Configures a collection of system sets in this schedule, adding them if they does not exist. + #[track_caller] pub fn configure_sets(&mut self, sets: impl IntoSystemSetConfigs) -> &mut Self { self.graph.configure_sets(sets); self @@ -660,6 +662,7 @@ impl ScheduleGraph { Ok(id) } + #[track_caller] fn configure_sets(&mut self, sets: impl IntoSystemSetConfigs) { let SystemSetConfigs { sets, chained } = sets.into_configs(); let mut set_iter = sets.into_iter(); @@ -678,10 +681,12 @@ impl ScheduleGraph { } } + #[track_caller] fn configure_set(&mut self, set: impl IntoSystemSetConfig) { self.configure_set_inner(set).unwrap(); } + #[track_caller] fn configure_set_inner( &mut self, set: impl IntoSystemSetConfig, From b054457f717f724a819f25431d58261c3787f78f Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Fri, 25 Aug 2023 12:44:17 -0700 Subject: [PATCH 2/3] clean up error messages --- crates/bevy_ecs/src/schedule/schedule.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index cb64160c6792e..31b066cfb4734 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -676,14 +676,14 @@ impl ScheduleGraph { } } else { for set in set_iter { - self.configure_set_inner(set).unwrap(); + self.configure_set_inner(set).unwrap_or_else(|e| panic!("{e}")); } } } #[track_caller] fn configure_set(&mut self, set: impl IntoSystemSetConfig) { - self.configure_set_inner(set).unwrap(); + self.configure_set_inner(set).unwrap_or_else(|e| panic!("{e}")); } #[track_caller] @@ -1487,7 +1487,7 @@ impl ScheduleGraph { #[non_exhaustive] pub enum ScheduleBuildError { /// A system set contains itself. - #[error("`{0:?}` contains itself.")] + #[error("`System set {0}` contains itself.")] HierarchyLoop(String), /// The hierarchy of system sets contains a cycle. #[error("System set hierarchy contains cycle(s).")] @@ -1498,7 +1498,7 @@ pub enum ScheduleBuildError { #[error("System set hierarchy contains redundant edges.")] HierarchyRedundancy, /// A system (set) has been told to run before itself. - #[error("`{0:?}` depends on itself.")] + #[error("`System set {0}` depends on itself.")] DependencyLoop(String), /// The dependency graph contains a cycle. #[error("System dependencies contain cycle(s).")] From 8a96412710c9da2085143b4591e1e914720aca71 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Fri, 25 Aug 2023 13:17:14 -0700 Subject: [PATCH 3/3] clean up error messages a bit more --- crates/bevy_ecs/src/schedule/schedule.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 31b066cfb4734..9e081bfa45b56 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -552,8 +552,8 @@ impl ScheduleGraph { let Some(prev) = config_iter.next() else { return AddSystemsInnerResult { nodes: Vec::new(), - densely_chained: true - } + densely_chained: true, + }; }; let mut previous_result = self.add_systems_inner(prev, true); densely_chained = previous_result.densely_chained; @@ -676,14 +676,22 @@ impl ScheduleGraph { } } else { for set in set_iter { - self.configure_set_inner(set).unwrap_or_else(|e| panic!("{e}")); + if let Err(e) = self.configure_set_inner(set) { + // using `unwrap_or_else(panic!)` led to the error being reported + // from this line instead of in the user code + panic!("{e}"); + }; } } } #[track_caller] fn configure_set(&mut self, set: impl IntoSystemSetConfig) { - self.configure_set_inner(set).unwrap_or_else(|e| panic!("{e}")); + if let Err(e) = self.configure_set_inner(set) { + // using `unwrap_or_else(panic!)` led to the error being reported + // from this line instead of in the user code + panic!("{e}"); + }; } #[track_caller] @@ -1487,7 +1495,7 @@ impl ScheduleGraph { #[non_exhaustive] pub enum ScheduleBuildError { /// A system set contains itself. - #[error("`System set {0}` contains itself.")] + #[error("System set `{0}` contains itself.")] HierarchyLoop(String), /// The hierarchy of system sets contains a cycle. #[error("System set hierarchy contains cycle(s).")] @@ -1498,7 +1506,7 @@ pub enum ScheduleBuildError { #[error("System set hierarchy contains redundant edges.")] HierarchyRedundancy, /// A system (set) has been told to run before itself. - #[error("`System set {0}` depends on itself.")] + #[error("System set `{0}` depends on itself.")] DependencyLoop(String), /// The dependency graph contains a cycle. #[error("System dependencies contain cycle(s).")]