From 2e5ce49c2935d39743de9a02222c3ee580a769f7 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Sat, 19 Nov 2022 12:02:28 -0500 Subject: [PATCH 01/10] relax restrictions on `SystemParam` lifetimes --- crates/bevy_ecs/macros/src/lib.rs | 12 +++++++++--- crates/bevy_ecs/src/system/system_param.rs | 22 ++++++++++++++++------ 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index f67e58cf19be6..6721386578b6f 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -404,10 +404,16 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { // The struct can still be accessed via SystemParam::Fetch, e.g. EventReaderState can be accessed via // as SystemParam>::Fetch const _: () = { - impl #impl_generics #path::system::SystemParam for #struct_name #ty_generics #where_clause { - type Fetch = FetchState <(#(<#field_types as #path::system::SystemParam>::Fetch,)*), #punctuated_generic_idents>; + impl<'w, 's, #punctuated_generics> #path::system::SystemParam for #struct_name #ty_generics #where_clause { + type Fetch = State<'w, 's, #punctuated_generic_idents>; } + #[doc(hidden)] + type State<'w, 's, #punctuated_generic_idents> = FetchState< + (#(<#field_types as #path::system::SystemParam>::Fetch,)*), + #punctuated_generic_idents + >; + #[doc(hidden)] #fetch_struct_visibility struct FetchState { state: TSystemParamState, @@ -431,7 +437,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { } } - impl #impl_generics #path::system::SystemParamFetch<'w, 's> for FetchState <(#(<#field_types as #path::system::SystemParam>::Fetch,)*), #punctuated_generic_idents> #where_clause { + impl<'w, 's, #punctuated_generics> #path::system::SystemParamFetch<'w, 's> for State<'w, 's, #punctuated_generic_idents> #where_clause { type Item = #struct_name #ty_generics; unsafe fn get_param( state: &'s mut Self, diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index acfdd8f7224e2..eb153137c330b 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -33,11 +33,8 @@ use std::{ /// See the *Generic `SystemParam`s* section for details and workarounds of the probable /// cause if this derive causes an error to be emitted. /// -/// -/// The struct for which `SystemParam` is derived must (currently) have exactly -/// two lifetime parameters. -/// The first is the lifetime of the world, and the second the lifetime -/// of the parameter's state. +/// Most `SystemParam` structs will have two lifetimes: `'w` for data stored in the [`World`], +/// and `'s` for data stored in the parameter's state. /// /// ## Attributes /// @@ -1648,7 +1645,7 @@ unsafe impl SystemParamState #[cfg(test)] mod tests { - use super::SystemParam; + use super::*; use crate::{ self as bevy_ecs, // Necessary for the `SystemParam` Derive when used inside `bevy_ecs`. query::{ReadOnlyWorldQuery, WorldQuery}, @@ -1665,4 +1662,17 @@ mod tests { > { _query: Query<'w, 's, Q, F>, } + + #[derive(SystemParam)] + pub struct SpecialRes<'w, T: Resource> { + _res: Res<'w, T>, + } + + #[derive(SystemParam)] + pub struct SpecialLocal<'s, T: FromWorld + Send + 'static> { + _res: Local<'s, T>, + } + + #[derive(SystemParam)] + pub struct UnitParam {} } From 89cc5c6eed918ef912d63d5564dde6758dfe37b4 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Sat, 19 Nov 2022 12:04:39 -0500 Subject: [PATCH 02/10] update some examples --- crates/bevy_ecs/src/system/system_param.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index eb153137c330b..b999e46ff2e4a 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -41,8 +41,7 @@ use std::{ /// `#[system_param(ignore)]`: /// Can be added to any field in the struct. Fields decorated with this attribute /// will be created with the default value upon realisation. -/// This is most useful for `PhantomData` fields, to ensure that the required lifetimes are -/// used, as shown in the example. +/// This is most useful for `PhantomData` fields, such as markers for generic types. /// /// # Example /// @@ -54,13 +53,13 @@ use std::{ /// use bevy_ecs::system::SystemParam; /// /// #[derive(SystemParam)] -/// struct MyParam<'w, 's> { +/// struct MyParam<'w, Marker: 'static> { /// foo: Res<'w, SomeResource>, /// #[system_param(ignore)] -/// marker: PhantomData<&'s ()>, +/// marker: PhantomData, /// } /// -/// fn my_system(param: MyParam) { +/// fn my_system(param: MyParam) { /// // Access the resource through `param.foo` /// } /// @@ -1561,7 +1560,7 @@ pub mod lifetimeless { /// struct GenericParam<'w,'s, T: SystemParam> { /// field: T, /// #[system_param(ignore)] -/// // Use the lifetimes, as the `SystemParam` derive requires them +/// // Use the lifetimes, or rustc will get angry. /// phantom: core::marker::PhantomData<&'w &'s ()> /// } /// # fn check_always_is_system(){ From 9a8e8bd633bb1f023790499c7af9ec8f12c2ac67 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Sat, 19 Nov 2022 12:04:59 -0500 Subject: [PATCH 03/10] remove the extra lifetime from `EventWriter` --- crates/bevy_ecs/src/event.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/event.rs b/crates/bevy_ecs/src/event.rs index 1c46c90dc0353..762d791b7d3c3 100644 --- a/crates/bevy_ecs/src/event.rs +++ b/crates/bevy_ecs/src/event.rs @@ -295,13 +295,11 @@ impl<'w, 's, E: Event> EventReader<'w, 's, E> { /// ``` /// Note that this is considered *non-idiomatic*, and should only be used when `EventWriter` will not work. #[derive(SystemParam)] -pub struct EventWriter<'w, 's, E: Event> { +pub struct EventWriter<'w, E: Event> { events: ResMut<'w, Events>, - #[system_param(ignore)] - marker: PhantomData<&'s usize>, } -impl<'w, 's, E: Event> EventWriter<'w, 's, E> { +impl<'w, E: Event> EventWriter<'w, E> { /// Sends an `event`. [`EventReader`]s can then read the event. /// See [`Events`] for details. pub fn send(&mut self, event: E) { From 20bb726f807706e8d7d88139225b962d51a87b9d Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Sat, 19 Nov 2022 12:13:59 -0500 Subject: [PATCH 04/10] silence a lint --- crates/bevy_ecs/macros/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 6721386578b6f..c41a605121471 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -373,7 +373,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { } let generics = ast.generics; - let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + let (_impl_generics, ty_generics, where_clause) = generics.split_for_impl(); let lifetimeless_generics: Vec<_> = generics .params From 8d0017c52663d502eb36dd9982697496ec12089f Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Sat, 19 Nov 2022 12:23:23 -0500 Subject: [PATCH 05/10] use marker --- crates/bevy_ecs/src/system/system_param.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index b999e46ff2e4a..347145b181e59 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -63,7 +63,7 @@ use std::{ /// // Access the resource through `param.foo` /// } /// -/// # bevy_ecs::system::assert_is_system(my_system); +/// # bevy_ecs::system::assert_is_system(my_system::<()>); /// ``` /// /// # Generic `SystemParam`s From fb27c480f5826d2df87a360f76910ea0fac0e620 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Sat, 19 Nov 2022 12:31:36 -0500 Subject: [PATCH 06/10] Clarify a limitation --- crates/bevy_ecs/src/system/system_param.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 347145b181e59..63f63753f5278 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -33,7 +33,7 @@ use std::{ /// See the *Generic `SystemParam`s* section for details and workarounds of the probable /// cause if this derive causes an error to be emitted. /// -/// Most `SystemParam` structs will have two lifetimes: `'w` for data stored in the [`World`], +/// Derived `SystemParam` structs may have two lifetimes: `'w` for data stored in the [`World`], /// and `'s` for data stored in the parameter's state. /// /// ## Attributes From 02613c591f8f88684e5db23549e18e1d53ec50fb Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Sat, 19 Nov 2022 12:43:46 -0500 Subject: [PATCH 07/10] improve the error message when lifetimes are incorrect --- crates/bevy_ecs/macros/src/lib.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index c41a605121471..f840554f0f153 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -373,6 +373,19 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { } let generics = ast.generics; + + // Emit an error if there's any unrecognized lifetime names. + for lt in generics.lifetimes() { + let ident = <.lifetime.ident; + let w = format_ident!("w"); + let s = format_ident!("s"); + if ident != &w && ident != &s { + return syn::Error::new_spanned(lt, "invalid lifetime name: expected `'w` or `'s`") + .into_compile_error() + .into(); + } + } + let (_impl_generics, ty_generics, where_clause) = generics.split_for_impl(); let lifetimeless_generics: Vec<_> = generics From d122be94452056e721901417f1db3d51eedab27f Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Sat, 19 Nov 2022 12:53:40 -0500 Subject: [PATCH 08/10] rename a field --- crates/bevy_ecs/src/system/system_param.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 63f63753f5278..2715a766a5494 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1669,7 +1669,7 @@ mod tests { #[derive(SystemParam)] pub struct SpecialLocal<'s, T: FromWorld + Send + 'static> { - _res: Local<'s, T>, + _local: Local<'s, T>, } #[derive(SystemParam)] From 1d25b81d19ebca5d4ecf1b7eae149faa89d07119 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Sat, 19 Nov 2022 13:34:14 -0500 Subject: [PATCH 09/10] rewrite a comment --- crates/bevy_ecs/src/system/system_param.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 2715a766a5494..61516444e0f3c 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1560,7 +1560,7 @@ pub mod lifetimeless { /// struct GenericParam<'w,'s, T: SystemParam> { /// field: T, /// #[system_param(ignore)] -/// // Use the lifetimes, or rustc will get angry. +/// // Use the lifetimes in this type, or they will be unbound. /// phantom: core::marker::PhantomData<&'w &'s ()> /// } /// # fn check_always_is_system(){ From 679fd1cda7096b1f42870a4dbc63f670b732ef83 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Mon, 28 Nov 2022 15:59:20 -0500 Subject: [PATCH 10/10] add more information to lifetime errors --- crates/bevy_ecs/macros/src/lib.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index f840554f0f153..53121a6c7f883 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -380,9 +380,14 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { let w = format_ident!("w"); let s = format_ident!("s"); if ident != &w && ident != &s { - return syn::Error::new_spanned(lt, "invalid lifetime name: expected `'w` or `'s`") - .into_compile_error() - .into(); + return syn::Error::new_spanned( + lt, + r#"invalid lifetime name: expected `'w` or `'s` + 'w -- refers to data stored in the World. + 's -- refers to data stored in the SystemParam's state.'"#, + ) + .into_compile_error() + .into(); } }