From 381d8a704f0c317c320c67cfe369944961477ecb Mon Sep 17 00:00:00 2001 From: harudagondi Date: Thu, 13 Jan 2022 21:04:19 +0800 Subject: [PATCH 1/5] 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 d6052f7cd6e5b64759005a74d4bfca0c04ef0667 Mon Sep 17 00:00:00 2001 From: harudagondi Date: Thu, 13 Jan 2022 21:14:32 +0800 Subject: [PATCH 2/5] Adjust trait impls based on code review --- crates/bevy_ecs/src/query/filter.rs | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index e8cd229fc6d60..b806b4349c55a 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -76,7 +76,6 @@ impl WorldQuery for With { } /// The [`Fetch`] of [`With`]. -#[derive(Copy)] pub struct WithFetch { marker: PhantomData, } @@ -175,6 +174,8 @@ impl Clone for WithFetch { } } +impl Copy for WithFetch {} + /// Filter that selects entities without a component `T`. /// /// This is the negation of [`With`]. @@ -208,7 +209,6 @@ impl WorldQuery for Without { } /// The [`Fetch`] of [`Without`]. -#[derive(Copy)] pub struct WithoutFetch { marker: PhantomData, } @@ -307,6 +307,8 @@ impl Clone for WithoutFetch { } } +impl Copy for WithoutFetch {} + /// 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 @@ -341,21 +343,12 @@ impl Clone for WithoutFetch { pub struct Or(pub T); /// The [`Fetch`] of [`Or`]. -#[derive(Copy)] +#[derive(Clone, 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)] @@ -485,7 +478,6 @@ 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, @@ -630,6 +622,8 @@ macro_rules! impl_tick_filter { } } } + + impl Copy for $fetch_name {} }; } From 2fd0620dc3c94dfaef699a55525d0df72b869371 Mon Sep 17 00:00:00 2001 From: harudagondi Date: Sun, 16 Jan 2022 14:41:25 +0800 Subject: [PATCH 3/5] Replace Vec with HashSet in unit test --- crates/bevy_ecs/src/query/mod.rs | 37 ++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index 9ff5672bb59fa..1d6c863e9efaf 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -12,9 +12,24 @@ pub use state::*; #[cfg(test)] mod tests { + macro_rules! hash_set { + ( $($key:expr,)+) => { hash_set!($($key),+) }; + ( $($key:expr),* ) => { + { + let mut _set = ::std::collections::HashSet::new(); + $( + let _ = _set.insert($key); + )* + _set + } + } + } + + use std::collections::HashSet; + use crate::{self as bevy_ecs, component::Component, world::World}; - #[derive(Component, Debug, Eq, PartialEq)] + #[derive(Component, Debug, Hash, Eq, PartialEq)] struct A(usize); #[derive(Component, Debug, Eq, PartialEq)] struct B(usize); @@ -227,20 +242,20 @@ mod tests { (0, Some(0)) ); - let values: Vec<[&A; 2]> = a_query_without_b.iter_combinations(&world).collect(); + let values: HashSet<[&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)],] + hash_set![[&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 values: HashSet<[&A; 3]> = a_query_without_b.iter_combinations(&world).collect(); + assert_eq!(values, hash_set![[&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(); + let values: HashSet<[&A; 2]> = query.iter_combinations(&world).collect(); assert_eq!( values, - vec![ + hash_set![ [&A(1), &A(2)], [&A(1), &A(3)], [&A(1), &A(4)], @@ -258,8 +273,8 @@ mod tests { 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)],]); + let values: HashSet<[&A; 3]> = a_query_without_b.iter_combinations(&world).collect(); + assert_eq!(values, hash_set![[&A(12), &A(103), &A(1004)],]); // Check if Added, Changed works let mut world = World::new(); @@ -301,10 +316,10 @@ mod tests { c.0 += 1000; } - let values: Vec<[&A; 3]> = query_changed.iter_combinations(&world).collect(); + let values: HashSet<[&A; 3]> = query_changed.iter_combinations(&world).collect(); assert_eq!( values, - vec![ + hash_set![ [&A(31), &A(212), &A(1203)], [&A(31), &A(212), &A(3004)], [&A(31), &A(1203), &A(3004)], From d015d15d7c62e653e52c6a390eaf556f45ce2e0b Mon Sep 17 00:00:00 2001 From: harudagondi Date: Sun, 16 Jan 2022 15:03:13 +0800 Subject: [PATCH 4/5] Remove unnecessary macro --- crates/bevy_ecs/src/query/mod.rs | 37 ++++++++++++++++---------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index 1d6c863e9efaf..25930b9915d6a 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -12,19 +12,6 @@ pub use state::*; #[cfg(test)] mod tests { - macro_rules! hash_set { - ( $($key:expr,)+) => { hash_set!($($key),+) }; - ( $($key:expr),* ) => { - { - let mut _set = ::std::collections::HashSet::new(); - $( - let _ = _set.insert($key); - )* - _set - } - } - } - use std::collections::HashSet; use crate::{self as bevy_ecs, component::Component, world::World}; @@ -245,17 +232,22 @@ mod tests { let values: HashSet<[&A; 2]> = a_query_without_b.iter_combinations(&world).collect(); assert_eq!( values, - hash_set![[&A(2), &A(3)], [&A(2), &A(4)], [&A(3), &A(4)],] + [[&A(2), &A(3)], [&A(2), &A(4)], [&A(3), &A(4)],] + .into_iter() + .collect::>() ); let values: HashSet<[&A; 3]> = a_query_without_b.iter_combinations(&world).collect(); - assert_eq!(values, hash_set![[&A(2), &A(3), &A(4)],]); + assert_eq!( + values, + [[&A(2), &A(3), &A(4)],].into_iter().collect::>() + ); let mut query = world.query_filtered::<&A, Or<(With, With)>>(); let values: HashSet<[&A; 2]> = query.iter_combinations(&world).collect(); assert_eq!( values, - hash_set![ + [ [&A(1), &A(2)], [&A(1), &A(3)], [&A(1), &A(4)], @@ -263,6 +255,8 @@ mod tests { [&A(2), &A(4)], [&A(3), &A(4)], ] + .into_iter() + .collect::>() ); let mut query = world.query_filtered::<&mut A, Without>(); @@ -274,7 +268,12 @@ mod tests { } let values: HashSet<[&A; 3]> = a_query_without_b.iter_combinations(&world).collect(); - assert_eq!(values, hash_set![[&A(12), &A(103), &A(1004)],]); + assert_eq!( + values, + [[&A(12), &A(103), &A(1004)],] + .into_iter() + .collect::>() + ); // Check if Added, Changed works let mut world = World::new(); @@ -319,12 +318,14 @@ mod tests { let values: HashSet<[&A; 3]> = query_changed.iter_combinations(&world).collect(); assert_eq!( values, - hash_set![ + [ [&A(31), &A(212), &A(1203)], [&A(31), &A(212), &A(3004)], [&A(31), &A(1203), &A(3004)], [&A(212), &A(1203), &A(3004)] ] + .into_iter() + .collect::>() ); } From b8adf61af4cdf44b026fc1562d2b4b29dcaebe66 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Thu, 7 Apr 2022 17:20:53 -0700 Subject: [PATCH 5/5] clippy --- crates/bevy_ecs/src/query/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index ddfbc80be47d9..191b3dd3497a7 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -12,9 +12,9 @@ pub use state::*; #[cfg(test)] mod tests { - use std::collections::HashSet; use super::AnyOf; use crate::{self as bevy_ecs, component::Component, world::World}; + use std::collections::HashSet; #[derive(Component, Debug, Hash, Eq, PartialEq)] struct A(usize);