bevy/crates/bevy_ecs/src/system/adapter_system.rs
Chris Russell 5d1fe16bfd
Fix run_system for adapter systems wrapping exclusive systems (#18406)
# Objective

Fix panic in `run_system` when running an exclusive system wrapped in a
`PipeSystem` or `AdapterSystem`.

#18076 introduced a `System::run_without_applying_deferred` method. It
normally calls `System::run_unsafe`, but
`ExclusiveFunctionSystem::run_unsafe` panics, so it was overridden for
that type. Unfortunately, `PipeSystem::run_without_applying_deferred`
still calls `PipeSystem::run_unsafe`, which can then call
`ExclusiveFunctionSystem::run_unsafe` and panic.

## Solution

Make `ExclusiveFunctionSystem::run_unsafe` work instead of panicking.
Clarify the safety requirements that make this sound.

The alternative is to override `run_without_applying_deferred` in
`PipeSystem`, `CombinatorSystem`, `AdapterSystem`,
`InfallibleSystemWrapper`, and `InfallibleObserverWrapper`. That seems
like a lot of extra code just to preserve a confusing special case!

Remove some implementations of `System::run` that are no longer
necessary with this change. This slightly changes the behavior of
`PipeSystem` and `CombinatorSystem`: Currently `run` will call
`apply_deferred` on the first system before running the second, but
after this change it will only call it after *both* systems have run.
The new behavior is consistent with `run_unsafe` and
`run_without_applying_deferred`, and restores the behavior prior to
#11823.

The panic was originally necessary because [`run_unsafe` took
`&World`](https://github.com/bevyengine/bevy/pull/6083/files#diff-708dfc60ec5eef432b20a6f471357a7ea9bfb254dc2f918d5ed4a66deb0e85baR90).
Now that it takes `UnsafeWorldCell`, it is possible to make it work. See
also Cart's concerns at
https://github.com/bevyengine/bevy/pull/4166#discussion_r979140356,
although those also predate `UnsafeWorldCell`.

And see #6698 for a previous bug caused by this panic.
2025-03-26 13:40:42 +00:00

234 lines
6.3 KiB
Rust

use alloc::{borrow::Cow, vec::Vec};
use super::{IntoSystem, ReadOnlySystem, System, SystemParamValidationError};
use crate::{
schedule::InternedSystemSet,
system::{input::SystemInput, SystemIn},
world::unsafe_world_cell::UnsafeWorldCell,
};
/// Customizes the behavior of an [`AdapterSystem`]
///
/// # Examples
///
/// ```
/// # use bevy_ecs::prelude::*;
/// use bevy_ecs::system::{Adapt, AdapterSystem};
///
/// // A system adapter that inverts the result of a system.
/// // NOTE: Instead of manually implementing this, you can just use `bevy_ecs::schedule::common_conditions::not`.
/// pub type NotSystem<S> = AdapterSystem<NotMarker, S>;
///
/// // This struct is used to customize the behavior of our adapter.
/// pub struct NotMarker;
///
/// impl<S> Adapt<S> for NotMarker
/// where
/// S: System,
/// S::Out: std::ops::Not,
/// {
/// type In = S::In;
/// type Out = <S::Out as std::ops::Not>::Output;
///
/// fn adapt(
/// &mut self,
/// input: <Self::In as SystemInput>::Inner<'_>,
/// run_system: impl FnOnce(SystemIn<'_, S>) -> S::Out,
/// ) -> Self::Out {
/// !run_system(input)
/// }
/// }
/// # let mut world = World::new();
/// # let mut system = NotSystem::new(NotMarker, IntoSystem::into_system(|| false), "".into());
/// # system.initialize(&mut world);
/// # assert!(system.run((), &mut world));
/// ```
#[diagnostic::on_unimplemented(
message = "`{Self}` can not adapt a system of type `{S}`",
label = "invalid system adapter"
)]
pub trait Adapt<S: System>: Send + Sync + 'static {
/// The [input](System::In) type for an [`AdapterSystem`].
type In: SystemInput;
/// The [output](System::Out) type for an [`AdapterSystem`].
type Out;
/// When used in an [`AdapterSystem`], this function customizes how the system
/// is run and how its inputs/outputs are adapted.
fn adapt(
&mut self,
input: <Self::In as SystemInput>::Inner<'_>,
run_system: impl FnOnce(SystemIn<'_, S>) -> S::Out,
) -> Self::Out;
}
/// An [`IntoSystem`] creating an instance of [`AdapterSystem`].
#[derive(Clone)]
pub struct IntoAdapterSystem<Func, S> {
func: Func,
system: S,
}
impl<Func, S> IntoAdapterSystem<Func, S> {
/// Creates a new [`IntoSystem`] that uses `func` to adapt `system`, via the [`Adapt`] trait.
pub const fn new(func: Func, system: S) -> Self {
Self { func, system }
}
}
#[doc(hidden)]
pub struct IsAdapterSystemMarker;
impl<Func, S, I, O, M> IntoSystem<Func::In, Func::Out, (IsAdapterSystemMarker, I, O, M)>
for IntoAdapterSystem<Func, S>
where
Func: Adapt<S::System>,
I: SystemInput,
S: IntoSystem<I, O, M>,
{
type System = AdapterSystem<Func, S::System>;
// Required method
fn into_system(this: Self) -> Self::System {
let system = IntoSystem::into_system(this.system);
let name = system.name();
AdapterSystem::new(this.func, system, name)
}
}
/// A [`System`] that takes the output of `S` and transforms it by applying `Func` to it.
#[derive(Clone)]
pub struct AdapterSystem<Func, S> {
func: Func,
system: S,
name: Cow<'static, str>,
}
impl<Func, S> AdapterSystem<Func, S>
where
Func: Adapt<S>,
S: System,
{
/// Creates a new [`System`] that uses `func` to adapt `system`, via the [`Adapt`] trait.
pub const fn new(func: Func, system: S, name: Cow<'static, str>) -> Self {
Self { func, system, name }
}
}
impl<Func, S> System for AdapterSystem<Func, S>
where
Func: Adapt<S>,
S: System,
{
type In = Func::In;
type Out = Func::Out;
fn name(&self) -> Cow<'static, str> {
self.name.clone()
}
fn component_access(&self) -> &crate::query::Access<crate::component::ComponentId> {
self.system.component_access()
}
#[inline]
fn archetype_component_access(
&self,
) -> &crate::query::Access<crate::archetype::ArchetypeComponentId> {
self.system.archetype_component_access()
}
fn is_send(&self) -> bool {
self.system.is_send()
}
fn is_exclusive(&self) -> bool {
self.system.is_exclusive()
}
fn has_deferred(&self) -> bool {
self.system.has_deferred()
}
#[inline]
unsafe fn run_unsafe(
&mut self,
input: SystemIn<'_, Self>,
world: UnsafeWorldCell,
) -> Self::Out {
// SAFETY: `system.run_unsafe` has the same invariants as `self.run_unsafe`.
self.func.adapt(input, |input| unsafe {
self.system.run_unsafe(input, world)
})
}
#[inline]
fn apply_deferred(&mut self, world: &mut crate::prelude::World) {
self.system.apply_deferred(world);
}
#[inline]
fn queue_deferred(&mut self, world: crate::world::DeferredWorld) {
self.system.queue_deferred(world);
}
#[inline]
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Delegate to other `System` implementations.
unsafe { self.system.validate_param_unsafe(world) }
}
fn initialize(&mut self, world: &mut crate::prelude::World) {
self.system.initialize(world);
}
#[inline]
fn update_archetype_component_access(&mut self, world: UnsafeWorldCell) {
self.system.update_archetype_component_access(world);
}
fn check_change_tick(&mut self, change_tick: crate::component::Tick) {
self.system.check_change_tick(change_tick);
}
fn default_system_sets(&self) -> Vec<InternedSystemSet> {
self.system.default_system_sets()
}
fn get_last_run(&self) -> crate::component::Tick {
self.system.get_last_run()
}
fn set_last_run(&mut self, last_run: crate::component::Tick) {
self.system.set_last_run(last_run);
}
}
// SAFETY: The inner system is read-only.
unsafe impl<Func, S> ReadOnlySystem for AdapterSystem<Func, S>
where
Func: Adapt<S>,
S: ReadOnlySystem,
{
}
impl<F, S, Out> Adapt<S> for F
where
F: Send + Sync + 'static + FnMut(S::Out) -> Out,
S: System,
{
type In = S::In;
type Out = Out;
fn adapt(
&mut self,
input: <Self::In as SystemInput>::Inner<'_>,
run_system: impl FnOnce(SystemIn<'_, S>) -> S::Out,
) -> Out {
self(run_system(input))
}
}