forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Replace ValidationOutcome with Result (#18541)

Browse Source 
# Objective

Make it easier to short-circuit system parameter validation.  

Simplify the API surface by combining `ValidationOutcome` with
`SystemParamValidationError`.

## Solution

Replace `ValidationOutcome` with `Result<(),
SystemParamValidationError>`. Move the docs from `ValidationOutcome` to
`SystemParamValidationError`.

Add a `skipped` field to `SystemParamValidationError` to distinguish the
`Skipped` and `Invalid` variants.

Use the `?` operator to short-circuit validation in tuples of system
params.
This commit is contained in:
Chris Russell 2025-03-25 23:36:16 -04:00 committed by François Mockers
parent 9705eaef02
commit 765e5842cd
20 changed files with 220 additions and 213 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
crates/bevy_ecs/macros/src/lib.rs
View File

@ -429,7 +429,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
state: &'s Self::State,
system_meta: &#path::system::SystemMeta,
world: #path::world::unsafe_world_cell::UnsafeWorldCell<'w>,
) -> #path::system::ValidationOutcome {
) -> Result<(), #path::system::SystemParamValidationError> {
<(#(#tuple_types,)*) as #path::system::SystemParam>::validate_param(&state.state, system_meta, world)
}

23
crates/bevy_ecs/src/observer/runner.rs
View File

