From acb4d7396740fcba54ab92702bf1b99a2d157126 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 20 Mar 2025 14:00:20 -0700 Subject: [PATCH 01/14] Document that the global error handler is used to handle missing system params --- crates/bevy_ecs/src/system/function_system.rs | 5 ++++- examples/ecs/fallible_params.rs | 10 ++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 0f3950d1d4ba6..13072ad9a8441 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -231,7 +231,10 @@ impl ParamWarnPolicy { } /// Trait for manipulating warn policy of systems. -#[doc(hidden)] +/// +/// By default, the fallback behavior of a system with invalid parameters is to panic, +/// although that can be configured globally via the [`GLOBAL_ERROR_HANDLER`](bevy_ecs::error::GLOBAL_ERROR_HANDLER)., +/// found in [`bevy_ecs::error`]. pub trait WithParamWarnPolicy where M: 'static, diff --git a/examples/ecs/fallible_params.rs b/examples/ecs/fallible_params.rs index deb4eaea49d0d..aed2171292e23 100644 --- a/examples/ecs/fallible_params.rs +++ b/examples/ecs/fallible_params.rs @@ -6,6 +6,9 @@ //! - [`Single`] - There must be exactly one matching entity. //! - [`Option>`] - There must be zero or one matching entity. //! - [`Populated`] - There must be at least one matching entity. +//! +//! To set the fallback behavior for when a parameter fails to be fetched, +//! please see the `error_handling.rs` example. use bevy::prelude::*; use rand::Rng; @@ -20,8 +23,11 @@ fn main() { App::new() .add_plugins(DefaultPlugins) .add_systems(Startup, setup) - // Default system policy is to panic if parameters fail to be fetched. - // We overwrite that configuration, to either warn us once or never. + // By default, if a parameter fail to be fetched, + // the `GLOBAL_ERROR_HANDLER` will be used to handle the error, + // which by default is set to panic. + // + // However, we can overwrite that configuration, to either warn us once or never. // This is good for catching unexpected behavior without crashing the app, // but can lead to spam. .add_systems( From a2e7f3f32ef45e44a354e1ac51d10d1da682bd51 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 20 Mar 2025 14:10:39 -0700 Subject: [PATCH 02/14] Move the fallible system error handling module into bevy_ecs::error --- .../bevy_ecs/src/error/failed_system_param.rs | 83 ++++++++++++++++++ crates/bevy_ecs/src/error/mod.rs | 2 + crates/bevy_ecs/src/lib.rs | 4 +- crates/bevy_ecs/src/schedule/executor/mod.rs | 3 +- crates/bevy_ecs/src/system/function_system.rs | 87 ++----------------- 5 files changed, 96 insertions(+), 83 deletions(-) create mode 100644 crates/bevy_ecs/src/error/failed_system_param.rs diff --git a/crates/bevy_ecs/src/error/failed_system_param.rs b/crates/bevy_ecs/src/error/failed_system_param.rs new file mode 100644 index 0000000000000..59f07ab93eb60 --- /dev/null +++ b/crates/bevy_ecs/src/error/failed_system_param.rs @@ -0,0 +1,83 @@ +//! Tools for controlling the error behavior of fallible system parameters. + +use crate::system::{FunctionSystem, IntoSystem, SystemParam, SystemParamFunction}; + +/// State machine for emitting warnings when [system params are invalid](System::validate_param). +#[derive(Clone, Copy)] +pub enum ParamWarnPolicy { + /// Stop app with a panic. + Panic, + /// No warning should ever be emitted. + Never, + /// The warning will be emitted once and status will update to [`Self::Never`]. + Warn, +} + +impl ParamWarnPolicy { + /// Advances the warn policy after validation failed. + #[inline] + pub(crate) fn advance(&mut self) { + // Ignore `Panic` case, because it stops execution before this function gets called. + *self = Self::Never; + } + + /// Emits a warning about inaccessible system param if policy allows it. + #[inline] + pub(crate) fn try_warn

(&self, name: &str) + where + P: SystemParam, + { + match self { + Self::Panic => panic!( + "{0} could not access system parameter {1}", + name, + disqualified::ShortName::of::

() + ), + Self::Warn => { + log::warn!( + "{0} did not run because it requested inaccessible system parameter {1}", + name, + disqualified::ShortName::of::

() + ); + } + Self::Never => {} + } + } +} + +/// Trait for manipulating warn policy of systems. +/// +/// By default, the fallback behavior of a system with invalid parameters is to panic, +/// although that can be configured globally via the [`GLOBAL_ERROR_HANDLER`](bevy_ecs::error::GLOBAL_ERROR_HANDLER)., +/// found in [`bevy_ecs::error`]. +pub trait WithParamWarnPolicy +where + M: 'static, + F: SystemParamFunction, + Self: Sized, +{ + /// Set warn policy. + fn with_param_warn_policy(self, warn_policy: ParamWarnPolicy) -> FunctionSystem; + + /// Warn and ignore systems with invalid parameters. + fn warn_param_missing(self) -> FunctionSystem { + self.with_param_warn_policy(ParamWarnPolicy::Warn) + } + + /// Silently ignore systems with invalid parameters. + fn ignore_param_missing(self) -> FunctionSystem { + self.with_param_warn_policy(ParamWarnPolicy::Never) + } +} + +impl WithParamWarnPolicy for F +where + M: 'static, + F: SystemParamFunction, +{ + fn with_param_warn_policy(self, param_warn_policy: ParamWarnPolicy) -> FunctionSystem { + let mut system = IntoSystem::into_system(self); + system.set_param_warn_policy(param_warn_policy); + system + } +} diff --git a/crates/bevy_ecs/src/error/mod.rs b/crates/bevy_ecs/src/error/mod.rs index 950deee3ecf97..caba6a84476a9 100644 --- a/crates/bevy_ecs/src/error/mod.rs +++ b/crates/bevy_ecs/src/error/mod.rs @@ -69,10 +69,12 @@ mod bevy_error; mod command_handling; +mod failed_system_param; mod handler; pub use bevy_error::*; pub use command_handling::*; +pub use failed_system_param::*; pub use handler::*; /// A result type for use in fallible systems, commands and observers. diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 7ccfd4caa9774..0c9f2a66d3e72 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -74,7 +74,7 @@ pub mod prelude { children, component::Component, entity::{Entity, EntityBorrow, EntityMapper}, - error::{BevyError, Result}, + error::{BevyError, Result, WithParamWarnPolicy}, event::{Event, EventMutator, EventReader, EventWriter, Events}, hierarchy::{ChildOf, ChildSpawner, ChildSpawnerCommands, Children}, name::{Name, NameOrEntity}, @@ -93,7 +93,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, WithParamWarnPolicy, + SystemParamFunction, }, world::{ EntityMut, EntityRef, EntityWorldMut, FilteredResources, FilteredResourcesMut, diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index c892db86c4d28..dd7adbae4bd72 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -312,9 +312,10 @@ mod __rust_begin_short_backtrace { #[cfg(test)] mod tests { use crate::{ + error::WithParamWarnPolicy, prelude::{IntoScheduleConfigs, Resource, Schedule, SystemSet}, schedule::ExecutorKind, - system::{Commands, Res, WithParamWarnPolicy}, + system::{Commands, Res}, world::World, }; diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 13072ad9a8441..766a52381e21a 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -1,6 +1,7 @@ use crate::{ archetype::{ArchetypeComponentId, ArchetypeGeneration}, component::{ComponentId, Tick}, + error::ParamWarnPolicy, prelude::FromWorld, query::{Access, FilteredAccessSet}, schedule::{InternedSystemSet, SystemSet}, @@ -187,86 +188,6 @@ impl SystemMeta { } } -/// State machine for emitting warnings when [system params are invalid](System::validate_param). -#[derive(Clone, Copy)] -pub enum ParamWarnPolicy { - /// Stop app with a panic. - Panic, - /// No warning should ever be emitted. - Never, - /// The warning will be emitted once and status will update to [`Self::Never`]. - Warn, -} - -impl ParamWarnPolicy { - /// Advances the warn policy after validation failed. - #[inline] - fn advance(&mut self) { - // Ignore `Panic` case, because it stops execution before this function gets called. - *self = Self::Never; - } - - /// Emits a warning about inaccessible system param if policy allows it. - #[inline] - fn try_warn

