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

Make #[system_param(ignore)] and #[world_query(ignore)] unnecessary #8030

Merged
merged 42 commits into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
c766e7c
impl `SystemParam` for `PhantomData`
JoJoJet Mar 10, 2023
8994d13
ensure state types don't collide for `SystemParam`
JoJoJet Mar 10, 2023
92d7921
add a dedicated method for generating identifiers with no collision
JoJoJet Mar 10, 2023
1c6d01f
add a comment describing the collision algorithm
JoJoJet Mar 10, 2023
a25fb60
use explicit lifetimes in the `SystemParam` derive
JoJoJet Mar 10, 2023
65f332e
remove `#system_param(ignore)`
JoJoJet Mar 10, 2023
43507a6
clean up state struct lifetimes
JoJoJet Mar 10, 2023
6df2d0a
Merge remote-tracking branch 'upstream/main' into system-param-phanto…
JoJoJet Mar 10, 2023
4e5797c
remove unused attributes
JoJoJet Mar 10, 2023
6eac07a
use a descriptive name for a macro variable
JoJoJet Mar 10, 2023
653fc46
implement `WorldQuery` for `PhantomData`
JoJoJet Mar 10, 2023
e18e7af
add a compile test
JoJoJet Mar 10, 2023
715f8ec
remove `#[world_query(ignore)]`
JoJoJet Mar 10, 2023
f60909b
make `world_query(ignore)` an error
JoJoJet Mar 10, 2023
4ee9978
add comments to `IS_DENSE` and `IS_ARCHETYPAL`
JoJoJet Mar 10, 2023
3a55ca1
update a comment
JoJoJet Mar 10, 2023
c4005ac
disallow `#[system_param]` attributes
JoJoJet Mar 11, 2023
136033c
Merge remote-tracking branch 'upstream/main' into system-param-phanto…
JoJoJet Mar 11, 2023
eaddf9a
remove an unused const
JoJoJet Mar 11, 2023
b2f4309
undo an overlapping change
JoJoJet Mar 13, 2023
0ab464c
add an example demonstrating `PhantomData` queries
JoJoJet Mar 17, 2023
978437a
import worldquery
JoJoJet Mar 17, 2023
08d6b60
remove collision code
JoJoJet Mar 18, 2023
244a589
remove needless whitespace
JoJoJet Mar 18, 2023
6f5d446
Merge remote-tracking branch 'upstream/main' into system-param-phanto…
JoJoJet Mar 18, 2023
3f0bd19
remove an unused import
JoJoJet Mar 18, 2023
47040f2
Merge remote-tracking branch 'upstream/main' into system-param-phanto…
JoJoJet Mar 21, 2023
988ec67
fix formatting for a `let ... else` block
JoJoJet Mar 21, 2023
986f10c
Merge remote-tracking branch 'upstream/main' into system-param-phanto…
JoJoJet Mar 22, 2023
aebb03d
Merge remote-tracking branch 'upstream/main' into system-param-phanto…
JoJoJet Mar 22, 2023
589361e
remove a space
JoJoJet Mar 22, 2023
5dfc332
use non-conflicting lifetime names
JoJoJet Mar 23, 2023
93624c3
add a test case for invariant lifetimes
JoJoJet Mar 24, 2023
eb3e17d
re-add `#[system_param(ignore)]`
JoJoJet Mar 28, 2023
f3572b1
re-add `#[world_query(ignore)]`
JoJoJet Mar 28, 2023
bcfee71
restore some docs
JoJoJet Mar 28, 2023
10edef0
cargo fmt
JoJoJet Mar 28, 2023
f8469c5
re-order an example
JoJoJet Mar 28, 2023
2579709
allow unlimited ignored fields
JoJoJet Mar 28, 2023
6a3cfce
reduce churn
JoJoJet Mar 28, 2023
8ff0178
Update crates/bevy_ecs/macros/src/lib.rs
JoJoJet Mar 29, 2023
464e095
deduplicate regression tests
JoJoJet Mar 29, 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
97 changes: 26 additions & 71 deletions crates/bevy_ecs/macros/src/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,33 +120,24 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> 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);
match read_world_query_field_info(field) {
Ok(WorldQueryFieldInfo { attrs }) => field_attrs.push(attrs),
Err(e) => return e.into_compile_error().into(),
}

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));
}
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;
Expand Down Expand Up @@ -184,7 +175,6 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> 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 @@ -196,7 +186,6 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> 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 @@ -215,9 +204,6 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream {
#(
#field_idents: <#field_types>::shrink(item.#field_idents),
)*
#(
#ignored_field_idents: item.#ignored_field_idents,
)*
}
}

