Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Make the SystemParam derive macro more flexible #6694

Closed
wants to merge 11 commits into from
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();
Comment on lines +383 to +390
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@james7132 do you think this is an improvement?

This usage is called out in the docs, but IMO the best way of teaching this is to just let users do it wrong, then have the error tell them the right way of doing it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks much better to me. Thanks!

}
}

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 {}
}