From 0e8b68952604430012d7668a0423934077cce857 Mon Sep 17 00:00:00 2001 From: Alice I Cecile Date: Sun, 2 Feb 2025 13:38:43 -0800 Subject: [PATCH 01/16] WIP: skeleton for `RelatedTo` filter --- .../src/relationship/relationship_query.rs | 198 ++++++++++++++++++ 1 file changed, 198 insertions(+) diff --git a/crates/bevy_ecs/src/relationship/relationship_query.rs b/crates/bevy_ecs/src/relationship/relationship_query.rs index cc7f66fb3a064..d44c3664b23dc 100644 --- a/crates/bevy_ecs/src/relationship/relationship_query.rs +++ b/crates/bevy_ecs/src/relationship/relationship_query.rs @@ -1,8 +1,14 @@ +use core::marker::PhantomData; + use crate::{ + archetype::Archetype, + component::Tick, entity::Entity, query::{QueryData, QueryFilter, WorldQuery}, relationship::{Relationship, RelationshipTarget}, + storage::{Table, TableRow}, system::Query, + world::unsafe_world_cell::UnsafeWorldCell, }; use alloc::collections::VecDeque; use smallvec::SmallVec; @@ -270,3 +276,195 @@ where self.next } } + +/// A [`QueryFilter`] type that filters for entities that are related via `R` to an entity that matches `F`. +pub struct RelatedTo { + _relationship: PhantomData, + _filter: PhantomData, +} + +unsafe impl WorldQuery for RelatedTo { + type Item<'a> = ::Item<'a>; + + type Fetch<'a> = RelatedToFetch<'a, R, F>; + + type State = RelatedToState; + + fn shrink<'wlong: 'wshort, 'wshort>(item: Self::Item<'wlong>) -> Self::Item<'wshort> { + item + } + + fn shrink_fetch<'wlong: 'wshort, 'wshort>(fetch: Self::Fetch<'wlong>) -> Self::Fetch<'wshort> { + fetch + } + + unsafe fn init_fetch<'w>( + world: UnsafeWorldCell<'w>, + state: &Self::State, + last_run: Tick, + this_run: Tick, + ) -> Self::Fetch<'w> { + RelatedToFetch { + relation_fetch: <&'static R as WorldQuery>::init_fetch( + world, + &state.relation_state, + last_run, + this_run, + ), + filter_fetch: ::init_fetch( + world, + &state.filter_state, + last_run, + this_run, + ), + } + } + + const IS_DENSE: bool = ::IS_DENSE & <&R as WorldQuery>::IS_DENSE; + + unsafe fn set_archetype<'w>( + fetch: &mut Self::Fetch<'w>, + state: &Self::State, + archetype: &'w Archetype, + table: &'w Table, + ) { + <&'static R as WorldQuery>::set_archetype( + &mut fetch.relation_fetch, + &state.relation_state, + archetype, + table, + ); + ::set_archetype( + &mut fetch.filter_fetch, + &state.filter_state, + archetype, + table, + ); + } + + unsafe fn set_table<'w>(fetch: &mut Self::Fetch<'w>, state: &Self::State, table: &'w Table) { + <&'static R as WorldQuery>::set_table( + &mut fetch.relation_fetch, + &state.relation_state, + table, + ); + ::set_table(&mut fetch.filter_fetch, &state.filter_state, table); + } + + unsafe fn fetch<'w>( + fetch: &mut Self::Fetch<'w>, + entity: Entity, + table_row: TableRow, + ) -> Self::Item<'w> { + // Look up the relationship + let relation = + <&'static R as WorldQuery>::fetch(&mut fetch.relation_fetch, entity, table_row); + // Then figure out what the related entity is + let related_entity = relation.get(); + + // Finally, check if the related entity matches the filter + ::fetch(&mut fetch.filter_fetch, related_entity, table_row) + } + + fn update_component_access( + state: &Self::State, + access: &mut crate::query::FilteredAccess, + ) { + <&'static R as WorldQuery>::update_component_access(&state.relation_state, access); + ::update_component_access(&state.filter_state, access); + } + + fn init_state(world: &mut crate::prelude::World) -> Self::State { + RelatedToState { + relation_state: <&'static R as WorldQuery>::init_state(world), + filter_state: ::init_state(world), + } + } + + fn get_state(components: &crate::component::Components) -> Option { + Some(RelatedToState { + relation_state: <&'static R as WorldQuery>::get_state(components)?, + filter_state: ::get_state(components)?, + }) + } + + fn matches_component_set( + state: &Self::State, + set_contains_id: &impl Fn(crate::component::ComponentId) -> bool, + ) -> bool { + // We need to look at both the relationship and the filter components, + // but they do not need to be on the same entity. + // As a result, we use an OR operation, rather than the AND operation used in other WorldQuery implementations. + <&'static R as WorldQuery>::matches_component_set(&state.relation_state, set_contains_id) + || ::matches_component_set(&state.filter_state, set_contains_id) + } +} + +unsafe impl QueryFilter for RelatedTo { + // The information about whether or not a related entity matches the filter + // varies between entities found in the same archetype, + // so rapidly pre-computing the length of the filtered set is not possible. + const IS_ARCHETYPAL: bool = false; + + unsafe fn filter_fetch( + fetch: &mut Self::Fetch<'_>, + entity: Entity, + table_row: TableRow, + ) -> bool { + todo!("How does this differ from `fetch`?") + } +} + +/// The [`WorldQuery::Fetch`] type for [`RelatedTo`]. +/// +/// This is used internally to implement [`WorldQuery`] for [`RelatedTo`]. +pub struct RelatedToFetch<'w, R: Relationship, F: QueryFilter> { + /// The fetch for the relationship component, + /// used to look up the target entity. + relation_fetch: <&'static R as WorldQuery>::Fetch<'w>, + /// The fetch for the filter component, + /// used to determine if the target entity matches the filter. + filter_fetch: ::Fetch<'w>, +} + +impl<'w, R: Relationship, F: QueryFilter> Clone for RelatedToFetch<'w, R, F> { + fn clone(&self) -> Self { + Self { + relation_fetch: self.relation_fetch.clone(), + filter_fetch: self.filter_fetch.clone(), + } + } +} + +/// The [`WorldQuery::State`] type for [`RelatedTo`]. +/// +/// This is used internally to implement [`WorldQuery`] for [`RelatedTo`]. +pub struct RelatedToState { + /// The state for the relationship component, + /// used to look up the target entity. + relation_state: <&'static R as WorldQuery>::State, + /// The state for the filter component, + /// used to determine if the target entity matches the filter. + filter_state: ::State, +} + +#[cfg(test)] +mod tests { + use super::RelatedTo; + use crate as bevy_ecs; + use crate::prelude::{ChildOf, Component, Entity, With, World}; + + #[derive(Component)] + struct A; + + #[test] + fn related_to() { + let mut world = World::default(); + let entity = world.spawn(A).id(); + let child = world.spawn(ChildOf(entity)).id(); + let mut query_state = world.query_filtered::>>(); + let fetched_child = query_state.iter(&world).next().unwrap(); + + assert_eq!(child, fetched_child); + } +} From 4233648abcda9c4de1594cf3e6f11266bfd712fd Mon Sep 17 00:00:00 2001 From: Alice I Cecile Date: Sun, 2 Feb 2025 14:10:19 -0800 Subject: [PATCH 02/16] Add a simple doc test --- .../src/relationship/relationship_query.rs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/crates/bevy_ecs/src/relationship/relationship_query.rs b/crates/bevy_ecs/src/relationship/relationship_query.rs index d44c3664b23dc..a7258545db83c 100644 --- a/crates/bevy_ecs/src/relationship/relationship_query.rs +++ b/crates/bevy_ecs/src/relationship/relationship_query.rs @@ -278,6 +278,36 @@ where } /// A [`QueryFilter`] type that filters for entities that are related via `R` to an entity that matches `F`. +/// +/// This works by looking up the related entity using the `R` relationship component, +/// then checking if that related entity matches the filter given in `F`. +/// +/// # Examples +/// +/// ```rust +/// use bevy_ecs::prelude::*; +/// use bevy_ecs::system::RunSystemOnce; +/// +/// #[derive(Component)] +/// struct A; +/// +/// let mut world = World::new(); +/// let parent = world.spawn(A).id(); +/// let child = world.spawn(ChildOf(parent))).id(); +/// let unrelated = world.spawn_empty().id(); +/// let grandchild = world.spawn(ChildOf(child)).id(); +/// +/// fn iterate_related_to_a(query: Query>>) { +/// for entity in query.iter() { +/// // Only the child entity should be iterated; +/// // the parent, unrelated and chrandchild entities should be skipped, +/// // as they are not related to an entity with the `A` component. +/// assert_eq!(entity, child); +/// } +/// } +/// +/// world.run_system_once(iterate_related_to_a); +/// ``` pub struct RelatedTo { _relationship: PhantomData, _filter: PhantomData, From 41879ca690c875811753d7e1fdf9aa7dae584544 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 2 Feb 2025 17:04:27 -0800 Subject: [PATCH 03/16] Fix compilation issues --- crates/bevy_ecs/src/relationship/relationship_query.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/relationship/relationship_query.rs b/crates/bevy_ecs/src/relationship/relationship_query.rs index d44c3664b23dc..5d7e4482cc32c 100644 --- a/crates/bevy_ecs/src/relationship/relationship_query.rs +++ b/crates/bevy_ecs/src/relationship/relationship_query.rs @@ -291,11 +291,14 @@ unsafe impl WorldQuery for RelatedTo { type State = RelatedToState; fn shrink<'wlong: 'wshort, 'wshort>(item: Self::Item<'wlong>) -> Self::Item<'wshort> { - item + ::shrink(item) } fn shrink_fetch<'wlong: 'wshort, 'wshort>(fetch: Self::Fetch<'wlong>) -> Self::Fetch<'wshort> { - fetch + RelatedToFetch { + relation_fetch: <&'static R as WorldQuery>::shrink_fetch(fetch.relation_fetch), + filter_fetch: ::shrink_fetch(fetch.filter_fetch), + } } unsafe fn init_fetch<'w>( From b3a86df7baa3081bbbf9e0dc03e5b0d364f0e6b7 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 2 Feb 2025 17:12:26 -0800 Subject: [PATCH 04/16] Complete filter_fetch method --- .../src/relationship/relationship_query.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/relationship/relationship_query.rs b/crates/bevy_ecs/src/relationship/relationship_query.rs index 5d7e4482cc32c..bb53c271a3039 100644 --- a/crates/bevy_ecs/src/relationship/relationship_query.rs +++ b/crates/bevy_ecs/src/relationship/relationship_query.rs @@ -414,7 +414,23 @@ unsafe impl QueryFilter for RelatedTo { entity: Entity, table_row: TableRow, ) -> bool { - todo!("How does this differ from `fetch`?") + // First, look up the relationship + // SAFETY: the caller promises that we only call this method after WorldQuery::set_table or WorldQuery::set_archetype, + // and that the entity and table_row are in the range of the current table and archetype. + // No simultaneous conflicting component accesses exist, as both parts of the filter are read-only. + let relation = unsafe { + <&'static R as WorldQuery>::fetch(&mut fetch.relation_fetch, entity, table_row) + }; + + // Then figure out what the related entity is + let related_entity = relation.get(); + + // Finally, check if the related entity matches the filter + // SAFETY: the safety invariants for calling `filter_fetch` on `F` are upheld by the caller, + // as they are the same as the safety invariants for calling this method + unsafe { + ::filter_fetch(&mut fetch.filter_fetch, related_entity, table_row) + } } } From 1e8ea63ac92a999a5e75a63887176e2942b79a00 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 2 Feb 2025 17:23:03 -0800 Subject: [PATCH 05/16] Add required serde_derive feature flag --- crates/bevy_ecs/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml index fc7e6d20b86dd..0106286c52d55 100644 --- a/crates/bevy_ecs/Cargo.toml +++ b/crates/bevy_ecs/Cargo.toml @@ -119,6 +119,7 @@ disqualified = { version = "1.0", default-features = false } fixedbitset = { version = "0.5", default-features = false } serde = { version = "1", default-features = false, features = [ "alloc", + "serde_derive", ], optional = true } thiserror = { version = "2", default-features = false } derive_more = { version = "1", default-features = false, features = [ From b2d0332b1c19b4877dbe6d82d5213055e9ddb1f6 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 2 Feb 2025 18:07:13 -0800 Subject: [PATCH 06/16] Test with empty filter --- .../src/relationship/relationship_query.rs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/crates/bevy_ecs/src/relationship/relationship_query.rs b/crates/bevy_ecs/src/relationship/relationship_query.rs index 01019e56e6e5e..d16c204aa7981 100644 --- a/crates/bevy_ecs/src/relationship/relationship_query.rs +++ b/crates/bevy_ecs/src/relationship/relationship_query.rs @@ -506,6 +506,27 @@ mod tests { #[derive(Component)] struct A; + #[test] + fn related_to_empty_filter() { + let mut world = World::default(); + let parent = world.spawn_empty().id(); + let child = world.spawn(ChildOf(parent)).id(); + let _unrelated = world.spawn_empty().id(); + let grandchild = world.spawn(ChildOf(child)).id(); + + let mut query_state = world.query_filtered::>(); + for matching_entity in query_state.iter(&world) { + let matches_child_or_grandchild = + matching_entity == child || matching_entity == grandchild; + assert!( + matches_child_or_grandchild, + "Entity {matching_entity} should have a parent" + ); + } + + assert_eq!(query_state.iter(&world).count(), 2); + } + #[test] fn related_to() { let mut world = World::default(); From 55fbd1380462748bae524b55e225715b9b07ded7 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 2 Feb 2025 18:15:12 -0800 Subject: [PATCH 07/16] Test for more query variants --- .../src/relationship/relationship_query.rs | 45 +++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/relationship/relationship_query.rs b/crates/bevy_ecs/src/relationship/relationship_query.rs index d16c204aa7981..7b68b168a0e01 100644 --- a/crates/bevy_ecs/src/relationship/relationship_query.rs +++ b/crates/bevy_ecs/src/relationship/relationship_query.rs @@ -501,7 +501,7 @@ pub struct RelatedToState { mod tests { use super::RelatedTo; use crate as bevy_ecs; - use crate::prelude::{ChildOf, Component, Entity, With, World}; + use crate::prelude::{Changed, ChildOf, Component, Entity, With, Without, World}; #[derive(Component)] struct A; @@ -528,13 +528,50 @@ mod tests { } #[test] - fn related_to() { + fn related_to_with() { let mut world = World::default(); - let entity = world.spawn(A).id(); - let child = world.spawn(ChildOf(entity)).id(); + let parent = world.spawn(A).id(); + let child = world.spawn(ChildOf(parent)).id(); let mut query_state = world.query_filtered::>>(); let fetched_child = query_state.iter(&world).next().unwrap(); assert_eq!(child, fetched_child); } + + #[test] + fn related_to_changed() { + let mut world = World::default(); + let parent = world.spawn(A).id(); + let child = world.spawn(ChildOf(parent)).id(); + // Changed is true when entities are first added, so this should match + let mut query_state = world.query_filtered::>>(); + let fetched_child = query_state.iter(&world).next().unwrap(); + + assert_eq!(child, fetched_child); + } + + #[test] + fn related_to_without() { + let mut world = World::default(); + let parent = world.spawn_empty().id(); + let child = world.spawn(ChildOf(parent)).id(); + let mut query_state = world.query_filtered::>>(); + let fetched_child = query_state.iter(&world).next().unwrap(); + + assert_eq!(child, fetched_child); + } + + #[test] + fn related_to_impossible_filter() { + let mut world = World::default(); + let parent = world.spawn_empty().id(); + let child = world.spawn(ChildOf(parent)).id(); + // No entity could possibly match this filter: + // it requires entities to both have and not have the `A` component. + let mut query_state = + world.query_filtered::, Without)>>(); + let maybe_fetched_child = query_state.get(&world, child); + + assert!(maybe_fetched_child.is_err()); + } } From 4663a60b1c6cc272ce602eed7655e45a85b953c4 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 2 Feb 2025 18:22:12 -0800 Subject: [PATCH 08/16] Unsafe method in unsafe fn blocks --- crates/bevy_ecs/src/relationship/relationship_query.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/relationship/relationship_query.rs b/crates/bevy_ecs/src/relationship/relationship_query.rs index 7b68b168a0e01..d9898e4efcd3f 100644 --- a/crates/bevy_ecs/src/relationship/relationship_query.rs +++ b/crates/bevy_ecs/src/relationship/relationship_query.rs @@ -390,13 +390,16 @@ unsafe impl WorldQuery for RelatedTo { table_row: TableRow, ) -> Self::Item<'w> { // Look up the relationship - let relation = - <&'static R as WorldQuery>::fetch(&mut fetch.relation_fetch, entity, table_row); + // SAFETY: the safety requirements for calling `fetch` on `R` are a subset of the safety requirements for calling this method + let relation = unsafe { + <&'static R as WorldQuery>::fetch(&mut fetch.relation_fetch, entity, table_row) + }; // Then figure out what the related entity is let related_entity = relation.get(); // Finally, check if the related entity matches the filter - ::fetch(&mut fetch.filter_fetch, related_entity, table_row) + // SAFETY: the safety requirements for calling `fetch` on `F` are a subset of the safety requirements for calling this method + unsafe { ::fetch(&mut fetch.filter_fetch, related_entity, table_row) } } fn update_component_access( From 14748a9e4d79855fb8cdd7e2be51de82349371dc Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 2 Feb 2025 18:25:47 -0800 Subject: [PATCH 09/16] Future proof Fetch and State type names --- .../src/relationship/relationship_query.rs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/bevy_ecs/src/relationship/relationship_query.rs b/crates/bevy_ecs/src/relationship/relationship_query.rs index d9898e4efcd3f..0d31d7132e658 100644 --- a/crates/bevy_ecs/src/relationship/relationship_query.rs +++ b/crates/bevy_ecs/src/relationship/relationship_query.rs @@ -316,16 +316,16 @@ pub struct RelatedTo { unsafe impl WorldQuery for RelatedTo { type Item<'a> = ::Item<'a>; - type Fetch<'a> = RelatedToFetch<'a, R, F>; + type Fetch<'a> = RelatedFilterFetch<'a, R, F>; - type State = RelatedToState; + type State = RelatedFilterState; fn shrink<'wlong: 'wshort, 'wshort>(item: Self::Item<'wlong>) -> Self::Item<'wshort> { ::shrink(item) } fn shrink_fetch<'wlong: 'wshort, 'wshort>(fetch: Self::Fetch<'wlong>) -> Self::Fetch<'wshort> { - RelatedToFetch { + RelatedFilterFetch { relation_fetch: <&'static R as WorldQuery>::shrink_fetch(fetch.relation_fetch), filter_fetch: ::shrink_fetch(fetch.filter_fetch), } @@ -337,7 +337,7 @@ unsafe impl WorldQuery for RelatedTo { last_run: Tick, this_run: Tick, ) -> Self::Fetch<'w> { - RelatedToFetch { + RelatedFilterFetch { relation_fetch: <&'static R as WorldQuery>::init_fetch( world, &state.relation_state, @@ -411,14 +411,14 @@ unsafe impl WorldQuery for RelatedTo { } fn init_state(world: &mut crate::prelude::World) -> Self::State { - RelatedToState { + RelatedFilterState { relation_state: <&'static R as WorldQuery>::init_state(world), filter_state: ::init_state(world), } } fn get_state(components: &crate::component::Components) -> Option { - Some(RelatedToState { + Some(RelatedFilterState { relation_state: <&'static R as WorldQuery>::get_state(components)?, filter_state: ::get_state(components)?, }) @@ -467,10 +467,10 @@ unsafe impl QueryFilter for RelatedTo { } } -/// The [`WorldQuery::Fetch`] type for [`RelatedTo`]. +/// The [`WorldQuery::Fetch`] type for [`RelatedTo`] and similar types. /// -/// This is used internally to implement [`WorldQuery`] for [`RelatedTo`]. -pub struct RelatedToFetch<'w, R: Relationship, F: QueryFilter> { +/// This is used internally to implement [`WorldQuery`]. +pub struct RelatedFilterFetch<'w, R: Relationship, F: QueryFilter> { /// The fetch for the relationship component, /// used to look up the target entity. relation_fetch: <&'static R as WorldQuery>::Fetch<'w>, @@ -479,7 +479,7 @@ pub struct RelatedToFetch<'w, R: Relationship, F: QueryFilter> { filter_fetch: ::Fetch<'w>, } -impl<'w, R: Relationship, F: QueryFilter> Clone for RelatedToFetch<'w, R, F> { +impl<'w, R: Relationship, F: QueryFilter> Clone for RelatedFilterFetch<'w, R, F> { fn clone(&self) -> Self { Self { relation_fetch: self.relation_fetch.clone(), @@ -488,10 +488,10 @@ impl<'w, R: Relationship, F: QueryFilter> Clone for RelatedToFetch<'w, R, F> { } } -/// The [`WorldQuery::State`] type for [`RelatedTo`]. +/// The [`WorldQuery::State`] type for [`RelatedTo`] and similar types.. /// -/// This is used internally to implement [`WorldQuery`] for [`RelatedTo`]. -pub struct RelatedToState { +/// This is used internally to implement [`WorldQuery`]. +pub struct RelatedFilterState { /// The state for the relationship component, /// used to look up the target entity. relation_state: <&'static R as WorldQuery>::State, From c2eff392a9d06c97de8d87f9f9c67a311bbdfb87 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 2 Feb 2025 18:49:25 -0800 Subject: [PATCH 10/16] Add another test --- .../src/relationship/relationship_query.rs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/crates/bevy_ecs/src/relationship/relationship_query.rs b/crates/bevy_ecs/src/relationship/relationship_query.rs index 0d31d7132e658..82552d7053b9a 100644 --- a/crates/bevy_ecs/src/relationship/relationship_query.rs +++ b/crates/bevy_ecs/src/relationship/relationship_query.rs @@ -577,4 +577,25 @@ mod tests { assert!(maybe_fetched_child.is_err()); } + + #[test] + fn related_to_compoound_filter() { + let mut world = World::default(); + let parent = world.spawn(A).id(); + let child_with = world.spawn((A, ChildOf(parent))).id(); + let child_without = world.spawn(ChildOf(parent)).id(); + + // We're double-checking checking the behavior of the `RelatedTo` filter when combined with other filters. + let mut query_state = + world.query_filtered::, RelatedTo>)>(); + + // This child has the `A` component and is a child of the parent, so it should be fetched. + let maybe_fetched_child_with = query_state.get(&world, child_with); + assert!(maybe_fetched_child_with.is_ok()); + + // This child does not have the `A` component but is not a child of the parent, so it should not be fetched, + // even though its parent has the `A` component. + let maybe_fetched_child_without = query_state.get(&world, child_without); + assert!(maybe_fetched_child_without.is_err()); + } } From a5666bf6350ed3165b85e2df89e25e3bb14f925a Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 2 Feb 2025 18:55:06 -0800 Subject: [PATCH 11/16] Add variant tests for debugging --- .../src/relationship/relationship_query.rs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/crates/bevy_ecs/src/relationship/relationship_query.rs b/crates/bevy_ecs/src/relationship/relationship_query.rs index 82552d7053b9a..a50060e5e08b6 100644 --- a/crates/bevy_ecs/src/relationship/relationship_query.rs +++ b/crates/bevy_ecs/src/relationship/relationship_query.rs @@ -509,6 +509,9 @@ mod tests { #[derive(Component)] struct A; + #[derive(Component)] + struct B; + #[test] fn related_to_empty_filter() { let mut world = World::default(); @@ -541,6 +544,32 @@ mod tests { assert_eq!(child, fetched_child); } + #[test] + fn related_to_same_with() { + let mut world = World::default(); + let parent = world.spawn(A).id(); + let child = world.spawn((A, ChildOf(parent))).id(); + let mut query_state = + world.query_filtered::, RelatedTo>)>(); + let fetched_child = query_state.iter(&world).next().unwrap(); + + assert_eq!(child, fetched_child); + } + + // If this test fails but the one above passes, + // the RelatedTo filter is probably not matching enough archetypes. + #[test] + fn related_to_different_with() { + let mut world = World::default(); + let parent = world.spawn(A).id(); + let child = world.spawn((B, ChildOf(parent))).id(); + let mut query_state = + world.query_filtered::, RelatedTo>)>(); + let fetched_child = query_state.iter(&world).next().unwrap(); + + assert_eq!(child, fetched_child); + } + #[test] fn related_to_changed() { let mut world = World::default(); From 220f42e41efac6c3dfb150f119e9f0138d9e4b20 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 2 Feb 2025 18:57:48 -0800 Subject: [PATCH 12/16] Also call set_table for &R --- crates/bevy_ecs/src/relationship/relationship_query.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_ecs/src/relationship/relationship_query.rs b/crates/bevy_ecs/src/relationship/relationship_query.rs index a50060e5e08b6..8895bdcfc6373 100644 --- a/crates/bevy_ecs/src/relationship/relationship_query.rs +++ b/crates/bevy_ecs/src/relationship/relationship_query.rs @@ -381,6 +381,7 @@ unsafe impl WorldQuery for RelatedTo { &state.relation_state, table, ); + <&R as WorldQuery>::set_table(&mut fetch.relation_fetch, &state.relation_state, table); ::set_table(&mut fetch.filter_fetch, &state.filter_state, table); } From 602337212bc03fa0a3f2b74a28e42162a1714860 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 2 Feb 2025 19:28:11 -0800 Subject: [PATCH 13/16] Set RelatedTo::Item = bool to match Changed --- .../src/relationship/relationship_query.rs | 28 ++++++------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/crates/bevy_ecs/src/relationship/relationship_query.rs b/crates/bevy_ecs/src/relationship/relationship_query.rs index 8895bdcfc6373..8ff933c23aa5f 100644 --- a/crates/bevy_ecs/src/relationship/relationship_query.rs +++ b/crates/bevy_ecs/src/relationship/relationship_query.rs @@ -314,14 +314,15 @@ pub struct RelatedTo { } unsafe impl WorldQuery for RelatedTo { - type Item<'a> = ::Item<'a>; + // The filter is non-archetypal, and must be evaluated for each entity. + type Item<'a> = bool; type Fetch<'a> = RelatedFilterFetch<'a, R, F>; type State = RelatedFilterState; fn shrink<'wlong: 'wshort, 'wshort>(item: Self::Item<'wlong>) -> Self::Item<'wshort> { - ::shrink(item) + item } fn shrink_fetch<'wlong: 'wshort, 'wshort>(fetch: Self::Fetch<'wlong>) -> Self::Fetch<'wshort> { @@ -400,7 +401,9 @@ unsafe impl WorldQuery for RelatedTo { // Finally, check if the related entity matches the filter // SAFETY: the safety requirements for calling `fetch` on `F` are a subset of the safety requirements for calling this method - unsafe { ::fetch(&mut fetch.filter_fetch, related_entity, table_row) } + unsafe { + ::filter_fetch(&mut fetch.filter_fetch, related_entity, table_row) + } } fn update_component_access( @@ -448,23 +451,8 @@ unsafe impl QueryFilter for RelatedTo { entity: Entity, table_row: TableRow, ) -> bool { - // First, look up the relationship - // SAFETY: the caller promises that we only call this method after WorldQuery::set_table or WorldQuery::set_archetype, - // and that the entity and table_row are in the range of the current table and archetype. - // No simultaneous conflicting component accesses exist, as both parts of the filter are read-only. - let relation = unsafe { - <&'static R as WorldQuery>::fetch(&mut fetch.relation_fetch, entity, table_row) - }; - - // Then figure out what the related entity is - let related_entity = relation.get(); - - // Finally, check if the related entity matches the filter - // SAFETY: the safety invariants for calling `filter_fetch` on `F` are upheld by the caller, - // as they are the same as the safety invariants for calling this method - unsafe { - ::filter_fetch(&mut fetch.filter_fetch, related_entity, table_row) - } + // SAFETY: safety requirements for calling `fetch` on `Self` are the same as the safety requirements for calling this method + unsafe { Self::fetch(fetch, entity, table_row) } } } From 04a3a9b30ef27c61075a2a9cff092a970a09156a Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 2 Feb 2025 19:29:08 -0800 Subject: [PATCH 14/16] Add comment to IS_DENSE --- crates/bevy_ecs/src/relationship/relationship_query.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_ecs/src/relationship/relationship_query.rs b/crates/bevy_ecs/src/relationship/relationship_query.rs index 8ff933c23aa5f..8140f2206f848 100644 --- a/crates/bevy_ecs/src/relationship/relationship_query.rs +++ b/crates/bevy_ecs/src/relationship/relationship_query.rs @@ -354,6 +354,7 @@ unsafe impl WorldQuery for RelatedTo { } } + // Both R and F must be dense for every archetype retrieved to correspond to a table. const IS_DENSE: bool = ::IS_DENSE & <&R as WorldQuery>::IS_DENSE; unsafe fn set_archetype<'w>( From e2d98475bd32cc1d72bb8a58817686743208390b Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 2 Feb 2025 19:41:02 -0800 Subject: [PATCH 15/16] Clarifying comment for relation_state type --- crates/bevy_ecs/src/relationship/relationship_query.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_ecs/src/relationship/relationship_query.rs b/crates/bevy_ecs/src/relationship/relationship_query.rs index 8140f2206f848..4b60f9e75536c 100644 --- a/crates/bevy_ecs/src/relationship/relationship_query.rs +++ b/crates/bevy_ecs/src/relationship/relationship_query.rs @@ -484,6 +484,7 @@ impl<'w, R: Relationship, F: QueryFilter> Clone for RelatedFilterFetch<'w, R, F> pub struct RelatedFilterState { /// The state for the relationship component, /// used to look up the target entity. + /// This type evaluates to [`ComponentId`](crate::components::ComponentId). relation_state: <&'static R as WorldQuery>::State, /// The state for the filter component, /// used to determine if the target entity matches the filter. From f66d81261530a62f71fcbd758755217d7893574d Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 2 Feb 2025 20:07:51 -0800 Subject: [PATCH 16/16] Add test and docs for nested RelatedTo filters --- .../src/relationship/relationship_query.rs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/crates/bevy_ecs/src/relationship/relationship_query.rs b/crates/bevy_ecs/src/relationship/relationship_query.rs index 4b60f9e75536c..6664ff43d7fc0 100644 --- a/crates/bevy_ecs/src/relationship/relationship_query.rs +++ b/crates/bevy_ecs/src/relationship/relationship_query.rs @@ -282,6 +282,7 @@ where /// This works by looking up the related entity using the `R` relationship component, /// then checking if that related entity matches the filter given in `F`. /// +/// /// # Examples /// /// ```rust @@ -308,6 +309,29 @@ where /// /// world.run_system_once(iterate_related_to_a); /// ``` +/// Note that type can be nested to filter for entities that have e.g. a grandparent with a certain component. +/// When defining particularly complex filters like this, type aliases for a specific choice of `R` can be helpful to improve legibility! +/// +/// ``` +/// use bevy_ecs::prelude::*; +/// use bevy_ecs::system::RunSystemOnce; +/// +/// #[derive(Component)] +/// struct A; +/// +/// type WithParent = RelatedTo>; +/// +/// let mut world = World::new(); +/// let parent = world.spawn(A).id(); +/// let child = world.spawn(ChildOf(parent))).id(); +/// let grandchild = world.spawn(ChildOf(child)).id(); +/// +/// let mut parent_query = world.query::>(); +/// assert!(parent_query.get(child).is_ok()); +/// +/// let mut grandparent_query = world.query::>>(); +/// assert!(grandparent_query.get(grandchild).is_ok()); +/// ``` pub struct RelatedTo { _relationship: PhantomData, _filter: PhantomData, @@ -618,4 +642,19 @@ mod tests { let maybe_fetched_child_without = query_state.get(&world, child_without); assert!(maybe_fetched_child_without.is_err()); } + + #[test] + fn related_to_nested() { + let mut world = World::default(); + let grandparent = world.spawn(A).id(); + let parent = world.spawn((B, ChildOf(grandparent))).id(); + let child = world.spawn(ChildOf(parent)).id(); + + // Look for entities that have a parent with A and a grandparent with B + let mut query_state = world + .query_filtered::, RelatedTo>)>>(); + assert!(query_state.get(&world, child).is_ok()); + assert!(query_state.get(&world, parent).is_err()); + assert!(query_state.get(&world, grandparent).is_err()); + } }