From ba31c9b1ae09c4e1350f00684625b7360bc8b93f Mon Sep 17 00:00:00 2001 From: mvlabat Date: Mon, 23 Aug 2021 00:37:22 +0300 Subject: [PATCH 01/14] Implement Fetch and FilterFetch derive macros --- Cargo.toml | 4 + crates/bevy_ecs/macros/src/fetch.rs | 746 +++++++++++++++++++++ crates/bevy_ecs/macros/src/lib.rs | 14 + crates/bevy_ecs/src/query/fetch.rs | 212 ++++++ crates/bevy_ecs/src/query/filter.rs | 47 ++ crates/bevy_ecs/src/system/system_param.rs | 3 +- examples/README.md | 1 + examples/ecs/custom_query_param.rs | 189 ++++++ 8 files changed, 1215 insertions(+), 1 deletion(-) create mode 100644 crates/bevy_ecs/macros/src/fetch.rs create mode 100644 examples/ecs/custom_query_param.rs diff --git a/Cargo.toml b/Cargo.toml index ef6cb40a7da7b..7e9d8df57fdf0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,6 +315,10 @@ path = "examples/ecs/ecs_guide.rs" name = "component_change_detection" path = "examples/ecs/component_change_detection.rs" +[[example]] +name = "custom_query_param" +path = "examples/ecs/custom_query_param.rs" + [[example]] name = "event" path = "examples/ecs/event.rs" diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs new file mode 100644 index 0000000000000..9386e491227bc --- /dev/null +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -0,0 +1,746 @@ +use proc_macro::TokenStream; +use proc_macro2::{Ident, Span}; +use quote::quote; +use syn::{ + parse_macro_input, punctuated::Punctuated, Data, DataStruct, DeriveInput, Fields, + GenericArgument, GenericParam, ImplGenerics, Lifetime, LifetimeDef, PathArguments, Token, Type, + TypeGenerics, TypePath, TypeReference, WhereClause, +}; + +use crate::bevy_ecs_path; + +static READ_ONLY_ATTRIBUTE_NAME: &str = "read_only"; +static READ_ONLY_DERIVE_ATTRIBUTE_NAME: &str = "read_only_derive"; + +pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { + let ast = parse_macro_input!(input as DeriveInput); + + let FetchImplTokens { + struct_name, + struct_name_read_only, + fetch_struct_name, + state_struct_name, + read_only_fetch_struct_name, + fetch_trait_punctuated_lifetimes, + impl_generics, + ty_generics, + where_clause, + has_world_lifetime, + has_read_only_attr, + world_lifetime, + state_lifetime, + phantom_field_idents, + phantom_field_types, + field_idents, + field_types: _, + query_types, + fetch_init_types, + } = fetch_impl_tokens(&ast); + + if !has_world_lifetime { + panic!("Expected a struct with a lifetime"); + } + + let read_only_derive_attr = ast.attrs.iter().find(|attr| { + attr.path + .get_ident() + .map_or(false, |ident| ident == READ_ONLY_DERIVE_ATTRIBUTE_NAME) + }); + let read_only_derive_macro_call = if let Some(read_only_derive_attr) = read_only_derive_attr { + if has_read_only_attr { + panic!("Attributes `read_only` and `read_only_derive` are mutually exclusive"); + } + let derive_args = &read_only_derive_attr.tokens; + quote! { #[derive #derive_args] } + } else { + quote! {} + }; + + // Fetch's HRTBs require this hack to make the implementation compile. I don't fully understand + // why this works though. If anyone's curious enough to try to find a better work-around, I'll + // leave playground links here: + // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=da5e260a5c2f3e774142d60a199e854a (this fails) + // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=802517bb3d8f83c45ee8c0be360bb250 (this compiles) + let fetch_lifetime = + GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'fetch", Span::call_site()))); + let mut fetch_generics = ast.generics.clone(); + fetch_generics.params.insert(1, state_lifetime); + fetch_generics.params.push(fetch_lifetime.clone()); + let (fetch_impl_generics, _, _) = fetch_generics.split_for_impl(); + + let mut fetch_generics = ast.generics.clone(); + *fetch_generics.params.first_mut().unwrap() = fetch_lifetime; + let (_, fetch_ty_generics, _) = fetch_generics.split_for_impl(); + + let path = bevy_ecs_path(); + + let struct_read_only_declaration = if has_read_only_attr { + quote! { + // Statically checks that the safety guarantee actually holds true. We need this to make + // sure that we don't compile `ReadOnlyFetch` if our struct contains nested `WorldQuery` + // members that don't implement it. + #[allow(dead_code)] + const _: () = { + fn assert_readonly() {} + + // We generate a readonly assertion for every struct member. + fn assert_all #impl_generics () #where_clause { + #(assert_readonly::<<#query_types as #path::query::WorldQuery>::Fetch>();)* + } + }; + } + } else { + quote! { + // TODO: it would be great to be able to dedup this by just deriving `Fetch` again with `read_only` attribute, + // but supporting QSelf types is tricky. + #read_only_derive_macro_call + struct #struct_name_read_only #impl_generics #where_clause { + #(#field_idents: <<#query_types as #path::query::WorldQuery>::ReadOnlyFetch as #path::query::Fetch<#world_lifetime, #world_lifetime>>::Item,)* + #(#phantom_field_idents: #phantom_field_types,)* + } + + struct #read_only_fetch_struct_name #impl_generics #where_clause { + #(#field_idents: <#query_types as #path::query::WorldQuery>::ReadOnlyFetch,)* + #(#phantom_field_idents: #phantom_field_types,)* + } + + impl #fetch_impl_generics #path::query::Fetch<#fetch_trait_punctuated_lifetimes> for #read_only_fetch_struct_name #fetch_ty_generics #where_clause { + type Item = #struct_name_read_only #ty_generics; + type State = #state_struct_name #fetch_ty_generics; + + unsafe fn init(_world: &#path::world::World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self { + #read_only_fetch_struct_name { + #(#field_idents: <#fetch_init_types as #path::query::WorldQuery>::ReadOnlyFetch::init(_world, &state.#field_idents, _last_change_tick, _change_tick),)* + #(#phantom_field_idents: Default::default(),)* + } + } + + const IS_DENSE: bool = true #(&& <#query_types as #path::query::WorldQuery>::ReadOnlyFetch::IS_DENSE)*; + + /// SAFETY: we call `set_archetype` for each member that implements `Fetch` + #[inline] + unsafe fn set_archetype(&mut self, _state: &Self::State, _archetype: &#path::archetype::Archetype, _tables: &#path::storage::Tables) { + #(self.#field_idents.set_archetype(&_state.#field_idents, _archetype, _tables);)* + } + + /// SAFETY: we call `set_table` for each member that implements `Fetch` + #[inline] + unsafe fn set_table(&mut self, _state: &Self::State, _table: &#path::storage::Table) { + #(self.#field_idents.set_table(&_state.#field_idents, _table);)* + } + + /// SAFETY: we call `table_fetch` for each member that implements `Fetch`. + #[inline] + unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item { + #struct_name_read_only { + #(#field_idents: self.#field_idents.table_fetch(_table_row),)* + #(#phantom_field_idents: Default::default(),)* + } + } + + /// SAFETY: we call `archetype_fetch` for each member that implements `Fetch`. + #[inline] + unsafe fn archetype_fetch(&mut self, _archetype_index: usize) -> Self::Item { + #struct_name_read_only { + #(#field_idents: self.#field_idents.archetype_fetch(_archetype_index),)* + #(#phantom_field_idents: Default::default(),)* + } + } + } + + impl #impl_generics #path::query::WorldQuery for #struct_name_read_only #ty_generics #where_clause { + type Fetch = #read_only_fetch_struct_name #ty_generics; + type State = #state_struct_name #ty_generics; + type ReadOnlyFetch = #read_only_fetch_struct_name #ty_generics; + } + } + }; + + let tokens = TokenStream::from(quote! { + struct #fetch_struct_name #impl_generics #where_clause { + #(#field_idents: <#query_types as #path::query::WorldQuery>::Fetch,)* + #(#phantom_field_idents: #phantom_field_types,)* + } + + struct #state_struct_name #impl_generics #where_clause { + #(#field_idents: <#query_types as #path::query::WorldQuery>::State,)* + #(#phantom_field_idents: #phantom_field_types,)* + } + + impl #fetch_impl_generics #path::query::Fetch<#fetch_trait_punctuated_lifetimes> for #fetch_struct_name #fetch_ty_generics #where_clause { + type Item = #struct_name #ty_generics; + type State = #state_struct_name #fetch_ty_generics; + + unsafe fn init(_world: &#path::world::World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self { + #fetch_struct_name { + #(#field_idents: <#fetch_init_types as #path::query::WorldQuery>::Fetch::init(_world, &state.#field_idents, _last_change_tick, _change_tick),)* + #(#phantom_field_idents: Default::default(),)* + } + } + + const IS_DENSE: bool = true #(&& <#query_types as #path::query::WorldQuery>::Fetch::IS_DENSE)*; + + /// SAFETY: we call `set_archetype` for each member that implements `Fetch` + #[inline] + unsafe fn set_archetype(&mut self, _state: &Self::State, _archetype: &#path::archetype::Archetype, _tables: &#path::storage::Tables) { + #(self.#field_idents.set_archetype(&_state.#field_idents, _archetype, _tables);)* + } + + /// SAFETY: we call `set_table` for each member that implements `Fetch` + #[inline] + unsafe fn set_table(&mut self, _state: &Self::State, _table: &#path::storage::Table) { + #(self.#field_idents.set_table(&_state.#field_idents, _table);)* + } + + /// SAFETY: we call `table_fetch` for each member that implements `Fetch`. + #[inline] + unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item { + #struct_name { + #(#field_idents: self.#field_idents.table_fetch(_table_row),)* + #(#phantom_field_idents: Default::default(),)* + } + } + + /// SAFETY: we call `archetype_fetch` for each member that implements `Fetch`. + #[inline] + unsafe fn archetype_fetch(&mut self, _archetype_index: usize) -> Self::Item { + #struct_name { + #(#field_idents: self.#field_idents.archetype_fetch(_archetype_index),)* + #(#phantom_field_idents: Default::default(),)* + } + } + } + + // SAFETY: `update_component_access` and `update_archetype_component_access` are called for each item in the struct + unsafe impl #impl_generics #path::query::FetchState for #state_struct_name #ty_generics #where_clause { + fn init(world: &mut #path::world::World) -> Self { + #state_struct_name { + #(#field_idents: <#query_types as #path::query::WorldQuery>::State::init(world),)* + #(#phantom_field_idents: Default::default(),)* + } + } + + fn update_component_access(&self, _access: &mut #path::query::FilteredAccess<#path::component::ComponentId>) { + #(self.#field_idents.update_component_access(_access);)* + } + + fn update_archetype_component_access(&self, _archetype: &#path::archetype::Archetype, _access: &mut #path::query::Access<#path::archetype::ArchetypeComponentId>) { + #(self.#field_idents.update_archetype_component_access(_archetype, _access);)* + } + + fn matches_archetype(&self, _archetype: &#path::archetype::Archetype) -> bool { + true #(&& self.#field_idents.matches_archetype(_archetype))* + } + + fn matches_table(&self, _table: &#path::storage::Table) -> bool { + true #(&& self.#field_idents.matches_table(_table))* + } + } + + #struct_read_only_declaration + + impl #impl_generics #path::query::WorldQuery for #struct_name #ty_generics #where_clause { + type Fetch = #fetch_struct_name #ty_generics; + type State = #state_struct_name #ty_generics; + type ReadOnlyFetch = #read_only_fetch_struct_name #ty_generics; + } + + /// SAFETY: each item in the struct is read only + unsafe impl #impl_generics #path::query::ReadOnlyFetch for #read_only_fetch_struct_name #ty_generics #where_clause {} + }); + tokens +} + +pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream { + let ast = parse_macro_input!(input as DeriveInput); + + let FetchImplTokens { + struct_name, + struct_name_read_only: _, + fetch_struct_name, + state_struct_name, + read_only_fetch_struct_name: _, + fetch_trait_punctuated_lifetimes, + impl_generics, + ty_generics, + where_clause, + world_lifetime, + has_read_only_attr: _, + has_world_lifetime, + state_lifetime, + phantom_field_idents, + phantom_field_types, + field_idents, + field_types, + query_types: _, + fetch_init_types: _, + } = fetch_impl_tokens(&ast); + + if has_world_lifetime { + panic!("Expected a struct without a lifetime"); + } + + // Fetch's HRTBs require this hack to make the implementation compile. I don't fully understand + // why this works though. If anyone's curious enough to try to find a better work-around, I'll + // leave playground links here: + // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=da5e260a5c2f3e774142d60a199e854a (this fails) + // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=802517bb3d8f83c45ee8c0be360bb250 (this compiles) + let fetch_lifetime = + GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'fetch", Span::call_site()))); + let mut fetch_generics = ast.generics.clone(); + fetch_generics.params.insert(0, world_lifetime); + fetch_generics.params.insert(1, state_lifetime); + fetch_generics.params.push(fetch_lifetime); + let (fetch_impl_generics, _, _) = fetch_generics.split_for_impl(); + + let path = bevy_ecs_path(); + + let tokens = TokenStream::from(quote! { + struct #fetch_struct_name #impl_generics #where_clause { + #(#field_idents: <#field_types as #path::query::WorldQuery>::Fetch,)* + #(#phantom_field_idents: #phantom_field_types,)* + } + + struct #state_struct_name #impl_generics #where_clause { + #(#field_idents: <#field_types as #path::query::WorldQuery>::State,)* + #(#phantom_field_idents: #phantom_field_types,)* + } + + impl #fetch_impl_generics #path::query::Fetch<#fetch_trait_punctuated_lifetimes> for #fetch_struct_name #ty_generics #where_clause { + type Item = bool; + type State = #state_struct_name #ty_generics; + + unsafe fn init(_world: &#path::world::World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self { + #fetch_struct_name { + #(#field_idents: <#field_types as #path::query::WorldQuery>::ReadOnlyFetch::init(_world, &state.#field_idents, _last_change_tick, _change_tick),)* + #(#phantom_field_idents: Default::default(),)* + } + } + + const IS_DENSE: bool = true #(&& <#field_types as #path::query::WorldQuery>::ReadOnlyFetch::IS_DENSE)*; + + #[inline] + unsafe fn set_archetype(&mut self, _state: &Self::State, _archetype: &#path::archetype::Archetype, _tables: &#path::storage::Tables) { + #(self.#field_idents.set_archetype(&_state.#field_idents, _archetype, _tables);)* + } + + #[inline] + unsafe fn set_table(&mut self, _state: &Self::State, _table: &#path::storage::Table) { + #(self.#field_idents.set_table(&_state.#field_idents, _table);)* + } + + #[inline] + unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item { + use #path::query::FilterFetch; + true #(&& self.#field_idents.table_filter_fetch(_table_row))* + } + + #[inline] + unsafe fn archetype_fetch(&mut self, _archetype_index: usize) -> Self::Item { + use #path::query::FilterFetch; + true #(&& self.#field_idents.archetype_filter_fetch(_archetype_index))* + } + } + + // SAFETY: update_component_access and update_archetype_component_access are called for each item in the struct + unsafe impl #impl_generics #path::query::FetchState for #state_struct_name #ty_generics #where_clause { + fn init(world: &mut #path::world::World) -> Self { + #state_struct_name { + #(#field_idents: <#field_types as #path::query::WorldQuery>::State::init(world),)* + #(#phantom_field_idents: Default::default(),)* + } + } + + fn update_component_access(&self, _access: &mut #path::query::FilteredAccess<#path::component::ComponentId>) { + #(self.#field_idents.update_component_access(_access);)* + } + + fn update_archetype_component_access(&self, _archetype: &#path::archetype::Archetype, _access: &mut #path::query::Access<#path::archetype::ArchetypeComponentId>) { + #(self.#field_idents.update_archetype_component_access(_archetype, _access);)* + } + + fn matches_archetype(&self, _archetype: &#path::archetype::Archetype) -> bool { + true #(&& self.#field_idents.matches_archetype(_archetype))* + } + + fn matches_table(&self, _table: &#path::storage::Table) -> bool { + true #(&& self.#field_idents.matches_table(_table))* + } + } + + impl #impl_generics #path::query::WorldQuery for #struct_name #ty_generics #where_clause { + type Fetch = #fetch_struct_name #ty_generics; + type State = #state_struct_name #ty_generics; + type ReadOnlyFetch = #fetch_struct_name #ty_generics; + } + + /// SAFETY: each item in the struct is read only + unsafe impl #impl_generics #path::query::ReadOnlyFetch for #fetch_struct_name #ty_generics #where_clause {} + }); + tokens +} + +// This struct is used to share common tokens between `Fetch` and `FilterFetch` implementations. +struct FetchImplTokens<'a> { + struct_name: Ident, + // Equals `struct_name` if `has_read_only_attr` is true. + struct_name_read_only: Ident, + fetch_struct_name: Ident, + state_struct_name: Ident, + read_only_fetch_struct_name: Ident, + fetch_trait_punctuated_lifetimes: Punctuated, + impl_generics: ImplGenerics<'a>, + ty_generics: TypeGenerics<'a>, + where_clause: Option<&'a WhereClause>, + has_read_only_attr: bool, + has_world_lifetime: bool, + world_lifetime: GenericParam, + state_lifetime: GenericParam, + phantom_field_idents: Vec, + phantom_field_types: Vec, + field_idents: Vec, + field_types: Vec, + query_types: Vec, + fetch_init_types: Vec, +} + +fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens { + let has_read_only_attr = ast.attrs.iter().any(|attr| { + attr.path + .get_ident() + .map_or(false, |ident| ident == READ_ONLY_ATTRIBUTE_NAME) + }); + + let world_lifetime = ast.generics.params.first().and_then(|param| match param { + lt @ GenericParam::Lifetime(_) => Some(lt.clone()), + _ => None, + }); + let has_world_lifetime = world_lifetime.is_some(); + let world_lifetime = world_lifetime.unwrap_or_else(|| { + GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'world", Span::call_site()))) + }); + let state_lifetime = + GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'state", Span::call_site()))); + + let mut fetch_trait_punctuated_lifetimes = Punctuated::<_, Token![,]>::new(); + fetch_trait_punctuated_lifetimes.push(world_lifetime.clone()); + fetch_trait_punctuated_lifetimes.push(state_lifetime.clone()); + + let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl(); + + let struct_name = ast.ident.clone(); + let struct_name_read_only = if has_read_only_attr { + ast.ident.clone() + } else { + Ident::new(&format!("{}ReadOnly", struct_name), Span::call_site()) + }; + let fetch_struct_name = Ident::new(&format!("{}Fetch", struct_name), Span::call_site()); + let state_struct_name = Ident::new(&format!("{}State", struct_name), Span::call_site()); + let read_only_fetch_struct_name = if has_read_only_attr { + fetch_struct_name.clone() + } else { + Ident::new(&format!("{}ReadOnlyFetch", struct_name), 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 mut phantom_field_idents = Vec::new(); + let mut phantom_field_types = Vec::new(); + let mut field_idents = Vec::new(); + let mut field_types = Vec::new(); + let mut query_types = Vec::new(); + let mut fetch_init_types = Vec::new(); + + let generic_names = ast + .generics + .params + .iter() + .filter_map(|param| match param { + GenericParam::Type(ty) => Some(ty.ident.to_string()), + _ => None, + }) + .collect::>(); + + for field in fields.iter() { + let WorldQueryFieldTypeInfo { + query_type, + fetch_init_type: init_type, + is_phantom, + } = read_world_query_field_type_info(&field.ty, false, &generic_names); + + let field_ident = field.ident.as_ref().unwrap().clone(); + if is_phantom { + phantom_field_idents.push(field_ident.clone()); + phantom_field_types.push(field.ty.clone()); + } else { + field_idents.push(field_ident.clone()); + field_types.push(field.ty.clone()); + query_types.push(query_type); + fetch_init_types.push(init_type); + } + } + + FetchImplTokens { + struct_name, + struct_name_read_only, + fetch_struct_name, + state_struct_name, + read_only_fetch_struct_name, + fetch_trait_punctuated_lifetimes, + impl_generics, + ty_generics, + where_clause, + has_read_only_attr, + has_world_lifetime, + world_lifetime, + state_lifetime, + phantom_field_idents, + phantom_field_types, + field_idents, + field_types, + query_types, + fetch_init_types, + } +} + +struct WorldQueryFieldTypeInfo { + /// We convert `Mut` to `&mut T` (because this is the type that implements `WorldQuery`) + /// and store it here. + query_type: Type, + /// The same as `query_type` but with `'fetch` lifetime. + fetch_init_type: Type, + is_phantom: bool, +} + +fn read_world_query_field_type_info( + ty: &Type, + is_tuple_element: bool, + generic_names: &[String], +) -> WorldQueryFieldTypeInfo { + let mut query_type = ty.clone(); + let mut fetch_init_type = ty.clone(); + let mut is_phantom = false; + + match (ty, &mut fetch_init_type) { + (Type::Path(path), Type::Path(path_init)) => { + if path.qself.is_some() { + // There's a risk that it contains a generic parameter that we can't test + // whether it's read-only or not. + panic!("Self type qualifiers aren't supported"); + } + + let segment = path.path.segments.last().unwrap(); + if segment.ident == "Option" { + // We expect that `Option` stores either `&T` or `Mut`. + let ty = match &segment.arguments { + PathArguments::AngleBracketed(args) => { + args.args.last().and_then(|arg| match arg { + GenericArgument::Type(ty) => Some(ty), + _ => None, + }) + } + _ => None, + }; + match ty.expect("Option type is expected to have generic arguments") { + // If it's a read-only reference, we just update the lifetime for `fetch_init_type` to `'fetch`. + Type::Reference(reference) => { + if reference.mutability.is_some() { + panic!("Invalid reference type: use `Mut` instead of `&mut T`"); + } + match &mut path_init.path.segments.last_mut().unwrap().arguments { + PathArguments::AngleBracketed(args) => { + match args.args.last_mut().unwrap() { + GenericArgument::Type(Type::Reference(ty)) => ty.lifetime = Some(Lifetime::new("'fetch", Span::call_site())), + _ => unreachable!(), + } + } + _ => unreachable!(), + } + } + // If it's a mutable reference, we set `query_type` and `fetch_init_type` to `&mut T`, + // we also update the lifetime for `fetch_init_type` to `'fetch`. + Type::Path(path) => { + assert_not_generic(path, generic_names); + + let segment = path.path.segments.last().unwrap(); + let ty_ident = &segment.ident; + if ty_ident == "Mut" { + let (mut_lifetime, mut_ty) = match &segment.arguments { + PathArguments::AngleBracketed(args) => { + (args.args.first().and_then(|arg| { + match arg { + GenericArgument::Lifetime(lifetime) => Some(lifetime.clone()), + _ => None, + } + }).expect("Mut is expected to have a lifetime"), + args.args.last().and_then(|arg| { + match arg { + GenericArgument::Type(ty) => Some(ty.clone()), + _ => None, + } + }).expect("Mut is expected to have a lifetime")) + } + _ => panic!("Mut type is expected to have generic arguments") + }; + + match query_type { + Type::Path(ref mut path) => { + let segment = path.path.segments.last_mut().unwrap(); + match segment.arguments { + PathArguments::AngleBracketed(ref mut args) => { + match args.args.last_mut().unwrap() { + GenericArgument::Type(ty) => { + *ty = Type::Reference(TypeReference { + and_token: Token![&](Span::call_site()), + lifetime: Some(mut_lifetime), + mutability: Some(Token![mut](Span::call_site())), + elem: Box::new(mut_ty.clone()), + }); + } + _ => unreachable!() + } + } + _ => unreachable!() + } + } + _ => unreachable!() + } + + let segment = path_init.path.segments.last_mut().unwrap(); + match segment.arguments { + PathArguments::AngleBracketed(ref mut args) => { + match args.args.last_mut().unwrap() { + GenericArgument::Type(ty) => { + *ty = Type::Reference(TypeReference { + and_token: Token![&](Span::call_site()), + lifetime: Some(Lifetime::new("'fetch", Span::call_site())), + mutability: Some(Token![mut](Span::call_site())), + elem: Box::new(mut_ty), + }); + } + _ => unreachable!() + } + } + _ => unreachable!() + } + } else { + panic!("Option type is expected to have a reference value (`Option<&T>` or `Option>`)"); + } + } + _ => panic!("Option type is expected to have a reference value (`Option<&T>` or `Option>`)"), + } + } else if segment.ident == "Mut" { + // If it's a mutable reference, we set `query_type` and `fetch_init_type` to `&mut T`, + // we also update the lifetime for `fetch_init_type` to `'fetch`. + let (mut_lifetime, mut_ty) = match &segment.arguments { + PathArguments::AngleBracketed(args) => { + let lt = args.args.first().and_then(|arg| { + match arg { + GenericArgument::Lifetime(lifetime) => Some(lifetime.clone()), + _ => None, + } + }).expect("`Mut` is expected to have a lifetime"); + let ty = args.args.last().and_then(|arg| { + match arg { + GenericArgument::Type(ty) => Some(ty.clone()), + _ => None, + } + }).expect("`Mut` is expected to have a lifetime"); + (lt, ty) + } + _ => panic!("`Mut` is expected to have generic arguments") + }; + + query_type = Type::Reference(TypeReference { + and_token: Token![&](Span::call_site()), + lifetime: Some(mut_lifetime), + mutability: Some(Token![mut](Span::call_site())), + elem: Box::new(mut_ty.clone()), + }); + fetch_init_type = Type::Reference(TypeReference { + and_token: Token![&](Span::call_site()), + lifetime: Some(Lifetime::new("'fetch", Span::call_site())), + mutability: Some(Token![mut](Span::call_site())), + elem: Box::new(mut_ty), + }); + } else if segment.ident == "PhantomData" { + if is_tuple_element { + panic!("Invalid tuple element: PhantomData"); + } + is_phantom = true; + } else if segment.ident != "Entity" { + assert_not_generic(path, generic_names); + + // Here, we assume that this member is another type that implements `Fetch`. + // If it doesn't, the code won't compile. + + // Also, we don't support `Fetch` implementations that specify custom `Item` types, + // except for the well-known ones, such as `WriteFetch`. + // See https://github.com/bevyengine/bevy/pull/2713#issuecomment-904773083. + + if let PathArguments::AngleBracketed(args) = &mut path_init.path.segments.last_mut().unwrap().arguments { + if let Some(GenericArgument::Lifetime(lt)) = args.args.first_mut() { + *lt = Lifetime::new("'fetch", Span::call_site()); + } + } + } + } + (Type::Reference(reference), Type::Reference(init_reference)) => { + if reference.mutability.is_some() { + panic!("Invalid reference type: use `Mut` instead of `&mut T`"); + } + init_reference.lifetime = Some(Lifetime::new("'fetch", Span::call_site())); + } + (Type::Tuple(tuple), Type::Tuple(init_tuple)) => { + let mut query_tuple_elems = tuple.elems.clone(); + query_tuple_elems.clear(); + let mut fetch_init_tuple_elems = query_tuple_elems.clone(); + for ty in tuple.elems.iter() { + let WorldQueryFieldTypeInfo { + query_type, + fetch_init_type, + is_phantom: _, + } = read_world_query_field_type_info( + ty, + true, + generic_names, + ); + query_tuple_elems.push(query_type); + fetch_init_tuple_elems.push(fetch_init_type); + } + match query_type { + Type::Tuple(ref mut tuple) => { + tuple.elems = query_tuple_elems; + } + _ => unreachable!(), + } + init_tuple.elems = fetch_init_tuple_elems; + } + _ => panic!("Only the following types (or their tuples) are supported for WorldQuery: &T, &mut T, Option<&T>, Option<&mut T>, Entity, or other structs that implement WorldQuery"), + } + + WorldQueryFieldTypeInfo { + query_type, + fetch_init_type, + is_phantom, + } +} + +fn assert_not_generic(type_path: &TypePath, generic_names: &[String]) { + // `get_ident` returns Some if it consists of a single segment, in this case it + // makes sense to ensure that it's not a generic. + if let Some(ident) = type_path.path.get_ident() { + let is_generic = generic_names + .iter() + .any(|generic_name| ident == generic_name.as_str()); + if is_generic { + panic!("Only references to generic types are supported: i.e. instead of `component: T`, use `component: &T` or `component: Mut` (optional references are supported as well)"); + } + } +} diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index ca5a6b17bff59..3dd18ec861c60 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -1,7 +1,9 @@ extern crate proc_macro; mod component; +mod fetch; +use crate::fetch::{derive_fetch_impl, derive_filter_fetch_impl}; use bevy_macro_utils::{derive_label, get_named_struct_fields, BevyManifest}; use proc_macro::TokenStream; use proc_macro2::Span; @@ -425,6 +427,18 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { }) } +/// Implement `WorldQuery` to use a struct as a parameter in a query +#[proc_macro_derive(Fetch, attributes(read_only, read_only_derive))] +pub fn derive_fetch(input: TokenStream) -> TokenStream { + derive_fetch_impl(input) +} + +/// Implement `FilterFetch` to use a struct as a filter parameter in a query +#[proc_macro_derive(FilterFetch)] +pub fn derive_filter_fetch(input: TokenStream) -> TokenStream { + derive_filter_fetch_impl(input) +} + #[proc_macro_derive(SystemLabel)] pub fn derive_system_label(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 5c3ed28038f70..0e4edc391ce0d 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -8,6 +8,7 @@ use crate::{ world::{Mut, World}, }; use bevy_ecs_macros::all_tuples; +pub use bevy_ecs_macros::{Fetch, FilterFetch}; use std::{ cell::UnsafeCell, marker::PhantomData, @@ -21,6 +22,10 @@ use std::{ /// /// See [`Query`](crate::system::Query) for a primer on queries. /// +/// If you want to implement a custom query, see [`Fetch`] trait documentation. +/// +/// If you want to implement a custom query filter, see [`FilterFetch`] trait documentation. +/// /// # Basic [`WorldQuery`]'s /// /// Here is a small list of the most important world queries to know about where `C` stands for a @@ -49,6 +54,213 @@ pub trait WorldQuery { pub type QueryItem<'w, 's, Q> = <::Fetch as Fetch<'w, 's>>::Item; +/// Types that implement this trait are responsible for fetching query items from tables or +/// archetypes. +/// +/// Every type that implements [`WorldQuery`] have their associated [`WorldQuery::Fetch`] and +/// [`WorldQuery::State`] types that are essential for fetching component data. If you want to +/// implement a custom query type, you'll need to implement [`Fetch`] and [`FetchState`] for +/// those associated types. +/// +/// You may want to implement a custom query for the following reasons: +/// - Named structs can be clearer and easier to use than complex query tuples. Access via struct +/// fields is more convenient than destructuring tuples or accessing them via `q.0, q.1, ...` +/// pattern and saves a lot of maintenance burden when adding or removing components. +/// - Nested queries enable the composition pattern and makes query types easier to re-use. +/// - You can bypass the limit of 15 components that exists for query tuples. +/// +/// Implementing the trait manually can allow for a fundamentally new type of behaviour. +/// +/// # Derive +/// +/// This trait can be derived with the [`derive@super::Fetch`] macro. +/// To do so, all fields in the struct must themselves impl [`WorldQuery`]. +/// +/// The derive macro implements [`WorldQuery`] for your type and declares two structs that +/// implement [`Fetch`] and [`FetchState`] and are used as [`WorldQuery::Fetch`] and +/// [`WorldQuery::State`] associated types respectively. +/// +/// **Note:** currently, the macro only supports named structs. +/// +/// ``` +/// # use bevy_ecs::prelude::*; +/// use bevy_ecs::query::Fetch; +/// +/// #[derive(Component)] +/// struct Foo; +/// #[derive(Component)] +/// struct Bar; +/// #[derive(Component)] +/// struct OptionalFoo; +/// #[derive(Component)] +/// struct OptionalBar; +/// +/// #[derive(Fetch)] +/// struct MyQuery<'w> { +/// entity: Entity, +/// foo: &'w Foo, +/// // `Mut<'w, T>` is a necessary replacement for `&'w mut T` +/// bar: Mut<'w, Bar>, +/// optional_foo: Option<&'w OptionalFoo>, +/// optional_bar: Option>, +/// } +/// +/// fn my_system(mut query: Query) { +/// for q in query.iter_mut() { +/// q.foo; +/// } +/// } +/// +/// # my_system.system(); +/// ``` +/// +/// ## Nested queries +/// +/// Using nested queries enable the composition pattern, which makes it possible to re-use other +/// query types. All types that implement [`WorldQuery`] (including the ones that use this derive +/// macro) are supported. +/// +/// ``` +/// # use bevy_ecs::prelude::*; +/// use bevy_ecs::query::Fetch; +/// +/// #[derive(Component)] +/// struct Foo; +/// #[derive(Component)] +/// struct Bar; +/// #[derive(Component)] +/// struct OptionalFoo; +/// #[derive(Component)] +/// struct OptionalBar; +/// +/// #[derive(Fetch)] +/// struct MyQuery<'w> { +/// foo: FooQuery<'w>, +/// bar: (&'w Bar, Option<&'w OptionalBar>) +/// } +/// +/// #[derive(Fetch)] +/// struct FooQuery<'w> { +/// foo: &'w Foo, +/// optional_foo: Option<&'w OptionalFoo>, +/// } +/// +/// ``` +/// +/// ## Read-only queries +/// +/// All queries that are derived with `Fetch` macro have their read-only variants with `ReadOnly` +/// suffix. If you are going to use a query only for reading, you can mark it with `read_only` +/// attribute. +/// +/// ``` +/// # use bevy_ecs::prelude::*; +/// use bevy_ecs::query::{Fetch, ReadOnlyFetch, WorldQuery}; +/// +/// #[derive(Component)] +/// struct Foo; +/// #[derive(Component)] +/// struct Bar; +/// +/// #[derive(Fetch)] +/// #[read_only] +/// struct FooQuery<'w> { +/// foo: &'w Foo, +/// bar_query: BarQueryReadOnly<'w>, +/// } +/// +/// #[derive(Fetch)] +/// struct BarQuery<'w> { +/// bar: &'w Bar, +/// } +/// +/// fn assert_read_only() {} +/// +/// assert_read_only::<::Fetch>(); +/// assert_read_only::<::ReadOnlyFetch>(); +/// // the following will fail to compile: +/// // assert_read_only::<::Fetch>(); +/// ``` +/// +/// If you want to use derive macros with read-only query variants, you need to pass them with +/// using `read_only_derive` attribute. +/// +/// # use bevy_ecs::prelude::*; +/// use bevy_ecs::query::{Fetch, ReadOnlyFetch, WorldQuery}; +/// +/// ``` +/// # use bevy_ecs::prelude::*; +/// use bevy_ecs::query::{Fetch, ReadOnlyFetch, WorldQuery}; +/// +/// #[derive(Component, Debug)] +/// struct Foo; +/// +/// #[derive(Fetch, Debug)] +/// #[read_only_derive(Debug)] +/// struct FooQuery<'w> { +/// foo: &'w Foo, +/// } +/// +/// fn assert_debug() {} +/// +/// assert_debug::(); +/// assert_debug::(); +/// ``` +/// +/// **Note:** if you mark a query that doesn't implement `ReadOnlyFetch` as `read_only`, +/// compilation will fail. We insert static checks as in the example above for every nested query +/// marked as `read_only`. (They neither affect the runtime, nor pollute your local namespace.) +/// +/// ```compile_fail +/// # use bevy_ecs::prelude::*; +/// use bevy_ecs::query::{Fetch, ReadOnlyFetch, WorldQuery}; +/// +/// #[derive(Component)] +/// struct Foo; +/// #[derive(Component)] +/// struct Bar; +/// +/// #[derive(Fetch)] +/// #[read_only] +/// struct FooQuery<'w> { +/// foo: &'w Foo, +/// bar_query: BarQuery<'w>, +/// } +/// +/// #[derive(Fetch)] +/// struct BarQuery<'w> { +/// bar: Mut<'w, Bar>, +/// } +/// ``` +/// +/// ## Limitations +/// +/// Currently, we don't support members that have a manual [`WorldQuery`] implementation if their +/// [`Fetch::Item`] is different from the member type. For instance, the following code won't +/// compile: +/// +/// ```ignore +/// #[derive(Component)] +/// struct CustomQueryParameter; +/// #[derive(Component)] +/// struct ItemDataType; +/// +/// struct CustomQueryParameterFetch { +/// // ... +/// } +/// +/// impl<'w, 's> Fetch<'w, 's> for CustomQueryParameterFetch { +/// type Item = ItemDataType; +/// +/// // ... +/// } +/// +/// #[derive(Fetch)] +/// struct MyQuery { +/// custom_item: ItemDataType, +/// } +/// +/// ``` pub trait Fetch<'world, 'state>: Sized { type Item; type State: FetchState; diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index e84ec3db26394..7dcd759b2fb90 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -11,6 +11,53 @@ use std::{cell::UnsafeCell, marker::PhantomData, ptr}; /// Extension trait for [`Fetch`] containing methods used by query filters. /// This trait exists to allow "short circuit" behaviors for relevant query filter fetches. +/// +/// This trait is automatically implemented for every type that implements [`Fetch`] trait and +/// specifies `bool` as the associated type for [`Fetch::Item`]. +/// +/// Using [`derive@super::FilterFetch`] macro allows creating custom query filters. +/// You may want to implement a custom query filter for the following reasons: +/// - Nested query filters enable the composition pattern and makes them easier to re-use. +/// - You can bypass the limit of 15 components that exists for query filters declared as tuples. +/// +/// Implementing the trait manually can allow for a fundamentally new type of behaviour. +/// +/// ## Derive +/// +/// This trait can be derived with the [`derive@super::FilterFetch`] macro. +/// To do so, all fields in the struct must be filters themselves (their [`WorldQuery::Fetch`] +/// associated types should implement [`FilterFetch`]). +/// +/// **Note:** currently, the macro only supports named structs. +/// +/// ``` +/// # use bevy_ecs::prelude::*; +/// use bevy_ecs::{query::FilterFetch, component::Component}; +/// +/// #[derive(Component)] +/// struct Foo; +/// #[derive(Component)] +/// struct Bar; +/// #[derive(Component)] +/// struct Baz; +/// #[derive(Component)] +/// struct Qux; +/// +/// #[derive(FilterFetch)] +/// struct MyFilter { +/// _foo: With, +/// _bar: With, +/// _or: Or<(With, Changed, Added)>, +/// _generic_tuple: (With, Without

), +/// _tp: std::marker::PhantomData<(T, P)>, +/// } +/// +/// fn my_system(query: Query>) { +/// for _ in query.iter() {} +/// } +/// +/// # my_system.system(); +/// ``` pub trait FilterFetch: for<'w, 's> Fetch<'w, 's> { /// # Safety /// diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 7ed59883ebce6..36e5195e82c42 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -189,7 +189,7 @@ fn assert_component_access_compatibility( .collect::>(); let accesses = conflicting_components.join(", "); panic!("error[B0001]: Query<{}, {}> in system {} accesses component(s) {} in a way that conflicts with a previous system parameter. Consider using `Without` to create disjoint Queries or merging conflicting Queries into a `QuerySet`.", - query_type, filter_type, system_name, accesses); + query_type, filter_type, system_name, accesses); } pub struct QuerySet<'w, 's, T> { @@ -204,6 +204,7 @@ pub struct QuerySetState(T); impl_query_set!(); pub trait Resource: Send + Sync + 'static {} + impl Resource for T where T: Send + Sync + 'static {} /// Shared borrow of a resource. diff --git a/examples/README.md b/examples/README.md index aeaff8c7845f2..d81f3d44ff47a 100644 --- a/examples/README.md +++ b/examples/README.md @@ -166,6 +166,7 @@ Example | File | Description --- | --- | --- `ecs_guide` | [`ecs/ecs_guide.rs`](./ecs/ecs_guide.rs) | Full guide to Bevy's ECS `component_change_detection` | [`ecs/component_change_detection.rs`](./ecs/component_change_detection.rs) | Change detection on components +`custom_query_param` | [`ecs/custom_query_param.rs`](./ecs/custom_query_param.rs) | Groups commonly used compound queries and query filters into a single type `event` | [`ecs/event.rs`](./ecs/event.rs) | Illustrates event creation, activation, and reception `fixed_timestep` | [`ecs/fixed_timestep.rs`](./ecs/fixed_timestep.rs) | Shows how to create systems that run every fixed timestep, rather than every tick `generic_system` | [`ecs/generic_system.rs`](./ecs/generic_system.rs) | Shows how to create systems that can be reused with different types diff --git a/examples/ecs/custom_query_param.rs b/examples/ecs/custom_query_param.rs new file mode 100644 index 0000000000000..3294c7a6deabf --- /dev/null +++ b/examples/ecs/custom_query_param.rs @@ -0,0 +1,189 @@ +use bevy::{ + ecs::{ + component::Component, + query::{Fetch, FilterFetch}, + }, + prelude::*, +}; +use std::{fmt::Debug, marker::PhantomData}; + +/// This examples illustrates the usage of `Fetch` and `FilterFetch` derive macros, that allow +/// defining custom query and filter types. +/// +/// While regular tuple queries work great in most of simple scenarios, using custom queries +/// declared as named structs can bring the following advantages: +/// - They help to avoid destructuring or using `q.0, q.1, ...` access pattern. +/// - Adding, removing components or changing items order with structs greatly reduces maintenance +/// burden, as you don't need to update statements that destructure tuples, care abort order +/// of elements, etc. Instead, you can just add or remove places where a certain element is used. +/// - Named structs enable the composition pattern, that makes query types easier to re-use. +/// - You can bypass the limit of 15 components that exists for query tuples. +/// +/// For more details on the `Fetch` and `FilterFetch` derive macros, see their documentation. +fn main() { + App::new() + .add_startup_system(spawn) + .add_system(print_components_iter_mut.label("print_components_iter_mut")) + .add_system( + print_components_iter + .label("print_components_iter") + .after("print_components_iter_mut"), + ) + .add_system( + print_components_read_only + .label("print_components_read_only") + .after("print_components_iter"), + ) + .add_system(print_components_tuple.after("print_components_read_only")) + .run(); +} + +#[derive(Component, Debug)] +struct ComponentA; +#[derive(Component, Debug)] +struct ComponentB; +#[derive(Component, Debug)] +struct ComponentC; +#[derive(Component, Debug)] +struct ComponentD; +#[derive(Component, Debug)] +struct ComponentZ; + +#[derive(Fetch)] +struct CustomQuery<'w, T: Component + Debug, P: Component + Debug> { + entity: Entity, + // `Mut<'w, T>` is a necessary replacement for `&'w mut T` + a: Mut<'w, ComponentA>, + b: Option>, + nested: NestedQuery<'w>, + generic: GenericQuery<'w, T, P>, + #[allow(dead_code)] + empty: EmptyQuery<'w>, +} + +// This is a valid query as well, which would iterate over every entity. +#[derive(Fetch)] +struct EmptyQuery<'w> { + _w: std::marker::PhantomData<&'w ()>, +} + +#[derive(Fetch, Debug)] +#[read_only_derive(Debug)] +#[allow(dead_code)] +struct NestedQuery<'w> { + c: &'w ComponentC, + d: Option<&'w ComponentD>, +} + +#[derive(Fetch, Debug)] +#[read_only_derive(Debug)] +#[allow(dead_code)] +struct GenericQuery<'w, T: Component, P: Component> { + generic: (&'w T, &'w P), +} + +#[derive(FilterFetch)] +struct QueryFilter { + _c: With, + _d: With, + _or: Or<(Added, Changed, Without)>, + _generic_tuple: (With, With

), + _tp: PhantomData<(T, P)>, +} + +fn spawn(mut commands: Commands) { + commands + .spawn() + .insert(ComponentA) + .insert(ComponentB) + .insert(ComponentC) + .insert(ComponentD); +} + +fn print_components_iter_mut( + mut query: Query, QueryFilter>, +) { + println!("Print components (iter_mut):"); + for e in query.iter_mut() { + println!("Entity: {:?}", e.entity); + println!("A: {:?}", e.a); + println!("B: {:?}", e.b); + println!("Nested: {:?}", e.nested); + println!("Generic: {:?}", e.generic); + } + println!(); +} + +fn print_components_iter( + query: Query, QueryFilter>, +) { + println!("Print components (iter):"); + for e in query.iter() { + // Note that the actual type is different when you iterate with `iter`. + let e: CustomQueryReadOnly<'_, _, _> = e; + println!("Entity: {:?}", e.entity); + println!("A: {:?}", e.a); + println!("B: {:?}", e.b); + println!("Nested: {:?}", e.nested); + println!("Generic: {:?}", e.generic); + } + println!(); +} + +// If you are going to use a query only for reading, you can mark it with `read_only` attribute +// to avoid creating an additional type with `ReadOnly` suffix. +#[derive(Fetch)] +#[read_only] +struct ReadOnlyCustomQuery<'w, T: Component + Debug, P: Component + Debug> { + entity: Entity, + a: &'w ComponentA, + b: Option<&'w ComponentB>, + nested: NestedQueryReadOnly<'w>, + generic: GenericQueryReadOnly<'w, T, P>, + #[allow(dead_code)] + empty: EmptyQueryReadOnly<'w>, +} + +fn print_components_read_only( + query: Query, QueryFilter>, +) { + println!("Print components (read_only):"); + for e in query.iter() { + let e: ReadOnlyCustomQuery<'_, _, _> = e; + println!("Entity: {:?}", e.entity); + println!("A: {:?}", e.a); + println!("B: {:?}", e.b); + println!("Nested: {:?}", e.nested); + println!("Generic: {:?}", e.generic); + } + println!(); +} + +type NestedTupleQuery<'w> = (&'w ComponentC, &'w ComponentD); +type GenericTupleQuery<'w, T, P> = (&'w T, &'w P); + +fn print_components_tuple( + query: Query< + ( + Entity, + &ComponentA, + &ComponentB, + NestedTupleQuery, + GenericTupleQuery, + ), + ( + With, + With, + Or<(Added, Changed, Without)>, + ), + >, +) { + println!("Print components (tuple):"); + for (entity, a, b, nested, (generic_c, generic_d)) in query.iter() { + println!("Entity: {:?}", entity); + println!("A: {:?}", a); + println!("B: {:?}", b); + println!("Nested: {:?} {:?}", nested.0, nested.1); + println!("Generic: {:?} {:?}", generic_c, generic_d); + } +} From 3cb3fb63bbe104b38b80513dd8d3a904bd11cca2 Mon Sep 17 00:00:00 2001 From: Vladyslav Batyrenko Date: Mon, 10 Jan 2022 20:49:36 +0200 Subject: [PATCH 02/14] Apply documentation suggestions Co-authored-by: Alice Cecile --- crates/bevy_ecs/macros/src/fetch.rs | 2 +- crates/bevy_ecs/src/query/fetch.rs | 80 ++++++++++++++++++++++------- examples/ecs/custom_query_param.rs | 4 +- 3 files changed, 64 insertions(+), 22 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 9386e491227bc..d5967e3edf9f1 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -374,7 +374,7 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream { type ReadOnlyFetch = #fetch_struct_name #ty_generics; } - /// SAFETY: each item in the struct is read only + /// SAFETY: each item in the struct is read-only as filters never actually fetch any data that could be mutated unsafe impl #impl_generics #path::query::ReadOnlyFetch for #fetch_struct_name #ty_generics #where_clause {} }); tokens diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 0e4edc391ce0d..70295fb747607 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -145,48 +145,90 @@ pub type QueryItem<'w, 's, Q> = <::Fetch as Fetch<'w, 's>>::Ite /// optional_foo: Option<&'w OptionalFoo>, /// } /// +/// // You can also compose derived queries with regular ones in tuples. +/// fn my_system(query: Query<(&Foo, MyQuery, FooQuery)>) { +/// for (foo, my_query, foo_query) in query.iter() { +/// foo; my_query; foo_query; +/// } +/// } +/// +/// # my_system.system(); /// ``` /// /// ## Read-only queries /// /// All queries that are derived with `Fetch` macro have their read-only variants with `ReadOnly` -/// suffix. If you are going to use a query only for reading, you can mark it with `read_only` -/// attribute. +/// suffix. If you are going to use a query which can only read data from the ECS, you can mark it +/// with the `read_only` attribute. /// /// ``` /// # use bevy_ecs::prelude::*; /// use bevy_ecs::query::{Fetch, ReadOnlyFetch, WorldQuery}; /// /// #[derive(Component)] -/// struct Foo; +/// struct A(i32); /// #[derive(Component)] -/// struct Bar; +/// struct B(i32); /// /// #[derive(Fetch)] -/// #[read_only] -/// struct FooQuery<'w> { -/// foo: &'w Foo, -/// bar_query: BarQueryReadOnly<'w>, +/// struct SumQuery<'w> { +/// a: &'w A, +/// b: &'w B, +/// } +/// +/// // This implementation is only available when iterating with `iter_mut`. +/// impl<'w> SumQuery<'w> { +/// fn calc(&self) -> i32 { +/// let Self { a: A(a), b: B(b) } = self; +/// a + b +/// } +/// } +/// +/// // If you want to use it with `iter`, you'll need to write an additional implementation. +/// impl<'w> SumQueryReadOnly<'w> { +/// fn calc(&self) -> i32 { +/// let Self { a: A(a), b: B(b) } = self; +/// a + b +/// } /// } /// +/// // If you are never going to mutate the data, you can mark the query with the `read_only` +/// // attribute and write the implementation only once. /// #[derive(Fetch)] -/// struct BarQuery<'w> { -/// bar: &'w Bar, +/// #[read_only] +/// struct ProductQuery<'w> { +/// a: &'w A, +/// b: &'w B, /// } /// -/// fn assert_read_only() {} +/// impl<'w> ProductQuery<'w> { +/// fn calc(&self) -> i32 { +/// let Self { a: A(a), b: B(b) } = self; +/// a * b +/// } +/// } /// -/// assert_read_only::<::Fetch>(); -/// assert_read_only::<::ReadOnlyFetch>(); -/// // the following will fail to compile: -/// // assert_read_only::<::Fetch>(); +/// fn my_system(mut sum_query: Query, product_query: Query) { +/// // Iterator's item is `SumQueryReadOnly`. +/// for sum in sum_query.iter() { +/// println!("Sum: {}", sum.calc()); +/// } +/// // Iterator's item is `SumQuery`. +/// for sum in sum_query.iter_mut() { +/// println!("Sum (mut): {}", sum.calc()); +/// } +/// // Iterator's item is `ProductQuery`. +/// for product in product_query.iter() { +/// println!("Product: {}", product.calc()); +/// } +/// } /// ``` /// /// If you want to use derive macros with read-only query variants, you need to pass them with -/// using `read_only_derive` attribute. -/// -/// # use bevy_ecs::prelude::*; -/// use bevy_ecs::query::{Fetch, ReadOnlyFetch, WorldQuery}; +/// using the `read_only_derive` attribute. As the `Fetch` macro generates an additional struct +/// for `ReadOnlyFetch` implementation (granted it isn't marked with the `read_only` attribute), +/// you may want it to inherit the same derives. Since derive macros can't access information about +/// other derives, they need to be passed manually with the `read_only_derive` attribute. /// /// ``` /// # use bevy_ecs::prelude::*; diff --git a/examples/ecs/custom_query_param.rs b/examples/ecs/custom_query_param.rs index 3294c7a6deabf..d8329a512d7b7 100644 --- a/examples/ecs/custom_query_param.rs +++ b/examples/ecs/custom_query_param.rs @@ -14,7 +14,7 @@ use std::{fmt::Debug, marker::PhantomData}; /// declared as named structs can bring the following advantages: /// - They help to avoid destructuring or using `q.0, q.1, ...` access pattern. /// - Adding, removing components or changing items order with structs greatly reduces maintenance -/// burden, as you don't need to update statements that destructure tuples, care abort order +/// burden, as you don't need to update statements that destructure tuples, care about order /// of elements, etc. Instead, you can just add or remove places where a certain element is used. /// - Named structs enable the composition pattern, that makes query types easier to re-use. /// - You can bypass the limit of 15 components that exists for query tuples. @@ -130,7 +130,7 @@ fn print_components_iter( println!(); } -// If you are going to use a query only for reading, you can mark it with `read_only` attribute +// If you are never going to mutate the data in a query, you can mark it with `read_only` attribute // to avoid creating an additional type with `ReadOnly` suffix. #[derive(Fetch)] #[read_only] From 8574a8a7bd8b919ecb65950190f20acecd9874a1 Mon Sep 17 00:00:00 2001 From: mvlabat Date: Wed, 12 Jan 2022 00:14:29 +0200 Subject: [PATCH 03/14] Make derived queries immutable by default --- crates/bevy_ecs/macros/src/fetch.rs | 68 +++++----- crates/bevy_ecs/macros/src/lib.rs | 2 +- crates/bevy_ecs/src/query/fetch.rs | 184 +++++++++++++--------------- examples/ecs/custom_query_param.rs | 82 +++++++------ 4 files changed, 159 insertions(+), 177 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index d5967e3edf9f1..1abae505537e1 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -9,7 +9,7 @@ use syn::{ use crate::bevy_ecs_path; -static READ_ONLY_ATTRIBUTE_NAME: &str = "read_only"; +static MUTABLE_ATTRIBUTE_NAME: &str = "mutable"; static READ_ONLY_DERIVE_ATTRIBUTE_NAME: &str = "read_only_derive"; pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { @@ -26,7 +26,7 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { ty_generics, where_clause, has_world_lifetime, - has_read_only_attr, + has_mutable_attr, world_lifetime, state_lifetime, phantom_field_idents, @@ -47,8 +47,8 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { .map_or(false, |ident| ident == READ_ONLY_DERIVE_ATTRIBUTE_NAME) }); let read_only_derive_macro_call = if let Some(read_only_derive_attr) = read_only_derive_attr { - if has_read_only_attr { - panic!("Attributes `read_only` and `read_only_derive` are mutually exclusive"); + if !has_mutable_attr { + panic!("Attribute `read_only_derive` can only be be used for a struct marked with the `mutable` attribute"); } let derive_args = &read_only_derive_attr.tokens; quote! { #[derive #derive_args] } @@ -74,25 +74,10 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { let path = bevy_ecs_path(); - let struct_read_only_declaration = if has_read_only_attr { + let struct_read_only_declaration = if has_mutable_attr { quote! { - // Statically checks that the safety guarantee actually holds true. We need this to make - // sure that we don't compile `ReadOnlyFetch` if our struct contains nested `WorldQuery` - // members that don't implement it. - #[allow(dead_code)] - const _: () = { - fn assert_readonly() {} - - // We generate a readonly assertion for every struct member. - fn assert_all #impl_generics () #where_clause { - #(assert_readonly::<<#query_types as #path::query::WorldQuery>::Fetch>();)* - } - }; - } - } else { - quote! { - // TODO: it would be great to be able to dedup this by just deriving `Fetch` again with `read_only` attribute, - // but supporting QSelf types is tricky. + // TODO: it would be great to be able to dedup this by just deriving `Fetch` again + // without the `mutable` attribute, but supporting QSelf types is tricky. #read_only_derive_macro_call struct #struct_name_read_only #impl_generics #where_clause { #(#field_idents: <<#query_types as #path::query::WorldQuery>::ReadOnlyFetch as #path::query::Fetch<#world_lifetime, #world_lifetime>>::Item,)* @@ -154,6 +139,21 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { type ReadOnlyFetch = #read_only_fetch_struct_name #ty_generics; } } + } else { + quote! { + // Statically checks that the safety guarantee actually holds true. We need this to make + // sure that we don't compile `ReadOnlyFetch` if our struct contains nested `WorldQuery` + // members that don't implement it. + #[allow(dead_code)] + const _: () = { + fn assert_readonly() {} + + // We generate a readonly assertion for every struct member. + fn assert_all #impl_generics () #where_clause { + #(assert_readonly::<<#query_types as #path::query::WorldQuery>::Fetch>();)* + } + }; + } }; let tokens = TokenStream::from(quote! { @@ -265,7 +265,7 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream { ty_generics, where_clause, world_lifetime, - has_read_only_attr: _, + has_mutable_attr: _, has_world_lifetime, state_lifetime, phantom_field_idents, @@ -383,7 +383,7 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream { // This struct is used to share common tokens between `Fetch` and `FilterFetch` implementations. struct FetchImplTokens<'a> { struct_name: Ident, - // Equals `struct_name` if `has_read_only_attr` is true. + // Equals `struct_name` if `has_mutable_attr` is false. struct_name_read_only: Ident, fetch_struct_name: Ident, state_struct_name: Ident, @@ -392,7 +392,7 @@ struct FetchImplTokens<'a> { impl_generics: ImplGenerics<'a>, ty_generics: TypeGenerics<'a>, where_clause: Option<&'a WhereClause>, - has_read_only_attr: bool, + has_mutable_attr: bool, has_world_lifetime: bool, world_lifetime: GenericParam, state_lifetime: GenericParam, @@ -405,10 +405,10 @@ struct FetchImplTokens<'a> { } fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens { - let has_read_only_attr = ast.attrs.iter().any(|attr| { + let has_mutable_attr = ast.attrs.iter().any(|attr| { attr.path .get_ident() - .map_or(false, |ident| ident == READ_ONLY_ATTRIBUTE_NAME) + .map_or(false, |ident| ident == MUTABLE_ATTRIBUTE_NAME) }); let world_lifetime = ast.generics.params.first().and_then(|param| match param { @@ -429,17 +429,17 @@ fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens { let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl(); let struct_name = ast.ident.clone(); - let struct_name_read_only = if has_read_only_attr { - ast.ident.clone() - } else { + let struct_name_read_only = if has_mutable_attr { Ident::new(&format!("{}ReadOnly", struct_name), Span::call_site()) + } else { + ast.ident.clone() }; let fetch_struct_name = Ident::new(&format!("{}Fetch", struct_name), Span::call_site()); let state_struct_name = Ident::new(&format!("{}State", struct_name), Span::call_site()); - let read_only_fetch_struct_name = if has_read_only_attr { - fetch_struct_name.clone() - } else { + let read_only_fetch_struct_name = if has_mutable_attr { Ident::new(&format!("{}ReadOnlyFetch", struct_name), Span::call_site()) + } else { + fetch_struct_name.clone() }; let fields = match &ast.data { @@ -496,7 +496,7 @@ fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens { impl_generics, ty_generics, where_clause, - has_read_only_attr, + has_mutable_attr, has_world_lifetime, world_lifetime, state_lifetime, diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 3dd18ec861c60..523cb1de54c96 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -428,7 +428,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { } /// Implement `WorldQuery` to use a struct as a parameter in a query -#[proc_macro_derive(Fetch, attributes(read_only, read_only_derive))] +#[proc_macro_derive(Fetch, attributes(mutable, read_only_derive))] pub fn derive_fetch(input: TokenStream) -> TokenStream { derive_fetch_impl(input) } diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 70295fb747607..346dc937d4505 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -90,23 +90,16 @@ pub type QueryItem<'w, 's, Q> = <::Fetch as Fetch<'w, 's>>::Ite /// struct Foo; /// #[derive(Component)] /// struct Bar; -/// #[derive(Component)] -/// struct OptionalFoo; -/// #[derive(Component)] -/// struct OptionalBar; /// /// #[derive(Fetch)] /// struct MyQuery<'w> { /// entity: Entity, /// foo: &'w Foo, -/// // `Mut<'w, T>` is a necessary replacement for `&'w mut T` -/// bar: Mut<'w, Bar>, -/// optional_foo: Option<&'w OptionalFoo>, -/// optional_bar: Option>, +/// bar: Option<&'w Bar>, /// } /// -/// fn my_system(mut query: Query) { -/// for q in query.iter_mut() { +/// fn my_system(query: Query) { +/// for q in query.iter() { /// q.foo; /// } /// } @@ -114,121 +107,65 @@ pub type QueryItem<'w, 's, Q> = <::Fetch as Fetch<'w, 's>>::Ite /// # my_system.system(); /// ``` /// -/// ## Nested queries +/// ## Mutable queries /// -/// Using nested queries enable the composition pattern, which makes it possible to re-use other -/// query types. All types that implement [`WorldQuery`] (including the ones that use this derive -/// macro) are supported. -/// -/// ``` -/// # use bevy_ecs::prelude::*; -/// use bevy_ecs::query::Fetch; -/// -/// #[derive(Component)] -/// struct Foo; -/// #[derive(Component)] -/// struct Bar; -/// #[derive(Component)] -/// struct OptionalFoo; -/// #[derive(Component)] -/// struct OptionalBar; -/// -/// #[derive(Fetch)] -/// struct MyQuery<'w> { -/// foo: FooQuery<'w>, -/// bar: (&'w Bar, Option<&'w OptionalBar>) -/// } -/// -/// #[derive(Fetch)] -/// struct FooQuery<'w> { -/// foo: &'w Foo, -/// optional_foo: Option<&'w OptionalFoo>, -/// } -/// -/// // You can also compose derived queries with regular ones in tuples. -/// fn my_system(query: Query<(&Foo, MyQuery, FooQuery)>) { -/// for (foo, my_query, foo_query) in query.iter() { -/// foo; my_query; foo_query; -/// } -/// } -/// -/// # my_system.system(); -/// ``` -/// -/// ## Read-only queries -/// -/// All queries that are derived with `Fetch` macro have their read-only variants with `ReadOnly` -/// suffix. If you are going to use a query which can only read data from the ECS, you can mark it -/// with the `read_only` attribute. +/// All queries that are derived with `Fetch` macro provide only an immutable access by default. +/// If you need a mutable access to components, you can mark a struct with the `mutable` attribute. +/// The macro will still generate a read-only variant of a query suffixed with `ReadOnly`. /// /// ``` /// # use bevy_ecs::prelude::*; /// use bevy_ecs::query::{Fetch, ReadOnlyFetch, WorldQuery}; /// /// #[derive(Component)] -/// struct A(i32); +/// struct Health(f32); /// #[derive(Component)] -/// struct B(i32); +/// struct Buff(f32); /// /// #[derive(Fetch)] -/// struct SumQuery<'w> { -/// a: &'w A, -/// b: &'w B, +/// #[mutable] +/// struct HealthQuery<'w> { +/// // `Mut<'w, T>` is a necessary replacement for `&'w mut T` +/// health: Mut<'w, Health>, +/// buff: Option>, /// } /// /// // This implementation is only available when iterating with `iter_mut`. -/// impl<'w> SumQuery<'w> { -/// fn calc(&self) -> i32 { -/// let Self { a: A(a), b: B(b) } = self; -/// a + b +/// impl<'w> HealthQuery<'w> { +/// fn damage(&mut self, value: f32) { +/// self.health.0 -= value; /// } -/// } /// -/// // If you want to use it with `iter`, you'll need to write an additional implementation. -/// impl<'w> SumQueryReadOnly<'w> { -/// fn calc(&self) -> i32 { -/// let Self { a: A(a), b: B(b) } = self; -/// a + b +/// fn total(&self) -> f32 { +/// self.health.0 + self.buff.as_deref().map_or(0.0, |Buff(buff)| *buff) /// } /// } /// -/// // If you are never going to mutate the data, you can mark the query with the `read_only` -/// // attribute and write the implementation only once. -/// #[derive(Fetch)] -/// #[read_only] -/// struct ProductQuery<'w> { -/// a: &'w A, -/// b: &'w B, -/// } -/// -/// impl<'w> ProductQuery<'w> { -/// fn calc(&self) -> i32 { -/// let Self { a: A(a), b: B(b) } = self; -/// a * b +/// // If you want to use it with `iter`, you'll need to write an additional implementation. +/// impl<'w> HealthQueryReadOnly<'w> { +/// fn total(&self) -> f32 { +/// self.health.0 + self.buff.map_or(0.0, |Buff(buff)| *buff) /// } /// } /// -/// fn my_system(mut sum_query: Query, product_query: Query) { -/// // Iterator's item is `SumQueryReadOnly`. -/// for sum in sum_query.iter() { -/// println!("Sum: {}", sum.calc()); +/// fn my_system(mut health_query: Query) { +/// // Iterator's item is `HealthQueryReadOnly`. +/// for health in health_query.iter() { +/// println!("Total: {}", health.total()); /// } -/// // Iterator's item is `SumQuery`. -/// for sum in sum_query.iter_mut() { -/// println!("Sum (mut): {}", sum.calc()); -/// } -/// // Iterator's item is `ProductQuery`. -/// for product in product_query.iter() { -/// println!("Product: {}", product.calc()); +/// // Iterator's item is `HealthQuery`. +/// for mut health in health_query.iter_mut() { +/// health.damage(1.0); +/// println!("Total (mut): {}", health.total()); /// } /// } /// ``` /// /// If you want to use derive macros with read-only query variants, you need to pass them with -/// using the `read_only_derive` attribute. As the `Fetch` macro generates an additional struct -/// for `ReadOnlyFetch` implementation (granted it isn't marked with the `read_only` attribute), -/// you may want it to inherit the same derives. Since derive macros can't access information about -/// other derives, they need to be passed manually with the `read_only_derive` attribute. +/// using the `read_only_derive` attribute. When `Fetch` macro generates an additional struct +/// for a mutable query, it doesn't automatically inherit the same derives. Since derive macros +/// can't access information about other derives, they need to be passed manually with the +/// `read_only_derive` attribute. /// /// ``` /// # use bevy_ecs::prelude::*; @@ -238,6 +175,7 @@ pub type QueryItem<'w, 's, Q> = <::Fetch as Fetch<'w, 's>>::Ite /// struct Foo; /// /// #[derive(Fetch, Debug)] +/// #[mutable] /// #[read_only_derive(Debug)] /// struct FooQuery<'w> { /// foo: &'w Foo, @@ -249,9 +187,10 @@ pub type QueryItem<'w, 's, Q> = <::Fetch as Fetch<'w, 's>>::Ite /// assert_debug::(); /// ``` /// -/// **Note:** if you mark a query that doesn't implement `ReadOnlyFetch` as `read_only`, -/// compilation will fail. We insert static checks as in the example above for every nested query -/// marked as `read_only`. (They neither affect the runtime, nor pollute your local namespace.) +/// **Note:** if you omit the `mutable` attribute for a query that doesn't implement +/// `ReadOnlyFetch`, compilation will fail. We insert static checks as in the example above for +/// every query component and a nested query. +/// (The checks neither affect the runtime, nor pollute your local namespace.) /// /// ```compile_fail /// # use bevy_ecs::prelude::*; @@ -263,18 +202,59 @@ pub type QueryItem<'w, 's, Q> = <::Fetch as Fetch<'w, 's>>::Ite /// struct Bar; /// /// #[derive(Fetch)] -/// #[read_only] /// struct FooQuery<'w> { /// foo: &'w Foo, /// bar_query: BarQuery<'w>, /// } /// /// #[derive(Fetch)] +/// #[mutable] /// struct BarQuery<'w> { /// bar: Mut<'w, Bar>, /// } /// ``` /// +/// ## Nested queries +/// +/// Using nested queries enable the composition pattern, which makes it possible to re-use other +/// query types. All types that implement [`WorldQuery`] (including the ones that use this derive +/// macro) are supported. +/// +/// ``` +/// # use bevy_ecs::prelude::*; +/// use bevy_ecs::query::Fetch; +/// +/// #[derive(Component)] +/// struct Foo; +/// #[derive(Component)] +/// struct Bar; +/// #[derive(Component)] +/// struct OptionalFoo; +/// #[derive(Component)] +/// struct OptionalBar; +/// +/// #[derive(Fetch)] +/// struct MyQuery<'w> { +/// foo: FooQuery<'w>, +/// bar: (&'w Bar, Option<&'w OptionalBar>) +/// } +/// +/// #[derive(Fetch)] +/// struct FooQuery<'w> { +/// foo: &'w Foo, +/// optional_foo: Option<&'w OptionalFoo>, +/// } +/// +/// // You can also compose derived queries with regular ones in tuples. +/// fn my_system(query: Query<(&Foo, MyQuery, FooQuery)>) { +/// for (foo, my_query, foo_query) in query.iter() { +/// foo; my_query; foo_query; +/// } +/// } +/// +/// # my_system.system(); +/// ``` +/// /// ## Limitations /// /// Currently, we don't support members that have a manual [`WorldQuery`] implementation if their diff --git a/examples/ecs/custom_query_param.rs b/examples/ecs/custom_query_param.rs index d8329a512d7b7..5c059f8838056 100644 --- a/examples/ecs/custom_query_param.rs +++ b/examples/ecs/custom_query_param.rs @@ -23,18 +23,18 @@ use std::{fmt::Debug, marker::PhantomData}; fn main() { App::new() .add_startup_system(spawn) - .add_system(print_components_iter_mut.label("print_components_iter_mut")) + .add_system(print_components_read_only.label("print_components_read_only")) + .add_system( + print_components_iter_mut + .label("print_components_iter_mut") + .after("print_components_read_only"), + ) .add_system( print_components_iter .label("print_components_iter") .after("print_components_iter_mut"), ) - .add_system( - print_components_read_only - .label("print_components_read_only") - .after("print_components_iter"), - ) - .add_system(print_components_tuple.after("print_components_read_only")) + .add_system(print_components_tuple.after("print_components_iter_mut")) .run(); } @@ -50,6 +50,39 @@ struct ComponentD; struct ComponentZ; #[derive(Fetch)] +struct ReadOnlyCustomQuery<'w, T: Component + Debug, P: Component + Debug> { + entity: Entity, + a: &'w ComponentA, + b: Option<&'w ComponentB>, + nested: NestedQuery<'w>, + generic: GenericQuery<'w, T, P>, + #[allow(dead_code)] + empty: EmptyQuery<'w>, +} + +fn print_components_read_only( + query: Query, QueryFilter>, +) { + println!("Print components (read_only):"); + for e in query.iter() { + let e: ReadOnlyCustomQuery<'_, _, _> = e; + println!("Entity: {:?}", e.entity); + println!("A: {:?}", e.a); + println!("B: {:?}", e.b); + println!("Nested: {:?}", e.nested); + println!("Generic: {:?}", e.generic); + } + println!(); +} + +// If you are going to mutate the data in a query, you must mark it with the `mutable` attribute. +// The `Fetch` derive macro will still create a read-only version, which will be have `ReadOnly` +// suffix. +// Note: if you want to use derive macros with read-only query variants, you need to pass them with +// using the `read_only_derive` attribute. +#[derive(Fetch, Debug)] +#[mutable] +#[read_only_derive(Debug)] struct CustomQuery<'w, T: Component + Debug, P: Component + Debug> { entity: Entity, // `Mut<'w, T>` is a necessary replacement for `&'w mut T` @@ -62,13 +95,12 @@ struct CustomQuery<'w, T: Component + Debug, P: Component + Debug> { } // This is a valid query as well, which would iterate over every entity. -#[derive(Fetch)] +#[derive(Fetch, Debug)] struct EmptyQuery<'w> { _w: std::marker::PhantomData<&'w ()>, } #[derive(Fetch, Debug)] -#[read_only_derive(Debug)] #[allow(dead_code)] struct NestedQuery<'w> { c: &'w ComponentC, @@ -76,7 +108,6 @@ struct NestedQuery<'w> { } #[derive(Fetch, Debug)] -#[read_only_derive(Debug)] #[allow(dead_code)] struct GenericQuery<'w, T: Component, P: Component> { generic: (&'w T, &'w P), @@ -119,7 +150,7 @@ fn print_components_iter( ) { println!("Print components (iter):"); for e in query.iter() { - // Note that the actual type is different when you iterate with `iter`. + // Note that the actual type is different when you iterate over mutable queries with `iter`. let e: CustomQueryReadOnly<'_, _, _> = e; println!("Entity: {:?}", e.entity); println!("A: {:?}", e.a); @@ -130,35 +161,6 @@ fn print_components_iter( println!(); } -// If you are never going to mutate the data in a query, you can mark it with `read_only` attribute -// to avoid creating an additional type with `ReadOnly` suffix. -#[derive(Fetch)] -#[read_only] -struct ReadOnlyCustomQuery<'w, T: Component + Debug, P: Component + Debug> { - entity: Entity, - a: &'w ComponentA, - b: Option<&'w ComponentB>, - nested: NestedQueryReadOnly<'w>, - generic: GenericQueryReadOnly<'w, T, P>, - #[allow(dead_code)] - empty: EmptyQueryReadOnly<'w>, -} - -fn print_components_read_only( - query: Query, QueryFilter>, -) { - println!("Print components (read_only):"); - for e in query.iter() { - let e: ReadOnlyCustomQuery<'_, _, _> = e; - println!("Entity: {:?}", e.entity); - println!("A: {:?}", e.a); - println!("B: {:?}", e.b); - println!("Nested: {:?}", e.nested); - println!("Generic: {:?}", e.generic); - } - println!(); -} - type NestedTupleQuery<'w> = (&'w ComponentC, &'w ComponentD); type GenericTupleQuery<'w, T, P> = (&'w T, &'w P); From a717b7bb0111ab4221ff1abcef0274110d6838a6 Mon Sep 17 00:00:00 2001 From: Vladyslav Batyrenko Date: Sat, 15 Jan 2022 16:17:27 +0200 Subject: [PATCH 04/14] Apply suggestions from code review Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com> --- crates/bevy_ecs/macros/src/fetch.rs | 4 +++- crates/bevy_ecs/src/query/fetch.rs | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 1abae505537e1..0fe305920f356 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -674,7 +674,9 @@ fn read_world_query_field_type_info( panic!("Invalid tuple element: PhantomData"); } is_phantom = true; - } else if segment.ident != "Entity" { + } else if segment.ident == "Entity" { + // Nothing to do here + } else { assert_not_generic(path, generic_names); // Here, we assume that this member is another type that implements `Fetch`. diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 346dc937d4505..b5efbd3f79ad0 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -159,6 +159,7 @@ pub type QueryItem<'w, 's, Q> = <::Fetch as Fetch<'w, 's>>::Ite /// println!("Total (mut): {}", health.total()); /// } /// } +/// # my_system.system(); /// ``` /// /// If you want to use derive macros with read-only query variants, you need to pass them with @@ -257,11 +258,11 @@ pub type QueryItem<'w, 's, Q> = <::Fetch as Fetch<'w, 's>>::Ite /// /// ## Limitations /// -/// Currently, we don't support members that have a manual [`WorldQuery`] implementation if their +/// Currently, we don't support members that have a [`WorldQuery`] implementation where their /// [`Fetch::Item`] is different from the member type. For instance, the following code won't /// compile: /// -/// ```ignore +/// ```compile_fail /// #[derive(Component)] /// struct CustomQueryParameter; /// #[derive(Component)] From f67876af6df5def99e9d158a74c9e7d15d0adbf1 Mon Sep 17 00:00:00 2001 From: mvlabat Date: Sat, 15 Jan 2022 21:57:13 +0200 Subject: [PATCH 05/14] Introduce trait FetchedItem to fix Fetch macro limitations --- crates/bevy_ecs/macros/src/fetch.rs | 358 +++++++++------------------- crates/bevy_ecs/src/query/fetch.rs | 61 ++--- crates/bevy_ecs/src/query/filter.rs | 20 +- examples/ecs/custom_query_param.rs | 13 +- 4 files changed, 178 insertions(+), 274 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 0fe305920f356..8f572f3002285 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -1,10 +1,10 @@ use proc_macro::TokenStream; use proc_macro2::{Ident, Span}; -use quote::quote; +use quote::{quote, ToTokens}; use syn::{ - parse_macro_input, punctuated::Punctuated, Data, DataStruct, DeriveInput, Fields, - GenericArgument, GenericParam, ImplGenerics, Lifetime, LifetimeDef, PathArguments, Token, Type, - TypeGenerics, TypePath, TypeReference, WhereClause, + parse_macro_input, parse_quote, punctuated::Punctuated, Data, DataStruct, DeriveInput, Fields, + GenericArgument, GenericParam, ImplGenerics, Lifetime, LifetimeDef, Path, PathArguments, + ReturnType, Token, Type, TypeGenerics, TypePath, WhereClause, }; use crate::bevy_ecs_path; @@ -29,6 +29,7 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { has_mutable_attr, world_lifetime, state_lifetime, + fetch_lifetime, phantom_field_idents, phantom_field_types, field_idents, @@ -56,13 +57,6 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { quote! {} }; - // Fetch's HRTBs require this hack to make the implementation compile. I don't fully understand - // why this works though. If anyone's curious enough to try to find a better work-around, I'll - // leave playground links here: - // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=da5e260a5c2f3e774142d60a199e854a (this fails) - // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=802517bb3d8f83c45ee8c0be360bb250 (this compiles) - let fetch_lifetime = - GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'fetch", Span::call_site()))); let mut fetch_generics = ast.generics.clone(); fetch_generics.params.insert(1, state_lifetime); fetch_generics.params.push(fetch_lifetime.clone()); @@ -138,6 +132,10 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { type State = #state_struct_name #ty_generics; type ReadOnlyFetch = #read_only_fetch_struct_name #ty_generics; } + + impl #impl_generics #path::query::FetchedItem for #struct_name_read_only #ty_generics #where_clause { + type Query = Self; + } } } else { quote! { @@ -215,7 +213,7 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { unsafe impl #impl_generics #path::query::FetchState for #state_struct_name #ty_generics #where_clause { fn init(world: &mut #path::world::World) -> Self { #state_struct_name { - #(#field_idents: <#query_types as #path::query::WorldQuery>::State::init(world),)* + #(#field_idents: <<#query_types as #path::query::WorldQuery>::State as #path::query::FetchState>::init(world),)* #(#phantom_field_idents: Default::default(),)* } } @@ -245,6 +243,10 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { type ReadOnlyFetch = #read_only_fetch_struct_name #ty_generics; } + impl #impl_generics #path::query::FetchedItem for #struct_name #ty_generics #where_clause { + type Query = Self; + } + /// SAFETY: each item in the struct is read only unsafe impl #impl_generics #path::query::ReadOnlyFetch for #read_only_fetch_struct_name #ty_generics #where_clause {} }); @@ -264,10 +266,11 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream { impl_generics, ty_generics, where_clause, - world_lifetime, has_mutable_attr: _, has_world_lifetime, + world_lifetime, state_lifetime, + fetch_lifetime, phantom_field_idents, phantom_field_types, field_idents, @@ -280,13 +283,6 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream { panic!("Expected a struct without a lifetime"); } - // Fetch's HRTBs require this hack to make the implementation compile. I don't fully understand - // why this works though. If anyone's curious enough to try to find a better work-around, I'll - // leave playground links here: - // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=da5e260a5c2f3e774142d60a199e854a (this fails) - // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=802517bb3d8f83c45ee8c0be360bb250 (this compiles) - let fetch_lifetime = - GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'fetch", Span::call_site()))); let mut fetch_generics = ast.generics.clone(); fetch_generics.params.insert(0, world_lifetime); fetch_generics.params.insert(1, state_lifetime); @@ -346,7 +342,7 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream { unsafe impl #impl_generics #path::query::FetchState for #state_struct_name #ty_generics #where_clause { fn init(world: &mut #path::world::World) -> Self { #state_struct_name { - #(#field_idents: <#field_types as #path::query::WorldQuery>::State::init(world),)* + #(#field_idents: <<#field_types as #path::query::WorldQuery>::State as #path::query::FetchState>::init(world),)* #(#phantom_field_idents: Default::default(),)* } } @@ -396,6 +392,7 @@ struct FetchImplTokens<'a> { has_world_lifetime: bool, world_lifetime: GenericParam, state_lifetime: GenericParam, + fetch_lifetime: GenericParam, phantom_field_idents: Vec, phantom_field_types: Vec, field_idents: Vec, @@ -415,6 +412,14 @@ fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens { lt @ GenericParam::Lifetime(_) => Some(lt.clone()), _ => None, }); + // Fetch's HRTBs require substituting world lifetime with an additional one to make the + // implementation compile. I don't fully understand why this works though. If anyone's curious + // enough to try to find a better work around, I'll leave playground links here: + // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=da5e260a5c2f3e774142d60a199e854a (this fails) + // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=802517bb3d8f83c45ee8c0be360bb250 (this compiles) + let fetch_lifetime = + GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'fetch", Span::call_site()))); + let has_world_lifetime = world_lifetime.is_some(); let world_lifetime = world_lifetime.unwrap_or_else(|| { GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'world", Span::call_site()))) @@ -457,25 +462,21 @@ fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens { let mut query_types = Vec::new(); let mut fetch_init_types = Vec::new(); - let generic_names = ast - .generics - .params - .iter() - .filter_map(|param| match param { - GenericParam::Type(ty) => Some(ty.ident.to_string()), - _ => None, - }) - .collect::>(); - for field in fields.iter() { + let (world_lifetime, fetch_lifetime) = match (&world_lifetime, &fetch_lifetime) { + (GenericParam::Lifetime(world), GenericParam::Lifetime(fetch)) => { + (&world.lifetime, &fetch.lifetime) + } + _ => unreachable!(), + }; let WorldQueryFieldTypeInfo { query_type, fetch_init_type: init_type, - is_phantom, - } = read_world_query_field_type_info(&field.ty, false, &generic_names); + is_phantom_data, + } = read_world_query_field_type_info(&field.ty, world_lifetime, fetch_lifetime); let field_ident = field.ident.as_ref().unwrap().clone(); - if is_phantom { + if is_phantom_data { phantom_field_idents.push(field_ident.clone()); phantom_field_types.push(field.ty.clone()); } else { @@ -500,6 +501,7 @@ fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens { has_world_lifetime, world_lifetime, state_lifetime, + fetch_lifetime, phantom_field_idents, phantom_field_types, field_idents, @@ -515,234 +517,106 @@ struct WorldQueryFieldTypeInfo { query_type: Type, /// The same as `query_type` but with `'fetch` lifetime. fetch_init_type: Type, - is_phantom: bool, + is_phantom_data: bool, } fn read_world_query_field_type_info( ty: &Type, - is_tuple_element: bool, - generic_names: &[String], + world_lifetime: &Lifetime, + fetch_lifetime: &Lifetime, ) -> WorldQueryFieldTypeInfo { - let mut query_type = ty.clone(); - let mut fetch_init_type = ty.clone(); - let mut is_phantom = false; - - match (ty, &mut fetch_init_type) { - (Type::Path(path), Type::Path(path_init)) => { - if path.qself.is_some() { - // There's a risk that it contains a generic parameter that we can't test - // whether it's read-only or not. - panic!("Self type qualifiers aren't supported"); - } + let path = bevy_ecs_path(); - let segment = path.path.segments.last().unwrap(); - if segment.ident == "Option" { - // We expect that `Option` stores either `&T` or `Mut`. - let ty = match &segment.arguments { - PathArguments::AngleBracketed(args) => { - args.args.last().and_then(|arg| match arg { - GenericArgument::Type(ty) => Some(ty), - _ => None, - }) - } - _ => None, - }; - match ty.expect("Option type is expected to have generic arguments") { - // If it's a read-only reference, we just update the lifetime for `fetch_init_type` to `'fetch`. - Type::Reference(reference) => { - if reference.mutability.is_some() { - panic!("Invalid reference type: use `Mut` instead of `&mut T`"); - } - match &mut path_init.path.segments.last_mut().unwrap().arguments { - PathArguments::AngleBracketed(args) => { - match args.args.last_mut().unwrap() { - GenericArgument::Type(Type::Reference(ty)) => ty.lifetime = Some(Lifetime::new("'fetch", Span::call_site())), - _ => unreachable!(), - } - } - _ => unreachable!(), - } - } - // If it's a mutable reference, we set `query_type` and `fetch_init_type` to `&mut T`, - // we also update the lifetime for `fetch_init_type` to `'fetch`. - Type::Path(path) => { - assert_not_generic(path, generic_names); - - let segment = path.path.segments.last().unwrap(); - let ty_ident = &segment.ident; - if ty_ident == "Mut" { - let (mut_lifetime, mut_ty) = match &segment.arguments { - PathArguments::AngleBracketed(args) => { - (args.args.first().and_then(|arg| { - match arg { - GenericArgument::Lifetime(lifetime) => Some(lifetime.clone()), - _ => None, - } - }).expect("Mut is expected to have a lifetime"), - args.args.last().and_then(|arg| { - match arg { - GenericArgument::Type(ty) => Some(ty.clone()), - _ => None, - } - }).expect("Mut is expected to have a lifetime")) - } - _ => panic!("Mut type is expected to have generic arguments") - }; - - match query_type { - Type::Path(ref mut path) => { - let segment = path.path.segments.last_mut().unwrap(); - match segment.arguments { - PathArguments::AngleBracketed(ref mut args) => { - match args.args.last_mut().unwrap() { - GenericArgument::Type(ty) => { - *ty = Type::Reference(TypeReference { - and_token: Token![&](Span::call_site()), - lifetime: Some(mut_lifetime), - mutability: Some(Token![mut](Span::call_site())), - elem: Box::new(mut_ty.clone()), - }); - } - _ => unreachable!() - } - } - _ => unreachable!() - } - } - _ => unreachable!() - } - - let segment = path_init.path.segments.last_mut().unwrap(); - match segment.arguments { - PathArguments::AngleBracketed(ref mut args) => { - match args.args.last_mut().unwrap() { - GenericArgument::Type(ty) => { - *ty = Type::Reference(TypeReference { - and_token: Token![&](Span::call_site()), - lifetime: Some(Lifetime::new("'fetch", Span::call_site())), - mutability: Some(Token![mut](Span::call_site())), - elem: Box::new(mut_ty), - }); - } - _ => unreachable!() - } - } - _ => unreachable!() - } - } else { - panic!("Option type is expected to have a reference value (`Option<&T>` or `Option>`)"); - } - } - _ => panic!("Option type is expected to have a reference value (`Option<&T>` or `Option>`)"), - } - } else if segment.ident == "Mut" { - // If it's a mutable reference, we set `query_type` and `fetch_init_type` to `&mut T`, - // we also update the lifetime for `fetch_init_type` to `'fetch`. - let (mut_lifetime, mut_ty) = match &segment.arguments { - PathArguments::AngleBracketed(args) => { - let lt = args.args.first().and_then(|arg| { - match arg { - GenericArgument::Lifetime(lifetime) => Some(lifetime.clone()), - _ => None, - } - }).expect("`Mut` is expected to have a lifetime"); - let ty = args.args.last().and_then(|arg| { - match arg { - GenericArgument::Type(ty) => Some(ty.clone()), - _ => None, - } - }).expect("`Mut` is expected to have a lifetime"); - (lt, ty) - } - _ => panic!("`Mut` is expected to have generic arguments") - }; - - query_type = Type::Reference(TypeReference { - and_token: Token![&](Span::call_site()), - lifetime: Some(mut_lifetime), - mutability: Some(Token![mut](Span::call_site())), - elem: Box::new(mut_ty.clone()), - }); - fetch_init_type = Type::Reference(TypeReference { - and_token: Token![&](Span::call_site()), - lifetime: Some(Lifetime::new("'fetch", Span::call_site())), - mutability: Some(Token![mut](Span::call_site())), - elem: Box::new(mut_ty), - }); - } else if segment.ident == "PhantomData" { - if is_tuple_element { - panic!("Invalid tuple element: PhantomData"); - } - is_phantom = true; - } else if segment.ident == "Entity" { - // Nothing to do here + let query_type = parse_quote!(<#ty as #path::query::FetchedItem>::Query); + let mut fetch_init_type = parse_quote!(<#ty as #path::query::FetchedItem>::Query); + + let is_phantom_data = match ty { + Type::Path(path) => { + if let Some(segment) = path.path.segments.last() { + segment.ident == "PhantomData" } else { - assert_not_generic(path, generic_names); + false + } + } + _ => false, + }; - // Here, we assume that this member is another type that implements `Fetch`. - // If it doesn't, the code won't compile. + replace_lifetime_for_type(&mut fetch_init_type, world_lifetime, fetch_lifetime); - // Also, we don't support `Fetch` implementations that specify custom `Item` types, - // except for the well-known ones, such as `WriteFetch`. - // See https://github.com/bevyengine/bevy/pull/2713#issuecomment-904773083. + WorldQueryFieldTypeInfo { + query_type, + fetch_init_type, + is_phantom_data, + } +} - if let PathArguments::AngleBracketed(args) = &mut path_init.path.segments.last_mut().unwrap().arguments { - if let Some(GenericArgument::Lifetime(lt)) = args.args.first_mut() { - *lt = Lifetime::new("'fetch", Span::call_site()); - } - } - } +fn replace_lifetime_for_type(ty: &mut Type, world_lifetime: &Lifetime, fetch_lifetime: &Lifetime) { + match ty { + Type::Path(ref mut path) => { + replace_world_lifetime_for_type_path(path, world_lifetime, fetch_lifetime) } - (Type::Reference(reference), Type::Reference(init_reference)) => { - if reference.mutability.is_some() { - panic!("Invalid reference type: use `Mut` instead of `&mut T`"); + Type::Reference(ref mut reference) => { + if let Some(lifetime) = reference.lifetime.as_mut() { + replace_lifetime(lifetime, world_lifetime, fetch_lifetime); } - init_reference.lifetime = Some(Lifetime::new("'fetch", Span::call_site())); + replace_lifetime_for_type(reference.elem.as_mut(), world_lifetime, fetch_lifetime); } - (Type::Tuple(tuple), Type::Tuple(init_tuple)) => { - let mut query_tuple_elems = tuple.elems.clone(); - query_tuple_elems.clear(); - let mut fetch_init_tuple_elems = query_tuple_elems.clone(); - for ty in tuple.elems.iter() { - let WorldQueryFieldTypeInfo { - query_type, - fetch_init_type, - is_phantom: _, - } = read_world_query_field_type_info( - ty, - true, - generic_names, - ); - query_tuple_elems.push(query_type); - fetch_init_tuple_elems.push(fetch_init_type); - } - match query_type { - Type::Tuple(ref mut tuple) => { - tuple.elems = query_tuple_elems; - } - _ => unreachable!(), + Type::Tuple(tuple) => { + for ty in tuple.elems.iter_mut() { + replace_lifetime_for_type(ty, world_lifetime, fetch_lifetime); } - init_tuple.elems = fetch_init_tuple_elems; } - _ => panic!("Only the following types (or their tuples) are supported for WorldQuery: &T, &mut T, Option<&T>, Option<&mut T>, Entity, or other structs that implement WorldQuery"), + ty => panic!("Unsupported type: {}", ty.to_token_stream()), } +} - WorldQueryFieldTypeInfo { - query_type, - fetch_init_type, - is_phantom, +fn replace_world_lifetime_for_type_path( + path: &mut TypePath, + world_lifetime: &Lifetime, + fetch_lifetime: &Lifetime, +) { + if let Some(qself) = path.qself.as_mut() { + replace_lifetime_for_type(qself.ty.as_mut(), world_lifetime, fetch_lifetime); } + + replace_world_lifetime_for_path(&mut path.path, world_lifetime, fetch_lifetime); } -fn assert_not_generic(type_path: &TypePath, generic_names: &[String]) { - // `get_ident` returns Some if it consists of a single segment, in this case it - // makes sense to ensure that it's not a generic. - if let Some(ident) = type_path.path.get_ident() { - let is_generic = generic_names - .iter() - .any(|generic_name| ident == generic_name.as_str()); - if is_generic { - panic!("Only references to generic types are supported: i.e. instead of `component: T`, use `component: &T` or `component: Mut` (optional references are supported as well)"); +fn replace_world_lifetime_for_path( + path: &mut Path, + world_lifetime: &Lifetime, + fetch_lifetime: &Lifetime, +) { + for segment in path.segments.iter_mut() { + match segment.arguments { + PathArguments::None => {} + PathArguments::AngleBracketed(ref mut args) => { + for arg in args.args.iter_mut() { + match arg { + GenericArgument::Lifetime(lifetime) => { + replace_lifetime(lifetime, world_lifetime, fetch_lifetime); + } + GenericArgument::Type(ty) => { + replace_lifetime_for_type(ty, world_lifetime, fetch_lifetime) + } + _ => {} + } + } + } + PathArguments::Parenthesized(ref mut args) => { + for input in args.inputs.iter_mut() { + replace_lifetime_for_type(input, world_lifetime, fetch_lifetime); + } + if let ReturnType::Type(_, _) = args.output { + panic!("Function types aren't supported"); + } + } } } } + +fn replace_lifetime(lifetime: &mut Lifetime, world_lifetime: &Lifetime, fetch_lifetime: &Lifetime) { + if lifetime.ident == world_lifetime.ident { + lifetime.ident = fetch_lifetime.ident.clone(); + } +} diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index b5efbd3f79ad0..3e2297b033891 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -54,6 +54,14 @@ pub trait WorldQuery { pub type QueryItem<'w, 's, Q> = <::Fetch as Fetch<'w, 's>>::Item; +/// Types that appear as [`Fetch::Item`] associated types. +/// +/// This trait comes in useful when it's needed to correlate such types as `Mut` to `&mut T`. +/// In most cases though, [`FetchedItem::Query`] corresponds to a type that implements the trait. +pub trait FetchedItem { + type Query: WorldQuery; +} + /// Types that implement this trait are responsible for fetching query items from tables or /// archetypes. /// @@ -255,35 +263,6 @@ pub type QueryItem<'w, 's, Q> = <::Fetch as Fetch<'w, 's>>::Ite /// /// # my_system.system(); /// ``` -/// -/// ## Limitations -/// -/// Currently, we don't support members that have a [`WorldQuery`] implementation where their -/// [`Fetch::Item`] is different from the member type. For instance, the following code won't -/// compile: -/// -/// ```compile_fail -/// #[derive(Component)] -/// struct CustomQueryParameter; -/// #[derive(Component)] -/// struct ItemDataType; -/// -/// struct CustomQueryParameterFetch { -/// // ... -/// } -/// -/// impl<'w, 's> Fetch<'w, 's> for CustomQueryParameterFetch { -/// type Item = ItemDataType; -/// -/// // ... -/// } -/// -/// #[derive(Fetch)] -/// struct MyQuery { -/// custom_item: ItemDataType, -/// } -/// -/// ``` pub trait Fetch<'world, 'state>: Sized { type Item; type State: FetchState; @@ -380,6 +359,10 @@ impl WorldQuery for Entity { type ReadOnlyFetch = EntityFetch; } +impl FetchedItem for Entity { + type Query = Self; +} + /// The [`Fetch`] of [`Entity`]. #[derive(Clone)] pub struct EntityFetch { @@ -467,6 +450,10 @@ impl WorldQuery for &T { type ReadOnlyFetch = ReadFetch; } +impl FetchedItem for &T { + type Query = Self; +} + /// The [`FetchState`] of `&T`. pub struct ReadState { component_id: ComponentId, @@ -622,6 +609,10 @@ impl WorldQuery for &mut T { type ReadOnlyFetch = ReadOnlyWriteFetch; } +impl<'a, T: Component> FetchedItem for Mut<'a, T> { + type Query = &'a mut T; +} + /// The [`Fetch`] of `&mut T`. pub struct WriteFetch { table_components: NonNull, @@ -905,6 +896,10 @@ impl WorldQuery for Option { type ReadOnlyFetch = OptionFetch; } +impl FetchedItem for Option { + type Query = Option; +} + /// The [`Fetch`] of `Option`. #[derive(Clone)] pub struct OptionFetch { @@ -1081,6 +1076,10 @@ impl WorldQuery for ChangeTrackers { type ReadOnlyFetch = ChangeTrackersFetch; } +impl FetchedItem for ChangeTrackers { + type Query = Self; +} + /// The [`FetchState`] of [`ChangeTrackers`]. pub struct ChangeTrackersState { component_id: ComponentId, @@ -1332,6 +1331,10 @@ macro_rules! impl_tuple_fetch { type ReadOnlyFetch = ($($name::ReadOnlyFetch,)*); } + impl<$($name: FetchedItem),*> FetchedItem for ($($name,)*) { + type Query = ($($name::Query,)*); + } + /// SAFETY: each item in the tuple is read only unsafe impl<$($name: ReadOnlyFetch),*> ReadOnlyFetch for ($($name,)*) {} diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 7dcd759b2fb90..84abe0b0570e9 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -2,7 +2,7 @@ use crate::{ archetype::{Archetype, ArchetypeComponentId}, component::{Component, ComponentId, ComponentStorage, ComponentTicks, StorageType}, entity::Entity, - query::{Access, Fetch, FetchState, FilteredAccess, ReadOnlyFetch, WorldQuery}, + query::{Access, Fetch, FetchState, FetchedItem, FilteredAccess, ReadOnlyFetch, WorldQuery}, storage::{ComponentSparseSet, Table, Tables}, world::World, }; @@ -122,6 +122,10 @@ impl WorldQuery for With { type ReadOnlyFetch = WithFetch; } +impl FetchedItem for With { + type Query = Self; +} + /// The [`Fetch`] of [`With`]. pub struct WithFetch { marker: PhantomData, @@ -245,6 +249,10 @@ impl WorldQuery for Without { type ReadOnlyFetch = WithoutFetch; } +impl FetchedItem for Without { + type Query = Self; +} + /// The [`Fetch`] of [`Without`]. pub struct WithoutFetch { marker: PhantomData, @@ -400,6 +408,12 @@ macro_rules! impl_query_filter_tuple { type ReadOnlyFetch = Or<($(OrFetch<$filter::ReadOnlyFetch>,)*)>; } + impl<$($filter: WorldQuery),*> FetchedItem for Or<($($filter,)*)> + where $($filter::Fetch: FilterFetch, $filter::ReadOnlyFetch: FilterFetch),* + { + type Query = Self; + } + /// SAFETY: this only works using the filter which doesn't write unsafe impl<$($filter: FilterFetch + ReadOnlyFetch),*> ReadOnlyFetch for Or<($(OrFetch<$filter>,)*)> {} @@ -525,6 +539,10 @@ macro_rules! impl_tick_filter { type ReadOnlyFetch = $fetch_name; } + impl FetchedItem for $name { + type Query = Self; + } + // SAFETY: this reads the T component. archetype component access and component access are updated to reflect that unsafe impl FetchState for $state_name { fn init(world: &mut World) -> Self { diff --git a/examples/ecs/custom_query_param.rs b/examples/ecs/custom_query_param.rs index 5c059f8838056..c96b07f116e45 100644 --- a/examples/ecs/custom_query_param.rs +++ b/examples/ecs/custom_query_param.rs @@ -34,7 +34,7 @@ fn main() { .label("print_components_iter") .after("print_components_iter_mut"), ) - .add_system(print_components_tuple.after("print_components_iter_mut")) + .add_system(print_components_tuple.after("print_components_iter")) .run(); } @@ -55,6 +55,8 @@ struct ReadOnlyCustomQuery<'w, T: Component + Debug, P: Component + Debug> { a: &'w ComponentA, b: Option<&'w ComponentB>, nested: NestedQuery<'w>, + optional_nested: Option>, + optional_tuple: Option<(&'w ComponentB, &'w ComponentZ)>, generic: GenericQuery<'w, T, P>, #[allow(dead_code)] empty: EmptyQuery<'w>, @@ -65,11 +67,12 @@ fn print_components_read_only( ) { println!("Print components (read_only):"); for e in query.iter() { - let e: ReadOnlyCustomQuery<'_, _, _> = e; println!("Entity: {:?}", e.entity); println!("A: {:?}", e.a); println!("B: {:?}", e.b); println!("Nested: {:?}", e.nested); + println!("Optional nested: {:?}", e.optional_nested); + println!("Optional tuple: {:?}", e.optional_tuple); println!("Generic: {:?}", e.generic); } println!(); @@ -89,6 +92,8 @@ struct CustomQuery<'w, T: Component + Debug, P: Component + Debug> { a: Mut<'w, ComponentA>, b: Option>, nested: NestedQuery<'w>, + optional_nested: Option>, + optional_tuple: Option<(NestedQuery<'w>, Mut<'w, ComponentZ>)>, generic: GenericQuery<'w, T, P>, #[allow(dead_code)] empty: EmptyQuery<'w>, @@ -97,6 +102,8 @@ struct CustomQuery<'w, T: Component + Debug, P: Component + Debug> { // This is a valid query as well, which would iterate over every entity. #[derive(Fetch, Debug)] struct EmptyQuery<'w> { + // The Fetch derive macro expect a lifetime. As Rust doesn't allow unused lifetimes, we need + // to use `PhantomData` as a work around. _w: std::marker::PhantomData<&'w ()>, } @@ -139,6 +146,8 @@ fn print_components_iter_mut( println!("Entity: {:?}", e.entity); println!("A: {:?}", e.a); println!("B: {:?}", e.b); + println!("Optional nested: {:?}", e.optional_nested); + println!("Optional tuple: {:?}", e.optional_tuple); println!("Nested: {:?}", e.nested); println!("Generic: {:?}", e.generic); } From 005fbf6592263606c322a3c0732c916f510deef9 Mon Sep 17 00:00:00 2001 From: Vladyslav Batyrenko Date: Sat, 15 Jan 2022 22:58:16 +0200 Subject: [PATCH 06/14] Apply suggestions from code review Co-authored-by: Alice Cecile --- crates/bevy_ecs/macros/src/fetch.rs | 44 +++++++++++++++++------------ crates/bevy_ecs/src/query/fetch.rs | 4 +-- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 8f572f3002285..190431e070a1b 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -57,11 +57,13 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { quote! {} }; + // Add `'state` and `'fetch` lifetimes that will be used in `Fetch` implementation. let mut fetch_generics = ast.generics.clone(); fetch_generics.params.insert(1, state_lifetime); - fetch_generics.params.push(fetch_lifetime.clone()); + fetch_generics.params.insert(2, fetch_lifetime.clone()); let (fetch_impl_generics, _, _) = fetch_generics.split_for_impl(); + // Replace lifetime `'world` with `'fetch`. See `replace_lifetime_for_type` for more details. let mut fetch_generics = ast.generics.clone(); *fetch_generics.params.first_mut().unwrap() = fetch_lifetime; let (_, fetch_ty_generics, _) = fetch_generics.split_for_impl(); @@ -283,10 +285,11 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream { panic!("Expected a struct without a lifetime"); } + // Add `'world`, `'state` and `'fetch` lifetimes that will be used in `Fetch` implementation. let mut fetch_generics = ast.generics.clone(); fetch_generics.params.insert(0, world_lifetime); fetch_generics.params.insert(1, state_lifetime); - fetch_generics.params.push(fetch_lifetime); + fetch_generics.params.insert(2, fetch_lifetime); let (fetch_impl_generics, _, _) = fetch_generics.split_for_impl(); let path = bevy_ecs_path(); @@ -417,19 +420,19 @@ fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens { // enough to try to find a better work around, I'll leave playground links here: // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=da5e260a5c2f3e774142d60a199e854a (this fails) // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=802517bb3d8f83c45ee8c0be360bb250 (this compiles) - let fetch_lifetime = + let fetch_lifetime_param = GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'fetch", Span::call_site()))); let has_world_lifetime = world_lifetime.is_some(); - let world_lifetime = world_lifetime.unwrap_or_else(|| { + let world_lifetime_param = world_lifetime.unwrap_or_else(|| { GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'world", Span::call_site()))) }); - let state_lifetime = + let state_lifetime_param = GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'state", Span::call_site()))); let mut fetch_trait_punctuated_lifetimes = Punctuated::<_, Token![,]>::new(); - fetch_trait_punctuated_lifetimes.push(world_lifetime.clone()); - fetch_trait_punctuated_lifetimes.push(state_lifetime.clone()); + fetch_trait_punctuated_lifetimes.push(world_lifetime_param.clone()); + fetch_trait_punctuated_lifetimes.push(state_lifetime_param.clone()); let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl(); @@ -462,13 +465,13 @@ fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens { let mut query_types = Vec::new(); let mut fetch_init_types = Vec::new(); + let (world_lifetime, fetch_lifetime) = match (&world_lifetime_param, &fetch_lifetime_param) { + (GenericParam::Lifetime(world), GenericParam::Lifetime(fetch)) => { + (&world.lifetime, &fetch.lifetime) + } + _ => unreachable!(), + }; for field in fields.iter() { - let (world_lifetime, fetch_lifetime) = match (&world_lifetime, &fetch_lifetime) { - (GenericParam::Lifetime(world), GenericParam::Lifetime(fetch)) => { - (&world.lifetime, &fetch.lifetime) - } - _ => unreachable!(), - }; let WorldQueryFieldTypeInfo { query_type, fetch_init_type: init_type, @@ -499,9 +502,9 @@ fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens { where_clause, has_mutable_attr, has_world_lifetime, - world_lifetime, - state_lifetime, - fetch_lifetime, + world_lifetime: world_lifetime_param, + state_lifetime: state_lifetime_param, + fetch_lifetime: fetch_lifetime_param, phantom_field_idents, phantom_field_types, field_idents, @@ -527,8 +530,8 @@ fn read_world_query_field_type_info( ) -> WorldQueryFieldTypeInfo { let path = bevy_ecs_path(); - let query_type = parse_quote!(<#ty as #path::query::FetchedItem>::Query); - let mut fetch_init_type = parse_quote!(<#ty as #path::query::FetchedItem>::Query); + let query_type: Type = parse_quote!(<#ty as #path::query::FetchedItem>::Query); + let mut fetch_init_type: Type = query_type.clone(); let is_phantom_data = match ty { Type::Path(path) => { @@ -550,6 +553,11 @@ fn read_world_query_field_type_info( } } +// Fetch's HRTBs require substituting world lifetime with an additional one to make the +// implementation compile. I don't fully understand why this works though. If anyone's curious +// enough to try to find a better work around, I'll leave playground links here: +// - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=da5e260a5c2f3e774142d60a199e854a (this fails) +// - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=802517bb3d8f83c45ee8c0be360bb250 (this compiles) fn replace_lifetime_for_type(ty: &mut Type, world_lifetime: &Lifetime, fetch_lifetime: &Lifetime) { match ty { Type::Path(ref mut path) => { diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 3e2297b033891..6073d9eb08f1b 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -56,7 +56,7 @@ pub type QueryItem<'w, 's, Q> = <::Fetch as Fetch<'w, 's>>::Ite /// Types that appear as [`Fetch::Item`] associated types. /// -/// This trait comes in useful when it's needed to correlate such types as `Mut` to `&mut T`. +/// This trait comes in useful when it's needed to correlate between types (such as `Mut` to `&mut T`). /// In most cases though, [`FetchedItem::Query`] corresponds to a type that implements the trait. pub trait FetchedItem { type Query: WorldQuery; @@ -171,7 +171,7 @@ pub trait FetchedItem { /// ``` /// /// If you want to use derive macros with read-only query variants, you need to pass them with -/// using the `read_only_derive` attribute. When `Fetch` macro generates an additional struct +/// using the `read_only_derive` attribute. When the `Fetch` macro generates an additional struct /// for a mutable query, it doesn't automatically inherit the same derives. Since derive macros /// can't access information about other derives, they need to be passed manually with the /// `read_only_derive` attribute. From ba68958967e261054ef0487d06d6f7efe7f3a8b3 Mon Sep 17 00:00:00 2001 From: mvlabat Date: Sat, 15 Jan 2022 23:40:43 +0200 Subject: [PATCH 07/14] Add FetchedItem implementation for bool --- crates/bevy_ecs/src/query/fetch.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 6073d9eb08f1b..27884d6d7fa79 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -264,7 +264,7 @@ pub trait FetchedItem { /// # my_system.system(); /// ``` pub trait Fetch<'world, 'state>: Sized { - type Item; + type Item: FetchedItem; type State: FetchState; /// Creates a new instance of this fetch. @@ -1497,3 +1497,9 @@ impl<'w, 's, State: FetchState> Fetch<'w, 's> for NopFetch { #[inline(always)] unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item {} } + +/// This implementation won't allow us to correlate a boolean to a filter type. But having a dummy +/// for `bool` allows us to add `FetchedItem` bound to [`Fetch::Item`]. +impl FetchedItem for bool { + type Query = (); +} From b7845d94e484b1d3840ae1a5cebe413f28f28757 Mon Sep 17 00:00:00 2001 From: mvlabat Date: Sat, 15 Jan 2022 23:47:21 +0200 Subject: [PATCH 08/14] Actualise a comment for the struct for ReadOnlyFetch --- crates/bevy_ecs/macros/src/fetch.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 190431e070a1b..94be8f2bafbd5 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -73,7 +73,8 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { let struct_read_only_declaration = if has_mutable_attr { quote! { // TODO: it would be great to be able to dedup this by just deriving `Fetch` again - // without the `mutable` attribute, but supporting QSelf types is tricky. + // without the `mutable` attribute, but we'd need a way to avoid creating a redundant + // `State` struct. #read_only_derive_macro_call struct #struct_name_read_only #impl_generics #where_clause { #(#field_idents: <<#query_types as #path::query::WorldQuery>::ReadOnlyFetch as #path::query::Fetch<#world_lifetime, #world_lifetime>>::Item,)* From 0a8ba7250a4d796326f6e7d88cdc61be858488f8 Mon Sep 17 00:00:00 2001 From: mvlabat Date: Sun, 16 Jan 2022 12:32:35 +0200 Subject: [PATCH 09/14] Introduce ignore attribute, namespace attributes with fetch and fetch_filter --- crates/bevy_ecs/macros/src/fetch.rs | 267 ++++++++++++++++++++-------- crates/bevy_ecs/macros/src/lib.rs | 4 +- crates/bevy_ecs/src/query/fetch.rs | 31 +++- crates/bevy_ecs/src/query/filter.rs | 1 + examples/ecs/custom_query_param.rs | 5 +- 5 files changed, 221 insertions(+), 87 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 94be8f2bafbd5..e8603991f49dc 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -2,19 +2,81 @@ use proc_macro::TokenStream; use proc_macro2::{Ident, Span}; use quote::{quote, ToTokens}; use syn::{ - parse_macro_input, parse_quote, punctuated::Punctuated, Data, DataStruct, DeriveInput, Fields, - GenericArgument, GenericParam, ImplGenerics, Lifetime, LifetimeDef, Path, PathArguments, - ReturnType, Token, Type, TypeGenerics, TypePath, WhereClause, + parse::{Parse, ParseStream}, + parse_macro_input, parse_quote, + punctuated::Punctuated, + Attribute, Data, DataStruct, DeriveInput, Field, Fields, GenericArgument, GenericParam, + ImplGenerics, Lifetime, LifetimeDef, Path, PathArguments, ReturnType, Token, Type, + TypeGenerics, TypePath, Visibility, WhereClause, }; use crate::bevy_ecs_path; +#[derive(Default)] +struct FetchStructAttributes { + pub mutable: bool, + pub read_only_derive_args: Punctuated, +} + static MUTABLE_ATTRIBUTE_NAME: &str = "mutable"; static READ_ONLY_DERIVE_ATTRIBUTE_NAME: &str = "read_only_derive"; +mod field_attr_keywords { + syn::custom_keyword!(ignore); +} + +static FETCH_ATTRIBUTE_NAME: &str = "fetch"; +static FILTER_FETCH_ATTRIBUTE_NAME: &str = "filter_fetch"; + pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { let ast = parse_macro_input!(input as DeriveInput); + let mut fetch_struct_attributes = FetchStructAttributes::default(); + for attr in &ast.attrs { + if !attr + .path + .get_ident() + .map_or(false, |ident| ident == FETCH_ATTRIBUTE_NAME) + { + continue; + } + + attr.parse_args_with(|input: ParseStream| { + let meta = input.parse_terminated::(syn::Meta::parse)?; + for meta in meta { + let ident = meta.path().get_ident(); + if ident.map_or(false, |ident| ident == MUTABLE_ATTRIBUTE_NAME) { + if let syn::Meta::Path(_) = meta { + fetch_struct_attributes.mutable = true; + } else { + panic!( + "The `{}` attribute is expected to have no value or arguments", + MUTABLE_ATTRIBUTE_NAME + ); + } + } else if ident.map_or(false, |ident| ident == READ_ONLY_DERIVE_ATTRIBUTE_NAME) { + if let syn::Meta::List(meta_list) = meta { + fetch_struct_attributes + .read_only_derive_args + .extend(meta_list.nested.iter().cloned()); + } else { + panic!( + "Expected a structured list within the `{}` attribute", + READ_ONLY_DERIVE_ATTRIBUTE_NAME + ); + } + } else { + panic!( + "Unrecognized attribute: `{}`", + meta.path().to_token_stream() + ); + } + } + Ok(()) + }) + .unwrap_or_else(|_| panic!("Invalid `{}` attribute format", FETCH_ATTRIBUTE_NAME)); + } + let FetchImplTokens { struct_name, struct_name_read_only, @@ -26,35 +88,36 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { ty_generics, where_clause, has_world_lifetime, - has_mutable_attr, world_lifetime, state_lifetime, fetch_lifetime, - phantom_field_idents, - phantom_field_types, + ignored_field_attrs, + ignored_field_visibilities, + ignored_field_idents, + ignored_field_types, + field_attrs, + field_visibilities, field_idents, field_types: _, query_types, fetch_init_types, - } = fetch_impl_tokens(&ast); + } = fetch_impl_tokens(&ast, FETCH_ATTRIBUTE_NAME, fetch_struct_attributes.mutable); if !has_world_lifetime { panic!("Expected a struct with a lifetime"); } - let read_only_derive_attr = ast.attrs.iter().find(|attr| { - attr.path - .get_ident() - .map_or(false, |ident| ident == READ_ONLY_DERIVE_ATTRIBUTE_NAME) - }); - let read_only_derive_macro_call = if let Some(read_only_derive_attr) = read_only_derive_attr { - if !has_mutable_attr { - panic!("Attribute `read_only_derive` can only be be used for a struct marked with the `mutable` attribute"); - } - let derive_args = &read_only_derive_attr.tokens; - quote! { #[derive #derive_args] } - } else { + let read_only_derive_macro_call = if fetch_struct_attributes.read_only_derive_args.is_empty() { quote! {} + } else { + if !fetch_struct_attributes.mutable { + panic!( + "Attribute `{}` can only be be used for a struct marked with the `{}` attribute", + READ_ONLY_DERIVE_ATTRIBUTE_NAME, MUTABLE_ATTRIBUTE_NAME + ); + } + let derive_args = &fetch_struct_attributes.read_only_derive_args; + quote! { #[derive(#derive_args)] } }; // Add `'state` and `'fetch` lifetimes that will be used in `Fetch` implementation. @@ -70,20 +133,20 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { let path = bevy_ecs_path(); - let struct_read_only_declaration = if has_mutable_attr { + let struct_read_only_declaration = if fetch_struct_attributes.mutable { quote! { // TODO: it would be great to be able to dedup this by just deriving `Fetch` again // without the `mutable` attribute, but we'd need a way to avoid creating a redundant // `State` struct. #read_only_derive_macro_call struct #struct_name_read_only #impl_generics #where_clause { - #(#field_idents: <<#query_types as #path::query::WorldQuery>::ReadOnlyFetch as #path::query::Fetch<#world_lifetime, #world_lifetime>>::Item,)* - #(#phantom_field_idents: #phantom_field_types,)* + #(#(#field_attrs)* #field_visibilities #field_idents: <<#query_types as #path::query::WorldQuery>::ReadOnlyFetch as #path::query::Fetch<#world_lifetime, #world_lifetime>>::Item,)* + #(#(#ignored_field_attrs)* #ignored_field_visibilities #ignored_field_idents: #ignored_field_types,)* } struct #read_only_fetch_struct_name #impl_generics #where_clause { #(#field_idents: <#query_types as #path::query::WorldQuery>::ReadOnlyFetch,)* - #(#phantom_field_idents: #phantom_field_types,)* + #(#ignored_field_idents: #ignored_field_types,)* } impl #fetch_impl_generics #path::query::Fetch<#fetch_trait_punctuated_lifetimes> for #read_only_fetch_struct_name #fetch_ty_generics #where_clause { @@ -93,7 +156,7 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { unsafe fn init(_world: &#path::world::World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self { #read_only_fetch_struct_name { #(#field_idents: <#fetch_init_types as #path::query::WorldQuery>::ReadOnlyFetch::init(_world, &state.#field_idents, _last_change_tick, _change_tick),)* - #(#phantom_field_idents: Default::default(),)* + #(#ignored_field_idents: Default::default(),)* } } @@ -116,7 +179,7 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item { #struct_name_read_only { #(#field_idents: self.#field_idents.table_fetch(_table_row),)* - #(#phantom_field_idents: Default::default(),)* + #(#ignored_field_idents: Default::default(),)* } } @@ -125,7 +188,7 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { unsafe fn archetype_fetch(&mut self, _archetype_index: usize) -> Self::Item { #struct_name_read_only { #(#field_idents: self.#field_idents.archetype_fetch(_archetype_index),)* - #(#phantom_field_idents: Default::default(),)* + #(#ignored_field_idents: Default::default(),)* } } } @@ -160,12 +223,12 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { let tokens = TokenStream::from(quote! { struct #fetch_struct_name #impl_generics #where_clause { #(#field_idents: <#query_types as #path::query::WorldQuery>::Fetch,)* - #(#phantom_field_idents: #phantom_field_types,)* + #(#ignored_field_idents: #ignored_field_types,)* } struct #state_struct_name #impl_generics #where_clause { #(#field_idents: <#query_types as #path::query::WorldQuery>::State,)* - #(#phantom_field_idents: #phantom_field_types,)* + #(#ignored_field_idents: #ignored_field_types,)* } impl #fetch_impl_generics #path::query::Fetch<#fetch_trait_punctuated_lifetimes> for #fetch_struct_name #fetch_ty_generics #where_clause { @@ -175,7 +238,7 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { unsafe fn init(_world: &#path::world::World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self { #fetch_struct_name { #(#field_idents: <#fetch_init_types as #path::query::WorldQuery>::Fetch::init(_world, &state.#field_idents, _last_change_tick, _change_tick),)* - #(#phantom_field_idents: Default::default(),)* + #(#ignored_field_idents: Default::default(),)* } } @@ -198,7 +261,7 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item { #struct_name { #(#field_idents: self.#field_idents.table_fetch(_table_row),)* - #(#phantom_field_idents: Default::default(),)* + #(#ignored_field_idents: Default::default(),)* } } @@ -207,7 +270,7 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { unsafe fn archetype_fetch(&mut self, _archetype_index: usize) -> Self::Item { #struct_name { #(#field_idents: self.#field_idents.archetype_fetch(_archetype_index),)* - #(#phantom_field_idents: Default::default(),)* + #(#ignored_field_idents: Default::default(),)* } } } @@ -217,7 +280,7 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { fn init(world: &mut #path::world::World) -> Self { #state_struct_name { #(#field_idents: <<#query_types as #path::query::WorldQuery>::State as #path::query::FetchState>::init(world),)* - #(#phantom_field_idents: Default::default(),)* + #(#ignored_field_idents: Default::default(),)* } } @@ -269,18 +332,21 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream { impl_generics, ty_generics, where_clause, - has_mutable_attr: _, has_world_lifetime, world_lifetime, state_lifetime, fetch_lifetime, - phantom_field_idents, - phantom_field_types, + ignored_field_attrs: _, + ignored_field_visibilities: _, + ignored_field_idents, + ignored_field_types, + field_attrs: _, + field_visibilities: _, field_idents, field_types, query_types: _, fetch_init_types: _, - } = fetch_impl_tokens(&ast); + } = fetch_impl_tokens(&ast, FILTER_FETCH_ATTRIBUTE_NAME, false); if has_world_lifetime { panic!("Expected a struct without a lifetime"); @@ -298,12 +364,12 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream { let tokens = TokenStream::from(quote! { struct #fetch_struct_name #impl_generics #where_clause { #(#field_idents: <#field_types as #path::query::WorldQuery>::Fetch,)* - #(#phantom_field_idents: #phantom_field_types,)* + #(#ignored_field_idents: #ignored_field_types,)* } struct #state_struct_name #impl_generics #where_clause { #(#field_idents: <#field_types as #path::query::WorldQuery>::State,)* - #(#phantom_field_idents: #phantom_field_types,)* + #(#ignored_field_idents: #ignored_field_types,)* } impl #fetch_impl_generics #path::query::Fetch<#fetch_trait_punctuated_lifetimes> for #fetch_struct_name #ty_generics #where_clause { @@ -313,7 +379,7 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream { unsafe fn init(_world: &#path::world::World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self { #fetch_struct_name { #(#field_idents: <#field_types as #path::query::WorldQuery>::ReadOnlyFetch::init(_world, &state.#field_idents, _last_change_tick, _change_tick),)* - #(#phantom_field_idents: Default::default(),)* + #(#ignored_field_idents: Default::default(),)* } } @@ -347,7 +413,7 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream { fn init(world: &mut #path::world::World) -> Self { #state_struct_name { #(#field_idents: <<#field_types as #path::query::WorldQuery>::State as #path::query::FetchState>::init(world),)* - #(#phantom_field_idents: Default::default(),)* + #(#ignored_field_idents: Default::default(),)* } } @@ -392,26 +458,27 @@ struct FetchImplTokens<'a> { impl_generics: ImplGenerics<'a>, ty_generics: TypeGenerics<'a>, where_clause: Option<&'a WhereClause>, - has_mutable_attr: bool, has_world_lifetime: bool, world_lifetime: GenericParam, state_lifetime: GenericParam, fetch_lifetime: GenericParam, - phantom_field_idents: Vec, - phantom_field_types: Vec, + ignored_field_attrs: Vec>, + ignored_field_visibilities: Vec, + ignored_field_idents: Vec, + ignored_field_types: Vec, + field_attrs: Vec>, + field_visibilities: Vec, field_idents: Vec, field_types: Vec, query_types: Vec, fetch_init_types: Vec, } -fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens { - let has_mutable_attr = ast.attrs.iter().any(|attr| { - attr.path - .get_ident() - .map_or(false, |ident| ident == MUTABLE_ATTRIBUTE_NAME) - }); - +fn fetch_impl_tokens<'a>( + ast: &'a DeriveInput, + field_attr_name: &'static str, + has_mutable_attr: bool, +) -> FetchImplTokens<'a> { let world_lifetime = ast.generics.params.first().and_then(|param| match param { lt @ GenericParam::Lifetime(_) => Some(lt.clone()), _ => None, @@ -459,8 +526,12 @@ fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens { _ => panic!("Expected a struct with named fields"), }; - let mut phantom_field_idents = Vec::new(); - let mut phantom_field_types = Vec::new(); + 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 query_types = Vec::new(); @@ -473,17 +544,22 @@ fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens { _ => unreachable!(), }; for field in fields.iter() { - let WorldQueryFieldTypeInfo { + let WorldQueryFieldInfo { query_type, fetch_init_type: init_type, - is_phantom_data, - } = read_world_query_field_type_info(&field.ty, world_lifetime, fetch_lifetime); + is_ignored, + attrs, + } = read_world_query_field_info(field, field_attr_name, world_lifetime, fetch_lifetime); let field_ident = field.ident.as_ref().unwrap().clone(); - if is_phantom_data { - phantom_field_idents.push(field_ident.clone()); - phantom_field_types.push(field.ty.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()); field_types.push(field.ty.clone()); query_types.push(query_type); @@ -501,13 +577,16 @@ fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens { impl_generics, ty_generics, where_clause, - has_mutable_attr, has_world_lifetime, world_lifetime: world_lifetime_param, state_lifetime: state_lifetime_param, fetch_lifetime: fetch_lifetime_param, - phantom_field_idents, - phantom_field_types, + ignored_field_attrs, + ignored_field_visibilities, + ignored_field_idents, + ignored_field_types, + field_attrs, + field_visibilities, field_idents, field_types, query_types, @@ -515,42 +594,72 @@ fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens { } } -struct WorldQueryFieldTypeInfo { +struct WorldQueryFieldInfo { /// We convert `Mut` to `&mut T` (because this is the type that implements `WorldQuery`) /// and store it here. query_type: Type, /// The same as `query_type` but with `'fetch` lifetime. fetch_init_type: Type, - is_phantom_data: bool, + /// Has `#[fetch(ignore)]` or `#[filter_fetch(ignore)]` attribute. + is_ignored: bool, + /// All field attributes except for `fetch` or `filter_fetch`. + attrs: Vec, } -fn read_world_query_field_type_info( - ty: &Type, +fn read_world_query_field_info( + field: &Field, + field_attr_name: &'static str, world_lifetime: &Lifetime, fetch_lifetime: &Lifetime, -) -> WorldQueryFieldTypeInfo { +) -> WorldQueryFieldInfo { let path = bevy_ecs_path(); + let is_ignored = field + .attrs + .iter() + .find(|attr| { + attr.path + .get_ident() + .map_or(false, |ident| ident == field_attr_name) + }) + .map_or(false, |attr| { + let mut is_ignored = false; + attr.parse_args_with(|input: ParseStream| { + if input + .parse::>()? + .is_some() + { + is_ignored = true; + } + Ok(()) + }) + .unwrap_or_else(|_| panic!("Invalid `{}` attribute format", field_attr_name)); + + is_ignored + }); + + let attrs = field + .attrs + .iter() + .cloned() + .filter(|attr| { + attr.path + .get_ident() + .map_or(true, |ident| ident != field_attr_name) + }) + .collect(); + + let ty = &field.ty; let query_type: Type = parse_quote!(<#ty as #path::query::FetchedItem>::Query); let mut fetch_init_type: Type = query_type.clone(); - let is_phantom_data = match ty { - Type::Path(path) => { - if let Some(segment) = path.path.segments.last() { - segment.ident == "PhantomData" - } else { - false - } - } - _ => false, - }; - replace_lifetime_for_type(&mut fetch_init_type, world_lifetime, fetch_lifetime); - WorldQueryFieldTypeInfo { + WorldQueryFieldInfo { query_type, fetch_init_type, - is_phantom_data, + is_ignored, + attrs, } } diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 523cb1de54c96..9c6caf39d42c6 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -428,13 +428,13 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { } /// Implement `WorldQuery` to use a struct as a parameter in a query -#[proc_macro_derive(Fetch, attributes(mutable, read_only_derive))] +#[proc_macro_derive(Fetch, attributes(fetch))] pub fn derive_fetch(input: TokenStream) -> TokenStream { derive_fetch_impl(input) } /// Implement `FilterFetch` to use a struct as a filter parameter in a query -#[proc_macro_derive(FilterFetch)] +#[proc_macro_derive(FilterFetch, attributes(filter_fetch))] pub fn derive_filter_fetch(input: TokenStream) -> TokenStream { derive_filter_fetch_impl(input) } diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 27884d6d7fa79..d5c25b98e86b8 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -131,7 +131,7 @@ pub trait FetchedItem { /// struct Buff(f32); /// /// #[derive(Fetch)] -/// #[mutable] +/// #[fetch(mutable)] /// struct HealthQuery<'w> { /// // `Mut<'w, T>` is a necessary replacement for `&'w mut T` /// health: Mut<'w, Health>, @@ -184,8 +184,7 @@ pub trait FetchedItem { /// struct Foo; /// /// #[derive(Fetch, Debug)] -/// #[mutable] -/// #[read_only_derive(Debug)] +/// #[fetch(mutable, read_only_derive(Debug))] /// struct FooQuery<'w> { /// foo: &'w Foo, /// } @@ -217,7 +216,7 @@ pub trait FetchedItem { /// } /// /// #[derive(Fetch)] -/// #[mutable] +/// #[fetch(mutable)] /// struct BarQuery<'w> { /// bar: Mut<'w, Bar>, /// } @@ -263,6 +262,30 @@ pub trait FetchedItem { /// /// # my_system.system(); /// ``` +/// +/// ## Ignored fields +/// +/// The macro also supports `ignore` attribute for struct members. Fields marked with this attribute +/// must implement the `Default` trait. +/// +/// This example demonstrates a query that would iterate over every entity. +/// +/// ``` +/// # use bevy_ecs::prelude::*; +/// use bevy_ecs::query::Fetch; +/// +/// #[derive(Fetch, Debug)] +/// struct EmptyQuery<'w> { +/// #[fetch(ignore)] +/// _w: std::marker::PhantomData<&'w ()>, +/// } +/// +/// fn my_system(query: Query) { +/// for _ in query.iter() {} +/// } +/// +/// # my_system.system(); +/// ``` pub trait Fetch<'world, 'state>: Sized { type Item: FetchedItem; type State: FetchState; diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 84abe0b0570e9..105ab5c08f4e1 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -49,6 +49,7 @@ use std::{cell::UnsafeCell, marker::PhantomData, ptr}; /// _bar: With, /// _or: Or<(With, Changed, Added)>, /// _generic_tuple: (With, Without

), +/// #[filter_fetch(ignore)] /// _tp: std::marker::PhantomData<(T, P)>, /// } /// diff --git a/examples/ecs/custom_query_param.rs b/examples/ecs/custom_query_param.rs index c96b07f116e45..a5118b1174e0a 100644 --- a/examples/ecs/custom_query_param.rs +++ b/examples/ecs/custom_query_param.rs @@ -84,8 +84,7 @@ fn print_components_read_only( // Note: if you want to use derive macros with read-only query variants, you need to pass them with // using the `read_only_derive` attribute. #[derive(Fetch, Debug)] -#[mutable] -#[read_only_derive(Debug)] +#[fetch(mutable, read_only_derive(Debug))] struct CustomQuery<'w, T: Component + Debug, P: Component + Debug> { entity: Entity, // `Mut<'w, T>` is a necessary replacement for `&'w mut T` @@ -104,6 +103,7 @@ struct CustomQuery<'w, T: Component + Debug, P: Component + Debug> { struct EmptyQuery<'w> { // The Fetch derive macro expect a lifetime. As Rust doesn't allow unused lifetimes, we need // to use `PhantomData` as a work around. + #[fetch(ignore)] _w: std::marker::PhantomData<&'w ()>, } @@ -126,6 +126,7 @@ struct QueryFilter { _d: With, _or: Or<(Added, Changed, Without)>, _generic_tuple: (With, With

), + #[filter_fetch(ignore)] _tp: PhantomData<(T, P)>, } From cfe8db34039b1977bbdf6c80ffa6100e2b48eee1 Mon Sep 17 00:00:00 2001 From: mvlabat Date: Sat, 5 Feb 2022 10:53:16 +0200 Subject: [PATCH 10/14] Rename Fetch and FilterFetch macros to WorldQuery --- crates/bevy_ecs/macros/src/fetch.rs | 26 +++--- crates/bevy_ecs/macros/src/lib.rs | 51 ++++++++-- crates/bevy_ecs/src/query/fetch.rs | 139 +++++++++++++++++----------- crates/bevy_ecs/src/query/filter.rs | 45 --------- examples/ecs/custom_query_param.rs | 24 +++-- 5 files changed, 153 insertions(+), 132 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index e8603991f49dc..9bfa6a919686a 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -3,7 +3,7 @@ use proc_macro2::{Ident, Span}; use quote::{quote, ToTokens}; use syn::{ parse::{Parse, ParseStream}, - parse_macro_input, parse_quote, + parse_quote, punctuated::Punctuated, Attribute, Data, DataStruct, DeriveInput, Field, Fields, GenericArgument, GenericParam, ImplGenerics, Lifetime, LifetimeDef, Path, PathArguments, ReturnType, Token, Type, @@ -18,6 +18,7 @@ struct FetchStructAttributes { pub read_only_derive_args: Punctuated, } +pub static FILTER_ATTRIBUTE_NAME: &str = "filter"; static MUTABLE_ATTRIBUTE_NAME: &str = "mutable"; static READ_ONLY_DERIVE_ATTRIBUTE_NAME: &str = "read_only_derive"; @@ -25,18 +26,15 @@ mod field_attr_keywords { syn::custom_keyword!(ignore); } -static FETCH_ATTRIBUTE_NAME: &str = "fetch"; -static FILTER_FETCH_ATTRIBUTE_NAME: &str = "filter_fetch"; - -pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { - let ast = parse_macro_input!(input as DeriveInput); +pub static WORLD_QUERY_ATTRIBUTE_NAME: &str = "world_query"; +pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { let mut fetch_struct_attributes = FetchStructAttributes::default(); for attr in &ast.attrs { if !attr .path .get_ident() - .map_or(false, |ident| ident == FETCH_ATTRIBUTE_NAME) + .map_or(false, |ident| ident == WORLD_QUERY_ATTRIBUTE_NAME) { continue; } @@ -74,7 +72,7 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { } Ok(()) }) - .unwrap_or_else(|_| panic!("Invalid `{}` attribute format", FETCH_ATTRIBUTE_NAME)); + .unwrap_or_else(|_| panic!("Invalid `{}` attribute format", WORLD_QUERY_ATTRIBUTE_NAME)); } let FetchImplTokens { @@ -101,7 +99,11 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { field_types: _, query_types, fetch_init_types, - } = fetch_impl_tokens(&ast, FETCH_ATTRIBUTE_NAME, fetch_struct_attributes.mutable); + } = fetch_impl_tokens( + &ast, + WORLD_QUERY_ATTRIBUTE_NAME, + fetch_struct_attributes.mutable, + ); if !has_world_lifetime { panic!("Expected a struct with a lifetime"); @@ -319,9 +321,7 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream { tokens } -pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream { - let ast = parse_macro_input!(input as DeriveInput); - +pub fn derive_world_query_filter_impl(ast: DeriveInput) -> TokenStream { let FetchImplTokens { struct_name, struct_name_read_only: _, @@ -346,7 +346,7 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream { field_types, query_types: _, fetch_init_types: _, - } = fetch_impl_tokens(&ast, FILTER_FETCH_ATTRIBUTE_NAME, false); + } = fetch_impl_tokens(&ast, WORLD_QUERY_ATTRIBUTE_NAME, false); if has_world_lifetime { panic!("Expected a struct without a lifetime"); diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 9c6caf39d42c6..c8996fc4521cb 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -3,7 +3,10 @@ extern crate proc_macro; mod component; mod fetch; -use crate::fetch::{derive_fetch_impl, derive_filter_fetch_impl}; +use crate::fetch::{ + derive_world_query_filter_impl, derive_world_query_impl, FILTER_ATTRIBUTE_NAME, + WORLD_QUERY_ATTRIBUTE_NAME, +}; use bevy_macro_utils::{derive_label, get_named_struct_fields, BevyManifest}; use proc_macro::TokenStream; use proc_macro2::Span; @@ -428,15 +431,45 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { } /// Implement `WorldQuery` to use a struct as a parameter in a query -#[proc_macro_derive(Fetch, attributes(fetch))] -pub fn derive_fetch(input: TokenStream) -> TokenStream { - derive_fetch_impl(input) -} +#[proc_macro_derive(WorldQuery, attributes(world_query))] +pub fn derive_world_query(input: TokenStream) -> TokenStream { + let ast = parse_macro_input!(input as DeriveInput); + + let mut is_filter = false; -/// Implement `FilterFetch` to use a struct as a filter parameter in a query -#[proc_macro_derive(FilterFetch, attributes(filter_fetch))] -pub fn derive_filter_fetch(input: TokenStream) -> TokenStream { - derive_filter_fetch_impl(input) + for attr in &ast.attrs { + if !attr + .path + .get_ident() + .map_or(false, |ident| ident == WORLD_QUERY_ATTRIBUTE_NAME) + { + continue; + } + attr.parse_args_with(|input: ParseStream| { + let meta = input.parse_terminated::(syn::Meta::parse)?; + for meta in meta { + let ident = meta.path().get_ident(); + if ident.map_or(false, |ident| ident == FILTER_ATTRIBUTE_NAME) { + if let syn::Meta::Path(_) = meta { + is_filter = true; + } else { + panic!( + "The `{}` attribute is expected to have no value or arguments", + FILTER_ATTRIBUTE_NAME + ); + } + } + } + Ok(()) + }) + .unwrap_or_else(|_| panic!("Invalid `{}` attribute format", WORLD_QUERY_ATTRIBUTE_NAME)); + } + + if is_filter { + derive_world_query_filter_impl(ast) + } else { + derive_world_query_impl(ast) + } } #[proc_macro_derive(SystemLabel)] diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index d5c25b98e86b8..586f1dd1fd9d6 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -8,7 +8,7 @@ use crate::{ world::{Mut, World}, }; use bevy_ecs_macros::all_tuples; -pub use bevy_ecs_macros::{Fetch, FilterFetch}; +pub use bevy_ecs_macros::WorldQuery; use std::{ cell::UnsafeCell, marker::PhantomData, @@ -22,10 +22,6 @@ use std::{ /// /// See [`Query`](crate::system::Query) for a primer on queries. /// -/// If you want to implement a custom query, see [`Fetch`] trait documentation. -/// -/// If you want to implement a custom query filter, see [`FilterFetch`] trait documentation. -/// /// # Basic [`WorldQuery`]'s /// /// Here is a small list of the most important world queries to know about where `C` stands for a @@ -45,32 +41,12 @@ use std::{ /// For more information on these consult the item's corresponding documentation. /// /// [`Or`]: crate::query::Or -pub trait WorldQuery { - type Fetch: for<'world, 'state> Fetch<'world, 'state, State = Self::State>; - type State: FetchState; - type ReadOnlyFetch: for<'world, 'state> Fetch<'world, 'state, State = Self::State> - + ReadOnlyFetch; -} - -pub type QueryItem<'w, 's, Q> = <::Fetch as Fetch<'w, 's>>::Item; - -/// Types that appear as [`Fetch::Item`] associated types. /// -/// This trait comes in useful when it's needed to correlate between types (such as `Mut` to `&mut T`). -/// In most cases though, [`FetchedItem::Query`] corresponds to a type that implements the trait. -pub trait FetchedItem { - type Query: WorldQuery; -} - -/// Types that implement this trait are responsible for fetching query items from tables or -/// archetypes. +/// # Derive /// -/// Every type that implements [`WorldQuery`] have their associated [`WorldQuery::Fetch`] and -/// [`WorldQuery::State`] types that are essential for fetching component data. If you want to -/// implement a custom query type, you'll need to implement [`Fetch`] and [`FetchState`] for -/// those associated types. +/// This trait can be derived with the [`derive@super::WorldQuery`] macro. /// -/// You may want to implement a custom query for the following reasons: +/// You may want to implement a custom query with the derive macro for the following reasons: /// - Named structs can be clearer and easier to use than complex query tuples. Access via struct /// fields is more convenient than destructuring tuples or accessing them via `q.0, q.1, ...` /// pattern and saves a lot of maintenance burden when adding or removing components. @@ -79,27 +55,25 @@ pub trait FetchedItem { /// /// Implementing the trait manually can allow for a fundamentally new type of behaviour. /// -/// # Derive -/// -/// This trait can be derived with the [`derive@super::Fetch`] macro. -/// To do so, all fields in the struct must themselves impl [`WorldQuery`]. -/// /// The derive macro implements [`WorldQuery`] for your type and declares two structs that /// implement [`Fetch`] and [`FetchState`] and are used as [`WorldQuery::Fetch`] and /// [`WorldQuery::State`] associated types respectively. /// +/// The derive macro requires each member to implement the [`FetchedItem`] trait. This trait +/// is automatically implemented when using the derive macro as well, to allow nested queries. +/// /// **Note:** currently, the macro only supports named structs. /// /// ``` /// # use bevy_ecs::prelude::*; -/// use bevy_ecs::query::Fetch; +/// use bevy_ecs::query::WorldQuery; /// /// #[derive(Component)] /// struct Foo; /// #[derive(Component)] /// struct Bar; /// -/// #[derive(Fetch)] +/// #[derive(WorldQuery)] /// struct MyQuery<'w> { /// entity: Entity, /// foo: &'w Foo, @@ -117,21 +91,21 @@ pub trait FetchedItem { /// /// ## Mutable queries /// -/// All queries that are derived with `Fetch` macro provide only an immutable access by default. +/// All queries that are derived with the `WorldQuery` macro provide only an immutable access by default. /// If you need a mutable access to components, you can mark a struct with the `mutable` attribute. /// The macro will still generate a read-only variant of a query suffixed with `ReadOnly`. /// /// ``` /// # use bevy_ecs::prelude::*; -/// use bevy_ecs::query::{Fetch, ReadOnlyFetch, WorldQuery}; +/// use bevy_ecs::query::WorldQuery; /// /// #[derive(Component)] /// struct Health(f32); /// #[derive(Component)] /// struct Buff(f32); /// -/// #[derive(Fetch)] -/// #[fetch(mutable)] +/// #[derive(WorldQuery)] +/// #[world_query(mutable)] /// struct HealthQuery<'w> { /// // `Mut<'w, T>` is a necessary replacement for `&'w mut T` /// health: Mut<'w, Health>, @@ -178,13 +152,13 @@ pub trait FetchedItem { /// /// ``` /// # use bevy_ecs::prelude::*; -/// use bevy_ecs::query::{Fetch, ReadOnlyFetch, WorldQuery}; +/// use bevy_ecs::query::WorldQuery; /// /// #[derive(Component, Debug)] /// struct Foo; /// -/// #[derive(Fetch, Debug)] -/// #[fetch(mutable, read_only_derive(Debug))] +/// #[derive(WorldQuery, Debug)] +/// #[world_query(mutable, read_only_derive(Debug))] /// struct FooQuery<'w> { /// foo: &'w Foo, /// } @@ -202,21 +176,21 @@ pub trait FetchedItem { /// /// ```compile_fail /// # use bevy_ecs::prelude::*; -/// use bevy_ecs::query::{Fetch, ReadOnlyFetch, WorldQuery}; +/// use bevy_ecs::query::WorldQuery; /// /// #[derive(Component)] /// struct Foo; /// #[derive(Component)] /// struct Bar; /// -/// #[derive(Fetch)] +/// #[derive(WorldQuery)] /// struct FooQuery<'w> { /// foo: &'w Foo, /// bar_query: BarQuery<'w>, /// } /// -/// #[derive(Fetch)] -/// #[fetch(mutable)] +/// #[derive(WorldQuery)] +/// #[world_query(mutable)] /// struct BarQuery<'w> { /// bar: Mut<'w, Bar>, /// } @@ -230,7 +204,7 @@ pub trait FetchedItem { /// /// ``` /// # use bevy_ecs::prelude::*; -/// use bevy_ecs::query::Fetch; +/// use bevy_ecs::query::WorldQuery; /// /// #[derive(Component)] /// struct Foo; @@ -241,13 +215,13 @@ pub trait FetchedItem { /// #[derive(Component)] /// struct OptionalBar; /// -/// #[derive(Fetch)] +/// #[derive(WorldQuery)] /// struct MyQuery<'w> { /// foo: FooQuery<'w>, /// bar: (&'w Bar, Option<&'w OptionalBar>) /// } /// -/// #[derive(Fetch)] +/// #[derive(WorldQuery)] /// struct FooQuery<'w> { /// foo: &'w Foo, /// optional_foo: Option<&'w OptionalFoo>, @@ -272,11 +246,11 @@ pub trait FetchedItem { /// /// ``` /// # use bevy_ecs::prelude::*; -/// use bevy_ecs::query::Fetch; +/// use bevy_ecs::query::WorldQuery; /// -/// #[derive(Fetch, Debug)] +/// #[derive(WorldQuery, Debug)] /// struct EmptyQuery<'w> { -/// #[fetch(ignore)] +/// #[world_query(ignore)] /// _w: std::marker::PhantomData<&'w ()>, /// } /// @@ -286,6 +260,67 @@ pub trait FetchedItem { /// /// # my_system.system(); /// ``` +/// +/// ## Filters +/// +/// Using [`derive@super::WorldQuery`] macro in conjunctions with the `#[world_query(filter)]` +/// attribute allows creating custom query filters. +/// +/// To do so, all fields in the struct must be filters themselves (their [`WorldQuery::Fetch`] +/// associated types should implement [`super::FilterFetch`]). +/// +/// ``` +/// # use bevy_ecs::prelude::*; +/// use bevy_ecs::{query::WorldQuery, component::Component}; +/// +/// #[derive(Component)] +/// struct Foo; +/// #[derive(Component)] +/// struct Bar; +/// #[derive(Component)] +/// struct Baz; +/// #[derive(Component)] +/// struct Qux; +/// +/// #[derive(WorldQuery)] +/// #[world_query(filter)] +/// struct MyFilter { +/// _foo: With, +/// _bar: With, +/// _or: Or<(With, Changed, Added)>, +/// _generic_tuple: (With, Without

), +/// #[world_query(ignore)] +/// _tp: std::marker::PhantomData<(T, P)>, +/// } +/// +/// fn my_system(query: Query>) { +/// for _ in query.iter() {} +/// } +/// +/// # my_system.system(); +/// ``` +pub trait WorldQuery { + type Fetch: for<'world, 'state> Fetch<'world, 'state, State = Self::State>; + type State: FetchState; + type ReadOnlyFetch: for<'world, 'state> Fetch<'world, 'state, State = Self::State> + + ReadOnlyFetch; +} + +pub type QueryItem<'w, 's, Q> = <::Fetch as Fetch<'w, 's>>::Item; + +/// Types that appear as [`Fetch::Item`] associated types. +/// +/// This trait comes in useful when it's needed to correlate between types (such as `Mut` to `&mut T`). +/// In most cases though, [`FetchedItem::Query`] corresponds to a type that implements the trait. +pub trait FetchedItem { + type Query: WorldQuery; +} + +/// Types that implement this trait are responsible for fetching query items from tables or +/// archetypes. +/// +/// Every type that implements [`WorldQuery`] have their associated [`WorldQuery::Fetch`] and +/// [`WorldQuery::State`] types that are essential for fetching component data. pub trait Fetch<'world, 'state>: Sized { type Item: FetchedItem; type State: FetchState; diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 105ab5c08f4e1..9e93cd3d6e31f 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -14,51 +14,6 @@ use std::{cell::UnsafeCell, marker::PhantomData, ptr}; /// /// This trait is automatically implemented for every type that implements [`Fetch`] trait and /// specifies `bool` as the associated type for [`Fetch::Item`]. -/// -/// Using [`derive@super::FilterFetch`] macro allows creating custom query filters. -/// You may want to implement a custom query filter for the following reasons: -/// - Nested query filters enable the composition pattern and makes them easier to re-use. -/// - You can bypass the limit of 15 components that exists for query filters declared as tuples. -/// -/// Implementing the trait manually can allow for a fundamentally new type of behaviour. -/// -/// ## Derive -/// -/// This trait can be derived with the [`derive@super::FilterFetch`] macro. -/// To do so, all fields in the struct must be filters themselves (their [`WorldQuery::Fetch`] -/// associated types should implement [`FilterFetch`]). -/// -/// **Note:** currently, the macro only supports named structs. -/// -/// ``` -/// # use bevy_ecs::prelude::*; -/// use bevy_ecs::{query::FilterFetch, component::Component}; -/// -/// #[derive(Component)] -/// struct Foo; -/// #[derive(Component)] -/// struct Bar; -/// #[derive(Component)] -/// struct Baz; -/// #[derive(Component)] -/// struct Qux; -/// -/// #[derive(FilterFetch)] -/// struct MyFilter { -/// _foo: With, -/// _bar: With, -/// _or: Or<(With, Changed, Added)>, -/// _generic_tuple: (With, Without

), -/// #[filter_fetch(ignore)] -/// _tp: std::marker::PhantomData<(T, P)>, -/// } -/// -/// fn my_system(query: Query>) { -/// for _ in query.iter() {} -/// } -/// -/// # my_system.system(); -/// ``` pub trait FilterFetch: for<'w, 's> Fetch<'w, 's> { /// # Safety /// diff --git a/examples/ecs/custom_query_param.rs b/examples/ecs/custom_query_param.rs index a5118b1174e0a..8f19d33131b4b 100644 --- a/examples/ecs/custom_query_param.rs +++ b/examples/ecs/custom_query_param.rs @@ -1,8 +1,5 @@ use bevy::{ - ecs::{ - component::Component, - query::{Fetch, FilterFetch}, - }, + ecs::{component::Component, query::WorldQuery}, prelude::*, }; use std::{fmt::Debug, marker::PhantomData}; @@ -49,7 +46,7 @@ struct ComponentD; #[derive(Component, Debug)] struct ComponentZ; -#[derive(Fetch)] +#[derive(WorldQuery)] struct ReadOnlyCustomQuery<'w, T: Component + Debug, P: Component + Debug> { entity: Entity, a: &'w ComponentA, @@ -83,8 +80,8 @@ fn print_components_read_only( // suffix. // Note: if you want to use derive macros with read-only query variants, you need to pass them with // using the `read_only_derive` attribute. -#[derive(Fetch, Debug)] -#[fetch(mutable, read_only_derive(Debug))] +#[derive(WorldQuery, Debug)] +#[world_query(mutable, read_only_derive(Debug))] struct CustomQuery<'w, T: Component + Debug, P: Component + Debug> { entity: Entity, // `Mut<'w, T>` is a necessary replacement for `&'w mut T` @@ -99,34 +96,35 @@ struct CustomQuery<'w, T: Component + Debug, P: Component + Debug> { } // This is a valid query as well, which would iterate over every entity. -#[derive(Fetch, Debug)] +#[derive(WorldQuery, Debug)] struct EmptyQuery<'w> { // The Fetch derive macro expect a lifetime. As Rust doesn't allow unused lifetimes, we need // to use `PhantomData` as a work around. - #[fetch(ignore)] + #[world_query(ignore)] _w: std::marker::PhantomData<&'w ()>, } -#[derive(Fetch, Debug)] +#[derive(WorldQuery, Debug)] #[allow(dead_code)] struct NestedQuery<'w> { c: &'w ComponentC, d: Option<&'w ComponentD>, } -#[derive(Fetch, Debug)] +#[derive(WorldQuery, Debug)] #[allow(dead_code)] struct GenericQuery<'w, T: Component, P: Component> { generic: (&'w T, &'w P), } -#[derive(FilterFetch)] +#[derive(WorldQuery)] +#[world_query(filter)] struct QueryFilter { _c: With, _d: With, _or: Or<(Added, Changed, Without)>, _generic_tuple: (With, With

), - #[filter_fetch(ignore)] + #[world_query(ignore)] _tp: PhantomData<(T, P)>, } From a2f3d3ba23c6dfc9c4882f09f731fc66758f8cdd Mon Sep 17 00:00:00 2001 From: mvlabat Date: Mon, 7 Feb 2022 01:51:54 +0200 Subject: [PATCH 11/14] Drop the FetchedItem trait, generate QueryItem structs --- crates/bevy_ecs/macros/src/fetch.rs | 101 ++++++++++++------------ crates/bevy_ecs/src/query/fetch.rs | 118 ++++++++++------------------ crates/bevy_ecs/src/query/filter.rs | 20 +---- examples/ecs/custom_query_param.rs | 39 ++++----- 4 files changed, 114 insertions(+), 164 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 9bfa6a919686a..fe0028615ef77 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -3,7 +3,6 @@ use proc_macro2::{Ident, Span}; use quote::{quote, ToTokens}; use syn::{ parse::{Parse, ParseStream}, - parse_quote, punctuated::Punctuated, Attribute, Data, DataStruct, DeriveInput, Field, Fields, GenericArgument, GenericParam, ImplGenerics, Lifetime, LifetimeDef, Path, PathArguments, ReturnType, Token, Type, @@ -15,12 +14,12 @@ use crate::bevy_ecs_path; #[derive(Default)] struct FetchStructAttributes { pub mutable: bool, - pub read_only_derive_args: Punctuated, + pub derive_args: Punctuated, } pub static FILTER_ATTRIBUTE_NAME: &str = "filter"; static MUTABLE_ATTRIBUTE_NAME: &str = "mutable"; -static READ_ONLY_DERIVE_ATTRIBUTE_NAME: &str = "read_only_derive"; +static DERIVE_ATTRIBUTE_NAME: &str = "derive"; mod field_attr_keywords { syn::custom_keyword!(ignore); @@ -52,15 +51,15 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { MUTABLE_ATTRIBUTE_NAME ); } - } else if ident.map_or(false, |ident| ident == READ_ONLY_DERIVE_ATTRIBUTE_NAME) { + } else if ident.map_or(false, |ident| ident == DERIVE_ATTRIBUTE_NAME) { if let syn::Meta::List(meta_list) = meta { fetch_struct_attributes - .read_only_derive_args + .derive_args .extend(meta_list.nested.iter().cloned()); } else { panic!( "Expected a structured list within the `{}` attribute", - READ_ONLY_DERIVE_ATTRIBUTE_NAME + DERIVE_ATTRIBUTE_NAME ); } } else { @@ -77,7 +76,8 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { let FetchImplTokens { struct_name, - struct_name_read_only, + item_struct_name, + read_only_item_struct_name, fetch_struct_name, state_struct_name, read_only_fetch_struct_name, @@ -109,16 +109,10 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { panic!("Expected a struct with a lifetime"); } - let read_only_derive_macro_call = if fetch_struct_attributes.read_only_derive_args.is_empty() { + let derive_macro_call = if fetch_struct_attributes.derive_args.is_empty() { quote! {} } else { - if !fetch_struct_attributes.mutable { - panic!( - "Attribute `{}` can only be be used for a struct marked with the `{}` attribute", - READ_ONLY_DERIVE_ATTRIBUTE_NAME, MUTABLE_ATTRIBUTE_NAME - ); - } - let derive_args = &fetch_struct_attributes.read_only_derive_args; + let derive_args = &fetch_struct_attributes.derive_args; quote! { #[derive(#derive_args)] } }; @@ -137,11 +131,11 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { let struct_read_only_declaration = if fetch_struct_attributes.mutable { quote! { - // TODO: it would be great to be able to dedup this by just deriving `Fetch` again + // TODO: it would be great to be able to dedup this by just deriving `WorldQuery` again // without the `mutable` attribute, but we'd need a way to avoid creating a redundant // `State` struct. - #read_only_derive_macro_call - struct #struct_name_read_only #impl_generics #where_clause { + #derive_macro_call + struct #read_only_item_struct_name #impl_generics #where_clause { #(#(#field_attrs)* #field_visibilities #field_idents: <<#query_types as #path::query::WorldQuery>::ReadOnlyFetch as #path::query::Fetch<#world_lifetime, #world_lifetime>>::Item,)* #(#(#ignored_field_attrs)* #ignored_field_visibilities #ignored_field_idents: #ignored_field_types,)* } @@ -152,7 +146,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { } impl #fetch_impl_generics #path::query::Fetch<#fetch_trait_punctuated_lifetimes> for #read_only_fetch_struct_name #fetch_ty_generics #where_clause { - type Item = #struct_name_read_only #ty_generics; + type Item = #read_only_item_struct_name #ty_generics; type State = #state_struct_name #fetch_ty_generics; unsafe fn init(_world: &#path::world::World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self { @@ -179,7 +173,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { /// SAFETY: we call `table_fetch` for each member that implements `Fetch`. #[inline] unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item { - #struct_name_read_only { + Self::Item { #(#field_idents: self.#field_idents.table_fetch(_table_row),)* #(#ignored_field_idents: Default::default(),)* } @@ -188,22 +182,18 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { /// SAFETY: we call `archetype_fetch` for each member that implements `Fetch`. #[inline] unsafe fn archetype_fetch(&mut self, _archetype_index: usize) -> Self::Item { - #struct_name_read_only { + Self::Item { #(#field_idents: self.#field_idents.archetype_fetch(_archetype_index),)* #(#ignored_field_idents: Default::default(),)* } } } - impl #impl_generics #path::query::WorldQuery for #struct_name_read_only #ty_generics #where_clause { + impl #impl_generics #path::query::WorldQuery for #read_only_item_struct_name #ty_generics #where_clause { type Fetch = #read_only_fetch_struct_name #ty_generics; type State = #state_struct_name #ty_generics; type ReadOnlyFetch = #read_only_fetch_struct_name #ty_generics; } - - impl #impl_generics #path::query::FetchedItem for #struct_name_read_only #ty_generics #where_clause { - type Query = Self; - } } } else { quote! { @@ -223,6 +213,12 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { }; let tokens = TokenStream::from(quote! { + #derive_macro_call + struct #item_struct_name #impl_generics #where_clause { + #(#(#field_attrs)* #field_visibilities #field_idents: <<#query_types as #path::query::WorldQuery>::Fetch as #path::query::Fetch<#world_lifetime, #world_lifetime>>::Item,)* + #(#(#ignored_field_attrs)* #ignored_field_visibilities #ignored_field_idents: #ignored_field_types,)* + } + struct #fetch_struct_name #impl_generics #where_clause { #(#field_idents: <#query_types as #path::query::WorldQuery>::Fetch,)* #(#ignored_field_idents: #ignored_field_types,)* @@ -234,11 +230,11 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { } impl #fetch_impl_generics #path::query::Fetch<#fetch_trait_punctuated_lifetimes> for #fetch_struct_name #fetch_ty_generics #where_clause { - type Item = #struct_name #ty_generics; + type Item = #item_struct_name #ty_generics; type State = #state_struct_name #fetch_ty_generics; unsafe fn init(_world: &#path::world::World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self { - #fetch_struct_name { + Self { #(#field_idents: <#fetch_init_types as #path::query::WorldQuery>::Fetch::init(_world, &state.#field_idents, _last_change_tick, _change_tick),)* #(#ignored_field_idents: Default::default(),)* } @@ -261,7 +257,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { /// SAFETY: we call `table_fetch` for each member that implements `Fetch`. #[inline] unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item { - #struct_name { + Self::Item { #(#field_idents: self.#field_idents.table_fetch(_table_row),)* #(#ignored_field_idents: Default::default(),)* } @@ -270,7 +266,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { /// SAFETY: we call `archetype_fetch` for each member that implements `Fetch`. #[inline] unsafe fn archetype_fetch(&mut self, _archetype_index: usize) -> Self::Item { - #struct_name { + Self::Item { #(#field_idents: self.#field_idents.archetype_fetch(_archetype_index),)* #(#ignored_field_idents: Default::default(),)* } @@ -311,12 +307,19 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { type ReadOnlyFetch = #read_only_fetch_struct_name #ty_generics; } - impl #impl_generics #path::query::FetchedItem for #struct_name #ty_generics #where_clause { - type Query = Self; - } - /// SAFETY: each item in the struct is read only unsafe impl #impl_generics #path::query::ReadOnlyFetch for #read_only_fetch_struct_name #ty_generics #where_clause {} + + // The original struct will most likely be left unused. As we don't want our users having + // to specify `#[allow(dead_code)]` for their custom queries, we are using this cursed + // workaround. + #[allow(dead_code)] + const _: () = { + fn dead_code_workaround #impl_generics (q: #struct_name #ty_generics) #where_clause { + #(q.#field_idents;)* + #(q.#ignored_field_idents;)* + } + }; }); tokens } @@ -324,7 +327,8 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { pub fn derive_world_query_filter_impl(ast: DeriveInput) -> TokenStream { let FetchImplTokens { struct_name, - struct_name_read_only: _, + item_struct_name: _, + read_only_item_struct_name: _, fetch_struct_name, state_struct_name, read_only_fetch_struct_name: _, @@ -446,11 +450,12 @@ pub fn derive_world_query_filter_impl(ast: DeriveInput) -> TokenStream { tokens } -// This struct is used to share common tokens between `Fetch` and `FilterFetch` implementations. +// This struct is used to share common tokens between query and filter implementations. struct FetchImplTokens<'a> { struct_name: Ident, - // Equals `struct_name` if `has_mutable_attr` is false. - struct_name_read_only: Ident, + item_struct_name: Ident, + // Equals `item_struct_name` if `has_mutable_attr` is false. + read_only_item_struct_name: Ident, fetch_struct_name: Ident, state_struct_name: Ident, read_only_fetch_struct_name: Ident, @@ -505,10 +510,11 @@ fn fetch_impl_tokens<'a>( let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl(); let struct_name = ast.ident.clone(); - let struct_name_read_only = if has_mutable_attr { - Ident::new(&format!("{}ReadOnly", struct_name), Span::call_site()) + let item_struct_name = Ident::new(&format!("{}Item", struct_name), Span::call_site()); + let read_only_item_struct_name = if has_mutable_attr { + Ident::new(&format!("{}ReadOnlyItem", struct_name), Span::call_site()) } else { - ast.ident.clone() + item_struct_name.clone() }; let fetch_struct_name = Ident::new(&format!("{}Fetch", struct_name), Span::call_site()); let state_struct_name = Ident::new(&format!("{}State", struct_name), Span::call_site()); @@ -569,7 +575,8 @@ fn fetch_impl_tokens<'a>( FetchImplTokens { struct_name, - struct_name_read_only, + item_struct_name, + read_only_item_struct_name, fetch_struct_name, state_struct_name, read_only_fetch_struct_name, @@ -595,14 +602,13 @@ fn fetch_impl_tokens<'a>( } struct WorldQueryFieldInfo { - /// We convert `Mut` to `&mut T` (because this is the type that implements `WorldQuery`) - /// and store it here. + /// The original field type. query_type: Type, /// The same as `query_type` but with `'fetch` lifetime. fetch_init_type: Type, /// Has `#[fetch(ignore)]` or `#[filter_fetch(ignore)]` attribute. is_ignored: bool, - /// All field attributes except for `fetch` or `filter_fetch`. + /// All field attributes except for `world_query` ones. attrs: Vec, } @@ -612,8 +618,6 @@ fn read_world_query_field_info( world_lifetime: &Lifetime, fetch_lifetime: &Lifetime, ) -> WorldQueryFieldInfo { - let path = bevy_ecs_path(); - let is_ignored = field .attrs .iter() @@ -649,8 +653,7 @@ fn read_world_query_field_info( }) .collect(); - let ty = &field.ty; - let query_type: Type = parse_quote!(<#ty as #path::query::FetchedItem>::Query); + let query_type: Type = field.ty.clone(); let mut fetch_init_type: Type = query_type.clone(); replace_lifetime_for_type(&mut fetch_init_type, world_lifetime, fetch_lifetime); diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 586f1dd1fd9d6..9ef4a7d7f4f7d 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -55,12 +55,12 @@ use std::{ /// /// Implementing the trait manually can allow for a fundamentally new type of behaviour. /// -/// The derive macro implements [`WorldQuery`] for your type and declares two structs that -/// implement [`Fetch`] and [`FetchState`] and are used as [`WorldQuery::Fetch`] and +/// The derive macro implements [`WorldQuery`] for your type and declares an additional struct +/// which will be used as an item for query iterators. The implementation also generates two other +/// structs that implement [`Fetch`] and [`FetchState`] and are used as [`WorldQuery::Fetch`] and /// [`WorldQuery::State`] associated types respectively. /// -/// The derive macro requires each member to implement the [`FetchedItem`] trait. This trait -/// is automatically implemented when using the derive macro as well, to allow nested queries. +/// The derive macro requires every struct field to implement the `WorldQuery` trait. /// /// **Note:** currently, the macro only supports named structs. /// @@ -82,6 +82,8 @@ use std::{ /// /// fn my_system(query: Query) { /// for q in query.iter() { +/// // Note the type of the returned item. +/// let q: MyQueryItem<'_> = q; /// q.foo; /// } /// } @@ -93,7 +95,6 @@ use std::{ /// /// All queries that are derived with the `WorldQuery` macro provide only an immutable access by default. /// If you need a mutable access to components, you can mark a struct with the `mutable` attribute. -/// The macro will still generate a read-only variant of a query suffixed with `ReadOnly`. /// /// ``` /// # use bevy_ecs::prelude::*; @@ -107,13 +108,12 @@ use std::{ /// #[derive(WorldQuery)] /// #[world_query(mutable)] /// struct HealthQuery<'w> { -/// // `Mut<'w, T>` is a necessary replacement for `&'w mut T` -/// health: Mut<'w, Health>, -/// buff: Option>, +/// health: &'w mut Health, +/// buff: Option<&'w mut Buff>, /// } /// /// // This implementation is only available when iterating with `iter_mut`. -/// impl<'w> HealthQuery<'w> { +/// impl<'w> HealthQueryItem<'w> { /// fn damage(&mut self, value: f32) { /// self.health.0 -= value; /// } @@ -124,18 +124,18 @@ use std::{ /// } /// /// // If you want to use it with `iter`, you'll need to write an additional implementation. -/// impl<'w> HealthQueryReadOnly<'w> { +/// impl<'w> HealthQueryReadOnlyItem<'w> { /// fn total(&self) -> f32 { /// self.health.0 + self.buff.map_or(0.0, |Buff(buff)| *buff) /// } /// } /// /// fn my_system(mut health_query: Query) { -/// // Iterator's item is `HealthQueryReadOnly`. +/// // Iterator's item is `HealthQueryReadOnlyItem`. /// for health in health_query.iter() { /// println!("Total: {}", health.total()); /// } -/// // Iterator's item is `HealthQuery`. +/// // Iterator's item is `HealthQueryItem`. /// for mut health in health_query.iter_mut() { /// health.damage(1.0); /// println!("Total (mut): {}", health.total()); @@ -144,31 +144,6 @@ use std::{ /// # my_system.system(); /// ``` /// -/// If you want to use derive macros with read-only query variants, you need to pass them with -/// using the `read_only_derive` attribute. When the `Fetch` macro generates an additional struct -/// for a mutable query, it doesn't automatically inherit the same derives. Since derive macros -/// can't access information about other derives, they need to be passed manually with the -/// `read_only_derive` attribute. -/// -/// ``` -/// # use bevy_ecs::prelude::*; -/// use bevy_ecs::query::WorldQuery; -/// -/// #[derive(Component, Debug)] -/// struct Foo; -/// -/// #[derive(WorldQuery, Debug)] -/// #[world_query(mutable, read_only_derive(Debug))] -/// struct FooQuery<'w> { -/// foo: &'w Foo, -/// } -/// -/// fn assert_debug() {} -/// -/// assert_debug::(); -/// assert_debug::(); -/// ``` -/// /// **Note:** if you omit the `mutable` attribute for a query that doesn't implement /// `ReadOnlyFetch`, compilation will fail. We insert static checks as in the example above for /// every query component and a nested query. @@ -192,8 +167,35 @@ use std::{ /// #[derive(WorldQuery)] /// #[world_query(mutable)] /// struct BarQuery<'w> { -/// bar: Mut<'w, Bar>, +/// bar: &'w mut Bar, +/// } +/// ``` +/// +/// ## Derives for items +/// +/// If you want query items to have derivable traits, you can pass them with using +/// the `world_query(derive)` attribute. When the `WorldQuery` macro generates the structs +/// for query items, it doesn't automatically inherit derives of a query itself. Since derive macros +/// can't access information about other derives, they need to be passed manually with the +/// `world_query(derive)` attribute. +/// +/// ``` +/// # use bevy_ecs::prelude::*; +/// use bevy_ecs::query::WorldQuery; +/// +/// #[derive(Component, Debug)] +/// struct Foo; +/// +/// #[derive(WorldQuery)] +/// #[world_query(mutable, derive(Debug))] +/// struct FooQuery<'w> { +/// foo: &'w Foo, /// } +/// +/// fn assert_debug() {} +/// +/// assert_debug::(); +/// assert_debug::(); /// ``` /// /// ## Nested queries @@ -308,21 +310,13 @@ pub trait WorldQuery { pub type QueryItem<'w, 's, Q> = <::Fetch as Fetch<'w, 's>>::Item; -/// Types that appear as [`Fetch::Item`] associated types. -/// -/// This trait comes in useful when it's needed to correlate between types (such as `Mut` to `&mut T`). -/// In most cases though, [`FetchedItem::Query`] corresponds to a type that implements the trait. -pub trait FetchedItem { - type Query: WorldQuery; -} - /// Types that implement this trait are responsible for fetching query items from tables or /// archetypes. /// /// Every type that implements [`WorldQuery`] have their associated [`WorldQuery::Fetch`] and /// [`WorldQuery::State`] types that are essential for fetching component data. pub trait Fetch<'world, 'state>: Sized { - type Item: FetchedItem; + type Item; type State: FetchState; /// Creates a new instance of this fetch. @@ -417,10 +411,6 @@ impl WorldQuery for Entity { type ReadOnlyFetch = EntityFetch; } -impl FetchedItem for Entity { - type Query = Self; -} - /// The [`Fetch`] of [`Entity`]. #[derive(Clone)] pub struct EntityFetch { @@ -508,10 +498,6 @@ impl WorldQuery for &T { type ReadOnlyFetch = ReadFetch; } -impl FetchedItem for &T { - type Query = Self; -} - /// The [`FetchState`] of `&T`. pub struct ReadState { component_id: ComponentId, @@ -667,10 +653,6 @@ impl WorldQuery for &mut T { type ReadOnlyFetch = ReadOnlyWriteFetch; } -impl<'a, T: Component> FetchedItem for Mut<'a, T> { - type Query = &'a mut T; -} - /// The [`Fetch`] of `&mut T`. pub struct WriteFetch { table_components: NonNull, @@ -954,10 +936,6 @@ impl WorldQuery for Option { type ReadOnlyFetch = OptionFetch; } -impl FetchedItem for Option { - type Query = Option; -} - /// The [`Fetch`] of `Option`. #[derive(Clone)] pub struct OptionFetch { @@ -1134,10 +1112,6 @@ impl WorldQuery for ChangeTrackers { type ReadOnlyFetch = ChangeTrackersFetch; } -impl FetchedItem for ChangeTrackers { - type Query = Self; -} - /// The [`FetchState`] of [`ChangeTrackers`]. pub struct ChangeTrackersState { component_id: ComponentId, @@ -1389,10 +1363,6 @@ macro_rules! impl_tuple_fetch { type ReadOnlyFetch = ($($name::ReadOnlyFetch,)*); } - impl<$($name: FetchedItem),*> FetchedItem for ($($name,)*) { - type Query = ($($name::Query,)*); - } - /// SAFETY: each item in the tuple is read only unsafe impl<$($name: ReadOnlyFetch),*> ReadOnlyFetch for ($($name,)*) {} @@ -1555,9 +1525,3 @@ impl<'w, 's, State: FetchState> Fetch<'w, 's> for NopFetch { #[inline(always)] unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item {} } - -/// This implementation won't allow us to correlate a boolean to a filter type. But having a dummy -/// for `bool` allows us to add `FetchedItem` bound to [`Fetch::Item`]. -impl FetchedItem for bool { - type Query = (); -} diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 9e93cd3d6e31f..6c936f2d0a538 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -2,7 +2,7 @@ use crate::{ archetype::{Archetype, ArchetypeComponentId}, component::{Component, ComponentId, ComponentStorage, ComponentTicks, StorageType}, entity::Entity, - query::{Access, Fetch, FetchState, FetchedItem, FilteredAccess, ReadOnlyFetch, WorldQuery}, + query::{Access, Fetch, FetchState, FilteredAccess, ReadOnlyFetch, WorldQuery}, storage::{ComponentSparseSet, Table, Tables}, world::World, }; @@ -78,10 +78,6 @@ impl WorldQuery for With { type ReadOnlyFetch = WithFetch; } -impl FetchedItem for With { - type Query = Self; -} - /// The [`Fetch`] of [`With`]. pub struct WithFetch { marker: PhantomData, @@ -205,10 +201,6 @@ impl WorldQuery for Without { type ReadOnlyFetch = WithoutFetch; } -impl FetchedItem for Without { - type Query = Self; -} - /// The [`Fetch`] of [`Without`]. pub struct WithoutFetch { marker: PhantomData, @@ -364,12 +356,6 @@ macro_rules! impl_query_filter_tuple { type ReadOnlyFetch = Or<($(OrFetch<$filter::ReadOnlyFetch>,)*)>; } - impl<$($filter: WorldQuery),*> FetchedItem for Or<($($filter,)*)> - where $($filter::Fetch: FilterFetch, $filter::ReadOnlyFetch: FilterFetch),* - { - type Query = Self; - } - /// SAFETY: this only works using the filter which doesn't write unsafe impl<$($filter: FilterFetch + ReadOnlyFetch),*> ReadOnlyFetch for Or<($(OrFetch<$filter>,)*)> {} @@ -495,10 +481,6 @@ macro_rules! impl_tick_filter { type ReadOnlyFetch = $fetch_name; } - impl FetchedItem for $name { - type Query = Self; - } - // SAFETY: this reads the T component. archetype component access and component access are updated to reflect that unsafe impl FetchState for $state_name { fn init(world: &mut World) -> Self { diff --git a/examples/ecs/custom_query_param.rs b/examples/ecs/custom_query_param.rs index 8f19d33131b4b..e1fadb78363f8 100644 --- a/examples/ecs/custom_query_param.rs +++ b/examples/ecs/custom_query_param.rs @@ -4,7 +4,7 @@ use bevy::{ }; use std::{fmt::Debug, marker::PhantomData}; -/// This examples illustrates the usage of `Fetch` and `FilterFetch` derive macros, that allow +/// This examples illustrates the usage of the `WorldQuery` derive macro, which allows /// defining custom query and filter types. /// /// While regular tuple queries work great in most of simple scenarios, using custom queries @@ -16,7 +16,7 @@ use std::{fmt::Debug, marker::PhantomData}; /// - Named structs enable the composition pattern, that makes query types easier to re-use. /// - You can bypass the limit of 15 components that exists for query tuples. /// -/// For more details on the `Fetch` and `FilterFetch` derive macros, see their documentation. +/// For more details on the `WorldQuery` derive macro, see the trait documentation. fn main() { App::new() .add_startup_system(spawn) @@ -47,6 +47,7 @@ struct ComponentD; struct ComponentZ; #[derive(WorldQuery)] +#[world_query(derive(Debug))] struct ReadOnlyCustomQuery<'w, T: Component + Debug, P: Component + Debug> { entity: Entity, a: &'w ComponentA, @@ -55,7 +56,6 @@ struct ReadOnlyCustomQuery<'w, T: Component + Debug, P: Component + Debug> { optional_nested: Option>, optional_tuple: Option<(&'w ComponentB, &'w ComponentZ)>, generic: GenericQuery<'w, T, P>, - #[allow(dead_code)] empty: EmptyQuery<'w>, } @@ -76,27 +76,26 @@ fn print_components_read_only( } // If you are going to mutate the data in a query, you must mark it with the `mutable` attribute. -// The `Fetch` derive macro will still create a read-only version, which will be have `ReadOnly` +// The `WorldQuery` derive macro will still create a read-only version, which will be have `ReadOnly` // suffix. // Note: if you want to use derive macros with read-only query variants, you need to pass them with -// using the `read_only_derive` attribute. -#[derive(WorldQuery, Debug)] -#[world_query(mutable, read_only_derive(Debug))] +// using the `derive` attribute. +#[derive(WorldQuery)] +#[world_query(mutable, derive(Debug))] struct CustomQuery<'w, T: Component + Debug, P: Component + Debug> { entity: Entity, - // `Mut<'w, T>` is a necessary replacement for `&'w mut T` - a: Mut<'w, ComponentA>, - b: Option>, + a: &'w mut ComponentA, + b: Option<&'w mut ComponentB>, nested: NestedQuery<'w>, optional_nested: Option>, - optional_tuple: Option<(NestedQuery<'w>, Mut<'w, ComponentZ>)>, + optional_tuple: Option<(NestedQuery<'w>, &'w mut ComponentZ)>, generic: GenericQuery<'w, T, P>, - #[allow(dead_code)] empty: EmptyQuery<'w>, } // This is a valid query as well, which would iterate over every entity. -#[derive(WorldQuery, Debug)] +#[derive(WorldQuery)] +#[world_query(derive(Debug))] struct EmptyQuery<'w> { // The Fetch derive macro expect a lifetime. As Rust doesn't allow unused lifetimes, we need // to use `PhantomData` as a work around. @@ -104,15 +103,15 @@ struct EmptyQuery<'w> { _w: std::marker::PhantomData<&'w ()>, } -#[derive(WorldQuery, Debug)] -#[allow(dead_code)] +#[derive(WorldQuery)] +#[world_query(derive(Debug))] struct NestedQuery<'w> { c: &'w ComponentC, d: Option<&'w ComponentD>, } -#[derive(WorldQuery, Debug)] -#[allow(dead_code)] +#[derive(WorldQuery)] +#[world_query(derive(Debug))] struct GenericQuery<'w, T: Component, P: Component> { generic: (&'w T, &'w P), } @@ -142,6 +141,8 @@ fn print_components_iter_mut( ) { println!("Print components (iter_mut):"); for e in query.iter_mut() { + // Re-declaring the variable to illustrate the type of the actual iterator item. + let e: CustomQueryItem<'_, _, _> = e; println!("Entity: {:?}", e.entity); println!("A: {:?}", e.a); println!("B: {:?}", e.b); @@ -158,8 +159,8 @@ fn print_components_iter( ) { println!("Print components (iter):"); for e in query.iter() { - // Note that the actual type is different when you iterate over mutable queries with `iter`. - let e: CustomQueryReadOnly<'_, _, _> = e; + // Re-declaring the variable to illustrate the type of the actual iterator item. + let e: CustomQueryReadOnlyItem<'_, _, _> = e; println!("Entity: {:?}", e.entity); println!("A: {:?}", e.a); println!("B: {:?}", e.b); From 27b1155e356a79502730023ceee7b325a4c11071 Mon Sep 17 00:00:00 2001 From: mvlabat Date: Thu, 10 Feb 2022 00:35:40 +0200 Subject: [PATCH 12/14] Dedup the code used for the WorldQuery impls --- crates/bevy_ecs/macros/src/fetch.rs | 724 +++++++++++----------------- crates/bevy_ecs/macros/src/lib.rs | 42 +- examples/ecs/custom_query_param.rs | 2 +- 3 files changed, 282 insertions(+), 486 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index fe0028615ef77..c6cf199695bf1 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -5,19 +5,19 @@ use syn::{ parse::{Parse, ParseStream}, punctuated::Punctuated, Attribute, Data, DataStruct, DeriveInput, Field, Fields, GenericArgument, GenericParam, - ImplGenerics, Lifetime, LifetimeDef, Path, PathArguments, ReturnType, Token, Type, - TypeGenerics, TypePath, Visibility, WhereClause, + Lifetime, LifetimeDef, Path, PathArguments, ReturnType, Token, Type, TypePath, }; use crate::bevy_ecs_path; #[derive(Default)] struct FetchStructAttributes { - pub mutable: bool, + pub is_filter: bool, + pub is_mutable: bool, pub derive_args: Punctuated, } -pub static FILTER_ATTRIBUTE_NAME: &str = "filter"; +static FILTER_ATTRIBUTE_NAME: &str = "filter"; static MUTABLE_ATTRIBUTE_NAME: &str = "mutable"; static DERIVE_ATTRIBUTE_NAME: &str = "derive"; @@ -44,7 +44,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { let ident = meta.path().get_ident(); if ident.map_or(false, |ident| ident == MUTABLE_ATTRIBUTE_NAME) { if let syn::Meta::Path(_) = meta { - fetch_struct_attributes.mutable = true; + fetch_struct_attributes.is_mutable = true; } else { panic!( "The `{}` attribute is expected to have no value or arguments", @@ -62,6 +62,15 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { DERIVE_ATTRIBUTE_NAME ); } + } else if ident.map_or(false, |ident| ident == FILTER_ATTRIBUTE_NAME) { + if let syn::Meta::Path(_) = meta { + fetch_struct_attributes.is_filter = true; + } else { + panic!( + "The `{}` attribute is expected to have no value or arguments", + FILTER_ATTRIBUTE_NAME + ); + } } else { panic!( "Unrecognized attribute: `{}`", @@ -74,38 +83,108 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { .unwrap_or_else(|_| panic!("Invalid `{}` attribute format", WORLD_QUERY_ATTRIBUTE_NAME)); } - let FetchImplTokens { - struct_name, - item_struct_name, - read_only_item_struct_name, - fetch_struct_name, - state_struct_name, - read_only_fetch_struct_name, - fetch_trait_punctuated_lifetimes, - impl_generics, - ty_generics, - where_clause, - has_world_lifetime, - world_lifetime, - state_lifetime, - fetch_lifetime, - ignored_field_attrs, - ignored_field_visibilities, - ignored_field_idents, - ignored_field_types, - field_attrs, - field_visibilities, - field_idents, - field_types: _, - query_types, - fetch_init_types, - } = fetch_impl_tokens( - &ast, - WORLD_QUERY_ATTRIBUTE_NAME, - fetch_struct_attributes.mutable, - ); + if fetch_struct_attributes.is_filter && fetch_struct_attributes.is_mutable { + panic!( + "The `{}` attribute is not expected to be used in conjunction with the `{}` attribute", + FILTER_ATTRIBUTE_NAME, MUTABLE_ATTRIBUTE_NAME + ); + } - if !has_world_lifetime { + let world_lifetime = ast.generics.params.first().and_then(|param| match param { + lt @ GenericParam::Lifetime(_) => Some(lt.clone()), + _ => None, + }); + // Fetch's HRTBs require substituting world lifetime with an additional one to make the + // implementation compile. I don't fully understand why this works though. If anyone's curious + // enough to try to find a better work around, I'll leave playground links here: + // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=da5e260a5c2f3e774142d60a199e854a (this fails) + // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=802517bb3d8f83c45ee8c0be360bb250 (this compiles) + let fetch_lifetime_param = + GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'fetch", Span::call_site()))); + + let has_world_lifetime = world_lifetime.is_some(); + let world_lifetime_param = world_lifetime.unwrap_or_else(|| { + GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'world", Span::call_site()))) + }); + let state_lifetime_param = + GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'state", Span::call_site()))); + + let mut fetch_trait_punctuated_lifetimes = Punctuated::<_, Token![,]>::new(); + fetch_trait_punctuated_lifetimes.push(world_lifetime_param.clone()); + fetch_trait_punctuated_lifetimes.push(state_lifetime_param.clone()); + + let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl(); + + let struct_name = ast.ident.clone(); + let item_struct_name = Ident::new(&format!("{}Item", struct_name), Span::call_site()); + let read_only_item_struct_name = if fetch_struct_attributes.is_mutable { + Ident::new(&format!("{}ReadOnlyItem", struct_name), Span::call_site()) + } else { + item_struct_name.clone() + }; + let fetch_struct_name = Ident::new(&format!("{}Fetch", struct_name), Span::call_site()); + let state_struct_name = Ident::new(&format!("{}State", struct_name), Span::call_site()); + let read_only_fetch_struct_name = if fetch_struct_attributes.is_mutable { + Ident::new(&format!("{}ReadOnlyFetch", struct_name), Span::call_site()) + } else { + fetch_struct_name.clone() + }; + let fetch_associated_type = Ident::new("Fetch", Span::call_site()); + let read_only_fetch_associated_type = Ident::new("ReadOnlyFetch", 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 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 fetch_init_types = Vec::new(); + + let (world_lifetime, fetch_lifetime) = match (&world_lifetime_param, &fetch_lifetime_param) { + (GenericParam::Lifetime(world), GenericParam::Lifetime(fetch)) => { + (&world.lifetime, &fetch.lifetime) + } + _ => unreachable!(), + }; + + for field in fields.iter() { + let WorldQueryFieldInfo { + field_type, + fetch_init_type: init_type, + is_ignored, + attrs, + } = read_world_query_field_info(field, world_lifetime, fetch_lifetime); + + 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()); + field_types.push(field_type); + fetch_init_types.push(init_type); + } + } + + if fetch_struct_attributes.is_filter { + if has_world_lifetime { + panic!("Expected a struct without a lifetime"); + } + } else if !has_world_lifetime { panic!("Expected a struct with a lifetime"); } @@ -118,166 +197,155 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { // Add `'state` and `'fetch` lifetimes that will be used in `Fetch` implementation. let mut fetch_generics = ast.generics.clone(); - fetch_generics.params.insert(1, state_lifetime); - fetch_generics.params.insert(2, fetch_lifetime.clone()); + if !has_world_lifetime { + fetch_generics + .params + .insert(0, world_lifetime_param.clone()); + } + fetch_generics.params.insert(1, state_lifetime_param); + fetch_generics + .params + .insert(2, fetch_lifetime_param.clone()); let (fetch_impl_generics, _, _) = fetch_generics.split_for_impl(); // Replace lifetime `'world` with `'fetch`. See `replace_lifetime_for_type` for more details. let mut fetch_generics = ast.generics.clone(); - *fetch_generics.params.first_mut().unwrap() = fetch_lifetime; - let (_, fetch_ty_generics, _) = fetch_generics.split_for_impl(); + *fetch_generics.params.first_mut().unwrap() = fetch_lifetime_param; - let path = bevy_ecs_path(); + let fetch_ty_generics = if fetch_struct_attributes.is_filter { + ty_generics.clone() + } else { + let (_, fetch_ty_generics, _) = fetch_generics.split_for_impl(); + fetch_ty_generics + }; - let struct_read_only_declaration = if fetch_struct_attributes.mutable { - quote! { - // TODO: it would be great to be able to dedup this by just deriving `WorldQuery` again - // without the `mutable` attribute, but we'd need a way to avoid creating a redundant - // `State` struct. - #derive_macro_call - struct #read_only_item_struct_name #impl_generics #where_clause { - #(#(#field_attrs)* #field_visibilities #field_idents: <<#query_types as #path::query::WorldQuery>::ReadOnlyFetch as #path::query::Fetch<#world_lifetime, #world_lifetime>>::Item,)* - #(#(#ignored_field_attrs)* #ignored_field_visibilities #ignored_field_idents: #ignored_field_types,)* - } + let path = bevy_ecs_path(); - struct #read_only_fetch_struct_name #impl_generics #where_clause { - #(#field_idents: <#query_types as #path::query::WorldQuery>::ReadOnlyFetch,)* - #(#ignored_field_idents: #ignored_field_types,)* - } + let impl_fetch = |is_filter: bool, + fetch_associated_type: Ident, + fetch_struct_name: Ident, + item_struct_name: Ident| { + if is_filter { + quote! { + struct #fetch_struct_name #impl_generics #where_clause { + #(#field_idents: <#field_types as #path::query::WorldQuery>::#fetch_associated_type,)* + #(#ignored_field_idents: #ignored_field_types,)* + } - impl #fetch_impl_generics #path::query::Fetch<#fetch_trait_punctuated_lifetimes> for #read_only_fetch_struct_name #fetch_ty_generics #where_clause { - type Item = #read_only_item_struct_name #ty_generics; - type State = #state_struct_name #fetch_ty_generics; + impl #fetch_impl_generics #path::query::Fetch<#fetch_trait_punctuated_lifetimes> for #fetch_struct_name #ty_generics #where_clause { + type Item = bool; + type State = #state_struct_name #ty_generics; - unsafe fn init(_world: &#path::world::World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self { - #read_only_fetch_struct_name { - #(#field_idents: <#fetch_init_types as #path::query::WorldQuery>::ReadOnlyFetch::init(_world, &state.#field_idents, _last_change_tick, _change_tick),)* - #(#ignored_field_idents: Default::default(),)* + unsafe fn init(_world: &#path::world::World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self { + #fetch_struct_name { + #(#field_idents: <#field_types as #path::query::WorldQuery>::ReadOnlyFetch::init(_world, &state.#field_idents, _last_change_tick, _change_tick),)* + #(#ignored_field_idents: Default::default(),)* + } } - } - const IS_DENSE: bool = true #(&& <#query_types as #path::query::WorldQuery>::ReadOnlyFetch::IS_DENSE)*; + const IS_DENSE: bool = true #(&& <#field_types as #path::query::WorldQuery>::ReadOnlyFetch::IS_DENSE)*; - /// SAFETY: we call `set_archetype` for each member that implements `Fetch` - #[inline] - unsafe fn set_archetype(&mut self, _state: &Self::State, _archetype: &#path::archetype::Archetype, _tables: &#path::storage::Tables) { - #(self.#field_idents.set_archetype(&_state.#field_idents, _archetype, _tables);)* - } + #[inline] + unsafe fn set_archetype(&mut self, _state: &Self::State, _archetype: &#path::archetype::Archetype, _tables: &#path::storage::Tables) { + #(self.#field_idents.set_archetype(&_state.#field_idents, _archetype, _tables);)* + } - /// SAFETY: we call `set_table` for each member that implements `Fetch` - #[inline] - unsafe fn set_table(&mut self, _state: &Self::State, _table: &#path::storage::Table) { - #(self.#field_idents.set_table(&_state.#field_idents, _table);)* - } + #[inline] + unsafe fn set_table(&mut self, _state: &Self::State, _table: &#path::storage::Table) { + #(self.#field_idents.set_table(&_state.#field_idents, _table);)* + } - /// SAFETY: we call `table_fetch` for each member that implements `Fetch`. - #[inline] - unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item { - Self::Item { - #(#field_idents: self.#field_idents.table_fetch(_table_row),)* - #(#ignored_field_idents: Default::default(),)* + #[inline] + unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item { + use #path::query::FilterFetch; + true #(&& self.#field_idents.table_filter_fetch(_table_row))* } - } - /// SAFETY: we call `archetype_fetch` for each member that implements `Fetch`. - #[inline] - unsafe fn archetype_fetch(&mut self, _archetype_index: usize) -> Self::Item { - Self::Item { - #(#field_idents: self.#field_idents.archetype_fetch(_archetype_index),)* - #(#ignored_field_idents: Default::default(),)* + #[inline] + unsafe fn archetype_fetch(&mut self, _archetype_index: usize) -> Self::Item { + use #path::query::FilterFetch; + true #(&& self.#field_idents.archetype_filter_fetch(_archetype_index))* } } } - - impl #impl_generics #path::query::WorldQuery for #read_only_item_struct_name #ty_generics #where_clause { - type Fetch = #read_only_fetch_struct_name #ty_generics; - type State = #state_struct_name #ty_generics; - type ReadOnlyFetch = #read_only_fetch_struct_name #ty_generics; - } - } - } else { - quote! { - // Statically checks that the safety guarantee actually holds true. We need this to make - // sure that we don't compile `ReadOnlyFetch` if our struct contains nested `WorldQuery` - // members that don't implement it. - #[allow(dead_code)] - const _: () = { - fn assert_readonly() {} - - // We generate a readonly assertion for every struct member. - fn assert_all #impl_generics () #where_clause { - #(assert_readonly::<<#query_types as #path::query::WorldQuery>::Fetch>();)* + } else { + quote! { + #derive_macro_call + struct #item_struct_name #impl_generics #where_clause { + #(#(#field_attrs)* #field_visibilities #field_idents: <<#field_types as #path::query::WorldQuery>::#fetch_associated_type as #path::query::Fetch<#world_lifetime, #world_lifetime>>::Item,)* + #(#(#ignored_field_attrs)* #ignored_field_visibilities #ignored_field_idents: #ignored_field_types,)* } - }; - } - }; - - let tokens = TokenStream::from(quote! { - #derive_macro_call - struct #item_struct_name #impl_generics #where_clause { - #(#(#field_attrs)* #field_visibilities #field_idents: <<#query_types as #path::query::WorldQuery>::Fetch as #path::query::Fetch<#world_lifetime, #world_lifetime>>::Item,)* - #(#(#ignored_field_attrs)* #ignored_field_visibilities #ignored_field_idents: #ignored_field_types,)* - } - struct #fetch_struct_name #impl_generics #where_clause { - #(#field_idents: <#query_types as #path::query::WorldQuery>::Fetch,)* - #(#ignored_field_idents: #ignored_field_types,)* - } + struct #fetch_struct_name #impl_generics #where_clause { + #(#field_idents: <#field_types as #path::query::WorldQuery>::#fetch_associated_type,)* + #(#ignored_field_idents: #ignored_field_types,)* + } - struct #state_struct_name #impl_generics #where_clause { - #(#field_idents: <#query_types as #path::query::WorldQuery>::State,)* - #(#ignored_field_idents: #ignored_field_types,)* - } + impl #fetch_impl_generics #path::query::Fetch<#fetch_trait_punctuated_lifetimes> for #fetch_struct_name #fetch_ty_generics #where_clause { + type Item = #item_struct_name #ty_generics; + type State = #state_struct_name #fetch_ty_generics; - impl #fetch_impl_generics #path::query::Fetch<#fetch_trait_punctuated_lifetimes> for #fetch_struct_name #fetch_ty_generics #where_clause { - type Item = #item_struct_name #ty_generics; - type State = #state_struct_name #fetch_ty_generics; + unsafe fn init(_world: &#path::world::World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self { + Self { + #(#field_idents: <#fetch_init_types as #path::query::WorldQuery>::#fetch_associated_type::init(_world, &state.#field_idents, _last_change_tick, _change_tick),)* + #(#ignored_field_idents: Default::default(),)* + } + } - unsafe fn init(_world: &#path::world::World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self { - Self { - #(#field_idents: <#fetch_init_types as #path::query::WorldQuery>::Fetch::init(_world, &state.#field_idents, _last_change_tick, _change_tick),)* - #(#ignored_field_idents: Default::default(),)* - } - } + const IS_DENSE: bool = true #(&& <#field_types as #path::query::WorldQuery>::#fetch_associated_type::IS_DENSE)*; - const IS_DENSE: bool = true #(&& <#query_types as #path::query::WorldQuery>::Fetch::IS_DENSE)*; + /// SAFETY: we call `set_archetype` for each member that implements `Fetch` + #[inline] + unsafe fn set_archetype(&mut self, _state: &Self::State, _archetype: &#path::archetype::Archetype, _tables: &#path::storage::Tables) { + #(self.#field_idents.set_archetype(&_state.#field_idents, _archetype, _tables);)* + } - /// SAFETY: we call `set_archetype` for each member that implements `Fetch` - #[inline] - unsafe fn set_archetype(&mut self, _state: &Self::State, _archetype: &#path::archetype::Archetype, _tables: &#path::storage::Tables) { - #(self.#field_idents.set_archetype(&_state.#field_idents, _archetype, _tables);)* - } + /// SAFETY: we call `set_table` for each member that implements `Fetch` + #[inline] + unsafe fn set_table(&mut self, _state: &Self::State, _table: &#path::storage::Table) { + #(self.#field_idents.set_table(&_state.#field_idents, _table);)* + } - /// SAFETY: we call `set_table` for each member that implements `Fetch` - #[inline] - unsafe fn set_table(&mut self, _state: &Self::State, _table: &#path::storage::Table) { - #(self.#field_idents.set_table(&_state.#field_idents, _table);)* - } + /// SAFETY: we call `table_fetch` for each member that implements `Fetch`. + #[inline] + unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item { + Self::Item { + #(#field_idents: self.#field_idents.table_fetch(_table_row),)* + #(#ignored_field_idents: Default::default(),)* + } + } - /// SAFETY: we call `table_fetch` for each member that implements `Fetch`. - #[inline] - unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item { - Self::Item { - #(#field_idents: self.#field_idents.table_fetch(_table_row),)* - #(#ignored_field_idents: Default::default(),)* + /// SAFETY: we call `archetype_fetch` for each member that implements `Fetch`. + #[inline] + unsafe fn archetype_fetch(&mut self, _archetype_index: usize) -> Self::Item { + Self::Item { + #(#field_idents: self.#field_idents.archetype_fetch(_archetype_index),)* + #(#ignored_field_idents: Default::default(),)* + } + } } } + } + }; - /// SAFETY: we call `archetype_fetch` for each member that implements `Fetch`. - #[inline] - unsafe fn archetype_fetch(&mut self, _archetype_index: usize) -> Self::Item { - Self::Item { - #(#field_idents: self.#field_idents.archetype_fetch(_archetype_index),)* - #(#ignored_field_idents: Default::default(),)* - } - } + let fetch_impl = impl_fetch( + fetch_struct_attributes.is_filter, + fetch_associated_type, + fetch_struct_name.clone(), + item_struct_name, + ); + + let state_impl = quote! { + struct #state_struct_name #impl_generics #where_clause { + #(#field_idents: <#field_types as #path::query::WorldQuery>::State,)* + #(#ignored_field_idents: #ignored_field_types,)* } // SAFETY: `update_component_access` and `update_archetype_component_access` are called for each item in the struct unsafe impl #impl_generics #path::query::FetchState for #state_struct_name #ty_generics #where_clause { fn init(world: &mut #path::world::World) -> Self { #state_struct_name { - #(#field_idents: <<#query_types as #path::query::WorldQuery>::State as #path::query::FetchState>::init(world),)* + #(#field_idents: <<#field_types as #path::query::WorldQuery>::State as #path::query::FetchState>::init(world),)* #(#ignored_field_idents: Default::default(),)* } } @@ -298,8 +366,50 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { true #(&& self.#field_idents.matches_table(_table))* } } + }; + + let read_only_impl = if fetch_struct_attributes.is_filter { + quote! {} + } else if fetch_struct_attributes.is_mutable { + let fetch_impl = impl_fetch( + false, + read_only_fetch_associated_type, + read_only_fetch_struct_name.clone(), + read_only_item_struct_name.clone(), + ); + + quote! { + #fetch_impl + + impl #impl_generics #path::query::WorldQuery for #read_only_item_struct_name #ty_generics #where_clause { + type Fetch = #read_only_fetch_struct_name #ty_generics; + type State = #state_struct_name #ty_generics; + type ReadOnlyFetch = #read_only_fetch_struct_name #ty_generics; + } + } + } else { + quote! { + // Statically checks that the safety guarantee actually holds true. We need this to make + // sure that we don't compile `ReadOnlyFetch` if our struct contains nested `WorldQuery` + // members that don't implement it. + #[allow(dead_code)] + const _: () = { + fn assert_readonly() {} + + // We generate a readonly assertion for every struct member. + fn assert_all #impl_generics () #where_clause { + #(assert_readonly::<<#field_types as #path::query::WorldQuery>::Fetch>();)* + } + }; + } + }; + + let tokens = TokenStream::from(quote! { + #fetch_impl + + #state_impl - #struct_read_only_declaration + #read_only_impl impl #impl_generics #path::query::WorldQuery for #struct_name #ty_generics #where_clause { type Fetch = #fetch_struct_name #ty_generics; @@ -324,286 +434,9 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { tokens } -pub fn derive_world_query_filter_impl(ast: DeriveInput) -> TokenStream { - let FetchImplTokens { - struct_name, - item_struct_name: _, - read_only_item_struct_name: _, - fetch_struct_name, - state_struct_name, - read_only_fetch_struct_name: _, - fetch_trait_punctuated_lifetimes, - impl_generics, - ty_generics, - where_clause, - has_world_lifetime, - world_lifetime, - state_lifetime, - fetch_lifetime, - ignored_field_attrs: _, - ignored_field_visibilities: _, - ignored_field_idents, - ignored_field_types, - field_attrs: _, - field_visibilities: _, - field_idents, - field_types, - query_types: _, - fetch_init_types: _, - } = fetch_impl_tokens(&ast, WORLD_QUERY_ATTRIBUTE_NAME, false); - - if has_world_lifetime { - panic!("Expected a struct without a lifetime"); - } - - // Add `'world`, `'state` and `'fetch` lifetimes that will be used in `Fetch` implementation. - let mut fetch_generics = ast.generics.clone(); - fetch_generics.params.insert(0, world_lifetime); - fetch_generics.params.insert(1, state_lifetime); - fetch_generics.params.insert(2, fetch_lifetime); - let (fetch_impl_generics, _, _) = fetch_generics.split_for_impl(); - - let path = bevy_ecs_path(); - - let tokens = TokenStream::from(quote! { - struct #fetch_struct_name #impl_generics #where_clause { - #(#field_idents: <#field_types as #path::query::WorldQuery>::Fetch,)* - #(#ignored_field_idents: #ignored_field_types,)* - } - - struct #state_struct_name #impl_generics #where_clause { - #(#field_idents: <#field_types as #path::query::WorldQuery>::State,)* - #(#ignored_field_idents: #ignored_field_types,)* - } - - impl #fetch_impl_generics #path::query::Fetch<#fetch_trait_punctuated_lifetimes> for #fetch_struct_name #ty_generics #where_clause { - type Item = bool; - type State = #state_struct_name #ty_generics; - - unsafe fn init(_world: &#path::world::World, state: &Self::State, _last_change_tick: u32, _change_tick: u32) -> Self { - #fetch_struct_name { - #(#field_idents: <#field_types as #path::query::WorldQuery>::ReadOnlyFetch::init(_world, &state.#field_idents, _last_change_tick, _change_tick),)* - #(#ignored_field_idents: Default::default(),)* - } - } - - const IS_DENSE: bool = true #(&& <#field_types as #path::query::WorldQuery>::ReadOnlyFetch::IS_DENSE)*; - - #[inline] - unsafe fn set_archetype(&mut self, _state: &Self::State, _archetype: &#path::archetype::Archetype, _tables: &#path::storage::Tables) { - #(self.#field_idents.set_archetype(&_state.#field_idents, _archetype, _tables);)* - } - - #[inline] - unsafe fn set_table(&mut self, _state: &Self::State, _table: &#path::storage::Table) { - #(self.#field_idents.set_table(&_state.#field_idents, _table);)* - } - - #[inline] - unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item { - use #path::query::FilterFetch; - true #(&& self.#field_idents.table_filter_fetch(_table_row))* - } - - #[inline] - unsafe fn archetype_fetch(&mut self, _archetype_index: usize) -> Self::Item { - use #path::query::FilterFetch; - true #(&& self.#field_idents.archetype_filter_fetch(_archetype_index))* - } - } - - // SAFETY: update_component_access and update_archetype_component_access are called for each item in the struct - unsafe impl #impl_generics #path::query::FetchState for #state_struct_name #ty_generics #where_clause { - fn init(world: &mut #path::world::World) -> Self { - #state_struct_name { - #(#field_idents: <<#field_types as #path::query::WorldQuery>::State as #path::query::FetchState>::init(world),)* - #(#ignored_field_idents: Default::default(),)* - } - } - - fn update_component_access(&self, _access: &mut #path::query::FilteredAccess<#path::component::ComponentId>) { - #(self.#field_idents.update_component_access(_access);)* - } - - fn update_archetype_component_access(&self, _archetype: &#path::archetype::Archetype, _access: &mut #path::query::Access<#path::archetype::ArchetypeComponentId>) { - #(self.#field_idents.update_archetype_component_access(_archetype, _access);)* - } - - fn matches_archetype(&self, _archetype: &#path::archetype::Archetype) -> bool { - true #(&& self.#field_idents.matches_archetype(_archetype))* - } - - fn matches_table(&self, _table: &#path::storage::Table) -> bool { - true #(&& self.#field_idents.matches_table(_table))* - } - } - - impl #impl_generics #path::query::WorldQuery for #struct_name #ty_generics #where_clause { - type Fetch = #fetch_struct_name #ty_generics; - type State = #state_struct_name #ty_generics; - type ReadOnlyFetch = #fetch_struct_name #ty_generics; - } - - /// SAFETY: each item in the struct is read-only as filters never actually fetch any data that could be mutated - unsafe impl #impl_generics #path::query::ReadOnlyFetch for #fetch_struct_name #ty_generics #where_clause {} - }); - tokens -} - -// This struct is used to share common tokens between query and filter implementations. -struct FetchImplTokens<'a> { - struct_name: Ident, - item_struct_name: Ident, - // Equals `item_struct_name` if `has_mutable_attr` is false. - read_only_item_struct_name: Ident, - fetch_struct_name: Ident, - state_struct_name: Ident, - read_only_fetch_struct_name: Ident, - fetch_trait_punctuated_lifetimes: Punctuated, - impl_generics: ImplGenerics<'a>, - ty_generics: TypeGenerics<'a>, - where_clause: Option<&'a WhereClause>, - has_world_lifetime: bool, - world_lifetime: GenericParam, - state_lifetime: GenericParam, - fetch_lifetime: GenericParam, - ignored_field_attrs: Vec>, - ignored_field_visibilities: Vec, - ignored_field_idents: Vec, - ignored_field_types: Vec, - field_attrs: Vec>, - field_visibilities: Vec, - field_idents: Vec, - field_types: Vec, - query_types: Vec, - fetch_init_types: Vec, -} - -fn fetch_impl_tokens<'a>( - ast: &'a DeriveInput, - field_attr_name: &'static str, - has_mutable_attr: bool, -) -> FetchImplTokens<'a> { - let world_lifetime = ast.generics.params.first().and_then(|param| match param { - lt @ GenericParam::Lifetime(_) => Some(lt.clone()), - _ => None, - }); - // Fetch's HRTBs require substituting world lifetime with an additional one to make the - // implementation compile. I don't fully understand why this works though. If anyone's curious - // enough to try to find a better work around, I'll leave playground links here: - // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=da5e260a5c2f3e774142d60a199e854a (this fails) - // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=802517bb3d8f83c45ee8c0be360bb250 (this compiles) - let fetch_lifetime_param = - GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'fetch", Span::call_site()))); - - let has_world_lifetime = world_lifetime.is_some(); - let world_lifetime_param = world_lifetime.unwrap_or_else(|| { - GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'world", Span::call_site()))) - }); - let state_lifetime_param = - GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'state", Span::call_site()))); - - let mut fetch_trait_punctuated_lifetimes = Punctuated::<_, Token![,]>::new(); - fetch_trait_punctuated_lifetimes.push(world_lifetime_param.clone()); - fetch_trait_punctuated_lifetimes.push(state_lifetime_param.clone()); - - let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl(); - - let struct_name = ast.ident.clone(); - let item_struct_name = Ident::new(&format!("{}Item", struct_name), Span::call_site()); - let read_only_item_struct_name = if has_mutable_attr { - Ident::new(&format!("{}ReadOnlyItem", struct_name), Span::call_site()) - } else { - item_struct_name.clone() - }; - let fetch_struct_name = Ident::new(&format!("{}Fetch", struct_name), Span::call_site()); - let state_struct_name = Ident::new(&format!("{}State", struct_name), Span::call_site()); - let read_only_fetch_struct_name = if has_mutable_attr { - Ident::new(&format!("{}ReadOnlyFetch", struct_name), Span::call_site()) - } else { - fetch_struct_name.clone() - }; - - let fields = match &ast.data { - Data::Struct(DataStruct { - fields: Fields::Named(fields), - .. - }) => &fields.named, - _ => 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 query_types = Vec::new(); - let mut fetch_init_types = Vec::new(); - - let (world_lifetime, fetch_lifetime) = match (&world_lifetime_param, &fetch_lifetime_param) { - (GenericParam::Lifetime(world), GenericParam::Lifetime(fetch)) => { - (&world.lifetime, &fetch.lifetime) - } - _ => unreachable!(), - }; - for field in fields.iter() { - let WorldQueryFieldInfo { - query_type, - fetch_init_type: init_type, - is_ignored, - attrs, - } = read_world_query_field_info(field, field_attr_name, world_lifetime, fetch_lifetime); - - 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()); - field_types.push(field.ty.clone()); - query_types.push(query_type); - fetch_init_types.push(init_type); - } - } - - FetchImplTokens { - struct_name, - item_struct_name, - read_only_item_struct_name, - fetch_struct_name, - state_struct_name, - read_only_fetch_struct_name, - fetch_trait_punctuated_lifetimes, - impl_generics, - ty_generics, - where_clause, - has_world_lifetime, - world_lifetime: world_lifetime_param, - state_lifetime: state_lifetime_param, - fetch_lifetime: fetch_lifetime_param, - ignored_field_attrs, - ignored_field_visibilities, - ignored_field_idents, - ignored_field_types, - field_attrs, - field_visibilities, - field_idents, - field_types, - query_types, - fetch_init_types, - } -} - struct WorldQueryFieldInfo { /// The original field type. - query_type: Type, + field_type: Type, /// The same as `query_type` but with `'fetch` lifetime. fetch_init_type: Type, /// Has `#[fetch(ignore)]` or `#[filter_fetch(ignore)]` attribute. @@ -614,7 +447,6 @@ struct WorldQueryFieldInfo { fn read_world_query_field_info( field: &Field, - field_attr_name: &'static str, world_lifetime: &Lifetime, fetch_lifetime: &Lifetime, ) -> WorldQueryFieldInfo { @@ -624,7 +456,7 @@ fn read_world_query_field_info( .find(|attr| { attr.path .get_ident() - .map_or(false, |ident| ident == field_attr_name) + .map_or(false, |ident| ident == WORLD_QUERY_ATTRIBUTE_NAME) }) .map_or(false, |attr| { let mut is_ignored = false; @@ -637,7 +469,9 @@ fn read_world_query_field_info( } Ok(()) }) - .unwrap_or_else(|_| panic!("Invalid `{}` attribute format", field_attr_name)); + .unwrap_or_else(|_| { + panic!("Invalid `{}` attribute format", WORLD_QUERY_ATTRIBUTE_NAME) + }); is_ignored }); @@ -649,17 +483,17 @@ fn read_world_query_field_info( .filter(|attr| { attr.path .get_ident() - .map_or(true, |ident| ident != field_attr_name) + .map_or(true, |ident| ident != WORLD_QUERY_ATTRIBUTE_NAME) }) .collect(); - let query_type: Type = field.ty.clone(); - let mut fetch_init_type: Type = query_type.clone(); + let field_type = field.ty.clone(); + let mut fetch_init_type: Type = field_type.clone(); replace_lifetime_for_type(&mut fetch_init_type, world_lifetime, fetch_lifetime); WorldQueryFieldInfo { - query_type, + field_type, fetch_init_type, is_ignored, attrs, diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index c8996fc4521cb..e4a13e4560754 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -3,10 +3,7 @@ extern crate proc_macro; mod component; mod fetch; -use crate::fetch::{ - derive_world_query_filter_impl, derive_world_query_impl, FILTER_ATTRIBUTE_NAME, - WORLD_QUERY_ATTRIBUTE_NAME, -}; +use crate::fetch::derive_world_query_impl; use bevy_macro_utils::{derive_label, get_named_struct_fields, BevyManifest}; use proc_macro::TokenStream; use proc_macro2::Span; @@ -434,42 +431,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { #[proc_macro_derive(WorldQuery, attributes(world_query))] pub fn derive_world_query(input: TokenStream) -> TokenStream { let ast = parse_macro_input!(input as DeriveInput); - - let mut is_filter = false; - - for attr in &ast.attrs { - if !attr - .path - .get_ident() - .map_or(false, |ident| ident == WORLD_QUERY_ATTRIBUTE_NAME) - { - continue; - } - attr.parse_args_with(|input: ParseStream| { - let meta = input.parse_terminated::(syn::Meta::parse)?; - for meta in meta { - let ident = meta.path().get_ident(); - if ident.map_or(false, |ident| ident == FILTER_ATTRIBUTE_NAME) { - if let syn::Meta::Path(_) = meta { - is_filter = true; - } else { - panic!( - "The `{}` attribute is expected to have no value or arguments", - FILTER_ATTRIBUTE_NAME - ); - } - } - } - Ok(()) - }) - .unwrap_or_else(|_| panic!("Invalid `{}` attribute format", WORLD_QUERY_ATTRIBUTE_NAME)); - } - - if is_filter { - derive_world_query_filter_impl(ast) - } else { - derive_world_query_impl(ast) - } + derive_world_query_impl(ast) } #[proc_macro_derive(SystemLabel)] diff --git a/examples/ecs/custom_query_param.rs b/examples/ecs/custom_query_param.rs index e1fadb78363f8..67a0369ee85af 100644 --- a/examples/ecs/custom_query_param.rs +++ b/examples/ecs/custom_query_param.rs @@ -97,7 +97,7 @@ struct CustomQuery<'w, T: Component + Debug, P: Component + Debug> { #[derive(WorldQuery)] #[world_query(derive(Debug))] struct EmptyQuery<'w> { - // The Fetch derive macro expect a lifetime. As Rust doesn't allow unused lifetimes, we need + // The derive macro expect a lifetime. As Rust doesn't allow unused lifetimes, we need // to use `PhantomData` as a work around. #[world_query(ignore)] _w: std::marker::PhantomData<&'w ()>, From 96f60438a88b417ee955fcb6ff33f5053dd5c4e2 Mon Sep 17 00:00:00 2001 From: Vladyslav Batyrenko Date: Thu, 10 Feb 2022 16:10:18 +0200 Subject: [PATCH 13/14] Apply suggestions from code review Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com> --- crates/bevy_ecs/macros/src/fetch.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index c6cf199695bf1..ddeb301c19974 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -41,8 +41,13 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { attr.parse_args_with(|input: ParseStream| { let meta = input.parse_terminated::(syn::Meta::parse)?; for meta in meta { - let ident = meta.path().get_ident(); - if ident.map_or(false, |ident| ident == MUTABLE_ATTRIBUTE_NAME) { + let ident = meta.path().get_ident().unwrap_or_else(|| { + panic!( + "Unrecognized attribute: `{}`", + meta.path().to_token_stream() + ) + }); + if ident == MUTABLE_ATTRIBUTE_NAME { if let syn::Meta::Path(_) = meta { fetch_struct_attributes.is_mutable = true; } else { @@ -51,7 +56,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { MUTABLE_ATTRIBUTE_NAME ); } - } else if ident.map_or(false, |ident| ident == DERIVE_ATTRIBUTE_NAME) { + } else if ident == DERIVE_ATTRIBUTE_NAME { if let syn::Meta::List(meta_list) = meta { fetch_struct_attributes .derive_args @@ -62,7 +67,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { DERIVE_ATTRIBUTE_NAME ); } - } else if ident.map_or(false, |ident| ident == FILTER_ATTRIBUTE_NAME) { + } else if ident == FILTER_ATTRIBUTE_NAME { if let syn::Meta::Path(_) = meta { fetch_struct_attributes.is_filter = true; } else { @@ -180,6 +185,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { } } + // We expect that only regular query declarations have a lifetime. if fetch_struct_attributes.is_filter { if has_world_lifetime { panic!("Expected a struct without a lifetime"); @@ -479,12 +485,12 @@ fn read_world_query_field_info( let attrs = field .attrs .iter() - .cloned() .filter(|attr| { attr.path .get_ident() .map_or(true, |ident| ident != WORLD_QUERY_ATTRIBUTE_NAME) }) + .cloned() .collect(); let field_type = field.ty.clone(); From 10510f2f91c22b4b30d7857a2f12ea9c303dc34d Mon Sep 17 00:00:00 2001 From: mvlabat Date: Sun, 13 Feb 2022 20:27:07 +0200 Subject: [PATCH 14/14] Update system assertions --- crates/bevy_ecs/src/query/fetch.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 9ef4a7d7f4f7d..3fc1718c0e2e2 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -88,7 +88,7 @@ use std::{ /// } /// } /// -/// # my_system.system(); +/// # bevy_ecs::system::assert_is_system(my_system); /// ``` /// /// ## Mutable queries @@ -141,7 +141,8 @@ use std::{ /// println!("Total (mut): {}", health.total()); /// } /// } -/// # my_system.system(); +/// +/// # bevy_ecs::system::assert_is_system(my_system); /// ``` /// /// **Note:** if you omit the `mutable` attribute for a query that doesn't implement @@ -236,7 +237,7 @@ use std::{ /// } /// } /// -/// # my_system.system(); +/// # bevy_ecs::system::assert_is_system(my_system); /// ``` /// /// ## Ignored fields @@ -260,7 +261,7 @@ use std::{ /// for _ in query.iter() {} /// } /// -/// # my_system.system(); +/// # bevy_ecs::system::assert_is_system(my_system); /// ``` /// /// ## Filters @@ -299,7 +300,7 @@ use std::{ /// for _ in query.iter() {} /// } /// -/// # my_system.system(); +/// # bevy_ecs::system::assert_is_system(my_system); /// ``` pub trait WorldQuery { type Fetch: for<'world, 'state> Fetch<'world, 'state, State = Self::State>;