From 13513ccbb7233fa4f1a4003dec14145e0280a462 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Mon, 14 Jul 2025 20:53:45 -0400 Subject: [PATCH] Don't ignore default query filters for `EntityRef` or `EntityMut`. But do ignore them for `EntityRefExcept` or `EntityMutExcept` that mention the filtered component. --- crates/bevy_ecs/src/entity_disabling.rs | 55 ++++++++++++++++++++----- crates/bevy_ecs/src/query/access.rs | 13 +++++- 2 files changed, 57 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/src/entity_disabling.rs b/crates/bevy_ecs/src/entity_disabling.rs index 5d62011174..c0d1bde7f1 100644 --- a/crates/bevy_ecs/src/entity_disabling.rs +++ b/crates/bevy_ecs/src/entity_disabling.rs @@ -205,8 +205,9 @@ mod tests { use super::*; use crate::{ - prelude::World, + prelude::{EntityMut, EntityRef, World}, query::{Has, With}, + world::{EntityMutExcept, EntityRefExcept}, }; use alloc::{vec, vec::Vec}; @@ -278,30 +279,64 @@ mod tests { let mut world = World::new(); world.register_disabling_component::(); + // Use powers of two so we can uniquely identify the set of matching archetypes from the count. world.spawn_empty(); - world.spawn(Disabled); - world.spawn(CustomDisabled); - world.spawn((Disabled, CustomDisabled)); + world.spawn_batch((0..2).map(|_| Disabled)); + world.spawn_batch((0..4).map(|_| CustomDisabled)); + world.spawn_batch((0..8).map(|_| (Disabled, CustomDisabled))); let mut query = world.query::<()>(); assert_eq!(1, query.iter(&world).count()); - let mut query = world.query_filtered::<(), With>(); + let mut query = world.query::(); assert_eq!(1, query.iter(&world).count()); + let mut query = world.query::(); + assert_eq!(1, query.iter(&world).count()); + + let mut query = world.query::>(); + assert_eq!(1, query.iter(&world).count()); + + let mut query = world.query::>(); + assert_eq!(1, query.iter(&world).count()); + + let mut query = world.query_filtered::<(), With>(); + assert_eq!(2, query.iter(&world).count()); + let mut query = world.query::>(); - assert_eq!(2, query.iter(&world).count()); + assert_eq!(3, query.iter(&world).count()); + + let mut query = world.query::>(); + assert_eq!(3, query.iter(&world).count()); + + let mut query = world.query::>(); + assert_eq!(3, query.iter(&world).count()); let mut query = world.query_filtered::<(), With>(); - assert_eq!(1, query.iter(&world).count()); + assert_eq!(4, query.iter(&world).count()); let mut query = world.query::>(); - assert_eq!(2, query.iter(&world).count()); + assert_eq!(5, query.iter(&world).count()); let mut query = world.query_filtered::<(), (With, With)>(); - assert_eq!(1, query.iter(&world).count()); + assert_eq!(8, query.iter(&world).count()); let mut query = world.query::<(Has, Has)>(); - assert_eq!(4, query.iter(&world).count()); + assert_eq!(15, query.iter(&world).count()); + + // Some edge cases: + + // Ideally this would include entities with `Disabled`, + // but the access is indistinguishable from `EntityRef`. + let mut query = world.query::<(EntityRef, Option<&Disabled>)>(); + assert_eq!(1, query.iter(&world).count()); + + // This is even true if the component is explicitly mentioned in `EntityMutExcept`. + let mut query = world.query::<(EntityMutExcept, Option<&Disabled>)>(); + assert_eq!(1, query.iter(&world).count()); + + // But note that without `Option`, this adds a filter and does include disabled entities. + let mut query = world.query::<(EntityRef, &Disabled)>(); + assert_eq!(2, query.iter(&world).count()); } } diff --git a/crates/bevy_ecs/src/query/access.rs b/crates/bevy_ecs/src/query/access.rs index 0c5b29f715..d634d31c03 100644 --- a/crates/bevy_ecs/src/query/access.rs +++ b/crates/bevy_ecs/src/query/access.rs @@ -272,6 +272,13 @@ impl Access { .contains(index.sparse_set_index()) } + /// Returns `true` if this either has bounded access including this component + /// or unbounded access not including this component. + pub(crate) fn has_component_read_exception(&self, index: T) -> bool { + self.component_read_and_writes + .contains(index.sparse_set_index()) + } + /// Returns `true` if this can access any component. pub fn has_any_component_read(&self) -> bool { self.component_read_and_writes_inverted || !self.component_read_and_writes.is_clear() @@ -1227,7 +1234,11 @@ 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_component_read(index.clone()) + // Check whether this component is an exception. + // For normal queries, we want to treat access as a use, + // but for `EntityRefExcept` and `EntityMutExcept`, + // treat the exceptions as a use since they were mentioned explicitly. + self.access().has_component_read_exception(index.clone()) || self.access().has_archetypal(index.clone()) || self.filter_sets.iter().any(|f| { f.with.contains(index.sparse_set_index())