Skip to content

Commit

Permalink
Make the SystemParam derive macro more flexible (#6694)
Browse files Browse the repository at this point in the history
# Objective

Currently, the `SystemParam` derive forces you to declare the lifetime parameters `<'w, 's>`, even if you don't use them.
If you don't follow this structure, the error message is quite nasty.

### Example (before):

```rust
#[derive(SystemParam)]
pub struct EventWriter<'w, 's, E: Event> {
    events: ResMut<'w, Events<E>>,
    // The derive forces us to declare the `'s` lifetime even though we don't use it,
    // so we have to add this `PhantomData` to please rustc.
    #[system_param(ignore)]
    _marker: PhantomData<&'s ()>,
}
```


## Solution

* Allow the user to omit either lifetime.
* Emit a descriptive error if any lifetimes used are invalid.

### Example (after):

```rust
#[derive(SystemParam)]
pub struct EventWriter<'w, E: Event> {
    events: ResMut<'w, Events<E>>,
}
```

---

## Changelog

* The `SystemParam` derive is now more flexible, allowing you to omit unused lifetime parameters.
  • Loading branch information
JoJoJet committed Dec 5, 2022
1 parent c55d553 commit 05b498a
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 21 deletions.
32 changes: 28 additions & 4 deletions crates/bevy_ecs/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,25 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
}

let generics = ast.generics;
let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();

// Emit an error if there's any unrecognized lifetime names.
for lt in generics.lifetimes() {
let ident = &lt.lifetime.ident;
let w = format_ident!("w");
let s = format_ident!("s");
if ident != &w && ident != &s {
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();
}
}

let (_impl_generics, ty_generics, where_clause) = generics.split_for_impl();

let lifetimeless_generics: Vec<_> = generics
.params
Expand Down Expand Up @@ -404,10 +422,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
// <EventReader<'static, 'static, T> 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 <TSystemParamState, #punctuated_generic_idents> {
state: TSystemParamState,
Expand All @@ -431,7 +455,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,
Expand Down
6 changes: 2 additions & 4 deletions crates/bevy_ecs/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<E>>,
#[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) {
Expand Down
35 changes: 22 additions & 13 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,15 @@ 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.
/// 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
///
/// `#[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
///
Expand All @@ -57,17 +53,17 @@ 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<Marker>,
/// }
///
/// fn my_system(param: MyParam) {
/// fn my_system<T: 'static>(param: MyParam<T>) {
/// // 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
Expand Down Expand Up @@ -1567,7 +1563,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 in this type, or they will be unbound.
/// phantom: core::marker::PhantomData<&'w &'s ()>
/// }
/// # fn check_always_is_system<T: SystemParam + 'static>(){
Expand Down Expand Up @@ -1651,7 +1647,7 @@ unsafe impl<S: SystemParamState, P: SystemParam + 'static> 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},
Expand All @@ -1668,4 +1664,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> {
_local: Local<'s, T>,
}

#[derive(SystemParam)]
pub struct UnitParam {}
}

0 comments on commit 05b498a

Please sign in to comment.