From bf7b9ca79c9320e65a243b2be3f1a09046730dbe Mon Sep 17 00:00:00 2001 From: harudagondi Date: Thu, 13 Jan 2022 15:13:16 +0800 Subject: [PATCH 1/2] Fix iter_combinations with filters (#3651) --- crates/bevy_ecs/src/query/filter.rs | 44 +++++++ crates/bevy_ecs/src/query/mod.rs | 176 ++++++++++++++++++++++++++++ 2 files changed, 220 insertions(+) diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 7849f1823029d..e8cd229fc6d60 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -76,6 +76,7 @@ impl WorldQuery for With { } /// The [`Fetch`] of [`With`]. +#[derive(Copy)] pub struct WithFetch { marker: PhantomData, } @@ -166,6 +167,14 @@ impl<'w, 's, T: Component> Fetch<'w, 's> for WithFetch { // SAFETY: no component access or archetype component access unsafe impl ReadOnlyFetch for WithFetch {} +impl Clone for WithFetch { + fn clone(&self) -> Self { + Self { + marker: self.marker, + } + } +} + /// Filter that selects entities without a component `T`. /// /// This is the negation of [`With`]. @@ -199,6 +208,7 @@ impl WorldQuery for Without { } /// The [`Fetch`] of [`Without`]. +#[derive(Copy)] pub struct WithoutFetch { marker: PhantomData, } @@ -289,6 +299,14 @@ impl<'w, 's, T: Component> Fetch<'w, 's> for WithoutFetch { // SAFETY: no component access or archetype component access unsafe impl ReadOnlyFetch for WithoutFetch {} +impl Clone for WithoutFetch { + fn clone(&self) -> Self { + Self { + marker: self.marker, + } + } +} + /// A filter that tests if any of the given filters apply. /// /// This is useful for example if a system with multiple components in a query only wants to run @@ -319,14 +337,25 @@ unsafe impl ReadOnlyFetch for WithoutFetch {} /// } /// # print_cool_entity_system.system(); /// ``` +#[derive(Clone, Copy)] pub struct Or(pub T); /// The [`Fetch`] of [`Or`]. +#[derive(Copy)] pub struct OrFetch { fetch: T, matches: bool, } +impl Clone for OrFetch { + fn clone(&self) -> Self { + Self { + fetch: self.fetch.clone(), + matches: self.matches, + } + } +} + macro_rules! impl_query_filter_tuple { ($(($filter: ident, $state: ident)),*) => { #[allow(unused_variables)] @@ -456,6 +485,7 @@ macro_rules! impl_tick_filter { pub struct $name(PhantomData); $(#[$fetch_meta])* + #[derive(Copy)] pub struct $fetch_name { table_ticks: *const UnsafeCell, entity_table_rows: *const usize, @@ -586,6 +616,20 @@ macro_rules! impl_tick_filter { /// SAFETY: read-only access unsafe impl ReadOnlyFetch for $fetch_name {} + + impl Clone for $fetch_name { + fn clone(&self) -> Self { + Self { + table_ticks: self.table_ticks.clone(), + entity_table_rows: self.entity_table_rows.clone(), + marker: self.marker.clone(), + entities: self.entities.clone(), + sparse_set: self.sparse_set.clone(), + last_change_tick: self.last_change_tick.clone(), + change_tick: self.change_tick.clone(), + } + } + } }; } diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index dcc2416940cc2..9ff5672bb59fa 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -137,6 +137,182 @@ mod tests { assert_eq!(values, Vec::<[&B; 2]>::new()); } + #[test] + fn query_filtered_iter_combinations() { + use bevy_ecs::query::{Added, Changed, Or, With, Without}; + + let mut world = World::new(); + + world.spawn().insert_bundle((A(1), B(1))); + world.spawn().insert_bundle((A(2),)); + world.spawn().insert_bundle((A(3),)); + world.spawn().insert_bundle((A(4),)); + + let mut a_query_with_b = world.query_filtered::<&A, With>(); + assert_eq!(a_query_with_b.iter_combinations::<0>(&world).count(), 0); + assert_eq!( + a_query_with_b.iter_combinations::<0>(&world).size_hint(), + (0, Some(0)) + ); + assert_eq!(a_query_with_b.iter_combinations::<1>(&world).count(), 1); + assert_eq!( + a_query_with_b.iter_combinations::<1>(&world).size_hint(), + (0, Some(1)) + ); + assert_eq!(a_query_with_b.iter_combinations::<2>(&world).count(), 0); + assert_eq!( + a_query_with_b.iter_combinations::<2>(&world).size_hint(), + (0, Some(0)) + ); + assert_eq!(a_query_with_b.iter_combinations::<3>(&world).count(), 0); + assert_eq!( + a_query_with_b.iter_combinations::<3>(&world).size_hint(), + (0, Some(0)) + ); + assert_eq!(a_query_with_b.iter_combinations::<4>(&world).count(), 0); + assert_eq!( + a_query_with_b.iter_combinations::<4>(&world).size_hint(), + (0, Some(0)) + ); + assert_eq!(a_query_with_b.iter_combinations::<5>(&world).count(), 0); + assert_eq!( + a_query_with_b.iter_combinations::<5>(&world).size_hint(), + (0, Some(0)) + ); + assert_eq!(a_query_with_b.iter_combinations::<1024>(&world).count(), 0); + assert_eq!( + a_query_with_b.iter_combinations::<1024>(&world).size_hint(), + (0, Some(0)) + ); + + let mut a_query_without_b = world.query_filtered::<&A, Without>(); + assert_eq!(a_query_without_b.iter_combinations::<0>(&world).count(), 0); + assert_eq!( + a_query_without_b.iter_combinations::<0>(&world).size_hint(), + (0, Some(0)) + ); + assert_eq!(a_query_without_b.iter_combinations::<1>(&world).count(), 3); + assert_eq!( + a_query_without_b.iter_combinations::<1>(&world).size_hint(), + (0, Some(3)) + ); + assert_eq!(a_query_without_b.iter_combinations::<2>(&world).count(), 3); + assert_eq!( + a_query_without_b.iter_combinations::<2>(&world).size_hint(), + (0, Some(3)) + ); + assert_eq!(a_query_without_b.iter_combinations::<3>(&world).count(), 1); + assert_eq!( + a_query_without_b.iter_combinations::<3>(&world).size_hint(), + (0, Some(1)) + ); + assert_eq!(a_query_without_b.iter_combinations::<4>(&world).count(), 0); + assert_eq!( + a_query_without_b.iter_combinations::<4>(&world).size_hint(), + (0, Some(0)) + ); + assert_eq!(a_query_without_b.iter_combinations::<5>(&world).count(), 0); + assert_eq!( + a_query_without_b.iter_combinations::<5>(&world).size_hint(), + (0, Some(0)) + ); + assert_eq!( + a_query_without_b.iter_combinations::<1024>(&world).count(), + 0 + ); + assert_eq!( + a_query_without_b + .iter_combinations::<1024>(&world) + .size_hint(), + (0, Some(0)) + ); + + let values: Vec<[&A; 2]> = a_query_without_b.iter_combinations(&world).collect(); + assert_eq!( + values, + vec![[&A(2), &A(3)], [&A(2), &A(4)], [&A(3), &A(4)],] + ); + + let values: Vec<[&A; 3]> = a_query_without_b.iter_combinations(&world).collect(); + assert_eq!(values, vec![[&A(2), &A(3), &A(4)],]); + + let mut query = world.query_filtered::<&A, Or<(With, With)>>(); + let values: Vec<[&A; 2]> = query.iter_combinations(&world).collect(); + assert_eq!( + values, + vec![ + [&A(1), &A(2)], + [&A(1), &A(3)], + [&A(1), &A(4)], + [&A(2), &A(3)], + [&A(2), &A(4)], + [&A(3), &A(4)], + ] + ); + + let mut query = world.query_filtered::<&mut A, Without>(); + let mut combinations = query.iter_combinations_mut(&mut world); + while let Some([mut a, mut b, mut c]) = combinations.fetch_next() { + a.0 += 10; + b.0 += 100; + c.0 += 1000; + } + + let values: Vec<[&A; 3]> = a_query_without_b.iter_combinations(&world).collect(); + assert_eq!(values, vec![[&A(12), &A(103), &A(1004)],]); + + // Check if Added, Changed works + let mut world = World::new(); + + world.spawn().insert_bundle((A(1), B(1))); + world.spawn().insert_bundle((A(2), B(2))); + world.spawn().insert_bundle((A(3), B(3))); + world.spawn().insert_bundle((A(4), B(4))); + + let mut query_added = world.query_filtered::<&A, Added>(); + + world.clear_trackers(); + world.spawn().insert_bundle((A(5),)); + + assert_eq!(query_added.iter_combinations::<2>(&world).count(), 0); + + world.clear_trackers(); + world.spawn().insert_bundle((A(6),)); + world.spawn().insert_bundle((A(7),)); + + assert_eq!(query_added.iter_combinations::<2>(&world).count(), 1); + + world.clear_trackers(); + world.spawn().insert_bundle((A(8),)); + world.spawn().insert_bundle((A(9),)); + world.spawn().insert_bundle((A(10),)); + + assert_eq!(query_added.iter_combinations::<2>(&world).count(), 3); + + world.clear_trackers(); + + let mut query_changed = world.query_filtered::<&A, Changed>(); + + let mut query = world.query_filtered::<&mut A, With>(); + let mut combinations = query.iter_combinations_mut(&mut world); + while let Some([mut a, mut b, mut c]) = combinations.fetch_next() { + a.0 += 10; + b.0 += 100; + c.0 += 1000; + } + + let values: Vec<[&A; 3]> = query_changed.iter_combinations(&world).collect(); + assert_eq!( + values, + vec![ + [&A(31), &A(212), &A(1203)], + [&A(31), &A(212), &A(3004)], + [&A(31), &A(1203), &A(3004)], + [&A(212), &A(1203), &A(3004)] + ] + ); + } + #[test] fn query_iter_combinations_sparse() { let mut world = World::new(); From 3f26e35e2d75f9aad38e87de2fe0139a5c222a56 Mon Sep 17 00:00:00 2001 From: Nicholas French <32023353+L-french@users.noreply.github.com> Date: Thu, 13 Jan 2022 01:50:54 +0000 Subject: [PATCH 2/2] Fix documentation for QueryState::iter_manual (#3644) # Objective - Fixes #3616 ## Solution - As described in the issue, documentation for `iter_manual` was copied from `iter_combinations` and did not reflect the behavior of the method. I've pulled some information from #2351 to create a more accurate description. --- crates/bevy_ecs/src/query/state.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 33d2ecab4fe7f..c13b0dc75700b 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -276,16 +276,10 @@ where } } - /// Returns an [`Iterator`] over all possible combinations of `K` query results without repetition. - /// This can only be called for read-only queries. - /// - /// For permutations of size K of query returning N results, you will get: - /// - if K == N: one permutation of all query results - /// - if K < N: all possible K-sized combinations of query results, without repetition - /// - if K > N: empty set (no K-sized combinations exist) + /// Returns an [`Iterator`] over the query results for the given [`World`] without updating the query's archetypes. + /// Archetypes must be manually updated before by using [`Self::update_archetypes`]. /// - /// This can only be called for read-only queries, see [`Self::iter_combinations_mut`] for - /// write-queries. + /// This can only be called for read-only queries. #[inline] pub fn iter_manual<'w, 's>( &'s self,