InfallibleSystemParam

_release-content/migration-guides/validation_merging.md

@@ -7,23 +7,53 @@ 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<Self::Item<'world, 'state>, SystemParamValidationError>`, instead of simply a `Self::Item<'world, 'state>`
+- `SystemParam::get_param` has been renamed `try_param` and now returns a `Result<Self::Item<'world, 'state>, 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 parameter 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<U>)>) {
+    // 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<U>)>) -> 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)]`.
+
+If you were using `ParamSet<(MessageReader<M>, MessageWriter<M>)>` to send and receive messages in the same system,
+you may instead use `MessageMutator<M>`, 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`.
@@ -32,11 +62,19 @@ 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<Self::Item<'world, 'state>, SystemParamValidationError>`.
+3. Rename `get_param` to `try_get_param` and return `Result<Self::Item<'world, 'state>, 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 +103,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 +120,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<Self::Item<'w, 's>, 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 get_param<'w, 's>(
+        state: &'s mut Self::State,
+        system_meta: &SystemMeta,
+        world: UnsafeWorldCell<'w>,
+        change_tick: Tick,
+    ) -> Self::Item<'w, 's> {
+        // fetch logic
+        MyParam { /* ... */ }
+    }
+}
 ```
 
 ## Custom `ExclusiveSystemParam` implementations
@@ -104,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<Self::Item<'s>, SystemParamValidationError> {

benches/benches/bevy_ecs/fragmentation.rs

@@ -25,7 +25,7 @@ fn iter_frag_empty(c: &mut Criterion) {
         spawn_empty_frag_archetype::<Table>(&mut world);
         let mut q: SystemState<Query<(Entity, &Table)>> =
             SystemState::<Query<(Entity, &Table<0>)>>::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::<Sparse>(&mut world);
         let mut q: SystemState<Query<(Entity, &Sparse)>> =
             SystemState::<Query<(Entity, &Sparse<0>)>>::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)| {

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::<Query<&Table>>::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::<Query<&Sparse>>::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<const N: usize>(criterion: &mut Criterion) {
             entity_groups.shuffle(&mut deterministic_rand());
 
             let mut query = SystemState::<Query<&Table>>::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<const N: usize>(criterion: &mut Criterion) {
             entity_groups.shuffle(&mut deterministic_rand());
 
             let mut query = SystemState::<Query<&Sparse>>::new(&mut world);
-            let query = query.get(&world).unwrap();
+            let query = query.get(&world);
 
             bencher.iter(|| {
                 let mut count = 0;

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)>,
 }

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();

crates/bevy_ecs/compile_fail/tests/ui/query_lifetime_safety.rs

@@ -10,7 +10,7 @@ fn main() {
 
     let mut system_state = SystemState::<Query<&mut Foo>>::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();

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::<Query<(&mut Foo, &Bar)>>::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>();

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::<Mutable>::new(&mut world);
     state.get(&world);

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();
 

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();
 

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::<Query<&mut A>>::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::<Vec<bevy_ecs::prelude::Mut<A>>>();
         assert_eq!(
             // this should fail to compile due to the later use of mut_vec

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::<Self>(err.skipped, #field_validation_messages, #field_validation_names))?;
                     )*
                     Result::Ok(#struct_name {
@@ -480,6 +480,65 @@ 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<TokenStream> {
+    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::<Vec<_>>();
+    let field_members = fields.members().collect::<Vec<_>>();
+    let field_types = fields.iter().map(|f| &f.ty).collect::<Vec<_>>();
+
+    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 {

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<T>>);
 
 impl<T: Component> ComponentIdFor<'_, T> {