(&self, name: &str) - where - P: SystemParam, - { - match self { - Self::Panic => panic!( - "{0} could not access system parameter {1}", - name, - disqualified::ShortName::of::

() - ), - Self::Warn => { - log::warn!( - "{0} did not run because it requested inaccessible system parameter {1}", - name, - disqualified::ShortName::of::

() - ); - } - Self::Never => {} - } - } -} - -/// Trait for manipulating warn policy of systems. -/// -/// By default, the fallback behavior of a system with invalid parameters is to panic, -/// although that can be configured globally via the [`GLOBAL_ERROR_HANDLER`](bevy_ecs::error::GLOBAL_ERROR_HANDLER)., -/// found in [`bevy_ecs::error`]. -pub trait WithParamWarnPolicy -where - M: 'static, - F: SystemParamFunction, - Self: Sized, -{ - /// Set warn policy. - fn with_param_warn_policy(self, warn_policy: ParamWarnPolicy) -> FunctionSystem; - - /// Warn and ignore systems with invalid parameters. - fn warn_param_missing(self) -> FunctionSystem { - self.with_param_warn_policy(ParamWarnPolicy::Warn) - } - - /// Silently ignore systems with invalid parameters. - fn ignore_param_missing(self) -> FunctionSystem { - self.with_param_warn_policy(ParamWarnPolicy::Never) - } -} - -impl WithParamWarnPolicy for F -where - M: 'static, - F: SystemParamFunction, -{ - fn with_param_warn_policy(self, param_warn_policy: ParamWarnPolicy) -> FunctionSystem { - let mut system = IntoSystem::into_system(self); - system.system_meta.set_param_warn_policy(param_warn_policy); - system - } -} - // TODO: Actually use this in FunctionSystem. We should probably only do this once Systems are constructed using a World reference // (to avoid the need for unwrapping to retrieve SystemMeta) /// Holds on to persistent state required to drive [`SystemParam`] for a [`System`]. @@ -702,6 +623,12 @@ where marker: PhantomData Marker>, } +impl> FunctionSystem { + pub(crate) fn set_param_warn_policy(&mut self, warn_policy: ParamWarnPolicy) { + self.system_meta.set_param_warn_policy(warn_policy); + } +} + /// The state of a [`FunctionSystem`], which must be initialized with /// [`System::initialize`] before the system can be run. A panic will occur if /// the system is run without being initialized. From c00ad94d3af06f2c5e4169f32fef221d57b10364 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 20 Mar 2025 14:24:11 -0700 Subject: [PATCH 03/14] Assorted cleanup for fallible_params example --- examples/ecs/fallible_params.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/examples/ecs/fallible_params.rs b/examples/ecs/fallible_params.rs index aed2171292e23..dfdfbf181d8b4 100644 --- a/examples/ecs/fallible_params.rs +++ b/examples/ecs/fallible_params.rs @@ -33,12 +33,16 @@ fn main() { .add_systems( Update, ( - user_input.warn_param_missing(), + user_input, + // This system uses `Populated`, and we want to silently skip it if there are no enemies. move_targets.ignore_param_missing(), - move_pointer.ignore_param_missing(), + // This system uses Single, and we want to silently skip it if the player doesn't exist. + track_targets.ignore_param_missing(), ) .chain(), ) + // This system will always fail validation, because we never create an entity with both `Player` and `Enemy` components. + // We want to warn exactly once if this system fails. .add_systems(Update, do_nothing_fail_validation.warn_param_missing()) .run(); } @@ -127,11 +131,11 @@ fn move_targets(mut enemies: Populated<(&mut Transform, &mut Enemy)>, time: Res< } } -/// System that moves the player. +/// System that moves the player, causing them to track a single enemy. /// The player will search for enemies if there are none. /// If there is one, player will track it. /// If there are too many enemies, the player will cease all action (the system will not run). -fn move_pointer( +fn track_targets( // `Single` ensures the system runs ONLY when exactly one matching entity exists. mut player: Single<(&mut Transform, &Player)>, // `Option` ensures that the system runs ONLY when zero or one matching entity exists. @@ -153,7 +157,7 @@ fn move_pointer( player_transform.translation += front * velocity; } } else { - // No enemy found, keep searching. + // 0 or multiple enemies found, keep searching. player_transform.rotate_axis(Dir3::Z, player.rotation_speed * time.delta_secs()); } } From 72a368bec381d9f6799f179bb2000eed677fa095 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 20 Mar 2025 14:58:36 -0700 Subject: [PATCH 04/14] Remove ParamWarnPolicy and friends completely --- .../bevy_ecs/src/error/failed_system_param.rs | 83 ------------------- crates/bevy_ecs/src/error/mod.rs | 2 - crates/bevy_ecs/src/lib.rs | 2 +- crates/bevy_ecs/src/schedule/executor/mod.rs | 10 +-- crates/bevy_ecs/src/system/function_system.rs | 33 -------- crates/bevy_ecs/src/system/system.rs | 2 +- crates/bevy_ecs/src/system/system_param.rs | 28 ++----- crates/bevy_ecs/src/system/system_registry.rs | 2 +- crates/bevy_render/src/extract_param.rs | 3 +- examples/ecs/fallible_params.rs | 6 +- 10 files changed, 15 insertions(+), 156 deletions(-) delete mode 100644 crates/bevy_ecs/src/error/failed_system_param.rs diff --git a/crates/bevy_ecs/src/error/failed_system_param.rs b/crates/bevy_ecs/src/error/failed_system_param.rs deleted file mode 100644 index 59f07ab93eb60..0000000000000 --- a/crates/bevy_ecs/src/error/failed_system_param.rs +++ /dev/null @@ -1,83 +0,0 @@ -//! Tools for controlling the error behavior of fallible system parameters. - -use crate::system::{FunctionSystem, IntoSystem, SystemParam, SystemParamFunction}; - -/// State machine for emitting warnings when [system params are invalid](System::validate_param). -#[derive(Clone, Copy)] -pub enum ParamWarnPolicy { - /// Stop app with a panic. - Panic, - /// No warning should ever be emitted. - Never, - /// The warning will be emitted once and status will update to [`Self::Never`]. - Warn, -} - -impl ParamWarnPolicy { - /// Advances the warn policy after validation failed. - #[inline] - pub(crate) fn advance(&mut self) { - // Ignore `Panic` case, because it stops execution before this function gets called. - *self = Self::Never; - } - - /// Emits a warning about inaccessible system param if policy allows it. - #[inline] - pub(crate) fn try_warn

