From 9d3a8ed3d03af009dd289325601ea40a20ffcb99 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 4 Mar 2026 10:20:38 -0800 Subject: [PATCH 01/23] Describe strategy in the migration guide --- .../migration-guides/validation_merging.md | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 release-content/migration-guides/validation_merging.md diff --git a/release-content/migration-guides/validation_merging.md b/release-content/migration-guides/validation_merging.md new file mode 100644 index 0000000000000..d3abf2a772be3 --- /dev/null +++ b/release-content/migration-guides/validation_merging.md @@ -0,0 +1,25 @@ +--- +title: "`SystemParam` validation is now done when fetching the data" +pull_requests: [TODO] +--- + +In an effort to improve performance by reducing redundant data fetches and simplify internals, +system parameter validation is now done as part of fetching the data for those system parameters. +To be more precise: + +- `SystemParam::get_param` now returns a `Result, SystemParamValidationError>`, instead of simply a `Self::Item<'world, 'state>` + - If validation fails, an appropriate `SystemParamValidationError` should be returned + - If validation passes, the item should be returned wrapped in `Ok` +- `SystemParam::validate_params` has been removed + - All logic that's currently done in this methods should be moved to the `get_param` method of that type + +When executing systems, we no longer check for system validation before running the systems. +As a result of these changes, `System::validate_param` and `System::validate_param_unsafe` have been removed. +Instead, validation has been moved to be part of the trait implementation for `System::run_unsafe`. +All implementations of the `System` should validate that their parameters are valid during this method, +bubbling up any errors originating in `SystemParam::get_param`. + +For the parallel `MultithreadedExecutor`, validation was previously done while checking run conditions. +Now, tasks will be spawned for systems which would fail or are skipped during validation. +If you find that this is a significant performance overhead for your use case, +the previous behavior can be recovered by adding run conditions. From b92e3d923f1d8c20860404c02f6c20b5837a9285 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 4 Mar 2026 10:42:51 -0800 Subject: [PATCH 02/23] Note `SystemState` changes that will be needed --- release-content/migration-guides/validation_merging.md | 1 + 1 file changed, 1 insertion(+) diff --git a/release-content/migration-guides/validation_merging.md b/release-content/migration-guides/validation_merging.md index d3abf2a772be3..8464cac780d3d 100644 --- a/release-content/migration-guides/validation_merging.md +++ b/release-content/migration-guides/validation_merging.md @@ -12,6 +12,7 @@ To be more precise: - If validation passes, the item should be returned wrapped in `Ok` - `SystemParam::validate_params` has been removed - All logic that's currently done in this methods should be moved to the `get_param` method of that type +- `SystemState::fetch`, `get_unchecked`, `get` and `get_mut` now return a `Result` which may hold a `SystemParamValidationError`. In most prior cases, this would panic if validation would have failed. When executing systems, we no longer check for system validation before running the systems. As a result of these changes, `System::validate_param` and `System::validate_param_unsafe` have been removed. From 1b739a6f83caaf2a20e2fd4238a920f345c964e9 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 4 Mar 2026 12:29:13 -0800 Subject: [PATCH 03/23] Initial migration --- benches/benches/bevy_ecs/fragmentation.rs | 4 +- benches/benches/bevy_ecs/world/world_get.rs | 8 +- crates/bevy_ecs/macros/src/lib.rs | 29 +- crates/bevy_ecs/src/lifecycle.rs | 6 +- crates/bevy_ecs/src/message/message_reader.rs | 25 +- crates/bevy_ecs/src/observer/runner.rs | 4 +- crates/bevy_ecs/src/query/mod.rs | 2 +- .../bevy_ecs/src/reflect/entity_commands.rs | 16 +- crates/bevy_ecs/src/schedule/executor/mod.rs | 11 +- .../src/schedule/executor/multi_threaded.rs | 39 +- crates/bevy_ecs/src/schedule/mod.rs | 2 +- crates/bevy_ecs/src/schedule/node.rs | 20 +- crates/bevy_ecs/src/system/adapter_system.rs | 11 +- crates/bevy_ecs/src/system/builder.rs | 18 - crates/bevy_ecs/src/system/combinator.rs | 33 +- crates/bevy_ecs/src/system/commands/mod.rs | 24 +- .../src/system/exclusive_function_system.rs | 11 +- crates/bevy_ecs/src/system/function_system.rs | 66 +-- crates/bevy_ecs/src/system/mod.rs | 28 +- crates/bevy_ecs/src/system/schedule_system.rs | 18 +- crates/bevy_ecs/src/system/system.rs | 36 -- crates/bevy_ecs/src/system/system_name.rs | 9 +- crates/bevy_ecs/src/system/system_param.rs | 493 +++++------------- crates/bevy_ecs/src/world/identifier.rs | 9 +- crates/bevy_gizmos/src/gizmos.rs | 47 +- crates/bevy_input_focus/src/tab_navigation.rs | 4 +- crates/bevy_pbr/src/material.rs | 2 +- .../bevy_pbr/src/meshlet/instance_manager.rs | 4 +- crates/bevy_pbr/src/prepass/mod.rs | 2 +- crates/bevy_pbr/src/render/light.rs | 2 +- crates/bevy_render/src/erased_render_asset.rs | 2 +- crates/bevy_render/src/extract_param.rs | 32 +- crates/bevy_render/src/render_asset.rs | 2 +- crates/bevy_render/src/render_phase/draw.rs | 2 +- crates/bevy_render/src/renderer/mod.rs | 2 +- .../src/renderer/render_context.rs | 56 +- crates/bevy_render/src/sync_world.rs | 2 +- .../bevy_render/src/view/window/screenshot.rs | 3 +- crates/bevy_transform/src/helper.rs | 2 +- .../src/experimental/ghost_hierarchy.rs | 4 +- crates/bevy_winit/src/cursor/mod.rs | 4 +- crates/bevy_winit/src/state.rs | 15 +- examples/async_tasks/async_compute.rs | 2 +- examples/ecs/fallible_params.rs | 4 +- 44 files changed, 293 insertions(+), 822 deletions(-) diff --git a/benches/benches/bevy_ecs/fragmentation.rs b/benches/benches/bevy_ecs/fragmentation.rs index 97864990a37e5..e7a2f4df78520 100644 --- a/benches/benches/bevy_ecs/fragmentation.rs +++ b/benches/benches/bevy_ecs/fragmentation.rs @@ -25,7 +25,7 @@ fn iter_frag_empty(c: &mut Criterion) { spawn_empty_frag_archetype::(&mut world); let mut q: SystemState> = SystemState::)>>::new(&mut world); - let query = q.get(&world); + let query = q.get(&world).unwrap(); b.iter(move || { let mut res = 0; query.iter().for_each(|(e, t)| { @@ -39,7 +39,7 @@ fn iter_frag_empty(c: &mut Criterion) { spawn_empty_frag_archetype::(&mut world); let mut q: SystemState> = SystemState::)>>::new(&mut world); - let query = q.get(&world); + let query = q.get(&world).unwrap(); b.iter(move || { let mut res = 0; query.iter().for_each(|(e, t)| { diff --git a/benches/benches/bevy_ecs/world/world_get.rs b/benches/benches/bevy_ecs/world/world_get.rs index bd7507b3047a0..19aa015b91afc 100644 --- a/benches/benches/bevy_ecs/world/world_get.rs +++ b/benches/benches/bevy_ecs/world/world_get.rs @@ -269,7 +269,7 @@ pub fn query_get(criterion: &mut Criterion) { .collect(); entities.shuffle(&mut deterministic_rand()); let mut query = SystemState::>::new(&mut world); - let query = query.get(&world); + let query = query.get(&world).unwrap(); bencher.iter(|| { let mut count = 0; @@ -288,7 +288,7 @@ pub fn query_get(criterion: &mut Criterion) { .collect(); entities.shuffle(&mut deterministic_rand()); let mut query = SystemState::>::new(&mut world); - let query = query.get(&world); + let query = query.get(&world).unwrap(); bencher.iter(|| { let mut count = 0; @@ -319,7 +319,7 @@ pub fn query_get_many(criterion: &mut Criterion) { entity_groups.shuffle(&mut deterministic_rand()); let mut query = SystemState::>::new(&mut world); - let query = query.get(&world); + let query = query.get(&world).unwrap(); bencher.iter(|| { let mut count = 0; @@ -342,7 +342,7 @@ pub fn query_get_many(criterion: &mut Criterion) { entity_groups.shuffle(&mut deterministic_rand()); let mut query = SystemState::>::new(&mut world); - let query = query.get(&world); + let query = query.get(&world).unwrap(); bencher.iter(|| { let mut count = 0; diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 6c830203ffce8..35bbd73322dbe 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -449,33 +449,22 @@ fn derive_system_param_impl( <#fields_alias::<'_, '_, #punctuated_generic_idents> as #path::system::SystemParam>::queue(&mut state.state, system_meta, world); } - #[inline] - unsafe fn validate_param<'w, 's>( - state: &'s mut Self::State, - _system_meta: &#path::system::SystemMeta, - _world: #path::world::unsafe_world_cell::UnsafeWorldCell<'w>, - ) -> Result<(), #path::system::SystemParamValidationError> { - let #state_struct_name { state: (#(#tuple_patterns,)*) } = state; - #( - <#field_types as #path::system::SystemParam>::validate_param(#field_locals, _system_meta, _world) - .map_err(|err| #path::system::SystemParamValidationError::new::(err.skipped, #field_validation_messages, #field_validation_names))?; - )* - Result::Ok(()) - } - #[inline] unsafe fn get_param<'w, 's>( state: &'s mut Self::State, system_meta: &#path::system::SystemMeta, world: #path::world::unsafe_world_cell::UnsafeWorldCell<'w>, change_tick: #path::change_detection::Tick, - ) -> Self::Item<'w, 's> { - let (#(#tuple_patterns,)*) = < - (#(#tuple_types,)*) as #path::system::SystemParam - >::get_param(&mut state.state, system_meta, world, change_tick); - #struct_name { + ) -> Result, #path::system::SystemParamValidationError> { + let (#(#tuple_patterns,)*) = &mut state.state; + #( + let #field_locals = unsafe { + <#field_types as #path::system::SystemParam>::get_param(#field_locals, system_meta, world, change_tick) + }.map_err(|err| #path::system::SystemParamValidationError::new::(err.skipped, #field_validation_messages, #field_validation_names))?; + )* + Result::Ok(#struct_name { #(#field_members: #field_locals,)* - } + }) } } diff --git a/crates/bevy_ecs/src/lifecycle.rs b/crates/bevy_ecs/src/lifecycle.rs index ace0dfd87abda..13685cf9ce933 100644 --- a/crates/bevy_ecs/src/lifecycle.rs +++ b/crates/bevy_ecs/src/lifecycle.rs @@ -60,7 +60,7 @@ use crate::{ query::FilteredAccessSet, relationship::RelationshipHookMode, storage::SparseSet, - system::{Local, ReadOnlySystemParam, SystemMeta, SystemParam}, + system::{Local, ReadOnlySystemParam, SystemMeta, SystemParam, SystemParamValidationError}, world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World}, }; @@ -639,7 +639,7 @@ unsafe impl<'a> SystemParam for &'a RemovedComponentMessages { _system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Self::Item<'w, 's> { - world.removed_components() + ) -> Result, SystemParamValidationError> { + Ok(world.removed_components()) } } diff --git a/crates/bevy_ecs/src/message/message_reader.rs b/crates/bevy_ecs/src/message/message_reader.rs index c53e044fcb000..b8d5ebc16f895 100644 --- a/crates/bevy_ecs/src/message/message_reader.rs +++ b/crates/bevy_ecs/src/message/message_reader.rs @@ -162,35 +162,22 @@ unsafe impl<'w, 's, M: Message> SystemParam for PopulatedMessageReader<'w, 's, M system_meta: &crate::system::SystemMeta, world: crate::world::unsafe_world_cell::UnsafeWorldCell<'world>, change_tick: crate::change_detection::Tick, - ) -> Self::Item<'world, 'state> { + ) -> Result, SystemParamValidationError> { // SAFETY: requirements are upheld by MessageReader's implementation - unsafe { - PopulatedMessageReader(MessageReader::get_param( + let reader = unsafe { + MessageReader::get_param( state, system_meta, world, change_tick, - )) - } - } - - unsafe fn validate_param( - state: &mut Self::State, - system_meta: &crate::system::SystemMeta, - world: crate::world::unsafe_world_cell::UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: requirements are upheld by MessageReader's implementation - unsafe { MessageReader::::validate_param(state, system_meta, world) }?; - - // SAFETY: requirements are upheld by MessageReader's implementation - let reader = - unsafe { MessageReader::get_param(state, system_meta, world, world.change_tick()) }; + )? + }; if reader.is_empty() { Err(SystemParamValidationError::skipped::( "message queue is empty", )) } else { - Ok(()) + Ok(PopulatedMessageReader(reader)) } } } diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index 79369dd4cd498..96b7f23e78114 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -108,9 +108,7 @@ pub(super) unsafe fn observer_system_runner>::new(&mut world); - let q = q.get_mut(&mut world); + let q = q.get_mut(&mut world).unwrap(); let _: Option<&Foo> = q.iter().next(); let _: Option<[&Foo; 2]> = q.iter_combinations::<2>().next(); let _: Option<&Foo> = q.iter_many([e]).next(); diff --git a/crates/bevy_ecs/src/reflect/entity_commands.rs b/crates/bevy_ecs/src/reflect/entity_commands.rs index 140c96eba440a..c80c464635f25 100644 --- a/crates/bevy_ecs/src/reflect/entity_commands.rs +++ b/crates/bevy_ecs/src/reflect/entity_commands.rs @@ -462,7 +462,7 @@ mod tests { world.insert_resource(type_registry); let mut system_state: SystemState = SystemState::new(&mut world); - let mut commands = system_state.get_mut(&mut world); + let mut commands = system_state.get_mut(&mut world).unwrap(); let entity = commands.spawn_empty().id(); let entity2 = commands.spawn_empty().id(); @@ -508,7 +508,7 @@ mod tests { world.insert_resource(type_registry); let mut system_state: SystemState = SystemState::new(&mut world); - let mut commands = system_state.get_mut(&mut world); + let mut commands = system_state.get_mut(&mut world).unwrap(); let entity = commands.spawn_empty().id(); @@ -538,7 +538,7 @@ mod tests { world.insert_resource(type_registry); let mut system_state: SystemState = SystemState::new(&mut world); - let mut commands = system_state.get_mut(&mut world); + let mut commands = system_state.get_mut(&mut world).unwrap(); let entity = commands.spawn(ComponentA(0)).id(); @@ -567,7 +567,7 @@ mod tests { world.insert_resource(type_registry); let mut system_state: SystemState = SystemState::new(&mut world); - let mut commands = system_state.get_mut(&mut world); + let mut commands = system_state.get_mut(&mut world).unwrap(); let entity = commands.spawn(ComponentA(0)).id(); @@ -596,7 +596,7 @@ mod tests { world.insert_resource(type_registry); let mut system_state: SystemState = SystemState::new(&mut world); - let mut commands = system_state.get_mut(&mut world); + let mut commands = system_state.get_mut(&mut world).unwrap(); let entity = commands.spawn_empty().id(); let bundle = Box::new(BundleA { @@ -626,7 +626,7 @@ mod tests { world.insert_resource(type_registry); let mut system_state: SystemState = SystemState::new(&mut world); - let mut commands = system_state.get_mut(&mut world); + let mut commands = system_state.get_mut(&mut world).unwrap(); let entity = commands.spawn_empty().id(); let bundle = Box::new(BundleA { @@ -656,7 +656,7 @@ mod tests { world.insert_resource(type_registry); let mut system_state: SystemState = SystemState::new(&mut world); - let mut commands = system_state.get_mut(&mut world); + let mut commands = system_state.get_mut(&mut world).unwrap(); let entity = commands .spawn(BundleA { @@ -694,7 +694,7 @@ mod tests { world.insert_resource(type_registry); let mut system_state: SystemState = SystemState::new(&mut world); - let mut commands = system_state.get_mut(&mut world); + let mut commands = system_state.get_mut(&mut world).unwrap(); let entity = commands .spawn(BundleA { diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index df297637a6037..73ee92e61b5cb 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -22,7 +22,7 @@ use crate::{ ConditionWithAccess, InternedSystemSet, SystemKey, SystemSetKey, SystemTypeSet, SystemWithAccess, }, - system::{RunSystemError, System, SystemIn, SystemParamValidationError, SystemStateFlags}, + system::{RunSystemError, System, SystemIn, SystemStateFlags}, world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World}, }; @@ -197,15 +197,6 @@ impl System for ApplyDeferred { fn queue_deferred(&mut self, _world: DeferredWorld) {} - 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. - Ok(()) - } - fn initialize(&mut self, _world: &mut World) -> FilteredAccessSet { FilteredAccessSet::new() } diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs index 33bc8d0b80079..a6e1f34ca7901 100644 --- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs @@ -644,32 +644,6 @@ impl ExecutorState { should_run &= system_conditions_met; - if should_run { - // SAFETY: - // - The caller ensures that `world` has permission to read any data - // required by the system. - let valid_params = match unsafe { system.validate_param_unsafe(world) } { - Ok(()) => true, - Err(e) => { - if !e.skipped { - error_handler( - e.into(), - ErrorContext::System { - name: system.name(), - last_run: system.get_last_run(), - }, - ); - } - false - } - }; - if !valid_params { - self.skipped_systems.insert(system_index); - } - - should_run &= valid_params; - } - should_run } @@ -852,16 +826,9 @@ unsafe fn evaluate_and_fold_conditions( // SAFETY: // - The caller ensures that `world` has permission to read any data // required by the condition. - unsafe { condition.validate_param_unsafe(world) } - .map_err(From::from) - .and_then(|()| { - // SAFETY: - // - The caller ensures that `world` has permission to read any data - // required by the condition. - unsafe { - __rust_begin_short_backtrace::readonly_run_unsafe(&mut **condition, world) - } - }) + unsafe { + __rust_begin_short_backtrace::readonly_run_unsafe(&mut **condition, world) + } .unwrap_or_else(|err| { if let RunSystemError::Failed(err) = err { error_handler( diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index 70a261f47b1fa..1c6f32deb1cb2 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -1304,7 +1304,7 @@ mod tests { // start a new frame by running ihe begin_frame() system let mut system_state: SystemState>> = SystemState::new(&mut world); - let res = system_state.get_mut(&mut world); + let res = system_state.get_mut(&mut world).unwrap(); Stepping::begin_frame(res); // now run the schedule; this will panic if the executor doesn't diff --git a/crates/bevy_ecs/src/schedule/node.rs b/crates/bevy_ecs/src/schedule/node.rs index f73d8f9d49c0f..c1d29c7f7c2b6 100644 --- a/crates/bevy_ecs/src/schedule/node.rs +++ b/crates/bevy_ecs/src/schedule/node.rs @@ -25,7 +25,7 @@ use crate::{ }, storage::SparseSetIndex, system::{ - ReadOnlySystem, RunSystemError, ScheduleSystem, System, SystemParamValidationError, + ReadOnlySystem, RunSystemError, ScheduleSystem, System, SystemStateFlags, }, world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World}, @@ -101,15 +101,6 @@ impl System for SystemWithAccess { self.system.queue_deferred(world); } - #[inline] - unsafe fn validate_param_unsafe( - &mut self, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Caller ensures the same safety requirements. - unsafe { self.system.validate_param_unsafe(world) } - } - #[inline] fn initialize(&mut self, world: &mut World) -> FilteredAccessSet { self.system.initialize(world) @@ -201,15 +192,6 @@ impl System for ConditionWithAccess { self.condition.queue_deferred(world); } - #[inline] - unsafe fn validate_param_unsafe( - &mut self, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Caller ensures the same safety requirements. - unsafe { self.condition.validate_param_unsafe(world) } - } - #[inline] fn initialize(&mut self, world: &mut World) -> FilteredAccessSet { self.condition.initialize(world) diff --git a/crates/bevy_ecs/src/system/adapter_system.rs b/crates/bevy_ecs/src/system/adapter_system.rs index 8d3d35e04c8c2..5d3a70161bf6e 100644 --- a/crates/bevy_ecs/src/system/adapter_system.rs +++ b/crates/bevy_ecs/src/system/adapter_system.rs @@ -1,7 +1,7 @@ use alloc::vec::Vec; use bevy_utils::prelude::DebugName; -use super::{IntoSystem, ReadOnlySystem, RunSystemError, System, SystemParamValidationError}; +use super::{IntoSystem, ReadOnlySystem, RunSystemError, System}; use crate::{ schedule::InternedSystemSet, system::{input::SystemInput, SystemIn}, @@ -162,15 +162,6 @@ where self.system.queue_deferred(world); } - #[inline] - unsafe fn validate_param_unsafe( - &mut self, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Delegate to other `System` implementations. - unsafe { self.system.validate_param_unsafe(world) } - } - fn initialize(&mut self, world: &mut crate::prelude::World) -> crate::query::FilteredAccessSet { self.system.initialize(world) } diff --git a/crates/bevy_ecs/src/system/builder.rs b/crates/bevy_ecs/src/system/builder.rs index e1b19c14403b7..a84563ea8a01b 100644 --- a/crates/bevy_ecs/src/system/builder.rs +++ b/crates/bevy_ecs/src/system/builder.rs @@ -420,24 +420,6 @@ where } } - #[inline] - unsafe fn validate_param_unsafe( - &mut self, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - match &mut self.inner { - // SAFETY: requirements upheld by the caller. - BuilderSystemInner::Initialized { system, .. } => unsafe { - system.validate_param_unsafe(world) - }, - BuilderSystemInner::Uninitialized { .. } => panic!( - "BuilderSystem {} was not initialized before calling validate_param_unsafe.", - self.name() - ), - BuilderSystemInner::Invalid => unreachable!(), - } - } - #[inline] fn initialize(&mut self, world: &mut World) -> FilteredAccessSet { let inner = mem::replace(&mut self.inner, BuilderSystemInner::Invalid); diff --git a/crates/bevy_ecs/src/system/combinator.rs b/crates/bevy_ecs/src/system/combinator.rs index 73dbb33d834d5..5efe51a025421 100644 --- a/crates/bevy_ecs/src/system/combinator.rs +++ b/crates/bevy_ecs/src/system/combinator.rs @@ -8,7 +8,7 @@ use crate::{ prelude::World, query::FilteredAccessSet, schedule::InternedSystemSet, - system::{input::SystemInput, SystemIn, SystemParamValidationError}, + system::{input::SystemInput, SystemIn}, world::unsafe_world_cell::UnsafeWorldCell, }; @@ -168,10 +168,7 @@ where world: &mut PrivateUnsafeWorldCell, ) -> Result { // SAFETY: see comment on `Func::combine` call - match (|| unsafe { - system.validate_param_unsafe(world.0)?; - system.run_unsafe(input, world.0) - })() { + match (|| unsafe { system.run_unsafe(input, world.0) })() { // let the world's default error handler handle the error if `Failed(_)` Err(RunSystemError::Failed(err)) => { // SAFETY: We registered access to DefaultErrorHandler in `initialize`. @@ -233,17 +230,6 @@ where self.b.queue_deferred(world); } - #[inline] - unsafe fn validate_param_unsafe( - &mut self, - _world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // Both systems are validated in `Self::run_unsafe`, so that we get the - // chance to run the second system even if the first one fails to - // validate. - Ok(()) - } - fn initialize(&mut self, world: &mut World) -> FilteredAccessSet { let mut a_access = self.a.initialize(world); let b_access = self.b.initialize(world); @@ -415,9 +401,6 @@ where // SAFETY: Upheld by caller unsafe { let value = self.a.run_unsafe(input, world)?; - // `Self::validate_param_unsafe` already validated the first system, - // but we still need to validate the second system once the first one runs. - self.b.validate_param_unsafe(world)?; self.b.run_unsafe(value, world) } } @@ -439,18 +422,6 @@ where self.b.queue_deferred(world); } - unsafe fn validate_param_unsafe( - &mut self, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // We only validate parameters for the first system, - // since it may make changes to the world that affect - // whether the second system has valid parameters. - // The second system will be validated in `Self::run_unsafe`. - // SAFETY: Delegate to the `System` implementation for `a`. - unsafe { self.a.validate_param_unsafe(world) } - } - fn initialize(&mut self, world: &mut World) -> FilteredAccessSet { let mut a_access = self.a.initialize(world); let b_access = self.b.initialize(world); diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index cc2cd12969579..db10dd3bab4e1 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -176,22 +176,6 @@ const _: () = { ); } - #[inline] - unsafe fn validate_param( - state: &mut Self::State, - system_meta: &bevy_ecs::system::SystemMeta, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Upheld by caller - unsafe { - <__StructFieldsAlias as bevy_ecs::system::SystemParam>::validate_param( - &mut state.state, - system_meta, - world, - ) - } - } - #[inline] #[track_caller] unsafe fn get_param<'w, 's>( @@ -199,7 +183,7 @@ const _: () = { system_meta: &bevy_ecs::system::SystemMeta, world: UnsafeWorldCell<'w>, change_tick: bevy_ecs::change_detection::Tick, - ) -> Self::Item<'w, 's> { + ) -> Result, SystemParamValidationError> { // SAFETY: Upheld by caller let params = unsafe { <__StructFieldsAlias as bevy_ecs::system::SystemParam>::get_param( @@ -207,13 +191,13 @@ const _: () = { system_meta, world, change_tick, - ) + )? }; - Commands { + Ok(Commands { queue: InternalQueue::CommandQueue(params.0), allocator: params.1, entities: params.2, - } + }) } } // SAFETY: Only reads Entities diff --git a/crates/bevy_ecs/src/system/exclusive_function_system.rs b/crates/bevy_ecs/src/system/exclusive_function_system.rs index b1e9de0b11ea5..809ded7cdf924 100644 --- a/crates/bevy_ecs/src/system/exclusive_function_system.rs +++ b/crates/bevy_ecs/src/system/exclusive_function_system.rs @@ -15,7 +15,7 @@ use bevy_utils::prelude::DebugName; use core::marker::PhantomData; use variadics_please::all_tuples; -use super::{RunSystemError, SystemParamValidationError, SystemStateFlags}; +use super::{RunSystemError, SystemStateFlags}; /// A function system that runs with exclusive [`World`] access. /// @@ -166,15 +166,6 @@ where // might have buffers to apply, but this is handled by `PipeSystem`. } - #[inline] - unsafe fn validate_param_unsafe( - &mut self, - _world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // All exclusive system params are always available. - Ok(()) - } - #[inline] fn initialize(&mut self, world: &mut World) -> FilteredAccessSet { self.system_meta.last_run = world.change_tick().relative_to(Tick::MAX); diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 1341224fc906f..c5f070f309764 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -183,7 +183,7 @@ impl SystemMeta { /// /// // Use system_state.get_mut(&mut world) and unpack your system parameters into variables! /// // system_state.get(&world) provides read-only versions of your system parameters instead. -/// let (message_writer, maybe_resource, query) = system_state.get_mut(&mut world); +/// let (message_writer, maybe_resource, query) = system_state.get_mut(&mut world).unwrap(); /// /// // If you are using `Commands`, you can choose when you want to apply them to the world. /// // You need to manually call `.apply(world)` on the `SystemState` to apply them. @@ -213,7 +213,7 @@ impl SystemMeta { /// /// // Later, fetch the cached system state, saving on overhead /// world.resource_scope(|world, mut cached_state: Mut| { -/// let mut message_reader = cached_state.message_state.get_mut(world); +/// let mut message_reader = cached_state.message_state.get_mut(world).unwrap(); /// /// for message in message_reader.read() { /// println!("Hello World!"); @@ -229,7 +229,7 @@ impl SystemMeta { /// # struct MyMessage; /// # /// fn exclusive_system(world: &mut World, system_state: &mut SystemState>) { -/// let mut message_reader = system_state.get_mut(world); +/// let mut message_reader = system_state.get_mut(world).unwrap(); /// /// for message in message_reader.read() { /// println!("Hello World!"); @@ -367,8 +367,13 @@ impl SystemState { } /// Retrieve the [`SystemParam`] values. This can only be called when all parameters are read-only. + /// + /// Returns an error if system parameter validation fails. #[inline] - pub fn get<'w, 's>(&'s mut self, world: &'w World) -> SystemParamItem<'w, 's, Param> + pub fn get<'w, 's>( + &'s mut self, + world: &'w World, + ) -> Result, SystemParamValidationError> where Param: ReadOnlySystemParam, { @@ -379,9 +384,14 @@ impl SystemState { } /// Retrieve the mutable [`SystemParam`] values. + /// + /// Returns an error if system parameter validation fails. #[inline] #[track_caller] - pub fn get_mut<'w, 's>(&'s mut self, world: &'w mut World) -> SystemParamItem<'w, 's, Param> { + pub fn get_mut<'w, 's>( + &'s mut self, + world: &'w mut World, + ) -> Result, SystemParamValidationError> { self.validate_world(world.id()); // SAFETY: World is uniquely borrowed and matches the World this SystemState was created with. unsafe { self.get_unchecked(world.as_unsafe_world_cell()) } @@ -395,21 +405,6 @@ impl SystemState { Param::apply(&mut self.param_state, &self.meta, world); } - /// Wrapper over [`SystemParam::validate_param`]. - /// - /// # Safety - /// - /// - The passed [`UnsafeWorldCell`] must have read-only access to - /// world data in `component_access_set`. - /// - `world` must be the same [`World`] that was used to initialize [`state`](SystemParam::init_state). - pub unsafe fn validate_param( - state: &mut Self, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Delegated to existing `SystemParam` implementations. - unsafe { Param::validate_param(&mut state.param_state, &state.meta, world) } - } - /// Returns `true` if `world_id` matches the [`World`] that was used to call [`SystemState::new`]. /// Otherwise, this returns false. #[inline] @@ -435,6 +430,8 @@ impl SystemState { /// Retrieve the [`SystemParam`] values. /// + /// Returns an error if system parameter validation fails. + /// /// # Safety /// This call might access any of the input parameters in a way that violates Rust's mutability rules. Make sure the data /// access is safe in the context of global [`World`] access. The passed-in [`World`] _must_ be the [`World`] the [`SystemState`] was @@ -444,7 +441,7 @@ impl SystemState { pub unsafe fn get_unchecked<'w, 's>( &'s mut self, world: UnsafeWorldCell<'w>, - ) -> SystemParamItem<'w, 's, Param> { + ) -> Result, SystemParamValidationError> { let change_tick = world.increment_change_tick(); // SAFETY: The invariants are upheld by the caller. unsafe { self.fetch(world, change_tick) } @@ -460,12 +457,12 @@ impl SystemState { &'s mut self, world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> SystemParamItem<'w, 's, Param> { + ) -> Result, SystemParamValidationError> { // SAFETY: The invariants are upheld by the caller. let param = - unsafe { Param::get_param(&mut self.param_state, &self.meta, world, change_tick) }; + unsafe { Param::get_param(&mut self.param_state, &self.meta, world, change_tick) }?; self.meta.last_run = change_tick; - param + Ok(param) } /// Returns a reference to the current system param states. @@ -523,7 +520,7 @@ struct FunctionSystemState { /// The cached state of the system's [`SystemParam`]s. param: P::State, /// The id of the [`World`] this system was initialized with. If the world - /// passed to [`System::run_unsafe`] or [`System::validate_param_unsafe`] does not match + /// passed to [`System::run_unsafe`] does not match /// this id, a panic will occur. world_id: WorldId, } @@ -680,8 +677,9 @@ where // - The above assert ensures the world matches. // - All world accesses used by `F::Param` have been registered, so the caller // will ensure that there are no data access conflicts. - let params = - unsafe { F::Param::get_param(&mut state.param, &self.system_meta, world, change_tick) }; + let params = unsafe { + F::Param::get_param(&mut state.param, &self.system_meta, world, change_tick) + }?; #[cfg(feature = "hotpatching")] let out = { @@ -723,20 +721,6 @@ where F::Param::queue(param_state, &self.system_meta, world); } - #[inline] - unsafe fn validate_param_unsafe( - &mut self, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - let state = self.state.as_mut().expect(Self::ERROR_UNINITIALIZED); - assert_eq!(state.world_id, world.id(), "Encountered a mismatched World. A System cannot be used with Worlds other than the one it was initialized with."); - // SAFETY: - // - The above assert ensures the world matches. - // - All world accesses used by `F::Param` have been registered, so the caller - // will ensure that there are no data access conflicts. - unsafe { F::Param::validate_param(&mut state.param, &self.system_meta, world) } - } - #[inline] fn initialize(&mut self, world: &mut World) -> FilteredAccessSet { if let Some(state) = &self.state { diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index 79fbdbd05c4a1..91f7ab26329f5 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -1318,7 +1318,7 @@ mod tests { Option>, ParamSet<(Query<&C>, Query<&D>)>, )> = SystemState::new(&mut world); - let (a, query, _) = system_state.get(&world); + let (a, query, _) = system_state.get(&world).unwrap(); assert_eq!(*a, A(42), "returned resource matches initial value"); assert_eq!( **query.unwrap(), @@ -1345,7 +1345,7 @@ mod tests { // The following line shouldn't compile because the parameters used are not ReadOnlySystemParam // let (a, query) = system_state.get(&world); - let (a, query) = system_state.get_mut(&mut world); + let (a, query) = system_state.get_mut(&mut world).unwrap(); assert_eq!(*a, A(42), "returned resource matches initial value"); assert_eq!( **query.unwrap(), @@ -1365,18 +1365,18 @@ mod tests { let mut system_state: SystemState>>> = SystemState::new(&mut world); { - let query = system_state.get(&world); + let query = system_state.get(&world).unwrap(); assert_eq!(**query.unwrap(), A(1)); } { - let query = system_state.get(&world); + let query = system_state.get(&world).unwrap(); assert!(query.is_none()); } world.entity_mut(entity).get_mut::().unwrap().0 = 2; { - let query = system_state.get(&world); + let query = system_state.get(&world).unwrap(); assert_eq!(**query.unwrap(), A(2)); } } @@ -1390,12 +1390,12 @@ mod tests { let mut system_state: SystemState>> = SystemState::new(&mut world); { - let query = system_state.get(&world); + let query = system_state.get(&world).unwrap(); assert_eq!(query.unwrap().1.spawn_tick(), spawn_tick); } { - let query = system_state.get(&world); + let query = system_state.get(&world).unwrap(); assert!(query.is_none()); } } @@ -1406,7 +1406,7 @@ mod tests { let mut world = World::default(); let mut system_state = SystemState::>::new(&mut world); let mismatched_world = World::default(); - system_state.get(&mismatched_world); + system_state.get(&mismatched_world).unwrap(); } #[test] @@ -1422,7 +1422,7 @@ mod tests { let mut system_state = SystemState::>::new(&mut world); { - let query = system_state.get(&world); + let query = system_state.get(&world).unwrap(); assert_eq!( query.iter().collect::>(), vec![&A(1)], @@ -1432,7 +1432,7 @@ mod tests { world.spawn((A(2), B(2))); { - let query = system_state.get(&world); + let query = system_state.get(&world).unwrap(); assert_eq!( query.iter().collect::>(), vec![&A(1), &A(2)], @@ -1462,19 +1462,19 @@ mod tests { impl State { fn hold_res<'w>(&mut self, world: &'w World) -> ResourceHolder<'w> { - let a = self.state.get(world); + let a = self.state.get(world).unwrap(); ResourceHolder { value: a.into_inner(), } } fn hold_component<'w>(&mut self, world: &'w World, entity: Entity) -> Holder<'w> { - let q = self.state_q.get(world); + let q = self.state_q.get(world).unwrap(); let a = q.get_inner(entity).unwrap(); Holder { value: a } } fn hold_components<'w>(&mut self, world: &'w World) -> Vec> { let mut components = Vec::new(); - let q = self.state_q.get(world); + let q = self.state_q.get(world).unwrap(); for a in q.iter_inner() { components.push(Holder { value: a }); } @@ -1494,7 +1494,7 @@ mod tests { let mut system_state = SystemState::>::new(&mut world); { - let mut query = system_state.get_mut(&mut world); + let mut query = system_state.get_mut(&mut world).unwrap(); assert_eq!( query.iter_mut().map(|m| *m).collect::>(), vec![A(1), A(2)], diff --git a/crates/bevy_ecs/src/system/schedule_system.rs b/crates/bevy_ecs/src/system/schedule_system.rs index 8116f6c06b400..4b555072f677e 100644 --- a/crates/bevy_ecs/src/system/schedule_system.rs +++ b/crates/bevy_ecs/src/system/schedule_system.rs @@ -8,7 +8,7 @@ use crate::{ world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, FromWorld, World}, }; -use super::{IntoSystem, SystemParamValidationError, SystemStateFlags}; +use super::{IntoSystem, SystemStateFlags}; /// See [`IntoSystem::with_input`] for details. pub struct WithInputWrapper @@ -84,14 +84,6 @@ where self.system.queue_deferred(world); } - unsafe fn validate_param_unsafe( - &mut self, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Upheld by caller - unsafe { self.system.validate_param_unsafe(world) } - } - fn initialize(&mut self, world: &mut World) -> FilteredAccessSet { self.system.initialize(world) } @@ -183,14 +175,6 @@ where self.system.queue_deferred(world); } - unsafe fn validate_param_unsafe( - &mut self, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Upheld by caller - unsafe { self.system.validate_param_unsafe(world) } - } - fn initialize(&mut self, world: &mut World) -> FilteredAccessSet { if self.value.is_none() { self.value = Some(T::from_world(world)); diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index aff628fa322e9..cf48d0d9c8843 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -132,9 +132,6 @@ pub trait System: Send + Sync + 'static { let world_cell = world.as_unsafe_world_cell(); // SAFETY: // - We have exclusive access to the entire world. - unsafe { self.validate_param_unsafe(world_cell) }?; - // SAFETY: - // - We have exclusive access to the entire world. unsafe { self.run_unsafe(input, world_cell) } } @@ -147,36 +144,6 @@ pub trait System: Send + Sync + 'static { /// of this system into the world's command buffer. fn queue_deferred(&mut self, world: DeferredWorld); - /// Validates that all parameters can be acquired and that system can run without panic. - /// Built-in executors use this to prevent invalid systems from running. - /// - /// However calling and respecting [`System::validate_param_unsafe`] or its safe variant - /// is not a strict requirement, both [`System::run`] and [`System::run_unsafe`] - /// should provide their own safety mechanism to prevent undefined behavior. - /// - /// This method has to be called directly before [`System::run_unsafe`] with no other (relevant) - /// world mutations in between. Otherwise, while it won't lead to any undefined behavior, - /// the validity of the param may change. - /// - /// # Safety - /// - /// - The caller must ensure that [`world`](UnsafeWorldCell) has permission to access any world data - /// registered in the access returned from [`System::initialize`]. There must be no conflicting - /// simultaneous accesses while the system is running. - unsafe fn validate_param_unsafe( - &mut self, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError>; - - /// Safe version of [`System::validate_param_unsafe`]. - /// that runs on exclusive, single-threaded `world` pointer. - fn validate_param(&mut self, world: &World) -> Result<(), SystemParamValidationError> { - let world_cell = world.as_unsafe_world_cell_readonly(); - // SAFETY: - // - We have exclusive access to the entire world. - unsafe { self.validate_param_unsafe(world_cell) } - } - /// Initialize the system. /// /// Returns a [`FilteredAccessSet`] with the access required to run the system. @@ -237,9 +204,6 @@ pub unsafe trait ReadOnlySystem: System { let world = world.as_unsafe_world_cell_readonly(); // SAFETY: // - We have read-only access to the entire world. - unsafe { self.validate_param_unsafe(world) }?; - // SAFETY: - // - We have read-only access to the entire world. unsafe { self.run_unsafe(input, world) } } } diff --git a/crates/bevy_ecs/src/system/system_name.rs b/crates/bevy_ecs/src/system/system_name.rs index af4fc91f3c9a2..9321cc6968bd6 100644 --- a/crates/bevy_ecs/src/system/system_name.rs +++ b/crates/bevy_ecs/src/system/system_name.rs @@ -2,7 +2,10 @@ use crate::{ change_detection::Tick, prelude::World, query::FilteredAccessSet, - system::{ExclusiveSystemParam, ReadOnlySystemParam, SystemMeta, SystemParam}, + system::{ + ExclusiveSystemParam, ReadOnlySystemParam, SystemMeta, SystemParam, + SystemParamValidationError, + }, world::unsafe_world_cell::UnsafeWorldCell, }; use bevy_utils::prelude::DebugName; @@ -65,8 +68,8 @@ unsafe impl SystemParam for SystemName { system_meta: &SystemMeta, _world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Self::Item<'w, 's> { - SystemName(system_meta.name.clone()) + ) -> Result, SystemParamValidationError> { + Ok(SystemName(system_meta.name.clone())) } } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 976ee8afff9ca..dc9fea08e4a84 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -15,14 +15,13 @@ use crate::{ QuerySingleError, QueryState, ReadOnlyQueryData, }, resource::{Resource, IS_RESOURCE}, - storage::NonSendData, system::{Query, Single, SystemMeta}, world::{ unsafe_world_cell::UnsafeWorldCell, DeferredWorld, FilteredResources, FilteredResourcesMut, FromWorld, World, }, }; -use alloc::{borrow::Cow, boxed::Box, vec::Vec}; +use alloc::{borrow::Cow, boxed::Box, format, vec::Vec}; pub use bevy_ecs_macros::SystemParam; use bevy_platform::cell::SyncCell; use bevy_ptr::UnsafeCellDeref; @@ -257,53 +256,16 @@ 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). + /// Creates a parameter to be passed into a [`SystemParamFunction`](super::SystemParamFunction). /// + /// This method also validates that the param can be acquired. If validation fails, + /// an appropriate [`SystemParamValidationError`] should be returned. /// 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`], - /// with a value of type [`SystemParamValidationError`]. + /// 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. /// - /// However calling and respecting [`SystemParam::validate_param`] - /// is not a strict requirement, [`SystemParam::get_param`] should - /// provide it's own safety mechanism to prevent undefined behavior. - /// - /// The [`world`](UnsafeWorldCell) can only be used to read param's data - /// and world metadata. No data can be written. - /// - /// When using system parameters that require `change_tick` you can use - /// [`UnsafeWorldCell::change_tick()`]. Even if this isn't the exact - /// same tick used for [`SystemParam::get_param`], the world access - /// ensures that the queried data will be the same in both calls. - /// - /// This method has to be called directly before [`SystemParam::get_param`] with no other (relevant) - /// world mutations inbetween. Otherwise, while it won't lead to any undefined behavior, - /// the validity of the param may change. - /// - /// [`System::validate_param`](super::system::System::validate_param), - /// calls this method for each supplied system param. - /// - /// # Safety - /// - /// - The passed [`UnsafeWorldCell`] must have read-only access to world data - /// registered in [`init_access`](SystemParam::init_access). - /// - `world` must be the same [`World`] that was used to initialize [`state`](SystemParam::init_state). - #[expect( - unused_variables, - reason = "The parameters here are intentionally unused by the default implementation; however, putting underscores here will result in the underscores being copied by rust-analyzer's tab completion." - )] - unsafe fn validate_param( - state: &mut Self::State, - system_meta: &SystemMeta, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - Ok(()) - } - - /// Creates a parameter to be passed into a [`SystemParamFunction`](super::SystemParamFunction). - /// /// # Safety /// /// - The passed [`UnsafeWorldCell`] must have access to any world data registered @@ -317,7 +279,7 @@ pub unsafe trait SystemParam: Sized { system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, change_tick: Tick, - ) -> Self::Item<'world, 'state>; + ) -> Result, SystemParamValidationError>; } /// A [`SystemParam`] that only reads a given [`World`]. @@ -363,12 +325,12 @@ unsafe impl SystemParam for Qu system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> Self::Item<'w, 's> { + ) -> Result, SystemParamValidationError> { // SAFETY: We have registered all of the query's world accesses, // so the caller ensures that `world` has permission to access any // world data that the query needs. // The caller ensures the world matches the one used in init_state. - unsafe { state.query_unchecked_with_ticks(world, system_meta.last_run, change_tick) } + Ok(unsafe { state.query_unchecked_with_ticks(world, system_meta.last_run, change_tick) }) } } @@ -399,39 +361,21 @@ unsafe impl<'a, 'b, D: IterQueryData + 'static, F: QueryFilter + 'static> System system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> Self::Item<'w, 's> { + ) -> Result, SystemParamValidationError> { // SAFETY: State ensures that the components it accesses are not accessible somewhere elsewhere. // The caller ensures the world matches the one used in init_state. let query = unsafe { state.query_unchecked_with_ticks(world, system_meta.last_run, change_tick) }; - let single = query - .single_inner() - .expect("The query was expected to contain exactly one matching entity."); - Single { - item: single, - _filter: PhantomData, - } - } - - #[inline] - unsafe fn validate_param( - state: &mut Self::State, - system_meta: &SystemMeta, - world: UnsafeWorldCell, - ) -> 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. - let query = unsafe { - state.query_unchecked_with_ticks(world, system_meta.last_run, world.change_tick()) - }; match query.single_inner() { - Ok(_) => Ok(()), + Ok(single) => Ok(Single { + item: single, + _filter: PhantomData, + }), Err(QuerySingleError::NoEntities(_)) => Err( SystemParamValidationError::skipped::("No matching entities"), ), Err(QuerySingleError::MultipleEntities(_)) => Err( - SystemParamValidationError::skipped::("Multiple matching entities"), + SystemParamValidationError::invalid::("Multiple matching entities"), ), } } @@ -470,30 +414,15 @@ unsafe impl SystemParam system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> Self::Item<'w, 's> { + ) -> Result, SystemParamValidationError> { // SAFETY: Delegate to existing `SystemParam` implementations. - let query = unsafe { Query::get_param(state, system_meta, world, change_tick) }; - Populated(query) - } - - #[inline] - unsafe fn validate_param( - state: &mut Self::State, - system_meta: &SystemMeta, - world: UnsafeWorldCell, - ) -> 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. - let query = unsafe { - state.query_unchecked_with_ticks(world, system_meta.last_run, world.change_tick()) - }; + let query = unsafe { Query::get_param(state, system_meta, world, change_tick) }?; if query.is_empty() { Err(SystemParamValidationError::skipped::( "No matching entities", )) } else { - Ok(()) + Ok(Populated(query)) } } } @@ -682,31 +611,19 @@ macro_rules! impl_param_set { <($($param,)*) as SystemParam>::queue(state, system_meta, world.reborrow()); } - #[inline] - unsafe fn validate_param<'w, 's>( - state: &'s mut Self::State, - system_meta: &SystemMeta, - world: UnsafeWorldCell<'w>, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Upheld by caller - unsafe { - <($($param,)*) as SystemParam>::validate_param(state, system_meta, world) - } - } - #[inline] unsafe fn get_param<'w, 's>( state: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> Self::Item<'w, 's> { - ParamSet { + ) -> Result, SystemParamValidationError> { + Ok(ParamSet { param_states: state, system_meta: system_meta.clone(), world, change_tick, - } + }) } } @@ -724,6 +641,7 @@ macro_rules! impl_param_set { unsafe { $param::get_param(&mut self.param_states.$index, &self.system_meta, self.world, self.change_tick) } + .unwrap_or_else(|err| panic!("ParamSet parameter validation failed: {err}")) } )* } @@ -769,42 +687,21 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { panic!("error[B0002]: Res<{}> in system {} conflicts with a previous system parameter. Consider removing the duplicate access using `Without` to create disjoint Queries or merging conflicting Queries into a `ParamSet`. See: https://bevy.org/learn/errors/b0002", DebugName::type_name::(), system_meta.name); } - #[inline] - unsafe fn validate_param( - &mut component_id: &mut Self::State, - _system_meta: &SystemMeta, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Read-only access to the resource - if let Some(entity) = unsafe { world.resource_entities() }.get(component_id) - && let Ok(entity_ref) = world.get_entity(*entity) - && entity_ref.contains_id(component_id) - { - Ok(()) - } else { - Err(SystemParamValidationError::invalid::( - "Resource does not exist", - )) - } - } - #[inline] unsafe fn get_param<'w, 's>( &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> Self::Item<'w, 's> { - let (ptr, ticks) = world - .get_resource_with_ticks(component_id) - .unwrap_or_else(|| { - panic!( - "Resource requested by {} does not exist: {}", - system_meta.name, - DebugName::type_name::() - ); - }); - Res { + ) -> Result, SystemParamValidationError> { + let (ptr, ticks) = world.get_resource_with_ticks(component_id).ok_or_else(|| { + SystemParamValidationError::invalid::(format!( + "Resource requested by {} does not exist: {}", + system_meta.name, + DebugName::type_name::() + )) + })?; + Ok(Res { value: ptr.deref(), ticks: ComponentTicksRef { added: ticks.added.deref(), @@ -813,7 +710,7 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { last_run: system_meta.last_run, this_run: change_tick, }, - } + }) } } @@ -851,42 +748,21 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { panic!("error[B0002]: ResMut<{}> in system {} conflicts with a previous system parameter. Consider removing the duplicate access or using `Without` to create disjoint Queries or merging conflicting Queries into a `ParamSet`. See: https://bevy.org/learn/errors/b0002", DebugName::type_name::(), system_meta.name); } - #[inline] - unsafe fn validate_param( - &mut component_id: &mut Self::State, - _system_meta: &SystemMeta, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Read-only access to the resource. - if let Some(entity) = unsafe { world.resource_entities() }.get(component_id) - && let Ok(entity_ref) = world.get_entity(*entity) - && entity_ref.contains_id(component_id) - { - Ok(()) - } else { - Err(SystemParamValidationError::invalid::( - "Resource does not exist", - )) - } - } - #[inline] unsafe fn get_param<'w, 's>( &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> Self::Item<'w, 's> { - let value = world - .get_resource_mut_by_id(component_id) - .unwrap_or_else(|| { - panic!( - "Resource requested by {} does not exist: {}", - system_meta.name, - DebugName::type_name::() - ); - }); - ResMut { + ) -> Result, SystemParamValidationError> { + let value = world.get_resource_mut_by_id(component_id).ok_or_else(|| { + SystemParamValidationError::invalid::(format!( + "Resource requested by {} does not exist: {}", + system_meta.name, + DebugName::type_name::() + )) + })?; + Ok(ResMut { value: value.value.deref_mut::(), ticks: ComponentTicksMut { added: value.ticks.added, @@ -895,7 +771,7 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { last_run: system_meta.last_run, this_run: change_tick, }, - } + }) } } @@ -933,9 +809,9 @@ unsafe impl SystemParam for &'_ World { _system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Self::Item<'w, 's> { + ) -> Result, SystemParamValidationError> { // SAFETY: Read-only access to the entire world was registered in `init_state`. - unsafe { world.world() } + Ok(unsafe { world.world() }) } } @@ -965,9 +841,9 @@ unsafe impl<'w> SystemParam for DeferredWorld<'w> { _system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, _change_tick: Tick, - ) -> Self::Item<'world, 'state> { + ) -> Result, SystemParamValidationError> { // SAFETY: Upheld by caller - unsafe { world.into_deferred() } + Ok(unsafe { world.into_deferred() }) } } @@ -1156,8 +1032,8 @@ unsafe impl<'a, T: FromWorld + Send + 'static> SystemParam for Local<'a, T> { _system_meta: &SystemMeta, _world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Self::Item<'w, 's> { - Local(state.get()) + ) -> Result, SystemParamValidationError> { + Ok(Local(state.get())) } } @@ -1360,8 +1236,8 @@ unsafe impl SystemParam for Deferred<'_, T> { _system_meta: &SystemMeta, _world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Self::Item<'w, 's> { - Deferred(state.get()) + ) -> Result, SystemParamValidationError> { + Ok(Deferred(state.get())) } } @@ -1391,8 +1267,8 @@ unsafe impl SystemParam for ExclusiveMarker { _system_meta: &SystemMeta, _world: UnsafeWorldCell<'world>, _change_tick: Tick, - ) -> Self::Item<'world, 'state> { - Self(PhantomData) + ) -> Result, SystemParamValidationError> { + Ok(Self(PhantomData)) } } @@ -1425,8 +1301,8 @@ unsafe impl SystemParam for NonSendMarker { _system_meta: &SystemMeta, _world: UnsafeWorldCell<'world>, _change_tick: Tick, - ) -> Self::Item<'world, 'state> { - Self(PhantomData) + ) -> Result, SystemParamValidationError> { + Ok(Self(PhantomData)) } } @@ -1464,46 +1340,24 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> { component_access_set.add_unfiltered_component_read(component_id); } - #[inline] - unsafe fn validate_param( - &mut component_id: &mut Self::State, - _system_meta: &SystemMeta, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Read-only access to non-send metadata. - if unsafe { world.storages() } - .non_sends - .get(component_id) - .is_some_and(NonSendData::is_present) - { - Ok(()) - } else { - Err(SystemParamValidationError::invalid::( - "Non-send resource does not exist", - )) - } - } - #[inline] unsafe fn get_param<'w, 's>( &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> Self::Item<'w, 's> { - let (ptr, ticks) = world - .get_non_send_with_ticks(component_id) - .unwrap_or_else(|| { - panic!( - "Non-send resource requested by {} does not exist: {}", - system_meta.name, - DebugName::type_name::() - ); - }); - NonSend { + ) -> Result, SystemParamValidationError> { + let (ptr, ticks) = world.get_non_send_with_ticks(component_id).ok_or_else(|| { + SystemParamValidationError::invalid::(format!( + "Non-send resource requested by {} does not exist: {}", + system_meta.name, + DebugName::type_name::() + )) + })?; + Ok(NonSend { value: ptr.deref(), ticks: ComponentTicksRef::from_tick_cells(ticks, system_meta.last_run, change_tick), - } + }) } } @@ -1538,46 +1392,24 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> { component_access_set.add_unfiltered_component_write(component_id); } - #[inline] - unsafe fn validate_param( - &mut component_id: &mut Self::State, - _system_meta: &SystemMeta, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Read-only access to non-send metadata. - if unsafe { world.storages() } - .non_sends - .get(component_id) - .is_some_and(NonSendData::is_present) - { - Ok(()) - } else { - Err(SystemParamValidationError::invalid::( - "Non-send resource does not exist", - )) - } - } - #[inline] unsafe fn get_param<'w, 's>( &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> Self::Item<'w, 's> { - let (ptr, ticks) = world - .get_non_send_with_ticks(component_id) - .unwrap_or_else(|| { - panic!( - "Non-send resource requested by {} does not exist: {}", - system_meta.name, - DebugName::type_name::() - ); - }); - NonSendMut { + ) -> Result, SystemParamValidationError> { + let (ptr, ticks) = world.get_non_send_with_ticks(component_id).ok_or_else(|| { + SystemParamValidationError::invalid::(format!( + "Non-send resource requested by {} does not exist: {}", + system_meta.name, + DebugName::type_name::() + )) + })?; + Ok(NonSendMut { value: ptr.assert_unique().deref_mut(), ticks: ComponentTicksMut::from_tick_cells(ticks, system_meta.last_run, change_tick), - } + }) } } @@ -1605,8 +1437,8 @@ unsafe impl<'a> SystemParam for &'a Archetypes { _system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Self::Item<'w, 's> { - world.archetypes() + ) -> Result, SystemParamValidationError> { + Ok(world.archetypes()) } } @@ -1634,8 +1466,8 @@ unsafe impl<'a> SystemParam for &'a Components { _system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Self::Item<'w, 's> { - world.components() + ) -> Result, SystemParamValidationError> { + Ok(world.components()) } } @@ -1663,8 +1495,8 @@ unsafe impl<'a> SystemParam for &'a Entities { _system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Self::Item<'w, 's> { - world.entities() + ) -> Result, SystemParamValidationError> { + Ok(world.entities()) } } @@ -1692,8 +1524,8 @@ unsafe impl<'a> SystemParam for &'a EntityAllocator { _system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Self::Item<'w, 's> { - world.entity_allocator() + ) -> Result, SystemParamValidationError> { + Ok(world.entity_allocator()) } } @@ -1721,8 +1553,8 @@ unsafe impl<'a> SystemParam for &'a Bundles { _system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Self::Item<'w, 's> { - world.bundles() + ) -> Result, SystemParamValidationError> { + Ok(world.bundles()) } } @@ -1779,11 +1611,11 @@ unsafe impl SystemParam for SystemChangeTick { system_meta: &SystemMeta, _world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> Self::Item<'w, 's> { - SystemChangeTick { + ) -> Result, SystemParamValidationError> { + Ok(SystemChangeTick { last_run: system_meta.last_run, this_run: change_tick, - } + }) } } @@ -1812,13 +1644,9 @@ unsafe impl SystemParam for Option { system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, change_tick: Tick, - ) -> Self::Item<'world, 'state> { + ) -> Result, SystemParamValidationError> { // SAFETY: Upheld by caller - unsafe { - T::validate_param(state, system_meta, world) - .ok() - .map(|()| T::get_param(state, system_meta, world, change_tick)) - } + Ok(unsafe { T::get_param(state, system_meta, world, change_tick) }.ok()) } fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) { @@ -1858,12 +1686,9 @@ unsafe impl SystemParam for Result, change_tick: Tick, - ) -> Self::Item<'world, 'state> { + ) -> Result, SystemParamValidationError> { // SAFETY: Upheld by caller - unsafe { - T::validate_param(state, system_meta, world) - .map(|()| T::get_param(state, system_meta, world, change_tick)) - } + Ok(unsafe { T::get_param(state, system_meta, world, change_tick) }) } fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) { @@ -1950,28 +1775,20 @@ unsafe impl SystemParam for If { T::init_access(state, system_meta, component_access_set, world); } - #[inline] - unsafe fn validate_param( - state: &mut Self::State, - system_meta: &SystemMeta, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Upheld by caller - unsafe { T::validate_param(state, system_meta, world) }.map_err(|mut e| { - e.skipped = true; - e - }) - } - #[inline] unsafe fn get_param<'world, 'state>( state: &'state mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, change_tick: Tick, - ) -> Self::Item<'world, 'state> { + ) -> Result, SystemParamValidationError> { // SAFETY: Upheld by caller. - If(unsafe { T::get_param(state, system_meta, world, change_tick) }) + unsafe { T::get_param(state, system_meta, world, change_tick) } + .map(If) + .map_err(|mut e| { + e.skipped = true; + e + }) } fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) { @@ -2008,26 +1825,13 @@ unsafe impl SystemParam for Vec { } } - #[inline] - unsafe fn validate_param( - state: &mut Self::State, - system_meta: &SystemMeta, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - for state in state { - // SAFETY: Upheld by caller - unsafe { T::validate_param(state, system_meta, world)? }; - } - Ok(()) - } - #[inline] unsafe fn get_param<'world, 'state>( state: &'state mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, change_tick: Tick, - ) -> Self::Item<'world, 'state> { + ) -> Result, SystemParamValidationError> { state .iter_mut() // SAFETY: @@ -2088,13 +1892,13 @@ unsafe impl SystemParam for ParamSet<'_, '_, Vec> { system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, change_tick: Tick, - ) -> Self::Item<'world, 'state> { - ParamSet { + ) -> Result, SystemParamValidationError> { + Ok(ParamSet { param_states: state, system_meta: system_meta.clone(), world, change_tick, - } + }) } fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) { @@ -2125,6 +1929,7 @@ impl ParamSet<'_, '_, Vec> { self.world, self.change_tick, ) + .expect("ParamSet parameter validation failed") } } @@ -2136,7 +1941,8 @@ impl ParamSet<'_, '_, Vec> { // - We initialized the access for each parameter, so the caller ensures we have access to any world data needed by any param. // We have mutable access to the ParamSet, so no other params in the set are active. // - The caller of `get_param` ensured that this was the world used to initialize our state, and we used that world to initialize parameter states - unsafe { T::get_param(state, &self.system_meta, self.world, self.change_tick) }, + unsafe { T::get_param(state, &self.system_meta, self.world, self.change_tick) } + .unwrap_or_else(|err| panic!("ParamSet parameter validation failed: {err}")), ); }); } @@ -2192,27 +1998,6 @@ macro_rules! impl_system_param_tuple { $($param::queue($param, system_meta, world.reborrow());)* } - #[inline] - unsafe fn validate_param( - state: &mut Self::State, - system_meta: &SystemMeta, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - let ($($param,)*) = state; - - #[allow( - unused_unsafe, - reason = "Zero-length tuples won't have any params to validate." - )] - // SAFETY: Upheld by caller - unsafe { - $( - $param::validate_param($param, system_meta, world)?; - )* - } - Ok(()) - } - #[inline] #[track_caller] unsafe fn get_param<'w, 's>( @@ -2220,7 +2005,7 @@ macro_rules! impl_system_param_tuple { system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> Self::Item<'w, 's> { + ) -> Result, SystemParamValidationError> { let ($($param,)*) = state; #[allow( @@ -2233,7 +2018,7 @@ macro_rules! impl_system_param_tuple { clippy::unused_unit, reason = "Zero-length tuples won't have any params to get." )] - ($($param::get_param($param, system_meta, world, change_tick),)*) + Ok(($($param::get_param($param, system_meta, world, change_tick)?,)*)) } } } @@ -2381,25 +2166,15 @@ unsafe impl SystemParam for StaticSystemParam<'_, '_, P::queue(state, system_meta, world); } - #[inline] - unsafe fn validate_param( - state: &mut Self::State, - system_meta: &SystemMeta, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Upheld by caller - unsafe { P::validate_param(state, system_meta, world) } - } - #[inline] unsafe fn get_param<'world, 'state>( state: &'state mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, change_tick: Tick, - ) -> Self::Item<'world, 'state> { + ) -> Result, SystemParamValidationError> { // SAFETY: Defer to the safety of P::SystemParam - StaticSystemParam(unsafe { P::get_param(state, system_meta, world, change_tick) }) + unsafe { P::get_param(state, system_meta, world, change_tick) }.map(StaticSystemParam) } } @@ -2424,8 +2199,8 @@ unsafe impl SystemParam for PhantomData { _system_meta: &SystemMeta, _world: UnsafeWorldCell<'world>, _change_tick: Tick, - ) -> Self::Item<'world, 'state> { - PhantomData + ) -> Result, SystemParamValidationError> { + Ok(PhantomData) } } @@ -2599,12 +2374,12 @@ where { state .downcast_mut::>>() - .map(|state| { + .and_then(|state| { // SAFETY: // - The caller ensures the world has access for the underlying system param, // and since the downcast succeeded, the underlying system param is T. // - The caller ensures the `world` matches. - unsafe { T::Item::get_param(&mut state.0, system_meta, world, change_tick) } + unsafe { T::Item::get_param(&mut state.0, system_meta, world, change_tick) }.ok() }) } @@ -2636,14 +2411,15 @@ trait DynParamState: Sync + Send + Any { world: &mut World, ); - /// Refer to [`SystemParam::validate_param`]. + /// Validates the inner parameter by calling [`SystemParam::get_param`] and discarding the value. /// /// # Safety - /// Refer to [`SystemParam::validate_param`]. - unsafe fn validate_param( + /// Refer to [`SystemParam::get_param`]. + unsafe fn validate( &mut self, system_meta: &SystemMeta, world: UnsafeWorldCell, + change_tick: Tick, ) -> Result<(), SystemParamValidationError>; } @@ -2668,13 +2444,14 @@ impl DynParamState for ParamState { T::init_access(&self.0, system_meta, component_access_set, world); } - unsafe fn validate_param( + unsafe fn validate( &mut self, system_meta: &SystemMeta, world: UnsafeWorldCell, + change_tick: Tick, ) -> Result<(), SystemParamValidationError> { - // SAFETY: Upheld by caller - unsafe { T::validate_param(&mut self.0, system_meta, world) } + // SAFETY: Upheld by caller. + unsafe { T::get_param(&mut self.0, system_meta, world, change_tick) }.map(drop) } } @@ -2699,29 +2476,25 @@ unsafe impl SystemParam for DynSystemParam<'_, '_> { .init_access(system_meta, component_access_set, world); } - #[inline] - unsafe fn validate_param( - state: &mut Self::State, - system_meta: &SystemMeta, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Upheld by caller. - unsafe { state.0.validate_param(system_meta, world) } - } - #[inline] unsafe fn get_param<'world, 'state>( state: &'state mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, change_tick: Tick, - ) -> Self::Item<'world, 'state> { + ) -> Result, SystemParamValidationError> { + // Validate the inner parameter eagerly so that systems using DynSystemParam + // are correctly skipped by the executor when the inner param is unavailable. + // SAFETY: Upheld by caller. + unsafe { state.0.validate(system_meta, world, change_tick) }?; // SAFETY: // - `state.0` is a boxed `ParamState`. // - `init_access` calls `DynParamState::init_access`, which calls `init_access` on the inner parameter, // so the caller ensures the world has the necessary access. // - The caller ensures that the provided world is the same and has the required access. - unsafe { DynSystemParam::new(state.0.as_mut(), world, system_meta.clone(), change_tick) } + Ok(unsafe { + DynSystemParam::new(state.0.as_mut(), world, system_meta.clone(), change_tick) + }) } fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) { @@ -2769,10 +2542,10 @@ unsafe impl SystemParam for FilteredResources<'_, '_> { system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, change_tick: Tick, - ) -> Self::Item<'world, 'state> { + ) -> Result, SystemParamValidationError> { // SAFETY: The caller ensures that `world` has access to anything registered in `init_access`, // and we registered all resource access in `state``. - unsafe { FilteredResources::new(world, state, system_meta.last_run, change_tick) } + Ok(unsafe { FilteredResources::new(world, state, system_meta.last_run, change_tick) }) } } @@ -2815,17 +2588,17 @@ unsafe impl SystemParam for FilteredResourcesMut<'_, '_> { system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, change_tick: Tick, - ) -> Self::Item<'world, 'state> { + ) -> Result, SystemParamValidationError> { // SAFETY: The caller ensures that `world` has access to anything registered in `init_access`, // and we registered all resource access in `state``. - unsafe { FilteredResourcesMut::new(world, state, system_meta.last_run, change_tick) } + Ok(unsafe { FilteredResourcesMut::new(world, state, system_meta.last_run, change_tick) }) } } /// An error that occurs when a system parameter is not valid, /// used by system executors to determine what to do with a system. /// -/// Returned as an error from [`SystemParam::validate_param`], +/// Returned as an error from [`SystemParam::get_param`], /// and handled using the unified error handling mechanisms defined in [`bevy_ecs::error`]. #[derive(Debug, PartialEq, Eq, Clone, Error)] pub struct SystemParamValidationError { diff --git a/crates/bevy_ecs/src/world/identifier.rs b/crates/bevy_ecs/src/world/identifier.rs index ea4a2b20bc74b..29cdde925362e 100644 --- a/crates/bevy_ecs/src/world/identifier.rs +++ b/crates/bevy_ecs/src/world/identifier.rs @@ -2,7 +2,10 @@ use crate::{ change_detection::Tick, query::FilteredAccessSet, storage::SparseSetIndex, - system::{ExclusiveSystemParam, ReadOnlySystemParam, SystemMeta, SystemParam}, + system::{ + ExclusiveSystemParam, ReadOnlySystemParam, SystemMeta, SystemParam, + SystemParamValidationError, + }, world::{FromWorld, World}, }; use bevy_platform::sync::atomic::{AtomicUsize, Ordering}; @@ -70,8 +73,8 @@ unsafe impl SystemParam for WorldId { _: &SystemMeta, world: UnsafeWorldCell<'world>, _: Tick, - ) -> Self::Item<'world, 'state> { - world.id() + ) -> Result, SystemParamValidationError> { + Ok(world.id()) } } diff --git a/crates/bevy_gizmos/src/gizmos.rs b/crates/bevy_gizmos/src/gizmos.rs index efe24bdfa35bd..605b52601d43a 100644 --- a/crates/bevy_gizmos/src/gizmos.rs +++ b/crates/bevy_gizmos/src/gizmos.rs @@ -228,44 +228,13 @@ where GizmosState::::queue(&mut state.state, system_meta, world); } - #[inline] - unsafe fn validate_param( - state: &mut Self::State, - system_meta: &SystemMeta, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Delegated to existing `SystemParam` implementation. - unsafe { - GizmosState::::validate_param(&mut state.state, system_meta, world)?; - } - - // SAFETY: Delegated to existing `SystemParam` implementation. - let (_, f1) = unsafe { - GizmosState::::get_param( - &mut state.state, - system_meta, - world, - world.change_tick(), - ) - }; - // This if-block is to accommodate an Option SystemParam. - // The user may decide not to initialize a gizmo group, so its config will not exist. - if f1.get_config::().is_none() { - Err(SystemParamValidationError::invalid::( - format!("Requested config {} does not exist in `GizmoConfigStore`! Did you forget to add it using `app.init_gizmo_group()`?", - Config::type_path()))) - } else { - Ok(()) - } - } - #[inline] unsafe fn get_param<'w, 's>( state: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> Self::Item<'w, 's> { + ) -> Result, SystemParamValidationError> { // SAFETY: Delegated to existing `SystemParam` implementations. let (mut f0, f1) = unsafe { GizmosState::::get_param( @@ -273,20 +242,28 @@ where system_meta, world, change_tick, - ) + )? }; + // This if-block is to accommodate an Option SystemParam. + // The user may decide not to initialize a gizmo group, so its config will not exist. + if f1.get_config::().is_none() { + return Err(SystemParamValidationError::invalid::( + format!("Requested config {} does not exist in `GizmoConfigStore`! Did you forget to add it using `app.init_gizmo_group()`?", + Config::type_path()))); + } + // Accessing the GizmoConfigStore in every API call reduces performance significantly. // Implementing SystemParam manually allows us to cache whether the config is currently enabled. // Having this available allows for cheap early returns when gizmos are disabled. let (config, config_ext) = f1.into_inner().config::(); f0.enabled = config.enabled; - Gizmos { + Ok(Gizmos { buffer: f0, config, config_ext, - } + }) } } diff --git a/crates/bevy_input_focus/src/tab_navigation.rs b/crates/bevy_input_focus/src/tab_navigation.rs index 26dfc613b7d4e..4ee868efdda38 100644 --- a/crates/bevy_input_focus/src/tab_navigation.rs +++ b/crates/bevy_input_focus/src/tab_navigation.rs @@ -478,7 +478,7 @@ mod tests { let tab_entity_2 = world.spawn((TabIndex(1), ChildOf(tab_group_entity))).id(); let mut system_state: SystemState = SystemState::new(world); - let tab_navigation = system_state.get(world); + let tab_navigation = system_state.get(world).unwrap(); assert_eq!(tab_navigation.tabgroup_query.iter().count(), 1); assert!(tab_navigation.tabindex_query.iter().count() >= 2); @@ -511,7 +511,7 @@ mod tests { let tab_entity_4 = world.spawn((TabIndex(1), ChildOf(tab_group_2))).id(); let mut system_state: SystemState = SystemState::new(world); - let tab_navigation = system_state.get(world); + let tab_navigation = system_state.get(world).unwrap(); assert_eq!(tab_navigation.tabgroup_query.iter().count(), 2); assert!(tab_navigation.tabindex_query.iter().count() >= 4); diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs index 7c5f008f54541..7cbbd4a314ab1 100644 --- a/crates/bevy_pbr/src/material.rs +++ b/crates/bevy_pbr/src/material.rs @@ -929,7 +929,7 @@ pub(crate) fn specialize_material_meshes( mut specialized_material_pipeline_cache, mut pending_mesh_material_queues, dirty_specializations, - } = state.get_mut(world); + } = state.get_mut(world).unwrap(); for (view, visible_entities) in &views { all_views.insert(view.retained_view_entity); diff --git a/crates/bevy_pbr/src/meshlet/instance_manager.rs b/crates/bevy_pbr/src/meshlet/instance_manager.rs index a7f0e6c0c2820..fe5a7067245ff 100644 --- a/crates/bevy_pbr/src/meshlet/instance_manager.rs +++ b/crates/bevy_pbr/src/meshlet/instance_manager.rs @@ -220,7 +220,7 @@ pub fn extract_meshlet_mesh_entities( } let system_state = system_state.as_mut().unwrap(); let (instances_query, asset_server, mut assets, mut asset_events) = - system_state.get_mut(&mut main_world); + system_state.get_mut(&mut main_world).unwrap(); // Reset per-frame data instance_manager.reset(render_entities); @@ -228,7 +228,7 @@ pub fn extract_meshlet_mesh_entities( // Free GPU buffer space for any modified or dropped MeshletMesh assets for asset_event in asset_events.read() { if let AssetEvent::Unused { id } | AssetEvent::Modified { id } = asset_event { - meshlet_mesh_manager.remove(id); + meshlet_mesh_manager.remove(&id); } } diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index 0df4717eb3572..f41f2c44b15f1 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -909,7 +909,7 @@ pub(crate) fn specialize_prepass_material_meshes( mut pending_prepass_mesh_material_queues, dirty_specializations, this_run: system_change_tick, - } = state.get_mut(world); + } = state.get_mut(world).unwrap(); this_run = system_change_tick.this_run(); diff --git a/crates/bevy_pbr/src/render/light.rs b/crates/bevy_pbr/src/render/light.rs index f4157b4a12f05..d1cb7e27c853b 100644 --- a/crates/bevy_pbr/src/render/light.rs +++ b/crates/bevy_pbr/src/render/light.rs @@ -1942,7 +1942,7 @@ pub(crate) fn specialize_shadows( mut specialized_shadow_material_pipeline_cache, mut pending_shadow_queues, dirty_specializations, - } = state.get_mut(world); + } = state.get_mut(world).unwrap(); for (entity, view_lights) in &view_lights { for view_light_entity in view_lights.lights.iter().copied() { diff --git a/crates/bevy_render/src/erased_render_asset.rs b/crates/bevy_render/src/erased_render_asset.rs index 05d55ecc23745..8e530bbf63527 100644 --- a/crates/bevy_render/src/erased_render_asset.rs +++ b/crates/bevy_render/src/erased_render_asset.rs @@ -248,7 +248,7 @@ pub(crate) fn extract_erased_render_asset( ) { main_world.resource_scope( |world, mut cached_state: Mut>| { - let (mut events, mut assets) = cached_state.state.get_mut(world); + let (mut events, mut assets) = cached_state.state.get_mut(world).unwrap(); let mut needs_extracting = >::default(); let mut removed = >::default(); diff --git a/crates/bevy_render/src/extract_param.rs b/crates/bevy_render/src/extract_param.rs index 4d81fb9eb1903..0659cec965403 100644 --- a/crates/bevy_render/src/extract_param.rs +++ b/crates/bevy_render/src/extract_param.rs @@ -94,37 +94,13 @@ where ); } - #[inline] - unsafe fn validate_param( - state: &mut Self::State, - _system_meta: &SystemMeta, - world: UnsafeWorldCell, - ) -> 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 Err(SystemParamValidationError::invalid::( - "`MainWorld` resource does not exist", - )); - }; - // SAFETY: Type is guaranteed by `SystemState`. - let main_world: &World = unsafe { main_world.deref() }; - // SAFETY: We provide the main world on which this system state was initialized on. - unsafe { - SystemState::

::validate_param( - &mut state.state, - main_world.as_unsafe_world_cell_readonly(), - ) - } - } - #[inline] unsafe fn get_param<'w, 's>( state: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> Self::Item<'w, 's> { + ) -> Result, SystemParamValidationError> { // SAFETY: // - The caller ensures that `world` is the same one that `init_state` was called with. // - The caller ensures that no other `SystemParam`s will conflict with the accesses we have registered. @@ -134,10 +110,10 @@ where system_meta, world, change_tick, - ) + )? }; - let item = state.state.get(main_world.into_inner()); - Extract { item } + let item = state.state.get(main_world.into_inner())?; + Ok(Extract { item }) } } diff --git a/crates/bevy_render/src/render_asset.rs b/crates/bevy_render/src/render_asset.rs index 8d90e5b5200b4..43ef2bb5807b5 100644 --- a/crates/bevy_render/src/render_asset.rs +++ b/crates/bevy_render/src/render_asset.rs @@ -262,7 +262,7 @@ pub(crate) fn extract_render_asset( ) { main_world.resource_scope( |world, mut cached_state: Mut>| { - let (mut events, mut assets, maybe_render_assets) = cached_state.state.get_mut(world); + let (mut events, mut assets, maybe_render_assets) = cached_state.state.get_mut(world).unwrap(); let mut needs_extracting = >::default(); let mut removed = >::default(); diff --git a/crates/bevy_render/src/render_phase/draw.rs b/crates/bevy_render/src/render_phase/draw.rs index 319c907843f68..13d4f67da52eb 100644 --- a/crates/bevy_render/src/render_phase/draw.rs +++ b/crates/bevy_render/src/render_phase/draw.rs @@ -326,7 +326,7 @@ where view: Entity, item: &P, ) -> Result<(), DrawError> { - let param = self.state.get(world); + let param = self.state.get(world).unwrap(); let view = match self.view.get_manual(world, view) { Ok(view) => view, Err(err) => match err { diff --git a/crates/bevy_render/src/renderer/mod.rs b/crates/bevy_render/src/renderer/mod.rs index d19d29f0eb08a..38bb7f1ccc674 100644 --- a/crates/bevy_render/src/renderer/mod.rs +++ b/crates/bevy_render/src/renderer/mod.rs @@ -89,7 +89,7 @@ pub fn render_system( let _span = info_span!("present_frames").entered(); world.resource_scope(|world, mut windows: Mut| { - let views = state.get(world); + let views = state.get(world).unwrap(); for window in windows.values_mut() { let view_needs_present = views.iter().any(|(view_target, camera)| { matches!( diff --git a/crates/bevy_render/src/renderer/render_context.rs b/crates/bevy_render/src/renderer/render_context.rs index 5a21ada6f1fb2..954a66b4b23ee 100644 --- a/crates/bevy_render/src/renderer/render_context.rs +++ b/crates/bevy_render/src/renderer/render_context.rs @@ -260,11 +260,12 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam } #[inline] - unsafe fn validate_param( - state: &mut Self::State, + unsafe fn get_param<'w, 's>( + state: &'s mut Self::State, _system_meta: &SystemMeta, - world: UnsafeWorldCell, - ) -> Result<(), SystemParamValidationError> { + world: UnsafeWorldCell<'w>, + _change_tick: Tick, + ) -> Result, SystemParamValidationError> { // SAFETY: We have registered resource read access in init_access let current_view = unsafe { world.get_resource::() }; @@ -278,47 +279,18 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam // SAFETY: Query state access is properly registered in init_access. // The caller ensures the world matches the one used in init_state. - let result = unsafe { state.query_state.get_unchecked(world, entity) }; - - if result.is_err() { - return Err(SystemParamValidationError::skipped::( - "Current view entity does not match query", - )); - } - - Ok(()) - } - - #[inline] - unsafe fn get_param<'w, 's>( - state: &'s mut Self::State, - _system_meta: &SystemMeta, - world: UnsafeWorldCell<'w>, - _change_tick: Tick, - ) -> Self::Item<'w, 's> { - // SAFETY: We have registered resource read access and validate_param succeeded - let current_view = unsafe { - world - .get_resource::() - .expect("CurrentView must exist") - }; - - let entity = current_view.entity(); - - // SAFETY: Query state access is properly registered in init_access. - // validate_param verified the entity matches. - let item = unsafe { - state - .query_state - .get_unchecked(world, entity) - .expect("view entity must match query") - }; - - ViewQuery { + let item = unsafe { state.query_state.get_unchecked(world, entity) } + .map_err(|_| { + SystemParamValidationError::skipped::( + "Current view entity does not match query", + ) + })?; + + Ok(ViewQuery { entity, item, _filter: PhantomData, - } + }) } } diff --git a/crates/bevy_render/src/sync_world.rs b/crates/bevy_render/src/sync_world.rs index c98afc2c57bf7..cf92577e53326 100644 --- a/crates/bevy_render/src/sync_world.rs +++ b/crates/bevy_render/src/sync_world.rs @@ -253,7 +253,7 @@ pub(crate) fn despawn_temporary_render_entities( state: &mut SystemState>>, mut local: Local>, ) { - let query = state.get(world); + let query = state.get(world).unwrap(); local.extend(query.iter()); diff --git a/crates/bevy_render/src/view/window/screenshot.rs b/crates/bevy_render/src/view/window/screenshot.rs index ae2a5f9efaaea..025aac44ac58f 100644 --- a/crates/bevy_render/src/view/window/screenshot.rs +++ b/crates/bevy_render/src/view/window/screenshot.rs @@ -229,7 +229,8 @@ fn extract_screenshots( *system_state = Some(SystemState::new(&mut main_world)); } let system_state = system_state.as_mut().unwrap(); - let (mut commands, primary_window, screenshots) = system_state.get_mut(&mut main_world); + let (mut commands, primary_window, screenshots) = + system_state.get_mut(&mut main_world).unwrap(); targets.clear(); seen_targets.clear(); diff --git a/crates/bevy_transform/src/helper.rs b/crates/bevy_transform/src/helper.rs index 1ab33b4b421bc..f0dcfc58b85bd 100644 --- a/crates/bevy_transform/src/helper.rs +++ b/crates/bevy_transform/src/helper.rs @@ -138,7 +138,7 @@ mod tests { let transform = *app.world().get::(leaf_entity).unwrap(); let mut state = SystemState::::new(app.world_mut()); - let helper = state.get(app.world()); + let helper = state.get(app.world()).unwrap(); let computed_transform = helper.compute_global_transform(leaf_entity).unwrap(); diff --git a/crates/bevy_ui/src/experimental/ghost_hierarchy.rs b/crates/bevy_ui/src/experimental/ghost_hierarchy.rs index 79d081c6bfd4d..2c942cde2ab46 100644 --- a/crates/bevy_ui/src/experimental/ghost_hierarchy.rs +++ b/crates/bevy_ui/src/experimental/ghost_hierarchy.rs @@ -230,7 +230,7 @@ mod tests { }); let mut system_state = SystemState::<(UiRootNodes, Query<&A>)>::new(world); - let (ui_root_nodes, a_query) = system_state.get(world); + let (ui_root_nodes, a_query) = system_state.get(world).unwrap(); let result: Vec<_> = a_query.iter_many(ui_root_nodes.iter()).collect(); @@ -263,7 +263,7 @@ mod tests { world.entity_mut(n9).add_children(&[n10]); let mut system_state = SystemState::<(UiChildren, Query<&A>)>::new(world); - let (ui_children, a_query) = system_state.get(world); + let (ui_children, a_query) = system_state.get(world).unwrap(); let result: Vec<_> = a_query .iter_many(ui_children.iter_ui_children(n1)) diff --git a/crates/bevy_winit/src/cursor/mod.rs b/crates/bevy_winit/src/cursor/mod.rs index 7dcbc6d99375b..6d5a5471967a6 100644 --- a/crates/bevy_winit/src/cursor/mod.rs +++ b/crates/bevy_winit/src/cursor/mod.rs @@ -67,13 +67,13 @@ impl WinitAppRunnerState { Query<(Entity, &mut PendingCursor), Changed>, )> = SystemState::new(self.world_mut()); #[cfg(feature = "custom_cursor")] - let (mut cursor_cache, mut windows) = windows_state.get_mut(self.world_mut()); + let (mut cursor_cache, mut windows) = windows_state.get_mut(self.world_mut()).unwrap(); #[cfg(not(feature = "custom_cursor"))] let mut windows_state: SystemState<( Query<(Entity, &mut PendingCursor), Changed>, )> = SystemState::new(self.world_mut()); #[cfg(not(feature = "custom_cursor"))] - let (mut windows,) = windows_state.get_mut(self.world_mut()); + let (mut windows,) = windows_state.get_mut(self.world_mut()).unwrap(); WINIT_WINDOWS.with_borrow(|winit_windows| { for (entity, mut pending_cursor) in windows.iter_mut() { diff --git a/crates/bevy_winit/src/state.rs b/crates/bevy_winit/src/state.rs index 1abaaaf6764e4..afc3d7573729a 100644 --- a/crates/bevy_winit/src/state.rs +++ b/crates/bevy_winit/src/state.rs @@ -181,7 +181,7 @@ impl ApplicationHandler for WinitAppRunnerState { // Create the initial window if needed let mut create_window = SystemState::::from_world(self.world_mut()); - create_windows(event_loop, create_window.get_mut(self.world_mut())); + create_windows(event_loop, create_window.get_mut(self.world_mut()).unwrap()); create_window.apply(self.world_mut()); } @@ -195,7 +195,7 @@ impl ApplicationHandler for WinitAppRunnerState { WinitUserEvent::WindowAdded => { let mut create_window = SystemState::::from_world(self.world_mut()); - create_windows(event_loop, create_window.get_mut(self.world_mut())); + create_windows(event_loop, create_window.get_mut(self.world_mut()).unwrap()); create_window.apply(self.world_mut()); } } @@ -224,7 +224,8 @@ impl ApplicationHandler for WinitAppRunnerState { mut windows, ) = self .message_writer_system_state - .get_mut(self.app.world_mut()); + .get_mut(self.app.world_mut()) + .unwrap(); let Some(window) = winit_windows.get_window_entity(window_id) else { warn!("Skipped event {event:?} for unknown winit Window Id {window_id:?}"); @@ -459,7 +460,7 @@ impl ApplicationHandler for WinitAppRunnerState { fn about_to_wait(&mut self, event_loop: &ActiveEventLoop) { let mut create_monitor = SystemState::::from_world(self.world_mut()); - create_monitors(event_loop, create_monitor.get_mut(self.world_mut())); + create_monitors(event_loop, create_monitor.get_mut(self.world_mut()).unwrap()); create_monitor.apply(self.world_mut()); // TODO: This is a workaround for https://github.com/bevyengine/bevy/issues/17488 @@ -516,7 +517,7 @@ impl WinitAppRunnerState { let mut focused_windows_state: SystemState<(Res, Query<(Entity, &Window)>)> = SystemState::new(self.world_mut()); - let (config, windows) = focused_windows_state.get(self.world()); + let (config, windows) = focused_windows_state.get(self.world()).unwrap(); let focused = windows.iter().any(|(_, window)| window.focused); let mut update_mode = config.update_mode(focused); @@ -605,7 +606,7 @@ impl WinitAppRunnerState { let begin_frame_time = Instant::now(); if should_update { - let (_, windows) = focused_windows_state.get(self.world()); + let (_, windows) = focused_windows_state.get(self.world()).unwrap(); // If no windows exist, this will evaluate to `true`. let all_invisible = windows.iter().all(|w| !w.1.visible); @@ -648,7 +649,7 @@ impl WinitAppRunnerState { } // Running the app may have changed the WinitSettings resource, so we have to re-extract it. - let (config, windows) = focused_windows_state.get(self.world()); + let (config, windows) = focused_windows_state.get(self.world()).unwrap(); let focused = windows.iter().any(|(_, window)| window.focused); update_mode = config.update_mode(focused); } diff --git a/examples/async_tasks/async_compute.rs b/examples/async_tasks/async_compute.rs index 1018a13c37548..7fab1c7e136bf 100644 --- a/examples/async_tasks/async_compute.rs +++ b/examples/async_tasks/async_compute.rs @@ -91,7 +91,7 @@ fn spawn_tasks(mut commands: Commands) { Res, )>::new(world); let (box_mesh_handle, box_material_handle) = - system_state.get_mut(world); + system_state.get_mut(world).unwrap(); (box_mesh_handle.clone(), box_material_handle.clone()) }; diff --git a/examples/ecs/fallible_params.rs b/examples/ecs/fallible_params.rs index d70ae13bfe191..65008403b51b9 100644 --- a/examples/ecs/fallible_params.rs +++ b/examples/ecs/fallible_params.rs @@ -10,14 +10,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 [`SystemParamValidationError`] returned -//! by [`SystemParam::validate_param`] for each system parameter. +//! by [`SystemParam::get_param`] for each system parameter. //! 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 [`SystemParamValidationError`] failures, //! please see the `error_handling.rs` example. //! //! [`SystemParamValidationError`]: bevy::ecs::system::SystemParamValidationError -//! [`SystemParam::validate_param`]: bevy::ecs::system::SystemParam::validate_param +//! [`SystemParam::get_param`]: bevy::ecs::system::SystemParam::get_param use bevy::ecs::error::warn; use bevy::prelude::*; From 7c0c0458155ee8c9ac203dbf186d5ef12020648e Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 4 Mar 2026 12:58:15 -0800 Subject: [PATCH 04/23] Beef up migration guide to account for changes required --- .../migration-guides/validation_merging.md | 78 ++++++++++++++++++- 1 file changed, 74 insertions(+), 4 deletions(-) diff --git a/release-content/migration-guides/validation_merging.md b/release-content/migration-guides/validation_merging.md index 8464cac780d3d..fba642b07db42 100644 --- a/release-content/migration-guides/validation_merging.md +++ b/release-content/migration-guides/validation_merging.md @@ -10,16 +10,86 @@ To be more precise: - `SystemParam::get_param` now returns a `Result, SystemParamValidationError>`, instead of simply a `Self::Item<'world, 'state>` - If validation fails, an appropriate `SystemParamValidationError` should be returned - If validation passes, the item should be returned wrapped in `Ok` -- `SystemParam::validate_params` has been removed - - All logic that's currently done in this methods should be moved to the `get_param` method of that type -- `SystemState::fetch`, `get_unchecked`, `get` and `get_mut` now return a `Result` which may hold a `SystemParamValidationError`. In most prior cases, this would panic if validation would have failed. +- `SystemParam::validate_param` has been removed + - All logic that was done in this method should be moved to the `get_param` method of that type +- `SystemState::validate_param` has been removed + - Validation now happens automatically when calling `get`, `get_mut`, or `get_unchecked` +- `SystemState::fetch`, `get_unchecked`, `get` and `get_mut` now return a `Result<..., SystemParamValidationError>`. Callers that previously destructured the result directly will need to add `.unwrap()` or handle the `Result`: + +```rust +// Before +let (res, query) = system_state.get(&world); + +// After +let (res, query) = system_state.get(&world).unwrap(); +``` When executing systems, we no longer check for system validation before running the systems. As a result of these changes, `System::validate_param` and `System::validate_param_unsafe` have been removed. Instead, validation has been moved to be part of the trait implementation for `System::run_unsafe`. -All implementations of the `System` should validate that their parameters are valid during this method, +All implementations of the `System` trait should validate that their parameters are valid during this method, bubbling up any errors originating in `SystemParam::get_param`. +### Custom `SystemParam` implementations + +If you have a custom `SystemParam` implementation, you need to: + +1. Remove the `validate_param` method. +2. Move any validation logic into `get_param`. +3. Change `get_param` to return `Result, SystemParamValidationError>`. + +```rust +// Before +unsafe impl SystemParam for MyParam<'_> { + // ... + unsafe fn validate_param( + state: &Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell, + ) -> Result<(), SystemParamValidationError> { + // validation logic + if !is_valid(state, world) { + return Err(SystemParamValidationError::invalid::("not valid")); + } + Ok(()) + } + + unsafe fn get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Self::Item<'w, 's> { + // fetch logic + MyParam { /* ... */ } + } +} + +// After +unsafe impl SystemParam for MyParam<'_> { + // ... + unsafe fn get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // validation logic merged into get_param + if !is_valid(state, world) { + return Err(SystemParamValidationError::invalid::("not valid")); + } + // fetch logic + Ok(MyParam { /* ... */ }) + } +} +``` + +### Custom `System` implementations + +If you have a custom `System` implementation, remove the `validate_param_unsafe` method. Parameter validation should now occur inside `run_unsafe` by propagating errors from `SystemParam::get_param`. + +### `MultithreadedExecutor` behavior change + For the parallel `MultithreadedExecutor`, validation was previously done while checking run conditions. Now, tasks will be spawned for systems which would fail or are skipped during validation. If you find that this is a significant performance overhead for your use case, From 8303d4fba2d957aecdc5481e52757da46f7c3012 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 4 Mar 2026 13:31:56 -0800 Subject: [PATCH 05/23] Add tests for validation occurring when systems are run --- crates/bevy_ecs/src/schedule/executor/mod.rs | 223 +++++++++++++++++++ 1 file changed, 223 insertions(+) diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index 73ee92e61b5cb..cb7215ecf308f 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -552,3 +552,226 @@ mod tests { assert_eq!(counter.0, 0); } } + +#[cfg(test)] +mod validation_tests { + use crate::{ + prelude::{Component, In, IntoSystem, Resource, Schedule}, + schedule::ExecutorKind, + system::{ + DynParamBuilder, DynSystemParam, ParamBuilder, Res, ResMut, RunSystemError, + RunSystemOnce, Single, SystemParamBuilder, + }, + world::World, + }; + + #[derive(Component)] + struct TestComponent; + + const EXECUTORS: [ExecutorKind; 2] = + [ExecutorKind::SingleThreaded, ExecutorKind::MultiThreaded]; + + #[derive(Resource, Default)] + struct Counter(u8); + + // A resource that won't be inserted, causing validation to fail. + #[derive(Resource)] + struct MissingResource; + + #[test] + fn function_system_validation_failure_is_error() { + fn system(_res: Res) {} + + let mut world = World::new(); + let result = world.run_system_once(system); + assert!( + matches!(result, Err(RunSystemError::Failed(_))), + "Expected Failed, got {result:?}" + ); + } + + #[test] + fn function_system_validation_skip() { + fn system(_single: Single<&TestComponent>) {} + + let mut world = World::new(); + let result = world.run_system_once(system); + assert!( + matches!(result, Err(RunSystemError::Skipped(_))), + "Expected Skipped, got {result:?}" + ); + } + + #[test] + fn function_system_validation_success() { + fn system(_res: Res) {} + + let mut world = World::new(); + world.init_resource::(); + let result = world.run_system_once(system); + assert!(result.is_ok(), "Expected Ok, got {result:?}"); + } + + #[test] + fn adapter_system_validation_failure() { + fn system(_res: Res) -> u32 { + 42 + } + + let mut world = World::new(); + let result = world.run_system_once(system.map(|_x| {})); + assert!( + matches!(result, Err(RunSystemError::Failed(_))), + "Expected Failed from adapter system, got {result:?}" + ); + } + + #[test] + fn adapter_system_validation_skip() { + fn system(_single: Single<&TestComponent>) -> u32 { + 42 + } + + let mut world = World::new(); + let result = world.run_system_once(system.map(|_x| {})); + assert!( + matches!(result, Err(RunSystemError::Skipped(_))), + "Expected Skipped from adapter system, got {result:?}" + ); + } + + #[test] + fn pipe_system_validation_failure_in_first() { + fn first(_res: Res) -> u32 { + 42 + } + fn second(_input: In) {} + + let mut world = World::new(); + let result = world.run_system_once(first.pipe(second)); + assert!( + matches!(result, Err(RunSystemError::Failed(_))), + "Expected Failed from pipe first system, got {result:?}" + ); + } + + #[test] + fn pipe_system_validation_failure_in_second() { + fn first() -> u32 { + 42 + } + fn second(_input: In, _res: Res) {} + + let mut world = World::new(); + let result = world.run_system_once(first.pipe(second)); + assert!( + matches!(result, Err(RunSystemError::Failed(_))), + "Expected Failed from pipe second system, got {result:?}" + ); + } + + #[test] + fn pipe_system_validation_skip_in_first() { + fn first(_single: Single<&TestComponent>) -> u32 { + 42 + } + fn second(_input: In) {} + + let mut world = World::new(); + let result = world.run_system_once(first.pipe(second)); + assert!( + matches!(result, Err(RunSystemError::Skipped(_))), + "Expected Skipped from pipe first system, got {result:?}" + ); + } + + #[test] + fn builder_system_validation_failure() { + fn system(_res: Res) {} + + let mut world = World::new(); + let result = world.run_system_once(ParamBuilder.build_system(system)); + assert!( + matches!(result, Err(RunSystemError::Failed(_))), + "Expected Failed from builder system, got {result:?}" + ); + } + + #[test] + fn builder_system_validation_skip() { + fn system(_single: Single<&TestComponent>) {} + + let mut world = World::new(); + let result = world.run_system_once(ParamBuilder.build_system(system)); + assert!( + matches!(result, Err(RunSystemError::Skipped(_))), + "Expected Skipped from builder system, got {result:?}" + ); + } + + #[test] + fn dyn_system_param_validation_failure() { + let mut world = World::new(); + let system = (DynParamBuilder::new::>(ParamBuilder),) + .build_state(&mut world) + .build_system(|_param: DynSystemParam| {}); + let result = world.run_system_once(system); + assert!( + matches!(result, Err(RunSystemError::Failed(_))), + "Expected Failed from DynSystemParam system, got {result:?}" + ); + } + + #[test] + fn dyn_system_param_validation_skip() { + let mut world = World::new(); + let system = (DynParamBuilder::new::>(ParamBuilder),) + .build_state(&mut world) + .build_system(|_param: DynSystemParam| {}); + let result = world.run_system_once(system); + assert!( + matches!(result, Err(RunSystemError::Skipped(_))), + "Expected Skipped from DynSystemParam system, got {result:?}" + ); + } + + #[test] + fn dyn_system_param_validation_success() { + let mut world = World::new(); + world.init_resource::(); + let system = (DynParamBuilder::new::>(ParamBuilder),) + .build_state(&mut world) + .build_system(|_param: DynSystemParam| {}); + let result = world.run_system_once(system); + assert!( + result.is_ok(), + "Expected Ok from DynSystemParam system, got {result:?}" + ); + } + + #[test] + fn validation_skips_system_in_schedule() { + // Ensure the executor properly handles validation failures by skipping + // and not running the system body. + fn skippable_system(_single: Single<&TestComponent>, mut counter: ResMut) { + counter.0 += 1; + } + + for executor in EXECUTORS { + let mut world = World::new(); + world.init_resource::(); + + let mut schedule = Schedule::default(); + schedule.set_executor_kind(executor); + schedule.add_systems(skippable_system); + + // No TestComponent entity exists, so the system should be skipped. + schedule.run(&mut world); + assert_eq!( + world.resource::().0, + 0, + "System should have been skipped with {executor:?}" + ); + } + } +} From fbcbbb935dbf162e0c9b9c2364f3222f215c5589 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 4 Mar 2026 13:41:32 -0800 Subject: [PATCH 06/23] Perform the same changes for ExclusiveSystemParam --- crates/bevy_ecs/src/schedule/executor/mod.rs | 48 ++++++++++++++++++- .../src/system/exclusive_function_system.rs | 2 +- .../src/system/exclusive_system_param.rs | 39 ++++++++++----- crates/bevy_ecs/src/system/system_name.rs | 7 ++- crates/bevy_ecs/src/world/identifier.rs | 4 +- .../migration-guides/validation_merging.md | 29 +++++++++++ 6 files changed, 110 insertions(+), 19 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index cb7215ecf308f..d10fb30908076 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -559,8 +559,9 @@ mod validation_tests { prelude::{Component, In, IntoSystem, Resource, Schedule}, schedule::ExecutorKind, system::{ - DynParamBuilder, DynSystemParam, ParamBuilder, Res, ResMut, RunSystemError, - RunSystemOnce, Single, SystemParamBuilder, + DynParamBuilder, DynSystemParam, ExclusiveSystemParam, Local, ParamBuilder, Res, + ResMut, RunSystemError, RunSystemOnce, Single, SystemMeta, SystemParamBuilder, + SystemParamValidationError, }, world::World, }; @@ -578,6 +579,25 @@ mod validation_tests { #[derive(Resource)] struct MissingResource; + /// An [`ExclusiveSystemParam`] that always fails validation. + struct AlwaysInvalid; + + impl ExclusiveSystemParam for AlwaysInvalid { + type State = (); + type Item<'s> = AlwaysInvalid; + + fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {} + + fn get_param<'s>( + _state: &'s mut Self::State, + _system_meta: &SystemMeta, + ) -> Result, SystemParamValidationError> { + Err(SystemParamValidationError::invalid::( + "always invalid", + )) + } + } + #[test] fn function_system_validation_failure_is_error() { fn system(_res: Res) {} @@ -749,6 +769,30 @@ mod validation_tests { ); } + #[test] + fn exclusive_system_validation_failure() { + fn system(_world: &mut World, _param: AlwaysInvalid) {} + + let mut world = World::new(); + let result = world.run_system_once(system); + assert!( + matches!(result, Err(RunSystemError::Failed(_))), + "Expected Failed from exclusive system, got {result:?}" + ); + } + + #[test] + fn exclusive_system_validation_success() { + fn system(_world: &mut World, mut _local: Local) {} + + let mut world = World::new(); + let result = world.run_system_once(system); + assert!( + result.is_ok(), + "Expected Ok from exclusive system, got {result:?}" + ); + } + #[test] fn validation_skips_system_in_schedule() { // Ensure the executor properly handles validation failures by skipping diff --git a/crates/bevy_ecs/src/system/exclusive_function_system.rs b/crates/bevy_ecs/src/system/exclusive_function_system.rs index 809ded7cdf924..7948ef197908c 100644 --- a/crates/bevy_ecs/src/system/exclusive_function_system.rs +++ b/crates/bevy_ecs/src/system/exclusive_function_system.rs @@ -117,7 +117,7 @@ where let params = F::Param::get_param( self.param_state.as_mut().expect(PARAM_MESSAGE), &self.system_meta, - ); + )?; #[cfg(feature = "hotpatching")] let out = { diff --git a/crates/bevy_ecs/src/system/exclusive_system_param.rs b/crates/bevy_ecs/src/system/exclusive_system_param.rs index 89bd1b0d8bcfe..7950c311bd6e7 100644 --- a/crates/bevy_ecs/src/system/exclusive_system_param.rs +++ b/crates/bevy_ecs/src/system/exclusive_system_param.rs @@ -1,7 +1,7 @@ use crate::{ prelude::{FromWorld, QueryState}, query::{QueryData, QueryFilter}, - system::{Local, SystemMeta, SystemParam, SystemState}, + system::{Local, SystemMeta, SystemParam, SystemParamValidationError, SystemState}, world::World, }; use bevy_platform::cell::SyncCell; @@ -27,7 +27,10 @@ pub trait ExclusiveSystemParam: Sized { /// Creates a parameter to be passed into an [`ExclusiveSystemParamFunction`]. /// /// [`ExclusiveSystemParamFunction`]: super::ExclusiveSystemParamFunction - fn get_param<'s>(state: &'s mut Self::State, system_meta: &SystemMeta) -> Self::Item<'s>; + fn get_param<'s>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + ) -> Result, SystemParamValidationError>; } /// Shorthand way of accessing the associated type [`ExclusiveSystemParam::Item`] @@ -44,8 +47,11 @@ impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> ExclusiveSystemParam QueryState::new(world) } - fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> { - state + fn get_param<'s>( + state: &'s mut Self::State, + _system_meta: &SystemMeta, + ) -> Result, SystemParamValidationError> { + Ok(state) } } @@ -57,8 +63,11 @@ impl<'a, P: SystemParam + 'static> ExclusiveSystemParam for &'a mut SystemState< SystemState::new(world) } - fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> { - state + fn get_param<'s>( + state: &'s mut Self::State, + _system_meta: &SystemMeta, + ) -> Result, SystemParamValidationError> { + Ok(state) } } @@ -70,8 +79,11 @@ impl<'_s, T: FromWorld + Send + 'static> ExclusiveSystemParam for Local<'_s, T> SyncCell::new(T::from_world(world)) } - fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> { - Local(state.get()) + fn get_param<'s>( + state: &'s mut Self::State, + _system_meta: &SystemMeta, + ) -> Result, SystemParamValidationError> { + Ok(Local(state.get())) } } @@ -81,8 +93,11 @@ impl ExclusiveSystemParam for PhantomData { fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {} - fn get_param<'s>(_state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> { - PhantomData + fn get_param<'s>( + _state: &'s mut Self::State, + _system_meta: &SystemMeta, + ) -> Result, SystemParamValidationError> { + Ok(PhantomData) } } @@ -115,13 +130,13 @@ macro_rules! impl_exclusive_system_param_tuple { fn get_param<'s>( state: &'s mut Self::State, system_meta: &SystemMeta, - ) -> Self::Item<'s> { + ) -> Result, SystemParamValidationError> { let ($($param,)*) = state; #[allow( clippy::unused_unit, reason = "Zero-length tuples won't have any params to get." )] - ($($param::get_param($param, system_meta),)*) + Ok(($($param::get_param($param, system_meta)?,)*)) } } }; diff --git a/crates/bevy_ecs/src/system/system_name.rs b/crates/bevy_ecs/src/system/system_name.rs index 9321cc6968bd6..ea74ff1558931 100644 --- a/crates/bevy_ecs/src/system/system_name.rs +++ b/crates/bevy_ecs/src/system/system_name.rs @@ -82,8 +82,11 @@ impl ExclusiveSystemParam for SystemName { fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {} - fn get_param<'s>(_state: &'s mut Self::State, system_meta: &SystemMeta) -> Self::Item<'s> { - SystemName(system_meta.name.clone()) + fn get_param<'s>( + _state: &'s mut Self::State, + system_meta: &SystemMeta, + ) -> Result, SystemParamValidationError> { + Ok(SystemName(system_meta.name.clone())) } } diff --git a/crates/bevy_ecs/src/world/identifier.rs b/crates/bevy_ecs/src/world/identifier.rs index 29cdde925362e..ee1b815d32b61 100644 --- a/crates/bevy_ecs/src/world/identifier.rs +++ b/crates/bevy_ecs/src/world/identifier.rs @@ -86,8 +86,8 @@ impl ExclusiveSystemParam for WorldId { world.id() } - fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> { - *state + fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Result, SystemParamValidationError> { + Ok(*state) } } diff --git a/release-content/migration-guides/validation_merging.md b/release-content/migration-guides/validation_merging.md index fba642b07db42..637abf0fe2fcf 100644 --- a/release-content/migration-guides/validation_merging.md +++ b/release-content/migration-guides/validation_merging.md @@ -84,6 +84,35 @@ unsafe impl SystemParam for MyParam<'_> { } ``` +### Custom `ExclusiveSystemParam` implementations + +Similarly, `ExclusiveSystemParam::get_param` now returns a `Result, SystemParamValidationError>` instead of `Self::Item<'s>`. +Existing implementations should wrap their return value in `Ok(...)` and return an appropriate `SystemParamValidationError` if validation fails. + +```rust +// Before +impl ExclusiveSystemParam for MyExclusiveParam { + // ... + fn get_param<'s>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + ) -> Self::Item<'s> { + MyExclusiveParam { /* ... */ } + } +} + +// After +impl ExclusiveSystemParam for MyExclusiveParam { + // ... + fn get_param<'s>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + ) -> Result, SystemParamValidationError> { + Ok(MyExclusiveParam { /* ... */ }) + } +} +``` + ### Custom `System` implementations If you have a custom `System` implementation, remove the `validate_param_unsafe` method. Parameter validation should now occur inside `run_unsafe` by propagating errors from `SystemParam::get_param`. From 026c974d694ed9ae2a7ca3679b37bfd76104705c Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 4 Mar 2026 13:49:43 -0800 Subject: [PATCH 07/23] Cargo fmt --- crates/bevy_ecs/src/message/message_reader.rs | 9 +-------- crates/bevy_ecs/src/observer/runner.rs | 4 +--- crates/bevy_ecs/src/schedule/executor/multi_threaded.rs | 4 +--- crates/bevy_ecs/src/schedule/node.rs | 5 +---- crates/bevy_ecs/src/world/identifier.rs | 5 ++++- crates/bevy_render/src/renderer/render_context.rs | 9 +++------ crates/bevy_winit/src/state.rs | 5 ++++- 7 files changed, 15 insertions(+), 26 deletions(-) diff --git a/crates/bevy_ecs/src/message/message_reader.rs b/crates/bevy_ecs/src/message/message_reader.rs index b8d5ebc16f895..110ffb9f96116 100644 --- a/crates/bevy_ecs/src/message/message_reader.rs +++ b/crates/bevy_ecs/src/message/message_reader.rs @@ -164,14 +164,7 @@ unsafe impl<'w, 's, M: Message> SystemParam for PopulatedMessageReader<'w, 's, M change_tick: crate::change_detection::Tick, ) -> Result, SystemParamValidationError> { // SAFETY: requirements are upheld by MessageReader's implementation - let reader = unsafe { - MessageReader::get_param( - state, - system_meta, - world, - change_tick, - )? - }; + let reader = unsafe { MessageReader::get_param(state, system_meta, world, change_tick)? }; if reader.is_empty() { Err(SystemParamValidationError::skipped::( "message queue is empty", diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index 96b7f23e78114..9180a377edc4b 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -107,9 +107,7 @@ pub(super) unsafe fn observer_system_runner(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Result, SystemParamValidationError> { + fn get_param<'s>( + state: &'s mut Self::State, + _system_meta: &SystemMeta, + ) -> Result, SystemParamValidationError> { Ok(*state) } } diff --git a/crates/bevy_render/src/renderer/render_context.rs b/crates/bevy_render/src/renderer/render_context.rs index 954a66b4b23ee..0fa4ac1eb67f0 100644 --- a/crates/bevy_render/src/renderer/render_context.rs +++ b/crates/bevy_render/src/renderer/render_context.rs @@ -279,12 +279,9 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam // SAFETY: Query state access is properly registered in init_access. // The caller ensures the world matches the one used in init_state. - let item = unsafe { state.query_state.get_unchecked(world, entity) } - .map_err(|_| { - SystemParamValidationError::skipped::( - "Current view entity does not match query", - ) - })?; + let item = unsafe { state.query_state.get_unchecked(world, entity) }.map_err(|_| { + SystemParamValidationError::skipped::("Current view entity does not match query") + })?; Ok(ViewQuery { entity, diff --git a/crates/bevy_winit/src/state.rs b/crates/bevy_winit/src/state.rs index afc3d7573729a..1a8fe5e30e854 100644 --- a/crates/bevy_winit/src/state.rs +++ b/crates/bevy_winit/src/state.rs @@ -460,7 +460,10 @@ impl ApplicationHandler for WinitAppRunnerState { fn about_to_wait(&mut self, event_loop: &ActiveEventLoop) { let mut create_monitor = SystemState::::from_world(self.world_mut()); - create_monitors(event_loop, create_monitor.get_mut(self.world_mut()).unwrap()); + create_monitors( + event_loop, + create_monitor.get_mut(self.world_mut()).unwrap(), + ); create_monitor.apply(self.world_mut()); // TODO: This is a workaround for https://github.com/bevyengine/bevy/issues/17488 From fbff8d2c8513c4a45ab2b91db6a99750b7ffcd9f Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 4 Mar 2026 13:55:10 -0800 Subject: [PATCH 08/23] Note panic in `ParamSet::get_mut` --- crates/bevy_ecs/src/system/system_param.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index dc9fea08e4a84..757561733a786 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1917,6 +1917,12 @@ unsafe impl SystemParam for ParamSet<'_, '_, Vec> { impl ParamSet<'_, '_, Vec> { /// Accesses the parameter at the given index. /// No other parameters may be accessed while this one is active. + /// + /// # Panics + /// + /// If system parameter validation fails, a panic will occur. + /// This most commonly occurs when the world is not in the expected state, + /// such as a resource that was removed or never initialized. pub fn get_mut(&mut self, index: usize) -> T::Item<'_, '_> { // SAFETY: // - We initialized the access for each parameter, so the caller ensures we have access to any world data needed by any param. @@ -1929,7 +1935,7 @@ impl ParamSet<'_, '_, Vec> { self.world, self.change_tick, ) - .expect("ParamSet parameter validation failed") + .unwrap() } } From e604357472633637dc4d1af8d5856b24a6c0d0a1 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 4 Mar 2026 13:59:25 -0800 Subject: [PATCH 09/23] Improve migration guide for Multithreaded executor --- release-content/migration-guides/validation_merging.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/release-content/migration-guides/validation_merging.md b/release-content/migration-guides/validation_merging.md index 637abf0fe2fcf..c2c536a25d548 100644 --- a/release-content/migration-guides/validation_merging.md +++ b/release-content/migration-guides/validation_merging.md @@ -117,9 +117,13 @@ impl ExclusiveSystemParam for MyExclusiveParam { If you have a custom `System` implementation, remove the `validate_param_unsafe` method. Parameter validation should now occur inside `run_unsafe` by propagating errors from `SystemParam::get_param`. -### `MultithreadedExecutor` behavior change +### `MultithreadedExecutor` performance changes -For the parallel `MultithreadedExecutor`, validation was previously done while checking run conditions. +For the parallel `MultithreadedExecutor`, validation was previously done as a cheap pre-validation step, +while checking run conditions. Now, tasks will be spawned for systems which would fail or are skipped during validation. + +In most cases, avoiding the extra overhead of looking up the required data twice should dominate. +However, this change may negatively affect systems which are frequently skipped (e.g. due to `Single`). If you find that this is a significant performance overhead for your use case, the previous behavior can be recovered by adding run conditions. From 0a280d63c004de34cbb3c1a58d8131a85213c821 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 4 Mar 2026 15:09:22 -0800 Subject: [PATCH 10/23] h2 headers in migration guide --- release-content/migration-guides/validation_merging.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/release-content/migration-guides/validation_merging.md b/release-content/migration-guides/validation_merging.md index c2c536a25d548..f26e768737c4a 100644 --- a/release-content/migration-guides/validation_merging.md +++ b/release-content/migration-guides/validation_merging.md @@ -30,7 +30,7 @@ Instead, validation has been moved to be part of the trait implementation for `S All implementations of the `System` trait should validate that their parameters are valid during this method, bubbling up any errors originating in `SystemParam::get_param`. -### Custom `SystemParam` implementations +## Custom `SystemParam` implementations If you have a custom `SystemParam` implementation, you need to: @@ -84,7 +84,7 @@ unsafe impl SystemParam for MyParam<'_> { } ``` -### Custom `ExclusiveSystemParam` implementations +## Custom `ExclusiveSystemParam` implementations Similarly, `ExclusiveSystemParam::get_param` now returns a `Result, SystemParamValidationError>` instead of `Self::Item<'s>`. Existing implementations should wrap their return value in `Ok(...)` and return an appropriate `SystemParamValidationError` if validation fails. @@ -113,11 +113,11 @@ impl ExclusiveSystemParam for MyExclusiveParam { } ``` -### Custom `System` implementations +## Custom `System` implementations If you have a custom `System` implementation, remove the `validate_param_unsafe` method. Parameter validation should now occur inside `run_unsafe` by propagating errors from `SystemParam::get_param`. -### `MultithreadedExecutor` performance changes +## `MultithreadedExecutor` performance changes For the parallel `MultithreadedExecutor`, validation was previously done as a cheap pre-validation step, while checking run conditions. From b2782ddee5011595ffff6c717bea283437f76c16 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 4 Mar 2026 15:10:23 -0800 Subject: [PATCH 11/23] Clippy --- crates/bevy_ecs/src/system/combinator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/combinator.rs b/crates/bevy_ecs/src/system/combinator.rs index 5efe51a025421..69d1b563ad232 100644 --- a/crates/bevy_ecs/src/system/combinator.rs +++ b/crates/bevy_ecs/src/system/combinator.rs @@ -168,7 +168,7 @@ where world: &mut PrivateUnsafeWorldCell, ) -> Result { // SAFETY: see comment on `Func::combine` call - match (|| unsafe { system.run_unsafe(input, world.0) })() { + match unsafe { system.run_unsafe(input, world.0) } { // let the world's default error handler handle the error if `Failed(_)` Err(RunSystemError::Failed(err)) => { // SAFETY: We registered access to DefaultErrorHandler in `initialize`. From eeab64a80071d314674c55889a0b2e21bee54e74 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 4 Mar 2026 15:12:44 -0800 Subject: [PATCH 12/23] Fill in PR number --- release-content/migration-guides/validation_merging.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release-content/migration-guides/validation_merging.md b/release-content/migration-guides/validation_merging.md index f26e768737c4a..ad4cfa8099002 100644 --- a/release-content/migration-guides/validation_merging.md +++ b/release-content/migration-guides/validation_merging.md @@ -1,6 +1,6 @@ --- title: "`SystemParam` validation is now done when fetching the data" -pull_requests: [TODO] +pull_requests: [23225] --- In an effort to improve performance by reducing redundant data fetches and simplify internals, From dddb08e8ed97c3269c97898c5011944bbfd1bc21 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 4 Mar 2026 15:13:00 -0800 Subject: [PATCH 13/23] Clippy --- crates/bevy_pbr/src/meshlet/instance_manager.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_pbr/src/meshlet/instance_manager.rs b/crates/bevy_pbr/src/meshlet/instance_manager.rs index fe5a7067245ff..9ae2b8237ac0a 100644 --- a/crates/bevy_pbr/src/meshlet/instance_manager.rs +++ b/crates/bevy_pbr/src/meshlet/instance_manager.rs @@ -228,7 +228,7 @@ pub fn extract_meshlet_mesh_entities( // Free GPU buffer space for any modified or dropped MeshletMesh assets for asset_event in asset_events.read() { if let AssetEvent::Unused { id } | AssetEvent::Modified { id } = asset_event { - meshlet_mesh_manager.remove(&id); + meshlet_mesh_manager.remove(id); } } From 3c9560f15c14a55222a1a6c90d5100b8063feaf3 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 4 Mar 2026 15:30:38 -0800 Subject: [PATCH 14/23] Fix failing tests --- .../compile_fail/tests/ui/query_lens_lifetime_safety.rs | 2 +- .../bevy_ecs/compile_fail/tests/ui/query_lifetime_safety.rs | 2 +- .../bevy_ecs/compile_fail/tests/ui/query_transmute_safety.rs | 2 +- .../compile_fail/tests/ui/system_state_get_lifetime_safety.rs | 4 ++-- .../tests/ui/system_state_iter_lifetime_safety.rs | 4 ++-- .../tests/ui/system_state_iter_mut_overlap_safety.rs | 2 +- crates/bevy_ecs/src/system/system.rs | 2 +- crates/bevy_ecs/src/system/system_registry.rs | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ecs/compile_fail/tests/ui/query_lens_lifetime_safety.rs b/crates/bevy_ecs/compile_fail/tests/ui/query_lens_lifetime_safety.rs index d3cdb0607842b..31a4f6713ea22 100644 --- a/crates/bevy_ecs/compile_fail/tests/ui/query_lens_lifetime_safety.rs +++ b/crates/bevy_ecs/compile_fail/tests/ui/query_lens_lifetime_safety.rs @@ -13,7 +13,7 @@ fn main() { let mut system_state = SystemState::<(Query<&mut Foo>, Query<&mut Bar>)>::new(&mut world); { - let (mut foo_query, mut bar_query) = system_state.get_mut(&mut world); + let (mut foo_query, mut bar_query) = system_state.get_mut(&mut world).unwrap(); dbg!("hi"); { let mut lens = foo_query.as_query_lens(); diff --git a/crates/bevy_ecs/compile_fail/tests/ui/query_lifetime_safety.rs b/crates/bevy_ecs/compile_fail/tests/ui/query_lifetime_safety.rs index 62d76bec1695b..7dafff0d6c3a6 100644 --- a/crates/bevy_ecs/compile_fail/tests/ui/query_lifetime_safety.rs +++ b/crates/bevy_ecs/compile_fail/tests/ui/query_lifetime_safety.rs @@ -10,7 +10,7 @@ fn main() { let mut system_state = SystemState::>::new(&mut world); { - let mut query = system_state.get_mut(&mut world); + let mut query = system_state.get_mut(&mut world).unwrap(); dbg!("hi"); { let data: &Foo = query.get(e).unwrap(); diff --git a/crates/bevy_ecs/compile_fail/tests/ui/query_transmute_safety.rs b/crates/bevy_ecs/compile_fail/tests/ui/query_transmute_safety.rs index 7518511feee26..97b2e1aa21266 100644 --- a/crates/bevy_ecs/compile_fail/tests/ui/query_transmute_safety.rs +++ b/crates/bevy_ecs/compile_fail/tests/ui/query_transmute_safety.rs @@ -12,7 +12,7 @@ fn main() { world.spawn(Foo(10)); let mut system_state = SystemState::>::new(&mut world); - let mut query = system_state.get_mut(&mut world); + let mut query = system_state.get_mut(&mut world).unwrap(); { let mut lens_a = query.transmute_lens::<&mut Foo>(); diff --git a/crates/bevy_ecs/compile_fail/tests/ui/system_state_get_lifetime_safety.rs b/crates/bevy_ecs/compile_fail/tests/ui/system_state_get_lifetime_safety.rs index e1912372b61fd..82680d44acbcf 100644 --- a/crates/bevy_ecs/compile_fail/tests/ui/system_state_get_lifetime_safety.rs +++ b/crates/bevy_ecs/compile_fail/tests/ui/system_state_get_lifetime_safety.rs @@ -14,10 +14,10 @@ struct State { impl State { fn get_component(&mut self, world: &mut World, entity: Entity) { - let q1 = self.state_r.get(&world); + let q1 = self.state_r.get(&world).unwrap(); let a1 = q1.get(entity).unwrap(); - let mut q2 = self.state_w.get_mut(world); + let mut q2 = self.state_w.get_mut(world).unwrap(); //~^ E0502 let _ = q2.get_mut(entity).unwrap(); diff --git a/crates/bevy_ecs/compile_fail/tests/ui/system_state_iter_lifetime_safety.rs b/crates/bevy_ecs/compile_fail/tests/ui/system_state_iter_lifetime_safety.rs index 88325b2a5f2db..6582cf63d0ff3 100644 --- a/crates/bevy_ecs/compile_fail/tests/ui/system_state_iter_lifetime_safety.rs +++ b/crates/bevy_ecs/compile_fail/tests/ui/system_state_iter_lifetime_safety.rs @@ -14,10 +14,10 @@ struct State { impl State { fn get_components(&mut self, world: &mut World) { - let q1 = self.state_r.get(&world); + let q1 = self.state_r.get(&world).unwrap(); let a1 = q1.iter().next().unwrap(); - let mut q2 = self.state_w.get_mut(world); + let mut q2 = self.state_w.get_mut(world).unwrap(); //~^ E0502 let _ = q2.iter_mut().next().unwrap(); diff --git a/crates/bevy_ecs/compile_fail/tests/ui/system_state_iter_mut_overlap_safety.rs b/crates/bevy_ecs/compile_fail/tests/ui/system_state_iter_mut_overlap_safety.rs index 3c767b91be919..bae4d55d9c49c 100644 --- a/crates/bevy_ecs/compile_fail/tests/ui/system_state_iter_mut_overlap_safety.rs +++ b/crates/bevy_ecs/compile_fail/tests/ui/system_state_iter_mut_overlap_safety.rs @@ -11,7 +11,7 @@ fn main() { let mut system_state = SystemState::>::new(&mut world); { - let mut query = system_state.get_mut(&mut world); + let mut query = system_state.get_mut(&mut world).unwrap(); let mut_vec = query.iter_mut().collect::>>(); assert_eq!( // this should fail to compile due to the later use of mut_vec diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index cf48d0d9c8843..09efc802f96f9 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -496,7 +496,7 @@ mod tests { assert!(matches!(result, Err(RunSystemError::Failed { .. }))); - let expected = "Resource does not exist"; + let expected = "does not exist"; let actual = result.unwrap_err().to_string(); assert!( diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index 3a097dd592c42..819f78c8c9c44 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -1015,7 +1015,7 @@ mod tests { let result = world.run_system(id); assert!(matches!(result, Err(RegisteredSystemError::Failed { .. }))); - let expected = "Resource does not exist"; + let expected = "does not exist"; let actual = result.unwrap_err().to_string(); assert!( From 40ef199e5e70200db2c565377abe79a52a9c7b3e Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 4 Mar 2026 22:20:59 -0500 Subject: [PATCH 15/23] Revert incidental behavior change to `Single` --- crates/bevy_ecs/src/system/system_param.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 757561733a786..2d17ecca8a1e7 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -375,7 +375,7 @@ unsafe impl<'a, 'b, D: IterQueryData + 'static, F: QueryFilter + 'static> System SystemParamValidationError::skipped::("No matching entities"), ), Err(QuerySingleError::MultipleEntities(_)) => Err( - SystemParamValidationError::invalid::("Multiple matching entities"), + SystemParamValidationError::skipped::("Multiple matching entities"), ), } } From 8bcbf40cd73ac269bb717b436771ab657950c78a Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 5 Mar 2026 10:39:59 -0800 Subject: [PATCH 16/23] Add pipe_system_validation_skip_in_second test --- crates/bevy_ecs/src/schedule/executor/mod.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index d10fb30908076..10cfc121ab1ed 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -705,6 +705,22 @@ mod validation_tests { ); } + #[test] + fn pipe_system_validation_skip_in_second() { + fn first() -> u32 { + 42 + } + + fn second(_input: In, _single: Single<&TestComponent>) {} + + let mut world = World::new(); + let result = world.run_system_once(first.pipe(second)); + assert!( + matches!(result, Err(RunSystemError::Skipped(_))), + "Expected Skipped from pipe second system, got {result:?}" + ); + } + #[test] fn builder_system_validation_failure() { fn system(_res: Res) {} From fdaaeb508055cb77c20c30396f55d100c6db01bd Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 5 Mar 2026 10:43:53 -0800 Subject: [PATCH 17/23] Refactor gizmos::get_config --- crates/bevy_gizmos/src/gizmos.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/crates/bevy_gizmos/src/gizmos.rs b/crates/bevy_gizmos/src/gizmos.rs index 605b52601d43a..02f13609e3470 100644 --- a/crates/bevy_gizmos/src/gizmos.rs +++ b/crates/bevy_gizmos/src/gizmos.rs @@ -245,18 +245,17 @@ where )? }; - // This if-block is to accommodate an Option SystemParam. - // The user may decide not to initialize a gizmo group, so its config will not exist. - if f1.get_config::().is_none() { - return Err(SystemParamValidationError::invalid::( - format!("Requested config {} does not exist in `GizmoConfigStore`! Did you forget to add it using `app.init_gizmo_group()`?", - Config::type_path()))); - } - // Accessing the GizmoConfigStore in every API call reduces performance significantly. // Implementing SystemParam manually allows us to cache whether the config is currently enabled. // Having this available allows for cheap early returns when gizmos are disabled. - let (config, config_ext) = f1.into_inner().config::(); + // + // We use `get_config` instead of `config` to accommodate `Option`: + // the user may decide not to initialize a gizmo group, so its config will not exist. + let (config, config_ext) = f1.into_inner().get_config::().ok_or_else(|| { + SystemParamValidationError::invalid::( + format!("Requested config {} does not exist in `GizmoConfigStore`! Did you forget to add it using `app.init_gizmo_group()`?", + Config::type_path())) + })?; f0.enabled = config.enabled; Ok(Gizmos { From d151037250bd35148453d2b87b3431e47aa070fc Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 5 Mar 2026 10:50:03 -0800 Subject: [PATCH 18/23] Don't format twice for no reason --- crates/bevy_ecs/src/system/system_param.rs | 34 ++++++---------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 2d17ecca8a1e7..5d52eee2fd78a 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -21,7 +21,7 @@ use crate::{ FromWorld, World, }, }; -use alloc::{borrow::Cow, boxed::Box, format, vec::Vec}; +use alloc::{borrow::Cow, boxed::Box, vec::Vec}; pub use bevy_ecs_macros::SystemParam; use bevy_platform::cell::SyncCell; use bevy_ptr::UnsafeCellDeref; @@ -694,13 +694,9 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Result, SystemParamValidationError> { - let (ptr, ticks) = world.get_resource_with_ticks(component_id).ok_or_else(|| { - SystemParamValidationError::invalid::(format!( - "Resource requested by {} does not exist: {}", - system_meta.name, - DebugName::type_name::() - )) - })?; + let (ptr, ticks) = world + .get_resource_with_ticks(component_id) + .ok_or_else(|| SystemParamValidationError::invalid::("Resource not found"))?; Ok(Res { value: ptr.deref(), ticks: ComponentTicksRef { @@ -755,13 +751,9 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Result, SystemParamValidationError> { - let value = world.get_resource_mut_by_id(component_id).ok_or_else(|| { - SystemParamValidationError::invalid::(format!( - "Resource requested by {} does not exist: {}", - system_meta.name, - DebugName::type_name::() - )) - })?; + let value = world + .get_resource_mut_by_id(component_id) + .ok_or_else(|| SystemParamValidationError::invalid::("Resource not found"))?; Ok(ResMut { value: value.value.deref_mut::(), ticks: ComponentTicksMut { @@ -1348,11 +1340,7 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> { change_tick: Tick, ) -> Result, SystemParamValidationError> { let (ptr, ticks) = world.get_non_send_with_ticks(component_id).ok_or_else(|| { - SystemParamValidationError::invalid::(format!( - "Non-send resource requested by {} does not exist: {}", - system_meta.name, - DebugName::type_name::() - )) + SystemParamValidationError::invalid::("Non-send data not found") })?; Ok(NonSend { value: ptr.deref(), @@ -1400,11 +1388,7 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> { change_tick: Tick, ) -> Result, SystemParamValidationError> { let (ptr, ticks) = world.get_non_send_with_ticks(component_id).ok_or_else(|| { - SystemParamValidationError::invalid::(format!( - "Non-send resource requested by {} does not exist: {}", - system_meta.name, - DebugName::type_name::() - )) + SystemParamValidationError::invalid::("Non-send data not found") })?; Ok(NonSendMut { value: ptr.assert_unique().deref_mut(), From 69cb5c04e97cc602c0febba0b5da8c912efef1b7 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 5 Mar 2026 18:29:41 -0800 Subject: [PATCH 19/23] Validate `ParamSet` sub-params eagerly, and skip if any fail --- crates/bevy_ecs/src/schedule/executor/mod.rs | 50 ++++++++++++++++++-- crates/bevy_ecs/src/system/system_param.rs | 20 ++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index 10cfc121ab1ed..551bb0fae30ff 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -559,9 +559,9 @@ mod validation_tests { prelude::{Component, In, IntoSystem, Resource, Schedule}, schedule::ExecutorKind, system::{ - DynParamBuilder, DynSystemParam, ExclusiveSystemParam, Local, ParamBuilder, Res, - ResMut, RunSystemError, RunSystemOnce, Single, SystemMeta, SystemParamBuilder, - SystemParamValidationError, + DynParamBuilder, DynSystemParam, ExclusiveSystemParam, Local, ParamBuilder, ParamSet, + Query, Res, ResMut, RunSystemError, RunSystemOnce, Single, SystemMeta, + SystemParamBuilder, SystemParamValidationError, }, world::World, }; @@ -834,4 +834,48 @@ mod validation_tests { ); } } + + #[test] + fn param_set_validation_skip() { + // A system using ParamSet with a Single sub-param should be skipped + // when the Single has no matching entities, rather than panicking. + fn system(mut _set: ParamSet<(Single<&TestComponent>,)>) {} + + let mut world = World::new(); + let result = world.run_system_once(system); + assert!( + matches!(result, Err(RunSystemError::Skipped(_))), + "Expected Skipped from ParamSet with invalid Single, got {result:?}" + ); + } + + #[test] + fn param_set_validation_failure() { + // A system using ParamSet with a Res sub-param should fail validation + // when the resource does not exist. + fn system(mut _set: ParamSet<(Query<&TestComponent>, Res)>) {} + + let mut world = World::new(); + let result = world.run_system_once(system); + assert!( + matches!(result, Err(RunSystemError::Failed(_))), + "Expected Failed from ParamSet with missing resource, got {result:?}" + ); + } + + #[test] + fn param_set_validation_success() { + // A system using ParamSet with valid sub-params should succeed. + fn system(mut set: ParamSet<(Query<&TestComponent>, Res)>) { + let _q = set.p0(); + } + + let mut world = World::new(); + world.init_resource::(); + let result = world.run_system_once(system); + assert!( + result.is_ok(), + "Expected Ok from ParamSet with valid params, got {result:?}" + ); + } } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 5d52eee2fd78a..1dbb4b66259e4 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -618,6 +618,16 @@ macro_rules! impl_param_set { world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Result, SystemParamValidationError> { + // Validate each sub-param eagerly so that the system is correctly + // skipped by the executor when any sub-param is unavailable. + // PERF: the sub-params will be fetched again lazily when accessed through + // the ParamSet, but this is no worse than the previous + // validate_param + get_param pattern. + $( + // SAFETY: Upheld by caller. + drop(unsafe { $param::get_param(&mut state.$index, system_meta, world, change_tick) }?); + )* + Ok(ParamSet { param_states: state, system_meta: system_meta.clone(), @@ -1877,6 +1887,16 @@ unsafe impl SystemParam for ParamSet<'_, '_, Vec> { world: UnsafeWorldCell<'world>, change_tick: Tick, ) -> Result, SystemParamValidationError> { + // Validate each sub-param eagerly so that the system is correctly + // skipped by the executor when any sub-param is unavailable. + // PERF: the sub-params will be fetched again lazily when accessed through + // the ParamSet, but this is no worse than the previous + // validate_param + get_param pattern. + for s in state.iter_mut() { + // SAFETY: Upheld by caller. + drop(unsafe { T::get_param(s, system_meta, world, change_tick) }?); + } + Ok(ParamSet { param_states: state, system_meta: system_meta.clone(), From 4958e801a91b23afb7ab7e3fc929f97235db075b Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 9 Mar 2026 15:03:31 -0400 Subject: [PATCH 20/23] Remove panics comment Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com> --- crates/bevy_ecs/src/system/system_param.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 1dbb4b66259e4..bbf2200527c58 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1921,12 +1921,6 @@ unsafe impl SystemParam for ParamSet<'_, '_, Vec> { impl ParamSet<'_, '_, Vec> { /// Accesses the parameter at the given index. /// No other parameters may be accessed while this one is active. - /// - /// # Panics - /// - /// If system parameter validation fails, a panic will occur. - /// This most commonly occurs when the world is not in the expected state, - /// such as a resource that was removed or never initialized. pub fn get_mut(&mut self, index: usize) -> T::Item<'_, '_> { // SAFETY: // - We initialized the access for each parameter, so the caller ensures we have access to any world data needed by any param. From 90ac6a1fdff10268237c5847b59b2d0611a909d2 Mon Sep 17 00:00:00 2001 From: Alice I Cecile Date: Mon, 9 Mar 2026 18:43:29 -0700 Subject: [PATCH 21/23] Fix failing test by making sure error message does not change --- crates/bevy_ecs/src/system/system.rs | 2 +- crates/bevy_ecs/src/system/system_param.rs | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index 09efc802f96f9..cf48d0d9c8843 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -496,7 +496,7 @@ mod tests { assert!(matches!(result, Err(RunSystemError::Failed { .. }))); - let expected = "does not exist"; + let expected = "Resource does not exist"; let actual = result.unwrap_err().to_string(); assert!( diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index bbf2200527c58..7b0c330970ace 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -704,9 +704,9 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Result, SystemParamValidationError> { - let (ptr, ticks) = world - .get_resource_with_ticks(component_id) - .ok_or_else(|| SystemParamValidationError::invalid::("Resource not found"))?; + let (ptr, ticks) = world.get_resource_with_ticks(component_id).ok_or_else(|| { + SystemParamValidationError::invalid::("Resource does not exist") + })?; Ok(Res { value: ptr.deref(), ticks: ComponentTicksRef { @@ -761,9 +761,9 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Result, SystemParamValidationError> { - let value = world - .get_resource_mut_by_id(component_id) - .ok_or_else(|| SystemParamValidationError::invalid::("Resource not found"))?; + let value = world.get_resource_mut_by_id(component_id).ok_or_else(|| { + SystemParamValidationError::invalid::("Resource does not exist") + })?; Ok(ResMut { value: value.value.deref_mut::(), ticks: ComponentTicksMut { From 5c5674fbab032b5bfb57b199909558c25ecbc422 Mon Sep 17 00:00:00 2001 From: Alice I Cecile Date: Mon, 9 Mar 2026 18:44:56 -0700 Subject: [PATCH 22/23] Update docs on get_param to match chescock's suggestion --- crates/bevy_ecs/src/system/system_param.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 7b0c330970ace..53a7c56b98e16 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -260,8 +260,9 @@ pub unsafe trait SystemParam: Sized { /// /// This method also validates that the param can be acquired. If validation fails, /// an appropriate [`SystemParamValidationError`] should be returned. - /// 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`]. + /// Systems will convert this to a [`RunSystemError`](super::RunSystemError), + /// and the built-in executors will ignore any "skipped" validation results, + /// but pass any "invalid" results to the default error handler defined in [`bevy_ecs::error`]. /// /// For nested [`SystemParam`]s validation will fail if any /// delegated validation fails. From 34fc43b8e9c2a2ab0a77f06d76de95491b12be88 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 11 Mar 2026 14:33:27 -0400 Subject: [PATCH 23/23] Fix missing unwrap... Co-authored-by: Kevin Chen --- crates/bevy_winit/src/state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_winit/src/state.rs b/crates/bevy_winit/src/state.rs index 1a8fe5e30e854..4af560e16045a 100644 --- a/crates/bevy_winit/src/state.rs +++ b/crates/bevy_winit/src/state.rs @@ -576,7 +576,7 @@ impl WinitAppRunnerState { SystemState::::from_world(self.world_mut()); let (.., mut handlers, accessibility_requested, monitors) = - create_window.get_mut(self.world_mut()); + create_window.get_mut(self.world_mut()).unwrap(); let winit_window = winit_windows.create_window( event_loop,