forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Remove #[system_param(ignore)] and #[world_query(ignore)] (#8265)

Browse Source 
# Objective

Follow-up to #8030.

Now that `SystemParam` and `WorldQuery` are implemented for
`PhantomData`, the `ignore` attributes are now unnecessary.

---

## Changelog

- Removed the attributes `#[system_param(ignore)]` and
`#[world_query(ignore)]`.

## Migration Guide

The attributes `#[system_param(ignore)]` and `#[world_query]` ignore
have been removed. If you were using either of these with `PhantomData`
fields, you can simply remove the attribute:

```rust
#[derive(SystemParam)]
struct MyParam<'w, 's, Marker> {
    ...
    // Before:
    #[system_param(ignore)
    _marker: PhantomData<Marker>,

    // After:
    _marker: PhantomData<Marker>,
}
#[derive(WorldQuery)]
struct MyQuery<Marker> {
    ...
    // Before:
    #[world_query(ignore)
    _marker: PhantomData<Marker>,

    // After:
    _marker: PhantomData<Marker>,
}
```

If you were using this for another type that implements `Default`,
consider wrapping that type in `Local<>` (this only works for
`SystemParam`):

```rust
#[derive(SystemParam)]
struct MyParam<'w, 's> {
    // Before:
    #[system_param(ignore)]
    value: MyDefaultType, // This will be initialized using `Default` each time `MyParam` is created.

    // After:
    value: Local<MyDefaultType>, // This will be initialized using `Default` the first time `MyParam` is created.
}
```

If you are implementing either trait and need to preserve the exact
behavior of the old `ignore` attributes, consider manually implementing
`SystemParam` or `WorldQuery` for a wrapper struct that uses the
`Default` trait:

```rust
// Before:

#[derive(WorldQuery)
struct MyQuery {
   #[world_query(ignore)]
    str: String,
}

// After:

#[derive(WorldQuery)
struct MyQuery {
    str: DefaultQuery<String>,
}

pub struct DefaultQuery<T: Default>(pub T);

unsafe impl<T: Default> WorldQuery for DefaultQuery<T> {
    type Item<'w> = Self;
    ...
    unsafe fn fetch<'w>(...) -> Self::Item<'w> {
        Self(T::default())
    }
}
```
This commit is contained in:
JoJoJet 2023-03-30 19:02:25 -04:00 committed by GitHub
parent 98954311b3
commit a954f3e150
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 45 additions and 158 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

98
crates/bevy_ecs/macros/src/fetch.rs
View File

@ -129,10 +129,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
_ => panic!("Expected a struct with named fields"), _ => panic!("Expected a struct with named fields"),
}; };
let mut ignored_field_attrs = Vec::new();
let mut ignored_field_visibilities = Vec::new();
let mut ignored_field_idents = Vec::new();
let mut ignored_field_types = Vec::new();
let mut field_attrs = Vec::new(); let mut field_attrs = Vec::new();
let mut field_visibilities = Vec::new(); let mut field_visibilities = Vec::new();
let mut field_idents = Vec::new(); let mut field_idents = Vec::new();
@ -140,22 +136,17 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
let mut read_only_field_types = Vec::new(); let mut read_only_field_types = Vec::new();
for field in fields { for field in fields {
let WorldQueryFieldInfo { is_ignored, attrs } = read_world_query_field_info(field); let attrs = match read_world_query_field_info(field) {
Ok(WorldQueryFieldInfo { attrs }) => attrs,
Err(e) => return e.into_compile_error().into(),
};
let field_ident = field.ident.as_ref().unwrap().clone(); field_attrs.push(attrs);
if is_ignored { field_visibilities.push(field.vis.clone());
ignored_field_attrs.push(attrs); field_idents.push(field.ident.as_ref().unwrap().clone());
ignored_field_visibilities.push(field.vis.clone()); let field_ty = field.ty.clone();
ignored_field_idents.push(field_ident.clone()); field_types.push(quote!(#field_ty));
ignored_field_types.push(field.ty.clone()); read_only_field_types.push(quote!(<#field_ty as #path::query::WorldQuery>::ReadOnly));
} else {
field_attrs.push(attrs);
field_visibilities.push(field.vis.clone());
field_idents.push(field_ident.clone());
let field_ty = field.ty.clone();
field_types.push(quote!(#field_ty));
read_only_field_types.push(quote!(<#field_ty as #path::query::WorldQuery>::ReadOnly));
}
} }
let derive_args = &fetch_struct_attributes.derive_args; let derive_args = &fetch_struct_attributes.derive_args;
@ -193,7 +184,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
#[automatically_derived] #[automatically_derived]
#visibility struct #item_struct_name #user_impl_generics_with_world #user_where_clauses_with_world { #visibility struct #item_struct_name #user_impl_generics_with_world #user_where_clauses_with_world {
#(#(#field_attrs)* #field_visibilities #field_idents: <#field_types as #path::query::WorldQuery>::Item<'__w>,)* #(#(#field_attrs)* #field_visibilities #field_idents: <#field_types as #path::query::WorldQuery>::Item<'__w>,)*
#(#(#ignored_field_attrs)* #ignored_field_visibilities #ignored_field_idents: #ignored_field_types,)*
} }
}; };
@ -205,7 +195,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
#[automatically_derived] #[automatically_derived]
#visibility struct #fetch_struct_name #user_impl_generics_with_world #user_where_clauses_with_world { #visibility struct #fetch_struct_name #user_impl_generics_with_world #user_where_clauses_with_world {
#(#field_idents: <#field_types as #path::query::WorldQuery>::Fetch<'__w>,)* #(#field_idents: <#field_types as #path::query::WorldQuery>::Fetch<'__w>,)*
#(#ignored_field_idents: #ignored_field_types,)*
} }
// SAFETY: `update_component_access` and `update_archetype_component_access` are called on every field // SAFETY: `update_component_access` and `update_archetype_component_access` are called on every field
@ -224,9 +213,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
#( #(
#field_idents: <#field_types>::shrink(item.#field_idents), #field_idents: <#field_types>::shrink(item.#field_idents),
)* )*
#(
#ignored_field_idents: item.#ignored_field_idents,
)*
} }
} }
@ -245,7 +231,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
_this_run, _this_run,
), ),
)* )*
#(#ignored_field_idents: Default::default(),)*
} }
} }
@ -256,9 +241,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
#( #(
#field_idents: <#field_types>::clone_fetch(& _fetch. #field_idents), #field_idents: <#field_types>::clone_fetch(& _fetch. #field_idents),
)* )*
#(
#ignored_field_idents: Default::default(),
)*
} }
} }
@ -296,7 +278,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
) -> <Self as #path::query::WorldQuery>::Item<'__w> { ) -> <Self as #path::query::WorldQuery>::Item<'__w> {
Self::Item { Self::Item {
#(#field_idents: <#field_types>::fetch(&mut _fetch.#field_idents, _entity, _table_row),)* #(#field_idents: <#field_types>::fetch(&mut _fetch.#field_idents, _entity, _table_row),)*
#(#ignored_field_idents: Default::default(),)*
} }
} }
@ -327,7 +308,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
fn init_state(world: &mut #path::world::World) -> #state_struct_name #user_ty_generics { fn init_state(world: &mut #path::world::World) -> #state_struct_name #user_ty_generics {
#state_struct_name { #state_struct_name {
#(#field_idents: <#field_types>::init_state(world),)* #(#field_idents: <#field_types>::init_state(world),)*
#(#ignored_field_idents: Default::default(),)*
} }
} }
@ -349,7 +329,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
#[automatically_derived] #[automatically_derived]
#visibility struct #read_only_struct_name #user_impl_generics #user_where_clauses { #visibility struct #read_only_struct_name #user_impl_generics #user_where_clauses {
#( #field_visibilities #field_idents: #read_only_field_types, )* #( #field_visibilities #field_idents: #read_only_field_types, )*
#(#(#ignored_field_attrs)* #ignored_field_visibilities #ignored_field_idents: #ignored_field_types,)*
} }
#readonly_state #readonly_state
@ -396,7 +375,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
#[automatically_derived] #[automatically_derived]
#visibility struct #state_struct_name #user_impl_generics #user_where_clauses { #visibility struct #state_struct_name #user_impl_generics #user_where_clauses {
#(#field_idents: <#field_types as #path::query::WorldQuery>::State,)* #(#field_idents: <#field_types as #path::query::WorldQuery>::State,)*
#(#ignored_field_idents: ::std::marker::PhantomData<fn() -> #ignored_field_types>,)*
} }
#mutable_impl #mutable_impl
@ -428,56 +406,32 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
q2: #read_only_struct_name #user_ty_generics q2: #read_only_struct_name #user_ty_generics
) #user_where_clauses { ) #user_where_clauses {
#(q.#field_idents;)* #(q.#field_idents;)*
#(q.#ignored_field_idents;)*
#(q2.#field_idents;)* #(q2.#field_idents;)*
#(q2.#ignored_field_idents;)*
} }
}; };
}) })
} }
struct WorldQueryFieldInfo { struct WorldQueryFieldInfo {
/// Has the `#[world_query(ignore)]` attribute.
is_ignored: bool,
/// All field attributes except for `world_query` ones. /// All field attributes except for `world_query` ones.
attrs: Vec<Attribute>, attrs: Vec<Attribute>,
} }
fn read_world_query_field_info(field: &Field) -> WorldQueryFieldInfo { fn read_world_query_field_info(field: &Field) -> syn::Result<WorldQueryFieldInfo> {
let is_ignored = field let mut attrs = Vec::new();
.attrs for attr in &field.attrs {
.iter() if attr
.find(|attr| { .path
attr.path .get_ident()
.get_ident() .map_or(false, |ident| ident == WORLD_QUERY_ATTRIBUTE_NAME)
.map_or(false, |ident| ident == WORLD_QUERY_ATTRIBUTE_NAME) {
}) return Err(syn::Error::new_spanned(
.map_or(false, |attr| { attr,
let mut is_ignored = false; "#[derive(WorldQuery)] does not support field attributes.",
attr.parse_args_with(|input: ParseStream| { ));
if input }
.parse::<Option<field_attr_keywords::ignore>>()? attrs.push(attr.clone());
.is_some() }
{
is_ignored = true;
}
Ok(())
})
.unwrap_or_else(|_| panic!("Invalid `{WORLD_QUERY_ATTRIBUTE_NAME}` attribute format"));
is_ignored Ok(WorldQueryFieldInfo { attrs })
});
let attrs = field
.attrs
.iter()
.filter(|attr| {
attr.path
.get_ident()
.map_or(true, |ident| ident != WORLD_QUERY_ATTRIBUTE_NAME)
})
.cloned()
.collect();
WorldQueryFieldInfo { is_ignored, attrs }
} }

75
crates/bevy_ecs/macros/src/lib.rs
View File

@ -13,9 +13,8 @@ use proc_macro::TokenStream;
use proc_macro2::Span; use proc_macro2::Span;
use quote::{format_ident, quote}; use quote::{format_ident, quote};
use syn::{ use syn::{
parse::ParseStream, parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned, parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned, ConstParam,
ConstParam, DeriveInput, GenericParam, Ident, Index, Meta, MetaList, NestedMeta, Token, DeriveInput, GenericParam, Ident, Index, Meta, MetaList, NestedMeta, Token, TypeParam,
TypeParam,
}; };
enum BundleFieldKind { enum BundleFieldKind {
@ -252,13 +251,6 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream {
tokens tokens
} }
#[derive(Default)]
struct SystemParamFieldAttributes {
pub ignore: bool,
}
static SYSTEM_PARAM_ATTRIBUTE_NAME: &str = "system_param";
/// Implement `SystemParam` to use a struct as a parameter in a system /// Implement `SystemParam` to use a struct as a parameter in a system
#[proc_macro_derive(SystemParam, attributes(system_param))] #[proc_macro_derive(SystemParam, attributes(system_param))]
pub fn derive_system_param(input: TokenStream) -> TokenStream { pub fn derive_system_param(input: TokenStream) -> TokenStream {
@ -271,53 +263,20 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
}; };
let path = bevy_ecs_path(); let path = bevy_ecs_path();
let field_attributes = field_definitions
.iter()
.map(|field| {
(
field,
field
.attrs
.iter()
.find(|a| *a.path.get_ident().as_ref().unwrap() == SYSTEM_PARAM_ATTRIBUTE_NAME)
.map_or_else(SystemParamFieldAttributes::default, |a| {
syn::custom_keyword!(ignore);
let mut attributes = SystemParamFieldAttributes::default();
a.parse_args_with(|input: ParseStream| {
if input.parse::<Option<ignore>>()?.is_some() {
attributes.ignore = true;
}
Ok(())
})
.expect("Invalid 'system_param' attribute format.");
attributes
}),
)
})
.collect::<Vec<_>>();
let mut field_locals = Vec::new(); let mut field_locals = Vec::new();
let mut fields = Vec::new(); let mut fields = Vec::new();
let mut field_types = Vec::new(); let mut field_types = Vec::new();
let mut ignored_fields = Vec::new(); for (i, field) in field_definitions.iter().enumerate() {
let mut ignored_field_types = Vec::new(); field_locals.push(format_ident!("f{i}"));
for (i, (field, attrs)) in field_attributes.iter().enumerate() { let i = Index::from(i);
if attrs.ignore { fields.push(
ignored_fields.push(field.ident.as_ref().unwrap()); field
ignored_field_types.push(&field.ty); .ident
} else { .as_ref()
field_locals.push(format_ident!("f{i}")); .map(|f| quote! { #f })
let i = Index::from(i); .unwrap_or_else(|| quote! { #i }),
fields.push( );
field field_types.push(&field.ty);
.ident
.as_ref()
.map(|f| quote! { #f })
.unwrap_or_else(|| quote! { #i }),
);
field_types.push(&field.ty);
}
} }
let generics = ast.generics; let generics = ast.generics;
@ -383,13 +342,6 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
let mut tuple_types: Vec<_> = field_types.iter().map(|x| quote! { #x }).collect(); let mut tuple_types: Vec<_> = field_types.iter().map(|x| quote! { #x }).collect();
let mut tuple_patterns: Vec<_> = field_locals.iter().map(|x| quote! { #x }).collect(); let mut tuple_patterns: Vec<_> = field_locals.iter().map(|x| quote! { #x }).collect();
tuple_types.extend(
ignored_field_types
.iter()
.map(|ty| parse_quote!(::std::marker::PhantomData::<#ty>)),
);
tuple_patterns.extend(ignored_field_types.iter().map(|_| parse_quote!(_)));
// If the number of fields exceeds the 16-parameter limit, // If the number of fields exceeds the 16-parameter limit,
// fold the fields into tuples of tuples until we are below the limit. // fold the fields into tuples of tuples until we are below the limit.
const LIMIT: usize = 16; const LIMIT: usize = 16;
@ -463,7 +415,6 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
>::get_param(&mut state.state, system_meta, world, change_tick); >::get_param(&mut state.state, system_meta, world, change_tick);
#struct_name { #struct_name {
#(#fields: #field_locals,)* #(#fields: #field_locals,)*
#(#ignored_fields: std::default::Default::default(),)*
} }
} }
} }

10
crates/bevy_ecs/src/query/fetch.rs
View File

@ -55,13 +55,7 @@ use std::{cell::UnsafeCell, marker::PhantomData};
/// - Methods can be implemented for the query items. /// - Methods can be implemented for the query items.
/// - There is no hardcoded limit on the number of elements. /// - There is no hardcoded limit on the number of elements.
/// ///
/// This trait can only be derived if each field either /// This trait can only be derived if each field also implements `WorldQuery`.
///
/// * also implements `WorldQuery`, or
/// * is marked with `#[world_query(ignore)]`. Fields decorated with this attribute
/// must implement [`Default`] and will be initialized to the default value as defined
/// by the trait.
///
/// The derive macro only supports regular structs (structs with named fields). /// The derive macro only supports regular structs (structs with named fields).
/// ///
/// ``` /// ```
@ -1485,9 +1479,7 @@ mod tests {
#[derive(WorldQuery)] #[derive(WorldQuery)]
pub struct IgnoredQuery<Marker> { pub struct IgnoredQuery<Marker> {
id: Entity, id: Entity,
#[world_query(ignore)]
_marker: PhantomData<Marker>, _marker: PhantomData<Marker>,
_marker2: PhantomData<Marker>,
} }
fn ignored_system(_: Query<IgnoredQuery<()>>) {} fn ignored_system(_: Query<IgnoredQuery<()>>) {}

20
crates/bevy_ecs/src/system/system_param.rs
View File

@ -36,12 +36,10 @@ use std::{
/// Derived `SystemParam` structs may 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. /// and `'s` for data stored in the parameter's state.
/// ///
/// ## Attributes /// ## `PhantomData`
/// ///
/// `#[system_param(ignore)]`: /// [`PhantomData`] is a special type of `SystemParam` that does nothing.
/// Can be added to any field in the struct. Fields decorated with this attribute /// This is useful for constraining generic types or lifetimes.
/// will be created with the default value upon realisation.
/// This is most useful for `PhantomData` fields, such as markers for generic types.
/// ///
/// # Example /// # Example
/// ///
@ -55,7 +53,6 @@ use std::{
/// #[derive(SystemParam)] /// #[derive(SystemParam)]
/// struct MyParam<'w, Marker: 'static> { /// struct MyParam<'w, Marker: 'static> {
/// foo: Res<'w, SomeResource>, /// foo: Res<'w, SomeResource>,
/// #[system_param(ignore)]
/// marker: PhantomData<Marker>, /// marker: PhantomData<Marker>,
/// } /// }
/// ///
@ -66,11 +63,6 @@ use std::{
/// # bevy_ecs::system::assert_is_system(my_system::<()>); /// # bevy_ecs::system::assert_is_system(my_system::<()>);
/// ``` /// ```
/// ///
/// ## `PhantomData`
///
/// [`PhantomData`] is a special type of `SystemParam` that does nothing.
/// This is useful for constraining generic types or lifetimes.
///
/// # Generic `SystemParam`s /// # Generic `SystemParam`s
/// ///
/// When using the derive macro, you may see an error in the form of: /// When using the derive macro, you may see an error in the form of:
@ -1652,14 +1644,12 @@ mod tests {
#[test] #[test]
fn system_param_phantom_data() { fn system_param_phantom_data() {
#[derive(SystemParam)] #[derive(SystemParam)]
struct IgnoredParam<'w, T: Resource, Marker: 'static> { struct PhantomParam<'w, T: Resource, Marker: 'static> {
_foo: Res<'w, T>, _foo: Res<'w, T>,
#[system_param(ignore)]
marker: PhantomData<&'w Marker>, marker: PhantomData<&'w Marker>,
marker2: PhantomData<&'w Marker>,
} }
fn my_system(_: IgnoredParam<R<0>, ()>) {} fn my_system(_: PhantomParam<R<0>, ()>) {}
assert_is_system(my_system); assert_is_system(my_system);
} }