(&self, name: &str) - where - P: SystemParam, - { - match self { - Self::Panic => panic!( - "{0} could not access system parameter {1}", - name, - disqualified::ShortName::of::

() - ), - Self::Warn => { - log::warn!( - "{0} did not run because it requested inaccessible system parameter {1}", - name, - disqualified::ShortName::of::

() - ); - } - Self::Never => {} - } - } -} - -/// Trait for manipulating warn policy of systems. -/// -/// By default, the fallback behavior of a system with invalid parameters is to panic, -/// although that can be configured globally via the [`GLOBAL_ERROR_HANDLER`](bevy_ecs::error::GLOBAL_ERROR_HANDLER)., -/// found in [`bevy_ecs::error`]. -pub trait WithParamWarnPolicy -where - M: 'static, - F: SystemParamFunction, - Self: Sized, -{ - /// Set warn policy. - fn with_param_warn_policy(self, warn_policy: ParamWarnPolicy) -> FunctionSystem; - - /// Warn and ignore systems with invalid parameters. - fn warn_param_missing(self) -> FunctionSystem { - self.with_param_warn_policy(ParamWarnPolicy::Warn) - } - - /// Silently ignore systems with invalid parameters. - fn ignore_param_missing(self) -> FunctionSystem { - self.with_param_warn_policy(ParamWarnPolicy::Never) - } -} - -impl WithParamWarnPolicy for F -where - M: 'static, - F: SystemParamFunction, -{ - fn with_param_warn_policy(self, param_warn_policy: ParamWarnPolicy) -> FunctionSystem { - let mut system = IntoSystem::into_system(self); - system.set_param_warn_policy(param_warn_policy); - system - } -} diff --git a/crates/bevy_ecs/src/error/mod.rs b/crates/bevy_ecs/src/error/mod.rs index caba6a84476a9..950deee3ecf97 100644 --- a/crates/bevy_ecs/src/error/mod.rs +++ b/crates/bevy_ecs/src/error/mod.rs @@ -69,12 +69,10 @@ mod bevy_error; mod command_handling; -mod failed_system_param; mod handler; pub use bevy_error::*; pub use command_handling::*; -pub use failed_system_param::*; pub use handler::*; /// A result type for use in fallible systems, commands and observers. diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 0c9f2a66d3e72..b46ee481e86ef 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -74,7 +74,7 @@ pub mod prelude { children, component::Component, entity::{Entity, EntityBorrow, EntityMapper}, - error::{BevyError, Result, WithParamWarnPolicy}, + error::{BevyError, Result}, event::{Event, EventMutator, EventReader, EventWriter, Events}, hierarchy::{ChildOf, ChildSpawner, ChildSpawnerCommands, Children}, name::{Name, NameOrEntity}, diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index dd7adbae4bd72..b03bac4083369 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -312,7 +312,6 @@ mod __rust_begin_short_backtrace { #[cfg(test)] mod tests { use crate::{ - error::WithParamWarnPolicy, prelude::{IntoScheduleConfigs, Resource, Schedule, SystemSet}, schedule::ExecutorKind, system::{Commands, Res}, @@ -347,8 +346,7 @@ mod tests { // This system depends on a system that is always skipped. (|mut commands: Commands| { commands.insert_resource(R2); - }) - .warn_param_missing(), + }), ) .chain(), ); @@ -371,20 +369,18 @@ mod tests { let mut world = World::new(); let mut schedule = Schedule::default(); schedule.set_executor_kind(executor); - schedule.configure_sets(S1.run_if((|_: Res| true).warn_param_missing())); + schedule.configure_sets(S1.run_if(|_: Res| true)); schedule.add_systems(( // System gets skipped if system set run conditions fail validation. (|mut commands: Commands| { commands.insert_resource(R1); }) - .warn_param_missing() .in_set(S1), // System gets skipped if run conditions fail validation. (|mut commands: Commands| { commands.insert_resource(R2); }) - .warn_param_missing() - .run_if((|_: Res| true).warn_param_missing()), + .run_if(|_: Res| true), )); schedule.run(&mut world); assert!(world.get_resource::().is_none()); diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 766a52381e21a..b3e8c06060f85 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -1,7 +1,6 @@ use crate::{ archetype::{ArchetypeComponentId, ArchetypeGeneration}, component::{ComponentId, Tick}, - error::ParamWarnPolicy, prelude::FromWorld, query::{Access, FilteredAccessSet}, schedule::{InternedSystemSet, SystemSet}, @@ -44,7 +43,6 @@ pub struct SystemMeta { is_send: bool, has_deferred: bool, pub(crate) last_run: Tick, - param_warn_policy: ParamWarnPolicy, #[cfg(feature = "trace")] pub(crate) system_span: Span, #[cfg(feature = "trace")] @@ -61,7 +59,6 @@ impl SystemMeta { is_send: true, has_deferred: false, last_run: Tick::new(0), - param_warn_policy: ParamWarnPolicy::Panic, #[cfg(feature = "trace")] system_span: info_span!("system", name = name), #[cfg(feature = "trace")] @@ -117,27 +114,6 @@ impl SystemMeta { self.has_deferred = true; } - /// Changes the warn policy. - #[inline] - pub(crate) fn set_param_warn_policy(&mut self, warn_policy: ParamWarnPolicy) { - self.param_warn_policy = warn_policy; - } - - /// Advances the warn policy after validation failed. - #[inline] - pub(crate) fn advance_param_warn_policy(&mut self) { - self.param_warn_policy.advance(); - } - - /// Emits a warning about inaccessible system param if policy allows it. - #[inline] - pub fn try_warn_param

