diff --git a/crates/bevy_ecs/src/change_detection.rs b/crates/bevy_ecs/src/change_detection.rs index 025c3804e73e2..72cf8c6784edb 100644 --- a/crates/bevy_ecs/src/change_detection.rs +++ b/crates/bevy_ecs/src/change_detection.rs @@ -544,7 +544,7 @@ impl<'w> From> for Ticks<'w> { /// If you need a unique mutable borrow, use [`ResMut`] instead. /// /// This [`SystemParam`](crate::system::SystemParam) fails validation if resource doesn't exist. -/// This will cause systems that use this parameter to be skipped. +/// This will cause a panic, but can be configured to do nothing or warn once. /// /// Use [`Option>`] instead if the resource might not always exist. pub struct Res<'w, T: ?Sized + Resource> { @@ -622,7 +622,7 @@ impl_debug!(Res<'w, T>, Resource); /// If you need a shared borrow, use [`Res`] instead. /// /// This [`SystemParam`](crate::system::SystemParam) fails validation if resource doesn't exist. -/// This will cause systems that use this parameter to be skipped. +/// /// This will cause a panic, but can be configured to do nothing or warn once. /// /// Use [`Option>`] instead if the resource might not always exist. pub struct ResMut<'w, T: ?Sized + Resource> { @@ -683,7 +683,7 @@ impl<'w, T: Resource> From> for Mut<'w, T> { /// over to another thread. /// /// This [`SystemParam`](crate::system::SystemParam) fails validation if non-send resource doesn't exist. -/// This will cause systems that use this parameter to be skipped. +/// /// This will cause a panic, but can be configured to do nothing or warn once. /// /// Use [`Option>`] instead if the resource might not always exist. pub struct NonSendMut<'w, T: ?Sized + 'static> { diff --git a/crates/bevy_ecs/src/schedule/condition.rs b/crates/bevy_ecs/src/schedule/condition.rs index 6956d2715069f..e6ddaa9590bc9 100644 --- a/crates/bevy_ecs/src/schedule/condition.rs +++ b/crates/bevy_ecs/src/schedule/condition.rs @@ -80,7 +80,7 @@ pub trait Condition: sealed::Condition /// /// # Examples /// - /// ``` + /// ```should_panic /// use bevy_ecs::prelude::*; /// /// #[derive(Resource, PartialEq)] @@ -90,7 +90,7 @@ pub trait Condition: sealed::Condition /// # let mut world = World::new(); /// # fn my_system() {} /// app.add_systems( - /// // The `resource_equals` run condition will fail since we don't initialize `R`, + /// // The `resource_equals` run condition will panic since we don't initialize `R`, /// // just like if we used `Res` in a system. /// my_system.run_if(resource_equals(R(0))), /// ); @@ -131,7 +131,7 @@ pub trait Condition: sealed::Condition /// /// # Examples /// - /// ``` + /// ```should_panic /// use bevy_ecs::prelude::*; /// /// #[derive(Resource, PartialEq)] @@ -141,7 +141,7 @@ pub trait Condition: sealed::Condition /// # let mut world = World::new(); /// # fn my_system() {} /// app.add_systems( - /// // The `resource_equals` run condition will fail since we don't initialize `R`, + /// // The `resource_equals` run condition will panic since we don't initialize `R`, /// // just like if we used `Res` in a system. /// my_system.run_if(resource_equals(R(0))), /// ); diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index d0231bb2f0c5b..aec181aff7b93 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -292,7 +292,7 @@ mod tests { self as bevy_ecs, prelude::{IntoSystemConfigs, IntoSystemSetConfigs, Resource, Schedule, SystemSet}, schedule::ExecutorKind, - system::{Commands, In, IntoSystem, Res}, + system::{Commands, Res, WithParamWarnPolicy}, world::World, }; @@ -321,15 +321,11 @@ mod tests { schedule.set_executor_kind(executor); schedule.add_systems( ( - // Combined systems get skipped together. - (|mut commands: Commands| { - commands.insert_resource(R1); - }) - .pipe(|_: In<()>, _: Res| {}), // This system depends on a system that is always skipped. - |mut commands: Commands| { + (|mut commands: Commands| { commands.insert_resource(R2); - }, + }) + .param_warn_once(), ) .chain(), ); @@ -352,18 +348,20 @@ 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)); + schedule.configure_sets(S1.run_if((|_: Res| true).param_warn_once())); schedule.add_systems(( // System gets skipped if system set run conditions fail validation. (|mut commands: Commands| { commands.insert_resource(R1); }) + .param_warn_once() .in_set(S1), // System gets skipped if run conditions fail validation. (|mut commands: Commands| { commands.insert_resource(R2); }) - .run_if(|_: Res| true), + .param_warn_once() + .run_if((|_: Res| true).param_warn_once()), )); schedule.run(&mut world); assert!(world.get_resource::().is_none()); diff --git a/crates/bevy_ecs/src/system/combinator.rs b/crates/bevy_ecs/src/system/combinator.rs index e21f35eee4e82..a312ace6ef9bd 100644 --- a/crates/bevy_ecs/src/system/combinator.rs +++ b/crates/bevy_ecs/src/system/combinator.rs @@ -214,7 +214,7 @@ where #[inline] unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> bool { // SAFETY: Delegate to other `System` implementations. - unsafe { self.a.validate_param_unsafe(world) && self.b.validate_param_unsafe(world) } + unsafe { self.a.validate_param_unsafe(world) } } fn initialize(&mut self, world: &mut World) { @@ -433,7 +433,7 @@ where unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> bool { // SAFETY: Delegate to other `System` implementations. - unsafe { self.a.validate_param_unsafe(world) && self.b.validate_param_unsafe(world) } + unsafe { self.a.validate_param_unsafe(world) } } fn validate_param(&mut self, world: &World) -> bool { diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 87950de4e4fb8..92a1aceeb4c13 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -60,7 +60,7 @@ impl SystemMeta { is_send: true, has_deferred: false, last_run: Tick::new(0), - param_warn_policy: ParamWarnPolicy::Once, + param_warn_policy: ParamWarnPolicy::Panic, #[cfg(feature = "trace")] system_span: info_span!("system", name = name), #[cfg(feature = "trace")] @@ -190,6 +190,8 @@ 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`]. @@ -200,6 +202,7 @@ 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; } @@ -209,15 +212,21 @@ impl ParamWarnPolicy { where P: SystemParam, { - if matches!(self, Self::Never) { - return; + match self { + Self::Panic => panic!( + "{0} could not access system parameter {1}", + name, + disqualified::ShortName::of::

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

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

() - ); } } @@ -232,6 +241,11 @@ where /// Set warn policy. fn with_param_warn_policy(self, warn_policy: ParamWarnPolicy) -> FunctionSystem; + /// Warn only once about invalid system parameters. + fn param_warn_once(self) -> FunctionSystem { + self.with_param_warn_policy(ParamWarnPolicy::Once) + } + /// Disable all param warnings. fn never_param_warn(self) -> FunctionSystem { self.with_param_warn_policy(ParamWarnPolicy::Never) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 26440c0c643f4..7279b8a154394 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1648,7 +1648,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 systems that use this parameter to be skipped. +/// /// This will cause a panic, but can be configured to do nothing or warn once. /// /// Use [`Option>`] instead if zero or one matching entities can exist. /// @@ -1684,7 +1684,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 systems that use this parameter to be skipped. +/// /// This will cause a panic, but can be configured to do nothing or warn once. /// /// 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) diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index c1c628cb2e1aa..1737d2d344b52 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -450,7 +450,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); + let result = world.run_system_once(system.param_warn_once()); 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 2186e2b8a3d10..0c6d50e160206 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1337,7 +1337,7 @@ unsafe impl SystemParam for Deferred<'_, T> { /// over to another thread. /// /// This [`SystemParam`] fails validation if non-send resource doesn't exist. -/// This will cause systems that use this parameter to be skipped. +/// /// This will cause a panic, but can be configured to do nothing or warn once. /// /// Use [`Option>`] instead if the resource might not always exist. pub struct NonSend<'w, T: 'static> { diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index f00740a1f7942..e5eca836dda7b 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -965,7 +965,7 @@ mod tests { fn system(_: Res) {} let mut world = World::new(); - let id = world.register_system_cached(system); + let id = world.register_system(system.param_warn_once()); // This fails because `T` has not been added to the world yet. let result = world.run_system(id); diff --git a/examples/ecs/fallible_params.rs b/examples/ecs/fallible_params.rs index 15a75944f01ae..129a3859d495f 100644 --- a/examples/ecs/fallible_params.rs +++ b/examples/ecs/fallible_params.rs @@ -20,22 +20,20 @@ fn main() { App::new() .add_plugins(DefaultPlugins) .add_systems(Startup, setup) - // Systems that fail parameter validation will emit warnings. - // The default policy is to emit a warning once per system. - // This is good for catching unexpected behavior, but can - // lead to spam. You can disable invalid param warnings - // per system using the `.never_param_warn()` method. + // Default system policy is to panic if parameters fail to be fetched. + // We 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, + user_input.param_warn_once(), move_targets.never_param_warn(), move_pointer.never_param_warn(), ) .chain(), ) - // We will leave this systems with default warning policy. - .add_systems(Update, do_nothing_fail_validation) + .add_systems(Update, do_nothing_fail_validation.param_warn_once()) .run(); }