Immediately apply deferred system params in System::run (#11823)

# Objective
Fixes #11821. 

## Solution
* Run `System::apply_deferred` in `System::run` after executing the
system.
* Switch to using `System::run_unsafe` in `SingleThreadedExecutor` to
preserve the current behavior.
* Remove the `System::apply_deferred` in `SimpleExecutor` as it's now
redundant.
* Remove the `System::apply_deferred` when running one-shot systems, as
it's now redundant.

---

## Changelog
Changed: `System::run` will now immediately apply deferred system params
after running the system.

## Migration Guide
`System::run` will now always run `System::apply_deferred` immediately
after running the system now. If you were running systems and then
applying their deferred buffers at a later point in time, you can
eliminate the latter.

```rust
// in 0.13
system.run(world);
// .. sometime later ...
system.apply_deferred(world);

// in 0.14
system.run(world);
```

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
This commit is contained in:
James Liu 2024-02-24 06:01:06 -08:00 committed by GitHub
parent 9d13ae3113
commit 42e61557f8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 30 additions and 18 deletions

View file

@ -99,8 +99,6 @@ impl SystemExecutor for SimpleExecutor {
eprintln!("Encountered a panic in system `{}`!", &*system.name());
std::panic::resume_unwind(payload);
}
system.apply_deferred(world);
}
self.evaluated_sets.clear();

View file

@ -96,16 +96,26 @@ impl SystemExecutor for SingleThreadedExecutor {
let system = &mut schedule.systems[system_index];
if is_apply_deferred(system) {
self.apply_deferred(schedule, world);
} else {
let res = std::panic::catch_unwind(AssertUnwindSafe(|| {
system.run((), world);
}));
if let Err(payload) = res {
eprintln!("Encountered a panic in system `{}`!", &*system.name());
std::panic::resume_unwind(payload);
}
self.unapplied_systems.insert(system_index);
continue;
}
let res = std::panic::catch_unwind(AssertUnwindSafe(|| {
if system.is_exclusive() {
system.run((), world);
} else {
// Use run_unsafe to avoid immediately applying deferred buffers
let world = world.as_unsafe_world_cell();
system.update_archetype_component_access(world);
// SAFETY: We have exclusive, single-threaded access to the world and
// update_archetype_component_access is being called immediately before this.
unsafe { system.run_unsafe((), world) };
}
}));
if let Err(payload) = res {
eprintln!("Encountered a panic in system `{}`!", &*system.name());
std::panic::resume_unwind(payload);
}
self.unapplied_systems.insert(system_index);
}
if self.apply_final_deferred {

View file

@ -52,6 +52,9 @@ pub trait System: Send + Sync + 'static {
/// can be called in parallel with other systems and may break Rust's aliasing rules
/// if used incorrectly, making it unsafe to call.
///
/// Unlike [`System::run`], this will not apply deferred parameters, which must be independently
/// applied by calling [`System::apply_deferred`] at later point in time.
///
/// # Safety
///
/// - The caller must ensure that `world` has permission to access any world data
@ -66,14 +69,18 @@ pub trait System: Send + Sync + 'static {
///
/// For [read-only](ReadOnlySystem) systems, see [`run_readonly`], which can be called using `&World`.
///
/// Unlike [`System::run_unsafe`], this will apply deferred parameters *immediately*.
///
/// [`run_readonly`]: ReadOnlySystem::run_readonly
fn run(&mut self, input: Self::In, world: &mut World) -> Self::Out {
let world = world.as_unsafe_world_cell();
self.update_archetype_component_access(world);
let world_cell = world.as_unsafe_world_cell();
self.update_archetype_component_access(world_cell);
// SAFETY:
// - We have exclusive access to the entire world.
// - `update_archetype_component_access` has been called.
unsafe { self.run_unsafe(input, world) }
let ret = unsafe { self.run_unsafe(input, world_cell) };
self.apply_deferred(world);
ret
}
/// Applies any [`Deferred`](crate::system::Deferred) system parameters (or other system buffers) of this system to the world.
@ -283,9 +290,7 @@ impl RunSystemOnce for &mut World {
) -> Out {
let mut system: T::System = IntoSystem::into_system(system);
system.initialize(self);
let out = system.run(input, self);
system.apply_deferred(self);
out
system.run(input, self)
}
}

View file

@ -284,7 +284,6 @@ impl World {
initialized = true;
}
let result = system.run(input, self);
system.apply_deferred(self);
// return ownership of system trait object (if entity still exists)
if let Some(mut entity) = self.get_entity_mut(id.0) {