(&self) - where - P: SystemParam, - { - self.param_warn_policy.try_warn::

(&self.name); - } - /// Archetype component access that is used to determine which systems can run in parallel with each other /// in the multithreaded executor. /// @@ -623,12 +599,6 @@ where marker: PhantomData Marker>, } -impl> FunctionSystem { - pub(crate) fn set_param_warn_policy(&mut self, warn_policy: ParamWarnPolicy) { - self.system_meta.set_param_warn_policy(warn_policy); - } -} - /// The state of a [`FunctionSystem`], which must be initialized with /// [`System::initialize`] before the system can be run. A panic will occur if /// the system is run without being initialized. @@ -785,9 +755,6 @@ where // - All world accesses used by `F::Param` have been registered, so the caller // will ensure that there are no data access conflicts. let is_valid = unsafe { F::Param::validate_param(param_state, &self.system_meta, world) }; - if !is_valid { - self.system_meta.advance_param_warn_policy(); - } is_valid } diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index b44cd29440546..7de3661d96ecb 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -462,7 +462,7 @@ mod tests { let mut world = World::default(); // This fails because `T` has not been added to the world yet. - let result = world.run_system_once(system.warn_param_missing()); + let result = world.run_system_once(system); assert!(matches!(result, Err(RunSystemError::InvalidParams(_)))); } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index ddfcddaa76e47..42d6d894e4194 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -433,9 +433,6 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam fo ) }; let is_valid = query.single_inner().is_ok(); - if !is_valid { - system_meta.try_warn_param::(); - } is_valid } } @@ -502,9 +499,6 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam }; let result = query.single_inner(); let is_valid = !matches!(result, Err(QuerySingleError::MultipleEntities(_))); - if !is_valid { - system_meta.try_warn_param::(); - } is_valid } } @@ -841,7 +835,7 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { #[inline] unsafe fn validate_param( &component_id: &Self::State, - system_meta: &SystemMeta, + _system_meta: &SystemMeta, world: UnsafeWorldCell, ) -> bool { // SAFETY: Read-only access to resource metadata. @@ -849,9 +843,6 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { .resources .get(component_id) .is_some_and(ResourceData::is_present); - if !is_valid { - system_meta.try_warn_param::(); - } is_valid } @@ -953,7 +944,7 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { #[inline] unsafe fn validate_param( &component_id: &Self::State, - system_meta: &SystemMeta, + _system_meta: &SystemMeta, world: UnsafeWorldCell, ) -> bool { // SAFETY: Read-only access to resource metadata. @@ -961,9 +952,6 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { .resources .get(component_id) .is_some_and(ResourceData::is_present); - if !is_valid { - system_meta.try_warn_param::(); - } is_valid } @@ -1523,7 +1511,7 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> { #[inline] unsafe fn validate_param( &component_id: &Self::State, - system_meta: &SystemMeta, + _system_meta: &SystemMeta, world: UnsafeWorldCell, ) -> bool { // SAFETY: Read-only access to resource metadata. @@ -1531,9 +1519,6 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> { .non_send_resources .get(component_id) .is_some_and(ResourceData::is_present); - if !is_valid { - system_meta.try_warn_param::(); - } is_valid } @@ -1632,7 +1617,7 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> { #[inline] unsafe fn validate_param( &component_id: &Self::State, - system_meta: &SystemMeta, + _system_meta: &SystemMeta, world: UnsafeWorldCell, ) -> bool { // SAFETY: Read-only access to resource metadata. @@ -1640,9 +1625,6 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> { .non_send_resources .get(component_id) .is_some_and(ResourceData::is_present); - if !is_valid { - system_meta.try_warn_param::(); - } is_valid } @@ -1999,7 +1981,7 @@ macro_rules! impl_system_param_tuple { reason = "Zero-length tuples won't use some of the parameters." )] $(#[$meta])* - // SAFETY: implementors of each `SystemParam` in the tuple have validated their impls + // SAFETY: implementers of each `SystemParam` in the tuple have validated their impls unsafe impl<$($param: SystemParam),*> SystemParam for ($($param,)*) { type State = ($($param::State,)*); type Item<'w, 's> = ($($param::Item::<'w, 's>,)*); diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index 11d74beca5c1a..2e74f4c5aa5d4 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -862,7 +862,7 @@ mod tests { fn system(_: Res) {} let mut world = World::new(); - let id = world.register_system(system.warn_param_missing()); + let id = world.register_system(system); // This fails because `T` has not been added to the world yet. let result = world.run_system(id); diff --git a/crates/bevy_render/src/extract_param.rs b/crates/bevy_render/src/extract_param.rs index 6ac7079bc5f49..e11c5160383f3 100644 --- a/crates/bevy_render/src/extract_param.rs +++ b/crates/bevy_render/src/extract_param.rs @@ -79,13 +79,12 @@ where #[inline] unsafe fn validate_param( state: &Self::State, - system_meta: &SystemMeta, + _system_meta: &SystemMeta, world: UnsafeWorldCell, ) -> bool { // 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 { - system_meta.try_warn_param::<&World>(); return false; }; // SAFETY: Type is guaranteed by `SystemState`. diff --git a/examples/ecs/fallible_params.rs b/examples/ecs/fallible_params.rs index dfdfbf181d8b4..8a24cab08cd5e 100644 --- a/examples/ecs/fallible_params.rs +++ b/examples/ecs/fallible_params.rs @@ -35,15 +35,15 @@ fn main() { ( user_input, // This system uses `Populated`, and we want to silently skip it if there are no enemies. - move_targets.ignore_param_missing(), + move_targets, // This system uses Single, and we want to silently skip it if the player doesn't exist. - track_targets.ignore_param_missing(), + track_targets, ) .chain(), ) // This system will always fail validation, because we never create an entity with both `Player` and `Enemy` components. // We want to warn exactly once if this system fails. - .add_systems(Update, do_nothing_fail_validation.warn_param_missing()) + .add_systems(Update, do_nothing_fail_validation) .run(); } From 0dcc436fa0b78b12350c877e3ed44edf0083405a Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 20 Mar 2025 15:04:32 -0700 Subject: [PATCH 05/14] Explain intended fallback behavior for validate_param --- crates/bevy_ecs/src/system/system_param.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 42d6d894e4194..a25f80c523d93 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -232,7 +232,10 @@ pub unsafe trait SystemParam: Sized { fn queue(state: &mut Self::State, system_meta: &SystemMeta, world: DeferredWorld) {} /// Validates that the param can be acquired by the [`get_param`](SystemParam::get_param). - /// Built-in executors use this to prevent systems with invalid params from running. + /// + /// Built-in executors use this to prevent systems with invalid params from running, + /// and any failures here will be bubbled up to the default error handler defined in [`bevy_ecs::error`]. + /// /// For nested [`SystemParam`]s validation will fail if any /// delegated validation fails. /// From 71f93cf370c9d4660a051db73faff0be2683f5b9 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 20 Mar 2025 15:30:01 -0700 Subject: [PATCH 06/14] Use the GLOBAL_ERROR_HANDLER for validation behavior in SystemParams for systems and run conditions --- .../src/schedule/executor/multi_threaded.rs | 23 +++++++++++++++++-- .../bevy_ecs/src/schedule/executor/simple.rs | 22 +++++++++++++++++- .../src/schedule/executor/single_threaded.rs | 21 ++++++++++++++++- crates/bevy_ecs/src/system/system.rs | 2 +- crates/bevy_ecs/src/system/system_param.rs | 18 ++++++++++++++- examples/ecs/fallible_params.rs | 13 +++++++---- 6 files changed, 89 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs index bf63de8dc57c0..51dac2741a92f 100644 --- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs @@ -14,11 +14,11 @@ use tracing::{info_span, Span}; use crate::{ archetype::ArchetypeComponentId, - error::{BevyError, ErrorContext, Result}, + error::{default_error_handler, BevyError, ErrorContext, Result}, prelude::Resource, query::Access, schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule}, - system::ScheduleSystem, + system::{ScheduleSystem, SystemParamValidationError}, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; @@ -536,6 +536,7 @@ impl ExecutorState { world: UnsafeWorldCell, ) -> bool { let mut should_run = !self.skipped_systems.contains(system_index); + let error_handler = default_error_handler(); for set_idx in conditions.sets_with_conditions_of_systems[system_index].ones() { if self.evaluated_sets.contains(set_idx) { @@ -582,6 +583,14 @@ impl ExecutorState { // - `update_archetype_component_access` has been called for system. let valid_params = unsafe { system.validate_param_unsafe(world) }; if !valid_params { + error_handler( + SystemParamValidationError::System.into(), + ErrorContext::System { + name: system.name(), + last_run: system.get_last_run(), + }, + ); + self.skipped_systems.insert(system_index); } should_run &= valid_params; @@ -767,6 +776,8 @@ unsafe fn evaluate_and_fold_conditions( conditions: &mut [BoxedCondition], world: UnsafeWorldCell, ) -> bool { + let error_handler = default_error_handler(); + #[expect( clippy::unnecessary_fold, reason = "Short-circuiting here would prevent conditions from mutating their own state as needed." @@ -779,6 +790,14 @@ unsafe fn evaluate_and_fold_conditions( // required by the condition. // - `update_archetype_component_access` has been called for condition. if !unsafe { condition.validate_param_unsafe(world) } { + error_handler( + SystemParamValidationError::RunCondition.into(), + ErrorContext::System { + name: condition.name(), + last_run: condition.get_last_run(), + }, + ); + return false; } // SAFETY: diff --git a/crates/bevy_ecs/src/schedule/executor/simple.rs b/crates/bevy_ecs/src/schedule/executor/simple.rs index 9088cadc10623..eec14f6936089 100644 --- a/crates/bevy_ecs/src/schedule/executor/simple.rs +++ b/crates/bevy_ecs/src/schedule/executor/simple.rs @@ -8,10 +8,11 @@ use tracing::info_span; use std::eprintln; use crate::{ - error::{BevyError, ErrorContext}, + error::{default_error_handler, BevyError, ErrorContext}, schedule::{ executor::is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule, }, + system::SystemParamValidationError, world::World, }; @@ -88,6 +89,16 @@ impl SystemExecutor for SimpleExecutor { let system = &mut schedule.systems[system_index]; if should_run { let valid_params = system.validate_param(world); + if !valid_params { + error_handler( + SystemParamValidationError::System.into(), + ErrorContext::System { + name: system.name(), + last_run: system.get_last_run(), + }, + ); + } + should_run &= valid_params; } @@ -153,6 +164,8 @@ impl SimpleExecutor { } fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut World) -> bool { + let error_handler = default_error_handler(); + #[expect( clippy::unnecessary_fold, reason = "Short-circuiting here would prevent conditions from mutating their own state as needed." @@ -161,6 +174,13 @@ fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut W .iter_mut() .map(|condition| { if !condition.validate_param(world) { + error_handler( + SystemParamValidationError::RunCondition.into(), + ErrorContext::System { + name: condition.name(), + last_run: condition.get_last_run(), + }, + ); return false; } __rust_begin_short_backtrace::readonly_run(&mut **condition, world) diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs index 0db5f7522efb5..d62409ff0842b 100644 --- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs @@ -8,8 +8,9 @@ use tracing::info_span; use std::eprintln; use crate::{ - error::{BevyError, ErrorContext}, + error::{default_error_handler, BevyError, ErrorContext}, schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule}, + system::SystemParamValidationError, world::World, }; @@ -94,6 +95,15 @@ impl SystemExecutor for SingleThreadedExecutor { let system = &mut schedule.systems[system_index]; if should_run { let valid_params = system.validate_param(world); + if !valid_params { + error_handler( + SystemParamValidationError::System.into(), + ErrorContext::System { + name: system.name(), + last_run: system.get_last_run(), + }, + ); + } should_run &= valid_params; } @@ -196,6 +206,8 @@ impl SingleThreadedExecutor { } fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut World) -> bool { + let error_handler: fn(BevyError, ErrorContext) = default_error_handler(); + #[expect( clippy::unnecessary_fold, reason = "Short-circuiting here would prevent conditions from mutating their own state as needed." @@ -204,6 +216,13 @@ fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut W .iter_mut() .map(|condition| { if !condition.validate_param(world) { + error_handler( + SystemParamValidationError::RunCondition.into(), + ErrorContext::System { + name: condition.name(), + last_run: condition.get_last_run(), + }, + ); return false; } __rust_begin_short_backtrace::readonly_run(&mut **condition, world) diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index 7de3661d96ecb..8289f88211c19 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -150,7 +150,7 @@ pub trait System: Send + Sync + 'static { /// Update the system's archetype component [`Access`]. /// - /// ## Note for implementors + /// ## 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); diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index a25f80c523d93..746d81c880a40 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -29,6 +29,7 @@ use core::{ panic::Location, }; use disqualified::ShortName; +use thiserror::Error; use super::Populated; use variadics_please::{all_tuples, all_tuples_enumerated}; @@ -234,7 +235,8 @@ pub unsafe trait SystemParam: Sized { /// Validates that the param can be acquired by the [`get_param`](SystemParam::get_param). /// /// Built-in executors use this to prevent systems with invalid params from running, - /// and any failures here will be bubbled up to the default error handler defined in [`bevy_ecs::error`]. + /// and any failures here will be bubbled up to the default error handler defined in [`bevy_ecs::error`], + /// with a value of type [`SystemParamValidationError`]. /// /// For nested [`SystemParam`]s validation will fail if any /// delegated validation fails. @@ -2577,6 +2579,20 @@ unsafe impl SystemParam for FilteredResourcesMut<'_, '_> { } } +/// An error that occurs when a system parameter is not valid. +/// +/// Generated when [`SystemParam::validate_param`] returns `false`, +/// and handled using the unified error handling mechanisms defined in [`bevy_ecs::error`]. +#[derive(Debug, PartialEq, Eq, Clone, Error)] +pub enum SystemParamValidationError { + /// A system failed to validate a parameter. + #[error("System failed to validate parameter")] + System, + /// A run condition failed to validate a parameter. + #[error("Run condition failed to validate parameter")] + RunCondition, +} + #[cfg(test)] mod tests { use super::*; diff --git a/examples/ecs/fallible_params.rs b/examples/ecs/fallible_params.rs index 8a24cab08cd5e..cecf8ef5f6461 100644 --- a/examples/ecs/fallible_params.rs +++ b/examples/ecs/fallible_params.rs @@ -7,13 +7,21 @@ //! - [`Option>`] - There must be zero or one matching entity. //! - [`Populated`] - There must be at least one matching entity. //! -//! To set the fallback behavior for when a parameter fails to be fetched, +//! To learn more about setting the fallback behavior for when a parameter fails to be fetched, //! please see the `error_handling.rs` example. use bevy::prelude::*; +use bevy_ecs::error::{warn, GLOBAL_ERROR_HANDLER}; use rand::Rng; fn main() { + // By default, if a parameter fail to be fetched, + // the `GLOBAL_ERROR_HANDLER` will be used to handle the error, + // which by default is set to panic. + GLOBAL_ERROR_HANDLER + .set(warn) + .expect("The error handler can only be set once, globally."); + println!(); println!("Press 'A' to add enemy ships and 'R' to remove them."); println!("Player ship will wait for enemy ships and track one if it exists,"); @@ -23,9 +31,6 @@ fn main() { App::new() .add_plugins(DefaultPlugins) .add_systems(Startup, setup) - // By default, if a parameter fail to be fetched, - // the `GLOBAL_ERROR_HANDLER` will be used to handle the error, - // which by default is set to panic. // // However, we can overwrite that configuration, to either warn us once or never. // This is good for catching unexpected behavior without crashing the app, From 098c8cdd41eda15695b0070b8d45eba5ef8138e3 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 20 Mar 2025 16:05:08 -0700 Subject: [PATCH 07/14] Uncontroversial CI fixes --- crates/bevy_ecs/src/system/function_system.rs | 3 +-- crates/bevy_ecs/src/system/system_param.rs | 3 +-- examples/ecs/fallible_params.rs | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index b3e8c06060f85..7bcd3887cfb07 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -754,8 +754,7 @@ where // if the world does not match. // - All world accesses used by `F::Param` have been registered, so the caller // will ensure that there are no data access conflicts. - let is_valid = unsafe { F::Param::validate_param(param_state, &self.system_meta, world) }; - is_valid + unsafe { F::Param::validate_param(param_state, &self.system_meta, world) } } #[inline] diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 746d81c880a40..c68f4c550572f 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -503,8 +503,7 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam ) }; let result = query.single_inner(); - let is_valid = !matches!(result, Err(QuerySingleError::MultipleEntities(_))); - is_valid + !matches!(result, Err(QuerySingleError::MultipleEntities(_))) } } diff --git a/examples/ecs/fallible_params.rs b/examples/ecs/fallible_params.rs index cecf8ef5f6461..55ba57af2018d 100644 --- a/examples/ecs/fallible_params.rs +++ b/examples/ecs/fallible_params.rs @@ -10,8 +10,8 @@ //! To learn more about setting the fallback behavior for when a parameter fails to be fetched, //! please see the `error_handling.rs` example. +use bevy::ecs::error::{warn, GLOBAL_ERROR_HANDLER}; use bevy::prelude::*; -use bevy_ecs::error::{warn, GLOBAL_ERROR_HANDLER}; use rand::Rng; fn main() { From 272ad8552c5089f80ff52bc7344466222c423ff8 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 20 Mar 2025 16:06:33 -0700 Subject: [PATCH 08/14] Remove test that cannot be fixed currently --- crates/bevy_ecs/src/schedule/executor/mod.rs | 31 +------------------- 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index b03bac4083369..74d7943f2b0fb 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -314,7 +314,7 @@ mod tests { use crate::{ prelude::{IntoScheduleConfigs, Resource, Schedule, SystemSet}, schedule::ExecutorKind, - system::{Commands, Res}, + system::Commands, world::World, }; @@ -357,33 +357,4 @@ mod tests { #[derive(SystemSet, Hash, Debug, PartialEq, Eq, Clone)] struct S1; - - #[test] - fn invalid_condition_param_skips_system() { - for executor in EXECUTORS { - invalid_condition_param_skips_system_core(executor); - } - } - - fn invalid_condition_param_skips_system_core(executor: ExecutorKind) { - let mut world = World::new(); - let mut schedule = Schedule::default(); - schedule.set_executor_kind(executor); - schedule.configure_sets(S1.run_if(|_: Res| true)); - schedule.add_systems(( - // System gets skipped if system set run conditions fail validation. - (|mut commands: Commands| { - commands.insert_resource(R1); - }) - .in_set(S1), - // System gets skipped if run conditions fail validation. - (|mut commands: Commands| { - commands.insert_resource(R2); - }) - .run_if(|_: Res| true), - )); - schedule.run(&mut world); - assert!(world.get_resource::().is_none()); - assert!(world.get_resource::().is_none()); - } } From d2ffe4a8c33c2aa3c17bc8cabfca163adfe6ac09 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 20 Mar 2025 16:09:56 -0700 Subject: [PATCH 09/14] Use global error handler when observers fail validation --- crates/bevy_ecs/src/observer/runner.rs | 10 +++++++++- crates/bevy_ecs/src/system/system_param.rs | 3 +++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index 95fda3b2f168a..ac478df56e96a 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -7,7 +7,7 @@ use crate::{ observer::{ObserverDescriptor, ObserverTrigger}, prelude::*, query::DebugCheckedUnwrap, - system::{IntoObserverSystem, ObserverSystem}, + system::{IntoObserverSystem, ObserverSystem, SystemParamValidationError}, world::DeferredWorld, }; use bevy_ptr::PtrMut; @@ -416,6 +416,14 @@ fn observer_system_runner>( ); }; (*system).queue_deferred(world.into_deferred()); + } else { + error_handler( + SystemParamValidationError::Observer.into(), + ErrorContext::Observer { + name: (*system).name(), + last_run: (*system).get_last_run(), + }, + ); } } } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index c68f4c550572f..f57120266c7e9 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -2590,6 +2590,9 @@ pub enum SystemParamValidationError { /// A run condition failed to validate a parameter. #[error("Run condition failed to validate parameter")] RunCondition, + /// An observer failed to validate a parameter. + #[error("Observer failed to validate parameter")] + Observer, } #[cfg(test)] From bd4fbaa685263f15335059e82c8c2d66f2ce3ac2 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 20 Mar 2025 16:24:28 -0700 Subject: [PATCH 10/14] Add required feature to example --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 476c1ba15f2fd..36ea0f452a000 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2214,6 +2214,7 @@ wasm = false name = "fallible_params" path = "examples/ecs/fallible_params.rs" doc-scrape-examples = true +required-features = ["configurable_error_handler"] [package.metadata.example.fallible_params] name = "Fallible System Parameters" From 5527f08d98b1c714b2e42623d89807731ba947a0 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 23 Mar 2025 14:53:51 -0700 Subject: [PATCH 11/14] Simplify SystemParamValidationError as type of failure is stored in ErrorContext --- crates/bevy_ecs/src/error/handler.rs | 14 +++++++++++++- crates/bevy_ecs/src/observer/runner.rs | 2 +- .../src/schedule/executor/multi_threaded.rs | 4 ++-- crates/bevy_ecs/src/schedule/executor/simple.rs | 6 +++--- .../src/schedule/executor/single_threaded.rs | 6 +++--- crates/bevy_ecs/src/system/system_param.rs | 15 +++------------ 6 files changed, 25 insertions(+), 22 deletions(-) diff --git a/crates/bevy_ecs/src/error/handler.rs b/crates/bevy_ecs/src/error/handler.rs index af3e45f89219d..4fb6507a34ab6 100644 --- a/crates/bevy_ecs/src/error/handler.rs +++ b/crates/bevy_ecs/src/error/handler.rs @@ -15,6 +15,13 @@ pub enum ErrorContext { /// The last tick that the system was run. last_run: Tick, }, + /// The error occurred in a run condition. + RunCondition { + /// The name of the run condition that failed. + name: Cow<'static, str>, + /// The last tick that the run condition was evaluated. + last_run: Tick, + }, /// The error occurred in a command. Command { /// The name of the command that failed. @@ -39,6 +46,9 @@ impl Display for ErrorContext { Self::Observer { name, .. } => { write!(f, "Observer `{}` failed", name) } + Self::RunCondition { name, .. } => { + write!(f, "Run condition `{}` failed", name) + } } } } @@ -49,7 +59,8 @@ impl ErrorContext { match self { Self::System { name, .. } | Self::Command { name, .. } - | Self::Observer { name, .. } => name, + | Self::Observer { name, .. } + | Self::RunCondition { name, .. } => name, } } @@ -61,6 +72,7 @@ impl ErrorContext { Self::System { .. } => "system", Self::Command { .. } => "command", Self::Observer { .. } => "observer", + Self::RunCondition { .. } => "run condition", } } } diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index ac478df56e96a..1e2f60d9635f2 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -418,7 +418,7 @@ fn observer_system_runner>( (*system).queue_deferred(world.into_deferred()); } else { error_handler( - SystemParamValidationError::Observer.into(), + SystemParamValidationError.into(), ErrorContext::Observer { name: (*system).name(), last_run: (*system).get_last_run(), diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs index 51dac2741a92f..56e1750ecb452 100644 --- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs @@ -584,7 +584,7 @@ impl ExecutorState { let valid_params = unsafe { system.validate_param_unsafe(world) }; if !valid_params { error_handler( - SystemParamValidationError::System.into(), + SystemParamValidationError.into(), ErrorContext::System { name: system.name(), last_run: system.get_last_run(), @@ -791,7 +791,7 @@ unsafe fn evaluate_and_fold_conditions( // - `update_archetype_component_access` has been called for condition. if !unsafe { condition.validate_param_unsafe(world) } { error_handler( - SystemParamValidationError::RunCondition.into(), + SystemParamValidationError.into(), ErrorContext::System { name: condition.name(), last_run: condition.get_last_run(), diff --git a/crates/bevy_ecs/src/schedule/executor/simple.rs b/crates/bevy_ecs/src/schedule/executor/simple.rs index eec14f6936089..6a92e35c119f3 100644 --- a/crates/bevy_ecs/src/schedule/executor/simple.rs +++ b/crates/bevy_ecs/src/schedule/executor/simple.rs @@ -91,7 +91,7 @@ impl SystemExecutor for SimpleExecutor { let valid_params = system.validate_param(world); if !valid_params { error_handler( - SystemParamValidationError::System.into(), + SystemParamValidationError.into(), ErrorContext::System { name: system.name(), last_run: system.get_last_run(), @@ -175,8 +175,8 @@ fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut W .map(|condition| { if !condition.validate_param(world) { error_handler( - SystemParamValidationError::RunCondition.into(), - ErrorContext::System { + SystemParamValidationError.into(), + ErrorContext::RunCondition { name: condition.name(), last_run: condition.get_last_run(), }, diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs index d62409ff0842b..de16b22b4709c 100644 --- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs @@ -97,7 +97,7 @@ impl SystemExecutor for SingleThreadedExecutor { let valid_params = system.validate_param(world); if !valid_params { error_handler( - SystemParamValidationError::System.into(), + SystemParamValidationError.into(), ErrorContext::System { name: system.name(), last_run: system.get_last_run(), @@ -217,8 +217,8 @@ fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut W .map(|condition| { if !condition.validate_param(world) { error_handler( - SystemParamValidationError::RunCondition.into(), - ErrorContext::System { + SystemParamValidationError.into(), + ErrorContext::RunCondition { name: condition.name(), last_run: condition.get_last_run(), }, diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index f57120266c7e9..868397a5eca96 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -28,6 +28,7 @@ use core::{ ops::{Deref, DerefMut}, panic::Location, }; +use derive_more::derive::Display; use disqualified::ShortName; use thiserror::Error; @@ -2582,18 +2583,8 @@ unsafe impl SystemParam for FilteredResourcesMut<'_, '_> { /// /// Generated when [`SystemParam::validate_param`] returns `false`, /// and handled using the unified error handling mechanisms defined in [`bevy_ecs::error`]. -#[derive(Debug, PartialEq, Eq, Clone, Error)] -pub enum SystemParamValidationError { - /// A system failed to validate a parameter. - #[error("System failed to validate parameter")] - System, - /// A run condition failed to validate a parameter. - #[error("Run condition failed to validate parameter")] - RunCondition, - /// An observer failed to validate a parameter. - #[error("Observer failed to validate parameter")] - Observer, -} +#[derive(Debug, PartialEq, Eq, Clone, Display, Error)] +pub struct SystemParamValidationError; #[cfg(test)] mod tests { From 8b983cb302f08698c5cdcbeb7a357b5ace4bbec2 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 23 Mar 2025 15:01:55 -0700 Subject: [PATCH 12/14] Remove misleading example comments for now --- examples/ecs/fallible_params.rs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/examples/ecs/fallible_params.rs b/examples/ecs/fallible_params.rs index 55ba57af2018d..fffac8dc6e7d8 100644 --- a/examples/ecs/fallible_params.rs +++ b/examples/ecs/fallible_params.rs @@ -35,19 +35,8 @@ fn main() { // However, we can overwrite that configuration, to either warn us once or never. // This is good for catching unexpected behavior without crashing the app, // but can lead to spam. - .add_systems( - Update, - ( - user_input, - // This system uses `Populated`, and we want to silently skip it if there are no enemies. - move_targets, - // This system uses Single, and we want to silently skip it if the player doesn't exist. - track_targets, - ) - .chain(), - ) + .add_systems(Update, (user_input, move_targets, track_targets).chain()) // This system will always fail validation, because we never create an entity with both `Player` and `Enemy` components. - // We want to warn exactly once if this system fails. .add_systems(Update, do_nothing_fail_validation) .run(); } From 3ae2f341018326c7bc56a8d2e539b6c653661bb2 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 23 Mar 2025 20:20:33 -0400 Subject: [PATCH 13/14] Remove another outdated comment Co-authored-by: MiniaczQ --- examples/ecs/fallible_params.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/examples/ecs/fallible_params.rs b/examples/ecs/fallible_params.rs index fffac8dc6e7d8..cb533f901a7de 100644 --- a/examples/ecs/fallible_params.rs +++ b/examples/ecs/fallible_params.rs @@ -31,10 +31,6 @@ fn main() { App::new() .add_plugins(DefaultPlugins) .add_systems(Startup, setup) - // - // However, we can overwrite that configuration, to either warn us once or never. - // This is good for catching unexpected behavior without crashing the app, - // but can lead to spam. .add_systems(Update, (user_input, move_targets, track_targets).chain()) // This system will always fail validation, because we never create an entity with both `Player` and `Enemy` components. .add_systems(Update, do_nothing_fail_validation) From fdae8aea4f42704875ef303e02adf6d958cfdda9 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 23 Mar 2025 19:17:27 -0700 Subject: [PATCH 14/14] Properly qualify NonSendMarker import --- crates/bevy_render/src/view/window/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_render/src/view/window/mod.rs b/crates/bevy_render/src/view/window/mod.rs index cb6eb5876269c..622b17bef78f4 100644 --- a/crates/bevy_render/src/view/window/mod.rs +++ b/crates/bevy_render/src/view/window/mod.rs @@ -304,7 +304,7 @@ const DEFAULT_DESIRED_MAXIMUM_FRAME_LATENCY: u32 = 2; pub fn create_surfaces( // By accessing a NonSend resource, we tell the scheduler to put this system on the main thread, // which is necessary for some OS's - #[cfg(any(target_os = "macos", target_os = "ios"))] _marker: NonSendMarker, + #[cfg(any(target_os = "macos", target_os = "ios"))] _marker: bevy_ecs::system::NonSendMarker, windows: Res, mut window_surfaces: ResMut, render_instance: Res,