Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Schedule build errors should include the schedule in which the problem occurred when reporting problems #9510

Closed
alice-i-cecile opened this issue Aug 20, 2023 · 1 comment · Fixed by #9600
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@alice-i-cecile
Copy link
Member

What problem does this solve or what need does it fill?

When errors are reported during schedule building, the schedule name is not included.

It would be helpful to be able to track down the exact source of the problem, especially since that information.

What solution would you like?

Currently:

2023-08-20T17:23:10.908705Z  WARN bevy_ecs::schedule::schedule: 1 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these:
 -- update_action_state<PlayerAction> (in set Update) and ui_focus_system (in set Focus)
    conflict on: ["bevy_ui::focus::Interaction"]

In the preamble to this reporting, we should report the schedule name if it is available.

Additional context

We won't always be able to report the schedule name, since this information is stored in Schedules, not on each individual Schedule. In theory I suppose we could change that though.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Aug 20, 2023
@hymm
Copy link
Contributor

hymm commented Aug 20, 2023

I think we should change Schedule to store the name. I had a branch with the change, but it needs updating.

github-merge-queue bot pushed a commit that referenced this issue Aug 28, 2023
# Objective

- Move schedule name into `Schedule` to allow the schedule name to be
used for errors and tracing in Schedule methods
- Fixes #9510

## Solution

- Move label onto `Schedule` and adjust api's on `World` and `Schedule`
to not pass explicit label where it makes sense to.
- add name to errors and tracing.
- `Schedule::new` now takes a label so either add the label or use
`Schedule::default` which uses a default label. `default` is mostly used
in doc examples and tests.

---

## Changelog

- move label onto `Schedule` to improve error message and logging for
schedules.

## Migration Guide

`Schedule::new` and `App::add_schedule`
```rust
// old
let schedule = Schedule::new();
app.add_schedule(MyLabel, schedule);

// new
let schedule = Schedule::new(MyLabel);
app.add_schedule(schedule);
```

if you aren't using a label and are using the schedule struct directly
you can use the default constructor.
```rust
// old
let schedule = Schedule::new();
schedule.run(world);

// new
let schedule = Schedule::default();
schedule.run(world);
```

`Schedules:insert`
```rust
// old
let schedule = Schedule::new();
schedules.insert(MyLabel, schedule);

// new
let schedule = Schedule::new(MyLabel);
schedules.insert(schedule);
```

`World::add_schedule`
```rust
// old
let schedule = Schedule::new();
world.add_schedule(MyLabel, schedule);

// new
let schedule = Schedule::new(MyLabel);
world.add_schedule(schedule);
```
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this issue Jan 9, 2024
# Objective

- Move schedule name into `Schedule` to allow the schedule name to be
used for errors and tracing in Schedule methods
- Fixes bevyengine#9510

## Solution

- Move label onto `Schedule` and adjust api's on `World` and `Schedule`
to not pass explicit label where it makes sense to.
- add name to errors and tracing.
- `Schedule::new` now takes a label so either add the label or use
`Schedule::default` which uses a default label. `default` is mostly used
in doc examples and tests.

---

## Changelog

- move label onto `Schedule` to improve error message and logging for
schedules.

## Migration Guide

`Schedule::new` and `App::add_schedule`
```rust
// old
let schedule = Schedule::new();
app.add_schedule(MyLabel, schedule);

// new
let schedule = Schedule::new(MyLabel);
app.add_schedule(schedule);
```

if you aren't using a label and are using the schedule struct directly
you can use the default constructor.
```rust
// old
let schedule = Schedule::new();
schedule.run(world);

// new
let schedule = Schedule::default();
schedule.run(world);
```

`Schedules:insert`
```rust
// old
let schedule = Schedule::new();
schedules.insert(MyLabel, schedule);

// new
let schedule = Schedule::new(MyLabel);
schedules.insert(schedule);
```

`World::add_schedule`
```rust
// old
let schedule = Schedule::new();
world.add_schedule(MyLabel, schedule);

// new
let schedule = Schedule::new(MyLabel);
world.add_schedule(schedule);
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants