Skip to content

Commit f0a8994

Browse files
Split WorldQuery into WorldQueryData and WorldQueryFilter (#9918)
# Objective - Fixes #7680 - This is an updated for #8899 which had the same objective but fell a long way behind the latest changes ## Solution The traits `WorldQueryData : WorldQuery` and `WorldQueryFilter : WorldQuery` have been added and some of the types and functions from `WorldQuery` has been moved into them. `ReadOnlyWorldQuery` has been replaced with `ReadOnlyWorldQueryData`. `WorldQueryFilter` is safe (as long as `WorldQuery` is implemented safely). `WorldQueryData` is unsafe - safely implementing it requires that `Self::ReadOnly` is a readonly version of `Self` (this used to be a safety requirement of `WorldQuery`) The type parameters `Q` and `F` of `Query` must now implement `WorldQueryData` and `WorldQueryFilter` respectively. This makes it impossible to accidentally use a filter in the data position or vice versa which was something that could lead to bugs. ~~Compile failure tests have been added to check this.~~ It was previously sometimes useful to use `Option<With<T>>` in the data position. Use `Has<T>` instead in these cases. The `WorldQuery` derive macro has been split into separate derive macros for `WorldQueryData` and `WorldQueryFilter`. Previously it was possible to derive both `WorldQuery` for a struct that had a mixture of data and filter items. This would not work correctly in some cases but could be a useful pattern in others. *This is no longer possible.* --- ## Notes - The changes outside of `bevy_ecs` are all changing type parameters to the new types, updating the macro use, or replacing `Option<With<T>>` with `Has<T>`. - All `WorldQueryData` types always returned `true` for `IS_ARCHETYPAL` so I moved it to `WorldQueryFilter` and replaced all calls to it with `true`. That should be the only logic change outside of the macro generation code. - `Changed<T>` and `Added<T>` were being generated by a macro that I have expanded. Happy to revert that if desired. - The two derive macros share some functions for implementing `WorldQuery` but the tidiest way I could find to implement them was to give them a ton of arguments and ask clippy to ignore that. ## Changelog ### Changed - Split `WorldQuery` into `WorldQueryData` and `WorldQueryFilter` which now have separate derive macros. It is not possible to derive both for the same type. - `Query` now requires that the first type argument implements `WorldQueryData` and the second implements `WorldQueryFilter` ## Migration Guide - Update derives ```rust // old #[derive(WorldQuery)] #[world_query(mutable, derive(Debug))] struct CustomQuery { entity: Entity, a: &'static mut ComponentA } #[derive(WorldQuery)] struct QueryFilter { _c: With<ComponentC> } // new #[derive(WorldQueryData)] #[world_query_data(mutable, derive(Debug))] struct CustomQuery { entity: Entity, a: &'static mut ComponentA, } #[derive(WorldQueryFilter)] struct QueryFilter { _c: With<ComponentC> } ``` - Replace `Option<With<T>>` with `Has<T>` ```rust /// old fn my_system(query: Query<(Entity, Option<With<ComponentA>>)>) { for (entity, has_a_option) in query.iter(){ let has_a:bool = has_a_option.is_some(); //todo!() } } /// new fn my_system(query: Query<(Entity, Has<ComponentA>)>) { for (entity, has_a) in query.iter(){ //todo!() } } ``` - Fix queries which had filters in the data position or vice versa. ```rust // old fn my_system(query: Query<(Entity, With<ComponentA>)>) { for (entity, _) in query.iter(){ //todo!() } } // new fn my_system(query: Query<Entity, With<ComponentA>>) { for entity in query.iter(){ //todo!() } } // old fn my_system(query: Query<AnyOf<(&ComponentA, With<ComponentB>)>>) { for (entity, _) in query.iter(){ //todo!() } } // new fn my_system(query: Query<Option<&ComponentA>, Or<(With<ComponentA>, With<ComponentB>)>>) { for entity in query.iter(){ //todo!() } } ``` --------- Co-authored-by: Alice Cecile <[email protected]>
1 parent 9095810 commit f0a8994

31 files changed

+1879
-1302
lines changed

crates/bevy_core/src/name.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
use bevy_ecs::{
2-
component::Component, entity::Entity, query::WorldQuery, reflect::ReflectComponent,
3-
};
1+
use bevy_ecs::query::WorldQueryData;
2+
use bevy_ecs::{component::Component, entity::Entity, reflect::ReflectComponent};
3+
44
use bevy_reflect::std_traits::ReflectDefault;
55
use bevy_reflect::Reflect;
66
use bevy_utils::AHasher;
@@ -102,7 +102,7 @@ impl std::fmt::Debug for Name {
102102
/// }
103103
/// # bevy_ecs::system::assert_is_system(increment_score);
104104
/// ```
105-
#[derive(WorldQuery)]
105+
#[derive(WorldQueryData)]
106106
pub struct DebugName {
107107
/// A [`Name`] that the entity might have that is displayed if available.
108108
pub name: Option<&'static Name>,

crates/bevy_ecs/macros/src/fetch.rs

-483
This file was deleted.

crates/bevy_ecs/macros/src/lib.rs

+17-6
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
extern crate proc_macro;
22

33
mod component;
4-
mod fetch;
54
mod states;
5+
mod world_query;
6+
mod world_query_data;
7+
mod world_query_filter;
68

7-
use crate::fetch::derive_world_query_impl;
9+
use crate::{
10+
world_query_data::derive_world_query_data_impl,
11+
world_query_filter::derive_world_query_filter_impl,
12+
};
813
use bevy_macro_utils::{derive_label, ensure_no_collision, get_struct_fields, BevyManifest};
914
use proc_macro::TokenStream;
1015
use proc_macro2::Span;
@@ -445,10 +450,16 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
445450
})
446451
}
447452

