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.

Sign up for GitHub

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

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

Merged
merged 3 commits into from
Mar 30, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e90e495
remove `#[system_param(ignore)]`
joseph-gio Mar 30, 2023
d647fb0
remove `#[world_query(ignore)]`
joseph-gio Mar 30, 2023
f1c4212
Merge remote-tracking branch 'upstream/main' into yeet-ignore
joseph-gio Mar 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 27 additions & 73 deletions crates/bevy_ecs/macros/src/fetch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,33 +129,24 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
_ => 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_visibilities = Vec::new();
let mut field_idents = Vec::new();
let mut field_types = Vec::new();
let mut read_only_field_types = Vec::new();

for field in fields {
let WorldQueryFieldInfo { is_ignored, attrs } = read_world_query_field_info(field);

let field_ident = field.ident.as_ref().unwrap().clone();
if is_ignored {
ignored_field_attrs.push(attrs);
ignored_field_visibilities.push(field.vis.clone());
ignored_field_idents.push(field_ident.clone());
ignored_field_types.push(field.ty.clone());
} 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 attrs = match read_world_query_field_info(field) {
Ok(WorldQueryFieldInfo { attrs }) => attrs,
Err(e) => return e.into_compile_error().into(),
};

field_attrs.push(attrs);
field_visibilities.push(field.vis.clone());
field_idents.push(field.ident.as_ref().unwrap().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;
Expand Down Expand Up @@ -193,7 +184,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
#[automatically_derived]
#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>,)*
#(#(#ignored_field_attrs)* #ignored_field_visibilities #ignored_field_idents: #ignored_field_types,)*
}
};

Expand All @@ -205,7 +195,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
#[automatically_derived]
#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>,)*
#(#ignored_field_idents: #ignored_field_types,)*
}

// SAFETY: `update_component_access` and `update_archetype_component_access` are called on every field
Expand All @@ -224,9 +213,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
#(
#field_idents: <#field_types>::shrink(item.#field_idents),
)*
#(
#ignored_field_idents: item.#ignored_field_idents,
)*
}
}

Expand All @@ -245,7 +231,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
_this_run,
),
)*
#(#ignored_field_idents: Default::default(),)*
}
}

Expand All @@ -256,9 +241,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
#(
#field_idents: <#field_types>::clone_fetch(& _fetch. #field_idents),
)*
#(
#ignored_field_idents: Default::default(),
)*
}
}

Expand Down Expand Up @@ -296,7 +278,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
) -> <Self as #path::query::WorldQuery>::Item<'__w> {
Self::Item {
#(#field_idents: <#field_types>::fetch(&mut _fetch.#field_idents, _entity, _table_row),)*
#(#ignored_field_idents: Default::default(),)*
}
}

Expand Down Expand Up @@ -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 {
#state_struct_name {
#(#field_idents: <#field_types>::init_state(world),)*
#(#ignored_field_idents: Default::default(),)*
}
}

Expand All @@ -349,7 +329,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
#[automatically_derived]
#visibility struct #read_only_struct_name #user_impl_generics #user_where_clauses {
#( #field_visibilities #field_idents: #read_only_field_types, )*
#(#(#ignored_field_attrs)* #ignored_field_visibilities #ignored_field_idents: #ignored_field_types,)*
}

#readonly_state
Expand Down Expand Up @@ -396,7 +375,6 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
#[automatically_derived]
#visibility struct #state_struct_name #user_impl_generics #user_where_clauses {
#(#field_idents: <#field_types as #path::query::WorldQuery>::State,)*
#(#ignored_field_idents: ::std::marker::PhantomData<fn() -> #ignored_field_types>,)*
}

#mutable_impl
Expand Down Expand Up @@ -428,56 +406,32 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
q2: #read_only_struct_name #user_ty_generics
) #user_where_clauses {
#(q.#field_idents;)*
#(q.#ignored_field_idents;)*
#(q2.#field_idents;)*
#(q2.#ignored_field_idents;)*
}
};
})
}

struct WorldQueryFieldInfo {
/// Has the `#[world_query(ignore)]` attribute.
is_ignored: bool,
/// All field attributes except for `world_query` ones.
attrs: Vec<Attribute>,
}

