bevy/crates/bevy_ecs/src/system/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

476 lines
18 KiB
Rust

#![expect(
clippy::module_inception,
reason = "This instance of module inception is being discussed; see #17353."
)]
use core::fmt::Debug;
use log::warn;
use thiserror::Error;
use crate::{
archetype::ArchetypeComponentId,
component::{ComponentId, Tick},
query::Access,
schedule::InternedSystemSet,
system::{input::SystemInput, SystemIn},
world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World},
};
use alloc::{borrow::Cow, boxed::Box, vec::Vec};
use core::any::TypeId;
use super::{IntoSystem, SystemParamValidationError};
/// An ECS system that can be added to a [`Schedule`](crate::schedule::Schedule)
///
/// Systems are functions with all arguments implementing
/// [`SystemParam`](crate::system::SystemParam).
///
/// Systems are added to an application using `App::add_systems(Update, my_system)`
/// or similar methods, and will generally run once per pass of the main loop.
///
/// Systems are executed in parallel, in opportunistic order; data access is managed automatically.
/// It's possible to specify explicit execution order between specific systems,
/// see [`IntoScheduleConfigs`](crate::schedule::IntoScheduleConfigs).
#[diagnostic::on_unimplemented(message = "`{Self}` is not a system", label = "invalid system")]
pub trait System: Send + Sync + 'static {
/// The system's input.
type In: SystemInput;
/// The system's output.
type Out;
/// Returns the system's name.
fn name(&self) -> Cow<'static, str>;
/// Returns the [`TypeId`] of the underlying system type.
#[inline]
fn type_id(&self) -> TypeId {
TypeId::of::<Self>()
}
/// Returns the system's component [`Access`].
fn component_access(&self) -> &Access<ComponentId>;
/// Returns the system's archetype component [`Access`].
fn archetype_component_access(&self) -> &Access<ArchetypeComponentId>;
/// Returns true if the system is [`Send`].
fn is_send(&self) -> bool;
/// Returns true if the system must be run exclusively.
fn is_exclusive(&self) -> bool;
/// Returns true if system has deferred buffers.
fn has_deferred(&self) -> bool;
/// Runs the system with the given input in the world. Unlike [`System::run`], this function
/// 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`](UnsafeWorldCell) has permission to access any world data
/// registered in `archetype_component_access`. There must be no conflicting
/// simultaneous accesses while the system is running.
/// - If [`System::is_exclusive`] returns `true`, then it must be valid to call
/// [`UnsafeWorldCell::world_mut`] on `world`.
/// - The method [`System::update_archetype_component_access`] must be called at some
/// point before this one, with the same exact [`World`]. If [`System::update_archetype_component_access`]
/// panics (or otherwise does not return for any reason), this method must not be called.
unsafe fn run_unsafe(&mut self, input: SystemIn<'_, Self>, world: UnsafeWorldCell)
-> Self::Out;
/// Runs the system with the given input in the world.
///
/// 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: SystemIn<'_, Self>, world: &mut World) -> Self::Out {
let ret = self.run_without_applying_deferred(input, world);
self.apply_deferred(world);
ret
}
/// Runs the system with the given input in the world.
///
/// [`run_readonly`]: ReadOnlySystem::run_readonly
fn run_without_applying_deferred(
&mut self,
input: SystemIn<'_, Self>,
world: &mut World,
) -> Self::Out {
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_cell) }
}
/// Applies any [`Deferred`](crate::system::Deferred) system parameters (or other system buffers) of this system to the world.
///
/// This is where [`Commands`](crate::system::Commands) get applied.
fn apply_deferred(&mut self, world: &mut World);
/// Enqueues any [`Deferred`](crate::system::Deferred) system parameters (or other system buffers)
/// of this system into the world's command buffer.
fn queue_deferred(&mut self, world: DeferredWorld);
/// Validates that all parameters can be acquired and that system can run without panic.
/// Built-in executors use this to prevent invalid systems from running.
///
/// However calling and respecting [`System::validate_param_unsafe`] or it's safe variant
/// is not a strict requirement, both [`System::run`] and [`System::run_unsafe`]
/// should provide their own safety mechanism to prevent undefined behavior.
///
/// This method has to be called directly before [`System::run_unsafe`] with no other (relevant)
/// world mutations in between. Otherwise, while it won't lead to any undefined behavior,
/// the validity of the param may change.
///
/// # Safety
///
/// - The caller must ensure that [`world`](UnsafeWorldCell) has permission to access any world data
/// registered in `archetype_component_access`. There must be no conflicting
/// simultaneous accesses while the system is running.
/// - The method [`System::update_archetype_component_access`] must be called at some
/// point before this one, with the same exact [`World`]. If [`System::update_archetype_component_access`]
/// panics (or otherwise does not return for any reason), this method must not be called.
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError>;
/// Safe version of [`System::validate_param_unsafe`].
/// that runs on exclusive, single-threaded `world` pointer.
fn validate_param(&mut self, world: &World) -> Result<(), SystemParamValidationError> {
let world_cell = world.as_unsafe_world_cell_readonly();
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.validate_param_unsafe(world_cell) }
}
/// Initialize the system.
fn initialize(&mut self, _world: &mut World);
/// Update the system's archetype component [`Access`].
///
/// ## Note for implementers
/// `world` may only be used to access metadata. This can be done in safe code
/// via functions such as [`UnsafeWorldCell::archetypes`].
fn update_archetype_component_access(&mut self, world: UnsafeWorldCell);
/// Checks any [`Tick`]s stored on this system and wraps their value if they get too old.
///
/// This method must be called periodically to ensure that change detection behaves correctly.
/// When using bevy's default configuration, this will be called for you as needed.
fn check_change_tick(&mut self, change_tick: Tick);
/// Returns the system's default [system sets](crate::schedule::SystemSet).
///
/// Each system will create a default system set that contains the system.
fn default_system_sets(&self) -> Vec<InternedSystemSet> {
Vec::new()
}
/// Gets the tick indicating the last time this system ran.
fn get_last_run(&self) -> Tick;
/// Overwrites the tick indicating the last time this system ran.
///
/// # Warning
/// This is a complex and error-prone operation, that can have unexpected consequences on any system relying on this code.
/// However, it can be an essential escape hatch when, for example,
/// you are trying to synchronize representations using change detection and need to avoid infinite recursion.
fn set_last_run(&mut self, last_run: Tick);
}
/// [`System`] types that do not modify the [`World`] when run.
/// This is implemented for any systems whose parameters all implement [`ReadOnlySystemParam`].
///
/// Note that systems which perform [deferred](System::apply_deferred) mutations (such as with [`Commands`])
/// may implement this trait.
///
/// [`ReadOnlySystemParam`]: crate::system::ReadOnlySystemParam
/// [`Commands`]: crate::system::Commands
///
/// # Safety
///
/// This must only be implemented for system types which do not mutate the `World`
/// when [`System::run_unsafe`] is called.
pub unsafe trait ReadOnlySystem: System {
/// Runs this system with the given input in the world.
///
/// Unlike [`System::run`], this can be called with a shared reference to the world,
/// since this system is known not to modify the world.
fn run_readonly(&mut self, input: SystemIn<'_, Self>, world: &World) -> Self::Out {
let world = world.as_unsafe_world_cell_readonly();
self.update_archetype_component_access(world);
// SAFETY:
// - We have read-only access to the entire world.
// - `update_archetype_component_access` has been called.
unsafe { self.run_unsafe(input, world) }
}
}
/// A convenience type alias for a boxed [`System`] trait object.
pub type BoxedSystem<In = (), Out = ()> = Box<dyn System<In = In, Out = Out>>;
pub(crate) fn check_system_change_tick(last_run: &mut Tick, this_run: Tick, system_name: &str) {
if last_run.check_tick(this_run) {
let age = this_run.relative_to(*last_run).get();
warn!(
"System '{system_name}' has not run for {age} ticks. \
Changes older than {} ticks will not be detected.",
Tick::MAX.get() - 1,
);
}
}
impl<In, Out> Debug for dyn System<In = In, Out = Out>
where
In: SystemInput + 'static,
Out: 'static,
{
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.debug_struct("System")
.field("name", &self.name())
.field("is_exclusive", &self.is_exclusive())
.field("is_send", &self.is_send())
.finish_non_exhaustive()
}
}
/// Trait used to run a system immediately on a [`World`].
///
/// # Warning
/// This function is not an efficient method of running systems and it's meant to be used as a utility
/// for testing and/or diagnostics.
///
/// Systems called through [`run_system_once`](RunSystemOnce::run_system_once) do not hold onto any state,
/// as they are created and destroyed every time [`run_system_once`](RunSystemOnce::run_system_once) is called.
/// Practically, this means that [`Local`](crate::system::Local) variables are
/// reset on every run and change detection does not work.
///
/// ```
/// # use bevy_ecs::prelude::*;
/// # use bevy_ecs::system::RunSystemOnce;
/// #[derive(Resource, Default)]
/// struct Counter(u8);
///
/// fn increment(mut counter: Local<Counter>) {
/// counter.0 += 1;
/// println!("{}", counter.0);
/// }
///
/// let mut world = World::default();
/// world.run_system_once(increment); // prints 1
/// world.run_system_once(increment); // still prints 1
/// ```
///
/// If you do need systems to hold onto state between runs, use [`World::run_system_cached`](World::run_system_cached)
/// or [`World::run_system`](World::run_system).
///
/// # Usage
/// Typically, to test a system, or to extract specific diagnostics information from a world,
/// you'd need a [`Schedule`](crate::schedule::Schedule) to run the system. This can create redundant boilerplate code
/// when writing tests or trying to quickly iterate on debug specific systems.
///
/// For these situations, this function can be useful because it allows you to execute a system
/// immediately with some custom input and retrieve its output without requiring the necessary boilerplate.
///
/// # Examples
///
/// ## Immediate Command Execution
///
/// This usage is helpful when trying to test systems or functions that operate on [`Commands`](crate::system::Commands):
/// ```
/// # use bevy_ecs::prelude::*;
/// # use bevy_ecs::system::RunSystemOnce;
/// let mut world = World::default();
/// let entity = world.run_system_once(|mut commands: Commands| {
/// commands.spawn_empty().id()
/// }).unwrap();
/// # assert!(world.get_entity(entity).is_ok());
/// ```
///
/// ## Immediate Queries
///
/// This usage is helpful when trying to run an arbitrary query on a world for testing or debugging purposes:
/// ```
/// # use bevy_ecs::prelude::*;
/// # use bevy_ecs::system::RunSystemOnce;
///
/// #[derive(Component)]
/// struct T(usize);
///
/// let mut world = World::default();
/// world.spawn(T(0));
/// world.spawn(T(1));
/// world.spawn(T(1));
/// let count = world.run_system_once(|query: Query<&T>| {
/// query.iter().filter(|t| t.0 == 1).count()
/// }).unwrap();
///
/// # assert_eq!(count, 2);
/// ```
///
/// Note that instead of closures you can also pass in regular functions as systems:
///
/// ```
/// # use bevy_ecs::prelude::*;
/// # use bevy_ecs::system::RunSystemOnce;
///
/// #[derive(Component)]
/// struct T(usize);
///
/// fn count(query: Query<&T>) -> usize {
/// query.iter().filter(|t| t.0 == 1).count()
/// }
///
/// let mut world = World::default();
/// world.spawn(T(0));
/// world.spawn(T(1));
/// world.spawn(T(1));
/// let count = world.run_system_once(count).unwrap();
///
/// # assert_eq!(count, 2);
/// ```
pub trait RunSystemOnce: Sized {
/// Tries to run a system and apply its deferred parameters.
fn run_system_once<T, Out, Marker>(self, system: T) -> Result<Out, RunSystemError>
where
T: IntoSystem<(), Out, Marker>,
{
self.run_system_once_with(system, ())
}
/// Tries to run a system with given input and apply deferred parameters.
fn run_system_once_with<T, In, Out, Marker>(
self,
system: T,
input: SystemIn<'_, T::System>,
) -> Result<Out, RunSystemError>
where
T: IntoSystem<In, Out, Marker>,
In: SystemInput;
}
impl RunSystemOnce for &mut World {
fn run_system_once_with<T, In, Out, Marker>(
self,
system: T,
input: SystemIn<'_, T::System>,
) -> Result<Out, RunSystemError>
where
T: IntoSystem<In, Out, Marker>,
In: SystemInput,
{
let mut system: T::System = IntoSystem::into_system(system);
system.initialize(self);
match system.validate_param(self) {
Ok(()) => Ok(system.run(input, self)),
// TODO: should we expse the fact that the system was skipped to the user?
// Should we somehow unify this better with system error handling?
Err(_) => Err(RunSystemError::InvalidParams(system.name())),
}
}
}
/// Running system failed.
#[derive(Error)]
pub enum RunSystemError {
/// System could not be run due to parameters that failed validation.
///
/// This can occur because the data required by the system was not present in the world.
#[error("The data required by the system {0:?} was not found in the world and the system did not run due to failed parameter validation.")]
InvalidParams(Cow<'static, str>),
}
impl Debug for RunSystemError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
match self {
Self::InvalidParams(arg0) => f.debug_tuple("InvalidParams").field(arg0).finish(),
}
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::prelude::*;
#[test]
fn run_system_once() {
struct T(usize);
impl Resource for T {}
fn system(In(n): In<usize>, mut commands: Commands) -> usize {
commands.insert_resource(T(n));
n + 1
}
let mut world = World::default();
let n = world.run_system_once_with(system, 1).unwrap();
assert_eq!(n, 2);
assert_eq!(world.resource::<T>().0, 1);
}
#[derive(Resource, Default, PartialEq, Debug)]
struct Counter(u8);
fn count_up(mut counter: ResMut<Counter>) {
counter.0 += 1;
}
#[test]
fn run_two_systems() {
let mut world = World::new();
world.init_resource::<Counter>();
assert_eq!(*world.resource::<Counter>(), Counter(0));
world.run_system_once(count_up).unwrap();
assert_eq!(*world.resource::<Counter>(), Counter(1));
world.run_system_once(count_up).unwrap();
assert_eq!(*world.resource::<Counter>(), Counter(2));
}
fn spawn_entity(mut commands: Commands) {
commands.spawn_empty();
}
#[test]
fn command_processing() {
let mut world = World::new();
assert_eq!(world.entities.len(), 0);
world.run_system_once(spawn_entity).unwrap();
assert_eq!(world.entities.len(), 1);
}
#[test]
fn non_send_resources() {
fn non_send_count_down(mut ns: NonSendMut<Counter>) {
ns.0 -= 1;
}
let mut world = World::new();
world.insert_non_send_resource(Counter(10));
assert_eq!(*world.non_send_resource::<Counter>(), Counter(10));
world.run_system_once(non_send_count_down).unwrap();
assert_eq!(*world.non_send_resource::<Counter>(), Counter(9));
}
#[test]
fn run_system_once_invalid_params() {
struct T;
impl Resource for T {}
fn system(_: Res<T>) {}
let mut world = World::default();
// This fails because `T` has not been added to the world yet.
let result = world.run_system_once(system);
assert!(matches!(result, Err(RunSystemError::InvalidParams(_))));
}
}