From 5ebebf75eb2013c9be19d41ddd8aff1ecd6997bb Mon Sep 17 00:00:00 2001 From: Nicola Papale Date: Sun, 11 Jun 2023 17:35:42 +0200 Subject: [PATCH 1/3] Add get_ref to EntityRef To mirror the `Ref` added as `WorldQuery`, and the `Mut` in `EntityMut::get_mut`, we add `EntityRef::get_ref`, which retrieves `T` with tick informations, but *immutably*. --- crates/bevy_ecs/src/world/entity_ref.rs | 12 +++++- .../bevy_ecs/src/world/unsafe_world_cell.rs | 43 ++++++++++++++++--- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index c97d9a21483a7..c5b2f4fa6efbd 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -12,7 +12,7 @@ use bevy_ptr::{OwningPtr, Ptr}; use bevy_utils::tracing::debug; use std::any::TypeId; -use super::unsafe_world_cell::UnsafeEntityCell; +use super::{unsafe_world_cell::UnsafeEntityCell, Ref}; /// A read-only reference to a particular [`Entity`] and all of its components #[derive(Copy, Clone)] @@ -121,6 +121,16 @@ impl<'w> EntityRef<'w> { unsafe { self.as_unsafe_world_cell_readonly().get::() } } + /// Gets access to the component of type `T` for the current entity, + /// including change detection information in [`Ref`]. + /// + /// Returns `None` if the entity does not have a component of type `T`. + #[inline] + pub fn get_ref(&self) -> Option> { + // SAFETY: &self implies shared access for duration of returned value + unsafe { self.as_unsafe_world_cell_readonly().get_ref::() } + } + /// Retrieves the change ticks for the given component. This can be useful for implementing change /// detection in custom runtimes. #[inline] diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index 997862981ae99..fca0d8191917d 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -2,11 +2,11 @@ #![warn(unsafe_op_in_unsafe_fn)] -use super::{Mut, World, WorldId}; +use super::{Mut, Ref, World, WorldId}; use crate::{ archetype::{Archetype, ArchetypeComponentId, Archetypes}, bundle::Bundles, - change_detection::{MutUntyped, TicksMut}, + change_detection::{MutUntyped, Ticks, TicksMut}, component::{ ComponentId, ComponentStorage, ComponentTicks, Components, StorageType, Tick, TickCells, }, @@ -651,15 +651,14 @@ impl<'w> UnsafeEntityCell<'w> { /// - no other mutable references to the component exist at the same time #[inline] pub unsafe fn get(self) -> Option<&'w T> { - let component_id = self.world.components().get_id(TypeId::of::())?; - // SAFETY: - // - entity location is valid - // - proper world access is promised by caller + // - `storage_type` is correct (T component_id + T::STORAGE_TYPE) + // - `location` is valid + // - proper aliasing is promised by caller unsafe { get_component( self.world, - component_id, + self.world.components().get_id(TypeId::of::())?, T::Storage::STORAGE_TYPE, self.entity, self.location, @@ -669,6 +668,35 @@ impl<'w> UnsafeEntityCell<'w> { } } + /// # Safety + /// It is the callers responsibility to ensure that + /// - the [`UnsafeEntityCell`] has permission to access the component + /// - no other mutable references to the component exist at the same time + #[inline] + pub unsafe fn get_ref(self) -> Option> { + let last_change_tick = self.world.last_change_tick(); + let change_tick = self.world.change_tick(); + + // SAFETY: + // - `storage_type` is correct (T component_id + T::STORAGE_TYPE) + // - `location` is valid + // - proper aliasing is promised by caller + unsafe { + get_component_and_ticks( + self.world, + self.world.components().get_id(TypeId::of::())?, + T::Storage::STORAGE_TYPE, + self.entity, + self.location, + ) + .map(|(value, cells)| Ref { + // SAFETY: returned component is of type T + value: value.deref::(), + ticks: Ticks::from_tick_cells(cells, last_change_tick, change_tick), + }) + } + } + /// Retrieves the change ticks for the given component. This can be useful for implementing change /// detection in custom runtimes. /// @@ -761,6 +789,7 @@ impl<'w> UnsafeEntityCell<'w> { self.location, ) .map(|(value, cells)| Mut { + // SAFETY: returned component is of type T value: value.assert_unique().deref_mut::(), ticks: TicksMut::from_tick_cells(cells, last_change_tick, change_tick), }) From 838a09e4cb2f54fff49525d0306645f4251c46f0 Mon Sep 17 00:00:00 2001 From: Nicola Papale Date: Mon, 12 Jun 2023 21:07:41 +0200 Subject: [PATCH 2/3] PR feedback Co-authored-by: James Liu --- 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 c5b2f4fa6efbd..0a74cecfa2067 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -122,7 +122,7 @@ impl<'w> EntityRef<'w> { } /// Gets access to the component of type `T` for the current entity, - /// including change detection information in [`Ref`]. + /// including change detection information as a [`Ref`]. /// /// Returns `None` if the entity does not have a component of type `T`. #[inline] From f66c3013d5f5eec3ee293919f142bfd32063aba5 Mon Sep 17 00:00:00 2001 From: Nicola Papale Date: Mon, 12 Jun 2023 21:26:28 +0200 Subject: [PATCH 3/3] extract safe code from unsafe block --- crates/bevy_ecs/src/world/unsafe_world_cell.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index fca0d8191917d..f95df96429549 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -651,6 +651,7 @@ impl<'w> UnsafeEntityCell<'w> { /// - no other mutable references to the component exist at the same time #[inline] pub unsafe fn get(self) -> Option<&'w T> { + let component_id = self.world.components().get_id(TypeId::of::())?; // SAFETY: // - `storage_type` is correct (T component_id + T::STORAGE_TYPE) // - `location` is valid @@ -658,7 +659,7 @@ impl<'w> UnsafeEntityCell<'w> { unsafe { get_component( self.world, - self.world.components().get_id(TypeId::of::())?, + component_id, T::Storage::STORAGE_TYPE, self.entity, self.location, @@ -676,6 +677,7 @@ impl<'w> UnsafeEntityCell<'w> { pub unsafe fn get_ref(self) -> Option> { let last_change_tick = self.world.last_change_tick(); let change_tick = self.world.change_tick(); + let component_id = self.world.components().get_id(TypeId::of::())?; // SAFETY: // - `storage_type` is correct (T component_id + T::STORAGE_TYPE) @@ -684,7 +686,7 @@ impl<'w> UnsafeEntityCell<'w> { unsafe { get_component_and_ticks( self.world, - self.world.components().get_id(TypeId::of::())?, + component_id, T::Storage::STORAGE_TYPE, self.entity, self.location,