From c07517a41c17cd6f1254e32c0936f972639f1990 Mon Sep 17 00:00:00 2001 From: TheRawMeatball Date: Sun, 25 Jul 2021 10:45:54 +0300 Subject: [PATCH 1/4] Add direct accessor for change ticks --- crates/bevy_ecs/src/world/entity_ref.rs | 28 +++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 1815d15798f43..51b11938ed1ec 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -69,6 +69,20 @@ impl<'w> EntityRef<'w> { } } + #[inline] + pub fn get_change_ticks(&self) -> Option<&'w ComponentTicks> { + // SAFE: entity location is valid and returned component is of type T + unsafe { + get_component_and_ticks_with_type( + self.world, + TypeId::of::(), + self.entity, + self.location, + ) + .map(|(_, ticks)| &*ticks) + } + } + /// # Safety /// This allows aliased mutability. You must make sure this call does not result in multiple /// mutable references to the same component @@ -151,6 +165,20 @@ impl<'w> EntityMut<'w> { } } + #[inline] + pub fn get_change_ticks(&self) -> Option<&'w ComponentTicks> { + // SAFE: entity location is valid and returned component is of type T + unsafe { + get_component_and_ticks_with_type( + self.world, + TypeId::of::(), + self.entity, + self.location, + ) + .map(|(_, ticks)| &*ticks) + } + } + #[inline] pub fn get_mut(&mut self) -> Option> { // SAFE: world access is unique, entity location is valid, and returned component is of type From c7c241476cf2eb506342eb352812bd76f76ab90e Mon Sep 17 00:00:00 2001 From: TheRawMeatball Date: Sun, 2 Jan 2022 11:22:30 +0300 Subject: [PATCH 2/4] ad and switch to get_ticks --- crates/bevy_ecs/src/query/fetch.rs | 4 +- crates/bevy_ecs/src/storage/sparse_set.rs | 4 +- crates/bevy_ecs/src/world/entity_ref.rs | 48 +++++++++++++++++++---- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 264b370e26dfb..6829920157e0e 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -998,7 +998,9 @@ impl<'w, 's, T: Component> Fetch<'w, 's> for ChangeTrackersFetch { StorageType::SparseSet => { let entity = *self.entities.add(archetype_index); ChangeTrackers { - component_ticks: (&*self.sparse_set).get_ticks(entity).cloned().unwrap(), + component_ticks: unsafe { + (&*(&*self.sparse_set).get_ticks_ptr(entity).unwrap()).clone() + }, marker: PhantomData, last_change_tick: self.last_change_tick, change_tick: self.change_tick, diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index ed11fdc65859c..9175ff75bb594 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -183,10 +183,10 @@ impl ComponentSparseSet { } #[inline] - pub fn get_ticks(&self, entity: Entity) -> Option<&ComponentTicks> { + pub fn get_ticks_ptr(&self, entity: Entity) -> Option<*mut ComponentTicks> { let dense_index = *self.sparse.get(entity)?; // SAFE: if the sparse index points to something in the dense vec, it exists - unsafe { Some(&*self.ticks.get_unchecked(dense_index).get()) } + unsafe { Some(self.ticks.get_unchecked(dense_index).get()) } } /// Removes the `entity` from this sparse set and returns a pointer to the associated value (if diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index c667819586576..92f1aeeb08c12 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -71,15 +71,10 @@ impl<'w> EntityRef<'w> { #[inline] pub fn get_change_ticks(&self) -> Option<&'w ComponentTicks> { - // SAFE: entity location is valid and returned component is of type T + // SAFE: entity location is valid unsafe { - get_component_and_ticks_with_type( - self.world, - TypeId::of::(), - self.entity, - self.location, - ) - .map(|(_, ticks)| &*ticks) + get_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) + .map(|ticks| &*ticks) } } @@ -552,6 +547,31 @@ unsafe fn get_component_and_ticks( } } +#[inline] +unsafe fn get_ticks( + world: &World, + component_id: ComponentId, + entity: Entity, + location: EntityLocation, +) -> Option<*mut ComponentTicks> { + let archetype = &world.archetypes[location.archetype_id]; + let component_info = world.components.get_info_unchecked(component_id); + match component_info.storage_type() { + StorageType::Table => { + let table = &world.storages.tables[archetype.table_id()]; + let components = table.get_column(component_id)?; + let table_row = archetype.entity_table_row(location.index); + // SAFE: archetypes only store valid table_rows and the stored component type is T + Some(components.get_ticks_mut_ptr_unchecked(table_row)) + } + StorageType::SparseSet => world + .storages + .sparse_sets + .get(component_id) + .and_then(|sparse_set| sparse_set.get_ticks_ptr(entity)), + } +} + // TODO: move to Storages? /// Moves component data out of storage. /// @@ -618,6 +638,18 @@ pub(crate) unsafe fn get_component_and_ticks_with_type( get_component_and_ticks(world, component_id, entity, location) } +/// # Safety +/// `entity_location` must be within bounds of an archetype that exists. +pub(crate) unsafe fn get_ticks_with_type( + world: &World, + type_id: TypeId, + entity: Entity, + location: EntityLocation, +) -> Option<*mut ComponentTicks> { + let component_id = world.components.get_id(type_id)?; + get_ticks(world, component_id, entity, location) +} + fn contains_component_with_type(world: &World, type_id: TypeId, location: EntityLocation) -> bool { if let Some(component_id) = world.components.get_id(type_id) { contains_component_with_id(world, component_id, location) From 30bf96e4c091f7519095f6bd4cd2108d31e46db3 Mon Sep 17 00:00:00 2001 From: TheRawMeatball Date: Sun, 2 Jan 2022 11:33:42 +0300 Subject: [PATCH 3/4] remove unused unsafe --- crates/bevy_ecs/src/query/fetch.rs | 4 +--- crates/bevy_ecs/src/query/filter.rs | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 6829920157e0e..fc681866c6fe4 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -998,9 +998,7 @@ impl<'w, 's, T: Component> Fetch<'w, 's> for ChangeTrackersFetch { StorageType::SparseSet => { let entity = *self.entities.add(archetype_index); ChangeTrackers { - component_ticks: unsafe { - (&*(&*self.sparse_set).get_ticks_ptr(entity).unwrap()).clone() - }, + component_ticks: (&*(&*self.sparse_set).get_ticks_ptr(entity).unwrap()).clone(), marker: PhantomData, last_change_tick: self.last_change_tick, change_tick: self.change_tick, diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 7849f1823029d..08ad675ace176 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -577,7 +577,7 @@ macro_rules! impl_tick_filter { } StorageType::SparseSet => { let entity = *self.entities.add(archetype_index); - let ticks = (&*self.sparse_set).get_ticks(entity).cloned().unwrap(); + let ticks = (&*(&*self.sparse_set).get_ticks_ptr(entity).unwrap()).clone(); $is_detected(&ticks, self.last_change_tick, self.change_tick) } } From a3a284da1074ab597141d1fe69310e6606baa623 Mon Sep 17 00:00:00 2001 From: TheRawMeatball Date: Mon, 2 May 2022 20:31:07 +0300 Subject: [PATCH 4/4] docs --- crates/bevy_ecs/src/world/entity_ref.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index c543384348a16..9a53fe8992b6c 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -72,6 +72,8 @@ impl<'w> EntityRef<'w> { } } + /// Retrieves the change ticks for the given component. This can be useful for implementing change + /// detection in custom runtimes. #[inline] pub fn get_change_ticks(&self) -> Option<&'w ComponentTicks> { // SAFE: entity location is valid @@ -178,6 +180,8 @@ impl<'w> EntityMut<'w> { unsafe { self.get_unchecked_mut::() } } + /// Retrieves the change ticks for the given component. This can be useful for implementing change + /// detection in custom runtimes. #[inline] pub fn get_change_ticks(&self) -> Option<&ComponentTicks> { // SAFE: entity location is valid