[Merged by Bors] - Fix PipeSystem panicking with exclusive systems #6698
Conversation
inodentry
added
C-Bug
|
Sorry if this is a dumb question, but does run taking |
|
No. There is the That tells bevy how to set up the system descriptors and where to place the system in the schedule. This PR has no effect on that. It simply makes sure that PipeSystem will behave correctly if it happens to be exclusive. Exclusive systems and parallel systems are ran in different ways. Exclusive systems (and parallel systems that the user wants to treat as exclusive, via Regular parallel systems will typically be run by the parallel system executor algorithm in the middle of the stage. It will verify the invariants (ensure safe non-overlapping data access) for the system and call it via Regular parallel systems can be called via either |
alice-i-cecile
added
the
S-Ready-For-Final-Review
|
bors r+ |
Without this fix, piped systems containing exclusive systems fail to run, giving a runtime panic.
With this PR, running piped systems that contain exclusive systems now works.
## Explanation of the bug
This is because, unless overridden, the default implementation of `run` from the `System` trait simply calls `run_unsafe`. That is not valid for exclusive systems. They must always be called via `run`, as `run_unsafe` takes `&World` instead of `&mut World`.
Trivial reproduction example:
```rust
fn main() {
App::new()
.add_plugins(DefaultPlugins)
.add_system(exclusive.pipe(another))
.run();
}
fn exclusive(_world: &mut World) {}
fn another() {}
```
If you run this, you will get a panic 'Cannot run exclusive systems with a shared World reference' and the backtrace shows how bevy (correctly) tries to call the `run` method (because the system is exclusive), but it is the implementation from the `System` trait (because `PipeSystem` does not have its own), which calls `run_unsafe` (incorrect):
- 3: <bevy_ecs::system::system_piping::PipeSystem<SystemA,SystemB> as bevy_ecs::system::system::System>::run_unsafe
- 4: bevy_ecs::system::system::System::run
bors
bot
changed the title
Without this fix, piped systems containing exclusive systems fail to run, giving a runtime panic.
With this PR, running piped systems that contain exclusive systems now works.
## Explanation of the bug
This is because, unless overridden, the default implementation of `run` from the `System` trait simply calls `run_unsafe`. That is not valid for exclusive systems. They must always be called via `run`, as `run_unsafe` takes `&World` instead of `&mut World`.
Trivial reproduction example:
```rust
fn main() {
App::new()
.add_plugins(DefaultPlugins)
.add_system(exclusive.pipe(another))
.run();
}
fn exclusive(_world: &mut World) {}
fn another() {}
```
If you run this, you will get a panic 'Cannot run exclusive systems with a shared World reference' and the backtrace shows how bevy (correctly) tries to call the `run` method (because the system is exclusive), but it is the implementation from the `System` trait (because `PipeSystem` does not have its own), which calls `run_unsafe` (incorrect):
- 3: <bevy_ecs::system::system_piping::PipeSystem<SystemA,SystemB> as bevy_ecs::system::system::System>::run_unsafe
- 4: bevy_ecs::system::system::System::run
Without this fix, piped systems containing exclusive systems fail to run, giving a runtime panic.
With this PR, running piped systems that contain exclusive systems now works.
## Explanation of the bug
This is because, unless overridden, the default implementation of `run` from the `System` trait simply calls `run_unsafe`. That is not valid for exclusive systems. They must always be called via `run`, as `run_unsafe` takes `&World` instead of `&mut World`.
Trivial reproduction example:
```rust
fn main() {
App::new()
.add_plugins(DefaultPlugins)
.add_system(exclusive.pipe(another))
.run();
}
fn exclusive(_world: &mut World) {}
fn another() {}
```
If you run this, you will get a panic 'Cannot run exclusive systems with a shared World reference' and the backtrace shows how bevy (correctly) tries to call the `run` method (because the system is exclusive), but it is the implementation from the `System` trait (because `PipeSystem` does not have its own), which calls `run_unsafe` (incorrect):
- 3: <bevy_ecs::system::system_piping::PipeSystem<SystemA,SystemB> as bevy_ecs::system::system::System>::run_unsafe
- 4: bevy_ecs::system::system::System::run
Without this fix, piped systems containing exclusive systems fail to run, giving a runtime panic.
With this PR, running piped systems that contain exclusive systems now works.
Explanation of the bug
This is because, unless overridden, the default implementation of
runfrom theSystemtrait simply callsrun_unsafe. That is not valid for exclusive systems. They must always be called viarun, asrun_unsafetakes&Worldinstead of&mut World.Trivial reproduction example:
If you run this, you will get a panic 'Cannot run exclusive systems with a shared World reference' and the backtrace shows how bevy (correctly) tries to call the
runmethod (because the system is exclusive), but it is the implementation from theSystemtrait (becausePipeSystemdoes not have its own), which callsrun_unsafe(incorrect):