Expand All @@ -236,7 +222,6 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream {
_this_run,
),
)*
#(#ignored_field_idents: Default::default(),)*
}
}

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

Expand Down Expand Up @@ -287,7 +269,6 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> 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 @@ -318,7 +299,6 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> 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 @@ -340,7 +320,6 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream {
#[automatically_derived]
#visibility struct #read_only_struct_name #user_impl_generics #user_where_clauses {
#( #field_idents: #read_only_field_types, )*
#(#(#ignored_field_attrs)* #ignored_field_visibilities #ignored_field_idents: #ignored_field_types,)*
}

#readonly_state
Expand Down Expand Up @@ -387,7 +366,6 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> 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: #ignored_field_types,)*
}

#mutable_impl
Expand Down Expand Up @@ -419,56 +397,33 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> 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 `#[fetch(ignore)]` or `#[filter_fetch(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 })
}
120 changes: 55 additions & 65 deletions crates/bevy_ecs/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,12 @@ mod states;

use crate::{fetch::derive_world_query_impl, set::derive_set};
use bevy_macro_utils::{derive_boxed_label, get_named_struct_fields, BevyManifest};
use proc_macro::TokenStream;
use proc_macro::{TokenStream, TokenTree};
use proc_macro2::Span;
use quote::{format_ident, quote};
use syn::{
parse::ParseStream, parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned,
ConstParam, DeriveInput, Field, 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 @@ -248,16 +247,29 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream {
tokens
}

#[derive(Default)]
struct SystemParamFieldAttributes {
pub ignore: bool,
/// Checks if the specified identifier occurs in the specified [`TokenStream`].
#[allow(clippy::cmp_owned)]
fn check_for_collision(haystack: TokenStream, value: &Ident) -> bool {
haystack.into_iter().any(|t| match t {
TokenTree::Group(g) => check_for_collision(g.stream(), value),
TokenTree::Ident(ident) => ident.to_string() == value.to_string(),
TokenTree::Punct(_) | TokenTree::Literal(_) => false,
})
}

static SYSTEM_PARAM_ATTRIBUTE_NAME: &str = "system_param";
fn ensure_no_collision(haystack: TokenStream, mut value: Ident) -> Ident {
// If there's a collision, add more characters to the identifier
// until it doesn't collide with anything anymore.
while check_for_collision(haystack.clone(), &value) {
JoJoJet marked this conversation as resolved.
Show resolved Hide resolved
JoJoJet marked this conversation as resolved.
Show resolved Hide resolved
value = format_ident!("{value}X");
}
value
}

/// Implement `SystemParam` to use a struct as a parameter in a system
#[proc_macro_derive(SystemParam, attributes(system_param))]
#[proc_macro_derive(SystemParam)]
pub fn derive_system_param(input: TokenStream) -> TokenStream {
let token_stream = input.clone();
let ast = parse_macro_input!(input as DeriveInput);
let syn::Data::Struct(syn::DataStruct { fields: field_definitions, ..}) = ast.data else {
return syn::Error::new(ast.span(), "Invalid `SystemParam` type: expected a `struct`")
Expand All @@ -266,52 +278,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<(&Field, SystemParamFieldAttributes)>>();
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 @@ -367,6 +347,18 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
_ => unreachable!(),
}));

let punctuated_generics_no_bounds: Punctuated<_, Token![,]> = lifetimeless_generics
.iter()
.map(|&g| match g.clone() {
GenericParam::Type(mut g) => {
g.bounds.clear();
GenericParam::Type(g)
}
g @ GenericParam::Const(_) => g,
_ => unreachable!(),
JoJoJet marked this conversation as resolved.
Show resolved Hide resolved
})
.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();

Expand All @@ -392,39 +384,38 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {

let struct_name = &ast.ident;
let state_struct_visibility = &ast.vis;
let fields_alias = ensure_no_collision(token_stream, format_ident!("FieldsAlias"));

TokenStream::from(quote! {
// We define the FetchState struct in an anonymous scope to avoid polluting the user namespace.
// The struct can still be accessed via SystemParam::State, e.g. EventReaderState can be accessed via
// <EventReader<'static, 'static, T> as SystemParam>::State
const _: () = {
// Allows rebinding the lifetimes of each field type.
type #fields_alias <'w, 's, #punctuated_generics_no_bounds> = (#(#tuple_types,)*);

#[doc(hidden)]
#state_struct_visibility struct FetchState <'w, 's, #(#lifetimeless_generics,)*>
#state_struct_visibility struct FetchState <#(#lifetimeless_generics,)*>
#where_clause {
state: (#(<#tuple_types as #path::system::SystemParam>::State,)*),
marker: std::marker::PhantomData<(
<#path::prelude::Query<'w, 's, ()> as #path::system::SystemParam>::State,
#(fn() -> #ignored_field_types,)*
)>,
state: <#fields_alias::<'static, 'static, #punctuated_generic_idents> as #path::system::SystemParam>::State,
}

unsafe impl<'w, 's, #punctuated_generics> #path::system::SystemParam for #struct_name #ty_generics #where_clause {
type State = FetchState<'static, 'static, #punctuated_generic_idents>;
type State = FetchState<#punctuated_generic_idents>;
type Item<'_w, '_s> = #struct_name <#(#shadowed_lifetimes,)* #punctuated_generic_idents>;

fn init_state(world: &mut #path::world::World, system_meta: &mut #path::system::SystemMeta) -> Self::State {
FetchState {
state: <(#(#tuple_types,)*) as #path::system::SystemParam>::init_state(world, system_meta),
marker: std::marker::PhantomData,
state: <#fields_alias::<'static, 'static, #punctuated_generic_idents> as #path::system::SystemParam>::init_state(world, system_meta),
}
}

fn new_archetype(state: &mut Self::State, archetype: &#path::archetype::Archetype, system_meta: &mut #path::system::SystemMeta) {
<(#(#tuple_types,)*) as #path::system::SystemParam>::new_archetype(&mut state.state, archetype, system_meta)
<#fields_alias::<'w, 's, #punctuated_generic_idents> as #path::system::SystemParam>::new_archetype(&mut state.state, archetype, system_meta)
}

fn apply(state: &mut Self::State, system_meta: &#path::system::SystemMeta, world: &mut #path::world::World) {
<(#(#tuple_types,)*) as #path::system::SystemParam>::apply(&mut state.state, system_meta, world);
<#fields_alias::<'w, 's, #punctuated_generic_idents> as #path::system::SystemParam>::apply(&mut state.state, system_meta, world);
}

unsafe fn get_param<'w2, 's2>(
Expand All @@ -434,11 +425,10 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
change_tick: #path::component::Tick,
) -> Self::Item<'w2, 's2> {
let (#(#tuple_patterns,)*) = <
(#(#tuple_types,)*) as #path::system::SystemParam
#fields_alias::<'w2, 's2, #punctuated_generic_idents> as #path::system::SystemParam
>::get_param(&mut state.state, system_meta, world, change_tick);
#struct_name {
#(#fields: #field_locals,)*
#(#ignored_fields: std::default::Default::default(),)*
}
}
}
Expand Down
Loading