448-
/// Implement `WorldQuery` to use a struct as a parameter in a query
449-
#[proc_macro_derive(WorldQuery, attributes(world_query))]
450-
pub fn derive_world_query(input: TokenStream) -> TokenStream {
451-
derive_world_query_impl(input)
453+
/// Implement `WorldQueryData` to use a struct as a data parameter in a query
454+
#[proc_macro_derive(WorldQueryData, attributes(world_query_data))]
455+
pub fn derive_world_query_data(input: TokenStream) -> TokenStream {
456+
derive_world_query_data_impl(input)
457+
}
458+
459+
/// Implement `WorldQueryFilter` to use a struct as a filter parameter in a query
460+
#[proc_macro_derive(WorldQueryFilter, attributes(world_query_filter))]
461+
pub fn derive_world_query_filter(input: TokenStream) -> TokenStream {
462+
derive_world_query_filter_impl(input)
452463
}
453464

454465
/// Derive macro generating an impl of the trait `ScheduleLabel`.
+189
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
use proc_macro2::Ident;
2+
use quote::quote;
3+
use syn::{Attribute, Fields, ImplGenerics, TypeGenerics, Visibility, WhereClause};
4+
5+
#[allow(clippy::too_many_arguments)]
6+
pub(crate) fn item_struct(
7+
path: &syn::Path,
8+
fields: &Fields,
9+
derive_macro_call: &proc_macro2::TokenStream,
10+
struct_name: &Ident,
11+
visibility: &Visibility,
12+
item_struct_name: &Ident,
13+
field_types: &Vec<proc_macro2::TokenStream>,
14+
user_impl_generics_with_world: &ImplGenerics,
15+
field_attrs: &Vec<Vec<Attribute>>,
16+
field_visibilities: &Vec<Visibility>,
17+
field_idents: &Vec<proc_macro2::TokenStream>,
18+
user_ty_generics: &TypeGenerics,
19+
user_ty_generics_with_world: &TypeGenerics,
20+
user_where_clauses_with_world: Option<&WhereClause>,
21+
) -> proc_macro2::TokenStream {
22+
let item_attrs = quote!(
23+
#[doc = "Automatically generated [`WorldQuery`] item type for [`"]
24+
#[doc = stringify!(#struct_name)]
25+
#[doc = "`], returned when iterating over query results."]
26+
#[automatically_derived]
27+
);
28+
29+
match fields {
30+
syn::Fields::Named(_) => quote! {
31+
#derive_macro_call
32+
#item_attrs
33+
#visibility struct #item_struct_name #user_impl_generics_with_world #user_where_clauses_with_world {
34+
#(#(#field_attrs)* #field_visibilities #field_idents: <#field_types as #path::query::WorldQuery>::Item<'__w>,)*
35+
}
36+
},
37+
syn::Fields::Unnamed(_) => quote! {
38+
#derive_macro_call
39+
#item_attrs
40+
#[automatically_derived]
41+
#visibility struct #item_struct_name #user_impl_generics_with_world #user_where_clauses_with_world(
42+
#( #field_visibilities <#field_types as #path::query::WorldQuery>::Item<'__w>, )*
43+
);
44+
},
45+
syn::Fields::Unit => quote! {
46+
#item_attrs
47+
#visibility type #item_struct_name #user_ty_generics_with_world = #struct_name #user_ty_generics;
48+
},
49+
}
50+
}
51+
52+
#[allow(clippy::too_many_arguments)]
53+
pub(crate) fn world_query_impl(
54+
path: &syn::Path,
55+
struct_name: &Ident,
56+
visibility: &Visibility,
57+
item_struct_name: &Ident,
58+
fetch_struct_name: &Ident,
59+
field_types: &Vec<proc_macro2::TokenStream>,
60+
user_impl_generics: &ImplGenerics,
61+
user_impl_generics_with_world: &ImplGenerics,
62+
field_idents: &Vec<proc_macro2::TokenStream>,
63+
user_ty_generics: &TypeGenerics,
64+
user_ty_generics_with_world: &TypeGenerics,
65+
named_field_idents: &Vec<Ident>,
66+
marker_name: &Ident,
67+
state_struct_name: &Ident,
68+
user_where_clauses: Option<&WhereClause>,
69+
user_where_clauses_with_world: Option<&WhereClause>,
70+
) -> proc_macro2::TokenStream {
71+
quote! {
72+
#[doc(hidden)]
73+
#[doc = "Automatically generated internal [`WorldQuery`] fetch type for [`"]
74+
#[doc = stringify!(#struct_name)]
75+
#[doc = "`], used to define the world data accessed by this query."]
76+
#[automatically_derived]
77+
#visibility struct #fetch_struct_name #user_impl_generics_with_world #user_where_clauses_with_world {
78+
#(#named_field_idents: <#field_types as #path::query::WorldQuery>::Fetch<'__w>,)*
79+
#marker_name: &'__w (),
80+
}
81+
82+
impl #user_impl_generics_with_world Clone for #fetch_struct_name #user_ty_generics_with_world
83+
#user_where_clauses_with_world {
84+
fn clone(&self) -> Self {
85+
Self {
86+
#(#named_field_idents: self.#named_field_idents.clone(),)*
87+
#marker_name: &(),
88+
}
89+
}
90+
}
91+
92+
// SAFETY: `update_component_access` and `update_archetype_component_access` are called on every field
93+
unsafe impl #user_impl_generics #path::query::WorldQuery
94+
for #struct_name #user_ty_generics #user_where_clauses {
95+
96+
type Item<'__w> = #item_struct_name #user_ty_generics_with_world;
97+
type Fetch<'__w> = #fetch_struct_name #user_ty_generics_with_world;
98+
type State = #state_struct_name #user_ty_generics;
99+
100+
fn shrink<'__wlong: '__wshort, '__wshort>(
101+
item: <#struct_name #user_ty_generics as #path::query::WorldQuery>::Item<'__wlong>
102+
) -> <#struct_name #user_ty_generics as #path::query::WorldQuery>::Item<'__wshort> {
103+
#item_struct_name {
104+
#(
105+
#field_idents: <#field_types>::shrink(item.#field_idents),
106+
)*
107+
}
108+
}
109+
110+
unsafe fn init_fetch<'__w>(
111+
_world: #path::world::unsafe_world_cell::UnsafeWorldCell<'__w>,
112+
state: &Self::State,
113+
_last_run: #path::component::Tick,
114+
_this_run: #path::component::Tick,
115+
) -> <Self as #path::query::WorldQuery>::Fetch<'__w> {
116+
#fetch_struct_name {
117+
#(#named_field_idents:
118+
<#field_types>::init_fetch(
119+
_world,
120+
&state.#named_field_idents,
121+
_last_run,
122+
_this_run,
123+
),
124+
)*
125+
#marker_name: &(),
126+
}
127+
}
128+
129+
const IS_DENSE: bool = true #(&& <#field_types>::IS_DENSE)*;
130+
131+
/// SAFETY: we call `set_archetype` for each member that implements `Fetch`
132+
#[inline]
133+
unsafe fn set_archetype<'__w>(
134+
_fetch: &mut <Self as #path::query::WorldQuery>::Fetch<'__w>,
135+
_state: &Self::State,
136+
_archetype: &'__w #path::archetype::Archetype,
137+
_table: &'__w #path::storage::Table
138+
) {
139+
#(<#field_types>::set_archetype(&mut _fetch.#named_field_idents, &_state.#named_field_idents, _archetype, _table);)*
140+
}
141+
142+
/// SAFETY: we call `set_table` for each member that implements `Fetch`
143+
#[inline]
144+
unsafe fn set_table<'__w>(
145+
_fetch: &mut <Self as #path::query::WorldQuery>::Fetch<'__w>,
146+
_state: &Self::State,
147+
_table: &'__w #path::storage::Table
148+
) {
149+
#(<#field_types>::set_table(&mut _fetch.#named_field_idents, &_state.#named_field_idents, _table);)*
150+
}
151+
152+
/// SAFETY: we call `fetch` for each member that implements `Fetch`.
153+
#[inline(always)]
154+
unsafe fn fetch<'__w>(
155+
_fetch: &mut <Self as #path::query::WorldQuery>::Fetch<'__w>,
156+
_entity: #path::entity::Entity,
157+
_table_row: #path::storage::TableRow,
158+
) -> <Self as #path::query::WorldQuery>::Item<'__w> {
159+
Self::Item {
160+
#(#field_idents: <#field_types>::fetch(&mut _fetch.#named_field_idents, _entity, _table_row),)*
161+
}
162+
}
163+
164+
fn update_component_access(state: &Self::State, _access: &mut #path::query::FilteredAccess<#path::component::ComponentId>) {
165+
#( <#field_types>::update_component_access(&state.#named_field_idents, _access); )*
166+
}
167+
168+
fn update_archetype_component_access(
169+
state: &Self::State,
170+
_archetype: &#path::archetype::Archetype,
171+
_access: &mut #path::query::Access<#path::archetype::ArchetypeComponentId>
172+
) {
173+
#(
174+
<#field_types>::update_archetype_component_access(&state.#named_field_idents, _archetype, _access);
175+
)*
176+
}
177+
178+
fn init_state(world: &mut #path::world::World) -> #state_struct_name #user_ty_generics {
179+
#state_struct_name {
180+
#(#named_field_idents: <#field_types>::init_state(world),)*
181+
}
182+
}
183+
184+
fn matches_component_set(state: &Self::State, _set_contains_id: &impl Fn(#path::component::ComponentId) -> bool) -> bool {
185+
true #(&& <#field_types>::matches_component_set(&state.#named_field_idents, _set_contains_id))*
186+
}
187+
}
188+
}
189+
}

0 commit comments

Comments
 (0)