From 846647d82b4e362371f33b4aff26d6fde9e0e9a9 Mon Sep 17 00:00:00 2001 From: NiseVoid Date: Sat, 27 Apr 2024 22:24:25 +0200 Subject: [PATCH 01/15] Add DefaultQueryFilters --- crates/bevy_ecs/src/query/default_filters.rs | 167 +++++++++++++++++++ crates/bevy_ecs/src/query/mod.rs | 2 + crates/bevy_ecs/src/query/state.rs | 51 +++++- 3 files changed, 216 insertions(+), 4 deletions(-) create mode 100644 crates/bevy_ecs/src/query/default_filters.rs diff --git a/crates/bevy_ecs/src/query/default_filters.rs b/crates/bevy_ecs/src/query/default_filters.rs new file mode 100644 index 0000000000000..1223aa85b996f --- /dev/null +++ b/crates/bevy_ecs/src/query/default_filters.rs @@ -0,0 +1,167 @@ +use crate as bevy_ecs; +use crate::{component::ComponentId, query::FilteredAccess}; +use bevy_ecs_macros::Resource; + +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +enum FilterKind { + With, + Without, +} + +#[derive(PartialEq, Eq, Debug)] +struct DefaultFilter { + component_id: ComponentId, + kind: FilterKind, +} + +/// A list of default query filters, these can be used to globally exclude entities from queries. +/// Each individual filter is only applied to queries that don't mention that component. +/// +/// These filters are only applied to queries initialized after updating this resource, +/// it should most likely only be modifed before the app starts. +#[derive(Resource, Default)] +pub struct DefaultQueryFilters(Vec); + +impl DefaultQueryFilters { + /// Add a With filter to the default query filters + pub fn with(&mut self, component_id: ComponentId) { + self.set(component_id, FilterKind::With); + } + + /// Add a Without filter to the default query filters + pub fn without(&mut self, component_id: ComponentId) { + self.set(component_id, FilterKind::Without); + } + + /// Remove a filter for the specified [`ComponentId`] + pub fn remove(&mut self, component_id: ComponentId) { + self.0.retain(|filter| filter.component_id != component_id); + } + + fn set(&mut self, component_id: ComponentId, kind: FilterKind) { + if let Some(filter) = self + .0 + .iter_mut() + .find(|filter| filter.component_id == component_id) + { + filter.kind = kind; + } else { + self.0.push(DefaultFilter { component_id, kind }); + } + } + + pub(super) fn apply(&self, component_access: &mut FilteredAccess) { + for filter in self.0.iter() { + if !contains_component(component_access, filter.component_id) { + match filter.kind { + FilterKind::With => component_access.and_with(filter.component_id), + FilterKind::Without => component_access.and_without(filter.component_id), + } + } + } + } +} + +fn contains_component( + component_access: &FilteredAccess, + component_id: ComponentId, +) -> bool { + component_access.access().has_read(component_id) + || component_access.access().has_archetypal(component_id) + || component_access.filter_sets.iter().any(|f| { + f.with.contains(component_id.index()) || f.without.contains(component_id.index()) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_set_filters() { + let mut filters = DefaultQueryFilters::default(); + filters.with(ComponentId::new(1)); + filters.with(ComponentId::new(3)); + filters.without(ComponentId::new(3)); + + assert_eq!(2, filters.0.len()); + assert_eq!( + DefaultFilter { + component_id: ComponentId::new(1), + kind: FilterKind::With + }, + filters.0[0] + ); + assert_eq!( + DefaultFilter { + component_id: ComponentId::new(3), + kind: FilterKind::Without + }, + filters.0[1] + ); + } + + #[test] + fn test_apply_filters() { + let mut filters = DefaultQueryFilters::default(); + filters.with(ComponentId::new(1)); + filters.without(ComponentId::new(3)); + + // A component access with an unrelated component + let mut component_access = FilteredAccess::::default(); + component_access.access_mut().add_read(ComponentId::new(2)); + + let mut applied_access = component_access.clone(); + filters.apply(&mut applied_access); + assert_eq!( + vec![ComponentId::new(1)], + applied_access.with_filters().collect::>() + ); + assert_eq!( + vec![ComponentId::new(3)], + applied_access.without_filters().collect::>() + ); + + // We add a with filter, now we expect to see both filters + component_access.and_with(ComponentId::new(4)); + + let mut applied_access = component_access.clone(); + filters.apply(&mut applied_access); + assert_eq!( + vec![ComponentId::new(1), ComponentId::new(4)], + applied_access.with_filters().collect::>() + ); + assert_eq!( + vec![ComponentId::new(3)], + applied_access.without_filters().collect::>() + ); + + // We add a rule targeting a default component, that filter should no longer be added + component_access.and_with(ComponentId::new(3)); + + let mut applied_access = component_access.clone(); + filters.apply(&mut applied_access); + assert_eq!( + vec![ + ComponentId::new(1), + ComponentId::new(3), + ComponentId::new(4) + ], + applied_access.with_filters().collect::>() + ); + assert!(applied_access.without_filters().next().is_none()); + + // Archetypal access should also filter rules + component_access + .access_mut() + .add_archetypal(ComponentId::new(1)); + + let mut applied_access = component_access.clone(); + filters.apply(&mut applied_access); + assert_eq!( + vec![ComponentId::new(3), ComponentId::new(4)], + applied_access.with_filters().collect::>() + ); + assert!(applied_access.without_filters().next().is_none()); + } +} diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index 4cc2b9f3458a8..ad580ffb1f0ff 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -2,6 +2,7 @@ mod access; mod builder; +mod default_filters; mod error; mod fetch; mod filter; @@ -13,6 +14,7 @@ mod world_query; pub use access::*; pub use bevy_ecs_macros::{QueryData, QueryFilter}; pub use builder::*; +pub use default_filters::DefaultQueryFilters; pub use error::*; pub use fetch::*; pub use filter::*; diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 5e52178837072..6adad868d1a89 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -5,7 +5,8 @@ use crate::{ entity::Entity, prelude::FromWorld, query::{ - Access, DebugCheckedUnwrap, FilteredAccess, QueryCombinationIter, QueryIter, QueryParIter, + default_filters::DefaultQueryFilters, Access, DebugCheckedUnwrap, FilteredAccess, + QueryCombinationIter, QueryIter, QueryParIter, }, storage::{SparseSetIndex, TableId}, world::{unsafe_world_cell::UnsafeWorldCell, World, WorldId}, @@ -194,6 +195,10 @@ impl QueryState { // properly considered in a global "cross-query" context (both within systems and across systems). component_access.extend(&filter_component_access); + if let Some(default_filters) = world.get_resource::() { + default_filters.apply(&mut component_access); + } + Self { world_id: world.id(), archetype_generation: ArchetypeGeneration::initial(), @@ -218,13 +223,19 @@ impl QueryState { let filter_state = F::init_state(builder.world_mut()); D::set_access(&mut fetch_state, builder.access()); + let mut component_access = builder.access().clone(); + + if let Some(default_filters) = builder.world().get_resource::() { + default_filters.apply(&mut component_access); + } + let mut state = Self { world_id: builder.world().id(), archetype_generation: ArchetypeGeneration::initial(), matched_storage_ids: Vec::new(), fetch_state, filter_state, - component_access: builder.access().clone(), + component_access, matched_tables: Default::default(), matched_archetypes: Default::default(), #[cfg(feature = "trace")] @@ -1657,8 +1668,12 @@ impl From> for QueryState>::new(&mut world); let _: QueryState> = query_1.join_filtered(&world, &query_2); } + + #[test] + fn query_respects_default_filters() { + let mut world = World::new(); + world.spawn((A(0), B(0))); + world.spawn((B(0), C(0))); + world.spawn(C(0)); + + let b = world.component_id::().unwrap(); + let c = world.component_id::().unwrap(); + + let mut default_filters = DefaultQueryFilters::default(); + default_filters.with(b); + default_filters.without(c); + world.insert_resource(default_filters); + + // With, Without only matches the first entity + let mut query = QueryState::<()>::new(&mut world); + assert_eq!(1, query.iter(&world).count()); + + // With, With only matches the second entity + let mut query = QueryState::<(), With>::new(&mut world); + assert_eq!(1, query.iter(&world).count()); + + // Without only matches the last entity + let mut query = QueryState::, Without>::new(&mut world); + assert_eq!(1, query.iter(&world).count()); + } } From e7c07a2452c6924e6fe705ef3af2311eb2efa8ba Mon Sep 17 00:00:00 2001 From: NiseVoid Date: Sat, 27 Apr 2024 22:51:08 +0200 Subject: [PATCH 02/15] Fix typo --- crates/bevy_ecs/src/query/default_filters.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/query/default_filters.rs b/crates/bevy_ecs/src/query/default_filters.rs index 1223aa85b996f..a5991d6ac5b7c 100644 --- a/crates/bevy_ecs/src/query/default_filters.rs +++ b/crates/bevy_ecs/src/query/default_filters.rs @@ -18,7 +18,7 @@ struct DefaultFilter { /// Each individual filter is only applied to queries that don't mention that component. /// /// These filters are only applied to queries initialized after updating this resource, -/// it should most likely only be modifed before the app starts. +/// it should most likely only be modified before the app starts. #[derive(Resource, Default)] pub struct DefaultQueryFilters(Vec); From 29ac28dbce4939c1af06e0a0a4286e2618a91c9f Mon Sep 17 00:00:00 2001 From: NiseVoid Date: Sun, 28 Apr 2024 19:16:27 +0200 Subject: [PATCH 03/15] Add DefaultQueryFilters to default app --- crates/bevy_app/src/app.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 28fdf42e8b977..ddd22f572a31d 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -7,6 +7,7 @@ use bevy_ecs::{ event::{event_update_system, ManualEventReader}, intern::Interned, prelude::*, + query::DefaultQueryFilters, schedule::{ScheduleBuildSettings, ScheduleLabel}, system::SystemId, }; @@ -94,6 +95,7 @@ impl Default for App { let mut app = App::empty(); app.sub_apps.main.update_schedule = Some(Main.intern()); + app.init_resource::(); #[cfg(feature = "bevy_reflect")] app.init_resource::(); app.add_plugins(MainSchedulePlugin); From 63d07b6f78bb88c5ea1d9b8a54cf42c9a984db9b Mon Sep 17 00:00:00 2001 From: NiseVoid Date: Sun, 19 May 2024 13:07:15 +0200 Subject: [PATCH 04/15] Minor doc improvements --- crates/bevy_ecs/src/query/default_filters.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/query/default_filters.rs b/crates/bevy_ecs/src/query/default_filters.rs index a5991d6ac5b7c..e3a9ce461b5d0 100644 --- a/crates/bevy_ecs/src/query/default_filters.rs +++ b/crates/bevy_ecs/src/query/default_filters.rs @@ -17,18 +17,24 @@ struct DefaultFilter { /// A list of default query filters, these can be used to globally exclude entities from queries. /// Each individual filter is only applied to queries that don't mention that component. /// +/// If for example we register a `Hidden` component using the `without` method, entities with this +/// component will only be visible to a [`Query`](crate::prelude::Query) containing something like +/// [`With`](crate::prelude::With) or [`Has`](crate::prelude::Has). +/// /// These filters are only applied to queries initialized after updating this resource, /// it should most likely only be modified before the app starts. #[derive(Resource, Default)] pub struct DefaultQueryFilters(Vec); impl DefaultQueryFilters { - /// Add a With filter to the default query filters + /// Add a With filter to the default query filters. + /// Removes any Without filter for this component if present. pub fn with(&mut self, component_id: ComponentId) { self.set(component_id, FilterKind::With); } - /// Add a Without filter to the default query filters + /// Add a Without filter to the default query filters. + /// Removes any With filter for this component if present. pub fn without(&mut self, component_id: ComponentId) { self.set(component_id, FilterKind::Without); } From 5b8aafe731ba3a840e43ba231d3b16707ff28d59 Mon Sep 17 00:00:00 2001 From: NiseVoid Date: Sun, 19 May 2024 16:15:54 +0200 Subject: [PATCH 05/15] Address review feedback --- crates/bevy_app/src/app.rs | 1 - crates/bevy_ecs/src/query/access.rs | 10 ++ crates/bevy_ecs/src/query/default_filters.rs | 146 +++++++++++-------- crates/bevy_ecs/src/query/state.rs | 14 +- crates/bevy_ecs/src/world/mod.rs | 27 ++++ 5 files changed, 122 insertions(+), 76 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index ddd22f572a31d..58ebea6c9d79f 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -95,7 +95,6 @@ impl Default for App { let mut app = App::empty(); app.sub_apps.main.update_schedule = Some(Main.intern()); - app.init_resource::(); #[cfg(feature = "bevy_reflect")] app.init_resource::(); app.add_plugins(MainSchedulePlugin); diff --git a/crates/bevy_ecs/src/query/access.rs b/crates/bevy_ecs/src/query/access.rs index e4880c33ad5f2..30e758e695a3a 100644 --- a/crates/bevy_ecs/src/query/access.rs +++ b/crates/bevy_ecs/src/query/access.rs @@ -508,6 +508,16 @@ impl FilteredAccess { .iter() .flat_map(|f| f.without.ones().map(T::get_sparse_set_index)) } + + /// Returns true if the index is used by this `FilteredAccess` in any way + pub fn contains(&self, index: T) -> bool { + self.access().has_read(index.clone()) + || self.access().has_archetypal(index.clone()) + || self.filter_sets.iter().any(|f| { + f.with.contains(index.sparse_set_index()) + || f.without.contains(index.sparse_set_index()) + }) + } } #[derive(Clone, Eq, PartialEq)] diff --git a/crates/bevy_ecs/src/query/default_filters.rs b/crates/bevy_ecs/src/query/default_filters.rs index e3a9ce461b5d0..78ed6b2487169 100644 --- a/crates/bevy_ecs/src/query/default_filters.rs +++ b/crates/bevy_ecs/src/query/default_filters.rs @@ -1,84 +1,111 @@ use crate as bevy_ecs; use crate::{component::ComponentId, query::FilteredAccess}; use bevy_ecs_macros::Resource; +use bevy_utils::HashMap; #[derive(Clone, Copy, PartialEq, Eq, Debug)] +#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))] enum FilterKind { With, Without, } -#[derive(PartialEq, Eq, Debug)] -struct DefaultFilter { - component_id: ComponentId, - kind: FilterKind, -} - /// A list of default query filters, these can be used to globally exclude entities from queries. -/// Each individual filter is only applied to queries that don't mention that component. +/// Default filters are applied to any queries that does not explicitly mention that component. +/// +/// If for example we register a `Hidden` component using the `without_untyped` method, entities +/// with this component will only be visible to a [`Query`](crate::prelude::Query) containing +/// something like [`With`](crate::prelude::With) or [`Has`](crate::prelude::Has). +/// See below for a more detailed example. +/// +/// These filters are only applied to queries whose cache is generated after updating this resource, +/// As a result, this resource should generally only be modified before the app starts (typically +/// during plugin construction) +/// +/// See `World::default_with_filter` and `World::default_without_filter` for easier access +/// +/// ### Example +/// +/// ```rust +/// # use bevy_ecs::{prelude::*, query::DefaultQueryFilters}; +/// # #[derive(Component)] +/// # struct Enabled; +/// # #[derive(Component)] +/// # struct TopSecret; +/// # +/// let mut world = World::new(); +/// let mut filters = DefaultQueryFilters::default(); +/// filters.with_untyped(world.init_component::()); +/// filters.without_untyped(world.init_component::()); +/// world.insert_resource(filters); +/// +/// // This entity is missing Enabled, so most queries won't see it +/// let entity_a = world.spawn_empty().id(); +/// +/// // This entity has Enabled, and isn't TopSecret, so most queries see it +/// let entity_b = world.spawn(Enabled).id(); +/// +/// // This entity is Enabled and TopSecret, so most queries won't see it +/// let entity_c = world.spawn((Enabled, TopSecret)).id(); /// -/// If for example we register a `Hidden` component using the `without` method, entities with this -/// component will only be visible to a [`Query`](crate::prelude::Query) containing something like -/// [`With`](crate::prelude::With) or [`Has`](crate::prelude::Has). +/// // This entity is TopSecret but not enabled, so only very specific queries will see it +/// let entity_d = world.spawn(TopSecret).id(); /// -/// These filters are only applied to queries initialized after updating this resource, -/// it should most likely only be modified before the app starts. -#[derive(Resource, Default)] -pub struct DefaultQueryFilters(Vec); +/// // This query does not mention either of the markers, so it only gets entity_b +/// let mut query = world.query::(); +/// assert_eq!(1, query.iter(&world).count()); +/// assert_eq!(entity_b, query.get_single(&world).unwrap()); +/// +/// // This query only wants entities that aren't Enabled, but can't see TopSecret entities, +/// // thus it only sees entity_a +/// let mut query = world.query_filtered::>(); +/// assert_eq!(1, query.iter(&world).count()); +/// assert_eq!(entity_a, query.get_single(&world).unwrap()); +/// +/// // This query only wants TopSecret entities, but still can't see entities that aren't Enabled, +/// // thus it only sees entity_c +/// let mut query = world.query_filtered::>(); +/// assert_eq!(1, query.iter(&world).count()); +/// assert_eq!(entity_c, query.get_single(&world).unwrap()); +/// +/// // This query mentions both, so it gets results as if the filters don't exist +/// let mut query = world.query::<(Entity, Has, Has)>(); +/// assert_eq!(4, query.iter(&world).count()); +/// ``` +#[derive(Resource, Default, Debug)] +#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))] +pub struct DefaultQueryFilters(HashMap); impl DefaultQueryFilters { /// Add a With filter to the default query filters. /// Removes any Without filter for this component if present. - pub fn with(&mut self, component_id: ComponentId) { - self.set(component_id, FilterKind::With); + pub fn with_untyped(&mut self, component_id: ComponentId) { + self.0.insert(component_id, FilterKind::With); } /// Add a Without filter to the default query filters. /// Removes any With filter for this component if present. - pub fn without(&mut self, component_id: ComponentId) { - self.set(component_id, FilterKind::Without); + pub fn without_untyped(&mut self, component_id: ComponentId) { + self.0.insert(component_id, FilterKind::Without); } /// Remove a filter for the specified [`ComponentId`] pub fn remove(&mut self, component_id: ComponentId) { - self.0.retain(|filter| filter.component_id != component_id); - } - - fn set(&mut self, component_id: ComponentId, kind: FilterKind) { - if let Some(filter) = self - .0 - .iter_mut() - .find(|filter| filter.component_id == component_id) - { - filter.kind = kind; - } else { - self.0.push(DefaultFilter { component_id, kind }); - } + self.0.remove(&component_id); } pub(super) fn apply(&self, component_access: &mut FilteredAccess) { - for filter in self.0.iter() { - if !contains_component(component_access, filter.component_id) { - match filter.kind { - FilterKind::With => component_access.and_with(filter.component_id), - FilterKind::Without => component_access.and_without(filter.component_id), + for (&component_id, kind) in self.0.iter() { + if !component_access.contains(component_id) { + match kind { + FilterKind::With => component_access.and_with(component_id), + FilterKind::Without => component_access.and_without(component_id), } } } } } -fn contains_component( - component_access: &FilteredAccess, - component_id: ComponentId, -) -> bool { - component_access.access().has_read(component_id) - || component_access.access().has_archetypal(component_id) - || component_access.filter_sets.iter().any(|f| { - f.with.contains(component_id.index()) || f.without.contains(component_id.index()) - }) -} - #[cfg(test)] mod tests { use super::*; @@ -86,32 +113,23 @@ mod tests { #[test] fn test_set_filters() { let mut filters = DefaultQueryFilters::default(); - filters.with(ComponentId::new(1)); - filters.with(ComponentId::new(3)); - filters.without(ComponentId::new(3)); + filters.with_untyped(ComponentId::new(1)); + filters.with_untyped(ComponentId::new(3)); + filters.without_untyped(ComponentId::new(3)); assert_eq!(2, filters.0.len()); + assert_eq!(Some(&FilterKind::With), filters.0.get(&ComponentId::new(1))); assert_eq!( - DefaultFilter { - component_id: ComponentId::new(1), - kind: FilterKind::With - }, - filters.0[0] - ); - assert_eq!( - DefaultFilter { - component_id: ComponentId::new(3), - kind: FilterKind::Without - }, - filters.0[1] + Some(&FilterKind::Without), + filters.0.get(&ComponentId::new(3)) ); } #[test] fn test_apply_filters() { let mut filters = DefaultQueryFilters::default(); - filters.with(ComponentId::new(1)); - filters.without(ComponentId::new(3)); + filters.with_untyped(ComponentId::new(1)); + filters.without_untyped(ComponentId::new(3)); // A component access with an unrelated component let mut component_access = FilteredAccess::::default(); diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 6adad868d1a89..04132549c966d 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -1669,10 +1669,7 @@ impl From> for QueryState().unwrap(); - let c = world.component_id::().unwrap(); - - let mut default_filters = DefaultQueryFilters::default(); - default_filters.with(b); - default_filters.without(c); - world.insert_resource(default_filters); + world.default_with_filter::(); + world.default_without_filter::(); // With, Without only matches the first entity let mut query = QueryState::<()>::new(&mut world); diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index ea52da782287b..7d925b86c2d0a 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -8,6 +8,7 @@ mod spawn_batch; pub mod unsafe_world_cell; pub use crate::change_detection::{Mut, Ref, CHECK_TICK_THRESHOLD}; +use crate::query::DefaultQueryFilters; pub use crate::world::command_queue::CommandQueue; pub use deferred_world::DeferredWorld; pub use entity_ref::{ @@ -1074,6 +1075,32 @@ impl World { QueryState::new(self) } + /// Add a [`With`](crate::prelude::With) filter to [`DefaultQueryFilters`]. + /// Inserts the [`DefaultQueryFilters`] resource if it does not exist + pub fn default_with_filter(&mut self) { + let id = self.init_component::(); + if let Some(mut filters) = self.get_resource_mut::() { + filters.with_untyped(id); + } else { + let mut filters = DefaultQueryFilters::default(); + filters.with_untyped(id); + self.insert_resource(filters); + } + } + + /// Add a [`Without`](crate::prelude::Without) filter to [`DefaultQueryFilters`]. + /// Inserts the [`DefaultQueryFilters`] resource if it does not exist + pub fn default_without_filter(&mut self) { + let id = self.init_component::(); + if let Some(mut filters) = self.get_resource_mut::() { + filters.without_untyped(id); + } else { + let mut filters = DefaultQueryFilters::default(); + filters.without_untyped(id); + self.insert_resource(filters); + } + } + /// Returns an iterator of entities that had components of type `T` removed /// since the last call to [`World::clear_trackers`]. pub fn removed(&self) -> impl Iterator + '_ { From 805784d2bcd0441642d08db14c08756744b8b358 Mon Sep 17 00:00:00 2001 From: NiseVoid Date: Sun, 19 May 2024 16:19:13 +0200 Subject: [PATCH 06/15] Remove bevy_app change --- crates/bevy_app/src/app.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 58ebea6c9d79f..28fdf42e8b977 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -7,7 +7,6 @@ use bevy_ecs::{ event::{event_update_system, ManualEventReader}, intern::Interned, prelude::*, - query::DefaultQueryFilters, schedule::{ScheduleBuildSettings, ScheduleLabel}, system::SystemId, }; From 2226dfd38b06e6f02d6d8ca3a8be82bd84824469 Mon Sep 17 00:00:00 2001 From: NiseVoid Date: Sun, 19 May 2024 22:42:17 +0200 Subject: [PATCH 07/15] Rename world methods and add unset version --- crates/bevy_ecs/src/query/default_filters.rs | 5 +++-- crates/bevy_ecs/src/query/state.rs | 4 ++-- crates/bevy_ecs/src/world/mod.rs | 12 ++++++++++-- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/query/default_filters.rs b/crates/bevy_ecs/src/query/default_filters.rs index 78ed6b2487169..480a8cd69122f 100644 --- a/crates/bevy_ecs/src/query/default_filters.rs +++ b/crates/bevy_ecs/src/query/default_filters.rs @@ -22,7 +22,8 @@ enum FilterKind { /// As a result, this resource should generally only be modified before the app starts (typically /// during plugin construction) /// -/// See `World::default_with_filter` and `World::default_without_filter` for easier access +/// See `World::set_default_with_filter`, `World::set_default_without_filter`, and +/// `World::unset_default_filter` for easier access. /// /// ### Example /// @@ -90,7 +91,7 @@ impl DefaultQueryFilters { } /// Remove a filter for the specified [`ComponentId`] - pub fn remove(&mut self, component_id: ComponentId) { + pub fn remove_untyped(&mut self, component_id: ComponentId) { self.0.remove(&component_id); } diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 04132549c966d..c20e0fee8f42d 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -2050,8 +2050,8 @@ mod tests { world.spawn((B(0), C(0))); world.spawn(C(0)); - world.default_with_filter::(); - world.default_without_filter::(); + world.set_default_with_filter::(); + world.set_default_without_filter::(); // With, Without only matches the first entity let mut query = QueryState::<()>::new(&mut world); diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 7d925b86c2d0a..129242c99cc12 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1077,7 +1077,7 @@ impl World { /// Add a [`With`](crate::prelude::With) filter to [`DefaultQueryFilters`]. /// Inserts the [`DefaultQueryFilters`] resource if it does not exist - pub fn default_with_filter(&mut self) { + pub fn set_default_with_filter(&mut self) { let id = self.init_component::(); if let Some(mut filters) = self.get_resource_mut::() { filters.with_untyped(id); @@ -1090,7 +1090,7 @@ impl World { /// Add a [`Without`](crate::prelude::Without) filter to [`DefaultQueryFilters`]. /// Inserts the [`DefaultQueryFilters`] resource if it does not exist - pub fn default_without_filter(&mut self) { + pub fn set_default_without_filter(&mut self) { let id = self.init_component::(); if let Some(mut filters) = self.get_resource_mut::() { filters.without_untyped(id); @@ -1101,6 +1101,14 @@ impl World { } } + /// Remove any default filter is specified for T. + pub fn unset_default_filter(&mut self) { + let id = self.init_component::(); + if let Some(mut filters) = self.get_resource_mut::() { + filters.remove_untyped(id); + } + } + /// Returns an iterator of entities that had components of type `T` removed /// since the last call to [`World::clear_trackers`]. pub fn removed(&self) -> impl Iterator + '_ { From 599f3a2af1ff13c249105df98c91f74bc552b06c Mon Sep 17 00:00:00 2001 From: NiseVoid Date: Sun, 19 May 2024 22:45:51 +0200 Subject: [PATCH 08/15] Fix error in doc comment --- crates/bevy_ecs/src/world/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 129242c99cc12..266cb8cd8c0c2 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1101,7 +1101,7 @@ impl World { } } - /// Remove any default filter is specified for T. + /// Remove any default filter specified for T. pub fn unset_default_filter(&mut self) { let id = self.init_component::(); if let Some(mut filters) = self.get_resource_mut::() { From 96758cbc7252e7ff2a71a13aec25ba48c71addab Mon Sep 17 00:00:00 2001 From: NiseVoid Date: Tue, 27 Aug 2024 03:47:54 +0200 Subject: [PATCH 09/15] Fix after merge and add test for is_dense --- crates/bevy_ecs/src/query/access.rs | 2 +- crates/bevy_ecs/src/query/default_filters.rs | 4 +- crates/bevy_ecs/src/query/state.rs | 50 ++++++++++++++++++-- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/query/access.rs b/crates/bevy_ecs/src/query/access.rs index e624f9251505b..e9ace1b2a0b0f 100644 --- a/crates/bevy_ecs/src/query/access.rs +++ b/crates/bevy_ecs/src/query/access.rs @@ -885,7 +885,7 @@ impl FilteredAccess { /// Returns true if the index is used by this `FilteredAccess` in any way pub fn contains(&self, index: T) -> bool { - self.access().has_read(index.clone()) + self.access().has_component_read(index.clone()) || self.access().has_archetypal(index.clone()) || self.filter_sets.iter().any(|f| { f.with.contains(index.sparse_set_index()) diff --git a/crates/bevy_ecs/src/query/default_filters.rs b/crates/bevy_ecs/src/query/default_filters.rs index 193cd2276ffb9..66ddb0c03045b 100644 --- a/crates/bevy_ecs/src/query/default_filters.rs +++ b/crates/bevy_ecs/src/query/default_filters.rs @@ -145,7 +145,9 @@ mod tests { // A component access with an unrelated component let mut component_access = FilteredAccess::::default(); - component_access.access_mut().add_read(ComponentId::new(2)); + component_access + .access_mut() + .add_component_read(ComponentId::new(2)); let mut applied_access = component_access.clone(); filters.apply(&mut applied_access); diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 01d1f156242cb..f3fb7584d7b16 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -207,7 +207,7 @@ impl QueryState { if let Some(default_filters) = world.get_resource::() { default_filters.apply(&mut component_access); - is_dense = is_dense && default_filters.is_dense(world.components()); + is_dense &= default_filters.is_dense(world.components()); } Self { @@ -237,16 +237,19 @@ impl QueryState { let mut component_access = builder.access().clone(); + // For dynamic queries the dense-ness is given by the query builder. + let mut is_dense = builder.is_dense(); + if let Some(default_filters) = builder.world().get_resource::() { default_filters.apply(&mut component_access); + is_dense &= default_filters.is_dense(builder.world().components()); } let mut state = Self { world_id: builder.world().id(), archetype_generation: ArchetypeGeneration::initial(), matched_storage_ids: Vec::new(), - // For dynamic queries the dense-ness is given by the query builder. - is_dense: builder.is_dense(), + is_dense, fetch_state, filter_state, component_access, @@ -2200,4 +2203,45 @@ mod tests { let mut query = QueryState::, Without>::new(&mut world); assert_eq!(1, query.iter(&world).count()); } + + #[derive(Component)] + struct Table; + + #[derive(Component)] + #[component(storage = "SparseSet")] + struct Sparse; + + #[test] + fn query_default_filters_updates_is_dense() { + let mut world = World::new(); + world.spawn((Table, Sparse)); + world.spawn(Table); + world.spawn(Sparse); + + let mut query = QueryState::<()>::new(&mut world); + // There are no sparse components involved thus the query is dense + assert!(query.is_dense); + assert_eq!(3, query.iter(&world).count()); + + world.set_default_with_filter::(); + + let mut query = QueryState::<()>::new(&mut world); + // The query doesn't ask for sparse components, but the default filters adds + // a sparse components thus it is NOT dense + assert!(!query.is_dense); + assert_eq!(2, query.iter(&world).count()); + + world.unset_default_filter::(); + world.set_default_with_filter::(); + + let mut query = QueryState::<()>::new(&mut world); + // If the filter is instead a table components, the query can still be dense + assert!(query.is_dense); + assert_eq!(2, query.iter(&world).count()); + + let mut query = QueryState::<&Sparse>::new(&mut world); + // But only if the original query was dense + assert!(!query.is_dense); + assert_eq!(1, query.iter(&world).count()); + } } From d4bee604f6a83229b0e29f5ccff5e12f63e3d44c Mon Sep 17 00:00:00 2001 From: NiseVoid Date: Wed, 18 Sep 2024 22:16:43 +0200 Subject: [PATCH 10/15] Mention performance implications of filter storage types --- crates/bevy_ecs/src/query/default_filters.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/bevy_ecs/src/query/default_filters.rs b/crates/bevy_ecs/src/query/default_filters.rs index 66ddb0c03045b..27065c604b6ff 100644 --- a/crates/bevy_ecs/src/query/default_filters.rs +++ b/crates/bevy_ecs/src/query/default_filters.rs @@ -25,6 +25,9 @@ enum FilterKind { /// As a result, this resource should generally only be modified before the app starts (typically /// during plugin construction) /// +/// Because these filters are applied to all queries, the storage type of the component has +/// implications for the entire app. See [`Query` performance] for more info. +/// /// See `World::set_default_with_filter`, `World::set_default_without_filter`, and /// `World::unset_default_filter` for easier access. /// @@ -76,6 +79,8 @@ enum FilterKind { /// let mut query = world.query::<(Entity, Has, Has)>(); /// assert_eq!(4, query.iter(&world).count()); /// ``` +/// +/// [`Query` performance]: crate::prelude::Query#performance #[derive(Resource, Default, Debug)] #[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))] pub struct DefaultQueryFilters(HashMap); From 92b00572699450a15abad9128bfdb40255a01adf Mon Sep 17 00:00:00 2001 From: NiseVoid Date: Tue, 24 Sep 2024 01:32:03 +0200 Subject: [PATCH 11/15] Limit default filters to one ComponentId per concept --- crates/bevy_ecs/src/query/default_filters.rs | 140 +++++++------------ crates/bevy_ecs/src/query/state.rs | 31 ++-- crates/bevy_ecs/src/world/mod.rs | 36 ----- 3 files changed, 70 insertions(+), 137 deletions(-) diff --git a/crates/bevy_ecs/src/query/default_filters.rs b/crates/bevy_ecs/src/query/default_filters.rs index 27065c604b6ff..3e50caf81bb2b 100644 --- a/crates/bevy_ecs/src/query/default_filters.rs +++ b/crates/bevy_ecs/src/query/default_filters.rs @@ -4,21 +4,13 @@ use crate::{ query::FilteredAccess, }; use bevy_ecs_macros::Resource; -use bevy_utils::HashMap; -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))] -enum FilterKind { - With, - Without, -} - -/// A list of default query filters, these can be used to globally exclude entities from queries. +/// The default filters for all queries, these are used to globally exclude entities from queries. /// Default filters are applied to any queries that does not explicitly mention that component. /// -/// If for example we register a `Hidden` component using the `without_untyped` method, entities -/// with this component will only be visible to a [`Query`](crate::prelude::Query) containing -/// something like [`With`](crate::prelude::With) or [`Has`](crate::prelude::Has). +/// If for example the component `Disabled` is registered here, entities with this component will +/// only be visible to a [`Query`](crate::prelude::Query) containing something like +/// [`With`](crate::prelude::With) or [`Has`](crate::prelude::Has). /// See below for a more detailed example. /// /// These filters are only applied to queries whose cache is generated after updating this resource, @@ -28,94 +20,71 @@ enum FilterKind { /// Because these filters are applied to all queries, the storage type of the component has /// implications for the entire app. See [`Query` performance] for more info. /// -/// See `World::set_default_with_filter`, `World::set_default_without_filter`, and -/// `World::unset_default_filter` for easier access. -/// /// ### Example /// /// ```rust /// # use bevy_ecs::{prelude::*, query::DefaultQueryFilters}; /// # #[derive(Component)] -/// # struct Enabled; -/// # #[derive(Component)] -/// # struct TopSecret; -/// # +/// # struct Disabled; /// let mut world = World::new(); /// let mut filters = DefaultQueryFilters::default(); -/// filters.with_untyped(world.init_component::()); -/// filters.without_untyped(world.init_component::()); +/// filters.set_disabled(world.init_component::()); /// world.insert_resource(filters); /// -/// // This entity is missing Enabled, so most queries won't see it +/// // This entity is not Disabled, so most queries will see it /// let entity_a = world.spawn_empty().id(); /// -/// // This entity has Enabled, and isn't TopSecret, so most queries see it -/// let entity_b = world.spawn(Enabled).id(); -/// -/// // This entity is Enabled and TopSecret, so most queries won't see it -/// let entity_c = world.spawn((Enabled, TopSecret)).id(); +/// // This entity has Disabled, so most queries won't see it +/// let entity_b = world.spawn(Disabled).id(); /// -/// // This entity is TopSecret but not enabled, so only very specific queries will see it -/// let entity_d = world.spawn(TopSecret).id(); -/// -/// // This query does not mention either of the markers, so it only gets entity_b +/// // This query does not mention either of the markers, so it only gets entity_a /// let mut query = world.query::(); /// assert_eq!(1, query.iter(&world).count()); -/// assert_eq!(entity_b, query.get_single(&world).unwrap()); -/// -/// // This query only wants entities that aren't Enabled, but can't see TopSecret entities, -/// // thus it only sees entity_a -/// let mut query = world.query_filtered::>(); -/// assert_eq!(1, query.iter(&world).count()); /// assert_eq!(entity_a, query.get_single(&world).unwrap()); /// -/// // This query only wants TopSecret entities, but still can't see entities that aren't Enabled, -/// // thus it only sees entity_c -/// let mut query = world.query_filtered::>(); +/// // This query only wants entities that are Disabled, thus it only sees entity_b +/// let mut query = world.query_filtered::>(); /// assert_eq!(1, query.iter(&world).count()); -/// assert_eq!(entity_c, query.get_single(&world).unwrap()); +/// assert_eq!(entity_b, query.get_single(&world).unwrap()); /// -/// // This query mentions both, so it gets results as if the filters don't exist -/// let mut query = world.query::<(Entity, Has, Has)>(); -/// assert_eq!(4, query.iter(&world).count()); +/// // This also works for query data +/// let mut query = world.query::<(Entity, Has)>(); +/// assert_eq!(2, query.iter(&world).count()); +/// assert_eq!(vec![(entity_a, false), (entity_b, true)], query.iter(&world).collect::>()); /// ``` /// /// [`Query` performance]: crate::prelude::Query#performance #[derive(Resource, Default, Debug)] #[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))] -pub struct DefaultQueryFilters(HashMap); +pub struct DefaultQueryFilters { + disabled: Option, +} impl DefaultQueryFilters { - /// Add a With filter to the default query filters. - /// Removes any Without filter for this component if present. - pub fn with_untyped(&mut self, component_id: ComponentId) { - self.0.insert(component_id, FilterKind::With); - } - - /// Add a Without filter to the default query filters. - /// Removes any With filter for this component if present. - pub fn without_untyped(&mut self, component_id: ComponentId) { - self.0.insert(component_id, FilterKind::Without); + /// Set the [`ComponentId`] for the entity disabling marker + pub fn set_disabled(&mut self, component_id: ComponentId) -> Option<()> { + if self.disabled.is_some() { + return None; + } + self.disabled = Some(component_id); + Some(()) } - /// Remove a filter for the specified [`ComponentId`] - pub fn remove_untyped(&mut self, component_id: ComponentId) { - self.0.remove(&component_id); + /// Get an iterator over all default filter components + pub fn ids(&self) -> impl Iterator { + [self.disabled].into_iter().flatten() } pub(super) fn apply(&self, component_access: &mut FilteredAccess) { - for (&component_id, kind) in self.0.iter() { + for component_id in self.ids() { if !component_access.contains(component_id) { - match kind { - FilterKind::With => component_access.and_with(component_id), - FilterKind::Without => component_access.and_without(component_id), - } + component_access.and_without(component_id); } } } pub(super) fn is_dense(&self, components: &Components) -> bool { - self.0.keys().all(|&component_id| { + self.ids().all(|component_id| { components .get_info(component_id) .map_or(false, |info| info.storage_type() == StorageType::Table) @@ -130,23 +99,19 @@ mod tests { #[test] fn test_set_filters() { let mut filters = DefaultQueryFilters::default(); - filters.with_untyped(ComponentId::new(1)); - filters.with_untyped(ComponentId::new(3)); - filters.without_untyped(ComponentId::new(3)); + assert_eq!(0, filters.ids().count()); - assert_eq!(2, filters.0.len()); - assert_eq!(Some(&FilterKind::With), filters.0.get(&ComponentId::new(1))); - assert_eq!( - Some(&FilterKind::Without), - filters.0.get(&ComponentId::new(3)) - ); + assert!(filters.set_disabled(ComponentId::new(1)).is_some()); + assert!(filters.set_disabled(ComponentId::new(3)).is_none()); + + assert_eq!(1, filters.ids().count()); + assert_eq!(Some(ComponentId::new(1)), filters.ids().next()); } #[test] fn test_apply_filters() { let mut filters = DefaultQueryFilters::default(); - filters.with_untyped(ComponentId::new(1)); - filters.without_untyped(ComponentId::new(3)); + filters.set_disabled(ComponentId::new(1)); // A component access with an unrelated component let mut component_access = FilteredAccess::::default(); @@ -156,12 +121,9 @@ mod tests { let mut applied_access = component_access.clone(); filters.apply(&mut applied_access); + assert_eq!(0, applied_access.with_filters().count()); assert_eq!( vec![ComponentId::new(1)], - applied_access.with_filters().collect::>() - ); - assert_eq!( - vec![ComponentId::new(3)], applied_access.without_filters().collect::>() ); @@ -171,30 +133,28 @@ mod tests { let mut applied_access = component_access.clone(); filters.apply(&mut applied_access); assert_eq!( - vec![ComponentId::new(1), ComponentId::new(4)], + vec![ComponentId::new(4)], applied_access.with_filters().collect::>() ); assert_eq!( - vec![ComponentId::new(3)], + vec![ComponentId::new(1)], applied_access.without_filters().collect::>() ); + let copy = component_access.clone(); // We add a rule targeting a default component, that filter should no longer be added - component_access.and_with(ComponentId::new(3)); + component_access.and_with(ComponentId::new(1)); let mut applied_access = component_access.clone(); filters.apply(&mut applied_access); assert_eq!( - vec![ - ComponentId::new(1), - ComponentId::new(3), - ComponentId::new(4) - ], + vec![ComponentId::new(1), ComponentId::new(4)], applied_access.with_filters().collect::>() ); - assert!(applied_access.without_filters().next().is_none()); + assert_eq!(0, applied_access.without_filters().count()); // Archetypal access should also filter rules + component_access = copy.clone(); component_access .access_mut() .add_archetypal(ComponentId::new(1)); @@ -202,9 +162,9 @@ mod tests { let mut applied_access = component_access.clone(); filters.apply(&mut applied_access); assert_eq!( - vec![ComponentId::new(3), ComponentId::new(4)], + vec![ComponentId::new(4)], applied_access.with_filters().collect::>() ); - assert!(applied_access.without_filters().next().is_none()); + assert_eq!(0, applied_access.without_filters().count()); } } diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index f3fb7584d7b16..fca7e78e3aa9d 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -1746,6 +1746,7 @@ impl From> for QueryState(); - world.set_default_without_filter::(); + let mut df = DefaultQueryFilters::default(); + df.set_disabled(world.init_component::()); + world.insert_resource(df); - // With, Without only matches the first entity + // Without only matches the first entity let mut query = QueryState::<()>::new(&mut world); assert_eq!(1, query.iter(&world).count()); - // With, With only matches the second entity + // With matches the last two entities let mut query = QueryState::<(), With>::new(&mut world); - assert_eq!(1, query.iter(&world).count()); + assert_eq!(2, query.iter(&world).count()); + + // Has should bypass the filter entirely + let mut query = QueryState::>::new(&mut world); + assert_eq!(3, query.iter(&world).count()); - // Without only matches the last entity + // Other filters should still be respected let mut query = QueryState::, Without>::new(&mut world); assert_eq!(1, query.iter(&world).count()); } @@ -2223,21 +2229,24 @@ mod tests { assert!(query.is_dense); assert_eq!(3, query.iter(&world).count()); - world.set_default_with_filter::(); + let mut df = DefaultQueryFilters::default(); + df.set_disabled(world.init_component::()); + world.insert_resource(df); let mut query = QueryState::<()>::new(&mut world); // The query doesn't ask for sparse components, but the default filters adds // a sparse components thus it is NOT dense assert!(!query.is_dense); - assert_eq!(2, query.iter(&world).count()); + assert_eq!(1, query.iter(&world).count()); - world.unset_default_filter::(); - world.set_default_with_filter::
(); + let mut df = DefaultQueryFilters::default(); + df.set_disabled(world.init_component::
()); + world.insert_resource(df); let mut query = QueryState::<()>::new(&mut world); // If the filter is instead a table components, the query can still be dense assert!(query.is_dense); - assert_eq!(2, query.iter(&world).count()); + assert_eq!(1, query.iter(&world).count()); let mut query = QueryState::<&Sparse>::new(&mut world); // But only if the original query was dense diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index a2a44939cc224..6f1f1a1214178 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -9,8 +9,6 @@ mod identifier; mod spawn_batch; pub mod unsafe_world_cell; -use crate::query::DefaultQueryFilters; - #[cfg(feature = "bevy_reflect")] pub mod reflect; @@ -1279,40 +1277,6 @@ impl World { QueryState::new(self) } - /// Add a [`With`](crate::prelude::With) filter to [`DefaultQueryFilters`]. - /// Inserts the [`DefaultQueryFilters`] resource if it does not exist - pub fn set_default_with_filter(&mut self) { - let id = self.init_component::(); - if let Some(mut filters) = self.get_resource_mut::() { - filters.with_untyped(id); - } else { - let mut filters = DefaultQueryFilters::default(); - filters.with_untyped(id); - self.insert_resource(filters); - } - } - - /// Add a [`Without`](crate::prelude::Without) filter to [`DefaultQueryFilters`]. - /// Inserts the [`DefaultQueryFilters`] resource if it does not exist - pub fn set_default_without_filter(&mut self) { - let id = self.init_component::(); - if let Some(mut filters) = self.get_resource_mut::() { - filters.without_untyped(id); - } else { - let mut filters = DefaultQueryFilters::default(); - filters.without_untyped(id); - self.insert_resource(filters); - } - } - - /// Remove any default filter specified for T. - pub fn unset_default_filter(&mut self) { - let id = self.init_component::(); - if let Some(mut filters) = self.get_resource_mut::() { - filters.remove_untyped(id); - } - } - /// Returns an iterator of entities that had components of type `T` removed /// since the last call to [`World::clear_trackers`]. pub fn removed(&self) -> impl Iterator + '_ { From bfd8992b7428d2b97cf2b16925820ee6be61365d Mon Sep 17 00:00:00 2001 From: NiseVoid Date: Wed, 2 Oct 2024 16:04:46 +0200 Subject: [PATCH 12/15] Move code around, move most docs to module-level docs --- ...default_filters.rs => entity_disabling.rs} | 80 +++++++------------ crates/bevy_ecs/src/lib.rs | 1 + crates/bevy_ecs/src/query/mod.rs | 2 - crates/bevy_ecs/src/query/state.rs | 6 +- 4 files changed, 34 insertions(+), 55 deletions(-) rename crates/bevy_ecs/src/{query/default_filters.rs => entity_disabling.rs} (62%) diff --git a/crates/bevy_ecs/src/query/default_filters.rs b/crates/bevy_ecs/src/entity_disabling.rs similarity index 62% rename from crates/bevy_ecs/src/query/default_filters.rs rename to crates/bevy_ecs/src/entity_disabling.rs index 3e50caf81bb2b..3ccafc7b7d3cc 100644 --- a/crates/bevy_ecs/src/query/default_filters.rs +++ b/crates/bevy_ecs/src/entity_disabling.rs @@ -1,3 +1,26 @@ +//! Types for entity disabling. +//! +//! Disabled entities do not show up in queries unless the query explicitly mentions them. +//! +//! If for example we have `Disabled` as an entity disabling component, when you add `Disabled` +//! to an entity, the entity will only be visible to queries with a filter like +//! [`With`]`` or query data like [`Has`]``. +//! +//! ### Note +//! +//! Currently only queries for which the cache is built after enabling a filter will have entities +//! with those components filtered. As a result, they should generally only be modified before the +//! app starts. +//! +//! Because filters are applied to all queries they can have performance implication for +//! the enire [`World`], especially when they cause queries to mix sparse and table components. +//! See [`Query` performance] for more info. +//! +//! [`With`]: crate::prelude::With +//! [`Has`]: crate::prelude::Has +//! [`World`]: crate::prelude::World +//! [`Query` performance]: crate::prelude::Query#performance + use crate as bevy_ecs; use crate::{ component::{ComponentId, Components, StorageType}, @@ -6,54 +29,7 @@ use crate::{ use bevy_ecs_macros::Resource; /// The default filters for all queries, these are used to globally exclude entities from queries. -/// Default filters are applied to any queries that does not explicitly mention that component. -/// -/// If for example the component `Disabled` is registered here, entities with this component will -/// only be visible to a [`Query`](crate::prelude::Query) containing something like -/// [`With`](crate::prelude::With) or [`Has`](crate::prelude::Has). -/// See below for a more detailed example. -/// -/// These filters are only applied to queries whose cache is generated after updating this resource, -/// As a result, this resource should generally only be modified before the app starts (typically -/// during plugin construction) -/// -/// Because these filters are applied to all queries, the storage type of the component has -/// implications for the entire app. See [`Query` performance] for more info. -/// -/// ### Example -/// -/// ```rust -/// # use bevy_ecs::{prelude::*, query::DefaultQueryFilters}; -/// # #[derive(Component)] -/// # struct Disabled; -/// let mut world = World::new(); -/// let mut filters = DefaultQueryFilters::default(); -/// filters.set_disabled(world.init_component::()); -/// world.insert_resource(filters); -/// -/// // This entity is not Disabled, so most queries will see it -/// let entity_a = world.spawn_empty().id(); -/// -/// // This entity has Disabled, so most queries won't see it -/// let entity_b = world.spawn(Disabled).id(); -/// -/// // This query does not mention either of the markers, so it only gets entity_a -/// let mut query = world.query::(); -/// assert_eq!(1, query.iter(&world).count()); -/// assert_eq!(entity_a, query.get_single(&world).unwrap()); -/// -/// // This query only wants entities that are Disabled, thus it only sees entity_b -/// let mut query = world.query_filtered::>(); -/// assert_eq!(1, query.iter(&world).count()); -/// assert_eq!(entity_b, query.get_single(&world).unwrap()); -/// -/// // This also works for query data -/// let mut query = world.query::<(Entity, Has)>(); -/// assert_eq!(2, query.iter(&world).count()); -/// assert_eq!(vec![(entity_a, false), (entity_b, true)], query.iter(&world).collect::>()); -/// ``` -/// -/// [`Query` performance]: crate::prelude::Query#performance +/// See the [module docs](crate::entity_disabling) for more info. #[derive(Resource, Default, Debug)] #[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))] pub struct DefaultQueryFilters { @@ -61,8 +37,12 @@ pub struct DefaultQueryFilters { } impl DefaultQueryFilters { + #[cfg_attr( + not(test), + expect(dead_code, reason = "No Disabled component exist yet") + )] /// Set the [`ComponentId`] for the entity disabling marker - pub fn set_disabled(&mut self, component_id: ComponentId) -> Option<()> { + pub(crate) fn set_disabled(&mut self, component_id: ComponentId) -> Option<()> { if self.disabled.is_some() { return None; } @@ -70,7 +50,7 @@ impl DefaultQueryFilters { Some(()) } - /// Get an iterator over all default filter components + /// Get an iterator over all currently enabled filter components pub fn ids(&self) -> impl Iterator { [self.disabled].into_iter().flatten() } diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index a08a62053e107..f765c9092c1ee 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -19,6 +19,7 @@ pub mod bundle; pub mod change_detection; pub mod component; pub mod entity; +pub mod entity_disabling; pub mod event; pub mod identifier; pub mod intern; diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index c84fbf7712516..f1719d8ba5292 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -2,7 +2,6 @@ mod access; mod builder; -mod default_filters; mod error; mod fetch; mod filter; @@ -14,7 +13,6 @@ mod world_query; pub use access::*; pub use bevy_ecs_macros::{QueryData, QueryFilter}; pub use builder::*; -pub use default_filters::DefaultQueryFilters; pub use error::*; pub use fetch::*; pub use filter::*; diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index fca7e78e3aa9d..2dc7ad94cc20f 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -3,10 +3,10 @@ use crate::{ batching::BatchingStrategy, component::{ComponentId, Tick}, entity::Entity, + entity_disabling::DefaultQueryFilters, prelude::FromWorld, query::{ - default_filters::DefaultQueryFilters, Access, DebugCheckedUnwrap, FilteredAccess, - QueryCombinationIter, QueryIter, QueryParIter, + Access, DebugCheckedUnwrap, FilteredAccess, QueryCombinationIter, QueryIter, QueryParIter, }, storage::{SparseSetIndex, TableId}, world::{unsafe_world_cell::UnsafeWorldCell, World, WorldId}, @@ -1746,7 +1746,7 @@ impl From> for QueryState Date: Wed, 2 Oct 2024 16:20:38 +0200 Subject: [PATCH 13/15] Fix new test failures --- crates/bevy_ecs/src/query/state.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 1dfa32f4a03c9..9a8f3f0c11caa 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -2178,7 +2178,7 @@ mod tests { world.spawn(C(0)); let mut df = DefaultQueryFilters::default(); - df.set_disabled(world.init_component::()); + df.set_disabled(world.register_component::()); world.insert_resource(df); // Without only matches the first entity @@ -2218,7 +2218,7 @@ mod tests { assert_eq!(3, query.iter(&world).count()); let mut df = DefaultQueryFilters::default(); - df.set_disabled(world.init_component::()); + df.set_disabled(world.register_component::()); world.insert_resource(df); let mut query = QueryState::<()>::new(&mut world); @@ -2228,7 +2228,7 @@ mod tests { assert_eq!(1, query.iter(&world).count()); let mut df = DefaultQueryFilters::default(); - df.set_disabled(world.init_component::
()); + df.set_disabled(world.register_component::
()); world.insert_resource(df); let mut query = QueryState::<()>::new(&mut world); From 63331ef2f1cbad882aba1db9e71de7385f96a5a0 Mon Sep 17 00:00:00 2001 From: NiseVoid Date: Mon, 20 Jan 2025 22:13:09 +0100 Subject: [PATCH 14/15] Fixes after merge --- crates/bevy_ecs/src/entity_disabling.rs | 5 ++++- crates/bevy_ecs/src/query/state.rs | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/entity_disabling.rs b/crates/bevy_ecs/src/entity_disabling.rs index 3ccafc7b7d3cc..80d78127eeed9 100644 --- a/crates/bevy_ecs/src/entity_disabling.rs +++ b/crates/bevy_ecs/src/entity_disabling.rs @@ -67,14 +67,17 @@ impl DefaultQueryFilters { self.ids().all(|component_id| { components .get_info(component_id) - .map_or(false, |info| info.storage_type() == StorageType::Table) + .is_some_and(|info| info.storage_type() == StorageType::Table) }) } } #[cfg(test)] mod tests { + use std::vec; + use super::*; + use alloc::vec::Vec; #[test] fn test_set_filters() { diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index b84ef247b795b..e0e0856fd2369 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -215,7 +215,7 @@ impl QueryState { fn new_uninitialized(world: &mut World) -> Self { let fetch_state = D::init_state(world); let filter_state = F::init_state(world); - Self::from_states_uninitialized(world.id(), fetch_state, filter_state) + Self::from_states_uninitialized(world, fetch_state, filter_state) } /// Creates a new [`QueryState`] but does not populate it with the matched results from the World yet @@ -226,7 +226,7 @@ impl QueryState { let fetch_state = D::get_state(world.components())?; let filter_state = F::get_state(world.components())?; Some(Self::from_states_uninitialized( - world.id(), + world, fetch_state, filter_state, )) @@ -237,7 +237,7 @@ impl QueryState { /// `new_archetype` and its variants must be called on all of the World's archetypes before the /// state can return valid query results. fn from_states_uninitialized( - world_id: WorldId, + world: &World, fetch_state: ::State, filter_state: ::State, ) -> Self { @@ -264,7 +264,7 @@ impl QueryState { } Self { - world_id, + world_id: world.id(), archetype_generation: ArchetypeGeneration::initial(), matched_storage_ids: Vec::new(), is_dense, From 74d5364afb719eb5a7c956bc5c278616041924b7 Mon Sep 17 00:00:00 2001 From: NiseVoid Date: Mon, 20 Jan 2025 22:39:39 +0100 Subject: [PATCH 15/15] No, not that vec! --- crates/bevy_ecs/src/entity_disabling.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/entity_disabling.rs b/crates/bevy_ecs/src/entity_disabling.rs index 80d78127eeed9..449ae3d7177c8 100644 --- a/crates/bevy_ecs/src/entity_disabling.rs +++ b/crates/bevy_ecs/src/entity_disabling.rs @@ -74,10 +74,9 @@ impl DefaultQueryFilters { #[cfg(test)] mod tests { - use std::vec; use super::*; - use alloc::vec::Vec; + use alloc::{vec, vec::Vec}; #[test] fn test_set_filters() {