From af2582663fcbd047c037567137a56e941c001287 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Tue, 23 Jan 2024 22:30:02 -0800 Subject: [PATCH 01/20] create a query join --- crates/bevy_ecs/src/query/state.rs | 96 +++++++++++++++++++++++++++++- 1 file changed, 95 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index ecd544954aa38..966389dee43e2 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -375,7 +375,11 @@ impl QueryState { NewF::update_component_access(&filter_state, &mut filter_component_access); component_access.extend(&filter_component_access); - assert!(component_access.is_subset(&self.component_access), "Transmuted state for {} attempts to access terms that are not allowed by original state {}.", std::any::type_name::<(NewD, NewF)>(), std::any::type_name::<(D, F)>() ); + assert!( + component_access.is_subset(&self.component_access), + "Transmuted state for {} attempts to access terms that are not allowed by original state {}.", + std::any::type_name::<(NewD, NewF)>(), std::any::type_name::<(D, F)>() + ); QueryState { world_id: self.world_id, @@ -397,6 +401,80 @@ impl QueryState { } } + + /// TODO: add docs + pub fn join(&self, world: &World, other: &QueryState) -> QueryState { + self.join_filtered::<_, (), NewD, ()>(world, other) + } + + + /// This takes the intersection of matches between two queries and creates a new query state. Similar to [`transmute`] you can change the terms. + /// TODO: extend this with more info + pub fn join_filtered< + OtherD: QueryData, + OtherF: QueryFilter, + NewD: QueryData, + NewF: QueryFilter, + >( + &self, + world: &World, + other: &QueryState, + ) -> QueryState { + let mut component_access = FilteredAccess::default(); + let mut new_fetch_state = NewD::get_state(world) + .expect("Could not create fetch_state, Please initialize all referenced components before transmuting."); + let new_filter_state = NewF::get_state(world) + .expect("Could not create filter_state, Please initialize all referenced components before transmuting."); + + NewD::set_access(&mut new_fetch_state, &self.component_access); + NewD::update_component_access(&new_fetch_state, &mut component_access); + + let mut new_filter_component_access = FilteredAccess::default(); + NewF::update_component_access(&new_filter_state, &mut new_filter_component_access); + + let mut joined_component_access = self.component_access.clone(); + joined_component_access.extend(&other.component_access); + + assert!( + component_access.is_subset(&joined_component_access), + "Transmuted state for {} attempts to access terms that are not allowed by original state {}.", + std::any::type_name::<(NewD, NewF)>(), std::any::type_name::<(D, F)>() + ); + + // take the intersection of the matched ids + let matched_table_ids: Vec<_> = self + .matched_table_ids + .iter() + .filter(|table_id| other.matched_table_ids.contains(table_id)) + .cloned() + .collect(); + let matched_archetype_ids: Vec<_> = self + .matched_archetype_ids + .iter() + .filter(|table_id| other.matched_archetype_ids.contains(table_id)) + .cloned() + .collect(); + + QueryState { + world_id: self.world_id, + archetype_generation: self.archetype_generation, + matched_table_ids, + matched_archetype_ids, + fetch_state: new_fetch_state, + filter_state: new_filter_state, + component_access: self.component_access.clone(), + matched_tables: self.matched_tables.clone(), + matched_archetypes: self.matched_archetypes.clone(), + archetype_component_access: self.archetype_component_access.clone(), + #[cfg(feature = "trace")] + par_iter_span: bevy_utils::tracing::info_span!( + "par_for_each", + query = std::any::type_name::(), + filter = std::any::type_name::(), + ), + } + } + /// Gets the query result for the given [`World`] and [`Entity`]. /// /// This can only be called for read-only queries, see [`Self::get_mut`] for write-queries. @@ -1797,4 +1875,20 @@ mod tests { assert_eq!(entity_a, detection_query.single(&world)); } + + #[test] + fn join() { + let mut world = World::new(); + world.spawn(A(0)); + world.spawn(B(1)); + let entity_ab = world.spawn((A(2), B(3))).id(); + + let query_1 = QueryState::<&A>::new(&mut world); + let query_2 = QueryState::<&B>::new(&mut world); + let mut new_query: QueryState = query_1.join(&world, &query_2); + + assert_eq!(new_query.single(&world), entity_ab); + } + + // TODO: add more tests for joins } From 2ed34be464e89ed39d3914e739fa7d27585577fd Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Wed, 24 Jan 2024 20:09:28 -0800 Subject: [PATCH 02/20] add methods to query --- crates/bevy_ecs/src/system/query.rs | 30 +++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index f13b38024451c..09790fad3bb44 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1574,6 +1574,36 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub fn as_query_lens(&mut self) -> QueryLens<'_, D> { self.transmute_lens() } + + pub fn join( + &mut self, + other: &Query, + ) -> QueryLens<'_, NewD> { + self.join_filtered(other) + } + + pub fn join_filtered< + OtherD: QueryData, + OtherF: QueryFilter, + NewD: QueryData, + NewF: QueryFilter, + >( + &mut self, + other: &Query, + ) -> QueryLens<'_, NewD, NewF> { + // SAFETY: There are no other active borrows of data from world + let world = unsafe { self.world.world() }; + let state = self + .state + .join_filtered::(world, &other.state); + QueryLens { + world: self.world, + state, + last_run: self.last_run, + this_run: self.this_run, + force_read_only_component_access: self.force_read_only_component_access, + } + } } impl<'w, 's, D: QueryData, F: QueryFilter> IntoIterator for &'w Query<'_, 's, D, F> { From 3807e4d2b1fb4eff2235c6013cc8b0e5ef57f53b Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Thu, 25 Jan 2024 21:40:47 -0800 Subject: [PATCH 03/20] check join filtered instead of join --- crates/bevy_ecs/src/query/state.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 966389dee43e2..6422b6639ecba 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -401,13 +401,15 @@ impl QueryState { } } - /// TODO: add docs - pub fn join(&self, world: &World, other: &QueryState) -> QueryState { + pub fn join( + &self, + world: &World, + other: &QueryState, + ) -> QueryState { self.join_filtered::<_, (), NewD, ()>(world, other) } - /// This takes the intersection of matches between two queries and creates a new query state. Similar to [`transmute`] you can change the terms. /// TODO: extend this with more info pub fn join_filtered< @@ -1882,13 +1884,12 @@ mod tests { world.spawn(A(0)); world.spawn(B(1)); let entity_ab = world.spawn((A(2), B(3))).id(); + world.spawn((A(4), B(5), C(6))); - let query_1 = QueryState::<&A>::new(&mut world); - let query_2 = QueryState::<&B>::new(&mut world); - let mut new_query: QueryState = query_1.join(&world, &query_2); + let query_1 = QueryState::<&A, Without>::new(&mut world); + let query_2 = QueryState::<&B, Without>::new(&mut world); + let mut new_query: QueryState = query_1.join_filtered(&world, &query_2); assert_eq!(new_query.single(&world), entity_ab); } - - // TODO: add more tests for joins } From 5572230eaec28ac32873be0ee9c667863a870978 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Thu, 25 Jan 2024 22:18:02 -0800 Subject: [PATCH 04/20] add docs --- crates/bevy_ecs/src/query/state.rs | 22 +++++++++++++++++++--- crates/bevy_ecs/src/system/query.rs | 22 ++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 6422b6639ecba..716e04f5035cf 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -401,7 +401,19 @@ impl QueryState { } } - /// TODO: add docs + /// Use this to combine two queries. The data accessed will be the intersection + /// of archetypes included in both queries. This can be useful for accessing a + /// subset of the entities between two queries. + /// + /// You should not call `update_archetypes` on the returned `QueryState` as the result + /// could be unpredictable. You might end up with a mix of archetypes that only matched + /// the original query + archetypes that only match the new QueryState. Most of the + /// safe methods on QueryState call QueryState::update_archetypes internally, so + /// this best used through a Query. + /// + /// ## Panics + /// + /// Will panic if `NewD` contains accesses not in `Q` or `OtherQ`. pub fn join( &self, world: &World, @@ -410,8 +422,12 @@ impl QueryState { self.join_filtered::<_, (), NewD, ()>(world, other) } - /// This takes the intersection of matches between two queries and creates a new query state. Similar to [`transmute`] you can change the terms. - /// TODO: extend this with more info + /// Use this to combine two queries. The data accessed will be the intersection + /// of archetypes included in both queries. + /// + /// ## Panics + /// + /// Will panic if `NewD` or `NewF` requires accesses not in `Q` or `OtherQ`. pub fn join_filtered< OtherD: QueryData, OtherF: QueryFilter, diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 09790fad3bb44..8711c915bbaac 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1575,6 +1575,21 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { self.transmute_lens() } + /// Returns a QueryLens that can be used to get a query with the combined fetch. + /// + /// For example, this can take a `Query<&A>` and a `Queryy<&B>` and return a `Query<&A, &B>`. + /// The returned query will only return items with both `A` and `B`. Note that since filter + /// are dropped, non-archetypal filters like `Added` and `Changed` will no be respected. + /// To maintain or change filter terms see `Self::join_filtered`. + /// + /// ## Panics + /// + /// This will panic if `NewD` is not a subset of the union of the original fetch `Q` and `OtherD`. + /// + /// ## Allowed Transmutes + /// + /// Like `transmute_lens` the query terms can be changed with some restrictions. + /// See [`transmute_lens`] for more details. pub fn join( &mut self, other: &Query, @@ -1582,6 +1597,13 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { self.join_filtered(other) } + /// Equivalent to [`join`] but also includes a `QueryFilter` type. + /// + /// Note that the lens with iterate a subset of the original queries tables + /// and arcchetypes. This means that additional archetypal query terms like + /// `With` and `Without` will not necessarily be respected and non-archetypal + /// terms like `Added` and `Changed` will only be respected if they are in + /// the type signature. pub fn join_filtered< OtherD: QueryData, OtherF: QueryFilter, From 8d2d15b3127a50ab1e42b41ede879f7c67a63b9e Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Fri, 26 Jan 2024 22:10:52 -0800 Subject: [PATCH 05/20] fix docs --- crates/bevy_ecs/src/system/query.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 8711c915bbaac..c6eb9f7350408 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1589,7 +1589,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// ## Allowed Transmutes /// /// Like `transmute_lens` the query terms can be changed with some restrictions. - /// See [`transmute_lens`] for more details. + /// See [`Self::transmute_lens`] for more details. pub fn join( &mut self, other: &Query, @@ -1597,7 +1597,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { self.join_filtered(other) } - /// Equivalent to [`join`] but also includes a `QueryFilter` type. + /// Equivalent to [`Self::join`] but also includes a `QueryFilter` type. /// /// Note that the lens with iterate a subset of the original queries tables /// and arcchetypes. This means that additional archetypal query terms like From b8e927048f0100668a49247bafffd9e42e6323d7 Mon Sep 17 00:00:00 2001 From: Mike Date: Sat, 27 Jan 2024 13:28:06 -0800 Subject: [PATCH 06/20] doc cleanup Co-authored-by: Alice Cecile --- crates/bevy_ecs/src/system/query.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index c6eb9f7350408..7d61c875e38fe 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1575,7 +1575,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { self.transmute_lens() } - /// Returns a QueryLens that can be used to get a query with the combined fetch. + /// Returns a [`QueryLens`] that can be used to get a query with the combined fetch. /// /// For example, this can take a `Query<&A>` and a `Queryy<&B>` and return a `Query<&A, &B>`. /// The returned query will only return items with both `A` and `B`. Note that since filter @@ -1597,10 +1597,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { self.join_filtered(other) } - /// Equivalent to [`Self::join`] but also includes a `QueryFilter` type. + /// Equivalent to [`Self::join`] but also includes a [`QueryFilter`] type. /// /// Note that the lens with iterate a subset of the original queries tables - /// and arcchetypes. This means that additional archetypal query terms like + /// and archetypes. This means that additional archetypal query terms like /// `With` and `Without` will not necessarily be respected and non-archetypal /// terms like `Added` and `Changed` will only be respected if they are in /// the type signature. From 170c9ee9029bd5c030b214c459ac4bf800dbf222 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Sat, 27 Jan 2024 15:45:28 -0800 Subject: [PATCH 07/20] clippy fixes --- crates/bevy_ecs/src/query/state.rs | 6 +++--- crates/bevy_ecs/src/system/query.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 716e04f5035cf..ca5f880d2a6c6 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -407,9 +407,9 @@ impl QueryState { /// /// You should not call `update_archetypes` on the returned `QueryState` as the result /// could be unpredictable. You might end up with a mix of archetypes that only matched - /// the original query + archetypes that only match the new QueryState. Most of the - /// safe methods on QueryState call QueryState::update_archetypes internally, so - /// this best used through a Query. + /// the original query + archetypes that only match the new `QueryState`. Most of the + /// safe methods on `QueryState` call [`QueryState::update_archetypes`] internally, so + /// this best used through a `Query`. /// /// ## Panics /// diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 7d61c875e38fe..3406f7a9c1c53 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1617,7 +1617,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { let world = unsafe { self.world.world() }; let state = self .state - .join_filtered::(world, &other.state); + .join_filtered::(world, other.state); QueryLens { world: self.world, state, From 1e4522e1622561d4a7e5e9f8ad6d20c6f54dc452 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Sun, 28 Jan 2024 21:08:11 -0800 Subject: [PATCH 08/20] add some tests for panics --- crates/bevy_ecs/src/query/state.rs | 39 ++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index ca5f880d2a6c6..8bc894ba3a3a8 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -450,6 +450,8 @@ impl QueryState { let mut new_filter_component_access = FilteredAccess::default(); NewF::update_component_access(&new_filter_state, &mut new_filter_component_access); + component_access.extend(&new_filter_component_access); + let mut joined_component_access = self.component_access.clone(); joined_component_access.extend(&other.component_access); @@ -1894,6 +1896,19 @@ mod tests { assert_eq!(entity_a, detection_query.single(&world)); } + #[test] + #[should_panic( + expected = "Transmuted state for (bevy_ecs::entity::Entity, bevy_ecs::query::filter::Changed) attempts to access terms that are not allowed by original state (&bevy_ecs::query::state::tests::A, ())." + )] + fn cannot_transmute_changed_without_access() { + let mut world = World::new(); + world.init_component::(); + world.init_component::(); + let query = QueryState::<&A>::new(&mut world); + let _new_query = query.transmute_filtered::>(&world); + + } + #[test] fn join() { let mut world = World::new(); @@ -1908,4 +1923,28 @@ mod tests { assert_eq!(new_query.single(&world), entity_ab); } + + #[test] + #[should_panic( + expected = "Transmuted state for (&bevy_ecs::query::state::tests::C, ()) attempts to access terms that are not allowed by original state (&bevy_ecs::query::state::tests::A, ())." + )] + fn cannot_join_wrong_fetch() { + let mut world = World::new(); + world.init_component::(); + let query_1 = QueryState::<&A>::new(&mut world); + let query_2 = QueryState::<&B>::new(&mut world); + let _query: QueryState<&C> = query_1.join(&world, &query_2); + } + + #[test] + #[should_panic( + expected = "Transmuted state for (bevy_ecs::entity::Entity, bevy_ecs::query::filter::Changed) attempts to access terms that are not allowed by original state (&bevy_ecs::query::state::tests::A, bevy_ecs::query::filter::Without)." + )] + fn cannot_join_wrong_filter() { + let mut world = World::new(); + let query_1 = QueryState::<&A, Without>::new(&mut world); + let query_2 = QueryState::<&B, Without>::new(&mut world); + let _: QueryState> = query_1.join_filtered(&world, &query_2); + + } } From a947c04c8785b1dea6d6c2793d450c40da90d227 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Sun, 28 Jan 2024 21:29:37 -0800 Subject: [PATCH 09/20] improve safety comments --- crates/bevy_ecs/src/system/query.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 3406f7a9c1c53..d8e032663a294 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1558,7 +1558,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub fn transmute_lens_filtered( &mut self, ) -> QueryLens<'_, NewD, NewF> { - // SAFETY: There are no other active borrows of data from world + // SAFETY: + // - We have exclusive access to the query + // - `self` has correctly captured it's access + // - Access is checked to be a subset of the query's access when the state is created. let world = unsafe { self.world.world() }; let state = self.state.transmute_filtered::(world); QueryLens { @@ -1592,7 +1595,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// See [`Self::transmute_lens`] for more details. pub fn join( &mut self, - other: &Query, + other: &mut Query, ) -> QueryLens<'_, NewD> { self.join_filtered(other) } @@ -1611,9 +1614,12 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { NewF: QueryFilter, >( &mut self, - other: &Query, + other: &mut Query, ) -> QueryLens<'_, NewD, NewF> { - // SAFETY: There are no other active borrows of data from world + // SAFETY: + // - The queries have correctly captured their access. + // - We have exclusive access to both queries. + // - Access for QueryLens is checked when state is created. let world = unsafe { self.world.world() }; let state = self .state From 78aa75e638ed84089743955be6702819265d9a02 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Sun, 28 Jan 2024 21:40:53 -0800 Subject: [PATCH 10/20] cargo fmt --- crates/bevy_ecs/src/query/state.rs | 4 +--- crates/bevy_ecs/src/system/query.rs | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 8bc894ba3a3a8..213bd828e19f1 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -450,7 +450,7 @@ impl QueryState { let mut new_filter_component_access = FilteredAccess::default(); NewF::update_component_access(&new_filter_state, &mut new_filter_component_access); - component_access.extend(&new_filter_component_access); + component_access.extend(&new_filter_component_access); let mut joined_component_access = self.component_access.clone(); joined_component_access.extend(&other.component_access); @@ -1906,7 +1906,6 @@ mod tests { world.init_component::(); let query = QueryState::<&A>::new(&mut world); let _new_query = query.transmute_filtered::>(&world); - } #[test] @@ -1945,6 +1944,5 @@ mod tests { let query_1 = QueryState::<&A, Without>::new(&mut world); let query_2 = QueryState::<&B, Without>::new(&mut world); let _: QueryState> = query_1.join_filtered(&world, &query_2); - } } diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index d8e032663a294..9b2f89f991ffa 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1616,7 +1616,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { &mut self, other: &mut Query, ) -> QueryLens<'_, NewD, NewF> { - // SAFETY: + // SAFETY: // - The queries have correctly captured their access. // - We have exclusive access to both queries. // - Access for QueryLens is checked when state is created. From 1981272e87870c4523194c305cc688072e1e644b Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Sun, 11 Feb 2024 13:42:58 -0800 Subject: [PATCH 11/20] add an example --- crates/bevy_ecs/src/system/query.rs | 39 +++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 9b2f89f991ffa..f529c09c4fd37 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1585,6 +1585,45 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// are dropped, non-archetypal filters like `Added` and `Changed` will no be respected. /// To maintain or change filter terms see `Self::join_filtered`. /// + /// ## Example + /// + /// ```rust + /// # use bevy_ecs::prelude::*; + /// # use bevy_ecs::system::QueryLens; + /// # + /// # #[derive(Component)] + /// # struct Transform; + /// # + /// # #[derive(Component)] + /// # struct Player; + /// # + /// # #[derive(Component)] + /// # struct Enemy; + /// # + /// # let mut world = World::default(); + /// # world.spawn((Transform, Player)); + /// # world.spawn((Transform, Enemy)); + /// + /// fn system( + /// mut transforms: Query<&Transform>, + /// mut players: Query<&Player>, + /// mut enemies: Query<&Enemy> + /// ) { + /// let mut players_transforms: QueryLens<(&Transform, &Player)> = transforms.join(&mut players); + /// for (transform, player) in &players_transforms.query() { + /// // do something with a and b + /// } + /// + /// let mut enemies_transforms: QueryLens<(&Transform, &Enemy)> = transforms.join(&mut enemies); + /// for (transform, enemy) in &enemies_transforms.query() { + /// // do something with a and b + /// } + /// } + /// + /// # let mut schedule = Schedule::default(); + /// # schedule.add_systems(system); + /// # schedule.run(&mut world); + /// ``` /// ## Panics /// /// This will panic if `NewD` is not a subset of the union of the original fetch `Q` and `OtherD`. From 9013d84fc8261bfde4696b6ea94fa9298433ebf5 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Fri, 1 Mar 2024 23:20:26 -0800 Subject: [PATCH 12/20] add a failing test --- crates/bevy_ecs/src/query/state.rs | 17 +++++++++++++++++ crates/bevy_ecs/src/system/query.rs | 1 - 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index e189e7c499dab..907f4e07a31ce 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -1760,6 +1760,23 @@ mod tests { assert_eq!(new_query.single(&world), entity_ab); } + #[test] + fn join_with_get() { + let mut world = World::new(); + world.spawn(A(0)); + world.spawn(B(1)); + let entity_ab = world.spawn((A(2), B(3))).id(); + let entity_abc = world.spawn((A(4), B(5), C(6))).id(); + + let query_1 = QueryState::<&A>::new(&mut world); + let query_2 = QueryState::<&B, Without>::new(&mut world); + let mut new_query: QueryState = query_1.join_filtered(&world, &query_2); + + assert!(new_query.get(&world, entity_ab).is_ok()); + // should not be able to get entity with c. + assert!(new_query.get(&world, entity_abc).is_err()); + } + #[test] #[should_panic( expected = "Transmuted state for (&bevy_ecs::query::state::tests::C, ()) attempts to access terms that are not allowed by original state (&bevy_ecs::query::state::tests::A, ())." diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 80d17255a534c..971ebe308461b 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1420,7 +1420,6 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { state, last_run: self.last_run, this_run: self.this_run, - force_read_only_component_access: self.force_read_only_component_access, } } } From a90e9188e232424a23cb7242e9aa1c77f65479e0 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Fri, 1 Mar 2024 23:24:23 -0800 Subject: [PATCH 13/20] fix new test --- crates/bevy_ecs/src/query/state.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 907f4e07a31ce..f517f7a951568 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -456,11 +456,19 @@ impl QueryState { assert!( component_access.is_subset(&joined_component_access), - "Transmuted state for {} attempts to access terms that are not allowed by original state {}.", - std::any::type_name::<(NewD, NewF)>(), std::any::type_name::<(D, F)>() + "Joined state for {} attempts to access terms that are not allowed by state {} joined with {}.", + std::any::type_name::<(NewD, NewF)>(), std::any::type_name::<(D, F)>(), std::any::type_name::<(OtherD, OtherF)>() ); // take the intersection of the matched ids + let matched_tables: FixedBitSet = self + .matched_tables + .intersection(&other.matched_tables) + .collect(); + let matched_archetypes: FixedBitSet = self + .matched_archetypes + .intersection(&other.matched_archetypes) + .collect(); let matched_table_ids: Vec<_> = self .matched_table_ids .iter() @@ -481,9 +489,9 @@ impl QueryState { matched_archetype_ids, fetch_state: new_fetch_state, filter_state: new_filter_state, - component_access: self.component_access.clone(), - matched_tables: self.matched_tables.clone(), - matched_archetypes: self.matched_archetypes.clone(), + component_access: joined_component_access, + matched_tables, + matched_archetypes, archetype_component_access: self.archetype_component_access.clone(), #[cfg(feature = "trace")] par_iter_span: bevy_utils::tracing::info_span!( From d4c9939b018331db451ec568ceed01ed006ca272 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Sat, 2 Mar 2024 15:55:50 -0800 Subject: [PATCH 14/20] use bitsets to generate all the ids. --- crates/bevy_ecs/src/query/state.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index f517f7a951568..3c6815cee6472 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -465,21 +465,17 @@ impl QueryState { .matched_tables .intersection(&other.matched_tables) .collect(); + let matched_table_ids: Vec = matched_tables + .ones() + .map(|id| TableId::from_usize(id)) + .collect(); let matched_archetypes: FixedBitSet = self .matched_archetypes .intersection(&other.matched_archetypes) .collect(); - let matched_table_ids: Vec<_> = self - .matched_table_ids - .iter() - .filter(|table_id| other.matched_table_ids.contains(table_id)) - .cloned() - .collect(); - let matched_archetype_ids: Vec<_> = self - .matched_archetype_ids - .iter() - .filter(|table_id| other.matched_archetype_ids.contains(table_id)) - .cloned() + let matched_archetype_ids: Vec = matched_archetypes + .ones() + .map(|id| ArchetypeId::new(id)) .collect(); QueryState { From f6f91645bc80a39de971d676c07af9a9808cb696 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Sat, 2 Mar 2024 16:25:21 -0800 Subject: [PATCH 15/20] update panic messages in tests --- crates/bevy_ecs/src/query/state.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 3c6815cee6472..f02e19b326da2 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -1783,7 +1783,7 @@ mod tests { #[test] #[should_panic( - expected = "Transmuted state for (&bevy_ecs::query::state::tests::C, ()) attempts to access terms that are not allowed by original state (&bevy_ecs::query::state::tests::A, ())." + expected = "Joined state for (&bevy_ecs::query::state::tests::C, ()) attempts to access terms that are not allowed by state (&bevy_ecs::query::state::tests::A, ()) joined with (&bevy_ecs::query::state::tests::B, ())." )] fn cannot_join_wrong_fetch() { let mut world = World::new(); @@ -1795,7 +1795,7 @@ mod tests { #[test] #[should_panic( - expected = "Transmuted state for (bevy_ecs::entity::Entity, bevy_ecs::query::filter::Changed) attempts to access terms that are not allowed by original state (&bevy_ecs::query::state::tests::A, bevy_ecs::query::filter::Without)." + expected = "Joined state for (bevy_ecs::entity::Entity, bevy_ecs::query::filter::Changed) attempts to access terms that are not allowed by state (&bevy_ecs::query::state::tests::A, bevy_ecs::query::filter::Without) joined with (&bevy_ecs::query::state::tests::B, bevy_ecs::query::filter::Without)." )] fn cannot_join_wrong_filter() { let mut world = World::new(); From ee344c865745092b87b2b9610980d5511ca9977c Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Sat, 2 Mar 2024 16:42:40 -0800 Subject: [PATCH 16/20] add panic for joining queries from different worlds --- crates/bevy_ecs/src/query/state.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index f02e19b326da2..3411c837449ec 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -408,7 +408,7 @@ impl QueryState { /// could be unpredictable. You might end up with a mix of archetypes that only matched /// the original query + archetypes that only match the new `QueryState`. Most of the /// safe methods on `QueryState` call [`QueryState::update_archetypes`] internally, so - /// this best used through a `Query`. + /// this is best used through a `Query`. /// /// ## Panics /// @@ -437,6 +437,10 @@ impl QueryState { world: &World, other: &QueryState, ) -> QueryState { + if self.world_id != other.world_id { + panic!("Joining queries initialized on different worlds is not allowed."); + } + let mut component_access = FilteredAccess::default(); let mut new_fetch_state = NewD::get_state(world) .expect("Could not create fetch_state, Please initialize all referenced components before transmuting."); From 8941396f4aeaaa8353b11f75bbc9c66fae1fb67a Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Sat, 2 Mar 2024 16:43:40 -0800 Subject: [PATCH 17/20] format error messages a little --- crates/bevy_ecs/src/query/state.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 3411c837449ec..e3d01e6c997ea 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -1786,9 +1786,9 @@ mod tests { } #[test] - #[should_panic( - expected = "Joined state for (&bevy_ecs::query::state::tests::C, ()) attempts to access terms that are not allowed by state (&bevy_ecs::query::state::tests::A, ()) joined with (&bevy_ecs::query::state::tests::B, ())." - )] + #[should_panic(expected = "Joined state for (&bevy_ecs::query::state::tests::C, ()) \ + attempts to access terms that are not allowed by state \ + (&bevy_ecs::query::state::tests::A, ()) joined with (&bevy_ecs::query::state::tests::B, ()).")] fn cannot_join_wrong_fetch() { let mut world = World::new(); world.init_component::(); @@ -1799,7 +1799,10 @@ mod tests { #[test] #[should_panic( - expected = "Joined state for (bevy_ecs::entity::Entity, bevy_ecs::query::filter::Changed) attempts to access terms that are not allowed by state (&bevy_ecs::query::state::tests::A, bevy_ecs::query::filter::Without) joined with (&bevy_ecs::query::state::tests::B, bevy_ecs::query::filter::Without)." + expected = "Joined state for (bevy_ecs::entity::Entity, bevy_ecs::query::filter::Changed) \ + attempts to access terms that are not allowed by state \ + (&bevy_ecs::query::state::tests::A, bevy_ecs::query::filter::Without) \ + joined with (&bevy_ecs::query::state::tests::B, bevy_ecs::query::filter::Without)." )] fn cannot_join_wrong_filter() { let mut world = World::new(); From 6767a825a5939cad453b6b06a887e0e602840236 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Sat, 2 Mar 2024 16:52:44 -0800 Subject: [PATCH 18/20] add warning for mismatched archetype generations --- crates/bevy_ecs/src/archetype.rs | 2 +- crates/bevy_ecs/src/query/state.rs | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index baa16e2503baa..6ab5bb2d420de 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -594,7 +594,7 @@ impl Archetype { /// /// This is used in archetype update methods to limit archetype updates to the /// ones added since the last time the method ran. -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Copy, Clone, PartialEq)] pub struct ArchetypeGeneration(ArchetypeId); impl ArchetypeGeneration { diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index e3d01e6c997ea..b952f0bfd6654 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -10,6 +10,7 @@ use crate::{ storage::{SparseSetIndex, TableId}, world::{unsafe_world_cell::UnsafeWorldCell, World, WorldId}, }; +use bevy_utils::tracing::warn; #[cfg(feature = "trace")] use bevy_utils::tracing::Span; use fixedbitset::FixedBitSet; @@ -464,6 +465,10 @@ impl QueryState { std::any::type_name::<(NewD, NewF)>(), std::any::type_name::<(D, F)>(), std::any::type_name::<(OtherD, OtherF)>() ); + if self.archetype_generation != other.archetype_generation { + warn!("You have tried to join queries with different archetype_generations. This could lead to unpredicatable results."); + } + // take the intersection of the matched ids let matched_tables: FixedBitSet = self .matched_tables From 6bfea2459b910e68637f7688eac63dcb0de443d8 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Sat, 2 Mar 2024 23:02:54 -0800 Subject: [PATCH 19/20] clippy --- crates/bevy_ecs/src/query/state.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index b952f0bfd6654..4e6e388787f40 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -466,7 +466,7 @@ impl QueryState { ); if self.archetype_generation != other.archetype_generation { - warn!("You have tried to join queries with different archetype_generations. This could lead to unpredicatable results."); + warn!("You have tried to join queries with different archetype_generations. This could lead to unpredictable results."); } // take the intersection of the matched ids @@ -474,18 +474,14 @@ impl QueryState { .matched_tables .intersection(&other.matched_tables) .collect(); - let matched_table_ids: Vec = matched_tables - .ones() - .map(|id| TableId::from_usize(id)) - .collect(); + let matched_table_ids: Vec = + matched_tables.ones().map(TableId::from_usize).collect(); let matched_archetypes: FixedBitSet = self .matched_archetypes .intersection(&other.matched_archetypes) .collect(); - let matched_archetype_ids: Vec = matched_archetypes - .ones() - .map(|id| ArchetypeId::new(id)) - .collect(); + let matched_archetype_ids: Vec = + matched_archetypes.ones().map(ArchetypeId::new).collect(); QueryState { world_id: self.world_id, From 2c0c48155d89e6174ba966b052f7aca4f813a52f Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Sat, 9 Mar 2024 22:15:44 -0800 Subject: [PATCH 20/20] add a perf note --- crates/bevy_ecs/src/query/state.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 4e6e388787f40..71eda8c14a864 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -411,6 +411,12 @@ impl QueryState { /// safe methods on `QueryState` call [`QueryState::update_archetypes`] internally, so /// this is best used through a `Query`. /// + /// ## Performance + /// + /// This will have similar performance as constructing a new `QueryState` since much of internal state + /// needs to be reconstructed. But it will be a little faster as it only needs to compare the intersection + /// of matching archetypes rather than iterating over all archetypes. + /// /// ## Panics /// /// Will panic if `NewD` contains accesses not in `Q` or `OtherQ`.