fn read_world_query_field_info(field: &Field) -> WorldQueryFieldInfo {
let is_ignored = field
.attrs
.iter()
.find(|attr| {
attr.path
.get_ident()
.map_or(false, |ident| ident == WORLD_QUERY_ATTRIBUTE_NAME)
})
.map_or(false, |attr| {
let mut is_ignored = false;
attr.parse_args_with(|input: ParseStream| {
if input
.parse::<Option<field_attr_keywords::ignore>>()?
.is_some()
{
is_ignored = true;
}
Ok(())
})
.unwrap_or_else(|_| panic!("Invalid `{WORLD_QUERY_ATTRIBUTE_NAME}` attribute format"));

is_ignored
});

let attrs = field
.attrs
.iter()
.filter(|attr| {
attr.path
.get_ident()
.map_or(true, |ident| ident != WORLD_QUERY_ATTRIBUTE_NAME)
})
.cloned()
.collect();
fn read_world_query_field_info(field: &Field) -> syn::Result<WorldQueryFieldInfo> {
let mut attrs = Vec::new();
for attr in &field.attrs {
if attr
.path
.get_ident()
.map_or(false, |ident| ident == WORLD_QUERY_ATTRIBUTE_NAME)
{
return Err(syn::Error::new_spanned(
attr,
"#[derive(WorldQuery)] does not support field attributes.",
));
}
attrs.push(attr.clone());
}

WorldQueryFieldInfo { is_ignored, attrs }
Ok(WorldQueryFieldInfo { attrs })
}
75 changes: 13 additions & 62 deletions crates/bevy_ecs/macros/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ use proc_macro::TokenStream;
use proc_macro2::Span;
use quote::{format_ident, quote};
use syn::{
parse::ParseStream, parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned,
ConstParam, DeriveInput, GenericParam, Ident, Index, Meta, MetaList, NestedMeta, Token,
TypeParam,
parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned, ConstParam,
DeriveInput, GenericParam, Ident, Index, Meta, MetaList, NestedMeta, Token, TypeParam,
};

enum BundleFieldKind {
Expand Down Expand Up @@ -252,13 +251,6 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream {
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
#[proc_macro_derive(SystemParam, attributes(system_param))]
pub fn derive_system_param(input: TokenStream) -> TokenStream {
Expand All @@ -271,53 +263,20 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
};
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 fields = Vec::new();
let mut field_types = Vec::new();
let mut ignored_fields = Vec::new();
let mut ignored_field_types = Vec::new();
for (i, (field, attrs)) in field_attributes.iter().enumerate() {
if attrs.ignore {
ignored_fields.push(field.ident.as_ref().unwrap());
ignored_field_types.push(&field.ty);
} else {
field_locals.push(format_ident!("f{i}"));
let i = Index::from(i);
fields.push(
field
.ident
.as_ref()
.map(|f| quote! { #f })
.unwrap_or_else(|| quote! { #i }),
);
field_types.push(&field.ty);
}
for (i, field) in field_definitions.iter().enumerate() {
field_locals.push(format_ident!("f{i}"));
let i = Index::from(i);
fields.push(
field
.ident
.as_ref()
.map(|f| quote! { #f })
.unwrap_or_else(|| quote! { #i }),
);
field_types.push(&field.ty);
}

let generics = ast.generics;
Expand Down Expand Up @@ -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_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,
// fold the fields into tuples of tuples until we are below the limit.
const LIMIT: usize = 16;
Expand Down Expand Up @@ -463,7 +415,6 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
>::get_param(&mut state.state, system_meta, world, change_tick);
#struct_name {
#(#fields: #field_locals,)*
#(#ignored_fields: std::default::Default::default(),)*
}
}
}
Expand Down
10 changes: 1 addition & 9 deletions crates/bevy_ecs/src/query/fetch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,7 @@ use std::{cell::UnsafeCell, marker::PhantomData};
/// - Methods can be implemented for the query items.
/// - There is no hardcoded limit on the number of elements.
///
/// This trait can only be derived if each field either
///
/// * 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.
///
/// This trait can only be derived if each field also implements `WorldQuery`.
/// The derive macro only supports regular structs (structs with named fields).
///
/// ```
Expand Down Expand Up @@ -1485,9 +1479,7 @@ mod tests {
#[derive(WorldQuery)]
pub struct IgnoredQuery<Marker> {
id: Entity,
#[world_query(ignore)]
_marker: PhantomData<Marker>,
_marker2: PhantomData<Marker>,
}

fn ignored_system(_: Query<IgnoredQuery<()>>) {}
Expand Down
20 changes: 5 additions & 15 deletions crates/bevy_ecs/src/system/system_param.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,10 @@ use std::{
/// 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
/// ## `PhantomData`
///
/// `#[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, such as markers for generic types.
/// [`PhantomData`] is a special type of `SystemParam` that does nothing.
/// This is useful for constraining generic types or lifetimes.
///
/// # Example
///
Expand All @@ -55,7 +53,6 @@ use std::{
/// #[derive(SystemParam)]
/// struct MyParam<'w, Marker: 'static> {
/// foo: Res<'w, SomeResource>,
/// #[system_param(ignore)]
/// marker: PhantomData<Marker>,
/// }
///
Expand All @@ -66,11 +63,6 @@ use std::{
/// # 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
///
/// When using the derive macro, you may see an error in the form of:
Expand Down Expand Up @@ -1652,14 +1644,12 @@ mod tests {
#[test]
fn system_param_phantom_data() {
#[derive(SystemParam)]
struct IgnoredParam<'w, T: Resource, Marker: 'static> {
struct PhantomParam<'w, T: Resource, Marker: 'static> {
_foo: Res<'w, T>,
#[system_param(ignore)]
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);
}

Expand Down