Skip to content

Commit

Permalink
clean up configure_set(s) erroring (bevyengine#9577)
Browse files Browse the repository at this point in the history
# Objective

- have errors in configure_set and configure_sets show the line number
of the user calling location rather than pointing to schedule.rs
- use display formatting for the errors

## Example Error Text
```text
// dependency loop
// before
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: DependencyLoop("A")', crates\bevy_ecs\src\schedule\schedule.rs:682:39
// after
thread 'main' panicked at 'System set `A` depends on itself.', examples/stress_tests/bevymark.rs:16:9

// hierarchy loop
// before
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: HierarchyLoop("A")', crates\bevy_ecs\src\schedule\schedule.rs:682:3
// after
thread 'main' panicked at 'System set `A` contains itself.', examples/stress_tests/bevymark.rs:16:9

// configuring a system type set
// before
thread 'main' panicked at 'configuring system type sets is not allowed', crates\bevy_ecs\src\schedule\config.rs:394:9
//after
thread 'main' panicked at 'configuring system type sets is not allowed', examples/stress_tests/bevymark.rs:16:9
```

Code to produce errors:
```rust
use bevy::prelude::*;

#[derive(SystemSet, Clone, Debug, PartialEq, Eq, Hash)]
enum TestSet {
    A,
}

fn main() {
    fn foo() {}
    let mut app = App::empty();
    // Hierarchy Loop
    app.configure_set(Main, TestSet::A.in_set(TestSet::A));
    // Dependency Loop
    app.configure_set(Main, TestSet::A.after(TestSet::A));
    // Configure System Type Set
    app.configure_set(Main, foo.into_system_set());
}
```
  • Loading branch information
hymm authored and Ray Redondo committed Jan 9, 2024
1 parent dec00e2 commit 813971c
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 4 deletions.
2 changes: 2 additions & 0 deletions crates/bevy_app/src/app.rs
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions crates/bevy_ecs/src/schedule/config.rs
Expand Up @@ -393,6 +393,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
Expand Down Expand Up @@ -449,6 +450,7 @@ pub trait IntoSystemSetConfig: Sized {
}

impl<S: SystemSet> IntoSystemSetConfig for S {
#[track_caller]
fn into_config(self) -> SystemSetConfig {
SystemSetConfig::new(Box::new(self))
}
Expand Down
21 changes: 17 additions & 4 deletions crates/bevy_ecs/src/schedule/schedule.rs
Expand Up @@ -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
Expand Down Expand Up @@ -656,6 +658,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();
Expand All @@ -669,15 +672,25 @@ impl ScheduleGraph {
}
} else {
for set in set_iter {
self.configure_set_inner(set).unwrap();
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();
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_inner(
&mut self,
set: impl IntoSystemSetConfig,
Expand Down Expand Up @@ -1560,7 +1573,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).\n{0}")]
Expand All @@ -1571,7 +1584,7 @@ pub enum ScheduleBuildError {
#[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.")]
#[error("System set `{0}` depends on itself.")]
DependencyLoop(String),
/// The dependency graph contains a cycle.
#[error("System dependencies contain cycle(s).\n{0}")]
Expand Down

0 comments on commit 813971c

Please sign in to comment.