@ -7,7 +7,7 @@ use crate::{
observer::{ObserverDescriptor, ObserverTrigger},
prelude::*,
query::DebugCheckedUnwrap,
system::{IntoObserverSystem, ObserverSystem, SystemParamValidationError, ValidationOutcome},
system::{IntoObserverSystem, ObserverSystem},
world::DeferredWorld,
};
use bevy_ptr::PtrMut;
@ -406,7 +406,7 @@ fn observer_system_runner<E: Event, B: Bundle, S: ObserverSystem<E, B>>(
unsafe {
(*system).update_archetype_component_access(world);
match (*system).validate_param_unsafe(world) {
ValidationOutcome::Valid => {
Ok(()) => {
if let Err(err) = (*system).run_unsafe(trigger, world) {
error_handler(
err,
@ -418,14 +418,17 @@ fn observer_system_runner<E: Event, B: Bundle, S: ObserverSystem<E, B>>(
};
(*system).queue_deferred(world.into_deferred());
}
ValidationOutcome::Invalid => error_handler(
SystemParamValidationError.into(),
ErrorContext::Observer {
name: (*system).name(),
last_run: (*system).get_last_run(),
},
),
ValidationOutcome::Skipped => (),
Err(e) => {
if !e.skipped {
error_handler(
e.into(),
ErrorContext::Observer {
name: (*system).name(),
last_run: (*system).get_last_run(),
},
);
}
}
}
}
}

9
crates/bevy_ecs/src/schedule/executor/mod.rs
View File

@ -20,7 +20,7 @@ use crate::{
prelude::{IntoSystemSet, SystemSet},
query::Access,
schedule::{BoxedCondition, InternedSystemSet, NodeId, SystemTypeSet},
system::{ScheduleSystem, System, SystemIn, ValidationOutcome},
system::{ScheduleSystem, System, SystemIn, SystemParamValidationError},
world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World},
};
@ -221,10 +221,13 @@ impl System for ApplyDeferred {
fn queue_deferred(&mut self, _world: DeferredWorld) {}
unsafe fn validate_param_unsafe(&mut self, _world: UnsafeWorldCell) -> ValidationOutcome {
unsafe fn validate_param_unsafe(
&mut self,
_world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// This system is always valid to run because it doesn't do anything,
// and only used as a marker for the executor.
ValidationOutcome::Valid
Ok(())
}
fn initialize(&mut self, _world: &mut World) {}

44
crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
View File

@ -18,7 +18,7 @@ use crate::{
prelude::Resource,
query::Access,
schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule},
system::{ScheduleSystem, SystemParamValidationError, ValidationOutcome},
system::ScheduleSystem,
world::{unsafe_world_cell::UnsafeWorldCell, World},
};
@ -582,18 +582,19 @@ impl ExecutorState {
// required by the system.
// - `update_archetype_component_access` has been called for system.
let valid_params = match unsafe { system.validate_param_unsafe(world) } {
ValidationOutcome::Valid => true,
ValidationOutcome::Invalid => {
error_handler(
SystemParamValidationError.into(),
ErrorContext::System {
name: system.name(),
last_run: system.get_last_run(),
},
);
Ok(()) => true,
Err(e) => {
if !e.skipped {
error_handler(
e.into(),
ErrorContext::System {
name: system.name(),
last_run: system.get_last_run(),
},
);
}
false
}
ValidationOutcome::Skipped => false,
};
if !valid_params {
self.skipped_systems.insert(system_index);
@ -796,18 +797,19 @@ unsafe fn evaluate_and_fold_conditions(
// required by the condition.
// - `update_archetype_component_access` has been called for condition.
match unsafe { condition.validate_param_unsafe(world) } {
ValidationOutcome::Valid => (),
ValidationOutcome::Invalid => {
error_handler(
SystemParamValidationError.into(),
ErrorContext::System {
name: condition.name(),
last_run: condition.get_last_run(),
},
);
Ok(()) => (),
Err(e) => {
if !e.skipped {
error_handler(
e.into(),
ErrorContext::System {
name: condition.name(),
last_run: condition.get_last_run(),
},
);
}
return false;
}
ValidationOutcome::Skipped => return false,
}
// SAFETY:
// - The caller ensures that `world` has permission to read any data

43
crates/bevy_ecs/src/schedule/executor/simple.rs
View File

@ -12,7 +12,6 @@ use crate::{
schedule::{
executor::is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule,
},
system::{SystemParamValidationError, ValidationOutcome},
world::World,
};
@ -89,18 +88,19 @@ impl SystemExecutor for SimpleExecutor {
let system = &mut schedule.systems[system_index];
if should_run {
let valid_params = match system.validate_param(world) {
ValidationOutcome::Valid => true,
ValidationOutcome::Invalid => {
error_handler(
SystemParamValidationError.into(),
ErrorContext::System {
name: system.name(),
last_run: system.get_last_run(),
},
);
Ok(()) => true,
Err(e) => {
if !e.skipped {
error_handler(
e.into(),
ErrorContext::System {
name: system.name(),
last_run: system.get_last_run(),
},
);
}
false
}
ValidationOutcome::Skipped => false,
};
should_run &= valid_params;
}
@ -177,18 +177,19 @@ fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut W
.iter_mut()
.map(|condition| {
match condition.validate_param(world) {
ValidationOutcome::Valid => (),
ValidationOutcome::Invalid => {
error_handler(
SystemParamValidationError.into(),
ErrorContext::System {
name: condition.name(),
last_run: condition.get_last_run(),
},
);
Ok(()) => (),
Err(e) => {
if !e.skipped {
error_handler(
e.into(),
ErrorContext::System {
name: condition.name(),
last_run: condition.get_last_run(),
},
);
}
return false;
}
ValidationOutcome::Skipped => return false,
}
__rust_begin_short_backtrace::readonly_run(&mut **condition, world)
})

43
crates/bevy_ecs/src/schedule/executor/single_threaded.rs
View File

@ -10,7 +10,6 @@ use std::eprintln;
use crate::{
error::{default_error_handler, BevyError, ErrorContext},
schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule},
system::{SystemParamValidationError, ValidationOutcome},
world::World,
};
@ -95,18 +94,19 @@ impl SystemExecutor for SingleThreadedExecutor {
let system = &mut schedule.systems[system_index];
if should_run {
let valid_params = match system.validate_param(world) {
ValidationOutcome::Valid => true,
ValidationOutcome::Invalid => {
error_handler(
SystemParamValidationError.into(),
ErrorContext::System {
name: system.name(),
last_run: system.get_last_run(),
},
);
Ok(()) => true,
Err(e) => {
if !e.skipped {
error_handler(
e.into(),
ErrorContext::System {
name: system.name(),
last_run: system.get_last_run(),
},
);
}
false
}
ValidationOutcome::Skipped => false,
};
should_run &= valid_params;
@ -221,18 +221,19 @@ fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut W
.iter_mut()
.map(|condition| {
match condition.validate_param(world) {
ValidationOutcome::Valid => (),
ValidationOutcome::Invalid => {
error_handler(
SystemParamValidationError.into(),
ErrorContext::System {
name: condition.name(),
last_run: condition.get_last_run(),
},
);
Ok(()) => (),
Err(e) => {
if !e.skipped {
error_handler(
e.into(),
ErrorContext::System {
name: condition.name(),
last_run: condition.get_last_run(),
},
);
}
return false;
}
ValidationOutcome::Skipped => return false,
}
__rust_begin_short_backtrace::readonly_run(&mut **condition, world)
})

7
crates/bevy_ecs/src/system/adapter_system.rs
View File

@ -1,6 +1,6 @@
use alloc::{borrow::Cow, vec::Vec};
use super::{IntoSystem, ReadOnlySystem, System, ValidationOutcome};
use super::{IntoSystem, ReadOnlySystem, System, SystemParamValidationError};
use crate::{
schedule::InternedSystemSet,
system::{input::SystemInput, SystemIn},
@ -179,7 +179,10 @@ where
}
#[inline]
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> ValidationOutcome {
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Delegate to other `System` implementations.
unsafe { self.system.validate_param_unsafe(world) }
}

23
crates/bevy_ecs/src/system/combinator.rs
View File

@ -7,7 +7,7 @@ use crate::{
prelude::World,
query::Access,
schedule::InternedSystemSet,
system::{input::SystemInput, SystemIn, ValidationOutcome},
system::{input::SystemInput, SystemIn, SystemParamValidationError},
world::unsafe_world_cell::UnsafeWorldCell,
};
@ -212,7 +212,10 @@ where
}
#[inline]
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> ValidationOutcome {
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Delegate to other `System` implementations.
unsafe { self.a.validate_param_unsafe(world) }
}
@ -431,18 +434,18 @@ where
self.b.queue_deferred(world);
}
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> ValidationOutcome {
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Delegate to other `System` implementations.
unsafe { self.a.validate_param_unsafe(world) }
}
fn validate_param(&mut self, world: &World) -> ValidationOutcome {
// This follows the logic of `ValidationOutcome::combine`, but short-circuits
let validate_a = self.a.validate_param(world);
match validate_a {
ValidationOutcome::Valid => self.b.validate_param(world),
ValidationOutcome::Invalid | ValidationOutcome::Skipped => validate_a,
}
fn validate_param(&mut self, world: &World) -> Result<(), SystemParamValidationError> {
self.a.validate_param(world)?;
self.b.validate_param(world)?;
Ok(())
}
fn initialize(&mut self, world: &mut World) {

4
crates/bevy_ecs/src/system/commands/mod.rs
View File

@ -27,7 +27,7 @@ use crate::{
schedule::ScheduleLabel,
system::{
Deferred, IntoObserverSystem, IntoSystem, RegisteredSystem, SystemId, SystemInput,
ValidationOutcome,
SystemParamValidationError,
},
world::{
command_queue::RawCommandQueue, unsafe_world_cell::UnsafeWorldCell, CommandQueue,
@ -182,7 +182,7 @@ const _: () = {
state: &Self::State,
system_meta: &bevy_ecs::system::SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
) -> Result<(), SystemParamValidationError> {
<(Deferred<CommandQueue>, &Entities) as bevy_ecs::system::SystemParam>::validate_param(
&state.state,
system_meta,

9
crates/bevy_ecs/src/system/exclusive_function_system.rs
View File

@ -14,7 +14,7 @@ use alloc::{borrow::Cow, vec, vec::Vec};
use core::marker::PhantomData;
use variadics_please::all_tuples;
use super::ValidationOutcome;
use super::SystemParamValidationError;
/// A function system that runs with exclusive [`World`] access.
///
@ -156,9 +156,12 @@ where
}
#[inline]
unsafe fn validate_param_unsafe(&mut self, _world: UnsafeWorldCell) -> ValidationOutcome {
unsafe fn validate_param_unsafe(
&mut self,
_world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// All exclusive system params are always available.
ValidationOutcome::Valid
Ok(())
}
#[inline]

12
crates/bevy_ecs/src/system/function_system.rs
View File

@ -18,7 +18,7 @@ use variadics_please::all_tuples;
#[cfg(feature = "trace")]
use tracing::{info_span, Span};
use super::{IntoSystem, ReadOnlySystem, SystemParamBuilder, ValidationOutcome};
use super::{IntoSystem, ReadOnlySystem, SystemParamBuilder, SystemParamValidationError};
/// The metadata of a [`System`].
#[derive(Clone)]
@ -417,7 +417,10 @@ impl<Param: SystemParam> SystemState<Param> {
/// - The passed [`UnsafeWorldCell`] must have read-only access to
/// world data in `archetype_component_access`.
/// - `world` must be the same [`World`] that was used to initialize [`state`](SystemParam::init_state).
pub unsafe fn validate_param(state: &Self, world: UnsafeWorldCell) -> ValidationOutcome {
pub unsafe fn validate_param(
state: &Self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Delegated to existing `SystemParam` implementations.
unsafe { Param::validate_param(&state.param_state, &state.meta, world) }
}
@ -747,7 +750,10 @@ where
}
#[inline]
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> ValidationOutcome {
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
let param_state = &self.state.as_ref().expect(Self::ERROR_UNINITIALIZED).param;
// SAFETY:
// - The caller has invoked `update_archetype_component_access`, which will panic

7
crates/bevy_ecs/src/system/observer_system.rs
View File

@ -12,7 +12,7 @@ use crate::{
world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World},
};
use super::{IntoSystem, ValidationOutcome};
use super::{IntoSystem, SystemParamValidationError};
/// Implemented for [`System`]s that have a [`Trigger`] as the first argument.
pub trait ObserverSystem<E: 'static, B: Bundle, Out = Result>:
@ -155,7 +155,10 @@ where
}
#[inline]
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> ValidationOutcome {
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
self.observer.validate_param_unsafe(world)
}

4
crates/bevy_ecs/src/system/query.rs
View File

@ -2574,7 +2574,7 @@ impl<'w, 'q, Q: QueryData, F: QueryFilter> From<&'q mut Query<'w, '_, Q, F>>
/// [System parameter] that provides access to single entity's components, much like [`Query::single`]/[`Query::single_mut`].
///
/// This [`SystemParam`](crate::system::SystemParam) fails validation if zero or more than one matching entity exists.
/// This will cause the system to be skipped, according to the rules laid out in [`ValidationOutcome`](crate::system::ValidationOutcome).
/// This will cause the system to be skipped, according to the rules laid out in [`SystemParamValidationError`](crate::system::SystemParamValidationError).
///
/// Use [`Option<Single<D, F>>`] instead if zero or one matching entities can exist.
///
@ -2610,7 +2610,7 @@ impl<'w, D: QueryData, F: QueryFilter> Single<'w, D, F> {
/// [System parameter] that works very much like [`Query`] except it always contains at least one matching entity.
///
/// This [`SystemParam`](crate::system::SystemParam) fails validation if no matching entities exist.
/// This will cause the system to be skipped, according to the rules laid out in [`ValidationOutcome`](crate::system::ValidationOutcome).
/// This will cause the system to be skipped, according to the rules laid out in [`SystemParamValidationError`](crate::system::SystemParamValidationError).
///
/// Much like [`Query::is_empty`] the worst case runtime will be `O(n)` where `n` is the number of *potential* matches.
/// This can be notably expensive for queries that rely on non-archetypal filters such as [`Added`](crate::query::Added) or [`Changed`](crate::query::Changed)

7
crates/bevy_ecs/src/system/schedule_system.rs
View File

@ -9,7 +9,7 @@ use crate::{
world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World},
};
use super::{IntoSystem, ValidationOutcome};
use super::{IntoSystem, SystemParamValidationError};
/// A wrapper system to change a system that returns `()` to return `Ok(())` to make it into a [`ScheduleSystem`]
pub struct InfallibleSystemWrapper<S: System<In = (), Out = ()>>(S);
@ -82,7 +82,10 @@ impl<S: System<In = (), Out = ()>> System for InfallibleSystemWrapper<S> {
}
#[inline]
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> ValidationOutcome {
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
self.0.validate_param_unsafe(world)
}

15
crates/bevy_ecs/src/system/system.rs
View File

@ -18,7 +18,7 @@ use crate::{
use alloc::{borrow::Cow, boxed::Box, vec::Vec};
use core::any::TypeId;
use super::{IntoSystem, ValidationOutcome};
use super::{IntoSystem, SystemParamValidationError};
/// An ECS system that can be added to a [`Schedule`](crate::schedule::Schedule)
///
@ -132,11 +132,14 @@ pub trait System: Send + Sync + 'static {
/// - 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) -> ValidationOutcome;
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) -> ValidationOutcome {
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:
@ -364,12 +367,10 @@ impl RunSystemOnce for &mut World {
let mut system: T::System = IntoSystem::into_system(system);
system.initialize(self);
match system.validate_param(self) {
ValidationOutcome::Valid => Ok(system.run(input, 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?
ValidationOutcome::Invalid | ValidationOutcome::Skipped => {
Err(RunSystemError::InvalidParams(system.name()))
}
Err(_) => Err(RunSystemError::InvalidParams(system.name())),
}
}
}

155
crates/bevy_ecs/src/system/system_param.rs
View File

@ -275,8 +275,8 @@ pub unsafe trait SystemParam: Sized {
state: &Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
ValidationOutcome::Valid
) -> Result<(), SystemParamValidationError> {
Ok(())
}
/// Creates a parameter to be passed into a [`SystemParamFunction`](super::SystemParamFunction).
@ -310,43 +310,6 @@ unsafe impl<'w, 's, D: ReadOnlyQueryData + 'static, F: QueryFilter + 'static> Re
{
}
/// The outcome of system / system param validation,
/// used by system executors to determine what to do with a system.
///
/// The behavior of each system parameter can be controlled by returning a different outcome from [`SystemParam::validate_param`].
pub enum ValidationOutcome {
/// All system parameters were validated successfully and the system can be run.
Valid,
/// At least one system parameter failed validation, and an error must be handled.
/// 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
/// failures in validation should be considered a bug in the user's logic that must be immediately addressed (like [`Res`]).
Invalid,
/// At least one system parameter failed validation, but the system should be skipped.
/// This is suitable for system params that are intended to only operate in certain application states, such as [`Single`].
Skipped,
}
impl ValidationOutcome {
/// Combines two [`ValidationOutcome`]s, returning the most severe one.
///
/// If either outcome is [`ValidationOutcome::Invalid`], the result will be [`ValidationOutcome::Invalid`].
/// Otherwise, if either outcome is [`ValidationOutcome::Skipped`], the result will be [`ValidationOutcome::Skipped`].
/// Finally, if both outcomes are [`ValidationOutcome::Valid`], the result will be [`ValidationOutcome::Valid`].
///
/// When called, you should typically return early if the result is [`ValidationOutcome::Invalid`] or [`ValidationOutcome::Skipped`],
/// to avoid unnecessary work validating irrelevant system parameters.
pub const fn combine(self, other: Self) -> Self {
match (self, other) {
(Self::Invalid, _) | (_, Self::Invalid) => Self::Invalid,
(Self::Skipped, _) | (_, Self::Skipped) => Self::Skipped,
(Self::Valid, Self::Valid) => Self::Valid,
}
}
}
// SAFETY: Relevant query ComponentId and ArchetypeComponentId access is applied to SystemMeta. If
// this Query conflicts with any prior access, a panic will occur.
unsafe impl<D: QueryData + 'static, F: QueryFilter + 'static> SystemParam for Query<'_, '_, D, F> {
@ -465,7 +428,7 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam fo
state: &Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
) -> Result<(), SystemParamValidationError> {
// SAFETY: State ensures that the components it accesses are not mutably accessible elsewhere
// and the query is read only.
// The caller ensures the world matches the one used in init_state.
@ -477,8 +440,8 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam fo
)
};
match query.single_inner() {
Ok(_) => ValidationOutcome::Valid,
Err(_) => ValidationOutcome::Skipped,
Ok(_) => Ok(()),
Err(_) => Err(SystemParamValidationError::skipped()),
}
}
}
@ -532,7 +495,7 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam
state: &Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
) -> Result<(), SystemParamValidationError> {
// SAFETY: State ensures that the components it accesses are not mutably accessible elsewhere
// and the query is read only.
// The caller ensures the world matches the one used in init_state.
@ -544,8 +507,10 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam
)
};
match query.single_inner() {
Ok(_) | Err(QuerySingleError::NoEntities(_)) => ValidationOutcome::Valid,
Err(QuerySingleError::MultipleEntities(_)) => ValidationOutcome::Skipped,
Ok(_) | Err(QuerySingleError::NoEntities(_)) => Ok(()),
Err(QuerySingleError::MultipleEntities(_)) => {
Err(SystemParamValidationError::skipped())
}
}
}
}
@ -600,7 +565,7 @@ unsafe impl<D: QueryData + 'static, F: QueryFilter + 'static> SystemParam
state: &Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
) -> Result<(), SystemParamValidationError> {
// SAFETY:
// - We have read-only access to the components accessed by query.
// - The caller ensures the world matches the one used in init_state.
@ -612,9 +577,9 @@ unsafe impl<D: QueryData + 'static, F: QueryFilter + 'static> SystemParam
)
};
if query.is_empty() {
ValidationOutcome::Skipped
Err(SystemParamValidationError::skipped())
} else {
ValidationOutcome::Valid
Ok(())
}
}
}
@ -810,7 +775,7 @@ macro_rules! impl_param_set {
state: &'s Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
) -> ValidationOutcome {
) -> Result<(), SystemParamValidationError> {
<($($param,)*) as SystemParam>::validate_param(state, system_meta, world)
}
@ -888,16 +853,16 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> {
&component_id: &Self::State,
_system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
) -> Result<(), SystemParamValidationError> {
// SAFETY: Read-only access to resource metadata.
if unsafe { world.storages() }
.resources
.get(component_id)
.is_some_and(ResourceData::is_present)
{
ValidationOutcome::Valid
Ok(())
} else {
ValidationOutcome::Invalid
Err(SystemParamValidationError::invalid())
}
}
@ -1001,16 +966,16 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> {
&component_id: &Self::State,
_system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
) -> Result<(), SystemParamValidationError> {
// SAFETY: Read-only access to resource metadata.
if unsafe { world.storages() }
.resources
.get(component_id)
.is_some_and(ResourceData::is_present)
{
ValidationOutcome::Valid
Ok(())
} else {
ValidationOutcome::Invalid
Err(SystemParamValidationError::invalid())
}
}
@ -1572,16 +1537,16 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> {
&component_id: &Self::State,
_system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
) -> Result<(), SystemParamValidationError> {
// SAFETY: Read-only access to resource metadata.
if unsafe { world.storages() }
.non_send_resources
.get(component_id)
.is_some_and(ResourceData::is_present)
{
ValidationOutcome::Valid
Ok(())
} else {
ValidationOutcome::Invalid
Err(SystemParamValidationError::invalid())
}
}
@ -1682,16 +1647,16 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> {
&component_id: &Self::State,
_system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
) -> Result<(), SystemParamValidationError> {
// SAFETY: Read-only access to resource metadata.
if unsafe { world.storages() }
.non_send_resources
.get(component_id)
.is_some_and(ResourceData::is_present)
{
ValidationOutcome::Valid
Ok(())
} else {
ValidationOutcome::Invalid
Err(SystemParamValidationError::invalid())
}
}
@ -1900,23 +1865,11 @@ unsafe impl<T: SystemParam> SystemParam for Vec<T> {
state: &Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
let mut validation_state = ValidationOutcome::Valid;
) -> Result<(), SystemParamValidationError> {
for state in state {
validation_state =
validation_state.combine(T::validate_param(state, system_meta, world));
// Short-circuit to avoid wasted validation work
if matches!(
validation_state,
ValidationOutcome::Invalid | ValidationOutcome::Skipped
) {
return validation_state;
}
T::validate_param(state, system_meta, world)?;
}
validation_state
Ok(())
}
#[inline]
@ -2100,12 +2053,12 @@ macro_rules! impl_system_param_tuple {
state: &Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
// PERF: short-circuit to avoid wasted validation work
) -> Result<(), SystemParamValidationError> {
let ($($param,)*) = state;
// Run validation on each parameter in the tuple,
// combining the results into a single `ValidationOutcome`.
ValidationOutcome::Valid$(.combine($param::validate_param($param, system_meta, world)))*
$(
$param::validate_param($param, system_meta, world)?;
)*
Ok(())
}
#[inline]
@ -2272,7 +2225,7 @@ unsafe impl<P: SystemParam + 'static> SystemParam for StaticSystemParam<'_, '_,
state: &Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
) -> Result<(), SystemParamValidationError> {
P::validate_param(state, system_meta, world)
}
@ -2522,7 +2475,7 @@ trait DynParamState: Sync + Send {
&self,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome;
) -> Result<(), SystemParamValidationError>;
}
/// A wrapper around a [`SystemParam::State`] that can be used as a trait object in a [`DynSystemParam`].
@ -2550,7 +2503,7 @@ impl<T: SystemParam + 'static> DynParamState for ParamState<T> {
&self,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
) -> Result<(), SystemParamValidationError> {
T::validate_param(&self.0, system_meta, world)
}
}
@ -2570,7 +2523,7 @@ unsafe impl SystemParam for DynSystemParam<'_, '_> {
state: &Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
) -> Result<(), SystemParamValidationError> {
state.0.validate_param(system_meta, world)
}
@ -2665,12 +2618,38 @@ unsafe impl SystemParam for FilteredResourcesMut<'_, '_> {
}
}
/// An error that occurs when a system parameter is not valid.
/// An error that occurs when a system parameter is not valid,
/// used by system executors to determine what to do with a system.
///
/// Generated when [`SystemParam::validate_param`] returns `false`,
/// Returned as an error from [`SystemParam::validate_param`],
/// and handled using the unified error handling mechanisms defined in [`bevy_ecs::error`].
#[derive(Debug, PartialEq, Eq, Clone, Display, Error)]
pub struct SystemParamValidationError;
pub struct SystemParamValidationError {
/// Whether the system should be skipped.
///
/// If `false`, the error should be handled.
/// 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
/// 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`].
pub skipped: bool,
}
impl SystemParamValidationError {
/// Constructs a `SystemParamValidationError` that skips the system.
pub const fn skipped() -> Self {
Self { skipped: true }
}
/// Constructs a `SystemParamValidationError` for an invalid parameter that should be treated as an error.
pub const fn invalid() -> Self {
Self { skipped: false }
}
}
#[cfg(test)]
mod tests {

4
crates/bevy_ecs/src/system/system_registry.rs
View File

@ -13,8 +13,6 @@ use bevy_reflect::{std_traits::ReflectDefault, Reflect};
use core::marker::PhantomData;
use thiserror::Error;
use super::ValidationOutcome;
/// A small wrapper for [`BoxedSystem`] that also keeps track whether or not the system has been initialized.
#[derive(Component)]
#[require(SystemIdMarker)]
@ -353,7 +351,7 @@ impl World {
initialized = true;
}
let result = if let ValidationOutcome::Valid = system.validate_param(self) {
let result = if system.validate_param(self).is_ok() {
// Wait to run the commands until the system is available again.
// This is needed so the systems can recursively run themselves.
let ret = system.run_without_applying_deferred(input, self);

4
crates/bevy_gizmos/src/gizmos.rs
View File

@ -13,7 +13,7 @@ use bevy_ecs::{
resource::Resource,
system::{
Deferred, ReadOnlySystemParam, Res, SystemBuffer, SystemMeta, SystemParam,
ValidationOutcome,
SystemParamValidationError,
},
world::{unsafe_world_cell::UnsafeWorldCell, World},
};
@ -225,7 +225,7 @@ where
state: &Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
) -> Result<(), SystemParamValidationError> {
// SAFETY: Delegated to existing `SystemParam` implementations.
unsafe { GizmosState::<Config, Clear>::validate_param(&state.state, system_meta, world) }
}

8
crates/bevy_render/src/extract_param.rs
View File

@ -3,8 +3,8 @@ use bevy_ecs::{
component::Tick,
prelude::*,
system::{
ReadOnlySystemParam, SystemMeta, SystemParam, SystemParamItem, SystemState,
ValidationOutcome,
ReadOnlySystemParam, SystemMeta, SystemParam, SystemParamItem, SystemParamValidationError,
SystemState,
},
world::unsafe_world_cell::UnsafeWorldCell,
};
@ -84,11 +84,11 @@ where
state: &Self::State,
_system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> ValidationOutcome {
) -> Result<(), SystemParamValidationError> {
// SAFETY: Read-only access to world data registered in `init_state`.
let result = unsafe { world.get_resource_by_id(state.main_world_state) };
let Some(main_world) = result else {
return ValidationOutcome::Invalid;
return Err(SystemParamValidationError::invalid());
};
// SAFETY: Type is guaranteed by `SystemState`.
let main_world: &World = unsafe { main_world.deref() };

10
examples/ecs/fallible_params.rs
View File

@ -9,16 +9,14 @@
//!
//! Other system parameters, such as [`Query`], will never fail validation: returning a query with no matching entities is valid.
//!
//! The result of failed system parameter validation is determined by the [`ValidationOutcome`] returned
//! The result of failed system parameter validation is determined by the [`SystemParamValidationError`] returned
//! by [`SystemParam::validate_param`] for each system parameter.
//! Each system will pass, fail, or skip based on the joint outcome of all its parameters,
//! according to the rules defined in [`ValidationOutcome::combine`].
//! Each system will pass if all of its parameters are valid, or else return [`SystemParamValidationError`] for the first failing parameter.
//!
//! To learn more about setting the fallback behavior for [`ValidationOutcome`] failures,
//! To learn more about setting the fallback behavior for [`SystemParamValidationError`] failures,
//! please see the `error_handling.rs` example.
//!
//! [`ValidationOutcome`]: bevy::ecs::system::ValidationOutcome
//! [`ValidationOutcome::combine`]: bevy::ecs::system::ValidationOutcome::combine
//! [`SystemParamValidationError`]: bevy::ecs::system::SystemParamValidationError
//! [`SystemParam::validate_param`]: bevy::ecs::system::SystemParam::validate_param
use bevy::ecs::error::{warn, GLOBAL_ERROR_HANDLER};