From d28e4908cabc11312cca068fa165180d72d34613 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Sun, 4 May 2025 04:41:42 -0400 Subject: [PATCH] Create a `When` system param wrapper for skipping systems that fail validation (#18765) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Objective Create a `When` system param wrapper for skipping systems that fail validation. Currently, the `Single` and `Populated` parameters cause systems to skip when they fail validation, while the `Res` family causes systems to error. Generalize this so that any fallible parameter can be used either to skip a system or to raise an error. A parameter used directly will always raise an error, and a parameter wrapped in `When

` will always cause the system to be silently skipped. ~~Note that this changes the behavior for `Single` and `Populated`. The current behavior will be available using `When` and `When`.~~ Fixes #18516 ## Solution Create a `When` system param wrapper that wraps an inner parameter and converts all validation errors to `skipped`. ~~Change the behavior of `Single` and `Populated` to fail by default.~~ ~~Replace in-engine use of `Single` with `When`. I updated the `fallible_systems` example, but not all of the others. The other examples I looked at appeared to always have one matching entity, and it seemed more clear to use the simpler type in those cases.~~ --------- Co-authored-by: Alice Cecile Co-authored-by: Zachary Harrold Co-authored-by: François Mockers --- crates/bevy_ecs/src/lib.rs | 2 +- crates/bevy_ecs/src/system/builder.rs | 15 ++- crates/bevy_ecs/src/system/system_param.rs | 111 ++++++++++++++++++++- 3 files changed, 124 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 99f95763d5..c366a73885 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -94,7 +94,7 @@ pub mod prelude { Command, Commands, Deferred, EntityCommand, EntityCommands, In, InMut, InRef, IntoSystem, Local, NonSend, NonSendMut, ParamSet, Populated, Query, ReadOnlySystem, Res, ResMut, Single, System, SystemIn, SystemInput, SystemParamBuilder, - SystemParamFunction, + SystemParamFunction, When, }, world::{ EntityMut, EntityRef, EntityWorldMut, FilteredResources, FilteredResourcesMut, diff --git a/crates/bevy_ecs/src/system/builder.rs b/crates/bevy_ecs/src/system/builder.rs index 6261b9e355..3f15e9ef17 100644 --- a/crates/bevy_ecs/src/system/builder.rs +++ b/crates/bevy_ecs/src/system/builder.rs @@ -7,7 +7,7 @@ use crate::{ query::{QueryData, QueryFilter, QueryState}, resource::Resource, system::{ - DynSystemParam, DynSystemParamState, Local, ParamSet, Query, SystemMeta, SystemParam, + DynSystemParam, DynSystemParamState, Local, ParamSet, Query, SystemMeta, SystemParam, When, }, world::{ FilteredResources, FilteredResourcesBuilder, FilteredResourcesMut, @@ -710,6 +710,19 @@ unsafe impl<'w, 's, T: FnOnce(&mut FilteredResourcesMutBuilder)> } } +/// A [`SystemParamBuilder`] for a [`When`]. +#[derive(Clone)] +pub struct WhenBuilder(T); + +// SAFETY: `WhenBuilder` builds a state that is valid for `P`, and any state valid for `P` is valid for `When

` +unsafe impl> SystemParamBuilder> + for WhenBuilder +{ + fn build(self, world: &mut World, meta: &mut SystemMeta) -> as SystemParam>::State { + self.0.build(world, meta) + } +} + #[cfg(test)] mod tests { use crate::{ diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 99d4c72df6..7a16c48519 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1916,6 +1916,112 @@ unsafe impl SystemParam for SystemChangeTick { } } +/// A [`SystemParam`] that wraps another parameter and causes its system to skip instead of failing when the parameter is invalid. +/// +/// # Example +/// +/// ``` +/// # use bevy_ecs::prelude::*; +/// # #[derive(Resource)] +/// # struct SomeResource; +/// // This system will fail if `SomeResource` is not present. +/// fn fails_on_missing_resource(res: Res) {} +/// +/// // This system will skip without error if `SomeResource` is not present. +/// fn skips_on_missing_resource(res: When>) { +/// // The inner parameter is available using `Deref` +/// let some_resource: &SomeResource = &res; +/// } +/// # bevy_ecs::system::assert_is_system(skips_on_missing_resource); +/// ``` +#[derive(Debug)] +pub struct When(pub T); + +impl When { + /// Returns the inner `T`. + /// + /// The inner value is `pub`, so you can also obtain it by destructuring the parameter: + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// # #[derive(Resource)] + /// # struct SomeResource; + /// fn skips_on_missing_resource(When(res): When>) { + /// let some_resource: Res = res; + /// } + /// # bevy_ecs::system::assert_is_system(skips_on_missing_resource); + /// ``` + pub fn into_inner(self) -> T { + self.0 + } +} + +impl Deref for When { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for When { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + +// SAFETY: Delegates to `T`, which ensures the safety requirements are met +unsafe impl SystemParam for When { + type State = T::State; + + type Item<'world, 'state> = When>; + + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { + T::init_state(world, system_meta) + } + + #[inline] + unsafe fn validate_param( + state: &Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell, + ) -> Result<(), SystemParamValidationError> { + T::validate_param(state, system_meta, world).map_err(|mut e| { + e.skipped = true; + e + }) + } + + #[inline] + unsafe fn get_param<'world, 'state>( + state: &'state mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'world>, + change_tick: Tick, + ) -> Self::Item<'world, 'state> { + When(T::get_param(state, system_meta, world, change_tick)) + } + + unsafe fn new_archetype( + state: &mut Self::State, + archetype: &Archetype, + system_meta: &mut SystemMeta, + ) { + // SAFETY: The caller ensures that `archetype` is from the World the state was initialized from in `init_state`. + unsafe { T::new_archetype(state, archetype, system_meta) }; + } + + fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) { + T::apply(state, system_meta, world); + } + + fn queue(state: &mut Self::State, system_meta: &SystemMeta, world: DeferredWorld) { + T::queue(state, system_meta, world); + } +} + +// SAFETY: Delegates to `T`, which ensures the safety requirements are met +unsafe impl ReadOnlySystemParam for When {} + // SAFETY: When initialized with `init_state`, `get_param` returns an empty `Vec` and does no access. // Therefore, `init_state` trivially registers all access, and no accesses can conflict. // Note that the safety requirements for non-empty `Vec`s are handled by the `SystemParamBuilder` impl that builds them. @@ -2699,11 +2805,12 @@ pub struct SystemParamValidationError { /// By default, this will result in a panic. See [`crate::error`] for more information. /// /// This is the default behavior, and is suitable for system params that should *always* be valid, - /// either because sensible fallback behavior exists (like [`Query`] or because + /// either because sensible fallback behavior exists (like [`Query`]) or because /// failures in validation should be considered a bug in the user's logic that must be immediately addressed (like [`Res`]). /// /// If `true`, the system should be skipped. - /// This is suitable for system params that are intended to only operate in certain application states, such as [`Single`]. + /// This is set by wrapping the system param in [`When`], + /// and indicates that the system is intended to only operate in certain application states. pub skipped: bool, /// A message describing the validation error.