From dc00509950e5d8f00ea385dd29563165f8dde870 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Sun, 28 Apr 2024 14:26:36 -0700 Subject: [PATCH 1/8] Entity::components / components_mut / get_components / get_components_mut --- crates/bevy_ecs/src/world/entity_ref.rs | 59 ++++++++++++++++++- .../bevy_ecs/src/world/unsafe_world_cell.rs | 47 +++++++++++++++ examples/ecs/dynamic.rs | 2 +- 3 files changed, 104 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 5adfb2d302021..624d697142cf4 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -6,7 +6,7 @@ use crate::{ entity::{Entities, Entity, EntityLocation}, event::Event, observer::{Observer, Observers}, - query::Access, + query::{Access, QueryData, ReadOnlyQueryData}, removal_detection::RemovedComponentEvents, storage::Storages, system::IntoObserverSystem, @@ -156,6 +156,17 @@ impl<'w> EntityRef<'w> { // SAFETY: We have read-only access to all components of this entity. unsafe { self.0.get_by_id(component_id) } } + + /// Returns read-only components for the current entity that match the query `Q`. + pub fn components(&self) -> Q::Item<'_> { + self.get_components::().unwrap() + } + + /// Returns read-only components for the current entity that match the query `Q`. + pub fn get_components(&self) -> Option> { + // SAFETY: &mut self implies exclusive access for duration of returned value + unsafe { self.0.get_components::() } + } } impl<'w> From> for EntityRef<'w> { @@ -351,6 +362,28 @@ impl<'w> EntityMut<'w> { self.as_readonly().get() } + /// Returns read-only components for the current entity that match the query `Q`. + pub fn components(&self) -> Q::Item<'_> { + self.get_components::().unwrap() + } + + /// Returns components for the current entity that match the query `Q`. + pub fn components_mut(&mut self) -> Q::Item<'_> { + self.get_components_mut::().unwrap() + } + + /// Returns read-only components for the current entity that match the query `Q`. + pub fn get_components(&self) -> Option> { + // SAFETY: &mut self implies exclusive access for duration of returned value + unsafe { self.0.get_components::() } + } + + /// Returns components for the current entity that match the query `Q`. + pub fn get_components_mut(&mut self) -> Option> { + // SAFETY: &mut self implies exclusive access for duration of returned value + unsafe { self.0.get_components::() } + } + /// Consumes `self` and gets access to the component of type `T` with the /// world `'w` lifetime for the current entity. /// @@ -1878,7 +1911,7 @@ impl<'w> FilteredEntityRef<'w> { /// Returns an iterator over the component ids that are accessed by self. #[inline] - pub fn components(&self) -> impl Iterator + '_ { + pub fn accessed_components(&self) -> impl Iterator + '_ { self.access.component_reads_and_writes() } @@ -2135,7 +2168,7 @@ impl<'w> FilteredEntityMut<'w> { /// Returns an iterator over the component ids that are accessed by self. #[inline] - pub fn components(&self) -> impl Iterator + '_ { + pub fn accessed_components(&self) -> impl Iterator + '_ { self.access.component_reads_and_writes() } @@ -3115,4 +3148,24 @@ mod tests { assert!(e.get_mut_by_id(a_id).is_none()); assert!(e.get_change_ticks_by_id(a_id).is_none()); } + + #[test] + fn get_components() { + #[derive(Component, PartialEq, Eq, Debug)] + struct X(usize); + + #[derive(Component, PartialEq, Eq, Debug)] + struct Y(usize); + let mut world = World::default(); + let e1 = world.spawn((X(7), Y(10))).id(); + let e2 = world.spawn(X(8)).id(); + let e3 = world.spawn_empty().id(); + + assert_eq!( + Some((&X(7), &Y(10))), + world.entity(e1).get_components::<(&X, &Y)>() + ); + assert_eq!(None, world.entity(e2).get_components::<(&X, &Y)>()); + assert_eq!(None, world.entity(e3).get_components::<(&X, &Y)>()); + } } diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index 00b5349d80c1a..63e523bf13b29 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -11,6 +11,7 @@ use crate::{ entity::{Entities, Entity, EntityLocation}, observer::Observers, prelude::Component, + query::{DebugCheckedUnwrap, QueryData}, removal_detection::RemovedComponentEvents, storage::{Column, ComponentSparseSet, Storages}, system::{Res, Resource}, @@ -882,6 +883,52 @@ impl<'w> UnsafeEntityCell<'w> { }) } } + + /// # Safety + /// It is the callers responsibility to ensure that + /// - the [`UnsafeEntityCell`] has permission to access the queried data mutably + /// - no other references to the queried data exist at the same time + pub(crate) unsafe fn get_components(&self) -> Option> { + // SAFETY: World is only used to access query data and initialize query state + let state = unsafe { + let world = self.world().world(); + Q::get_state(world.components())? + }; + let location = self.location(); + // SAFETY: Location is guaranteed to exist + let archetype = unsafe { + self.world + .archetypes() + .get(location.archetype_id) + .debug_checked_unwrap() + }; + if Q::matches_component_set(&state, &|id| archetype.contains(id)) { + // SAFETY: state was initialized above using the world passed into this function + let mut fetch = unsafe { + Q::init_fetch( + self.world, + &state, + self.world.last_change_tick(), + self.world.change_tick(), + ) + }; + // SAFETY: Table is guaranteed to exist + let table = unsafe { + self.world + .storages() + .tables + .get(location.table_id) + .debug_checked_unwrap() + }; + // SAFETY: Archetype and table are from the same world used to initialize state and fetch. + // Table corresponds to archetype. State is the same state used to init fetch above. + unsafe { Q::set_archetype(&mut fetch, &state, archetype, table) } + // SAFETY: Called after set_archetype above. Entity and location are guaranteed to exist. + unsafe { Some(Q::fetch(&mut fetch, self.id(), location.table_row)) } + } else { + None + } + } } impl<'w> UnsafeEntityCell<'w> { diff --git a/examples/ecs/dynamic.rs b/examples/ecs/dynamic.rs index 79ec202ae4452..e7d20a8997920 100644 --- a/examples/ecs/dynamic.rs +++ b/examples/ecs/dynamic.rs @@ -151,7 +151,7 @@ fn main() { query.iter_mut(&mut world).for_each(|filtered_entity| { let terms = filtered_entity - .components() + .accessed_components() .map(|id| { let ptr = filtered_entity.get_by_id(id).unwrap(); let info = component_info.get(&id).unwrap(); From 5308c6a5a512df5ea6b72bbdd7a0675d02c03f48 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 14 May 2024 16:55:42 -0700 Subject: [PATCH 2/8] Add World lifetimes where possible, add EntityWorldMut variants, add panic message --- crates/bevy_ecs/src/world/entity_ref.rs | 37 +++++++++++++++++++++---- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 624d697142cf4..2c44ff6fbe76c 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -158,12 +158,12 @@ impl<'w> EntityRef<'w> { } /// Returns read-only components for the current entity that match the query `Q`. - pub fn components(&self) -> Q::Item<'_> { - self.get_components::().unwrap() + pub fn components(&self) -> Q::Item<'w> { + self.get_components::().expect(QUERY_MISMATCH_ERROR) } /// Returns read-only components for the current entity that match the query `Q`. - pub fn get_components(&self) -> Option> { + pub fn get_components(&self) -> Option> { // SAFETY: &mut self implies exclusive access for duration of returned value unsafe { self.0.get_components::() } } @@ -364,12 +364,12 @@ impl<'w> EntityMut<'w> { /// Returns read-only components for the current entity that match the query `Q`. pub fn components(&self) -> Q::Item<'_> { - self.get_components::().unwrap() + self.get_components::().expect(QUERY_MISMATCH_ERROR) } /// Returns components for the current entity that match the query `Q`. pub fn components_mut(&mut self) -> Q::Item<'_> { - self.get_components_mut::().unwrap() + self.get_components_mut::().expect(QUERY_MISMATCH_ERROR) } /// Returns read-only components for the current entity that match the query `Q`. @@ -681,6 +681,31 @@ impl<'w> EntityWorldMut<'w> { EntityRef::from(self).get() } + /// Returns read-only components for the current entity that match the query `Q`. + #[inline] + pub fn components(&self) -> Q::Item<'_> { + EntityRef::from(self).components::() + } + + /// Returns components for the current entity that match the query `Q`. + #[inline] + pub fn components_mut(&mut self) -> Q::Item<'_> { + self.get_components_mut::().expect(QUERY_MISMATCH_ERROR) + } + + /// Returns read-only components for the current entity that match the query `Q`. + #[inline] + pub fn get_components(&self) -> Option> { + EntityRef::from(self).get_components::() + } + + /// Returns components for the current entity that match the query `Q`. + #[inline] + pub fn get_components_mut(&mut self) -> Option> { + // SAFETY: &mut self implies exclusive access for duration of returned value + unsafe { self.as_unsafe_entity_cell().get_components::() } + } + /// Consumes `self` and gets access to the component of type `T` with /// the world `'w` lifetime for the current entity. /// Returns `None` if the entity does not have a component of type `T`. @@ -1524,6 +1549,8 @@ unsafe fn trigger_on_replace_and_on_remove_hooks_and_observers( } } +const QUERY_MISMATCH_ERROR: &str = "Query does not match the current entity"; + /// A view into a single entity and component in a world, which may either be vacant or occupied. /// /// This `enum` can only be constructed from the [`entry`] method on [`EntityWorldMut`]. From c4b6a2ba7f6542f661fd758b9f1e6030cb13ca5a Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 14 May 2024 16:58:18 -0700 Subject: [PATCH 3/8] Update crates/bevy_ecs/src/world/entity_ref.rs Co-authored-by: Periwink --- crates/bevy_ecs/src/world/entity_ref.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 2c44ff6fbe76c..4ee8650e87f30 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -374,7 +374,7 @@ impl<'w> EntityMut<'w> { /// Returns read-only components for the current entity that match the query `Q`. pub fn get_components(&self) -> Option> { - // SAFETY: &mut self implies exclusive access for duration of returned value + // SAFETY: We have read-only access to all components of this entity. unsafe { self.0.get_components::() } } From efa27604a541a9925062b2c7f0693e541e536a19 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 14 May 2024 16:58:27 -0700 Subject: [PATCH 4/8] Update crates/bevy_ecs/src/world/entity_ref.rs Co-authored-by: Periwink --- crates/bevy_ecs/src/world/entity_ref.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 4ee8650e87f30..06a84ad70b91c 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -164,7 +164,7 @@ impl<'w> EntityRef<'w> { /// Returns read-only components for the current entity that match the query `Q`. pub fn get_components(&self) -> Option> { - // SAFETY: &mut self implies exclusive access for duration of returned value + // SAFETY: We have read-only access to all components of this entity. unsafe { self.0.get_components::() } } } From 2195504d1c42e67df0d02630fec6388e1e2a99fa Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Sat, 7 Sep 2024 21:13:17 -0500 Subject: [PATCH 5/8] Remove mutable variants due to UB issues --- crates/bevy_ecs/src/world/entity_ref.rs | 24 ------------------- .../bevy_ecs/src/world/unsafe_world_cell.rs | 8 +++---- 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 06a84ad70b91c..64e9410464bf7 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -367,23 +367,12 @@ impl<'w> EntityMut<'w> { self.get_components::().expect(QUERY_MISMATCH_ERROR) } - /// Returns components for the current entity that match the query `Q`. - pub fn components_mut(&mut self) -> Q::Item<'_> { - self.get_components_mut::().expect(QUERY_MISMATCH_ERROR) - } - /// Returns read-only components for the current entity that match the query `Q`. pub fn get_components(&self) -> Option> { // SAFETY: We have read-only access to all components of this entity. unsafe { self.0.get_components::() } } - /// Returns components for the current entity that match the query `Q`. - pub fn get_components_mut(&mut self) -> Option> { - // SAFETY: &mut self implies exclusive access for duration of returned value - unsafe { self.0.get_components::() } - } - /// Consumes `self` and gets access to the component of type `T` with the /// world `'w` lifetime for the current entity. /// @@ -687,25 +676,12 @@ impl<'w> EntityWorldMut<'w> { EntityRef::from(self).components::() } - /// Returns components for the current entity that match the query `Q`. - #[inline] - pub fn components_mut(&mut self) -> Q::Item<'_> { - self.get_components_mut::().expect(QUERY_MISMATCH_ERROR) - } - /// Returns read-only components for the current entity that match the query `Q`. #[inline] pub fn get_components(&self) -> Option> { EntityRef::from(self).get_components::() } - /// Returns components for the current entity that match the query `Q`. - #[inline] - pub fn get_components_mut(&mut self) -> Option> { - // SAFETY: &mut self implies exclusive access for duration of returned value - unsafe { self.as_unsafe_entity_cell().get_components::() } - } - /// Consumes `self` and gets access to the component of type `T` with /// the world `'w` lifetime for the current entity. /// Returns `None` if the entity does not have a component of type `T`. diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index 63e523bf13b29..ffebdc55a7fb1 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -11,7 +11,7 @@ use crate::{ entity::{Entities, Entity, EntityLocation}, observer::Observers, prelude::Component, - query::{DebugCheckedUnwrap, QueryData}, + query::{DebugCheckedUnwrap, ReadOnlyQueryData}, removal_detection::RemovedComponentEvents, storage::{Column, ComponentSparseSet, Storages}, system::{Res, Resource}, @@ -886,9 +886,9 @@ impl<'w> UnsafeEntityCell<'w> { /// # Safety /// It is the callers responsibility to ensure that - /// - the [`UnsafeEntityCell`] has permission to access the queried data mutably - /// - no other references to the queried data exist at the same time - pub(crate) unsafe fn get_components(&self) -> Option> { + /// - the [`UnsafeEntityCell`] has permission to access the queried data immutably + /// - no mutable references to the queried data exist at the same time + pub(crate) unsafe fn get_components(&self) -> Option> { // SAFETY: World is only used to access query data and initialize query state let state = unsafe { let world = self.world().world(); From 2e5e163094269a7378efb46285e83ce468db045b Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Sat, 7 Sep 2024 22:38:28 -0500 Subject: [PATCH 6/8] add comments about when the methods can fail --- crates/bevy_ecs/src/world/entity_ref.rs | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 64e9410464bf7..bd4591ecd626a 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -6,7 +6,7 @@ use crate::{ entity::{Entities, Entity, EntityLocation}, event::Event, observer::{Observer, Observers}, - query::{Access, QueryData, ReadOnlyQueryData}, + query::{Access, ReadOnlyQueryData}, removal_detection::RemovedComponentEvents, storage::Storages, system::IntoObserverSystem, @@ -158,11 +158,16 @@ impl<'w> EntityRef<'w> { } /// Returns read-only components for the current entity that match the query `Q`. + /// + /// # Panics + /// + /// If the entitiy does not have the components required by the query `Q`. pub fn components(&self) -> Q::Item<'w> { self.get_components::().expect(QUERY_MISMATCH_ERROR) } - /// Returns read-only components for the current entity that match the query `Q`. + /// Returns read-only components for the current entity that match the query `Q`, + /// or `None` if the entity does not have the components required by the query `Q`. pub fn get_components(&self) -> Option> { // SAFETY: We have read-only access to all components of this entity. unsafe { self.0.get_components::() } @@ -363,11 +368,16 @@ impl<'w> EntityMut<'w> { } /// Returns read-only components for the current entity that match the query `Q`. + /// + /// # Panics + /// + /// If the entity does not have the components required by the query `Q`. pub fn components(&self) -> Q::Item<'_> { self.get_components::().expect(QUERY_MISMATCH_ERROR) } - /// Returns read-only components for the current entity that match the query `Q`. + /// Returns read-only components for the current entity that match the query `Q`, + /// or `None` if the entity does not have the components required by the query `Q`. pub fn get_components(&self) -> Option> { // SAFETY: We have read-only access to all components of this entity. unsafe { self.0.get_components::() } @@ -671,12 +681,17 @@ impl<'w> EntityWorldMut<'w> { } /// Returns read-only components for the current entity that match the query `Q`. + /// + /// # Panics + /// + /// If the entity does not have the components required by the query `Q`. #[inline] pub fn components(&self) -> Q::Item<'_> { EntityRef::from(self).components::() } - /// Returns read-only components for the current entity that match the query `Q`. + /// Returns read-only components for the current entity that match the query `Q`, + /// or `None` if the entity does not have the components required by the query `Q`. #[inline] pub fn get_components(&self) -> Option> { EntityRef::from(self).get_components::() From 341f15b489b2e1e29c72dd3a9d47174fdecc0bf7 Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Sat, 7 Sep 2024 22:43:09 -0500 Subject: [PATCH 7/8] doc the unsafe version --- crates/bevy_ecs/src/world/unsafe_world_cell.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index ffebdc55a7fb1..07e0591c29a01 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -884,6 +884,9 @@ impl<'w> UnsafeEntityCell<'w> { } } + /// Returns read-only components for the current entity that match the query `Q`, + /// or `None` if the entity does not have the components required by the query `Q`. + /// /// # Safety /// It is the callers responsibility to ensure that /// - the [`UnsafeEntityCell`] has permission to access the queried data immutably From 4321dccbacd4726f3f9f4d878480ce489c564871 Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Sat, 7 Sep 2024 22:48:12 -0500 Subject: [PATCH 8/8] fix typo --- crates/bevy_ecs/src/world/entity_ref.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index bd4591ecd626a..69a6aa3ccd73c 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -161,7 +161,7 @@ impl<'w> EntityRef<'w> { /// /// # Panics /// - /// If the entitiy does not have the components required by the query `Q`. + /// If the entity does not have the components required by the query `Q`. pub fn components(&self) -> Q::Item<'w> { self.get_components::().expect(QUERY_MISMATCH_ERROR) }