From 543415c4821666d47a8afb87610a18224a2bcdd6 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Fri, 17 Mar 2023 19:40:02 -0400 Subject: [PATCH 1/8] allow using tuple structs in the world query derive --- crates/bevy_ecs/macros/src/fetch.rs | 122 +++++++++++++++++++--------- 1 file changed, 82 insertions(+), 40 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 8bb5597f0d292..b53631e7c768f 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -1,11 +1,11 @@ use proc_macro::TokenStream; use proc_macro2::{Ident, Span}; -use quote::{quote, ToTokens}; +use quote::{format_ident, quote, ToTokens}; use syn::{ parse::{Parse, ParseStream}, parse_quote, punctuated::Punctuated, - Attribute, Data, DataStruct, DeriveInput, Field, Fields, + Attribute, Data, DataStruct, DeriveInput, Field, Index, }; use crate::bevy_ecs_path; @@ -112,37 +112,59 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { let state_struct_name = Ident::new(&format!("{struct_name}State"), Span::call_site()); - let fields = match &ast.data { - Data::Struct(DataStruct { - fields: Fields::Named(fields), - .. - }) => &fields.named, - _ => panic!("Expected a struct with named fields"), + let Data::Struct(DataStruct { fields, .. }) = &ast.data else { + return syn::Error::new( + Span::call_site(), + "#[derive(WorldQuery)]` only supports structs", + ) + .into_compile_error() + .into() }; + if matches!(fields, syn::Fields::Unit) { + return syn::Error::new( + Span::call_site(), + "#[derive(WorldQuery)]` does not support unit structs", + ) + .into_compile_error() + .into(); + } let mut ignored_field_attrs = Vec::new(); let mut ignored_field_visibilities = Vec::new(); let mut ignored_field_idents = Vec::new(); + let mut ignored_named_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 named_field_idents = Vec::new(); let mut field_types = Vec::new(); let mut read_only_field_types = Vec::new(); - for field in fields { + for (i, field) in fields.iter().enumerate() { let WorldQueryFieldInfo { is_ignored, attrs } = read_world_query_field_info(field); - let field_ident = field.ident.as_ref().unwrap().clone(); + let named_field_ident = field + .ident + .as_ref() + .cloned() + .unwrap_or_else(|| format_ident!("f{i}")); + let i = Index::from(i); + let field_ident = field + .ident + .as_ref() + .map_or(quote! { #i }, |i| quote! { #i }); if is_ignored { ignored_field_attrs.push(attrs); ignored_field_visibilities.push(field.vis.clone()); - ignored_field_idents.push(field_ident.clone()); + ignored_field_idents.push(field_ident); + ignored_named_field_idents.push(named_field_ident); ignored_field_types.push(field.ty.clone()); } else { field_attrs.push(attrs); field_visibilities.push(field.vis.clone()); - field_idents.push(field_ident.clone()); + field_idents.push(field_ident); + named_field_idents.push(named_field_ident); 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)); @@ -176,16 +198,36 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { &field_types }; - let item_struct = quote! { - #derive_macro_call - #[doc = "Automatically generated [`WorldQuery`] item type for [`"] - #[doc = stringify!(#struct_name)] - #[doc = "`], returned when iterating over query results."] - #[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,)* - } + let item_struct = match fields { + syn::Fields::Named(_) => quote! { + #derive_macro_call + #[doc = "Automatically generated [`WorldQuery`] item type for [`"] + #[doc = stringify!(#struct_name)] + #[doc = "`], returned when iterating over query results."] + #[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,)* + } + }, + syn::Fields::Unnamed(_) => quote! { + #derive_macro_call + #[doc = "Automatically generated [`WorldQuery`] item type for [`"] + #[doc = stringify!(#struct_name)] + #[doc = "`], returned when iterating over query results."] + #[automatically_derived] + #visibility struct #item_struct_name #user_impl_generics_with_world #user_where_clauses_with_world( + #( #field_visibilities <#field_types as #path::query::WorldQuery>::Item<'__w>, )* + ); + }, + syn::Fields::Unit => quote! { + #derive_macro_call + #[doc = "Automatically generated [`WorldQuery`] item type for [`"] + #[doc = stringify!(#struct_name)] + #[doc = "`], returned when iterating over query results."] + #[automatically_derived] + #visibility struct #item_struct_name #user_impl_generics_with_world #user_where_clauses_with_world; + }, }; let query_impl = quote! { @@ -195,8 +237,8 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { #[doc = "`], used to define the world data accessed by this query."] #[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,)* + #(#named_field_idents: <#field_types as #path::query::WorldQuery>::Fetch<'__w>,)* + #(#ignored_named_field_idents: #ignored_field_types,)* } // SAFETY: `update_component_access` and `update_archetype_component_access` are called on every field @@ -228,15 +270,15 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { _this_run: #path::component::Tick, ) -> ::Fetch<'__w> { #fetch_struct_name { - #(#field_idents: + #(#named_field_idents: <#field_types>::init_fetch( _world, - &state.#field_idents, + &state.#named_field_idents, _last_run, _this_run, ), )* - #(#ignored_field_idents: Default::default(),)* + #(#ignored_named_field_idents: Default::default(),)* } } @@ -245,10 +287,10 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { ) -> ::Fetch<'__w> { #fetch_struct_name { #( - #field_idents: <#field_types>::clone_fetch(& _fetch. #field_idents), + #named_field_idents: <#field_types>::clone_fetch(& _fetch. #named_field_idents), )* #( - #ignored_field_idents: Default::default(), + #ignored_named_field_idents: Default::default(), )* } } @@ -265,7 +307,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { _archetype: &'__w #path::archetype::Archetype, _table: &'__w #path::storage::Table ) { - #(<#field_types>::set_archetype(&mut _fetch.#field_idents, &_state.#field_idents, _archetype, _table);)* + #(<#field_types>::set_archetype(&mut _fetch.#named_field_idents, &_state.#named_field_idents, _archetype, _table);)* } /// SAFETY: we call `set_table` for each member that implements `Fetch` @@ -275,7 +317,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { _state: &Self::State, _table: &'__w #path::storage::Table ) { - #(<#field_types>::set_table(&mut _fetch.#field_idents, &_state.#field_idents, _table);)* + #(<#field_types>::set_table(&mut _fetch.#named_field_idents, &_state.#named_field_idents, _table);)* } /// SAFETY: we call `fetch` for each member that implements `Fetch`. @@ -286,7 +328,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { _table_row: #path::storage::TableRow, ) -> ::Item<'__w> { Self::Item { - #(#field_idents: <#field_types>::fetch(&mut _fetch.#field_idents, _entity, _table_row),)* + #(#field_idents: <#field_types>::fetch(&mut _fetch.#named_field_idents, _entity, _table_row),)* #(#ignored_field_idents: Default::default(),)* } } @@ -298,11 +340,11 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { _entity: #path::entity::Entity, _table_row: #path::storage::TableRow, ) -> bool { - true #(&& <#field_types>::filter_fetch(&mut _fetch.#field_idents, _entity, _table_row))* + true #(&& <#field_types>::filter_fetch(&mut _fetch.#named_field_idents, _entity, _table_row))* } fn update_component_access(state: &Self::State, _access: &mut #path::query::FilteredAccess<#path::component::ComponentId>) { - #( <#field_types>::update_component_access(&state.#field_idents, _access); )* + #( <#field_types>::update_component_access(&state.#named_field_idents, _access); )* } fn update_archetype_component_access( @@ -311,19 +353,19 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { _access: &mut #path::query::Access<#path::archetype::ArchetypeComponentId> ) { #( - <#field_types>::update_archetype_component_access(&state.#field_idents, _archetype, _access); + <#field_types>::update_archetype_component_access(&state.#named_field_idents, _archetype, _access); )* } fn init_state(world: &mut #path::world::World) -> #state_struct_name #user_ty_generics { #state_struct_name { - #(#field_idents: <#field_types>::init_state(world),)* + #(#named_field_idents: <#field_types>::init_state(world),)* #(#ignored_field_idents: Default::default(),)* } } fn matches_component_set(state: &Self::State, _set_contains_id: &impl Fn(#path::component::ComponentId) -> bool) -> bool { - true #(&& <#field_types>::matches_component_set(&state.#field_idents, _set_contains_id))* + true #(&& <#field_types>::matches_component_set(&state.#named_field_idents, _set_contains_id))* } } }; @@ -339,7 +381,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { #[doc = "`]."] #[automatically_derived] #visibility struct #read_only_struct_name #user_impl_generics #user_where_clauses { - #( #field_idents: #read_only_field_types, )* + #( #named_field_idents: #read_only_field_types, )* #(#(#ignored_field_attrs)* #ignored_field_visibilities #ignored_field_idents: #ignored_field_types,)* } @@ -386,8 +428,8 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { #[doc = "`], used for caching."] #[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,)* + #(#named_field_idents: <#field_types as #path::query::WorldQuery>::State,)* + #(#ignored_named_field_idents: #ignored_field_types,)* } #mutable_impl From cebd696194363935ce0419fc0e66ab8ddaa5a92b Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Fri, 17 Mar 2023 19:40:12 -0400 Subject: [PATCH 2/8] add a test case --- crates/bevy_ecs/src/query/fetch.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 3b4a6a076f143..943e649f60d3c 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1388,3 +1388,33 @@ unsafe impl WorldQuery for NopWorldQuery { /// SAFETY: `NopFetch` never accesses any data unsafe impl ReadOnlyWorldQuery for NopWorldQuery {} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + self as bevy_ecs, + system::{assert_is_system, Query}, + }; + + #[derive(Component)] + struct A; + + #[derive(Component)] + struct B; + + #[test] + fn world_query_struct_variants() { + #[derive(WorldQuery)] + pub struct NamedQuery { + id: Entity, + a: &'static A, + } + + #[derive(WorldQuery)] + pub struct TupleQuery(&'static A, &'static B); + + fn my_system(_: Query<(NamedQuery, TupleQuery)>) {} + assert_is_system(my_system); + } +} From 78805ec51880b73a97fcbca45a02f1a30d844835 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Fri, 17 Mar 2023 19:45:02 -0400 Subject: [PATCH 3/8] tweak documentation --- crates/bevy_ecs/src/query/fetch.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 943e649f60d3c..4938bf958fca7 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -62,7 +62,7 @@ use std::{cell::UnsafeCell, marker::PhantomData}; /// 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 structs. /// /// ``` /// # use bevy_ecs::prelude::*; From 52ada0eb943251dfc63cf20716330e8f4f550cb5 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Fri, 17 Mar 2023 19:45:10 -0400 Subject: [PATCH 4/8] make an error case less strict --- crates/bevy_ecs/macros/src/fetch.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index b53631e7c768f..4b28fc142ef8d 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -120,10 +120,10 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { .into_compile_error() .into() }; - if matches!(fields, syn::Fields::Unit) { + if fields.is_empty() { return syn::Error::new( Span::call_site(), - "#[derive(WorldQuery)]` does not support unit structs", + "#[derive(WorldQuery)]` does not support fieldless structs", ) .into_compile_error() .into(); From 567ad8a4b097ba00f7d34952d7f78c38f1b160d7 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Fri, 17 Mar 2023 20:24:56 -0400 Subject: [PATCH 5/8] fix the lifetime error and support fieldless structs --- crates/bevy_ecs/macros/src/fetch.rs | 14 ++++---------- crates/bevy_ecs/src/query/fetch.rs | 5 ++++- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 4b28fc142ef8d..6bd38583cd17c 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -120,14 +120,6 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { .into_compile_error() .into() }; - if fields.is_empty() { - return syn::Error::new( - Span::call_site(), - "#[derive(WorldQuery)]` does not support fieldless structs", - ) - .into_compile_error() - .into(); - } let mut ignored_field_attrs = Vec::new(); let mut ignored_field_visibilities = Vec::new(); @@ -221,12 +213,11 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { ); }, syn::Fields::Unit => quote! { - #derive_macro_call #[doc = "Automatically generated [`WorldQuery`] item type for [`"] #[doc = stringify!(#struct_name)] #[doc = "`], returned when iterating over query results."] #[automatically_derived] - #visibility struct #item_struct_name #user_impl_generics_with_world #user_where_clauses_with_world; + #visibility type #item_struct_name #user_ty_generics_with_world = #struct_name #user_ty_generics; }, }; @@ -239,6 +230,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { #visibility struct #fetch_struct_name #user_impl_generics_with_world #user_where_clauses_with_world { #(#named_field_idents: <#field_types as #path::query::WorldQuery>::Fetch<'__w>,)* #(#ignored_named_field_idents: #ignored_field_types,)* + __world_query_lifetime_marker: &'__w (), } // SAFETY: `update_component_access` and `update_archetype_component_access` are called on every field @@ -279,6 +271,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { ), )* #(#ignored_named_field_idents: Default::default(),)* + __world_query_lifetime_marker: &(), } } @@ -292,6 +285,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { #( #ignored_named_field_idents: Default::default(), )* + __world_query_lifetime_marker: &(), } } diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 4938bf958fca7..a575b8b2a0e8b 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1414,7 +1414,10 @@ mod tests { #[derive(WorldQuery)] pub struct TupleQuery(&'static A, &'static B); - fn my_system(_: Query<(NamedQuery, TupleQuery)>) {} + #[derive(WorldQuery)] + pub struct UnitQuery; + + fn my_system(_: Query<(NamedQuery, TupleQuery, UnitQuery)>) {} assert_is_system(my_system); } } From 769e26665423602c170e65847ec5e06f8c3f00f4 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Wed, 22 Mar 2023 15:37:29 -0400 Subject: [PATCH 6/8] deduplicate a struct --- crates/bevy_ecs/src/query/fetch.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index cda4f549c0758..9fae7fc310c11 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1398,10 +1398,10 @@ mod tests { }; #[derive(Component)] - struct A; + pub struct A; #[derive(Component)] - struct B; + pub struct B; // Tests that each variant of struct can be used as a `WorldQuery`. #[test] @@ -1423,9 +1423,6 @@ mod tests { assert_is_system(my_system); } - #[derive(Component)] - pub struct A; - // Ensures that each field of a `WorldQuery` struct's read-only variant // has the same visibility as its corresponding mutable field. #[test] From e388d57e14a8e41511db2c50a00f29bca274c61d Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Wed, 22 Mar 2023 16:05:14 -0400 Subject: [PATCH 7/8] prevent name collisions for the fetch struct marker field --- crates/bevy_ecs/macros/src/fetch.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index b150a78ea97b1..f4ae77e8d4f4f 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -116,6 +116,9 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { fetch_struct_name.clone() }; + let marker_name = + ensure_no_collision(format_ident!("_world_query_derive_marker"), tokens.clone()); + // Generate a name for the state struct that doesn't conflict // with the struct definition. let state_struct_name = Ident::new(&format!("{struct_name}State"), Span::call_site()); @@ -239,7 +242,7 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { #visibility struct #fetch_struct_name #user_impl_generics_with_world #user_where_clauses_with_world { #(#named_field_idents: <#field_types as #path::query::WorldQuery>::Fetch<'__w>,)* #(#ignored_named_field_idents: #ignored_field_types,)* - __world_query_lifetime_marker: &'__w (), + #marker_name: &'__w (), } // SAFETY: `update_component_access` and `update_archetype_component_access` are called on every field @@ -280,7 +283,7 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { ), )* #(#ignored_named_field_idents: Default::default(),)* - __world_query_lifetime_marker: &(), + #marker_name: &(), } } @@ -294,7 +297,7 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { #( #ignored_named_field_idents: Default::default(), )* - __world_query_lifetime_marker: &(), + #marker_name: &(), } } From 7ef1e4caa13dbbf528c1df8426f58180a10113f8 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Thu, 30 Mar 2023 21:08:48 -0400 Subject: [PATCH 8/8] re-order insertions --- crates/bevy_ecs/macros/src/fetch.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 0ae32e1437d5a..6f5dc575edbfc 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -155,10 +155,10 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { .ident .as_ref() .map_or(quote! { #i }, |i| quote! { #i }); - field_attrs.push(attrs); - field_visibilities.push(field.vis.clone()); field_idents.push(field_ident); named_field_idents.push(named_field_ident); + field_attrs.push(attrs); + field_visibilities.push(field.vis.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));