From bc54cd68da76cb22f10f2236d62f1a77b302c11d Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Thu, 5 Mar 2026 15:43:49 -0500 Subject: [PATCH 1/7] Introduce `InfallibleSystemParam` trait. --- benches/benches/bevy_ecs/fragmentation.rs | 4 +- benches/benches/bevy_ecs/world/world_get.rs | 8 +- crates/bevy_audio/src/audio_output.rs | 7 +- .../tests/ui/query_lens_lifetime_safety.rs | 2 +- .../tests/ui/query_lifetime_safety.rs | 2 +- .../tests/ui/query_transmute_safety.rs | 2 +- .../ui/system_state_get_lifetime_safety.rs | 4 +- .../ui/system_state_iter_lifetime_safety.rs | 4 +- .../system_state_iter_mut_overlap_safety.rs | 2 +- crates/bevy_ecs/macros/src/lib.rs | 65 +- crates/bevy_ecs/src/component/mod.rs | 4 +- crates/bevy_ecs/src/lifecycle.rs | 24 +- crates/bevy_ecs/src/message/message_reader.rs | 5 +- crates/bevy_ecs/src/query/mod.rs | 2 +- .../bevy_ecs/src/reflect/entity_commands.rs | 16 +- crates/bevy_ecs/src/schedule/executor/mod.rs | 48 +- crates/bevy_ecs/src/schedule/mod.rs | 2 +- crates/bevy_ecs/src/system/builder.rs | 2 +- crates/bevy_ecs/src/system/commands/mod.rs | 31 +- .../src/system/commands/parallel_scope.rs | 4 +- .../src/system/exclusive_function_system.rs | 2 +- .../src/system/exclusive_system_param.rs | 14 +- crates/bevy_ecs/src/system/function_system.rs | 120 ++- crates/bevy_ecs/src/system/mod.rs | 28 +- crates/bevy_ecs/src/system/system_name.rs | 21 +- crates/bevy_ecs/src/system/system_param.rs | 745 ++++++++++++++---- crates/bevy_ecs/src/world/identifier.rs | 21 +- .../bevy_ecs/src/world/unsafe_world_cell.rs | 2 +- crates/bevy_gizmos/src/gizmos.rs | 4 +- crates/bevy_input_focus/src/lib.rs | 8 +- crates/bevy_input_focus/src/tab_navigation.rs | 8 +- crates/bevy_pbr/src/material.rs | 2 +- .../bevy_pbr/src/meshlet/instance_manager.rs | 2 +- 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 | 6 +- 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 | 2 +- crates/bevy_render/src/sync_world.rs | 2 +- .../bevy_render/src/view/window/screenshot.rs | 3 +- crates/bevy_text/src/text_access.rs | 4 +- crates/bevy_transform/src/helper.rs | 6 +- .../src/experimental/ghost_hierarchy.rs | 15 +- crates/bevy_ui/src/ui_node.rs | 7 +- crates/bevy_ui_render/src/lib.rs | 4 +- crates/bevy_winit/src/cursor/mod.rs | 4 +- crates/bevy_winit/src/state.rs | 22 +- examples/async_tasks/async_compute.rs | 2 +- examples/ecs/fallible_params.rs | 4 +- examples/ecs/send_and_receive_messages.rs | 7 +- .../migration-guides/validation_merging.md | 74 +- 54 files changed, 1061 insertions(+), 328 deletions(-) diff --git a/benches/benches/bevy_ecs/fragmentation.rs b/benches/benches/bevy_ecs/fragmentation.rs index e7a2f4df78520..97864990a37e5 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).unwrap(); + let query = q.get(&world); 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).unwrap(); + let query = q.get(&world); 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 19aa015b91afc..bd7507b3047a0 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).unwrap(); + let query = query.get(&world); 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).unwrap(); + let query = query.get(&world); 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).unwrap(); + let query = query.get(&world); 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).unwrap(); + let query = query.get(&world); bencher.iter(|| { let mut count = 0; diff --git a/crates/bevy_audio/src/audio_output.rs b/crates/bevy_audio/src/audio_output.rs index d02d326501298..6a154a67c1e29 100644 --- a/crates/bevy_audio/src/audio_output.rs +++ b/crates/bevy_audio/src/audio_output.rs @@ -3,7 +3,10 @@ use crate::{ SpatialAudioSink, SpatialListener, }; use bevy_asset::{Asset, Assets}; -use bevy_ecs::{prelude::*, system::SystemParam}; +use bevy_ecs::{ + prelude::*, + system::{InfallibleSystemParam, SystemParam}, +}; use bevy_math::Vec3; use bevy_transform::prelude::GlobalTransform; use rodio::{OutputStream, OutputStreamHandle, Sink, Source, SpatialSink}; @@ -53,7 +56,7 @@ pub struct PlaybackDespawnMarker; #[derive(Component, Default)] pub struct PlaybackRemoveMarker; -#[derive(SystemParam)] +#[derive(SystemParam, InfallibleSystemParam)] pub(crate) struct EarPositions<'w, 's> { pub(crate) query: Query<'w, 's, (Entity, &'static GlobalTransform, &'static SpatialListener)>, } 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 31a4f6713ea22..d3cdb0607842b 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).unwrap(); + let (mut foo_query, mut bar_query) = system_state.get_mut(&mut world); 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 7dafff0d6c3a6..62d76bec1695b 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).unwrap(); + let mut query = system_state.get_mut(&mut world); 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 97b2e1aa21266..7518511feee26 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).unwrap(); + let mut query = system_state.get_mut(&mut world); { 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 82680d44acbcf..e1912372b61fd 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).unwrap(); + let q1 = self.state_r.get(&world); let a1 = q1.get(entity).unwrap(); - let mut q2 = self.state_w.get_mut(world).unwrap(); + let mut q2 = self.state_w.get_mut(world); //~^ 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 6582cf63d0ff3..88325b2a5f2db 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).unwrap(); + let q1 = self.state_r.get(&world); let a1 = q1.iter().next().unwrap(); - let mut q2 = self.state_w.get_mut(world).unwrap(); + let mut q2 = self.state_w.get_mut(world); //~^ 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 bae4d55d9c49c..3c767b91be919 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).unwrap(); + let mut query = system_state.get_mut(&mut world); 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/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 19ecef332d553..5d66c6de37dca 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -452,7 +452,7 @@ fn derive_system_param_impl( } #[inline] - unsafe fn get_param<'w, 's>( + unsafe fn try_get_param<'w, 's>( state: &'s mut Self::State, system_meta: &#path::system::SystemMeta, world: #path::world::unsafe_world_cell::UnsafeWorldCell<'w>, @@ -461,7 +461,7 @@ fn derive_system_param_impl( 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) + <#field_types as #path::system::SystemParam>::try_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 { @@ -480,6 +480,67 @@ fn derive_system_param_impl( })) } +/// Implement `SystemParam` to use a struct as a parameter in a system +#[proc_macro_derive(InfallibleSystemParam)] +pub fn derive_infallible_system_param(input: TokenStream) -> TokenStream { + let ast = parse_macro_input!(input as DeriveInput); + + match derive_infallible_system_param_impl(ast) { + Ok(t) => t, + Err(e) => e.into_compile_error().into(), + } +} +fn derive_infallible_system_param_impl( + ast: DeriveInput, +) -> syn::Result { + let fields = get_struct_fields(&ast.data, "derive(InfallibleSystemParam)")?; + let path = bevy_ecs_path(); + + let field_locals = fields + .members() + .map(|m| format_ident!("field{}", m)) + .collect::>(); + let field_members = fields.members().collect::>(); + let field_types = fields.iter().map(|f| &f.ty).collect::>(); + + let generics = ast.generics; + + let (impl_generics, type_generics, where_clause) = generics.split_for_impl(); + + let mut tuple_patterns: Vec<_> = field_locals.iter().map(ToTokens::to_token_stream).collect(); + + // If the number of fields exceeds the 16-parameter limit, + // fold the fields into tuples of tuples until we are below the limit. + const LIMIT: usize = 16; + while tuple_patterns.len() > LIMIT { + let end = Vec::from_iter(tuple_patterns.drain(..LIMIT)); + tuple_patterns.push(parse_quote!( (#(#end,)*) )); + } + + let struct_name = &ast.ident; + + Ok(TokenStream::from(quote!{ + impl #impl_generics #path::system::InfallibleSystemParam for + #struct_name #type_generics #where_clause + { + #[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,)*) = &mut state.state; + #struct_name { + #(#field_members: unsafe { + <#field_types as #path::system::InfallibleSystemParam>::get_param(#field_locals, system_meta, world, change_tick) + },)* + } + } + } + })) +} + /// Implement `QueryData` to use a struct as a data parameter in a query #[proc_macro_derive(QueryData, attributes(query_data))] pub fn derive_query_data(input: TokenStream) -> TokenStream { diff --git a/crates/bevy_ecs/src/component/mod.rs b/crates/bevy_ecs/src/component/mod.rs index c12dfb06e1524..4d7b22fc33581 100644 --- a/crates/bevy_ecs/src/component/mod.rs +++ b/crates/bevy_ecs/src/component/mod.rs @@ -16,7 +16,7 @@ use crate::{ entity::EntityMapper, lifecycle::ComponentHook, relationship::ComponentRelationshipAccessor, - system::{Local, SystemParam}, + system::{InfallibleSystemParam, Local, SystemParam}, world::{FromWorld, World}, }; pub use bevy_ecs_macros::Component; @@ -748,7 +748,7 @@ pub enum StorageType { /// // ... /// } /// ``` -#[derive(SystemParam)] +#[derive(SystemParam, InfallibleSystemParam)] pub struct ComponentIdFor<'s, T: Component>(Local<'s, InitComponentId>); impl ComponentIdFor<'_, T> { diff --git a/crates/bevy_ecs/src/lifecycle.rs b/crates/bevy_ecs/src/lifecycle.rs index 13685cf9ce933..2f8f7f84bd916 100644 --- a/crates/bevy_ecs/src/lifecycle.rs +++ b/crates/bevy_ecs/src/lifecycle.rs @@ -60,7 +60,10 @@ use crate::{ query::FilteredAccessSet, relationship::RelationshipHookMode, storage::SparseSet, - system::{Local, ReadOnlySystemParam, SystemMeta, SystemParam, SystemParamValidationError}, + system::{ + InfallibleSystemParam, Local, ReadOnlySystemParam, SystemMeta, SystemParam, + SystemParamValidationError, + }, world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World}, }; @@ -506,7 +509,7 @@ impl RemovedComponentMessages { /// } /// # bevy_ecs::system::assert_is_system(react_on_removal); /// ``` -#[derive(SystemParam)] +#[derive(SystemParam, InfallibleSystemParam)] pub struct RemovedComponents<'w, 's, T: Component> { component_id: ComponentIdFor<'s, T>, reader: Local<'s, RemovedComponentReader>, @@ -633,13 +636,26 @@ unsafe impl<'a> SystemParam for &'a RemovedComponentMessages { ) { } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for &RemovedComponentMessages { #[inline] unsafe fn get_param<'w, 's>( _state: &'s mut Self::State, _system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Result, SystemParamValidationError> { - Ok(world.removed_components()) + ) -> Self::Item<'w, 's> { + world.removed_components() } } diff --git a/crates/bevy_ecs/src/message/message_reader.rs b/crates/bevy_ecs/src/message/message_reader.rs index 110ffb9f96116..f23e868155866 100644 --- a/crates/bevy_ecs/src/message/message_reader.rs +++ b/crates/bevy_ecs/src/message/message_reader.rs @@ -157,14 +157,15 @@ unsafe impl<'w, 's, M: Message> SystemParam for PopulatedMessageReader<'w, 's, M MessageReader::::init_access(state, system_meta, component_access_set, world); } - unsafe fn get_param<'world, 'state>( + unsafe fn try_get_param<'world, 'state>( state: &'state mut Self::State, system_meta: &crate::system::SystemMeta, world: crate::world::unsafe_world_cell::UnsafeWorldCell<'world>, 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::try_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/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index 2c95722c7a189..b7b4223c68f41 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -774,7 +774,7 @@ mod tests { // system param let mut q = SystemState::>::new(&mut world); - let q = q.get_mut(&mut world).unwrap(); + let q = q.get_mut(&mut world); 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 c80c464635f25..140c96eba440a 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).unwrap(); + let mut commands = system_state.get_mut(&mut world); 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).unwrap(); + let mut commands = system_state.get_mut(&mut world); 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).unwrap(); + let mut commands = system_state.get_mut(&mut world); 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).unwrap(); + let mut commands = system_state.get_mut(&mut world); 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).unwrap(); + let mut commands = system_state.get_mut(&mut world); 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).unwrap(); + let mut commands = system_state.get_mut(&mut world); 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).unwrap(); + let mut commands = system_state.get_mut(&mut world); 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).unwrap(); + let mut commands = system_state.get_mut(&mut world); 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 551bb0fae30ff..0c4fea3a0e12e 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -588,7 +588,7 @@ mod validation_tests { fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {} - fn get_param<'s>( + fn try_get_param<'s>( _state: &'s mut Self::State, _system_meta: &SystemMeta, ) -> Result, SystemParamValidationError> { @@ -750,8 +750,11 @@ mod validation_tests { 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); + .build_system(|param: DynSystemParam| -> Result<(), RunSystemError> { + param.downcast::>()?; + Ok(()) + }); + let result: Result<(), _> = world.run_system_once(system); assert!( matches!(result, Err(RunSystemError::Failed(_))), "Expected Failed from DynSystemParam system, got {result:?}" @@ -763,8 +766,11 @@ mod validation_tests { 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); + .build_system(|param: DynSystemParam| -> Result<(), RunSystemError> { + param.downcast::>()?; + Ok(()) + }); + let result: Result<(), _> = world.run_system_once(system); assert!( matches!(result, Err(RunSystemError::Skipped(_))), "Expected Skipped from DynSystemParam system, got {result:?}" @@ -777,8 +783,11 @@ mod validation_tests { world.init_resource::(); let system = (DynParamBuilder::new::>(ParamBuilder),) .build_state(&mut world) - .build_system(|_param: DynSystemParam| {}); - let result = world.run_system_once(system); + .build_system(|param: DynSystemParam| -> Result<(), RunSystemError> { + param.downcast::>()?; + Ok(()) + }); + let result: Result<(), _> = world.run_system_once(system); assert!( result.is_ok(), "Expected Ok from DynSystemParam system, got {result:?}" @@ -839,10 +848,13 @@ mod validation_tests { 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>,)>) {} + fn system(mut set: ParamSet<(Single<&TestComponent>,)>) -> Result<(), RunSystemError> { + set.try_p0()?; + Ok(()) + } let mut world = World::new(); - let result = world.run_system_once(system); + let result: Result<(), _> = world.run_system_once(system); assert!( matches!(result, Err(RunSystemError::Skipped(_))), "Expected Skipped from ParamSet with invalid Single, got {result:?}" @@ -853,10 +865,15 @@ mod validation_tests { 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)>) {} + fn system( + mut set: ParamSet<(Query<&TestComponent>, Res)>, + ) -> Result<(), RunSystemError> { + set.try_p1()?; + Ok(()) + } let mut world = World::new(); - let result = world.run_system_once(system); + let result: Result<(), _> = world.run_system_once(system); assert!( matches!(result, Err(RunSystemError::Failed(_))), "Expected Failed from ParamSet with missing resource, got {result:?}" @@ -866,13 +883,16 @@ mod validation_tests { #[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(); + fn system( + mut set: ParamSet<(Query<&TestComponent>, Res)>, + ) -> Result<(), RunSystemError> { + set.try_p1()?; + Ok(()) } let mut world = World::new(); world.init_resource::(); - let result = world.run_system_once(system); + let result: 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/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index 1c6f32deb1cb2..70a261f47b1fa 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).unwrap(); + let res = system_state.get_mut(&mut world); Stepping::begin_frame(res); // now run the schedule; this will panic if the executor doesn't diff --git a/crates/bevy_ecs/src/system/builder.rs b/crates/bevy_ecs/src/system/builder.rs index a84563ea8a01b..6dc38f04d0286 100644 --- a/crates/bevy_ecs/src/system/builder.rs +++ b/crates/bevy_ecs/src/system/builder.rs @@ -1223,7 +1223,7 @@ mod tests { let local = *p0.downcast_mut::>().unwrap(); let query_count = p1.downcast_mut::>().unwrap().iter().count(); let _entities = p2.downcast_mut::<&Entities>().unwrap(); - assert!(p0.downcast_mut::>().is_none()); + assert!(p0.downcast_mut::>().is_err()); local + query_count }, ); diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index db10dd3bab4e1..082f305a0cb9d 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -178,26 +178,47 @@ const _: () = { #[inline] #[track_caller] - unsafe fn get_param<'w, 's>( + unsafe fn try_get_param<'w, 's>( state: &'s mut Self::State, system_meta: &bevy_ecs::system::SystemMeta, world: UnsafeWorldCell<'w>, change_tick: bevy_ecs::change_detection::Tick, ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { + ::get_param( + state, + system_meta, + world, + change_tick, + ) + }) + } + } + + impl bevy_ecs::system::InfallibleSystemParam for Commands<'_, '_> { + #[inline] + #[track_caller] + unsafe fn get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &bevy_ecs::system::SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: bevy_ecs::change_detection::Tick, + ) -> Self::Item<'w, 's> { // SAFETY: Upheld by caller let params = unsafe { - <__StructFieldsAlias as bevy_ecs::system::SystemParam>::get_param( + <__StructFieldsAlias as bevy_ecs::system::InfallibleSystemParam>::get_param( &mut state.state, system_meta, world, change_tick, - )? + ) }; - Ok(Commands { + Commands { queue: InternalQueue::CommandQueue(params.0), allocator: params.1, entities: params.2, - }) + } } } // SAFETY: Only reads Entities diff --git a/crates/bevy_ecs/src/system/commands/parallel_scope.rs b/crates/bevy_ecs/src/system/commands/parallel_scope.rs index 5fb2dbeddecec..91f355ea06a07 100644 --- a/crates/bevy_ecs/src/system/commands/parallel_scope.rs +++ b/crates/bevy_ecs/src/system/commands/parallel_scope.rs @@ -3,7 +3,7 @@ use bevy_utils::Parallel; use crate::{ entity::{Entities, EntityAllocator}, prelude::World, - system::{Deferred, SystemBuffer, SystemMeta, SystemParam}, + system::{Deferred, InfallibleSystemParam, SystemBuffer, SystemMeta, SystemParam}, world::DeferredWorld, }; @@ -49,7 +49,7 @@ struct ParallelCommandQueue { /// } /// # bevy_ecs::system::assert_is_system(parallel_command_system); /// ``` -#[derive(SystemParam)] +#[derive(SystemParam, InfallibleSystemParam)] pub struct ParallelCommands<'w, 's> { state: Deferred<'s, ParallelCommandQueue>, allocator: &'w EntityAllocator, diff --git a/crates/bevy_ecs/src/system/exclusive_function_system.rs b/crates/bevy_ecs/src/system/exclusive_function_system.rs index 7948ef197908c..771313bfbf347 100644 --- a/crates/bevy_ecs/src/system/exclusive_function_system.rs +++ b/crates/bevy_ecs/src/system/exclusive_function_system.rs @@ -114,7 +114,7 @@ where #[cfg(feature = "trace")] let _span_guard = self.system_meta.system_span.enter(); - let params = F::Param::get_param( + let params = F::Param::try_get_param( self.param_state.as_mut().expect(PARAM_MESSAGE), &self.system_meta, )?; diff --git a/crates/bevy_ecs/src/system/exclusive_system_param.rs b/crates/bevy_ecs/src/system/exclusive_system_param.rs index 7950c311bd6e7..e7d5ac76df0e7 100644 --- a/crates/bevy_ecs/src/system/exclusive_system_param.rs +++ b/crates/bevy_ecs/src/system/exclusive_system_param.rs @@ -27,7 +27,7 @@ pub trait ExclusiveSystemParam: Sized { /// Creates a parameter to be passed into an [`ExclusiveSystemParamFunction`]. /// /// [`ExclusiveSystemParamFunction`]: super::ExclusiveSystemParamFunction - fn get_param<'s>( + fn try_get_param<'s>( state: &'s mut Self::State, system_meta: &SystemMeta, ) -> Result, SystemParamValidationError>; @@ -47,7 +47,7 @@ impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> ExclusiveSystemParam QueryState::new(world) } - fn get_param<'s>( + fn try_get_param<'s>( state: &'s mut Self::State, _system_meta: &SystemMeta, ) -> Result, SystemParamValidationError> { @@ -63,7 +63,7 @@ impl<'a, P: SystemParam + 'static> ExclusiveSystemParam for &'a mut SystemState< SystemState::new(world) } - fn get_param<'s>( + fn try_get_param<'s>( state: &'s mut Self::State, _system_meta: &SystemMeta, ) -> Result, SystemParamValidationError> { @@ -79,7 +79,7 @@ impl<'_s, T: FromWorld + Send + 'static> ExclusiveSystemParam for Local<'_s, T> SyncCell::new(T::from_world(world)) } - fn get_param<'s>( + fn try_get_param<'s>( state: &'s mut Self::State, _system_meta: &SystemMeta, ) -> Result, SystemParamValidationError> { @@ -93,7 +93,7 @@ impl ExclusiveSystemParam for PhantomData { fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {} - fn get_param<'s>( + fn try_get_param<'s>( _state: &'s mut Self::State, _system_meta: &SystemMeta, ) -> Result, SystemParamValidationError> { @@ -127,7 +127,7 @@ macro_rules! impl_exclusive_system_param_tuple { } #[inline] - fn get_param<'s>( + fn try_get_param<'s>( state: &'s mut Self::State, system_meta: &SystemMeta, ) -> Result, SystemParamValidationError> { @@ -136,7 +136,7 @@ macro_rules! impl_exclusive_system_param_tuple { clippy::unused_unit, reason = "Zero-length tuples won't have any params to get." )] - Ok(($($param::get_param($param, system_meta)?,)*)) + Ok(($($param::try_get_param($param, system_meta)?,)*)) } } }; diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index c5f070f309764..cfa131479a633 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -6,8 +6,8 @@ use crate::{ query::FilteredAccessSet, schedule::{InternedSystemSet, SystemSet}, system::{ - check_system_change_tick, FromInput, ReadOnlySystemParam, System, SystemIn, SystemInput, - SystemParam, SystemParamItem, + check_system_change_tick, FromInput, InfallibleSystemParam, ReadOnlySystemParam, System, + SystemIn, SystemInput, SystemParam, SystemParamItem, }, world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World, WorldId}, }; @@ -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).unwrap(); +/// let (message_writer, maybe_resource, query) = system_state.try_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).unwrap(); +/// let mut message_reader = cached_state.message_state.try_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).unwrap(); +/// let mut message_reader = system_state.try_get_mut(world).unwrap(); /// /// for message in message_reader.read() { /// println!("Hello World!"); @@ -366,11 +366,29 @@ impl SystemState { &mut self.meta } + /// Retrieve the [`SystemParam`] values. This can only be called when all parameters are read-only and infallible. + /// + /// To obtain the value of a fallible [`SystemParam`], + /// instead call [`Self::try_get`]. + #[inline] + pub fn get<'w, 's>(&'s mut self, world: &'w World) -> SystemParamItem<'w, 's, Param> + where + Param: ReadOnlySystemParam + InfallibleSystemParam, + { + self.validate_world(world.id()); + // SAFETY: Param is read-only and doesn't allow mutable access to World. + // It also matches the World this SystemState was created with. + unsafe { self.get_unchecked(world.as_unsafe_world_cell_readonly()) } + } + /// Retrieve the [`SystemParam`] values. This can only be called when all parameters are read-only. /// /// Returns an error if system parameter validation fails. + /// + /// To obtain the value of an [`InfallibleSystemParam`] + /// without needing to unwrap a result, instead call [`Self::get`]. #[inline] - pub fn get<'w, 's>( + pub fn try_get<'w, 's>( &'s mut self, world: &'w World, ) -> Result, SystemParamValidationError> @@ -380,21 +398,39 @@ impl SystemState { self.validate_world(world.id()); // SAFETY: Param is read-only and doesn't allow mutable access to World. // It also matches the World this SystemState was created with. - unsafe { self.get_unchecked(world.as_unsafe_world_cell_readonly()) } + unsafe { self.try_get_unchecked(world.as_unsafe_world_cell_readonly()) } + } + + /// Retrieve the mutable [`SystemParam`] values. This can only be called when all parameters are infallible. + /// + /// To obtain the value of a fallible [`SystemParam`], + /// instead call [`Self::try_get_mut`]. + #[inline] + #[track_caller] + pub fn get_mut<'w, 's>(&'s mut self, world: &'w mut World) -> SystemParamItem<'w, 's, Param> + where + Param: InfallibleSystemParam, + { + 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()) } } /// Retrieve the mutable [`SystemParam`] values. /// /// Returns an error if system parameter validation fails. + /// + /// To obtain the value of an [`InfallibleSystemParam`] + /// without needing to unwrap a result, instead call [`Self::get_mut`]. #[inline] #[track_caller] - pub fn get_mut<'w, 's>( + pub fn try_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()) } + unsafe { self.try_get_unchecked(world.as_unsafe_world_cell()) } } /// Applies all state queued up for [`SystemParam`] values. For example, this will apply commands queued up @@ -428,23 +464,49 @@ impl SystemState { } } + /// Retrieve the [`SystemParam`] values. This can only be called when all parameters are infallible. + /// + /// To obtain the value of a fallible [`SystemParam`], + /// instead call [`Self::try_get_unchecked`]. + /// + /// # 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 + /// created with. + #[inline] + #[track_caller] + pub unsafe fn get_unchecked<'w, 's>( + &'s mut self, + world: UnsafeWorldCell<'w>, + ) -> SystemParamItem<'w, 's, Param> + where + Param: InfallibleSystemParam, + { + let change_tick = world.increment_change_tick(); + // SAFETY: The invariants are upheld by the caller. + unsafe { self.fetch(world, change_tick) } + } + /// Retrieve the [`SystemParam`] values. /// /// Returns an error if system parameter validation fails. /// + /// To obtain the value of an [`InfallibleSystemParam`] + /// without needing to unwrap a result, instead call [`Self::get_unchecked`]. + /// /// # 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 /// created with. #[inline] #[track_caller] - pub unsafe fn get_unchecked<'w, 's>( + pub unsafe fn try_get_unchecked<'w, 's>( &'s mut self, world: UnsafeWorldCell<'w>, ) -> Result, SystemParamValidationError> { let change_tick = world.increment_change_tick(); // SAFETY: The invariants are upheld by the caller. - unsafe { self.fetch(world, change_tick) } + unsafe { self.try_fetch(world, change_tick) } } /// # Safety @@ -457,10 +519,31 @@ impl SystemState { &'s mut self, world: UnsafeWorldCell<'w>, change_tick: Tick, + ) -> SystemParamItem<'w, 's, Param> + where + Param: InfallibleSystemParam, + { + // SAFETY: The invariants are upheld by the caller. + let param = + unsafe { Param::get_param(&mut self.param_state, &self.meta, world, change_tick) }; + self.meta.last_run = change_tick; + param + } + + /// # 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 + /// created with. + #[inline] + #[track_caller] + unsafe fn try_fetch<'w, 's>( + &'s mut self, + world: UnsafeWorldCell<'w>, + change_tick: Tick, ) -> 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::try_get_param(&mut self.param_state, &self.meta, world, change_tick) }?; self.meta.last_run = change_tick; Ok(param) } @@ -678,7 +761,7 @@ where // - 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) + F::Param::try_get_param(&mut state.param, &self.system_meta, world, change_tick) }?; #[cfg(feature = "hotpatching")] @@ -800,13 +883,16 @@ where /// use std::num::ParseIntError; /// /// use bevy_ecs::prelude::*; -/// use bevy_ecs::system::StaticSystemInput; +/// use bevy_ecs::system::{RunSystemError, StaticSystemInput, SystemParamValidationError}; /// /// /// Pipe creates a new system which calls `a`, then calls `b` with the output of `a` /// pub fn pipe( /// mut a: A, /// mut b: B, -/// ) -> impl FnMut(StaticSystemInput, ParamSet<(A::Param, B::Param)>) -> B::Out +/// ) -> impl FnMut( +/// StaticSystemInput, +/// ParamSet<(A::Param, B::Param)>, +/// ) -> Result /// where /// // We need A and B to be systems, add those bounds /// A: SystemParamFunction, @@ -815,8 +901,8 @@ where /// { /// // The type of `params` is inferred based on the return of this function above /// move |StaticSystemInput(a_in), mut params| { -/// let shared = a.run(a_in, params.p0()); -/// b.run(shared, params.p1()) +/// let shared = a.run(a_in, params.try_p0()?); +/// Ok(b.run(shared, params.try_p1()?)) /// } /// } /// diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index 91f7ab26329f5..12fe88ac9891b 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).unwrap(); + let (a, query, _) = system_state.try_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).unwrap(); + let (a, query) = system_state.try_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).unwrap(); + let query = system_state.get(&world); assert_eq!(**query.unwrap(), A(1)); } { - let query = system_state.get(&world).unwrap(); + let query = system_state.get(&world); assert!(query.is_none()); } world.entity_mut(entity).get_mut::().unwrap().0 = 2; { - let query = system_state.get(&world).unwrap(); + let query = system_state.get(&world); 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).unwrap(); + let query = system_state.get(&world); assert_eq!(query.unwrap().1.spawn_tick(), spawn_tick); } { - let query = system_state.get(&world).unwrap(); + let query = system_state.get(&world); 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).unwrap(); + system_state.get(&mismatched_world); } #[test] @@ -1422,7 +1422,7 @@ mod tests { let mut system_state = SystemState::>::new(&mut world); { - let query = system_state.get(&world).unwrap(); + let query = system_state.get(&world); 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).unwrap(); + let query = system_state.get(&world); 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).unwrap(); + let a = self.state.try_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).unwrap(); + let q = self.state_q.get(world); 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).unwrap(); + let q = self.state_q.get(world); 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).unwrap(); + let mut query = system_state.get_mut(&mut world); assert_eq!( query.iter_mut().map(|m| *m).collect::>(), vec![A(1), A(2)], diff --git a/crates/bevy_ecs/src/system/system_name.rs b/crates/bevy_ecs/src/system/system_name.rs index ea74ff1558931..8400021318532 100644 --- a/crates/bevy_ecs/src/system/system_name.rs +++ b/crates/bevy_ecs/src/system/system_name.rs @@ -3,7 +3,7 @@ use crate::{ prelude::World, query::FilteredAccessSet, system::{ - ExclusiveSystemParam, ReadOnlySystemParam, SystemMeta, SystemParam, + ExclusiveSystemParam, InfallibleSystemParam, ReadOnlySystemParam, SystemMeta, SystemParam, SystemParamValidationError, }, world::unsafe_world_cell::UnsafeWorldCell, @@ -62,14 +62,27 @@ unsafe impl SystemParam for SystemName { ) { } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for SystemName { #[inline] unsafe fn get_param<'w, 's>( _state: &'s mut Self::State, system_meta: &SystemMeta, _world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Result, SystemParamValidationError> { - Ok(SystemName(system_meta.name.clone())) + ) -> Self::Item<'w, 's> { + SystemName(system_meta.name.clone()) } } @@ -82,7 +95,7 @@ impl ExclusiveSystemParam for SystemName { fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {} - fn get_param<'s>( + fn try_get_param<'s>( _state: &'s mut Self::State, system_meta: &SystemMeta, ) -> Result, SystemParamValidationError> { diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 53a7c56b98e16..872d253310d78 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -22,7 +22,7 @@ use crate::{ }, }; use alloc::{borrow::Cow, boxed::Box, vec::Vec}; -pub use bevy_ecs_macros::SystemParam; +pub use bevy_ecs_macros::{InfallibleSystemParam, SystemParam}; use bevy_platform::cell::SyncCell; use bevy_ptr::UnsafeCellDeref; use bevy_utils::prelude::DebugName; @@ -211,7 +211,7 @@ use variadics_please::{all_tuples, all_tuples_enumerated}; /// /// The implementor must ensure the following is true. /// - [`SystemParam::init_access`] correctly registers all [`World`] accesses used -/// by [`SystemParam::get_param`] with the provided [`system_meta`](SystemMeta). +/// by [`SystemParam::try_get_param`] with the provided [`system_meta`](SystemMeta). /// - None of the world accesses may conflict with any prior accesses registered /// on `system_meta`. pub unsafe trait SystemParam: Sized { @@ -275,7 +275,7 @@ pub unsafe trait SystemParam: Sized { /// If `Self` is `ReadOnlySystemParam`, the access is read-only and can never conflict. /// Otherwise, [`SystemParam::init_access`] must be called to ensure it does not panic. /// - `world` must be the same [`World`] that was used to initialize [`state`](SystemParam::init_state). - unsafe fn get_param<'world, 'state>( + unsafe fn try_get_param<'world, 'state>( state: &'state mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, @@ -283,10 +283,46 @@ pub unsafe trait SystemParam: Sized { ) -> Result, SystemParamValidationError>; } +/// A [`SystemParam`] that is always valid. +/// +/// Infallible system parameters can be obtained from methods like [`SystemState::get`](crate::system::SystemState::get) and [`ParamSet::p0`] +/// without needing any error handling. +/// +/// For example, [`Query`] implements this trait. +/// It is always possible to create a query, even if it would not match anything. +/// +/// In contrast, [`Res`] and [`ResMut`] are fallible because the resource may not exist, +/// as are types like [`MessageReader`] that use resources internally. +/// +/// # Derive +/// +/// This trait can be derived with the [`derive@super::InfallibleSystemParam`] macro. +/// It has the same restrictions as deriving [`SystemParam`], with the additional requirement that all fields implement `InfallibleSystemParam`. +pub trait InfallibleSystemParam: SystemParam { + /// Creates a parameter to be passed into a [`SystemParamFunction`](super::SystemParamFunction). + /// + /// This is an infallible version of [`SystemParam::try_get_param`] available for implementations that always return `Ok`. + /// + /// # Safety + /// + /// - The passed [`UnsafeWorldCell`] must have access to any world data registered + /// in [`init_access`](SystemParam::init_access). + /// - [`SystemParam::init_access`] must not request conflicting access. + /// If `Self` is `ReadOnlySystemParam`, the access is read-only and can never conflict. + /// Otherwise, [`SystemParam::init_access`] must be called to ensure it does not panic. + /// - `world` must be the same [`World`] that was used to initialize [`state`](SystemParam::init_state). + unsafe fn get_param<'world, 'state>( + state: &'state mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'world>, + change_tick: Tick, + ) -> Self::Item<'world, 'state>; +} + /// A [`SystemParam`] that only reads a given [`World`]. /// /// # Safety -/// This must only be implemented for [`SystemParam`] impls that exclusively read the World passed in to [`SystemParam::get_param`] +/// This must only be implemented for [`SystemParam`] impls that exclusively read the World passed in to [`SystemParam::try_get_param`] pub unsafe trait ReadOnlySystemParam: SystemParam {} /// Shorthand way of accessing the associated type [`SystemParam::Item`] for a given [`SystemParam`]. @@ -321,17 +357,32 @@ unsafe impl SystemParam for Qu } #[inline] - unsafe fn get_param<'w, 's>( + unsafe fn try_get_param<'w, 's>( state: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam + for Query<'_, '_, D, F> +{ + #[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 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. - Ok(unsafe { state.query_unchecked_with_ticks(world, system_meta.last_run, change_tick) }) + unsafe { state.query_unchecked_with_ticks(world, system_meta.last_run, change_tick) } } } @@ -357,7 +408,7 @@ unsafe impl<'a, 'b, D: IterQueryData + 'static, F: QueryFilter + 'static> System } #[inline] - unsafe fn get_param<'w, 's>( + unsafe fn try_get_param<'w, 's>( state: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, @@ -410,14 +461,14 @@ unsafe impl SystemParam } #[inline] - unsafe fn get_param<'w, 's>( + unsafe fn try_get_param<'w, 's>( state: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Result, SystemParamValidationError> { // SAFETY: Delegate to existing `SystemParam` implementations. - let query = unsafe { Query::get_param(state, system_meta, 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", @@ -514,6 +565,7 @@ unsafe impl<'w, 's, D: ReadOnlyQueryData + 'static, F: QueryFilter + 'static> Re /// /// ``` /// # use bevy_ecs::prelude::*; +/// # use bevy_ecs::system::SystemParamValidationError; /// # /// # #[derive(Message)] /// # struct MyMessage; @@ -534,18 +586,19 @@ unsafe impl<'w, 's, D: ReadOnlyQueryData + 'static, F: QueryFilter + 'static> Re /// // using the other mutable parameters. /// &World, /// )>, -/// ) { -/// for message in set.p0().read() { +/// ) -> Result<()> { +/// for message in set.try_p0()?.read() { /// // ... /// # let _message = message; /// } -/// set.p1().write(MyMessage::new()); +/// set.try_p1()?.write(MyMessage::new()); /// /// let entities = set.p2().entities(); /// // ... /// # let _entities = entities; +/// Ok(()) /// } -/// # bevy_ecs::system::assert_is_system(message_system); +/// # bevy_ecs::system::assert_is_system::<(), (), _>(message_system); /// ``` pub struct ParamSet<'w, 's, T: SystemParam> { param_states: &'s mut T::State, @@ -555,7 +608,7 @@ pub struct ParamSet<'w, 's, T: SystemParam> { } macro_rules! impl_param_set { - ($(($index: tt, $param: ident, $fn_name: ident)),*) => { + ($(($index: tt, $param: ident, $fn_name: ident, $try_fn_name: ident)),*) => { // SAFETY: All parameters are constrained to ReadOnlySystemParam, so World is only read unsafe impl<'w, 's, $($param,)*> ReadOnlySystemParam for ParamSet<'w, 's, ($($param,)*)> where $($param: ReadOnlySystemParam,)* @@ -613,28 +666,32 @@ macro_rules! impl_param_set { } #[inline] - unsafe fn get_param<'w, 's>( + unsafe fn try_get_param<'w, 's>( state: &'s mut Self::State, system_meta: &SystemMeta, 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) }?); - )* + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok( unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } + } - Ok(ParamSet { + impl<'_w, '_s, $($param: SystemParam,)*> InfallibleSystemParam for ParamSet<'_w, '_s, ($($param,)*)> + { + #[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 { param_states: state, system_meta: system_meta.clone(), world, change_tick, - }) + } } } @@ -645,21 +702,45 @@ macro_rules! impl_param_set { #[doc = stringify!($index)] /// in this [`ParamSet`]. /// No other parameters may be accessed while this one is active. - pub fn $fn_name<'a>(&'a mut self) -> SystemParamItem<'a, 'a, $param> { + /// + /// + /// To obtain the value of an [`InfallibleSystemParam`] + /// without needing to unwrap a result, instead call + #[doc = concat!("[`Self::", stringify!($fn_name), "`].")] + pub fn $try_fn_name<'a>(&'a mut self) -> Result, SystemParamValidationError> { + // SAFETY: systems run without conflicts with other systems. + // Conflicting params in ParamSet are not accessible at the same time + // ParamSets are guaranteed to not conflict with other SystemParams + unsafe { + $param::try_get_param(&mut self.param_states.$index, &self.system_meta, self.world, self.change_tick) + } + } + + /// Gets exclusive access to the parameter at index + #[doc = stringify!($index)] + /// in this [`ParamSet`]. + /// No other parameters may be accessed while this one is active. + /// + /// To obtain the value of a fallible [`SystemParam`], + /// instead call + #[doc = concat!("[`Self::", stringify!($try_fn_name), "`].")] + pub fn $fn_name<'a>(&'a mut self) -> SystemParamItem<'a, 'a, $param> + where + $param: InfallibleSystemParam + { // SAFETY: systems run without conflicts with other systems. // Conflicting params in ParamSet are not accessible at the same time // ParamSets are guaranteed to not conflict with other SystemParams 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}")) } )* } } } -all_tuples_enumerated!(impl_param_set, 1, 8, P, p); +all_tuples_enumerated!(impl_param_set, 1, 8, P, p, try_p); // SAFETY: Res only reads a single World resource unsafe impl<'a, T: Resource> ReadOnlySystemParam for Res<'a, T> {} @@ -699,7 +780,7 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { } #[inline] - unsafe fn get_param<'w, 's>( + unsafe fn try_get_param<'w, 's>( &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, @@ -756,7 +837,7 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { } #[inline] - unsafe fn get_param<'w, 's>( + unsafe fn try_get_param<'w, 's>( &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, @@ -806,20 +887,33 @@ unsafe impl SystemParam for &'_ World { component_access_set.add(filtered_access); } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for &'_ World { #[inline] unsafe fn get_param<'w, 's>( _state: &'s mut Self::State, _system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Result, SystemParamValidationError> { + ) -> Self::Item<'w, 's> { // SAFETY: Read-only access to the entire world was registered in `init_state`. - Ok(unsafe { world.world() }) + unsafe { world.world() } } } // SAFETY: `DeferredWorld` can read all components and resources but cannot be used to gain any other mutable references. -unsafe impl<'w> SystemParam for DeferredWorld<'w> { +unsafe impl SystemParam for DeferredWorld<'_> { type State = (); type Item<'world, 'state> = DeferredWorld<'world>; @@ -839,14 +933,27 @@ unsafe impl<'w> SystemParam for DeferredWorld<'w> { component_access_set.write_all(); } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for DeferredWorld<'_> { unsafe fn get_param<'world, 'state>( _state: &'state mut Self::State, _system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, _change_tick: Tick, - ) -> Result, SystemParamValidationError> { + ) -> Self::Item<'world, 'state> { // SAFETY: Upheld by caller - Ok(unsafe { world.into_deferred() }) + unsafe { world.into_deferred() } } } @@ -1029,14 +1136,27 @@ unsafe impl<'a, T: FromWorld + Send + 'static> SystemParam for Local<'a, T> { ) { } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl<'a, T: FromWorld + Send + 'static> InfallibleSystemParam for Local<'a, T> { #[inline] unsafe fn get_param<'w, 's>( state: &'s mut Self::State, _system_meta: &SystemMeta, _world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Result, SystemParamValidationError> { - Ok(Local(state.get())) + ) -> Self::Item<'w, 's> { + Local(state.get()) } } @@ -1233,14 +1353,27 @@ unsafe impl SystemParam for Deferred<'_, T> { state.get().queue(system_meta, world); } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for Deferred<'_, T> { #[inline] unsafe fn get_param<'w, 's>( state: &'s mut Self::State, _system_meta: &SystemMeta, _world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Result, SystemParamValidationError> { - Ok(Deferred(state.get())) + ) -> Self::Item<'w, 's> { + Deferred(state.get()) } } @@ -1264,14 +1397,27 @@ unsafe impl SystemParam for ExclusiveMarker { system_meta.set_exclusive(); } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for ExclusiveMarker { #[inline] unsafe fn get_param<'world, 'state>( _state: &'state mut Self::State, _system_meta: &SystemMeta, _world: UnsafeWorldCell<'world>, _change_tick: Tick, - ) -> Result, SystemParamValidationError> { - Ok(Self(PhantomData)) + ) -> Self::Item<'world, 'state> { + Self(PhantomData) } } @@ -1298,14 +1444,27 @@ unsafe impl SystemParam for NonSendMarker { system_meta.set_non_send(); } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for NonSendMarker { #[inline] unsafe fn get_param<'world, 'state>( _state: &'state mut Self::State, _system_meta: &SystemMeta, _world: UnsafeWorldCell<'world>, _change_tick: Tick, - ) -> Result, SystemParamValidationError> { - Ok(Self(PhantomData)) + ) -> Self::Item<'world, 'state> { + Self(PhantomData) } } @@ -1344,7 +1503,7 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> { } #[inline] - unsafe fn get_param<'w, 's>( + unsafe fn try_get_param<'w, 's>( &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, @@ -1392,7 +1551,7 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> { } #[inline] - unsafe fn get_param<'w, 's>( + unsafe fn try_get_param<'w, 's>( &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, @@ -1426,14 +1585,27 @@ unsafe impl<'a> SystemParam for &'a Archetypes { ) { } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for &Archetypes { #[inline] unsafe fn get_param<'w, 's>( _state: &'s mut Self::State, _system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Result, SystemParamValidationError> { - Ok(world.archetypes()) + ) -> Self::Item<'w, 's> { + world.archetypes() } } @@ -1455,14 +1627,27 @@ unsafe impl<'a> SystemParam for &'a Components { ) { } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for &Components { #[inline] unsafe fn get_param<'w, 's>( _state: &'s mut Self::State, _system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Result, SystemParamValidationError> { - Ok(world.components()) + ) -> Self::Item<'w, 's> { + world.components() } } @@ -1484,14 +1669,27 @@ unsafe impl<'a> SystemParam for &'a Entities { ) { } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for &Entities { #[inline] unsafe fn get_param<'w, 's>( _state: &'s mut Self::State, _system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Result, SystemParamValidationError> { - Ok(world.entities()) + ) -> Self::Item<'w, 's> { + world.entities() } } @@ -1513,14 +1711,27 @@ unsafe impl<'a> SystemParam for &'a EntityAllocator { ) { } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for &EntityAllocator { #[inline] unsafe fn get_param<'w, 's>( _state: &'s mut Self::State, _system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Result, SystemParamValidationError> { - Ok(world.entity_allocator()) + ) -> Self::Item<'w, 's> { + world.entity_allocator() } } @@ -1542,14 +1753,27 @@ unsafe impl<'a> SystemParam for &'a Bundles { ) { } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for &Bundles { #[inline] unsafe fn get_param<'w, 's>( _state: &'s mut Self::State, _system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, _change_tick: Tick, - ) -> Result, SystemParamValidationError> { - Ok(world.bundles()) + ) -> Self::Item<'w, 's> { + world.bundles() } } @@ -1600,17 +1824,30 @@ unsafe impl SystemParam for SystemChangeTick { ) { } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for SystemChangeTick { #[inline] unsafe fn get_param<'w, 's>( _state: &'s mut Self::State, system_meta: &SystemMeta, _world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> Result, SystemParamValidationError> { - Ok(SystemChangeTick { + ) -> Self::Item<'w, 's> { + SystemChangeTick { last_run: system_meta.last_run, this_run: change_tick, - }) + } } } @@ -1634,14 +1871,14 @@ unsafe impl SystemParam for Option { } #[inline] - unsafe fn get_param<'world, 'state>( - state: &'state mut Self::State, + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, system_meta: &SystemMeta, - world: UnsafeWorldCell<'world>, + world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> Result, SystemParamValidationError> { - // SAFETY: Upheld by caller - Ok(unsafe { T::get_param(state, system_meta, world, change_tick) }.ok()) + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) } fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) { @@ -1653,6 +1890,19 @@ unsafe impl SystemParam for Option { } } +impl InfallibleSystemParam for Option { + #[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> { + // SAFETY: Upheld by caller + unsafe { T::try_get_param(state, system_meta, world, change_tick) }.ok() + } +} + // SAFETY: Delegates to `T`, which ensures the safety requirements are met unsafe impl ReadOnlySystemParam for Option {} @@ -1676,14 +1926,14 @@ unsafe impl SystemParam for Result( - state: &'state mut Self::State, + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, system_meta: &SystemMeta, - world: UnsafeWorldCell<'world>, + world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> Result, SystemParamValidationError> { - // SAFETY: Upheld by caller - Ok(unsafe { T::get_param(state, system_meta, world, change_tick) }) + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) } fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) { @@ -1695,6 +1945,19 @@ unsafe impl SystemParam for Result InfallibleSystemParam for Result { + #[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> { + // SAFETY: Upheld by caller + unsafe { T::try_get_param(state, system_meta, world, change_tick) } + } +} + // SAFETY: Delegates to `T`, which ensures the safety requirements are met unsafe impl ReadOnlySystemParam for Result {} @@ -1771,14 +2034,14 @@ unsafe impl SystemParam for If { } #[inline] - unsafe fn get_param<'world, 'state>( + unsafe fn try_get_param<'world, 'state>( state: &'state mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, change_tick: Tick, ) -> Result, SystemParamValidationError> { // SAFETY: Upheld by caller. - unsafe { T::get_param(state, system_meta, world, change_tick) } + unsafe { T::try_get_param(state, system_meta, world, change_tick) } .map(If) .map_err(|mut e| { e.skipped = true; @@ -1795,6 +2058,19 @@ unsafe impl SystemParam for If { } } +impl InfallibleSystemParam for If { + #[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> { + // SAFETY: Upheld by caller. + If(unsafe { T::get_param(state, system_meta, world, change_tick) }) + } +} + // SAFETY: Delegates to `T`, which ensures the safety requirements are met unsafe impl ReadOnlySystemParam for If {} @@ -1821,7 +2097,7 @@ unsafe impl SystemParam for Vec { } #[inline] - unsafe fn get_param<'world, 'state>( + unsafe fn try_get_param<'world, 'state>( state: &'state mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, @@ -1832,7 +2108,7 @@ unsafe impl SystemParam for Vec { // SAFETY: // - We initialized the access for each parameter in `init_access`, so the caller ensures we have access to any world data needed by each param. // - The caller ensures this was the world used to initialize our state, and we used that world to initialize parameter states - .map(|state| unsafe { T::get_param(state, system_meta, world, change_tick) }) + .map(|state| unsafe { T::try_get_param(state, system_meta, world, change_tick) }) .collect() } @@ -1849,6 +2125,24 @@ unsafe impl SystemParam for Vec { } } +impl InfallibleSystemParam for Vec { + #[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> { + state + .iter_mut() + // SAFETY: + // - We initialized the access for each parameter in `init_access`, so the caller ensures we have access to any world data needed by each param. + // - The caller ensures this was the world used to initialize our state, and we used that world to initialize parameter states + .map(|state| unsafe { T::get_param(state, system_meta, world, change_tick) }) + .collect() + } +} + // SAFETY: Registers access for each element of `state`. // If any one conflicts with a previous parameter, // the call passing a copy of the current access will panic. @@ -1882,28 +2176,14 @@ unsafe impl SystemParam for ParamSet<'_, '_, Vec> { } #[inline] - unsafe fn get_param<'world, 'state>( - state: &'state mut Self::State, + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, system_meta: &SystemMeta, - world: UnsafeWorldCell<'world>, + 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. - 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(), - world, - change_tick, - }) + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) } fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) { @@ -1919,9 +2199,29 @@ unsafe impl SystemParam for ParamSet<'_, '_, Vec> { } } -impl ParamSet<'_, '_, Vec> { +impl InfallibleSystemParam for ParamSet<'_, '_, Vec> { + #[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> { + ParamSet { + param_states: state, + system_meta: system_meta.clone(), + world, + change_tick, + } + } +} + +impl ParamSet<'_, '_, Vec> { /// Accesses the parameter at the given index. /// No other parameters may be accessed while this one is active. + /// + /// To obtain the value of a fallible [`SystemParam`], + /// instead call [`Self::try_get_mut`]. 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. @@ -1934,7 +2234,6 @@ impl ParamSet<'_, '_, Vec> { self.world, self.change_tick, ) - .unwrap() } } @@ -1946,13 +2245,37 @@ 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) } - .unwrap_or_else(|err| panic!("ParamSet parameter validation failed: {err}")), + unsafe { T::get_param(state, &self.system_meta, self.world, self.change_tick) }, ); }); } } +impl ParamSet<'_, '_, Vec> { + /// Accesses the parameter at the given index. + /// No other parameters may be accessed while this one is active. + /// + /// To obtain the value of an in fallible [`SystemParam`] + /// without needing to unwrap a result, instead call [`Self::get_mut`]. + pub fn try_get_mut( + &mut self, + index: usize, + ) -> Result, SystemParamValidationError> { + // SAFETY: + // - 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::try_get_param( + &mut self.param_states[index], + &self.system_meta, + self.world, + self.change_tick, + ) + } + } +} + macro_rules! impl_system_param_tuple { ($(#[$meta:meta])* $($param: ident),*) => { $(#[$meta])* @@ -2005,7 +2328,7 @@ macro_rules! impl_system_param_tuple { #[inline] #[track_caller] - unsafe fn get_param<'w, 's>( + unsafe fn try_get_param<'w, 's>( state: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, @@ -2023,7 +2346,48 @@ macro_rules! impl_system_param_tuple { clippy::unused_unit, reason = "Zero-length tuples won't have any params to get." )] - Ok(($($param::get_param($param, system_meta, world, change_tick)?,)*)) + Ok(($($param::try_get_param($param, system_meta, world, change_tick)?,)*)) + } + } + } + + #[expect( + clippy::allow_attributes, + reason = "This is in a macro, and as such, the below lints may not always apply." + )] + #[allow( + non_snake_case, + reason = "Certain variable names are provided by the caller, not by us." + )] + #[allow( + unused_variables, + reason = "Zero-length tuples won't use some of the parameters." + )] + #[allow(clippy::unused_unit, reason = "Zero length tuple is unit.")] + $(#[$meta])* + // SAFETY: implementers of each `SystemParam` in the tuple have validated their impls + impl<$($param: InfallibleSystemParam),*> InfallibleSystemParam for ($($param,)*) { + #[inline] + #[track_caller] + unsafe fn get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Self::Item<'w, 's> { + let ($($param,)*) = state; + + #[allow( + unused_unsafe, + reason = "Zero-length tuples won't have any params to validate." + )] + // SAFETY: Upheld by caller + unsafe { + #[allow( + clippy::unused_unit, + reason = "Zero-length tuples won't have any params to get." + )] + ($($param::get_param($param, system_meta, world, change_tick),)*) } } } @@ -2171,15 +2535,28 @@ unsafe impl SystemParam for StaticSystemParam<'_, '_, P::queue(state, system_meta, world); } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: Defer to the safety of P::SystemParam + unsafe { P::try_get_param(state, system_meta, world, change_tick) }.map(StaticSystemParam) + } +} + +impl InfallibleSystemParam for StaticSystemParam<'_, '_, P> { #[inline] unsafe fn get_param<'world, 'state>( state: &'state mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, change_tick: Tick, - ) -> Result, SystemParamValidationError> { + ) -> Self::Item<'world, 'state> { // SAFETY: Defer to the safety of P::SystemParam - unsafe { P::get_param(state, system_meta, world, change_tick) }.map(StaticSystemParam) + StaticSystemParam(unsafe { P::get_param(state, system_meta, world, change_tick) }) } } @@ -2198,14 +2575,27 @@ unsafe impl SystemParam for PhantomData { ) { } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for PhantomData { #[inline] unsafe fn get_param<'world, 'state>( _state: &'state mut Self::State, _system_meta: &SystemMeta, _world: UnsafeWorldCell<'world>, _change_tick: Tick, - ) -> Result, SystemParamValidationError> { - Ok(PhantomData) + ) -> Self::Item<'world, 'state> { + PhantomData } } @@ -2249,7 +2639,7 @@ unsafe impl ReadOnlySystemParam for PhantomData {} /// // They will return `None` if the type does not match. /// assert!(param.is::>()); /// assert!(!param.is::>()); -/// assert!(param.downcast_mut::>().is_none()); +/// assert!(param.downcast_mut::>().is_err()); /// let res = param.downcast_mut::>().unwrap(); /// // The type parameter can be left out if it can be determined from use. /// let res: Res = param.downcast().unwrap(); @@ -2308,9 +2698,13 @@ impl<'w, 's> DynSystemParam<'w, 's> { self.state.is::>>() } - /// Returns the inner system param if it is the correct type. + /// Returns the inner system param if it is the correct type and is valid. + /// + /// Returns an error if the inner system param is the wrong type, + /// or returns the error from the inner system param if it is invalid. + /// /// This consumes the dyn param, so the returned param can have its original world and state lifetimes. - pub fn downcast(self) -> Option + pub fn downcast(self) -> Result // See downcast() function for an explanation of the where clause where T::Item<'static, 'static>: SystemParam = T> + 'static, @@ -2322,9 +2716,13 @@ impl<'w, 's> DynSystemParam<'w, 's> { unsafe { downcast::(self.state, &self.system_meta, self.world, self.change_tick) } } - /// Returns the inner system parameter if it is the correct type. + /// Returns the inner system parameter if it is the correct type and is valid. + /// + /// Returns an error if the inner system param is the wrong type, + /// or returns the error from the inner system param if it is invalid. + /// /// This borrows the dyn param, so the returned param is only valid for the duration of that borrow. - pub fn downcast_mut<'a, T: SystemParam>(&'a mut self) -> Option + pub fn downcast_mut<'a, T: SystemParam>(&'a mut self) -> Result // See downcast() function for an explanation of the where clause where T::Item<'static, 'static>: SystemParam = T> + 'static, @@ -2336,12 +2734,18 @@ impl<'w, 's> DynSystemParam<'w, 's> { unsafe { downcast::(self.state, &self.system_meta, self.world, self.change_tick) } } - /// Returns the inner system parameter if it is the correct type. + /// Returns the inner system parameter if it is the correct type and is valid. + /// + /// Returns an error if the inner system param is the wrong type, + /// or returns the error from the inner system param if it is invalid. + /// /// This borrows the dyn param, so the returned param is only valid for the duration of that borrow, /// but since it only performs read access it can keep the original world lifetime. /// This can be useful with methods like [`Query::iter_inner()`] or [`Res::into_inner()`] /// to obtain references with the original world lifetime. - pub fn downcast_mut_inner<'a, T: ReadOnlySystemParam>(&'a mut self) -> Option + pub fn downcast_mut_inner<'a, T: ReadOnlySystemParam>( + &'a mut self, + ) -> Result // See downcast() function for an explanation of the where clause where T::Item<'static, 'static>: SystemParam = T> + 'static, @@ -2365,7 +2769,7 @@ unsafe fn downcast<'w, 's, T: SystemParam>( system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, change_tick: Tick, -) -> Option +) -> Result // We need a 'static version of the SystemParam to use with `Any::downcast_mut()`, // and we need a <'w, 's> version to actually return. // The type parameter T must be the one we return in order to get type inference from the return value. @@ -2379,12 +2783,13 @@ where { state .downcast_mut::>>() + .ok_or_else(|| SystemParamValidationError::invalid::("`DynSystemParam` had wrong type")) .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) }.ok() + unsafe { T::Item::try_get_param(&mut state.0, system_meta, world, change_tick) } }) } @@ -2415,17 +2820,6 @@ trait DynParamState: Sync + Send + Any { component_access_set: &mut FilteredAccessSet, world: &mut World, ); - - /// Validates the inner parameter by calling [`SystemParam::get_param`] and discarding the value. - /// - /// # Safety - /// Refer to [`SystemParam::get_param`]. - unsafe fn validate( - &mut self, - system_meta: &SystemMeta, - world: UnsafeWorldCell, - change_tick: Tick, - ) -> Result<(), SystemParamValidationError>; } /// A wrapper around a [`SystemParam::State`] that can be used as a trait object in a [`DynSystemParam`]. @@ -2448,16 +2842,6 @@ impl DynParamState for ParamState { ) { T::init_access(&self.0, system_meta, component_access_set, world); } - - unsafe fn validate( - &mut self, - system_meta: &SystemMeta, - world: UnsafeWorldCell, - change_tick: Tick, - ) -> Result<(), SystemParamValidationError> { - // SAFETY: Upheld by caller. - unsafe { T::get_param(&mut self.0, system_meta, world, change_tick) }.map(drop) - } } // SAFETY: Delegates to the wrapped parameter, which ensures the safety requirements are met @@ -2482,24 +2866,14 @@ unsafe impl SystemParam for DynSystemParam<'_, '_> { } #[inline] - unsafe fn get_param<'world, 'state>( - state: &'state mut Self::State, + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, system_meta: &SystemMeta, - world: UnsafeWorldCell<'world>, + world: UnsafeWorldCell<'w>, change_tick: Tick, - ) -> 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. - Ok(unsafe { - DynSystemParam::new(state.0.as_mut(), world, system_meta.clone(), change_tick) - }) + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) } fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) { @@ -2511,6 +2885,23 @@ unsafe impl SystemParam for DynSystemParam<'_, '_> { } } +impl InfallibleSystemParam for DynSystemParam<'_, '_> { + #[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> { + // 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) } + } +} + // SAFETY: Resource ComponentId access is applied to the access. If this FilteredResources // conflicts with any prior access, a panic will occur. unsafe impl SystemParam for FilteredResources<'_, '_> { @@ -2542,15 +2933,28 @@ unsafe impl SystemParam for FilteredResources<'_, '_> { component_access_set.add(filter); } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for FilteredResources<'_, '_> { unsafe fn get_param<'world, 'state>( state: &'state mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, change_tick: Tick, - ) -> Result, SystemParamValidationError> { + ) -> Self::Item<'world, 'state> { // SAFETY: The caller ensures that `world` has access to anything registered in `init_access`, // and we registered all resource access in `state``. - Ok(unsafe { FilteredResources::new(world, state, system_meta.last_run, change_tick) }) + unsafe { FilteredResources::new(world, state, system_meta.last_run, change_tick) } } } @@ -2588,22 +2992,35 @@ unsafe impl SystemParam for FilteredResourcesMut<'_, '_> { component_access_set.add(filter); } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for FilteredResourcesMut<'_, '_> { unsafe fn get_param<'world, 'state>( state: &'state mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'world>, change_tick: Tick, - ) -> Result, SystemParamValidationError> { + ) -> Self::Item<'world, 'state> { // SAFETY: The caller ensures that `world` has access to anything registered in `init_access`, // and we registered all resource access in `state``. - Ok(unsafe { FilteredResourcesMut::new(world, state, system_meta.last_run, change_tick) }) + 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::get_param`], +/// Returned as an error from [`SystemParam::try_get_param`], /// and handled using the unified error handling mechanisms defined in [`bevy_ecs::error`]. #[derive(Debug, PartialEq, Eq, Clone, Error)] pub struct SystemParamValidationError { @@ -2692,7 +3109,7 @@ mod tests { use super::*; use crate::query::Without; use crate::resource::IsResource; - use crate::system::assert_is_system; + use crate::system::{assert_is_system, RunSystemError}; use crate::world::EntityMut; use core::cell::RefCell; @@ -2943,9 +3360,12 @@ mod tests { // Regression test for https://github.com/bevyengine/bevy/issues/10207. #[test] fn param_set_non_send_first() { - fn non_send_param_set(mut p: ParamSet<(NonSend<*mut u8>, ())>) { - let _ = p.p0(); + fn non_send_param_set( + mut p: ParamSet<(NonSend<*mut u8>, ())>, + ) -> Result<(), RunSystemError> { + let _ = p.try_p0()?; p.p1(); + Ok(()) } let mut world = World::new(); @@ -2958,9 +3378,12 @@ mod tests { // Regression test for https://github.com/bevyengine/bevy/issues/10207. #[test] fn param_set_non_send_second() { - fn non_send_param_set(mut p: ParamSet<((), NonSendMut<*mut u8>)>) { + fn non_send_param_set( + mut p: ParamSet<((), NonSendMut<*mut u8>)>, + ) -> Result<(), RunSystemError> { p.p0(); - let _ = p.p1(); + let _ = p.try_p1()?; + Ok(()) } let mut world = World::new(); diff --git a/crates/bevy_ecs/src/world/identifier.rs b/crates/bevy_ecs/src/world/identifier.rs index 63b1a9ed6c11b..03c0a7d928895 100644 --- a/crates/bevy_ecs/src/world/identifier.rs +++ b/crates/bevy_ecs/src/world/identifier.rs @@ -3,7 +3,7 @@ use crate::{ query::FilteredAccessSet, storage::SparseSetIndex, system::{ - ExclusiveSystemParam, ReadOnlySystemParam, SystemMeta, SystemParam, + ExclusiveSystemParam, InfallibleSystemParam, ReadOnlySystemParam, SystemMeta, SystemParam, SystemParamValidationError, }, world::{FromWorld, World}, @@ -67,14 +67,27 @@ unsafe impl SystemParam for WorldId { ) { } + #[inline] + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for WorldId { #[inline] unsafe fn get_param<'world, 'state>( _: &'state mut Self::State, _: &SystemMeta, world: UnsafeWorldCell<'world>, _: Tick, - ) -> Result, SystemParamValidationError> { - Ok(world.id()) + ) -> Self::Item<'world, 'state> { + world.id() } } @@ -86,7 +99,7 @@ impl ExclusiveSystemParam for WorldId { world.id() } - fn get_param<'s>( + fn try_get_param<'s>( state: &'s mut Self::State, _system_meta: &SystemMeta, ) -> Result, SystemParamValidationError> { diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index 7452d545bd456..cdd22a1239d57 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -148,7 +148,7 @@ impl<'w> UnsafeWorldCell<'w> { /// Calling this method implies mutable access to the *whole* world (see first point on safety section /// below), which includes all entities, components, and resources. Notably, calling this on /// [`WorldQuery::init_fetch`](crate::query::WorldQuery::init_fetch) and - /// [`SystemParam::get_param`](crate::system::SystemParam::get_param) are most likely *unsound* unless + /// [`SystemParam::try_get_param`](crate::system::SystemParam::try_get_param) are most likely *unsound* unless /// you can prove that the underlying [`World`] is exclusive, which in normal circumstances is not. /// /// # Safety diff --git a/crates/bevy_gizmos/src/gizmos.rs b/crates/bevy_gizmos/src/gizmos.rs index 02f13609e3470..53b802d3a302d 100644 --- a/crates/bevy_gizmos/src/gizmos.rs +++ b/crates/bevy_gizmos/src/gizmos.rs @@ -229,7 +229,7 @@ where } #[inline] - unsafe fn get_param<'w, 's>( + unsafe fn try_get_param<'w, 's>( state: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, @@ -237,7 +237,7 @@ where ) -> Result, SystemParamValidationError> { // SAFETY: Delegated to existing `SystemParam` implementations. let (mut f0, f1) = unsafe { - GizmosState::::get_param( + GizmosState::::try_get_param( &mut state.state, system_meta, world, diff --git a/crates/bevy_input_focus/src/lib.rs b/crates/bevy_input_focus/src/lib.rs index 47efd05797875..723bdc1023c4e 100644 --- a/crates/bevy_input_focus/src/lib.rs +++ b/crates/bevy_input_focus/src/lib.rs @@ -35,7 +35,11 @@ pub use autofocus::*; use bevy_app::PreUpdate; use bevy_app::{App, Plugin, PostStartup}; use bevy_ecs::{ - entity::Entities, prelude::*, query::QueryData, system::SystemParam, traversal::Traversal, + entity::Entities, + prelude::*, + query::QueryData, + system::{InfallibleSystemParam, SystemParam}, + traversal::Traversal, }; #[cfg(feature = "gamepad")] use bevy_input::gamepad::GamepadButtonChangedEvent; @@ -341,7 +345,7 @@ pub trait IsFocused { /// A system param that helps get information about the current focused entity. /// /// When working with the entire [`World`], consider using the [`IsFocused`] instead. -#[derive(SystemParam)] +#[derive(SystemParam, InfallibleSystemParam)] pub struct IsFocusedHelper<'w, 's> { parent_query: Query<'w, 's, &'static ChildOf>, input_focus: Option>, diff --git a/crates/bevy_input_focus/src/tab_navigation.rs b/crates/bevy_input_focus/src/tab_navigation.rs index 4ee868efdda38..d807cb5b66ef7 100644 --- a/crates/bevy_input_focus/src/tab_navigation.rs +++ b/crates/bevy_input_focus/src/tab_navigation.rs @@ -32,7 +32,7 @@ use bevy_ecs::{ hierarchy::{ChildOf, Children}, observer::On, query::{With, Without}, - system::{Commands, Query, Res, ResMut, SystemParam}, + system::{Commands, InfallibleSystemParam, Query, Res, ResMut, SystemParam}, }; use bevy_input::{ keyboard::{KeyCode, KeyboardInput}, @@ -150,7 +150,7 @@ pub enum TabNavigationError { /// An injectable helper object that provides tab navigation functionality. #[doc(hidden)] -#[derive(SystemParam)] +#[derive(SystemParam, InfallibleSystemParam)] pub struct TabNavigation<'w, 's> { // Query for tab groups. tabgroup_query: Query<'w, 's, (Entity, &'static TabGroup, &'static Children)>, @@ -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).unwrap(); + let tab_navigation = system_state.get(world); 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).unwrap(); + let tab_navigation = system_state.get(world); 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 7cbbd4a314ab1..82a6b19c55ef9 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).unwrap(); + } = state.try_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 9ae2b8237ac0a..8dd3a696f7aae 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).unwrap(); + system_state.try_get_mut(&mut main_world).unwrap(); // Reset per-frame data instance_manager.reset(render_entities); diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index f41f2c44b15f1..9d196fff55e14 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).unwrap(); + } = state.try_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 d1cb7e27c853b..99fd6cd699173 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).unwrap(); + } = state.try_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 8e530bbf63527..b558b5ed4295b 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).unwrap(); + let (mut events, mut assets) = cached_state.state.try_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 0659cec965403..f79c4cd8a9436 100644 --- a/crates/bevy_render/src/extract_param.rs +++ b/crates/bevy_render/src/extract_param.rs @@ -95,7 +95,7 @@ where } #[inline] - unsafe fn get_param<'w, 's>( + unsafe fn try_get_param<'w, 's>( state: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, @@ -105,14 +105,14 @@ where // - 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. let main_world = unsafe { - Res::::get_param( + Res::::try_get_param( &mut state.main_world_state, system_meta, world, change_tick, )? }; - let item = state.state.get(main_world.into_inner())?; + let item = state.state.try_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 43ef2bb5807b5..1521dbd84eee5 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).unwrap(); + let (mut events, mut assets, maybe_render_assets) = cached_state.state.try_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 13d4f67da52eb..a267639ac0c69 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).unwrap(); + let param = self.state.try_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 38bb7f1ccc674..d19d29f0eb08a 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).unwrap(); + let views = state.get(world); 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 0fa4ac1eb67f0..48d12255b01b6 100644 --- a/crates/bevy_render/src/renderer/render_context.rs +++ b/crates/bevy_render/src/renderer/render_context.rs @@ -260,7 +260,7 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam } #[inline] - unsafe fn get_param<'w, 's>( + unsafe fn try_get_param<'w, 's>( state: &'s mut Self::State, _system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, diff --git a/crates/bevy_render/src/sync_world.rs b/crates/bevy_render/src/sync_world.rs index cf92577e53326..c98afc2c57bf7 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).unwrap(); + let query = state.get(world); 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 025aac44ac58f..ae2a5f9efaaea 100644 --- a/crates/bevy_render/src/view/window/screenshot.rs +++ b/crates/bevy_render/src/view/window/screenshot.rs @@ -229,8 +229,7 @@ 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).unwrap(); + let (mut commands, primary_window, screenshots) = system_state.get_mut(&mut main_world); targets.clear(); seen_targets.clear(); diff --git a/crates/bevy_text/src/text_access.rs b/crates/bevy_text/src/text_access.rs index 9d04c1dbb5da8..a76bc1917619e 100644 --- a/crates/bevy_text/src/text_access.rs +++ b/crates/bevy_text/src/text_access.rs @@ -2,7 +2,7 @@ use bevy_color::Color; use bevy_ecs::{ component::Mutable, prelude::*, - system::{Query, SystemParam}, + system::{InfallibleSystemParam, Query, SystemParam}, }; use crate::{LineHeight, TextColor, TextFont, TextSpan}; @@ -47,7 +47,7 @@ impl TextIterScratch { /// System parameter for reading text spans in a text block. /// /// `R` is the root text component. -#[derive(SystemParam)] +#[derive(SystemParam, InfallibleSystemParam)] pub struct TextReader<'w, 's, R: TextRoot> { // This is a local to avoid system ambiguities when TextReaders run in parallel. scratch: Local<'s, TextIterScratch>, diff --git a/crates/bevy_transform/src/helper.rs b/crates/bevy_transform/src/helper.rs index f0dcfc58b85bd..5194954fc07e6 100644 --- a/crates/bevy_transform/src/helper.rs +++ b/crates/bevy_transform/src/helper.rs @@ -5,7 +5,7 @@ use bevy_ecs::{ hierarchy::ChildOf, prelude::Entity, query::QueryEntityError, - system::{Query, SystemParam}, + system::{Query, SystemParam, InfallibleSystemParam}, }; use thiserror::Error; @@ -17,7 +17,7 @@ use crate::components::{GlobalTransform, Transform}; /// you use the [`GlobalTransform`] component stored on the entity, unless you need /// a [`GlobalTransform`] that reflects the changes made to any [`Transform`]s since /// the last time the transform propagation systems ran. -#[derive(SystemParam)] +#[derive(SystemParam, InfallibleSystemParam)] pub struct TransformHelper<'w, 's> { parent_query: Query<'w, 's, &'static ChildOf>, transform_query: Query<'w, 's, &'static Transform>, @@ -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()).unwrap(); + let helper = state.get(app.world()); 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 2c942cde2ab46..29b33a135ead5 100644 --- a/crates/bevy_ui/src/experimental/ghost_hierarchy.rs +++ b/crates/bevy_ui/src/experimental/ghost_hierarchy.rs @@ -5,7 +5,10 @@ use crate::ui_node::ComputedUiTargetCamera; use crate::Node; #[cfg(feature = "ghost_nodes")] use bevy_camera::visibility::Visibility; -use bevy_ecs::{prelude::*, system::SystemParam}; +use bevy_ecs::{ + prelude::*, + system::{InfallibleSystemParam, SystemParam}, +}; #[cfg(feature = "ghost_nodes")] use bevy_reflect::prelude::*; #[cfg(feature = "ghost_nodes")] @@ -28,7 +31,7 @@ pub struct GhostNode; /// System param that allows iteration of all UI root nodes. /// /// A UI root node is either a [`Node`] without a [`ChildOf`], or with only [`GhostNode`] ancestors. -#[derive(SystemParam)] +#[derive(SystemParam, InfallibleSystemParam)] pub struct UiRootNodes<'w, 's> { root_node_query: Query<'w, 's, Entity, (With, Without)>, root_ghost_node_query: Query<'w, 's, Entity, (With, Without)>, @@ -53,7 +56,7 @@ impl<'w, 's> UiRootNodes<'w, 's> { #[cfg(feature = "ghost_nodes")] /// System param that gives access to UI children utilities, skipping over [`GhostNode`]. -#[derive(SystemParam)] +#[derive(SystemParam, InfallibleSystemParam)] pub struct UiChildren<'w, 's> { ui_children_query: Query< 'w, @@ -69,7 +72,7 @@ pub struct UiChildren<'w, 's> { #[cfg(not(feature = "ghost_nodes"))] /// System param that gives access to UI children utilities. -#[derive(SystemParam)] +#[derive(SystemParam, InfallibleSystemParam)] pub struct UiChildren<'w, 's> { ui_children_query: Query<'w, 's, Option<&'static Children>, With>, changed_children_query: Query<'w, 's, Entity, Changed>, @@ -230,7 +233,7 @@ mod tests { }); let mut system_state = SystemState::<(UiRootNodes, Query<&A>)>::new(world); - let (ui_root_nodes, a_query) = system_state.get(world).unwrap(); + let (ui_root_nodes, a_query) = system_state.get(world); let result: Vec<_> = a_query.iter_many(ui_root_nodes.iter()).collect(); @@ -263,7 +266,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).unwrap(); + let (ui_children, a_query) = system_state.get(world); let result: Vec<_> = a_query .iter_many(ui_children.iter_ui_children(n1)) diff --git a/crates/bevy_ui/src/ui_node.rs b/crates/bevy_ui/src/ui_node.rs index bff20eaabe7a2..59b6f5f92afc1 100644 --- a/crates/bevy_ui/src/ui_node.rs +++ b/crates/bevy_ui/src/ui_node.rs @@ -5,7 +5,10 @@ use crate::{ use bevy_camera::{visibility::Visibility, Camera, RenderTarget}; use bevy_color::{Alpha, Color}; use bevy_derive::{Deref, DerefMut}; -use bevy_ecs::{prelude::*, system::SystemParam}; +use bevy_ecs::{ + prelude::*, + system::{InfallibleSystemParam, SystemParam}, +}; use bevy_math::{BVec2, Rect, UVec2, Vec2, Vec4, Vec4Swizzles}; use bevy_reflect::prelude::*; use bevy_sprite::BorderRect; @@ -2947,7 +2950,7 @@ impl UiTargetCamera { #[derive(Component, Default)] pub struct IsDefaultUiCamera; -#[derive(SystemParam)] +#[derive(SystemParam, InfallibleSystemParam)] pub struct DefaultUiCamera<'w, 's> { cameras: Query<'w, 's, (Entity, &'static Camera, &'static RenderTarget)>, default_cameras: Query<'w, 's, Entity, (With, With)>, diff --git a/crates/bevy_ui_render/src/lib.rs b/crates/bevy_ui_render/src/lib.rs index 41b3d6d0e0109..6c2d203be58a1 100644 --- a/crates/bevy_ui_render/src/lib.rs +++ b/crates/bevy_ui_render/src/lib.rs @@ -38,7 +38,7 @@ use bevy_core_pipeline::schedule::{Core2d, Core2dSystems, Core3d, Core3dSystems} use bevy_core_pipeline::upscaling::upscaling; use bevy_ecs::prelude::*; use bevy_ecs::schedule::IntoScheduleConfigs; -use bevy_ecs::system::SystemParam; +use bevy_ecs::system::{InfallibleSystemParam, SystemParam}; use bevy_image::{prelude::*, TRANSPARENT_IMAGE_HANDLE}; use bevy_math::{Affine2, FloatOrd, Mat4, Rect, UVec4, Vec2}; use bevy_render::{ @@ -266,7 +266,7 @@ impl Plugin for UiRenderPlugin { } } -#[derive(SystemParam)] +#[derive(SystemParam, InfallibleSystemParam)] pub struct UiCameraMap<'w, 's> { mapping: Query<'w, 's, RenderEntity>, } diff --git a/crates/bevy_winit/src/cursor/mod.rs b/crates/bevy_winit/src/cursor/mod.rs index 6d5a5471967a6..b38b627352691 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()).unwrap(); + let (mut cursor_cache, mut windows) = windows_state.try_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()).unwrap(); + let (mut windows,) = windows_state.get_mut(self.world_mut()); 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 4af560e16045a..bad25d2632ccc 100644 --- a/crates/bevy_winit/src/state.rs +++ b/crates/bevy_winit/src/state.rs @@ -181,7 +181,10 @@ 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()).unwrap()); + create_windows( + event_loop, + create_window.try_get_mut(self.world_mut()).unwrap(), + ); create_window.apply(self.world_mut()); } @@ -195,7 +198,10 @@ 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()).unwrap()); + create_windows( + event_loop, + create_window.try_get_mut(self.world_mut()).unwrap(), + ); create_window.apply(self.world_mut()); } } @@ -224,7 +230,7 @@ impl ApplicationHandler for WinitAppRunnerState { mut windows, ) = self .message_writer_system_state - .get_mut(self.app.world_mut()) + .try_get_mut(self.app.world_mut()) .unwrap(); let Some(window) = winit_windows.get_window_entity(window_id) else { @@ -462,7 +468,7 @@ impl ApplicationHandler for WinitAppRunnerState { let mut create_monitor = SystemState::::from_world(self.world_mut()); create_monitors( event_loop, - create_monitor.get_mut(self.world_mut()).unwrap(), + create_monitor.try_get_mut(self.world_mut()).unwrap(), ); create_monitor.apply(self.world_mut()); @@ -520,7 +526,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()).unwrap(); + let (config, windows) = focused_windows_state.try_get(self.world()).unwrap(); let focused = windows.iter().any(|(_, window)| window.focused); let mut update_mode = config.update_mode(focused); @@ -576,7 +582,7 @@ impl WinitAppRunnerState { SystemState::::from_world(self.world_mut()); let (.., mut handlers, accessibility_requested, monitors) = - create_window.get_mut(self.world_mut()).unwrap(); + create_window.try_get_mut(self.world_mut()).unwrap(); let winit_window = winit_windows.create_window( event_loop, @@ -609,7 +615,7 @@ impl WinitAppRunnerState { let begin_frame_time = Instant::now(); if should_update { - let (_, windows) = focused_windows_state.get(self.world()).unwrap(); + let (_, windows) = focused_windows_state.try_get(self.world()).unwrap(); // If no windows exist, this will evaluate to `true`. let all_invisible = windows.iter().all(|w| !w.1.visible); @@ -652,7 +658,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()).unwrap(); + let (config, windows) = focused_windows_state.try_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 7fab1c7e136bf..145c958e2b76e 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).unwrap(); + system_state.try_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 65008403b51b9..d70ae13bfe191 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::get_param`] for each system parameter. +//! by [`SystemParam::validate_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::get_param`]: bevy::ecs::system::SystemParam::get_param +//! [`SystemParam::validate_param`]: bevy::ecs::system::SystemParam::validate_param use bevy::ecs::error::warn; use bevy::prelude::*; diff --git a/examples/ecs/send_and_receive_messages.rs b/examples/ecs/send_and_receive_messages.rs index 83b7945bcbf04..adecd57d4dd35 100644 --- a/examples/ecs/send_and_receive_messages.rs +++ b/examples/ecs/send_and_receive_messages.rs @@ -108,7 +108,7 @@ fn debug_messages(mut messages: MessageReader) { fn send_and_receive_param_set( mut param_set: ParamSet<(MessageReader, MessageWriter)>, frame_count: Res, -) { +) -> Result { println!( "Sending and receiving messages for frame {} with a `ParamSet`", frame_count.0 @@ -118,7 +118,7 @@ fn send_and_receive_param_set( let mut messages_to_resend = Vec::new(); // This is p0, as the first parameter in the `ParamSet` is the reader. - for message in param_set.p0().read() { + for message in param_set.try_p0()?.read() { if message.resend_from_param_set { messages_to_resend.push(message.clone()); } @@ -127,8 +127,9 @@ fn send_and_receive_param_set( // This is p1, as the second parameter in the `ParamSet` is the writer. for mut message in messages_to_resend { message.times_sent += 1; - param_set.p1().write(message); + param_set.try_p1()?.write(message); } + Ok(()) } /// A system that both sends and receives messages using a [`Local`] [`MessageCursor`]. diff --git a/release-content/migration-guides/validation_merging.md b/release-content/migration-guides/validation_merging.md index ad4cfa8099002..d76708b6d28bf 100644 --- a/release-content/migration-guides/validation_merging.md +++ b/release-content/migration-guides/validation_merging.md @@ -7,23 +7,50 @@ In an effort to improve performance by reducing redundant data fetches and simpl 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>` +- `SystemParam::get_param` has been renamed `try_param` and 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` +- A new `InfallibleSystemParam` trait has been added for parameters like `Query` that are always valid. This has a `get_param` method that returns a `Self::Item<'world, 'state>`. - `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`: +- `SystemState::fetch`, `get_unchecked`, `get` and `get_mut` now require `InfallibleSystemParam`. For fallible parameters, there are now `try_` variants that return a `Result<..., SystemParamValidationError>`. Callers that previously destructured the result of a fallible paramter directly will need to call `try_get` and add `.unwrap()` or handle the `Result`: ```rust // Before let (res, query) = system_state.get(&world); // After -let (res, query) = system_state.get(&world).unwrap(); +let (res, query) = system_state.try_get(&world).unwrap(); ``` +- Similarly, the `pN()` family of methods on `ParamSet` now require `InfallibleSystemParam`, and `try_pN()` methods have been added that return a `Result`. +- `ParamSet` subparameters are no longer validated before running the system, so a system with a `ParamSet` containing a `Single` will now run. To have the system skip, ensure it returns `Result<(), RunSystemError>` and replace `pN()` with `try_pN()?`, where `N` is the index of the `Single`. + +```rust +// Before +fn system(param_set: ParamSet<(Query<&mut T>, Single<&mut T, With)>) { + // This will not run at all if there would be no matching entity + do_something(); + let t = param_set.p1(); +} + +// After +fn system(param_set: ParamSet<(Query<&mut T>, Single<&mut T, With)>) -> Result<(), RunSystemError> { + // This will always run, because we have not yet validated the `Single`. + do_something(); + let t = param_set.try_p1()?; +} +``` + +If you get a compiler error like ``the trait bound `SomeType: InfallibleSystemParam` is not satisfied`` +on a call to `SystemState::get()` or `ParamSet::pN()`, +switch to `try_get` or `try_pN` as described above. + +If the type is defined using `#[derive(SystemParam)]` and only includes infallible parameters, +you may instead `#[derive(SystemParam, InfallibleSystemParam)]`. + 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`. @@ -32,11 +59,17 @@ bubbling up any errors originating in `SystemParam::get_param`. ## Custom `SystemParam` implementations -If you have a custom `SystemParam` implementation, you need to: +If you have a custom `SystemParam` implementation, you need to either: +If `validate_param` was overridden, because the parameter was sometimes invalid: 1. Remove the `validate_param` method. 2. Move any validation logic into `get_param`. -3. Change `get_param` to return `Result, SystemParamValidationError>`. +3. Rename `get_param` to `try_get_param` and return `Result, SystemParamValidationError>`. + +If `validate_param` was not overridden, because the parameter was always valid: +1. Add an `InfallibleSystemParam` impl +2. Move `get_param` to that impl +3. Create a `SystemParam::try_get_param` method that calls `get_param` ```rust // Before @@ -65,10 +98,10 @@ unsafe impl SystemParam for MyParam<'_> { } } -// After +// After, for fallible parameters unsafe impl SystemParam for MyParam<'_> { // ... - unsafe fn get_param<'w, 's>( + unsafe fn try_get_param<'w, 's>( state: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, @@ -82,6 +115,33 @@ unsafe impl SystemParam for MyParam<'_> { Ok(MyParam { /* ... */ }) } } + +// After, for infallible parameters +unsafe impl SystemParam for MyParam<'_> { + // ... + unsafe fn try_get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Result, SystemParamValidationError> { + // SAFETY: `try_get_param` has the same safety requirements as `get_param` + Ok(unsafe { Self::get_param(state, system_meta, world, change_tick) }) + } +} + +impl InfallibleSystemParam for MyParam<'_> { + // ... + unsafe fn param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Self::Item<'w, 's> { + // fetch logic + Ok(MyParam { /* ... */ }) + } +} ``` ## Custom `ExclusiveSystemParam` implementations From ba0c3f5e6c15e158c627b3006b05f8701c8e7353 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Thu, 12 Mar 2026 13:47:05 -0400 Subject: [PATCH 2/7] `cargo fmt` --- crates/bevy_ecs/macros/src/lib.rs | 6 ++---- crates/bevy_transform/src/helper.rs | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 5d66c6de37dca..ebb7416352bf9 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -490,9 +490,7 @@ pub fn derive_infallible_system_param(input: TokenStream) -> TokenStream { Err(e) => e.into_compile_error().into(), } } -fn derive_infallible_system_param_impl( - ast: DeriveInput, -) -> syn::Result { +fn derive_infallible_system_param_impl(ast: DeriveInput) -> syn::Result { let fields = get_struct_fields(&ast.data, "derive(InfallibleSystemParam)")?; let path = bevy_ecs_path(); @@ -519,7 +517,7 @@ fn derive_infallible_system_param_impl( let struct_name = &ast.ident; - Ok(TokenStream::from(quote!{ + Ok(TokenStream::from(quote! { impl #impl_generics #path::system::InfallibleSystemParam for #struct_name #type_generics #where_clause { diff --git a/crates/bevy_transform/src/helper.rs b/crates/bevy_transform/src/helper.rs index 5194954fc07e6..3f4a23c71d87d 100644 --- a/crates/bevy_transform/src/helper.rs +++ b/crates/bevy_transform/src/helper.rs @@ -5,7 +5,7 @@ use bevy_ecs::{ hierarchy::ChildOf, prelude::Entity, query::QueryEntityError, - system::{Query, SystemParam, InfallibleSystemParam}, + system::{InfallibleSystemParam, Query, SystemParam}, }; use thiserror::Error; From 3bf5ae31d3361ba1f723a1c2f968390fef8ba482 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Thu, 12 Mar 2026 13:49:55 -0400 Subject: [PATCH 3/7] Fix typo. Surround lists with blank lines. --- release-content/migration-guides/validation_merging.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/release-content/migration-guides/validation_merging.md b/release-content/migration-guides/validation_merging.md index d76708b6d28bf..d8cb08b19b938 100644 --- a/release-content/migration-guides/validation_merging.md +++ b/release-content/migration-guides/validation_merging.md @@ -15,7 +15,7 @@ To be more precise: - 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 require `InfallibleSystemParam`. For fallible parameters, there are now `try_` variants that return a `Result<..., SystemParamValidationError>`. Callers that previously destructured the result of a fallible paramter directly will need to call `try_get` and add `.unwrap()` or handle the `Result`: +- `SystemState::fetch`, `get_unchecked`, `get` and `get_mut` now require `InfallibleSystemParam`. For fallible parameters, there are now `try_` variants that return a `Result<..., SystemParamValidationError>`. Callers that previously destructured the result of a fallible parameter directly will need to call `try_get` and add `.unwrap()` or handle the `Result`: ```rust // Before @@ -62,11 +62,13 @@ bubbling up any errors originating in `SystemParam::get_param`. If you have a custom `SystemParam` implementation, you need to either: If `validate_param` was overridden, because the parameter was sometimes invalid: + 1. Remove the `validate_param` method. 2. Move any validation logic into `get_param`. 3. Rename `get_param` to `try_get_param` and return `Result, SystemParamValidationError>`. If `validate_param` was not overridden, because the parameter was always valid: + 1. Add an `InfallibleSystemParam` impl 2. Move `get_param` to that impl 3. Create a `SystemParam::try_get_param` method that calls `get_param` From f532fefc013c39b688a741cbbad32ef5ce1e03be Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Thu, 12 Mar 2026 14:13:39 -0400 Subject: [PATCH 4/7] Fix extra error in compile-fail test. Fix broken doc link. --- .../compile_fail/tests/ui/system_param_derive_readonly.rs | 5 ++--- crates/bevy_ecs/src/system/system_param.rs | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/compile_fail/tests/ui/system_param_derive_readonly.rs b/crates/bevy_ecs/compile_fail/tests/ui/system_param_derive_readonly.rs index 84691f7a2704a..831494ebdb67c 100644 --- a/crates/bevy_ecs/compile_fail/tests/ui/system_param_derive_readonly.rs +++ b/crates/bevy_ecs/compile_fail/tests/ui/system_param_derive_readonly.rs @@ -1,16 +1,15 @@ use bevy_ecs::prelude::*; -use bevy_ecs::system::{ReadOnlySystemParam, SystemParam, SystemState}; +use bevy_ecs::system::{InfallibleSystemParam, ReadOnlySystemParam, SystemParam, SystemState}; #[derive(Component)] struct Foo; -#[derive(SystemParam)] +#[derive(SystemParam, InfallibleSystemParam)] struct Mutable<'w, 's> { a: Query<'w, 's, &'static mut Foo>, } fn main() { - let mut world = World::default(); let state = SystemState::::new(&mut world); state.get(&world); diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 872d253310d78..08dc42efbaf6f 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -292,7 +292,7 @@ pub unsafe trait SystemParam: Sized { /// It is always possible to create a query, even if it would not match anything. /// /// In contrast, [`Res`] and [`ResMut`] are fallible because the resource may not exist, -/// as are types like [`MessageReader`] that use resources internally. +/// as are types like [`MessageReader`](crate::message::MessageReader) that use resources internally. /// /// # Derive /// From 864c3a82893e95aad1d8eba3a6e0c47a2e33c280 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Sat, 14 Mar 2026 10:38:09 -0400 Subject: [PATCH 5/7] Mention `MessageMutator` as another solution to users with fallible parameters in `ParamSet`. --- release-content/migration-guides/validation_merging.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/release-content/migration-guides/validation_merging.md b/release-content/migration-guides/validation_merging.md index d8cb08b19b938..e83de50a3dd2c 100644 --- a/release-content/migration-guides/validation_merging.md +++ b/release-content/migration-guides/validation_merging.md @@ -51,6 +51,9 @@ switch to `try_get` or `try_pN` as described above. If the type is defined using `#[derive(SystemParam)]` and only includes infallible parameters, you may instead `#[derive(SystemParam, InfallibleSystemParam)]`. +If you were using `ParamSet<(MessageReader, MessageWriter)>` to send and receive messages in the same system, +you may instead use `MessageMutator`, which can read and write messages without needing `ParamSet` at all. + 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`. From 860af7224f6682f8a2037873dafd443e3914e4dc Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Sat, 14 Mar 2026 16:43:05 -0400 Subject: [PATCH 6/7] Apply suggestions from code review Co-authored-by: Kevin Chen --- crates/bevy_ecs/src/system/function_system.rs | 2 +- crates/bevy_ecs/src/system/system_param.rs | 2 +- release-content/migration-guides/validation_merging.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index cfa131479a633..4e6c27c3be3ef 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -181,7 +181,7 @@ impl SystemMeta { /// Query<&MyComponent>, /// )> = SystemState::new(&mut world); /// -/// // Use system_state.get_mut(&mut world) and unpack your system parameters into variables! +/// // Use system_state.try_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.try_get_mut(&mut world).unwrap(); /// diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 917832279e472..8676d7bd3348e 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -2257,7 +2257,7 @@ impl ParamSet<'_, '_, Vec> { /// Accesses the parameter at the given index. /// No other parameters may be accessed while this one is active. /// - /// To obtain the value of an in fallible [`SystemParam`] + /// To obtain the value of an infallible [`SystemParam`] /// without needing to unwrap a result, instead call [`Self::get_mut`]. pub fn try_get_mut( &mut self, diff --git a/release-content/migration-guides/validation_merging.md b/release-content/migration-guides/validation_merging.md index e83de50a3dd2c..d8b9d5b5f7335 100644 --- a/release-content/migration-guides/validation_merging.md +++ b/release-content/migration-guides/validation_merging.md @@ -144,7 +144,7 @@ impl InfallibleSystemParam for MyParam<'_> { change_tick: Tick, ) -> Self::Item<'w, 's> { // fetch logic - Ok(MyParam { /* ... */ }) + MyParam { /* ... */ } } } ``` From d43b6aabbecff641785c2ffaf8146db7d6c8dc99 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Sat, 14 Mar 2026 16:45:39 -0400 Subject: [PATCH 7/7] More doc fixes. --- examples/ecs/fallible_params.rs | 4 ++-- release-content/migration-guides/validation_merging.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/ecs/fallible_params.rs b/examples/ecs/fallible_params.rs index d70ae13bfe191..e0e8ca7486fa1 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::try_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::try_get_param`]: bevy::ecs::system::SystemParam::try_get_param use bevy::ecs::error::warn; use bevy::prelude::*; diff --git a/release-content/migration-guides/validation_merging.md b/release-content/migration-guides/validation_merging.md index d8b9d5b5f7335..6bfaf6578f6de 100644 --- a/release-content/migration-guides/validation_merging.md +++ b/release-content/migration-guides/validation_merging.md @@ -137,7 +137,7 @@ unsafe impl SystemParam for MyParam<'_> { impl InfallibleSystemParam for MyParam<'_> { // ... - unsafe fn param<'w, 's>( + unsafe fn get_param<'w, 's>( state: &'s mut Self::State, system_meta: &SystemMeta, world: UnsafeWorldCell<'w>, @@ -169,7 +169,7 @@ impl ExclusiveSystemParam for MyExclusiveParam { // After impl ExclusiveSystemParam for MyExclusiveParam { // ... - fn get_param<'s>( + fn try_get_param<'s>( state: &'s mut Self::State, system_meta: &SystemMeta, ) -> Result, SystemParamValidationError> {