From 24eec8f511d81f7a22a91eb62eb60e7f2d3e7523 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sat, 29 Oct 2022 13:22:57 +0200 Subject: [PATCH 01/30] add InteriorMutableWorld with resource access methods --- .../src/world/interior_mutable_world.rs | 181 ++++++++++++++++++ crates/bevy_ecs/src/world/mod.rs | 11 +- 2 files changed, 190 insertions(+), 2 deletions(-) create mode 100644 crates/bevy_ecs/src/world/interior_mutable_world.rs diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs new file mode 100644 index 0000000000000..79a75f18243b1 --- /dev/null +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -0,0 +1,181 @@ +#![warn(unsafe_op_in_unsafe_fn)] + +use super::{Mut, World}; +use crate::{ + change_detection::{MutUntyped, Ticks}, + component::ComponentId, + system::Resource, +}; +use bevy_ptr::Ptr; +use bevy_ptr::UnsafeCellDeref; +use std::any::TypeId; + +/// Variant of the [`World`] where resource and component accesses takes a `&World`, and the responsibility to avoid +/// aliasing violations are given to the caller instead of being checked at compile-time by rust's unique XOR shared rule. +/// +/// ### Rationale +/// In rust, having a `&mut World` means that there are absolutely no other references to the safe world alive at the same time, +/// without exceptions. Not even unsafe code can change this. +/// +/// But there are situations where careful shared mutable access through a type is possible and safe. For this, rust provides the [`UnsafeCell`](std::cell::UnsafeCell) +/// escape hatch, which allows you to get a `*mut T` from a `&UnsafeCell` and around which safe abstractions can be built. +/// +/// Access to resources and components can be done uniquely using [`World::resource_mut`] and [`World::entity_mut`], and shared using [`World::resource`] and [`World::entity`]. +/// These methods use lifetimes to check at compile time that no aliasing rules are being broken. +/// +/// This alone is not enough to implement bevy systems where multiple systems can access *disjoint* parts of the world concurrently. For this, bevy stores all values of +/// resources and components (and [`ComponentTicks`](crate::component::ComponentTicks)) in [`UnsafeCell`](std::cell::UnsafeCell)s, and carefully validates disjoint access patterns using +/// APIs like [`System::component_access`](crate::system::System::component_access). +/// +/// A system then can be executed using [`System::run_unsafe`](crate::system::System::run_unsafe) with a `&World` and use methods with interior mutability to access resource values. +/// access resource values. +/// +/// ### Example Usage +/// +/// [`InteriorMutableWorld`] can be used as a building block for writing APIs that safely allow disjoint access into the world. +/// In the following example, the world is split into a resource access half and a component access half, where each one can +/// safely hand out mutable references. +/// +/// ``` +/// use bevy_ecs::world::World; +/// use bevy_ecs::change_detection::Mut; +/// use bevy_ecs::system::Resource; +/// use bevy_ecs::world::interior_mutable_world::InteriorMutableWorld; +/// +/// // INVARIANT: existance of this struct means that users of it are the only ones being able to access resources in the world +/// struct OnlyResourceAccessWorld<'w>(InteriorMutableWorld<'w>); +/// // INVARIANT: existance of this struct means that users of it are the only ones being able to access components in the world +/// struct OnlyComponentAccessWorld<'w>(InteriorMutableWorld<'w>); +/// +/// impl<'w> OnlyResourceAccessWorld<'w> { +/// fn get_resource_mut(&mut self) -> Option> { +/// // SAFETY: resource access is allowed through this InteriorMutableWorld +/// unsafe { self.0.get_resource_mut::() } +/// } +/// } +/// // impl<'w> OnlyComponentAccessWorld<'w> { +/// // ... +/// // } +/// +/// // the two interior mutable worlds borrow from the `&mut World`, so it cannot be accessed while they are live +/// fn split_world_access(world: &mut World) -> (OnlyResourceAccessWorld<'_>, OnlyComponentAccessWorld<'_>) { +/// let resource_access = OnlyResourceAccessWorld(unsafe { world.as_interior_mutable() }); +/// let component_access = OnlyComponentAccessWorld(unsafe { world.as_interior_mutable() }); +/// (resource_access, component_access) +/// } +/// ``` +#[derive(Copy, Clone)] +pub struct InteriorMutableWorld<'w>(&'w World); + +impl<'w> InteriorMutableWorld<'w> { + pub(crate) fn new(world: &'w World) -> Self { + InteriorMutableWorld(world) + } + + /// Gets a reference to the [`&World`](crate::world::World) this [`InteriorMutableWorld`] belongs to. + /// This can be used to call methods like [`World::read_change_tick`] which aren't exposed here but don't perform any accesses. + /// + /// **Note**: You *must not* hand out a `&World` reference to arbitrary safe code when the [`InteriorMutableWorld`] is currently + /// being used for mutable accesses. + /// + /// SAFETY: + /// - the world must not be used to access any resources or components. You can use it to safely access metadata. + pub unsafe fn world(&self) -> &'w World { + self.0 + } + + /// Gets a reference to the resource of the given type if it exists + /// + /// # Safety + /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. + /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, + /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + #[inline] + pub unsafe fn get_resource(&self) -> Option<&'w R> { + self.0.get_resource::() + } + + /// Gets a pointer to the resource with the id [`ComponentId`] if it exists. + /// The returned pointer must not be used to modify the resource, and must not be + /// dereferenced after the borrow of the [`World`] ends. + /// + /// **You should prefer to use the typed API [`InteriorMutableWorld::get_resource`] where possible and only + /// use this in cases where the actual types are not known at compile time.** + /// + /// # Safety + /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. + /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, + /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + #[inline] + pub unsafe fn get_resource_by_id(&self, component_id: ComponentId) -> Option> { + self.0.get_resource_by_id(component_id) + } + + /// Gets a mutable reference to the resource of the given type if it exists + /// + /// # Safety + /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. + /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, + /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + #[inline] + pub unsafe fn get_resource_mut(&self) -> Option> { + let component_id = self.0.components.get_resource_id(TypeId::of::())?; + // SAFETY: + // - component_id is of type `R` + // - caller ensures aliasing rules + // - `R` is Send + Sync + unsafe { self.0.get_resource_unchecked_mut_with_id(component_id) } + } + + /// Gets a pointer to the resource with the id [`ComponentId`] if it exists. + /// The returned pointer may be used to modify the resource, as long as the mutable borrow + /// of the [`InteriorMutableWorld`] is still valid. + /// + /// **You should prefer to use the typed API [`InteriorMutableWorld::get_resource_mut`] where possible and only + /// use this in cases where the actual types are not known at compile time.** + /// + /// # Safety + /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. + /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, + /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + #[inline] + pub unsafe fn get_resource_mut_by_id( + &self, + component_id: ComponentId, + ) -> Option> { + let info = self.0.components.get_info(component_id)?; + if !info.is_send_and_sync() { + self.0.validate_non_send_access_untyped(info.name()); + } + + let (ptr, ticks) = self.0.get_resource_with_ticks(component_id)?; + + // SAFETY: + // - index is in-bounds because the column is initialized and non-empty + // - the caller promises that no other reference to the ticks of the same row can exist at the same time + let ticks = unsafe { + Ticks::from_tick_cells(ticks, self.0.last_change_tick(), self.0.read_change_tick()) + }; + + Some(MutUntyped { + // SAFETY: This function has exclusive access to the world so nothing aliases `ptr`. + value: unsafe { ptr.assert_unique() }, + ticks, + }) + } + + /// Gets a reference to the non-send resource of the given type, if it exists. + /// Otherwise returns [None] + #[inline] + pub unsafe fn get_non_send_resource(&self) -> Option<&R> { + self.0.get_non_send_resource::() + } + /// Gets a mutable reference to the non-send resource of the given type, if it exists. + /// Otherwise returns [None] + #[inline] + pub unsafe fn get_non_send_resource_mut(&mut self) -> Option> { + let component_id = self.0.components.get_resource_id(TypeId::of::())?; + // SAFETY: safety requirement is deferred to the caller + unsafe { self.0.get_non_send_unchecked_mut_with_id(component_id) } + } +} diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index dd09a5341aaa4..a25d6ca579688 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1,4 +1,5 @@ mod entity_ref; +pub mod interior_mutable_world; mod spawn_batch; mod world_cell; @@ -33,6 +34,8 @@ mod identifier; pub use identifier::WorldId; +use self::interior_mutable_world::InteriorMutableWorld; + /// Stores and exposes operations on [entities](Entity), [components](Component), resources, /// and their associated metadata. /// @@ -105,6 +108,10 @@ impl World { self.id } + pub fn as_interior_mutable(&self) -> InteriorMutableWorld<'_> { + InteriorMutableWorld::new(self) + } + /// Retrieves this world's [Entities] collection #[inline] pub fn entities(&self) -> &Entities { @@ -1655,7 +1662,7 @@ impl World { } impl World { - /// Gets a resource to the resource with the id [`ComponentId`] if it exists. + /// Gets a pointer to the resource with the id [`ComponentId`] if it exists. /// The returned pointer must not be used to modify the resource, and must not be /// dereferenced after the immutable borrow of the [`World`] ends. /// @@ -1666,7 +1673,7 @@ impl World { self.storages.resources.get(component_id)?.get_data() } - /// Gets a resource to the resource with the id [`ComponentId`] if it exists. + /// Gets a pointer to the resource with the id [`ComponentId`] if it exists. /// The returned pointer may be used to modify the resource, as long as the mutable borrow /// of the [`World`] is still valid. /// From ee4efc3e7ce522b0e01fd4c26afac0749c457e13 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sat, 17 Dec 2022 13:04:45 +0100 Subject: [PATCH 02/30] add some safe accessors to InteriorMutableWorld --- .../src/world/interior_mutable_world.rs | 36 +++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index 79a75f18243b1..ab3821c90f4d4 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -2,12 +2,15 @@ use super::{Mut, World}; use crate::{ + archetype::Archetypes, + bundle::Bundles, change_detection::{MutUntyped, Ticks}, - component::ComponentId, + component::{ComponentId, Components}, + entity::Entities, + storage::Storages, system::Resource, }; use bevy_ptr::Ptr; -use bevy_ptr::UnsafeCellDeref; use std::any::TypeId; /// Variant of the [`World`] where resource and component accesses takes a `&World`, and the responsibility to avoid @@ -84,6 +87,35 @@ impl<'w> InteriorMutableWorld<'w> { self.0 } + /// Retrieves this world's [Entities] collection + #[inline] + pub fn entities(&self) -> &'w Entities { + &self.0.entities + } + + /// Retrieves this world's [Archetypes] collection + #[inline] + pub fn archetypes(&self) -> &'w Archetypes { + &self.0.archetypes + } + + /// Retrieves this world's [Components] collection + #[inline] + pub fn components(&self) -> &'w Components { + &self.0.components + } + + /// Retrieves this world's [Storages] collection + #[inline] + pub fn storages(&self) -> &'w Storages { + &self.0.storages + } + + /// Retrieves this world's [Bundles] collection + #[inline] + pub fn bundles(&self) -> &'w Bundles { + &self.0.bundles + } /// Gets a reference to the resource of the given type if it exists /// /// # Safety From 11956cce155e89ba65690271c42844d2f658f0bb Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sat, 29 Oct 2022 13:47:16 +0200 Subject: [PATCH 03/30] move unchecked methods from world to InteriorMutableWorld --- crates/bevy_asset/src/reflect.rs | 3 +- crates/bevy_ecs/src/reflect.rs | 11 ++- crates/bevy_ecs/src/system/system_param.rs | 6 +- .../src/world/interior_mutable_world.rs | 79 +++++++++++---- crates/bevy_ecs/src/world/mod.rs | 96 ++----------------- crates/bevy_ecs/src/world/world_cell.rs | 12 ++- 6 files changed, 94 insertions(+), 113 deletions(-) diff --git a/crates/bevy_asset/src/reflect.rs b/crates/bevy_asset/src/reflect.rs index 3d01ac148d635..5aa224177a56a 100644 --- a/crates/bevy_asset/src/reflect.rs +++ b/crates/bevy_asset/src/reflect.rs @@ -148,7 +148,8 @@ impl FromType for ReflectAsset { get_unchecked_mut: |world, handle| { let assets = unsafe { world - .get_resource_unchecked_mut::>() + .as_interior_mutable() + .get_resource_mut::>() .unwrap() .into_inner() }; diff --git a/crates/bevy_ecs/src/reflect.rs b/crates/bevy_ecs/src/reflect.rs index 88e25dfaf86b5..c4bb074bab4ae 100644 --- a/crates/bevy_ecs/src/reflect.rs +++ b/crates/bevy_ecs/src/reflect.rs @@ -385,10 +385,13 @@ impl FromType for ReflectResource { // SAFETY: all usages of `reflect_unchecked_mut` guarantee that there is either a single mutable // reference or multiple immutable ones alive at any given point unsafe { - world.get_resource_unchecked_mut::().map(|res| Mut { - value: res.value as &mut dyn Reflect, - ticks: res.ticks, - }) + world + .as_interior_mutable() + .get_resource_mut::() + .map(|res| Mut { + value: res.value as &mut dyn Reflect, + ticks: res.ticks, + }) } }, copy: |source_world, destination_world| { diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 48df3c9f4032b..3c6461c27916b 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -541,7 +541,8 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { change_tick: u32, ) -> Self::Item<'w, 's> { let value = world - .get_resource_unchecked_mut_with_id(component_id) + .as_interior_mutable() + .get_resource_mut_with_id(component_id) .unwrap_or_else(|| { panic!( "Resource requested by {} does not exist: {}", @@ -578,7 +579,8 @@ unsafe impl<'a, T: Resource> SystemParam for Option> { change_tick: u32, ) -> Self::Item<'w, 's> { world - .get_resource_unchecked_mut_with_id(component_id) + .as_interior_mutable() + .get_resource_mut_with_id(component_id) .map(|value| ResMut { value: value.value, ticks: TicksMut { diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index ab3821c90f4d4..e780d5c54cdcd 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -4,7 +4,7 @@ use super::{Mut, World}; use crate::{ archetype::Archetypes, bundle::Bundles, - change_detection::{MutUntyped, Ticks}, + change_detection::{MutUntyped, TicksMut}, component::{ComponentId, Components}, entity::Entities, storage::Storages, @@ -156,7 +156,7 @@ impl<'w> InteriorMutableWorld<'w> { // - component_id is of type `R` // - caller ensures aliasing rules // - `R` is Send + Sync - unsafe { self.0.get_resource_unchecked_mut_with_id(component_id) } + unsafe { self.get_resource_mut_with_id(component_id) } } /// Gets a pointer to the resource with the id [`ComponentId`] if it exists. @@ -175,18 +175,13 @@ impl<'w> InteriorMutableWorld<'w> { &self, component_id: ComponentId, ) -> Option> { - let info = self.0.components.get_info(component_id)?; - if !info.is_send_and_sync() { - self.0.validate_non_send_access_untyped(info.name()); - } - let (ptr, ticks) = self.0.get_resource_with_ticks(component_id)?; // SAFETY: // - index is in-bounds because the column is initialized and non-empty // - the caller promises that no other reference to the ticks of the same row can exist at the same time let ticks = unsafe { - Ticks::from_tick_cells(ticks, self.0.last_change_tick(), self.0.read_change_tick()) + TicksMut::from_tick_cells(ticks, self.0.last_change_tick(), self.0.read_change_tick()) }; Some(MutUntyped { @@ -195,19 +190,67 @@ impl<'w> InteriorMutableWorld<'w> { ticks, }) } +} - /// Gets a reference to the non-send resource of the given type, if it exists. - /// Otherwise returns [None] +impl<'w> InteriorMutableWorld<'w> { + /// # Safety + /// - `component_id` must be assigned to a component of type `R` + /// - Caller must ensure this doesn't violate Rust mutability rules for the given resource. + /// - resource is either Send+Sync or the main thread is validated + /// + /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. + /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, + /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. #[inline] - pub unsafe fn get_non_send_resource(&self) -> Option<&R> { - self.0.get_non_send_resource::() + pub(crate) unsafe fn get_resource_mut_with_id( + &self, + component_id: ComponentId, + ) -> Option> { + let (ptr, ticks) = self.0.get_resource_with_ticks(component_id)?; + + // SAFETY: + // - This caller ensures that nothing aliases `ticks`. + // - index is in-bounds because the column is initialized and non-empty + let ticks = unsafe { + TicksMut::from_tick_cells(ticks, self.0.last_change_tick(), self.0.read_change_tick()) + }; + + Some(Mut { + // SAFETY: caller ensures aliasing rules, ptr is of type `R` + value: unsafe { ptr.assert_unique().deref_mut() }, + ticks, + }) } - /// Gets a mutable reference to the non-send resource of the given type, if it exists. - /// Otherwise returns [None] + + /// # Safety + /// - `component_id` must be assigned to a component of type `R`. + /// - Caller must ensure this doesn't violate Rust mutability rules for the given resource. + /// + /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. + /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, + /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. #[inline] - pub unsafe fn get_non_send_resource_mut(&mut self) -> Option> { - let component_id = self.0.components.get_resource_id(TypeId::of::())?; - // SAFETY: safety requirement is deferred to the caller - unsafe { self.0.get_non_send_unchecked_mut_with_id(component_id) } + pub(crate) unsafe fn get_non_send_mut_with_id( + &self, + component_id: ComponentId, + ) -> Option> { + let (ptr, ticks) = self + .0 + .storages + .non_send_resources + .get(component_id)? + .get_with_ticks()?; + Some(Mut { + // SAFETY: caller ensures unique access + value: unsafe { ptr.assert_unique().deref_mut() }, + // SAFETY: caller ensures unique access + ticks: unsafe { + TicksMut::from_tick_cells( + ticks, + self.0.last_change_tick(), + self.0.read_change_tick(), + ) + }, + }) } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index a25d6ca579688..7c1dda4a489d4 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -18,7 +18,6 @@ use crate::{ }, entity::{AllocAtWithoutReplacement, Entities, Entity, EntityLocation}, event::{Event, Events}, - ptr::UnsafeCellDeref, query::{DebugCheckedUnwrap, QueryState, ReadOnlyWorldQuery, WorldQuery}, storage::{Column, ComponentSparseSet, ResourceData, SparseSet, Storages, TableRow}, system::Resource, @@ -998,7 +997,7 @@ impl World { #[inline] pub fn get_resource_mut(&mut self) -> Option> { // SAFETY: unique world access - unsafe { self.get_resource_unchecked_mut() } + unsafe { self.as_interior_mutable().get_resource_mut() } } /// Gets a mutable reference to the resource of type `T` if it exists, @@ -1031,18 +1030,6 @@ impl World { unsafe { data.with_type::() } } - /// Gets a mutable reference to the resource of the given type, if it exists - /// Otherwise returns [None] - /// - /// # Safety - /// This will allow aliased mutable access to the given resource type. The caller must ensure - /// that there is either only one mutable access or multiple immutable accesses at a time. - #[inline] - pub unsafe fn get_resource_unchecked_mut(&self) -> Option> { - let component_id = self.components.get_resource_id(TypeId::of::())?; - self.get_resource_unchecked_mut_with_id(component_id) - } - /// Gets an immutable reference to the non-send resource of the given type, if it exists. /// /// # Panics @@ -1107,22 +1094,11 @@ impl World { #[inline] pub fn get_non_send_resource_mut(&mut self) -> Option> { // SAFETY: unique world access - unsafe { self.get_non_send_resource_unchecked_mut() } - } - - /// Gets a mutable reference to the non-send resource of the given type, if it exists. - /// Otherwise returns [None] - /// - /// # Panics - /// This function will panic if it isn't called from the same thread that the resource was inserted from. - /// - /// # Safety - /// This will allow aliased mutable access to the given non-send resource type. The caller must - /// ensure that there is either only one mutable access or multiple immutable accesses at a time. - #[inline] - pub unsafe fn get_non_send_resource_unchecked_mut(&self) -> Option> { let component_id = self.components.get_resource_id(TypeId::of::())?; - self.get_non_send_unchecked_mut_with_id(component_id) + unsafe { + self.as_interior_mutable() + .get_non_send_mut_with_id::(component_id) + } } // Shorthand helper function for getting the data and change ticks for a resource. @@ -1405,25 +1381,6 @@ impl World { .map(|ptr| ptr.deref()) } - /// # Safety - /// `component_id` must be assigned to a component of type `R` - /// Caller must ensure this doesn't violate Rust mutability rules for the given resource. - #[inline] - pub(crate) unsafe fn get_resource_unchecked_mut_with_id( - &self, - component_id: ComponentId, - ) -> Option> { - let (ptr, ticks) = self.get_resource_with_ticks(component_id)?; - Some(Mut { - value: ptr.assert_unique().deref_mut(), - ticks: TicksMut::from_tick_cells( - ticks, - self.last_change_tick(), - self.read_change_tick(), - ), - }) - } - /// # Safety /// `component_id` must be assigned to a component of type `R` #[inline] @@ -1440,30 +1397,6 @@ impl World { ) } - /// # Safety - /// `component_id` must be assigned to a component of type `R`. - /// Caller must ensure this doesn't violate Rust mutability rules for the given resource. - #[inline] - pub(crate) unsafe fn get_non_send_unchecked_mut_with_id( - &self, - component_id: ComponentId, - ) -> Option> { - let (ptr, ticks) = self - .storages - .non_send_resources - .get(component_id)? - .get_with_ticks()?; - Some(Mut { - value: ptr.assert_unique().deref_mut(), - ticks: TicksMut { - added: ticks.added.deref_mut(), - changed: ticks.changed.deref_mut(), - last_change_tick: self.last_change_tick(), - change_tick: self.read_change_tick(), - }, - }) - } - /// Inserts a new resource with the given `value`. Will replace the value if it already existed. /// /// **You should prefer to use the typed API [`World::insert_resource`] where possible and only @@ -1681,20 +1614,11 @@ impl World { /// use this in cases where the actual types are not known at compile time.** #[inline] pub fn get_resource_mut_by_id(&mut self, component_id: ComponentId) -> Option> { - let change_tick = self.change_tick(); - let (ptr, ticks) = self.get_resource_with_ticks(component_id)?; - - let ticks = - // SAFETY: This function has exclusive access to the world so nothing aliases `ticks`. - // - index is in-bounds because the column is initialized and non-empty - // - no other reference to the ticks of the same row can exist at the same time - unsafe { TicksMut::from_tick_cells(ticks, self.last_change_tick(), change_tick) }; - - Some(MutUntyped { - // SAFETY: This function has exclusive access to the world so nothing aliases `ptr`. - value: unsafe { ptr.assert_unique() }, - ticks, - }) + // SAFETY: unique world access + unsafe { + self.as_interior_mutable() + .get_resource_mut_by_id(component_id) + } } /// Gets a `!Send` resource to the resource with the id [`ComponentId`] if it exists. diff --git a/crates/bevy_ecs/src/world/world_cell.rs b/crates/bevy_ecs/src/world/world_cell.rs index 8e6699e076694..7feaec7f823ed 100644 --- a/crates/bevy_ecs/src/world/world_cell.rs +++ b/crates/bevy_ecs/src/world/world_cell.rs @@ -226,7 +226,11 @@ impl<'w> WorldCell<'w> { .get_resource_archetype_component_id(component_id)?; WorldBorrowMut::try_new( // SAFETY: ComponentId matches TypeId and access is checked by WorldBorrowMut - || unsafe { self.world.get_resource_unchecked_mut_with_id(component_id) }, + || unsafe { + self.world + .as_interior_mutable() + .get_resource_mut_with_id(component_id) + }, archetype_component_id, self.access.clone(), ) @@ -292,7 +296,11 @@ impl<'w> WorldCell<'w> { .get_non_send_archetype_component_id(component_id)?; WorldBorrowMut::try_new( // SAFETY: ComponentId matches TypeId and access is checked by WorldBorrowMut - || unsafe { self.world.get_non_send_unchecked_mut_with_id(component_id) }, + || unsafe { + self.world + .as_interior_mutable() + .get_non_send_mut_with_id(component_id) + }, archetype_component_id, self.access.clone(), ) From 6561462863eda594386ec3d4b97965f8ec8183a5 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sat, 17 Dec 2022 13:05:28 +0100 Subject: [PATCH 04/30] add get_non_send_resource and get_non_send_resource_mut to InteriorMutableWorld --- .../src/world/interior_mutable_world.rs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index e780d5c54cdcd..0f1bab504df79 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -143,6 +143,17 @@ impl<'w> InteriorMutableWorld<'w> { self.0.get_resource_by_id(component_id) } + /// Gets a reference to the non-send resource of the given type if it exists + /// + /// # Safety + /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. + /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, + /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + #[inline] + pub unsafe fn get_non_send_resource(&self) -> Option<&'w R> { + self.0.get_non_send_resource() + } + /// Gets a mutable reference to the resource of the given type if it exists /// /// # Safety @@ -190,6 +201,19 @@ impl<'w> InteriorMutableWorld<'w> { ticks, }) } + + /// Gets a mutable reference to the non-send resource of the given type if it exists + /// + /// # Safety + /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. + /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, + /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + #[inline] + pub unsafe fn get_non_send_resource_mut(&self) -> Option> { + let component_id = self.0.components.get_resource_id(TypeId::of::())?; + // SAFETY: component_id matches `R`, rest is promised by caller + unsafe { self.get_non_send_mut_with_id(component_id) } + } } impl<'w> InteriorMutableWorld<'w> { From 1580e28f63dcf0c431199a873a64c02c0501d70f Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sat, 29 Oct 2022 14:12:14 +0200 Subject: [PATCH 05/30] use InteriorMutableWorld for ReflectComponent/ReflectResource/ReflectAsset --- crates/bevy_asset/src/reflect.rs | 32 ++++++++++--------------- crates/bevy_ecs/src/reflect.rs | 40 ++++++++++++++++---------------- 2 files changed, 32 insertions(+), 40 deletions(-) diff --git a/crates/bevy_asset/src/reflect.rs b/crates/bevy_asset/src/reflect.rs index 5aa224177a56a..7b15f534f9e97 100644 --- a/crates/bevy_asset/src/reflect.rs +++ b/crates/bevy_asset/src/reflect.rs @@ -1,6 +1,6 @@ use std::any::{Any, TypeId}; -use bevy_ecs::world::World; +use bevy_ecs::world::{interior_mutable_world::InteriorMutableWorld, World}; use bevy_reflect::{FromReflect, FromType, Reflect, Uuid}; use crate::{Asset, Assets, Handle, HandleId, HandleUntyped}; @@ -18,8 +18,11 @@ pub struct ReflectAsset { assets_resource_type_id: TypeId, get: fn(&World, HandleUntyped) -> Option<&dyn Reflect>, - get_mut: fn(&mut World, HandleUntyped) -> Option<&mut dyn Reflect>, - get_unchecked_mut: unsafe fn(&World, HandleUntyped) -> Option<&mut dyn Reflect>, + // SAFETY: + // - may only be called with a [`IteriorMutableWorld`] which can be used to access the corresponding `Assets` resource mutably + // - may only be used to access **at most one** access at once + get_unchecked_mut: + unsafe fn(InteriorMutableWorld<'_>, HandleUntyped) -> Option<&mut dyn Reflect>, add: fn(&mut World, &dyn Reflect) -> HandleUntyped, set: fn(&mut World, HandleUntyped, &dyn Reflect) -> HandleUntyped, len: fn(&World) -> usize, @@ -54,10 +57,11 @@ impl ReflectAsset { world: &'w mut World, handle: HandleUntyped, ) -> Option<&'w mut dyn Reflect> { - (self.get_mut)(world, handle) + // SAFETY: unique world access + unsafe { (self.get_unchecked_mut)(world.as_interior_mutable(), handle) } } - /// Equivalent of [`Assets::get_mut`], but does not require a mutable reference to the world. + /// Equivalent of [`Assets::get_mut`], but works with a [`InteriorMutableWorld`]. /// /// Only use this method when you have ensured that you are the *only* one with access to the [`Assets`] resource of the asset type. /// Furthermore, this does *not* allow you to have look up two distinct handles, @@ -81,12 +85,11 @@ impl ReflectAsset { /// # Safety /// This method does not prevent you from having two mutable pointers to the same data, /// violating Rust's aliasing rules. To avoid this: - /// * Only call this method when you have exclusive access to the world - /// (or use a scheduler that enforces unique access to the `Assets` resource). + /// * Only call this method if you know that the [`InteriorMutableWorld`] may be used to access the corresponding `Assets` /// * Don't call this method more than once in the same scope. pub unsafe fn get_unchecked_mut<'w>( &self, - world: &'w World, + world: InteriorMutableWorld<'w>, handle: HandleUntyped, ) -> Option<&'w mut dyn Reflect> { // SAFETY: requirements are deferred to the caller @@ -140,19 +143,8 @@ impl FromType for ReflectAsset { let asset = assets.get(&handle.typed()); asset.map(|asset| asset as &dyn Reflect) }, - get_mut: |world, handle| { - let assets = world.resource_mut::>().into_inner(); - let asset = assets.get_mut(&handle.typed()); - asset.map(|asset| asset as &mut dyn Reflect) - }, get_unchecked_mut: |world, handle| { - let assets = unsafe { - world - .as_interior_mutable() - .get_resource_mut::>() - .unwrap() - .into_inner() - }; + let assets = unsafe { world.get_resource_mut::>().unwrap().into_inner() }; let asset = assets.get_mut(&handle.typed()); asset.map(|asset| asset as &mut dyn Reflect) }, diff --git a/crates/bevy_ecs/src/reflect.rs b/crates/bevy_ecs/src/reflect.rs index c4bb074bab4ae..8e06df1215a39 100644 --- a/crates/bevy_ecs/src/reflect.rs +++ b/crates/bevy_ecs/src/reflect.rs @@ -5,7 +5,7 @@ use crate::{ component::Component, entity::{Entity, EntityMap, MapEntities, MapEntitiesError}, system::Resource, - world::{FromWorld, World}, + world::{interior_mutable_world::InteriorMutableWorld, FromWorld, World}, }; use bevy_reflect::{ impl_from_reflect_value, impl_reflect_value, FromType, Reflect, ReflectDeserialize, @@ -52,7 +52,8 @@ pub struct ReflectComponentFns { /// Function pointer implementing [`ReflectComponent::reflect()`]. pub reflect: fn(&World, Entity) -> Option<&dyn Reflect>, /// Function pointer implementing [`ReflectComponent::reflect_mut()`]. - pub reflect_mut: unsafe fn(&World, Entity) -> Option>, + /// The function may only be called with a InteriorMutableWorld that can be used to access the relevant component on the given entity + pub reflect_mut: unsafe fn(InteriorMutableWorld<'_>, Entity) -> Option>, /// Function pointer implementing [`ReflectComponent::copy()`]. pub copy: fn(&World, &mut World, Entity, Entity), } @@ -117,20 +118,20 @@ impl ReflectComponent { entity: Entity, ) -> Option> { // SAFETY: unique world access - unsafe { (self.0.reflect_mut)(world, entity) } + unsafe { (self.0.reflect_mut)(world.as_interior_mutable(), entity) } } /// # Safety /// This method does not prevent you from having two mutable pointers to the same data, /// violating Rust's aliasing rules. To avoid this: - /// * Only call this method in an exclusive system to avoid sharing across threads (or use a - /// scheduler that enforces safe memory access). + /// * Only call this method with a [`InteriorMutableWorld`] that may be used to access the component on the entity `entity` /// * Don't call this method more than once in the same scope for a given [`Component`]. pub unsafe fn reflect_unchecked_mut<'a>( &self, - world: &'a World, + world: InteriorMutableWorld<'a>, entity: Entity, ) -> Option> { + // SAFETY: safety requirements deferred to caller (self.0.reflect_mut)(world, entity) } @@ -212,6 +213,9 @@ impl FromType for ReflectComponent { // SAFETY: reflect_mut is an unsafe function pointer used by `reflect_unchecked_mut` which promises to never // produce aliasing mutable references, and reflect_mut, which has mutable world access unsafe { + // SAFETY: entity access through the InteriorMutableWorld is not implemented yet. + // The following code only accesses the component `C` through the entity `entity` + let world = world.world(); world .get_entity(entity)? .get_unchecked_mut::(world.last_change_tick(), world.read_change_tick()) @@ -265,7 +269,7 @@ pub struct ReflectResourceFns { /// Function pointer implementing [`ReflectResource::reflect()`]. pub reflect: fn(&World) -> Option<&dyn Reflect>, /// Function pointer implementing [`ReflectResource::reflect_unchecked_mut()`]. - pub reflect_unchecked_mut: unsafe fn(&World) -> Option>, + pub reflect_unchecked_mut: unsafe fn(InteriorMutableWorld<'_>) -> Option>, /// Function pointer implementing [`ReflectResource::copy()`]. pub copy: fn(&World, &mut World), } @@ -314,19 +318,18 @@ impl ReflectResource { /// Gets the value of this [`Resource`] type from the world as a mutable reflected reference. pub fn reflect_mut<'a>(&self, world: &'a mut World) -> Option> { // SAFETY: unique world access - unsafe { (self.0.reflect_unchecked_mut)(world) } + unsafe { (self.0.reflect_unchecked_mut)(world.as_interior_mutable()) } } /// # Safety /// This method does not prevent you from having two mutable pointers to the same data, /// violating Rust's aliasing rules. To avoid this: - /// * Only call this method in an exclusive system to avoid sharing across threads (or use a - /// scheduler that enforces safe memory access). + /// * Only call this method with a [`InteriorMutableWorld`] which can be used to access the resource. /// * Don't call this method more than once in the same scope for a given [`Resource`]. - pub unsafe fn reflect_unchecked_mut<'a>( + pub unsafe fn reflect_unchecked_mut<'w>( &self, - world: &'a World, - ) -> Option> { + world: InteriorMutableWorld<'w>, + ) -> Option> { // SAFETY: caller promises to uphold uniqueness guarantees (self.0.reflect_unchecked_mut)(world) } @@ -385,13 +388,10 @@ impl FromType for ReflectResource { // SAFETY: all usages of `reflect_unchecked_mut` guarantee that there is either a single mutable // reference or multiple immutable ones alive at any given point unsafe { - world - .as_interior_mutable() - .get_resource_mut::() - .map(|res| Mut { - value: res.value as &mut dyn Reflect, - ticks: res.ticks, - }) + world.get_resource_mut::().map(|res| Mut { + value: res.value as &mut dyn Reflect, + ticks: res.ticks, + }) } }, copy: |source_world, destination_world| { From f1a08a68e9984569c2aa9aa26a3124e26b5bca8b Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sat, 29 Oct 2022 14:32:23 +0200 Subject: [PATCH 06/30] add InteriorMutableEntityRef --- crates/bevy_ecs/src/reflect.rs | 19 +- crates/bevy_ecs/src/system/query.rs | 3 +- crates/bevy_ecs/src/world/entity_ref.rs | 95 ++------ .../src/world/interior_mutable_world.rs | 223 +++++++++++++++++- crates/bevy_ecs/src/world/world_cell.rs | 4 +- 5 files changed, 253 insertions(+), 91 deletions(-) diff --git a/crates/bevy_ecs/src/reflect.rs b/crates/bevy_ecs/src/reflect.rs index 8e06df1215a39..0ffb49fef788f 100644 --- a/crates/bevy_ecs/src/reflect.rs +++ b/crates/bevy_ecs/src/reflect.rs @@ -210,19 +210,14 @@ impl FromType for ReflectComponent { .map(|c| c as &dyn Reflect) }, reflect_mut: |world, entity| { - // SAFETY: reflect_mut is an unsafe function pointer used by `reflect_unchecked_mut` which promises to never - // produce aliasing mutable references, and reflect_mut, which has mutable world access + // SAFETY: reflect_mut is an unsafe function pointer used by + // 1. `reflect_unchecked_mut` which must be called with an InteriorMutableWorld with access the the component `C` on the `entity`, and + // 2. reflect_mut, which has mutable world access unsafe { - // SAFETY: entity access through the InteriorMutableWorld is not implemented yet. - // The following code only accesses the component `C` through the entity `entity` - let world = world.world(); - world - .get_entity(entity)? - .get_unchecked_mut::(world.last_change_tick(), world.read_change_tick()) - .map(|c| Mut { - value: c.value as &mut dyn Reflect, - ticks: c.ticks, - }) + world.get_entity(entity)?.get_mut::().map(|c| Mut { + value: c.value as &mut dyn Reflect, + ticks: c.ticks, + }) } }, }) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index a86d40e62b038..51e7a756d8fbf 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1134,6 +1134,7 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { } let world = self.world; let entity_ref = world + .as_interior_mutable() .get_entity(entity) .ok_or(QueryComponentError::NoSuchEntity)?; let component_id = world @@ -1150,7 +1151,7 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { .has_write(archetype_component) { entity_ref - .get_unchecked_mut::(self.last_change_tick, self.change_tick) + .get_mut_using_ticks::(self.last_change_tick, self.change_tick) .ok_or(QueryComponentError::MissingComponent) } else { Err(QueryComponentError::MissingWriteAccess) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index a165678516987..a203f5678feda 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -135,42 +135,6 @@ impl<'w> EntityRef<'w> { ) } } - - /// Gets a mutable reference to the component of type `T` associated with - /// this entity without ensuring there are no other borrows active and without - /// ensuring that the returned reference will stay valid. - /// - /// # Safety - /// - /// - The returned reference must never alias a mutable borrow of this component. - /// - The returned reference must not be used after this component is moved which - /// may happen from **any** `insert_component`, `remove_component` or `despawn` - /// operation on this world (non-exhaustive list). - #[inline] - pub unsafe fn get_unchecked_mut( - &self, - last_change_tick: u32, - change_tick: u32, - ) -> Option> { - // SAFETY: - // - entity location and entity is valid - // - returned component is of type T - // - the storage type provided is correct for T - self.world - .get_component_and_ticks_with_type( - TypeId::of::(), - T::Storage::STORAGE_TYPE, - self.entity, - self.location, - ) - .map(|(value, ticks)| Mut { - // SAFETY: - // - returned component is of type T - // - Caller guarantees that this reference will not alias. - value: value.assert_unique().deref_mut::(), - ticks: TicksMut::from_tick_cells(ticks, last_change_tick, change_tick), - }) - } } impl<'w> EntityRef<'w> { @@ -291,7 +255,23 @@ impl<'w> EntityMut<'w> { #[inline] pub fn get_mut(&mut self) -> Option> { // SAFETY: world access is unique, and lifetimes enforce correct usage of returned borrow - unsafe { self.get_unchecked_mut::() } + unsafe { + self.world + .get_component_and_ticks_with_type( + TypeId::of::(), + T::Storage::STORAGE_TYPE, + self.entity, + self.location, + ) + .map(|(value, cells)| Mut { + value: value.assert_unique().deref_mut::(), + ticks: TicksMut::from_tick_cells( + cells, + self.world.last_change_tick, + self.world.read_change_tick(), + ), + }) + } } /// Retrieves the change ticks for the given component. This can be useful for implementing change @@ -335,39 +315,6 @@ impl<'w> EntityMut<'w> { } } - /// Gets a mutable reference to the component of type `T` associated with - /// this entity without ensuring there are no other borrows active and without - /// ensuring that the returned reference will stay valid. - /// - /// # Safety - /// - /// - The returned reference must never alias a mutable borrow of this component. - /// - The returned reference must not be used after this component is moved which - /// may happen from **any** `insert_component`, `remove_component` or `despawn` - /// operation on this world (non-exhaustive list). - #[inline] - pub unsafe fn get_unchecked_mut(&self) -> Option> { - // SAFETY: - // - entity location and entity is valid - // - returned component is of type T - // - the storage type provided is correct for T - self.world - .get_component_and_ticks_with_type( - TypeId::of::(), - T::Storage::STORAGE_TYPE, - self.entity, - self.location, - ) - .map(|(value, ticks)| Mut { - value: value.assert_unique().deref_mut::(), - ticks: TicksMut::from_tick_cells( - ticks, - self.world.last_change_tick(), - self.world.read_change_tick(), - ), - }) - } - /// Adds a [`Bundle`] of components to the entity. /// /// This will overwrite any previous value(s) of the same component type. @@ -712,7 +659,11 @@ impl<'w> EntityMut<'w> { } } -fn contains_component_with_type(world: &World, type_id: TypeId, location: EntityLocation) -> bool { +pub(crate) 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) } else { @@ -720,7 +671,7 @@ fn contains_component_with_type(world: &World, type_id: TypeId, location: Entity } } -fn contains_component_with_id( +pub(crate) fn contains_component_with_id( world: &World, component_id: ComponentId, location: EntityLocation, diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index 0f1bab504df79..f1cb5570a5023 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -1,12 +1,13 @@ #![warn(unsafe_op_in_unsafe_fn)] -use super::{Mut, World}; +use super::{entity_ref, Mut, World}; use crate::{ - archetype::Archetypes, + archetype::{Archetype, Archetypes}, bundle::Bundles, change_detection::{MutUntyped, TicksMut}, - component::{ComponentId, Components}, - entity::Entities, + component::{ComponentId, ComponentStorage, ComponentTicks, Components}, + entity::{Entities, Entity, EntityLocation}, + prelude::Component, storage::Storages, system::Resource, }; @@ -116,6 +117,18 @@ impl<'w> InteriorMutableWorld<'w> { pub fn bundles(&self) -> &'w Bundles { &self.0.bundles } + + /// Retrieves an [`InteriorMutableEntityRef`] that exposes read and write operations for the given `entity`. + /// Similar to the [`InteriorMutableWorld`], you are in charge of making sure that no aliasing rules are violated. + pub fn get_entity(&self, entity: Entity) -> Option> { + let location = self.0.entities.get(entity)?; + Some(InteriorMutableEntityRef { + world: InteriorMutableWorld(self.0), + entity, + location, + }) + } + /// Gets a reference to the resource of the given type if it exists /// /// # Safety @@ -278,3 +291,205 @@ impl<'w> InteriorMutableWorld<'w> { }) } } + +/// A interior-mutable reference to a particular [`Entity`] and all of its components +#[derive(Copy, Clone)] +pub struct InteriorMutableEntityRef<'w> { + world: InteriorMutableWorld<'w>, + entity: Entity, + location: EntityLocation, +} + +impl<'w> InteriorMutableEntityRef<'w> { + #[inline] + #[must_use = "Omit the .id() call if you do not need to store the `Entity` identifier."] + pub fn id(&self) -> Entity { + self.entity + } + + #[inline] + pub fn location(&self) -> EntityLocation { + self.location + } + + #[inline] + pub fn archetype(&self) -> &Archetype { + &self.world.0.archetypes[self.location.archetype_id] + } + + #[inline] + pub fn world(&self) -> InteriorMutableWorld<'w> { + self.world + } + + #[inline] + pub fn contains(&self) -> bool { + self.contains_type_id(TypeId::of::()) + } + + #[inline] + pub fn contains_id(&self, component_id: ComponentId) -> bool { + entity_ref::contains_component_with_id(self.world.0, component_id, self.location) + } + + #[inline] + pub fn contains_type_id(&self, type_id: TypeId) -> bool { + entity_ref::contains_component_with_type(self.world.0, type_id, self.location) + } + + /// # Safety + /// All [`InteriorMutableEntityRef`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. + /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, + /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + #[inline] + pub unsafe fn get(&self) -> Option<&'w T> { + // SAFETY: + // - entity location is valid + // - archetypes and components come from the same world + // - world access is immutable, lifetime tied to `&self` + unsafe { + self.world + .0 + .get_component_with_type( + TypeId::of::(), + T::Storage::STORAGE_TYPE, + self.entity, + self.location, + ) + // SAFETY: returned component is of type T + .map(|value| value.deref::()) + } + } + + /// Retrieves the change ticks for the given component. This can be useful for implementing change + /// detection in custom runtimes. + /// + /// # Safety + /// All [`InteriorMutableEntityRef`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. + /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, + /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + #[inline] + pub unsafe fn get_change_ticks(&self) -> Option { + // SAFETY: + // - entity location is valid + // - archetypes and components come from the same world + // - world access is immutable, lifetime tied to `&self` + unsafe { + self.world.0.get_ticks_with_type( + TypeId::of::(), + T::Storage::STORAGE_TYPE, + self.entity, + self.location, + ) + } + } + + /// # Safety + /// All [`InteriorMutableEntityRef`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. + /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, + /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + #[inline] + pub unsafe fn get_mut(&self) -> Option> { + // SAFETY: same safety requirements + unsafe { + self.get_mut_using_ticks( + self.world.0.last_change_tick(), + self.world.0.read_change_tick(), + ) + } + } + + /// # Safety + /// All [`InteriorMutableEntityRef`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. + /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, + /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + #[inline] + pub(crate) unsafe fn get_mut_using_ticks( + &self, + last_change_tick: u32, + change_tick: u32, + ) -> Option> { + // # Safety + // - `storage_type` is correct + // - `location` is valid + // - aliasing rules are ensured by caller + unsafe { + self.world + .0 + .get_component_and_ticks_with_type( + TypeId::of::(), + T::Storage::STORAGE_TYPE, + self.entity, + self.location, + ) + .map(|(value, cells)| Mut { + value: value.assert_unique().deref_mut::(), + ticks: TicksMut::from_tick_cells(cells, last_change_tick, change_tick), + }) + } + } +} + +impl<'w> InteriorMutableEntityRef<'w> { + /// Gets the component of the given [`ComponentId`] from the entity. + /// + /// **You should prefer to use the typed API where possible and only + /// use this in cases where the actual component types are not known at + /// compile time.** + /// + /// Unlike [`InteriorMutableEntityRef::get`], this returns a raw pointer to the component, + /// which is only valid while the `'w` borrow of the lifetime is active. + /// + /// # Safety + /// All [`InteriorMutableEntityRef`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. + /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, + /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + #[inline] + pub unsafe fn get_by_id(&self, component_id: ComponentId) -> Option> { + let info = self.world.0.components.get_info(component_id)?; + // SAFETY: entity_location is valid, component_id is valid as checked by the line above + unsafe { + self.world.0.get_component( + component_id, + info.storage_type(), + self.entity, + self.location, + ) + } + } + + /// Retrieves a mutable untyped reference to the given `entity`'s [Component] of the given [`ComponentId`]. + /// Returns [None] if the `entity` does not have a [Component] of the given type. + /// + /// **You should prefer to use the typed API [`InteriorMutableEntityRef::get_mut`] where possible and only + /// use this in cases where the actual types are not known at compile time.** + /// + /// # Safety + /// All [`InteriorMutableEntityRef`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. + /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, + /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + #[inline] + pub unsafe fn get_mut_by_id(&self, component_id: ComponentId) -> Option> { + let info = self.world.0.components.get_info(component_id)?; + // SAFETY: entity_location is valid, component_id is valid as checked by the line above + unsafe { + self.world + .0 + .get_component_and_ticks( + component_id, + info.storage_type(), + self.entity, + self.location, + ) + .map(|(value, cells)| MutUntyped { + // SAFETY: world access validated by caller and ties world lifetime to `MutUntyped` lifetime + value: value.assert_unique(), + ticks: TicksMut::from_tick_cells( + cells, + self.world.0.last_change_tick, + self.world.0.read_change_tick(), + ), + }) + } + } +} diff --git a/crates/bevy_ecs/src/world/world_cell.rs b/crates/bevy_ecs/src/world/world_cell.rs index 7feaec7f823ed..d591043cfe1cb 100644 --- a/crates/bevy_ecs/src/world/world_cell.rs +++ b/crates/bevy_ecs/src/world/world_cell.rs @@ -295,11 +295,11 @@ impl<'w> WorldCell<'w> { .world .get_non_send_archetype_component_id(component_id)?; WorldBorrowMut::try_new( - // SAFETY: ComponentId matches TypeId and access is checked by WorldBorrowMut + // SAFETY: access is checked by WorldBorrowMut || unsafe { self.world .as_interior_mutable() - .get_non_send_mut_with_id(component_id) + .get_non_send_resource_mut::() }, archetype_component_id, self.access.clone(), From fdc4a01039cea7cea25f2e4ad1eb186c2c227183 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sat, 17 Dec 2022 12:44:24 +0100 Subject: [PATCH 07/30] add as_interior_mutable(&mut self), as_interior_mutable_readonly(&self) and temporary migration method --- crates/bevy_ecs/src/system/query.rs | 2 +- crates/bevy_ecs/src/system/system_param.rs | 4 ++-- crates/bevy_ecs/src/world/mod.rs | 14 +++++++++++++- crates/bevy_ecs/src/world/world_cell.rs | 4 ++-- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 51e7a756d8fbf..b86cd0eebbb8e 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1134,7 +1134,7 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { } let world = self.world; let entity_ref = world - .as_interior_mutable() + .as_interior_mutable_migration_internal() .get_entity(entity) .ok_or(QueryComponentError::NoSuchEntity)?; let component_id = world diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 3c6461c27916b..ae25ada35f861 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -541,7 +541,7 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { change_tick: u32, ) -> Self::Item<'w, 's> { let value = world - .as_interior_mutable() + .as_interior_mutable_migration_internal() .get_resource_mut_with_id(component_id) .unwrap_or_else(|| { panic!( @@ -579,7 +579,7 @@ unsafe impl<'a, T: Resource> SystemParam for Option> { change_tick: u32, ) -> Self::Item<'w, 's> { world - .as_interior_mutable() + .as_interior_mutable_migration_internal() .get_resource_mut_with_id(component_id) .map(|value| ResMut { value: value.value, diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 7c1dda4a489d4..c9b1f540d9373 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -107,7 +107,19 @@ impl World { self.id } - pub fn as_interior_mutable(&self) -> InteriorMutableWorld<'_> { + /// Creates a new [`InteriorMutableWorld`] view with complete read+write access + pub fn as_interior_mutable(&mut self) -> InteriorMutableWorld<'_> { + InteriorMutableWorld::new(self) + } + /// Creates a new [`InteriorMutableWorld`] view only read access to everything + pub fn as_interior_mutable_readonly(&self) -> InteriorMutableWorld<'_> { + InteriorMutableWorld::new(self) + } + + /// Creates a new [`InteriorMutableWorld`] with read+write access from a [&World](World). + /// This is only a temporary measure until every `&World` that is semantically a [InteriorMutableWorld] + /// has been replaced. + pub(crate) fn as_interior_mutable_migration_internal(&self) -> InteriorMutableWorld<'_> { InteriorMutableWorld::new(self) } diff --git a/crates/bevy_ecs/src/world/world_cell.rs b/crates/bevy_ecs/src/world/world_cell.rs index d591043cfe1cb..6ec25f4eed6be 100644 --- a/crates/bevy_ecs/src/world/world_cell.rs +++ b/crates/bevy_ecs/src/world/world_cell.rs @@ -228,7 +228,7 @@ impl<'w> WorldCell<'w> { // SAFETY: ComponentId matches TypeId and access is checked by WorldBorrowMut || unsafe { self.world - .as_interior_mutable() + .as_interior_mutable_migration_internal() .get_resource_mut_with_id(component_id) }, archetype_component_id, @@ -298,7 +298,7 @@ impl<'w> WorldCell<'w> { // SAFETY: access is checked by WorldBorrowMut || unsafe { self.world - .as_interior_mutable() + .as_interior_mutable_migration_internal() .get_non_send_resource_mut::() }, archetype_component_id, From 1416efb35c7144083db68dc74271762c684e409f Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sat, 17 Dec 2022 13:05:48 +0100 Subject: [PATCH 08/30] migrate WorldCell to InteriorMutableWorld --- crates/bevy_ecs/src/world/mod.rs | 11 ++-- crates/bevy_ecs/src/world/world_cell.rs | 73 ++++++++++++++----------- 2 files changed, 44 insertions(+), 40 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index c9b1f540d9373..39a7f8111e12a 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -26,6 +26,7 @@ use bevy_ptr::{OwningPtr, Ptr}; use bevy_utils::tracing::warn; use std::{ any::TypeId, + cell::UnsafeCell, fmt, sync::atomic::{AtomicU32, Ordering}, }; @@ -62,8 +63,8 @@ pub struct World { pub(crate) storages: Storages, pub(crate) bundles: Bundles, pub(crate) removed_components: SparseSet>, - /// Access cache used by [WorldCell]. - pub(crate) archetype_component_access: ArchetypeComponentAccess, + /// Access cache used by [WorldCell]. Is only accessed in the `Drop` impl of `WorldCell`. + pub(crate) archetype_component_access: UnsafeCell, pub(crate) change_tick: AtomicU32, pub(crate) last_change_tick: u32, pub(crate) last_check_tick: u32, @@ -1106,11 +1107,7 @@ impl World { #[inline] pub fn get_non_send_resource_mut(&mut self) -> Option> { // SAFETY: unique world access - let component_id = self.components.get_resource_id(TypeId::of::())?; - unsafe { - self.as_interior_mutable() - .get_non_send_mut_with_id::(component_id) - } + unsafe { self.as_interior_mutable().get_non_send_resource_mut() } } // Shorthand helper function for getting the data and change ticks for a resource. diff --git a/crates/bevy_ecs/src/world/world_cell.rs b/crates/bevy_ecs/src/world/world_cell.rs index 6ec25f4eed6be..dddf64c103aa5 100644 --- a/crates/bevy_ecs/src/world/world_cell.rs +++ b/crates/bevy_ecs/src/world/world_cell.rs @@ -14,10 +14,12 @@ use std::{ rc::Rc, }; +use super::interior_mutable_world::InteriorMutableWorld; + /// Exposes safe mutable access to multiple resources at a time in a World. Attempting to access /// World in a way that violates Rust's mutability rules will panic thanks to runtime checks. pub struct WorldCell<'w> { - pub(crate) world: &'w mut World, + pub(crate) world: InteriorMutableWorld<'w>, pub(crate) access: Rc>, } @@ -76,8 +78,16 @@ impl ArchetypeComponentAccess { impl<'w> Drop for WorldCell<'w> { fn drop(&mut self) { let mut access = self.access.borrow_mut(); - // give world ArchetypeComponentAccess back to reuse allocations - std::mem::swap(&mut self.world.archetype_component_access, &mut *access); + + { + // SAFETY: we only swap `archetype_component_access` + let world = unsafe { self.world.world() }; + // SAFETY: the WorldCell has exclusive world access + let world_cached_access = unsafe { &mut *world.archetype_component_access.get() }; + + // give world ArchetypeComponentAccess back to reuse allocations + std::mem::swap(world_cached_access, &mut *access); + } } } @@ -175,25 +185,25 @@ impl<'w> WorldCell<'w> { pub(crate) fn new(world: &'w mut World) -> Self { // this is cheap because ArchetypeComponentAccess::new() is const / allocation free let access = std::mem::replace( - &mut world.archetype_component_access, + world.archetype_component_access.get_mut(), ArchetypeComponentAccess::new(), ); // world's ArchetypeComponentAccess is recycled to cut down on allocations Self { - world, + world: world.as_interior_mutable(), access: Rc::new(RefCell::new(access)), } } /// Gets a reference to the resource of the given type pub fn get_resource(&self) -> Option> { - let component_id = self.world.components.get_resource_id(TypeId::of::())?; - let archetype_component_id = self - .world - .get_resource_archetype_component_id(component_id)?; + let component_id = self.world.components().get_resource_id(TypeId::of::())?; + + let archetype_component_id = self.world.storages().resources.get(component_id)?.id(); + WorldBorrow::try_new( - // SAFETY: ComponentId matches TypeId - || unsafe { self.world.get_resource_with_id(component_id) }, + // SAFETY: access is checked by WorldBorrow + || unsafe { self.world.get_resource::() }, archetype_component_id, self.access.clone(), ) @@ -220,17 +230,11 @@ impl<'w> WorldCell<'w> { /// Gets a mutable reference to the resource of the given type pub fn get_resource_mut(&self) -> Option> { - let component_id = self.world.components.get_resource_id(TypeId::of::())?; - let archetype_component_id = self - .world - .get_resource_archetype_component_id(component_id)?; + let component_id = self.world.components().get_resource_id(TypeId::of::())?; + let archetype_component_id = self.world.storages().resources.get(component_id)?.id(); WorldBorrowMut::try_new( - // SAFETY: ComponentId matches TypeId and access is checked by WorldBorrowMut - || unsafe { - self.world - .as_interior_mutable_migration_internal() - .get_resource_mut_with_id(component_id) - }, + // SAFETY: access is checked by WorldBorrowMut + || unsafe { self.world.get_resource_mut::() }, archetype_component_id, self.access.clone(), ) @@ -257,13 +261,16 @@ impl<'w> WorldCell<'w> { /// Gets an immutable reference to the non-send resource of the given type, if it exists. pub fn get_non_send_resource(&self) -> Option> { - let component_id = self.world.components.get_resource_id(TypeId::of::())?; + let component_id = self.world.components().get_resource_id(TypeId::of::())?; let archetype_component_id = self .world - .get_non_send_archetype_component_id(component_id)?; + .storages() + .non_send_resources + .get(component_id)? + .id(); WorldBorrow::try_new( - // SAFETY: ComponentId matches TypeId - || unsafe { self.world.get_non_send_with_id(component_id) }, + // SAFETY: access is checked by WorldBorrowMut + || unsafe { self.world.get_non_send_resource::() }, archetype_component_id, self.access.clone(), ) @@ -290,17 +297,16 @@ impl<'w> WorldCell<'w> { /// Gets a mutable reference to the non-send resource of the given type, if it exists. pub fn get_non_send_resource_mut(&self) -> Option> { - let component_id = self.world.components.get_resource_id(TypeId::of::())?; + let component_id = self.world.components().get_resource_id(TypeId::of::())?; let archetype_component_id = self .world - .get_non_send_archetype_component_id(component_id)?; + .storages() + .non_send_resources + .get(component_id)? + .id(); WorldBorrowMut::try_new( // SAFETY: access is checked by WorldBorrowMut - || unsafe { - self.world - .as_interior_mutable_migration_internal() - .get_non_send_resource_mut::() - }, + || unsafe { self.world.get_non_send_resource_mut::() }, archetype_component_id, self.access.clone(), ) @@ -416,10 +422,11 @@ mod tests { let u32_archetype_component_id = world .get_resource_archetype_component_id(u32_component_id) .unwrap(); - assert_eq!(world.archetype_component_access.access.len(), 1); + assert_eq!(world.archetype_component_access.get_mut().access.len(), 1); assert_eq!( world .archetype_component_access + .get_mut() .access .get(u32_archetype_component_id), Some(&BASE_ACCESS), From 92a32d072ee585420e6f233feacd0d93d5faf16c Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sat, 17 Dec 2022 13:28:12 +0100 Subject: [PATCH 09/30] fix lints --- crates/bevy_ecs/src/world/interior_mutable_world.rs | 4 ++-- crates/bevy_ecs/src/world/mod.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index f1cb5570a5023..051e6fa5a13b2 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -82,7 +82,7 @@ impl<'w> InteriorMutableWorld<'w> { /// **Note**: You *must not* hand out a `&World` reference to arbitrary safe code when the [`InteriorMutableWorld`] is currently /// being used for mutable accesses. /// - /// SAFETY: + /// # Safety: /// - the world must not be used to access any resources or components. You can use it to safely access metadata. pub unsafe fn world(&self) -> &'w World { self.0 @@ -409,7 +409,7 @@ impl<'w> InteriorMutableEntityRef<'w> { last_change_tick: u32, change_tick: u32, ) -> Option> { - // # Safety + // SAFETY: // - `storage_type` is correct // - `location` is valid // - aliasing rules are ensured by caller diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 39a7f8111e12a..89629a5ddb86d 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -118,7 +118,7 @@ impl World { } /// Creates a new [`InteriorMutableWorld`] with read+write access from a [&World](World). - /// This is only a temporary measure until every `&World` that is semantically a [InteriorMutableWorld] + /// This is only a temporary measure until every `&World` that is semantically a [`InteriorMutableWorld`] /// has been replaced. pub(crate) fn as_interior_mutable_migration_internal(&self) -> InteriorMutableWorld<'_> { InteriorMutableWorld::new(self) From e89655530e5c5c05bde32f6ed975b1cab9c7683b Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sat, 17 Dec 2022 13:34:30 +0100 Subject: [PATCH 10/30] fix doc test --- crates/bevy_ecs/src/world/interior_mutable_world.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index 051e6fa5a13b2..ff50f23b032c0 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -63,8 +63,9 @@ use std::any::TypeId; /// /// // the two interior mutable worlds borrow from the `&mut World`, so it cannot be accessed while they are live /// fn split_world_access(world: &mut World) -> (OnlyResourceAccessWorld<'_>, OnlyComponentAccessWorld<'_>) { -/// let resource_access = OnlyResourceAccessWorld(unsafe { world.as_interior_mutable() }); -/// let component_access = OnlyComponentAccessWorld(unsafe { world.as_interior_mutable() }); +/// let interior_mutable_world = world.as_interior_mutable(); +/// let resource_access = OnlyResourceAccessWorld(interior_mutable_world); +/// let component_access = OnlyComponentAccessWorld(interior_mutable_world); /// (resource_access, component_access) /// } /// ``` From 62e5b345cd22b2f52021f367853704f75152de91 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sat, 17 Dec 2022 13:40:45 +0100 Subject: [PATCH 11/30] remove : from Safety block --- crates/bevy_ecs/src/world/interior_mutable_world.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index ff50f23b032c0..a022053be9400 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -83,7 +83,7 @@ impl<'w> InteriorMutableWorld<'w> { /// **Note**: You *must not* hand out a `&World` reference to arbitrary safe code when the [`InteriorMutableWorld`] is currently /// being used for mutable accesses. /// - /// # Safety: + /// # Safety /// - the world must not be used to access any resources or components. You can use it to safely access metadata. pub unsafe fn world(&self) -> &'w World { self.0 From 4b27b656de8ff1c506f9e5d8ef2f599f7f3c2fe5 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sat, 17 Dec 2022 13:48:32 +0100 Subject: [PATCH 12/30] fix bevy_asset doc test --- crates/bevy_asset/src/reflect.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/bevy_asset/src/reflect.rs b/crates/bevy_asset/src/reflect.rs index 7b15f534f9e97..f5fee49caffab 100644 --- a/crates/bevy_asset/src/reflect.rs +++ b/crates/bevy_asset/src/reflect.rs @@ -71,11 +71,12 @@ impl ReflectAsset { /// # use bevy_asset::{ReflectAsset, HandleUntyped}; /// # use bevy_ecs::prelude::World; /// # let reflect_asset: ReflectAsset = unimplemented!(); - /// # let world: World = unimplemented!(); + /// # let mut world: World = unimplemented!(); /// # let handle_1: HandleUntyped = unimplemented!(); /// # let handle_2: HandleUntyped = unimplemented!(); - /// let a = unsafe { reflect_asset.get_unchecked_mut(&world, handle_1).unwrap() }; - /// let b = unsafe { reflect_asset.get_unchecked_mut(&world, handle_2).unwrap() }; + /// let interior_mutable_world = world.as_interior_mutable(); + /// let a = unsafe { reflect_asset.get_unchecked_mut(interior_mutable_world, handle_1).unwrap() }; + /// let b = unsafe { reflect_asset.get_unchecked_mut(interior_mutable_world, handle_2).unwrap() }; /// // ^ not allowed, two mutable references through the same asset resource, even though the /// // handles are distinct /// From 6fb762f0fad8274ce4a462382c3a9aa89d415f62 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Fri, 13 Jan 2023 15:28:09 +0100 Subject: [PATCH 13/30] tweak safety comments --- crates/bevy_ecs/src/reflect.rs | 7 ++++--- crates/bevy_ecs/src/world/entity_ref.rs | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/reflect.rs b/crates/bevy_ecs/src/reflect.rs index 0ffb49fef788f..a5c9164c0ddbd 100644 --- a/crates/bevy_ecs/src/reflect.rs +++ b/crates/bevy_ecs/src/reflect.rs @@ -52,7 +52,7 @@ pub struct ReflectComponentFns { /// Function pointer implementing [`ReflectComponent::reflect()`]. pub reflect: fn(&World, Entity) -> Option<&dyn Reflect>, /// Function pointer implementing [`ReflectComponent::reflect_mut()`]. - /// The function may only be called with a InteriorMutableWorld that can be used to access the relevant component on the given entity + /// SAFETY: The function may only be called with an InteriorMutableWorld that can be used to mutably access the relevant component on the given entity pub reflect_mut: unsafe fn(InteriorMutableWorld<'_>, Entity) -> Option>, /// Function pointer implementing [`ReflectComponent::copy()`]. pub copy: fn(&World, &mut World, Entity, Entity), @@ -124,7 +124,7 @@ impl ReflectComponent { /// # Safety /// This method does not prevent you from having two mutable pointers to the same data, /// violating Rust's aliasing rules. To avoid this: - /// * Only call this method with a [`InteriorMutableWorld`] that may be used to access the component on the entity `entity` + /// * Only call this method with a [`InteriorMutableWorld`] that may be used to mutably access the component on the entity `entity` /// * Don't call this method more than once in the same scope for a given [`Component`]. pub unsafe fn reflect_unchecked_mut<'a>( &self, @@ -264,6 +264,7 @@ pub struct ReflectResourceFns { /// Function pointer implementing [`ReflectResource::reflect()`]. pub reflect: fn(&World) -> Option<&dyn Reflect>, /// Function pointer implementing [`ReflectResource::reflect_unchecked_mut()`]. + /// SAFETY: The function may only be called with an InteriorMutableWorld that can be used to mutably access the relevant resource pub reflect_unchecked_mut: unsafe fn(InteriorMutableWorld<'_>) -> Option>, /// Function pointer implementing [`ReflectResource::copy()`]. pub copy: fn(&World, &mut World), @@ -319,7 +320,7 @@ impl ReflectResource { /// # Safety /// This method does not prevent you from having two mutable pointers to the same data, /// violating Rust's aliasing rules. To avoid this: - /// * Only call this method with a [`InteriorMutableWorld`] which can be used to access the resource. + /// * Only call this method with a [`InteriorMutableWorld`] which can be used to mutably access the resource. /// * Don't call this method more than once in the same scope for a given [`Resource`]. pub unsafe fn reflect_unchecked_mut<'w>( &self, diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index a203f5678feda..1ccea738c74b8 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -151,7 +151,7 @@ impl<'w> EntityRef<'w> { let info = self.world.components().get_info(component_id)?; // SAFETY: // - entity_location and entity are valid - // . component_id is valid as checked by the line above + // - component_id is valid as checked by the line above // - the storage type is accurate as checked by the fetched ComponentInfo unsafe { self.world.get_component( @@ -808,7 +808,7 @@ pub(crate) unsafe fn get_mut( // SAFETY: // - world access is unique // - entity location is valid - // - and returned component is of type T + // - returned component is of type T world .get_component_and_ticks_with_type( TypeId::of::(), @@ -839,7 +839,7 @@ pub(crate) unsafe fn get_mut_by_id( // SAFETY: // - world access is unique // - entity location is valid - // - and returned component is of type T + // - returned component is of type T world .get_component_and_ticks(component_id, info.storage_type(), entity, location) .map(|(value, ticks)| MutUntyped { From a5f38c19ac52acc0cd621dfd48a83487cc39b301 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Fri, 13 Jan 2023 15:30:04 +0100 Subject: [PATCH 14/30] fix interiormutableworld doc comments --- crates/bevy_ecs/src/world/interior_mutable_world.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index a022053be9400..9e40bfd0d8785 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -14,7 +14,7 @@ use crate::{ use bevy_ptr::Ptr; use std::any::TypeId; -/// Variant of the [`World`] where resource and component accesses takes a `&World`, and the responsibility to avoid +/// Variant of the [`World`] where resource and component accesses take `&self`, and the responsibility to avoid /// aliasing violations are given to the caller instead of being checked at compile-time by rust's unique XOR shared rule. /// /// ### Rationale @@ -46,9 +46,9 @@ use std::any::TypeId; /// use bevy_ecs::system::Resource; /// use bevy_ecs::world::interior_mutable_world::InteriorMutableWorld; /// -/// // INVARIANT: existance of this struct means that users of it are the only ones being able to access resources in the world +/// // INVARIANT: existence of this struct means that users of it are the only ones being able to access resources in the world /// struct OnlyResourceAccessWorld<'w>(InteriorMutableWorld<'w>); -/// // INVARIANT: existance of this struct means that users of it are the only ones being able to access components in the world +/// // INVARIANT: existence of this struct means that users of it are the only ones being able to access components in the world /// struct OnlyComponentAccessWorld<'w>(InteriorMutableWorld<'w>); /// /// impl<'w> OnlyResourceAccessWorld<'w> { From dabfd0b1c90b5c840fa95298484fa3e540d6b060 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Fri, 13 Jan 2023 15:37:37 +0100 Subject: [PATCH 15/30] add {read,last,increment}_change_tick to interiormutableworld --- .../src/world/interior_mutable_world.rs | 35 ++++++++++++------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index 9e40bfd0d8785..6ab9ad7b585bb 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -119,6 +119,22 @@ impl<'w> InteriorMutableWorld<'w> { &self.0.bundles } + /// Reads the current change tick of this world. + #[inline] + pub fn read_change_tick(&self) -> u32 { + self.0.read_change_tick() + } + + #[inline] + pub fn last_change_tick(&self) -> u32 { + self.0.last_change_tick() + } + + #[inline] + pub fn increment_change_tick(&self) -> u32 { + self.0.increment_change_tick() + } + /// Retrieves an [`InteriorMutableEntityRef`] that exposes read and write operations for the given `entity`. /// Similar to the [`InteriorMutableWorld`], you are in charge of making sure that no aliasing rules are violated. pub fn get_entity(&self, entity: Entity) -> Option> { @@ -206,7 +222,7 @@ impl<'w> InteriorMutableWorld<'w> { // - index is in-bounds because the column is initialized and non-empty // - the caller promises that no other reference to the ticks of the same row can exist at the same time let ticks = unsafe { - TicksMut::from_tick_cells(ticks, self.0.last_change_tick(), self.0.read_change_tick()) + TicksMut::from_tick_cells(ticks, self.last_change_tick(), self.read_change_tick()) }; Some(MutUntyped { @@ -250,7 +266,7 @@ impl<'w> InteriorMutableWorld<'w> { // - This caller ensures that nothing aliases `ticks`. // - index is in-bounds because the column is initialized and non-empty let ticks = unsafe { - TicksMut::from_tick_cells(ticks, self.0.last_change_tick(), self.0.read_change_tick()) + TicksMut::from_tick_cells(ticks, self.last_change_tick(), self.read_change_tick()) }; Some(Mut { @@ -283,11 +299,7 @@ impl<'w> InteriorMutableWorld<'w> { value: unsafe { ptr.assert_unique().deref_mut() }, // SAFETY: caller ensures unique access ticks: unsafe { - TicksMut::from_tick_cells( - ticks, - self.0.last_change_tick(), - self.0.read_change_tick(), - ) + TicksMut::from_tick_cells(ticks, self.last_change_tick(), self.read_change_tick()) }, }) } @@ -393,10 +405,7 @@ impl<'w> InteriorMutableEntityRef<'w> { pub unsafe fn get_mut(&self) -> Option> { // SAFETY: same safety requirements unsafe { - self.get_mut_using_ticks( - self.world.0.last_change_tick(), - self.world.0.read_change_tick(), - ) + self.get_mut_using_ticks(self.world.last_change_tick(), self.world.read_change_tick()) } } @@ -487,8 +496,8 @@ impl<'w> InteriorMutableEntityRef<'w> { value: value.assert_unique(), ticks: TicksMut::from_tick_cells( cells, - self.world.0.last_change_tick, - self.world.0.read_change_tick(), + self.world.last_change_tick(), + self.world.read_change_tick(), ), }) } From 2ad5d25db3da79c395ae0bfea06b1df6ff6f30c3 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Fri, 13 Jan 2023 16:22:56 +0100 Subject: [PATCH 16/30] use contains_resource as example for unexposed method --- .../src/world/interior_mutable_world.rs | 97 +++++++++---------- 1 file changed, 45 insertions(+), 52 deletions(-) diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index 6ab9ad7b585bb..2cb2de0987ecd 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -78,7 +78,7 @@ impl<'w> InteriorMutableWorld<'w> { } /// Gets a reference to the [`&World`](crate::world::World) this [`InteriorMutableWorld`] belongs to. - /// This can be used to call methods like [`World::read_change_tick`] which aren't exposed here but don't perform any accesses. + /// This can be used to call methods like [`World::contains_resource`] which aren't exposed and but don't perform any accesses. /// /// **Note**: You *must not* hand out a `&World` reference to arbitrary safe code when the [`InteriorMutableWorld`] is currently /// being used for mutable accesses. @@ -149,9 +149,9 @@ impl<'w> InteriorMutableWorld<'w> { /// Gets a reference to the resource of the given type if it exists /// /// # Safety - /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. - /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, - /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + /// It is the callers responsibility to ensure that + /// - the [`InteriorMutableWorld`] has permission to access the resource + /// - no other mutable references to the resource exist at the same time #[inline] pub unsafe fn get_resource(&self) -> Option<&'w R> { self.0.get_resource::() @@ -165,9 +165,9 @@ impl<'w> InteriorMutableWorld<'w> { /// use this in cases where the actual types are not known at compile time.** /// /// # Safety - /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. - /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, - /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + /// It is the callers responsibility to ensure that + /// - the [`InteriorMutableWorld`] has permission to access the resource + /// - no other mutable references to the resource exist at the same time #[inline] pub unsafe fn get_resource_by_id(&self, component_id: ComponentId) -> Option> { self.0.get_resource_by_id(component_id) @@ -176,9 +176,9 @@ impl<'w> InteriorMutableWorld<'w> { /// Gets a reference to the non-send resource of the given type if it exists /// /// # Safety - /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. - /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, - /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + /// It is the callers responsibility to ensure that + /// - the [`InteriorMutableWorld`] has permission to access the resource + /// - no other mutable references to the resource exist at the same time #[inline] pub unsafe fn get_non_send_resource(&self) -> Option<&'w R> { self.0.get_non_send_resource() @@ -187,9 +187,9 @@ impl<'w> InteriorMutableWorld<'w> { /// Gets a mutable reference to the resource of the given type if it exists /// /// # Safety - /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. - /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, - /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + /// It is the callers responsibility to ensure that + /// - the [`InteriorMutableWorld`] has permission to access the resource mutably + /// - no other references to the resource exist at the same time #[inline] pub unsafe fn get_resource_mut(&self) -> Option> { let component_id = self.0.components.get_resource_id(TypeId::of::())?; @@ -208,9 +208,9 @@ impl<'w> InteriorMutableWorld<'w> { /// use this in cases where the actual types are not known at compile time.** /// /// # Safety - /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. - /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, - /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + /// It is the callers responsibility to ensure that + /// - the [`InteriorMutableWorld`] has permission to access the resource mutably + /// - no other references to the resource exist at the same time #[inline] pub unsafe fn get_resource_mut_by_id( &self, @@ -235,9 +235,9 @@ impl<'w> InteriorMutableWorld<'w> { /// Gets a mutable reference to the non-send resource of the given type if it exists /// /// # Safety - /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. - /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, - /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + /// It is the callers responsibility to ensure that + /// - the [`InteriorMutableWorld`] has permission to access the resource mutably + /// - no other references to the resource exist at the same time #[inline] pub unsafe fn get_non_send_resource_mut(&self) -> Option> { let component_id = self.0.components.get_resource_id(TypeId::of::())?; @@ -248,13 +248,10 @@ impl<'w> InteriorMutableWorld<'w> { impl<'w> InteriorMutableWorld<'w> { /// # Safety + /// It is the callers responsibility to ensure that /// - `component_id` must be assigned to a component of type `R` - /// - Caller must ensure this doesn't violate Rust mutability rules for the given resource. - /// - resource is either Send+Sync or the main thread is validated - /// - /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. - /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, - /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + /// - the [`InteriorMutableWorld`] has permission to access the resource + /// - no other mutable references to the resource exist at the same time #[inline] pub(crate) unsafe fn get_resource_mut_with_id( &self, @@ -277,12 +274,10 @@ impl<'w> InteriorMutableWorld<'w> { } /// # Safety + /// It is the callers responsibility to ensure that /// - `component_id` must be assigned to a component of type `R`. - /// - Caller must ensure this doesn't violate Rust mutability rules for the given resource. - /// - /// All [`InteriorMutableWorld`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. - /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, - /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + /// - the [`InteriorMutableWorld`] has permission to access the resource mutably + /// - no other references to the resource exist at the same time #[inline] pub(crate) unsafe fn get_non_send_mut_with_id( &self, @@ -351,15 +346,14 @@ impl<'w> InteriorMutableEntityRef<'w> { } /// # Safety - /// All [`InteriorMutableEntityRef`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. - /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, - /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + /// It is the callers responsibility to ensure that + /// - the [`InteriorMutableEntityRef`] has permission to access the component + /// - no other mutable references to the component exist at the same time #[inline] pub unsafe fn get(&self) -> Option<&'w T> { // SAFETY: // - entity location is valid - // - archetypes and components come from the same world - // - world access is immutable, lifetime tied to `&self` + // - proper world access is promised by caller unsafe { self.world .0 @@ -378,15 +372,14 @@ impl<'w> InteriorMutableEntityRef<'w> { /// detection in custom runtimes. /// /// # Safety - /// All [`InteriorMutableEntityRef`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. - /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, - /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + /// It is the callers responsibility to ensure that + /// - the [`InteriorMutableEntityRef`] has permission to access the component + /// - no other mutable references to the component exist at the same time #[inline] pub unsafe fn get_change_ticks(&self) -> Option { // SAFETY: // - entity location is valid - // - archetypes and components come from the same world - // - world access is immutable, lifetime tied to `&self` + // - proper world acess is promised by caller unsafe { self.world.0.get_ticks_with_type( TypeId::of::(), @@ -398,9 +391,9 @@ impl<'w> InteriorMutableEntityRef<'w> { } /// # Safety - /// All [`InteriorMutableEntityRef`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. - /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, - /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + /// It is the callers responsibility to ensure that + /// - the [`InteriorMutableEntityRef`] has permission to access the component mutably + /// - no other references to the component exist at the same time #[inline] pub unsafe fn get_mut(&self) -> Option> { // SAFETY: same safety requirements @@ -410,9 +403,9 @@ impl<'w> InteriorMutableEntityRef<'w> { } /// # Safety - /// All [`InteriorMutableEntityRef`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. - /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, - /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + /// It is the callers responsibility to ensure that + /// - the [`InteriorMutableEntityRef`] has permission to access the component mutably + /// - no other references to the component exist at the same time #[inline] pub(crate) unsafe fn get_mut_using_ticks( &self, @@ -451,9 +444,9 @@ impl<'w> InteriorMutableEntityRef<'w> { /// which is only valid while the `'w` borrow of the lifetime is active. /// /// # Safety - /// All [`InteriorMutableEntityRef`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. - /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, - /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + /// It is the callers responsibility to ensure that + /// - the [`InteriorMutableEntityRef`] has permission to access the component + /// - no other mutable references to the component exist at the same time #[inline] pub unsafe fn get_by_id(&self, component_id: ComponentId) -> Option> { let info = self.world.0.components.get_info(component_id)?; @@ -475,9 +468,9 @@ impl<'w> InteriorMutableEntityRef<'w> { /// use this in cases where the actual types are not known at compile time.** /// /// # Safety - /// All [`InteriorMutableEntityRef`] methods take `&self` and thus do not check that there is only one unique reference or multiple shared ones. - /// It is the callers responsibility to make sure that there will never be a mutable reference to a value that has other references pointing to it, - /// and that no arbitrary safe code can access a `&World` while some value is mutably borrowed. + /// It is the callers responsibility to ensure that + /// - the [`InteriorMutableEntityRef`] has permission to access the component mutably + /// - no other references to the component exist at the same time #[inline] pub unsafe fn get_mut_by_id(&self, component_id: ComponentId) -> Option> { let info = self.world.0.components.get_info(component_id)?; From 3d263b5795972fdb1139cbe59a847a87e6a879b3 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Fri, 13 Jan 2023 16:23:39 +0100 Subject: [PATCH 17/30] remove extra space --- crates/bevy_ecs/src/reflect.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/reflect.rs b/crates/bevy_ecs/src/reflect.rs index a5c9164c0ddbd..7982f822a50e3 100644 --- a/crates/bevy_ecs/src/reflect.rs +++ b/crates/bevy_ecs/src/reflect.rs @@ -52,7 +52,7 @@ pub struct ReflectComponentFns { /// Function pointer implementing [`ReflectComponent::reflect()`]. pub reflect: fn(&World, Entity) -> Option<&dyn Reflect>, /// Function pointer implementing [`ReflectComponent::reflect_mut()`]. - /// SAFETY: The function may only be called with an InteriorMutableWorld that can be used to mutably access the relevant component on the given entity + /// SAFETY: The function may only be called with an InteriorMutableWorld that can be used to mutably access the relevant component on the given entity pub reflect_mut: unsafe fn(InteriorMutableWorld<'_>, Entity) -> Option>, /// Function pointer implementing [`ReflectComponent::copy()`]. pub copy: fn(&World, &mut World, Entity, Entity), From 9503a66a43dbf6cbecb2c9e04c1a50441158fc24 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Thu, 19 Jan 2023 21:35:42 +0100 Subject: [PATCH 18/30] add get_non_send_resource[_mut]_by_id --- .../src/world/interior_mutable_world.rs | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index 2cb2de0987ecd..cf48bf169e547 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -184,6 +184,29 @@ impl<'w> InteriorMutableWorld<'w> { self.0.get_non_send_resource() } + /// Gets a `!Send` resource to the resource with the id [`ComponentId`] if it exists. + /// The returned pointer must not be used to modify the resource, and must not be + /// dereferenced after the immutable borrow of the [`World`] ends. + /// + /// **You should prefer to use the typed API [`InteriorMutableWorld::get_non_send_resource`] where possible and only + /// use this in cases where the actual types are not known at compile time.** + /// + /// # Panics + /// This function will panic if it isn't called from the same thread that the resource was inserted from. + /// + /// # Safety + /// It is the callers responsibility to ensure that + /// - the [`InteriorMutableWorld`] has permission to access the resource + /// - no other mutable references to the resource exist at the same time + #[inline] + pub unsafe fn get_non_send_resource_by_id(&self, component_id: ComponentId) -> Option> { + self.0 + .storages + .non_send_resources + .get(component_id)? + .get_data() + } + /// Gets a mutable reference to the resource of the given type if it exists /// /// # Safety @@ -244,6 +267,41 @@ impl<'w> InteriorMutableWorld<'w> { // SAFETY: component_id matches `R`, rest is promised by caller unsafe { self.get_non_send_mut_with_id(component_id) } } + + /// Gets a `!Send` resource to the resource with the id [`ComponentId`] if it exists. + /// The returned pointer may be used to modify the resource, as long as the mutable borrow + /// of the [`World`] is still valid. + /// + /// **You should prefer to use the typed API [`InteriorMutableWorld::get_non_send_resource_mut`] where possible and only + /// use this in cases where the actual types are not known at compile time.** + /// + /// # Panics + /// This function will panic if it isn't called from the same thread that the resource was inserted from. + /// + /// # Safety + /// It is the callers responsibility to ensure that + /// - the [`InteriorMutableWorld`] has permission to access the resource mutably + /// - no other references to the resource exist at the same time + #[inline] + pub unsafe fn get_non_send_resource_mut_by_id( + &mut self, + component_id: ComponentId, + ) -> Option> { + let change_tick = self.read_change_tick(); + let (ptr, ticks) = self.0.get_non_send_with_ticks(component_id)?; + + let ticks = + // SAFETY: This function has exclusive access to the world so nothing aliases `ticks`. + // - index is in-bounds because the column is initialized and non-empty + // - no other reference to the ticks of the same row can exist at the same time + unsafe { TicksMut::from_tick_cells(ticks, self.last_change_tick(), change_tick) }; + + Some(MutUntyped { + // SAFETY: This function has exclusive access to the world so nothing aliases `ptr`. + value: unsafe { ptr.assert_unique() }, + ticks, + }) + } } impl<'w> InteriorMutableWorld<'w> { From 17afd9be14c2ca6e53a9da1413238df94ae0066a Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Thu, 19 Jan 2023 21:37:41 +0100 Subject: [PATCH 19/30] pass self instead of &self --- .../src/world/interior_mutable_world.rs | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index cf48bf169e547..087b5246e46e6 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -85,59 +85,59 @@ impl<'w> InteriorMutableWorld<'w> { /// /// # Safety /// - the world must not be used to access any resources or components. You can use it to safely access metadata. - pub unsafe fn world(&self) -> &'w World { + pub unsafe fn world(self) -> &'w World { self.0 } /// Retrieves this world's [Entities] collection #[inline] - pub fn entities(&self) -> &'w Entities { + pub fn entities(self) -> &'w Entities { &self.0.entities } /// Retrieves this world's [Archetypes] collection #[inline] - pub fn archetypes(&self) -> &'w Archetypes { + pub fn archetypes(self) -> &'w Archetypes { &self.0.archetypes } /// Retrieves this world's [Components] collection #[inline] - pub fn components(&self) -> &'w Components { + pub fn components(self) -> &'w Components { &self.0.components } /// Retrieves this world's [Storages] collection #[inline] - pub fn storages(&self) -> &'w Storages { + pub fn storages(self) -> &'w Storages { &self.0.storages } /// Retrieves this world's [Bundles] collection #[inline] - pub fn bundles(&self) -> &'w Bundles { + pub fn bundles(self) -> &'w Bundles { &self.0.bundles } /// Reads the current change tick of this world. #[inline] - pub fn read_change_tick(&self) -> u32 { + pub fn read_change_tick(self) -> u32 { self.0.read_change_tick() } #[inline] - pub fn last_change_tick(&self) -> u32 { + pub fn last_change_tick(self) -> u32 { self.0.last_change_tick() } #[inline] - pub fn increment_change_tick(&self) -> u32 { + pub fn increment_change_tick(self) -> u32 { self.0.increment_change_tick() } /// Retrieves an [`InteriorMutableEntityRef`] that exposes read and write operations for the given `entity`. /// Similar to the [`InteriorMutableWorld`], you are in charge of making sure that no aliasing rules are violated. - pub fn get_entity(&self, entity: Entity) -> Option> { + pub fn get_entity(self, entity: Entity) -> Option> { let location = self.0.entities.get(entity)?; Some(InteriorMutableEntityRef { world: InteriorMutableWorld(self.0), @@ -153,7 +153,7 @@ impl<'w> InteriorMutableWorld<'w> { /// - the [`InteriorMutableWorld`] has permission to access the resource /// - no other mutable references to the resource exist at the same time #[inline] - pub unsafe fn get_resource(&self) -> Option<&'w R> { + pub unsafe fn get_resource(self) -> Option<&'w R> { self.0.get_resource::() } @@ -169,7 +169,7 @@ impl<'w> InteriorMutableWorld<'w> { /// - the [`InteriorMutableWorld`] has permission to access the resource /// - no other mutable references to the resource exist at the same time #[inline] - pub unsafe fn get_resource_by_id(&self, component_id: ComponentId) -> Option> { + pub unsafe fn get_resource_by_id(self, component_id: ComponentId) -> Option> { self.0.get_resource_by_id(component_id) } @@ -180,7 +180,7 @@ impl<'w> InteriorMutableWorld<'w> { /// - the [`InteriorMutableWorld`] has permission to access the resource /// - no other mutable references to the resource exist at the same time #[inline] - pub unsafe fn get_non_send_resource(&self) -> Option<&'w R> { + pub unsafe fn get_non_send_resource(self) -> Option<&'w R> { self.0.get_non_send_resource() } @@ -199,7 +199,7 @@ impl<'w> InteriorMutableWorld<'w> { /// - the [`InteriorMutableWorld`] has permission to access the resource /// - no other mutable references to the resource exist at the same time #[inline] - pub unsafe fn get_non_send_resource_by_id(&self, component_id: ComponentId) -> Option> { + pub unsafe fn get_non_send_resource_by_id(self, component_id: ComponentId) -> Option> { self.0 .storages .non_send_resources @@ -214,7 +214,7 @@ impl<'w> InteriorMutableWorld<'w> { /// - the [`InteriorMutableWorld`] has permission to access the resource mutably /// - no other references to the resource exist at the same time #[inline] - pub unsafe fn get_resource_mut(&self) -> Option> { + pub unsafe fn get_resource_mut(self) -> Option> { let component_id = self.0.components.get_resource_id(TypeId::of::())?; // SAFETY: // - component_id is of type `R` @@ -262,7 +262,7 @@ impl<'w> InteriorMutableWorld<'w> { /// - the [`InteriorMutableWorld`] has permission to access the resource mutably /// - no other references to the resource exist at the same time #[inline] - pub unsafe fn get_non_send_resource_mut(&self) -> Option> { + pub unsafe fn get_non_send_resource_mut(self) -> Option> { let component_id = self.0.components.get_resource_id(TypeId::of::())?; // SAFETY: component_id matches `R`, rest is promised by caller unsafe { self.get_non_send_mut_with_id(component_id) } @@ -369,37 +369,37 @@ pub struct InteriorMutableEntityRef<'w> { impl<'w> InteriorMutableEntityRef<'w> { #[inline] #[must_use = "Omit the .id() call if you do not need to store the `Entity` identifier."] - pub fn id(&self) -> Entity { + pub fn id(self) -> Entity { self.entity } #[inline] - pub fn location(&self) -> EntityLocation { + pub fn location(self) -> EntityLocation { self.location } #[inline] - pub fn archetype(&self) -> &Archetype { + pub fn archetype(self) -> &'w Archetype { &self.world.0.archetypes[self.location.archetype_id] } #[inline] - pub fn world(&self) -> InteriorMutableWorld<'w> { + pub fn world(self) -> InteriorMutableWorld<'w> { self.world } #[inline] - pub fn contains(&self) -> bool { + pub fn contains(self) -> bool { self.contains_type_id(TypeId::of::()) } #[inline] - pub fn contains_id(&self, component_id: ComponentId) -> bool { + pub fn contains_id(self, component_id: ComponentId) -> bool { entity_ref::contains_component_with_id(self.world.0, component_id, self.location) } #[inline] - pub fn contains_type_id(&self, type_id: TypeId) -> bool { + pub fn contains_type_id(self, type_id: TypeId) -> bool { entity_ref::contains_component_with_type(self.world.0, type_id, self.location) } @@ -408,7 +408,7 @@ impl<'w> InteriorMutableEntityRef<'w> { /// - the [`InteriorMutableEntityRef`] has permission to access the component /// - no other mutable references to the component exist at the same time #[inline] - pub unsafe fn get(&self) -> Option<&'w T> { + pub unsafe fn get(self) -> Option<&'w T> { // SAFETY: // - entity location is valid // - proper world access is promised by caller @@ -434,7 +434,7 @@ impl<'w> InteriorMutableEntityRef<'w> { /// - the [`InteriorMutableEntityRef`] has permission to access the component /// - no other mutable references to the component exist at the same time #[inline] - pub unsafe fn get_change_ticks(&self) -> Option { + pub unsafe fn get_change_ticks(self) -> Option { // SAFETY: // - entity location is valid // - proper world acess is promised by caller @@ -453,7 +453,7 @@ impl<'w> InteriorMutableEntityRef<'w> { /// - the [`InteriorMutableEntityRef`] has permission to access the component mutably /// - no other references to the component exist at the same time #[inline] - pub unsafe fn get_mut(&self) -> Option> { + pub unsafe fn get_mut(self) -> Option> { // SAFETY: same safety requirements unsafe { self.get_mut_using_ticks(self.world.last_change_tick(), self.world.read_change_tick()) @@ -506,7 +506,7 @@ impl<'w> InteriorMutableEntityRef<'w> { /// - the [`InteriorMutableEntityRef`] has permission to access the component /// - no other mutable references to the component exist at the same time #[inline] - pub unsafe fn get_by_id(&self, component_id: ComponentId) -> Option> { + pub unsafe fn get_by_id(self, component_id: ComponentId) -> Option> { let info = self.world.0.components.get_info(component_id)?; // SAFETY: entity_location is valid, component_id is valid as checked by the line above unsafe { @@ -530,7 +530,7 @@ impl<'w> InteriorMutableEntityRef<'w> { /// - the [`InteriorMutableEntityRef`] has permission to access the component mutably /// - no other references to the component exist at the same time #[inline] - pub unsafe fn get_mut_by_id(&self, component_id: ComponentId) -> Option> { + pub unsafe fn get_mut_by_id(self, component_id: ComponentId) -> Option> { let info = self.world.0.components.get_info(component_id)?; // SAFETY: entity_location is valid, component_id is valid as checked by the line above unsafe { From b065073ccba6725f84eb29d08b891771a134a20a Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Thu, 19 Jan 2023 21:49:47 +0100 Subject: [PATCH 20/30] implement EntityMut::get_mut in terms of InteriorMutableEntityRef --- crates/bevy_ecs/src/world/entity_ref.rs | 27 +++++++------------ .../src/world/interior_mutable_world.rs | 18 +++++++++---- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 1ccea738c74b8..6695298e12680 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -13,6 +13,8 @@ use bevy_ptr::{OwningPtr, Ptr}; use bevy_utils::tracing::debug; use std::any::TypeId; +use super::interior_mutable_world::InteriorMutableEntityRef; + /// A read-only reference to a particular [`Entity`] and all of its components #[derive(Copy, Clone)] pub struct EntityRef<'w> { @@ -254,24 +256,13 @@ impl<'w> EntityMut<'w> { #[inline] pub fn get_mut(&mut self) -> Option> { - // SAFETY: world access is unique, and lifetimes enforce correct usage of returned borrow - unsafe { - self.world - .get_component_and_ticks_with_type( - TypeId::of::(), - T::Storage::STORAGE_TYPE, - self.entity, - self.location, - ) - .map(|(value, cells)| Mut { - value: value.assert_unique().deref_mut::(), - ticks: TicksMut::from_tick_cells( - cells, - self.world.last_change_tick, - self.world.read_change_tick(), - ), - }) - } + let interior_mutable = InteriorMutableEntityRef::new( + self.world.as_interior_mutable(), + self.entity, + self.location, + ); + // SAFETY: interiormutableentityref has exclusive access + unsafe { interior_mutable.get_mut() } } /// Retrieves the change ticks for the given component. This can be useful for implementing change diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index 087b5246e46e6..94fe3fa0b0f04 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -139,11 +139,7 @@ impl<'w> InteriorMutableWorld<'w> { /// Similar to the [`InteriorMutableWorld`], you are in charge of making sure that no aliasing rules are violated. pub fn get_entity(self, entity: Entity) -> Option> { let location = self.0.entities.get(entity)?; - Some(InteriorMutableEntityRef { - world: InteriorMutableWorld(self.0), - entity, - location, - }) + Some(InteriorMutableEntityRef::new(self, entity, location)) } /// Gets a reference to the resource of the given type if it exists @@ -367,6 +363,18 @@ pub struct InteriorMutableEntityRef<'w> { } impl<'w> InteriorMutableEntityRef<'w> { + pub(crate) fn new( + world: InteriorMutableWorld<'w>, + entity: Entity, + location: EntityLocation, + ) -> Self { + InteriorMutableEntityRef { + world, + entity, + location, + } + } + #[inline] #[must_use = "Omit the .id() call if you do not need to store the `Entity` identifier."] pub fn id(self) -> Entity { From a5754cc6bef4792e6f5a7a3a8de992f341af4736 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Thu, 19 Jan 2023 21:53:15 +0100 Subject: [PATCH 21/30] add InteriorMutableEntityRef::get_change_ticks_by_id --- .../src/world/interior_mutable_world.rs | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index 94fe3fa0b0f04..00a02fb58d056 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -456,6 +456,34 @@ impl<'w> InteriorMutableEntityRef<'w> { } } + /// Retrieves the change ticks for the given [`ComponentId`]. This can be useful for implementing change + /// detection in custom runtimes. + /// + /// **You should prefer to use the typed API [`InteriorMutableEntityRef::get_change_ticks`] where possible and only + /// use this in cases where the actual component types are not known at + /// compile time.** + /// + /// # Safety + /// It is the callers responsibility to ensure that + /// - the [`InteriorMutableEntityRef`] has permission to access the component + /// - no other mutable references to the component exist at the same time + #[inline] + pub fn get_change_ticks_by_id(&self, component_id: ComponentId) -> Option { + let info = self.world.components().get_info(component_id)?; + // SAFETY: + // - entity location and entity is valid + // - world access is immutable, lifetime tied to `&self` + // - the storage type provided is correct for T + unsafe { + self.world.0.get_ticks( + component_id, + info.storage_type(), + self.entity, + self.location, + ) + } + } + /// # Safety /// It is the callers responsibility to ensure that /// - the [`InteriorMutableEntityRef`] has permission to access the component mutably From 68a9bca895abd639a67604b7bd278d4800cc0834 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Thu, 19 Jan 2023 22:00:05 +0100 Subject: [PATCH 22/30] docs fixes --- crates/bevy_ecs/src/world/interior_mutable_world.rs | 8 ++++---- crates/bevy_ecs/src/world/mod.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index 00a02fb58d056..bd8270c3cf53c 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -147,7 +147,7 @@ impl<'w> InteriorMutableWorld<'w> { /// # Safety /// It is the callers responsibility to ensure that /// - the [`InteriorMutableWorld`] has permission to access the resource - /// - no other mutable references to the resource exist at the same time + /// - no mutable reference to the resource exists at the same time #[inline] pub unsafe fn get_resource(self) -> Option<&'w R> { self.0.get_resource::() @@ -163,7 +163,7 @@ impl<'w> InteriorMutableWorld<'w> { /// # Safety /// It is the callers responsibility to ensure that /// - the [`InteriorMutableWorld`] has permission to access the resource - /// - no other mutable references to the resource exist at the same time + /// - no mutable reference to the resource exists at the same time #[inline] pub unsafe fn get_resource_by_id(self, component_id: ComponentId) -> Option> { self.0.get_resource_by_id(component_id) @@ -174,7 +174,7 @@ impl<'w> InteriorMutableWorld<'w> { /// # Safety /// It is the callers responsibility to ensure that /// - the [`InteriorMutableWorld`] has permission to access the resource - /// - no other mutable references to the resource exist at the same time + /// - no mutable reference to the resource exists at the same time #[inline] pub unsafe fn get_non_send_resource(self) -> Option<&'w R> { self.0.get_non_send_resource() @@ -193,7 +193,7 @@ impl<'w> InteriorMutableWorld<'w> { /// # Safety /// It is the callers responsibility to ensure that /// - the [`InteriorMutableWorld`] has permission to access the resource - /// - no other mutable references to the resource exist at the same time + /// - no mutable reference to the resource exists at the same time #[inline] pub unsafe fn get_non_send_resource_by_id(self, component_id: ComponentId) -> Option> { self.0 diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 89629a5ddb86d..69fab037d702b 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -108,11 +108,11 @@ impl World { self.id } - /// Creates a new [`InteriorMutableWorld`] view with complete read+write access + /// Creates a new [`InteriorMutableWorld`] view with complete read+write access. pub fn as_interior_mutable(&mut self) -> InteriorMutableWorld<'_> { InteriorMutableWorld::new(self) } - /// Creates a new [`InteriorMutableWorld`] view only read access to everything + /// Creates a new [`InteriorMutableWorld`] view with only read access to everything. pub fn as_interior_mutable_readonly(&self) -> InteriorMutableWorld<'_> { InteriorMutableWorld::new(self) } From 4f192330fd722c36bd90b2f1c09fadba86b37cd4 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Thu, 19 Jan 2023 22:00:42 +0100 Subject: [PATCH 23/30] raw pointer -> untyped pointer --- crates/bevy_ecs/src/world/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 69fab037d702b..356a9cb31767a 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1751,7 +1751,7 @@ impl World { } impl World { - /// Get a raw pointer to a particular [`Component`](crate::component::Component) and its [`ComponentTicks`] identified by their [`TypeId`] + /// Get an untyped pointer to a particular [`Component`](crate::component::Component) and its [`ComponentTicks`] identified by their [`TypeId`] /// /// # Safety /// - `storage_type` must accurately reflect where the components for `component_id` are stored. @@ -1770,7 +1770,7 @@ impl World { self.get_component_and_ticks(component_id, storage_type, entity, location) } - /// Get a raw pointer to a particular [`Component`](crate::component::Component) and its [`ComponentTicks`] + /// Get an untyped pointer to a particular [`Component`](crate::component::Component) and its [`ComponentTicks`] /// /// # Safety /// - `location` must refer to an archetype that contains `entity` @@ -1802,7 +1802,7 @@ impl World { } } - /// Get a raw pointer to a particular [`Component`](crate::component::Component) on a particular [`Entity`], identified by the component's type + /// Get an untyped pointer to a particular [`Component`](crate::component::Component) on a particular [`Entity`], identified by the component's type /// /// # Safety /// - `location` must refer to an archetype that contains `entity` @@ -1822,7 +1822,7 @@ impl World { self.get_component(component_id, storage_type, entity, location) } - /// Get a raw pointer to a particular [`Component`](crate::component::Component) on a particular [`Entity`] in the provided [`World`](crate::world::World). + /// Get an untyped pointer to a particular [`Component`](crate::component::Component) on a particular [`Entity`] in the provided [`World`](crate::world::World). /// /// # Safety /// - `location` must refer to an archetype that contains `entity` @@ -1849,7 +1849,7 @@ impl World { } } - /// Get a raw pointer to the [`ComponentTicks`] on a particular [`Entity`], identified by the component's [`TypeId`] + /// Get an untyped pointer to the [`ComponentTicks`] on a particular [`Entity`], identified by the component's [`TypeId`] /// /// # Safety /// - `location` must refer to an archetype that contains `entity` @@ -1869,7 +1869,7 @@ impl World { self.get_ticks(component_id, storage_type, entity, location) } - /// Get a raw pointer to the [`ComponentTicks`] on a particular [`Entity`] + /// Get an untyped pointer to the [`ComponentTicks`] on a particular [`Entity`] /// /// # Safety /// - `location` must refer to an archetype that contains `entity` From d637af0fc896224a7fa0897f7c94d281ae7e6d77 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Thu, 19 Jan 2023 22:14:50 +0100 Subject: [PATCH 24/30] more docs fixes --- crates/bevy_asset/src/reflect.rs | 2 +- crates/bevy_ecs/src/reflect.rs | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/bevy_asset/src/reflect.rs b/crates/bevy_asset/src/reflect.rs index f5fee49caffab..dbceb24e4a46e 100644 --- a/crates/bevy_asset/src/reflect.rs +++ b/crates/bevy_asset/src/reflect.rs @@ -61,7 +61,7 @@ impl ReflectAsset { unsafe { (self.get_unchecked_mut)(world.as_interior_mutable(), handle) } } - /// Equivalent of [`Assets::get_mut`], but works with a [`InteriorMutableWorld`]. + /// Equivalent of [`Assets::get_mut`], but works with an [`InteriorMutableWorld`]. /// /// Only use this method when you have ensured that you are the *only* one with access to the [`Assets`] resource of the asset type. /// Furthermore, this does *not* allow you to have look up two distinct handles, diff --git a/crates/bevy_ecs/src/reflect.rs b/crates/bevy_ecs/src/reflect.rs index 7982f822a50e3..6fba1dbd666f3 100644 --- a/crates/bevy_ecs/src/reflect.rs +++ b/crates/bevy_ecs/src/reflect.rs @@ -52,7 +52,9 @@ pub struct ReflectComponentFns { /// Function pointer implementing [`ReflectComponent::reflect()`]. pub reflect: fn(&World, Entity) -> Option<&dyn Reflect>, /// Function pointer implementing [`ReflectComponent::reflect_mut()`]. - /// SAFETY: The function may only be called with an InteriorMutableWorld that can be used to mutably access the relevant component on the given entity + /// + /// # Safety + /// The function may only be called with an [`InteriorMutableWorld`] that can be used to mutably access the relevant component on the given entity. pub reflect_mut: unsafe fn(InteriorMutableWorld<'_>, Entity) -> Option>, /// Function pointer implementing [`ReflectComponent::copy()`]. pub copy: fn(&World, &mut World, Entity, Entity), @@ -211,8 +213,8 @@ impl FromType for ReflectComponent { }, reflect_mut: |world, entity| { // SAFETY: reflect_mut is an unsafe function pointer used by - // 1. `reflect_unchecked_mut` which must be called with an InteriorMutableWorld with access the the component `C` on the `entity`, and - // 2. reflect_mut, which has mutable world access + // 1. `reflect_unchecked_mut` which must be called with an InteriorMutableWorld with access to the the component `C` on the `entity`, and + // 2. `reflect_mut`, which has mutable world access unsafe { world.get_entity(entity)?.get_mut::().map(|c| Mut { value: c.value as &mut dyn Reflect, @@ -264,7 +266,9 @@ pub struct ReflectResourceFns { /// Function pointer implementing [`ReflectResource::reflect()`]. pub reflect: fn(&World) -> Option<&dyn Reflect>, /// Function pointer implementing [`ReflectResource::reflect_unchecked_mut()`]. - /// SAFETY: The function may only be called with an InteriorMutableWorld that can be used to mutably access the relevant resource + /// + /// # Safety + /// The function may only be called with an [`InteriorMutableWorld`] that can be used to mutably access the relevant resource. pub reflect_unchecked_mut: unsafe fn(InteriorMutableWorld<'_>) -> Option>, /// Function pointer implementing [`ReflectResource::copy()`]. pub copy: fn(&World, &mut World), @@ -320,7 +324,7 @@ impl ReflectResource { /// # Safety /// This method does not prevent you from having two mutable pointers to the same data, /// violating Rust's aliasing rules. To avoid this: - /// * Only call this method with a [`InteriorMutableWorld`] which can be used to mutably access the resource. + /// * Only call this method with an [`InteriorMutableWorld`] which can be used to mutably access the resource. /// * Don't call this method more than once in the same scope for a given [`Resource`]. pub unsafe fn reflect_unchecked_mut<'w>( &self, From efbebce2964f61e8d03d102523ed504f402622ce Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Tue, 24 Jan 2023 15:39:08 +0100 Subject: [PATCH 25/30] add missing safety comment --- crates/bevy_asset/src/reflect.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/bevy_asset/src/reflect.rs b/crates/bevy_asset/src/reflect.rs index dbceb24e4a46e..8f3dc0e6d9954 100644 --- a/crates/bevy_asset/src/reflect.rs +++ b/crates/bevy_asset/src/reflect.rs @@ -145,6 +145,8 @@ impl FromType for ReflectAsset { asset.map(|asset| asset as &dyn Reflect) }, get_unchecked_mut: |world, handle| { + // SAFETY: `get_unchecked_mut` must be callied with `InteriorMutableWorld` having access to `Assets`, + // and must ensure to only have at most one reference to it live at all times. let assets = unsafe { world.get_resource_mut::>().unwrap().into_inner() }; let asset = assets.get_mut(&handle.typed()); asset.map(|asset| asset as &mut dyn Reflect) From ed140f5a2ce6b519157338e90351859900712b67 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Tue, 24 Jan 2023 15:39:43 +0100 Subject: [PATCH 26/30] add missing newline --- crates/bevy_ecs/src/world/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 356a9cb31767a..1bc628d3fdfee 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -112,6 +112,7 @@ impl World { pub fn as_interior_mutable(&mut self) -> InteriorMutableWorld<'_> { InteriorMutableWorld::new(self) } + /// Creates a new [`InteriorMutableWorld`] view with only read access to everything. pub fn as_interior_mutable_readonly(&self) -> InteriorMutableWorld<'_> { InteriorMutableWorld::new(self) From e20ca25fd916d743a748501ccf257ba3107ea6d8 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Tue, 24 Jan 2023 15:57:27 +0100 Subject: [PATCH 27/30] make unsafe fn unsafe --- crates/bevy_ecs/src/world/interior_mutable_world.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/interior_mutable_world.rs index bd8270c3cf53c..e58a32e3451a1 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/interior_mutable_world.rs @@ -468,7 +468,10 @@ impl<'w> InteriorMutableEntityRef<'w> { /// - the [`InteriorMutableEntityRef`] has permission to access the component /// - no other mutable references to the component exist at the same time #[inline] - pub fn get_change_ticks_by_id(&self, component_id: ComponentId) -> Option { + pub unsafe fn get_change_ticks_by_id( + &self, + component_id: ComponentId, + ) -> Option { let info = self.world.components().get_info(component_id)?; // SAFETY: // - entity location and entity is valid From 3b3901871e6e1167b8f41c2d80ef839b0370b11a Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Tue, 24 Jan 2023 15:57:40 +0100 Subject: [PATCH 28/30] reuse InteriorMutableEntityRef more --- crates/bevy_ecs/src/world/entity_ref.rs | 133 +++++++----------------- 1 file changed, 37 insertions(+), 96 deletions(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 6695298e12680..c5b34ac976396 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -42,6 +42,14 @@ impl<'w> EntityRef<'w> { } } + fn as_interior_mutable_readonly(&self) -> InteriorMutableEntityRef<'w> { + InteriorMutableEntityRef::new( + self.world.as_interior_mutable_readonly(), + self.entity, + self.location, + ) + } + #[inline] #[must_use = "Omit the .id() call if you do not need to store the `Entity` identifier."] pub fn id(&self) -> Entity { @@ -80,39 +88,16 @@ impl<'w> EntityRef<'w> { #[inline] pub fn get(&self) -> Option<&'w T> { - // SAFETY: - // - entity location and entity is valid - // - the storage type provided is correct for T - // - world access is immutable, lifetime tied to `&self` - unsafe { - self.world - .get_component_with_type( - TypeId::of::(), - T::Storage::STORAGE_TYPE, - self.entity, - self.location, - ) - // SAFETY: returned component is of type T - .map(|value| value.deref::()) - } + // SAFETY: &self implies shared access for duration of returned value + unsafe { self.as_interior_mutable_readonly().get::() } } /// 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 { - // SAFETY: - // - entity location and entity is valid - // - world access is immutable, lifetime tied to `&self` - // - the storage type provided is correct for T - unsafe { - self.world.get_ticks_with_type( - TypeId::of::(), - T::Storage::STORAGE_TYPE, - self.entity, - self.location, - ) - } + // SAFETY: &self implies shared access + unsafe { self.as_interior_mutable_readonly().get_change_ticks::() } } /// Retrieves the change ticks for the given [`ComponentId`]. This can be useful for implementing change @@ -123,18 +108,10 @@ impl<'w> EntityRef<'w> { /// compile time.** #[inline] pub fn get_change_ticks_by_id(&self, component_id: ComponentId) -> Option { - let info = self.world.components().get_info(component_id)?; - // SAFETY: - // - entity location and entity is valid - // - world access is immutable, lifetime tied to `&self` - // - the storage type provided is correct for T + // SAFETY: &self implies shared access unsafe { - self.world.get_ticks( - component_id, - info.storage_type(), - self.entity, - self.location, - ) + self.as_interior_mutable_readonly() + .get_change_ticks_by_id(component_id) } } } @@ -150,19 +127,8 @@ impl<'w> EntityRef<'w> { /// which is only valid while the `'w` borrow of the lifetime is active. #[inline] pub fn get_by_id(&self, component_id: ComponentId) -> Option> { - let info = self.world.components().get_info(component_id)?; - // SAFETY: - // - entity_location and entity are valid - // - component_id is valid as checked by the line above - // - the storage type is accurate as checked by the fetched ComponentInfo - unsafe { - self.world.get_component( - component_id, - info.storage_type(), - self.entity, - self.location, - ) - } + // SAFETY: &self implies shared access for duration of returned value + unsafe { self.as_interior_mutable_readonly().get_by_id(component_id) } } } @@ -182,6 +148,17 @@ pub struct EntityMut<'w> { } impl<'w> EntityMut<'w> { + fn as_interior_mutable_readonly(&self) -> InteriorMutableEntityRef<'_> { + InteriorMutableEntityRef::new( + self.world.as_interior_mutable_readonly(), + self.entity, + self.location, + ) + } + fn as_interior_mutable(&mut self) -> InteriorMutableEntityRef<'_> { + InteriorMutableEntityRef::new(self.world.as_interior_mutable(), self.entity, self.location) + } + /// # Safety /// /// - `entity` must be valid for `world`: the generation should match that of the entity at the same index. @@ -237,50 +214,22 @@ impl<'w> EntityMut<'w> { #[inline] pub fn get(&self) -> Option<&'_ T> { - // SAFETY: - // - entity location is valid - // - world access is immutable, lifetime tied to `&self` - // - the storage type provided is correct for T - unsafe { - self.world - .get_component_with_type( - TypeId::of::(), - T::Storage::STORAGE_TYPE, - self.entity, - self.location, - ) - // SAFETY: returned component is of type T - .map(|value| value.deref::()) - } + // SAFETY: &self implies shared access for duration of returned value + unsafe { self.as_interior_mutable_readonly().get::() } } #[inline] pub fn get_mut(&mut self) -> Option> { - let interior_mutable = InteriorMutableEntityRef::new( - self.world.as_interior_mutable(), - self.entity, - self.location, - ); - // SAFETY: interiormutableentityref has exclusive access - unsafe { interior_mutable.get_mut() } + // SAFETY: &mut self implies exclusive access for duration of returned value + unsafe { self.as_interior_mutable().get_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 { - // SAFETY: - // - entity location is valid - // - world access is immutable, lifetime tied to `&self` - // - the storage type provided is correct for T - unsafe { - self.world.get_ticks_with_type( - TypeId::of::(), - T::Storage::STORAGE_TYPE, - self.entity, - self.location, - ) - } + // SAFETY: &self implies shared access + unsafe { self.as_interior_mutable_readonly().get_change_ticks::() } } /// Retrieves the change ticks for the given [`ComponentId`]. This can be useful for implementing change @@ -291,18 +240,10 @@ impl<'w> EntityMut<'w> { /// compile time.** #[inline] pub fn get_change_ticks_by_id(&self, component_id: ComponentId) -> Option { - let info = self.world.components().get_info(component_id)?; - // SAFETY: - // - entity location is valid - // - world access is immutable, lifetime tied to `&self` - // - the storage type provided is correct for T + // SAFETY: &self implies shared access unsafe { - self.world.get_ticks( - component_id, - info.storage_type(), - self.entity, - self.location, - ) + self.as_interior_mutable_readonly() + .get_change_ticks_by_id(component_id) } } From d6f742b15ad0822021a6f7e0bfbc5984b8df799b Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Tue, 24 Jan 2023 16:03:18 +0100 Subject: [PATCH 29/30] InteriorMutableWorld -> UnsafeWorldCell --- crates/bevy_asset/src/reflect.rs | 21 ++-- crates/bevy_ecs/src/reflect.rs | 24 ++--- crates/bevy_ecs/src/system/query.rs | 2 +- crates/bevy_ecs/src/system/system_param.rs | 4 +- crates/bevy_ecs/src/world/entity_ref.rs | 38 ++++--- crates/bevy_ecs/src/world/mod.rs | 30 +++--- ..._mutable_world.rs => unsafe_world_cell.rs} | 102 +++++++++--------- crates/bevy_ecs/src/world/world_cell.rs | 6 +- 8 files changed, 115 insertions(+), 112 deletions(-) rename crates/bevy_ecs/src/world/{interior_mutable_world.rs => unsafe_world_cell.rs} (83%) diff --git a/crates/bevy_asset/src/reflect.rs b/crates/bevy_asset/src/reflect.rs index 8f3dc0e6d9954..27a0d8ce24cea 100644 --- a/crates/bevy_asset/src/reflect.rs +++ b/crates/bevy_asset/src/reflect.rs @@ -1,6 +1,6 @@ use std::any::{Any, TypeId}; -use bevy_ecs::world::{interior_mutable_world::InteriorMutableWorld, World}; +use bevy_ecs::world::{unsafe_world_cell::UnsafeWorldCell, World}; use bevy_reflect::{FromReflect, FromType, Reflect, Uuid}; use crate::{Asset, Assets, Handle, HandleId, HandleUntyped}; @@ -21,8 +21,7 @@ pub struct ReflectAsset { // SAFETY: // - may only be called with a [`IteriorMutableWorld`] which can be used to access the corresponding `Assets` resource mutably // - may only be used to access **at most one** access at once - get_unchecked_mut: - unsafe fn(InteriorMutableWorld<'_>, HandleUntyped) -> Option<&mut dyn Reflect>, + get_unchecked_mut: unsafe fn(UnsafeWorldCell<'_>, HandleUntyped) -> Option<&mut dyn Reflect>, add: fn(&mut World, &dyn Reflect) -> HandleUntyped, set: fn(&mut World, HandleUntyped, &dyn Reflect) -> HandleUntyped, len: fn(&World) -> usize, @@ -58,10 +57,10 @@ impl ReflectAsset { handle: HandleUntyped, ) -> Option<&'w mut dyn Reflect> { // SAFETY: unique world access - unsafe { (self.get_unchecked_mut)(world.as_interior_mutable(), handle) } + unsafe { (self.get_unchecked_mut)(world.as_unsafe_world_cell(), handle) } } - /// Equivalent of [`Assets::get_mut`], but works with an [`InteriorMutableWorld`]. + /// Equivalent of [`Assets::get_mut`], but works with an [`UnsafeWorldCell`]. /// /// Only use this method when you have ensured that you are the *only* one with access to the [`Assets`] resource of the asset type. /// Furthermore, this does *not* allow you to have look up two distinct handles, @@ -74,9 +73,9 @@ impl ReflectAsset { /// # let mut world: World = unimplemented!(); /// # let handle_1: HandleUntyped = unimplemented!(); /// # let handle_2: HandleUntyped = unimplemented!(); - /// let interior_mutable_world = world.as_interior_mutable(); - /// let a = unsafe { reflect_asset.get_unchecked_mut(interior_mutable_world, handle_1).unwrap() }; - /// let b = unsafe { reflect_asset.get_unchecked_mut(interior_mutable_world, handle_2).unwrap() }; + /// let unsafe_world_cell = world.as_unsafe_world_cell(); + /// let a = unsafe { reflect_asset.get_unchecked_mut(unsafe_world_cell, handle_1).unwrap() }; + /// let b = unsafe { reflect_asset.get_unchecked_mut(unsafe_world_cell, handle_2).unwrap() }; /// // ^ not allowed, two mutable references through the same asset resource, even though the /// // handles are distinct /// @@ -86,11 +85,11 @@ impl ReflectAsset { /// # Safety /// This method does not prevent you from having two mutable pointers to the same data, /// violating Rust's aliasing rules. To avoid this: - /// * Only call this method if you know that the [`InteriorMutableWorld`] may be used to access the corresponding `Assets` + /// * Only call this method if you know that the [`UnsafeWorldCell`] may be used to access the corresponding `Assets` /// * Don't call this method more than once in the same scope. pub unsafe fn get_unchecked_mut<'w>( &self, - world: InteriorMutableWorld<'w>, + world: UnsafeWorldCell<'w>, handle: HandleUntyped, ) -> Option<&'w mut dyn Reflect> { // SAFETY: requirements are deferred to the caller @@ -145,7 +144,7 @@ impl FromType for ReflectAsset { asset.map(|asset| asset as &dyn Reflect) }, get_unchecked_mut: |world, handle| { - // SAFETY: `get_unchecked_mut` must be callied with `InteriorMutableWorld` having access to `Assets`, + // SAFETY: `get_unchecked_mut` must be callied with `UnsafeWorldCell` having access to `Assets`, // and must ensure to only have at most one reference to it live at all times. let assets = unsafe { world.get_resource_mut::>().unwrap().into_inner() }; let asset = assets.get_mut(&handle.typed()); diff --git a/crates/bevy_ecs/src/reflect.rs b/crates/bevy_ecs/src/reflect.rs index 6fba1dbd666f3..b865844731b66 100644 --- a/crates/bevy_ecs/src/reflect.rs +++ b/crates/bevy_ecs/src/reflect.rs @@ -5,7 +5,7 @@ use crate::{ component::Component, entity::{Entity, EntityMap, MapEntities, MapEntitiesError}, system::Resource, - world::{interior_mutable_world::InteriorMutableWorld, FromWorld, World}, + world::{unsafe_world_cell::UnsafeWorldCell, FromWorld, World}, }; use bevy_reflect::{ impl_from_reflect_value, impl_reflect_value, FromType, Reflect, ReflectDeserialize, @@ -54,8 +54,8 @@ pub struct ReflectComponentFns { /// Function pointer implementing [`ReflectComponent::reflect_mut()`]. /// /// # Safety - /// The function may only be called with an [`InteriorMutableWorld`] that can be used to mutably access the relevant component on the given entity. - pub reflect_mut: unsafe fn(InteriorMutableWorld<'_>, Entity) -> Option>, + /// The function may only be called with an [`UnsafeWorldCell`] that can be used to mutably access the relevant component on the given entity. + pub reflect_mut: unsafe fn(UnsafeWorldCell<'_>, Entity) -> Option>, /// Function pointer implementing [`ReflectComponent::copy()`]. pub copy: fn(&World, &mut World, Entity, Entity), } @@ -120,17 +120,17 @@ impl ReflectComponent { entity: Entity, ) -> Option> { // SAFETY: unique world access - unsafe { (self.0.reflect_mut)(world.as_interior_mutable(), entity) } + unsafe { (self.0.reflect_mut)(world.as_unsafe_world_cell(), entity) } } /// # Safety /// This method does not prevent you from having two mutable pointers to the same data, /// violating Rust's aliasing rules. To avoid this: - /// * Only call this method with a [`InteriorMutableWorld`] that may be used to mutably access the component on the entity `entity` + /// * Only call this method with a [`UnsafeWorldCell`] that may be used to mutably access the component on the entity `entity` /// * Don't call this method more than once in the same scope for a given [`Component`]. pub unsafe fn reflect_unchecked_mut<'a>( &self, - world: InteriorMutableWorld<'a>, + world: UnsafeWorldCell<'a>, entity: Entity, ) -> Option> { // SAFETY: safety requirements deferred to caller @@ -213,7 +213,7 @@ impl FromType for ReflectComponent { }, reflect_mut: |world, entity| { // SAFETY: reflect_mut is an unsafe function pointer used by - // 1. `reflect_unchecked_mut` which must be called with an InteriorMutableWorld with access to the the component `C` on the `entity`, and + // 1. `reflect_unchecked_mut` which must be called with an UnsafeWorldCell with access to the the component `C` on the `entity`, and // 2. `reflect_mut`, which has mutable world access unsafe { world.get_entity(entity)?.get_mut::().map(|c| Mut { @@ -268,8 +268,8 @@ pub struct ReflectResourceFns { /// Function pointer implementing [`ReflectResource::reflect_unchecked_mut()`]. /// /// # Safety - /// The function may only be called with an [`InteriorMutableWorld`] that can be used to mutably access the relevant resource. - pub reflect_unchecked_mut: unsafe fn(InteriorMutableWorld<'_>) -> Option>, + /// The function may only be called with an [`UnsafeWorldCell`] that can be used to mutably access the relevant resource. + pub reflect_unchecked_mut: unsafe fn(UnsafeWorldCell<'_>) -> Option>, /// Function pointer implementing [`ReflectResource::copy()`]. pub copy: fn(&World, &mut World), } @@ -318,17 +318,17 @@ impl ReflectResource { /// Gets the value of this [`Resource`] type from the world as a mutable reflected reference. pub fn reflect_mut<'a>(&self, world: &'a mut World) -> Option> { // SAFETY: unique world access - unsafe { (self.0.reflect_unchecked_mut)(world.as_interior_mutable()) } + unsafe { (self.0.reflect_unchecked_mut)(world.as_unsafe_world_cell()) } } /// # Safety /// This method does not prevent you from having two mutable pointers to the same data, /// violating Rust's aliasing rules. To avoid this: - /// * Only call this method with an [`InteriorMutableWorld`] which can be used to mutably access the resource. + /// * Only call this method with an [`UnsafeWorldCell`] which can be used to mutably access the resource. /// * Don't call this method more than once in the same scope for a given [`Resource`]. pub unsafe fn reflect_unchecked_mut<'w>( &self, - world: InteriorMutableWorld<'w>, + world: UnsafeWorldCell<'w>, ) -> Option> { // SAFETY: caller promises to uphold uniqueness guarantees (self.0.reflect_unchecked_mut)(world) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index b86cd0eebbb8e..4d3a12ed49bc7 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1134,7 +1134,7 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { } let world = self.world; let entity_ref = world - .as_interior_mutable_migration_internal() + .as_unsafe_world_cell_migration_internal() .get_entity(entity) .ok_or(QueryComponentError::NoSuchEntity)?; let component_id = world diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index ae25ada35f861..4dc3a85bdbf1b 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -541,7 +541,7 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { change_tick: u32, ) -> Self::Item<'w, 's> { let value = world - .as_interior_mutable_migration_internal() + .as_unsafe_world_cell_migration_internal() .get_resource_mut_with_id(component_id) .unwrap_or_else(|| { panic!( @@ -579,7 +579,7 @@ unsafe impl<'a, T: Resource> SystemParam for Option> { change_tick: u32, ) -> Self::Item<'w, 's> { world - .as_interior_mutable_migration_internal() + .as_unsafe_world_cell_migration_internal() .get_resource_mut_with_id(component_id) .map(|value| ResMut { value: value.value, diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index c5b34ac976396..d57d3b52a34e4 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -13,7 +13,7 @@ use bevy_ptr::{OwningPtr, Ptr}; use bevy_utils::tracing::debug; use std::any::TypeId; -use super::interior_mutable_world::InteriorMutableEntityRef; +use super::unsafe_world_cell::UnsafeEntityRefCell; /// A read-only reference to a particular [`Entity`] and all of its components #[derive(Copy, Clone)] @@ -42,9 +42,9 @@ impl<'w> EntityRef<'w> { } } - fn as_interior_mutable_readonly(&self) -> InteriorMutableEntityRef<'w> { - InteriorMutableEntityRef::new( - self.world.as_interior_mutable_readonly(), + fn as_unsafe_world_cell_readonly(&self) -> UnsafeEntityRefCell<'w> { + UnsafeEntityRefCell::new( + self.world.as_unsafe_world_cell_readonly(), self.entity, self.location, ) @@ -89,7 +89,7 @@ impl<'w> EntityRef<'w> { #[inline] pub fn get(&self) -> Option<&'w T> { // SAFETY: &self implies shared access for duration of returned value - unsafe { self.as_interior_mutable_readonly().get::() } + unsafe { self.as_unsafe_world_cell_readonly().get::() } } /// Retrieves the change ticks for the given component. This can be useful for implementing change @@ -97,7 +97,7 @@ impl<'w> EntityRef<'w> { #[inline] pub fn get_change_ticks(&self) -> Option { // SAFETY: &self implies shared access - unsafe { self.as_interior_mutable_readonly().get_change_ticks::() } + unsafe { self.as_unsafe_world_cell_readonly().get_change_ticks::() } } /// Retrieves the change ticks for the given [`ComponentId`]. This can be useful for implementing change @@ -110,7 +110,7 @@ impl<'w> EntityRef<'w> { pub fn get_change_ticks_by_id(&self, component_id: ComponentId) -> Option { // SAFETY: &self implies shared access unsafe { - self.as_interior_mutable_readonly() + self.as_unsafe_world_cell_readonly() .get_change_ticks_by_id(component_id) } } @@ -128,7 +128,7 @@ impl<'w> EntityRef<'w> { #[inline] pub fn get_by_id(&self, component_id: ComponentId) -> Option> { // SAFETY: &self implies shared access for duration of returned value - unsafe { self.as_interior_mutable_readonly().get_by_id(component_id) } + unsafe { self.as_unsafe_world_cell_readonly().get_by_id(component_id) } } } @@ -148,15 +148,19 @@ pub struct EntityMut<'w> { } impl<'w> EntityMut<'w> { - fn as_interior_mutable_readonly(&self) -> InteriorMutableEntityRef<'_> { - InteriorMutableEntityRef::new( - self.world.as_interior_mutable_readonly(), + fn as_unsafe_world_cell_readonly(&self) -> UnsafeEntityRefCell<'_> { + UnsafeEntityRefCell::new( + self.world.as_unsafe_world_cell_readonly(), self.entity, self.location, ) } - fn as_interior_mutable(&mut self) -> InteriorMutableEntityRef<'_> { - InteriorMutableEntityRef::new(self.world.as_interior_mutable(), self.entity, self.location) + fn as_unsafe_world_cell(&mut self) -> UnsafeEntityRefCell<'_> { + UnsafeEntityRefCell::new( + self.world.as_unsafe_world_cell(), + self.entity, + self.location, + ) } /// # Safety @@ -215,13 +219,13 @@ impl<'w> EntityMut<'w> { #[inline] pub fn get(&self) -> Option<&'_ T> { // SAFETY: &self implies shared access for duration of returned value - unsafe { self.as_interior_mutable_readonly().get::() } + unsafe { self.as_unsafe_world_cell_readonly().get::() } } #[inline] pub fn get_mut(&mut self) -> Option> { // SAFETY: &mut self implies exclusive access for duration of returned value - unsafe { self.as_interior_mutable().get_mut() } + unsafe { self.as_unsafe_world_cell().get_mut() } } /// Retrieves the change ticks for the given component. This can be useful for implementing change @@ -229,7 +233,7 @@ impl<'w> EntityMut<'w> { #[inline] pub fn get_change_ticks(&self) -> Option { // SAFETY: &self implies shared access - unsafe { self.as_interior_mutable_readonly().get_change_ticks::() } + unsafe { self.as_unsafe_world_cell_readonly().get_change_ticks::() } } /// Retrieves the change ticks for the given [`ComponentId`]. This can be useful for implementing change @@ -242,7 +246,7 @@ impl<'w> EntityMut<'w> { pub fn get_change_ticks_by_id(&self, component_id: ComponentId) -> Option { // SAFETY: &self implies shared access unsafe { - self.as_interior_mutable_readonly() + self.as_unsafe_world_cell_readonly() .get_change_ticks_by_id(component_id) } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 1bc628d3fdfee..87913d2b59ddb 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1,6 +1,6 @@ mod entity_ref; -pub mod interior_mutable_world; mod spawn_batch; +pub mod unsafe_world_cell; mod world_cell; pub use crate::change_detection::{Mut, Ref, CHECK_TICK_THRESHOLD}; @@ -34,7 +34,7 @@ mod identifier; pub use identifier::WorldId; -use self::interior_mutable_world::InteriorMutableWorld; +use self::unsafe_world_cell::UnsafeWorldCell; /// Stores and exposes operations on [entities](Entity), [components](Component), resources, /// and their associated metadata. @@ -108,21 +108,21 @@ impl World { self.id } - /// Creates a new [`InteriorMutableWorld`] view with complete read+write access. - pub fn as_interior_mutable(&mut self) -> InteriorMutableWorld<'_> { - InteriorMutableWorld::new(self) + /// Creates a new [`UnsafeWorldCell`] view with complete read+write access. + pub fn as_unsafe_world_cell(&mut self) -> UnsafeWorldCell<'_> { + UnsafeWorldCell::new(self) } - /// Creates a new [`InteriorMutableWorld`] view with only read access to everything. - pub fn as_interior_mutable_readonly(&self) -> InteriorMutableWorld<'_> { - InteriorMutableWorld::new(self) + /// Creates a new [`UnsafeWorldCell`] view with only read access to everything. + pub fn as_unsafe_world_cell_readonly(&self) -> UnsafeWorldCell<'_> { + UnsafeWorldCell::new(self) } - /// Creates a new [`InteriorMutableWorld`] with read+write access from a [&World](World). - /// This is only a temporary measure until every `&World` that is semantically a [`InteriorMutableWorld`] + /// Creates a new [`UnsafeWorldCell`] with read+write access from a [&World](World). + /// This is only a temporary measure until every `&World` that is semantically a [`UnsafeWorldCell`] /// has been replaced. - pub(crate) fn as_interior_mutable_migration_internal(&self) -> InteriorMutableWorld<'_> { - InteriorMutableWorld::new(self) + pub(crate) fn as_unsafe_world_cell_migration_internal(&self) -> UnsafeWorldCell<'_> { + UnsafeWorldCell::new(self) } /// Retrieves this world's [Entities] collection @@ -1011,7 +1011,7 @@ impl World { #[inline] pub fn get_resource_mut(&mut self) -> Option> { // SAFETY: unique world access - unsafe { self.as_interior_mutable().get_resource_mut() } + unsafe { self.as_unsafe_world_cell().get_resource_mut() } } /// Gets a mutable reference to the resource of type `T` if it exists, @@ -1108,7 +1108,7 @@ impl World { #[inline] pub fn get_non_send_resource_mut(&mut self) -> Option> { // SAFETY: unique world access - unsafe { self.as_interior_mutable().get_non_send_resource_mut() } + unsafe { self.as_unsafe_world_cell().get_non_send_resource_mut() } } // Shorthand helper function for getting the data and change ticks for a resource. @@ -1626,7 +1626,7 @@ impl World { pub fn get_resource_mut_by_id(&mut self, component_id: ComponentId) -> Option> { // SAFETY: unique world access unsafe { - self.as_interior_mutable() + self.as_unsafe_world_cell() .get_resource_mut_by_id(component_id) } } diff --git a/crates/bevy_ecs/src/world/interior_mutable_world.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs similarity index 83% rename from crates/bevy_ecs/src/world/interior_mutable_world.rs rename to crates/bevy_ecs/src/world/unsafe_world_cell.rs index e58a32e3451a1..c90385526c365 100644 --- a/crates/bevy_ecs/src/world/interior_mutable_world.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -36,7 +36,7 @@ use std::any::TypeId; /// /// ### Example Usage /// -/// [`InteriorMutableWorld`] can be used as a building block for writing APIs that safely allow disjoint access into the world. +/// [`UnsafeWorldCell`] can be used as a building block for writing APIs that safely allow disjoint access into the world. /// In the following example, the world is split into a resource access half and a component access half, where each one can /// safely hand out mutable references. /// @@ -44,16 +44,16 @@ use std::any::TypeId; /// use bevy_ecs::world::World; /// use bevy_ecs::change_detection::Mut; /// use bevy_ecs::system::Resource; -/// use bevy_ecs::world::interior_mutable_world::InteriorMutableWorld; +/// use bevy_ecs::world::unsafe_world_cell::UnsafeWorldCell; /// /// // INVARIANT: existence of this struct means that users of it are the only ones being able to access resources in the world -/// struct OnlyResourceAccessWorld<'w>(InteriorMutableWorld<'w>); +/// struct OnlyResourceAccessWorld<'w>(UnsafeWorldCell<'w>); /// // INVARIANT: existence of this struct means that users of it are the only ones being able to access components in the world -/// struct OnlyComponentAccessWorld<'w>(InteriorMutableWorld<'w>); +/// struct OnlyComponentAccessWorld<'w>(UnsafeWorldCell<'w>); /// /// impl<'w> OnlyResourceAccessWorld<'w> { /// fn get_resource_mut(&mut self) -> Option> { -/// // SAFETY: resource access is allowed through this InteriorMutableWorld +/// // SAFETY: resource access is allowed through this UnsafeWorldCell /// unsafe { self.0.get_resource_mut::() } /// } /// } @@ -61,26 +61,26 @@ use std::any::TypeId; /// // ... /// // } /// -/// // the two interior mutable worlds borrow from the `&mut World`, so it cannot be accessed while they are live +/// // the two `UnsafeWorldCell`s borrow from the `&mut World`, so it cannot be accessed while they are live /// fn split_world_access(world: &mut World) -> (OnlyResourceAccessWorld<'_>, OnlyComponentAccessWorld<'_>) { -/// let interior_mutable_world = world.as_interior_mutable(); -/// let resource_access = OnlyResourceAccessWorld(interior_mutable_world); -/// let component_access = OnlyComponentAccessWorld(interior_mutable_world); +/// let unsafe_world_cell = world.as_unsafe_world_cell(); +/// let resource_access = OnlyResourceAccessWorld(unsafe_world_cell); +/// let component_access = OnlyComponentAccessWorld(unsafe_world_cell); /// (resource_access, component_access) /// } /// ``` #[derive(Copy, Clone)] -pub struct InteriorMutableWorld<'w>(&'w World); +pub struct UnsafeWorldCell<'w>(&'w World); -impl<'w> InteriorMutableWorld<'w> { +impl<'w> UnsafeWorldCell<'w> { pub(crate) fn new(world: &'w World) -> Self { - InteriorMutableWorld(world) + UnsafeWorldCell(world) } - /// Gets a reference to the [`&World`](crate::world::World) this [`InteriorMutableWorld`] belongs to. + /// Gets a reference to the [`&World`](crate::world::World) this [`UnsafeWorldCell`] belongs to. /// This can be used to call methods like [`World::contains_resource`] which aren't exposed and but don't perform any accesses. /// - /// **Note**: You *must not* hand out a `&World` reference to arbitrary safe code when the [`InteriorMutableWorld`] is currently + /// **Note**: You *must not* hand out a `&World` reference to arbitrary safe code when the [`UnsafeWorldCell`] is currently /// being used for mutable accesses. /// /// # Safety @@ -135,18 +135,18 @@ impl<'w> InteriorMutableWorld<'w> { self.0.increment_change_tick() } - /// Retrieves an [`InteriorMutableEntityRef`] that exposes read and write operations for the given `entity`. - /// Similar to the [`InteriorMutableWorld`], you are in charge of making sure that no aliasing rules are violated. - pub fn get_entity(self, entity: Entity) -> Option> { + /// Retrieves an [`UnsafeEntityRefCell`] that exposes read and write operations for the given `entity`. + /// Similar to the [`UnsafeWorldCell`], you are in charge of making sure that no aliasing rules are violated. + pub fn get_entity(self, entity: Entity) -> Option> { let location = self.0.entities.get(entity)?; - Some(InteriorMutableEntityRef::new(self, entity, location)) + Some(UnsafeEntityRefCell::new(self, entity, location)) } /// Gets a reference to the resource of the given type if it exists /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`InteriorMutableWorld`] has permission to access the resource + /// - the [`UnsafeWorldCell`] has permission to access the resource /// - no mutable reference to the resource exists at the same time #[inline] pub unsafe fn get_resource(self) -> Option<&'w R> { @@ -157,12 +157,12 @@ impl<'w> InteriorMutableWorld<'w> { /// The returned pointer must not be used to modify the resource, and must not be /// dereferenced after the borrow of the [`World`] ends. /// - /// **You should prefer to use the typed API [`InteriorMutableWorld::get_resource`] where possible and only + /// **You should prefer to use the typed API [`UnsafeWorldCell::get_resource`] where possible and only /// use this in cases where the actual types are not known at compile time.** /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`InteriorMutableWorld`] has permission to access the resource + /// - the [`UnsafeWorldCell`] has permission to access the resource /// - no mutable reference to the resource exists at the same time #[inline] pub unsafe fn get_resource_by_id(self, component_id: ComponentId) -> Option> { @@ -173,7 +173,7 @@ impl<'w> InteriorMutableWorld<'w> { /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`InteriorMutableWorld`] has permission to access the resource + /// - the [`UnsafeWorldCell`] has permission to access the resource /// - no mutable reference to the resource exists at the same time #[inline] pub unsafe fn get_non_send_resource(self) -> Option<&'w R> { @@ -184,7 +184,7 @@ impl<'w> InteriorMutableWorld<'w> { /// The returned pointer must not be used to modify the resource, and must not be /// dereferenced after the immutable borrow of the [`World`] ends. /// - /// **You should prefer to use the typed API [`InteriorMutableWorld::get_non_send_resource`] where possible and only + /// **You should prefer to use the typed API [`UnsafeWorldCell::get_non_send_resource`] where possible and only /// use this in cases where the actual types are not known at compile time.** /// /// # Panics @@ -192,7 +192,7 @@ impl<'w> InteriorMutableWorld<'w> { /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`InteriorMutableWorld`] has permission to access the resource + /// - the [`UnsafeWorldCell`] has permission to access the resource /// - no mutable reference to the resource exists at the same time #[inline] pub unsafe fn get_non_send_resource_by_id(self, component_id: ComponentId) -> Option> { @@ -207,7 +207,7 @@ impl<'w> InteriorMutableWorld<'w> { /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`InteriorMutableWorld`] has permission to access the resource mutably + /// - the [`UnsafeWorldCell`] has permission to access the resource mutably /// - no other references to the resource exist at the same time #[inline] pub unsafe fn get_resource_mut(self) -> Option> { @@ -221,14 +221,14 @@ impl<'w> InteriorMutableWorld<'w> { /// Gets a pointer to the resource with the id [`ComponentId`] if it exists. /// The returned pointer may be used to modify the resource, as long as the mutable borrow - /// of the [`InteriorMutableWorld`] is still valid. + /// of the [`UnsafeWorldCell`] is still valid. /// - /// **You should prefer to use the typed API [`InteriorMutableWorld::get_resource_mut`] where possible and only + /// **You should prefer to use the typed API [`UnsafeWorldCell::get_resource_mut`] where possible and only /// use this in cases where the actual types are not known at compile time.** /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`InteriorMutableWorld`] has permission to access the resource mutably + /// - the [`UnsafeWorldCell`] has permission to access the resource mutably /// - no other references to the resource exist at the same time #[inline] pub unsafe fn get_resource_mut_by_id( @@ -255,7 +255,7 @@ impl<'w> InteriorMutableWorld<'w> { /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`InteriorMutableWorld`] has permission to access the resource mutably + /// - the [`UnsafeWorldCell`] has permission to access the resource mutably /// - no other references to the resource exist at the same time #[inline] pub unsafe fn get_non_send_resource_mut(self) -> Option> { @@ -268,7 +268,7 @@ impl<'w> InteriorMutableWorld<'w> { /// The returned pointer may be used to modify the resource, as long as the mutable borrow /// of the [`World`] is still valid. /// - /// **You should prefer to use the typed API [`InteriorMutableWorld::get_non_send_resource_mut`] where possible and only + /// **You should prefer to use the typed API [`UnsafeWorldCell::get_non_send_resource_mut`] where possible and only /// use this in cases where the actual types are not known at compile time.** /// /// # Panics @@ -276,7 +276,7 @@ impl<'w> InteriorMutableWorld<'w> { /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`InteriorMutableWorld`] has permission to access the resource mutably + /// - the [`UnsafeWorldCell`] has permission to access the resource mutably /// - no other references to the resource exist at the same time #[inline] pub unsafe fn get_non_send_resource_mut_by_id( @@ -300,11 +300,11 @@ impl<'w> InteriorMutableWorld<'w> { } } -impl<'w> InteriorMutableWorld<'w> { +impl<'w> UnsafeWorldCell<'w> { /// # Safety /// It is the callers responsibility to ensure that /// - `component_id` must be assigned to a component of type `R` - /// - the [`InteriorMutableWorld`] has permission to access the resource + /// - the [`UnsafeWorldCell`] has permission to access the resource /// - no other mutable references to the resource exist at the same time #[inline] pub(crate) unsafe fn get_resource_mut_with_id( @@ -330,7 +330,7 @@ impl<'w> InteriorMutableWorld<'w> { /// # Safety /// It is the callers responsibility to ensure that /// - `component_id` must be assigned to a component of type `R`. - /// - the [`InteriorMutableWorld`] has permission to access the resource mutably + /// - the [`UnsafeWorldCell`] has permission to access the resource mutably /// - no other references to the resource exist at the same time #[inline] pub(crate) unsafe fn get_non_send_mut_with_id( @@ -356,19 +356,19 @@ impl<'w> InteriorMutableWorld<'w> { /// A interior-mutable reference to a particular [`Entity`] and all of its components #[derive(Copy, Clone)] -pub struct InteriorMutableEntityRef<'w> { - world: InteriorMutableWorld<'w>, +pub struct UnsafeEntityRefCell<'w> { + world: UnsafeWorldCell<'w>, entity: Entity, location: EntityLocation, } -impl<'w> InteriorMutableEntityRef<'w> { +impl<'w> UnsafeEntityRefCell<'w> { pub(crate) fn new( - world: InteriorMutableWorld<'w>, + world: UnsafeWorldCell<'w>, entity: Entity, location: EntityLocation, ) -> Self { - InteriorMutableEntityRef { + UnsafeEntityRefCell { world, entity, location, @@ -392,7 +392,7 @@ impl<'w> InteriorMutableEntityRef<'w> { } #[inline] - pub fn world(self) -> InteriorMutableWorld<'w> { + pub fn world(self) -> UnsafeWorldCell<'w> { self.world } @@ -413,7 +413,7 @@ impl<'w> InteriorMutableEntityRef<'w> { /// # Safety /// It is the callers responsibility to ensure that - /// - the [`InteriorMutableEntityRef`] has permission to access the component + /// - the [`UnsafeEntityRefCell`] has permission to access the component /// - no other mutable references to the component exist at the same time #[inline] pub unsafe fn get(self) -> Option<&'w T> { @@ -439,7 +439,7 @@ impl<'w> InteriorMutableEntityRef<'w> { /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`InteriorMutableEntityRef`] has permission to access the component + /// - the [`UnsafeEntityRefCell`] has permission to access the component /// - no other mutable references to the component exist at the same time #[inline] pub unsafe fn get_change_ticks(self) -> Option { @@ -459,13 +459,13 @@ impl<'w> InteriorMutableEntityRef<'w> { /// Retrieves the change ticks for the given [`ComponentId`]. This can be useful for implementing change /// detection in custom runtimes. /// - /// **You should prefer to use the typed API [`InteriorMutableEntityRef::get_change_ticks`] where possible and only + /// **You should prefer to use the typed API [`UnsafeEntityRefCell::get_change_ticks`] where possible and only /// use this in cases where the actual component types are not known at /// compile time.** /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`InteriorMutableEntityRef`] has permission to access the component + /// - the [`UnsafeEntityRefCell`] has permission to access the component /// - no other mutable references to the component exist at the same time #[inline] pub unsafe fn get_change_ticks_by_id( @@ -489,7 +489,7 @@ impl<'w> InteriorMutableEntityRef<'w> { /// # Safety /// It is the callers responsibility to ensure that - /// - the [`InteriorMutableEntityRef`] has permission to access the component mutably + /// - the [`UnsafeEntityRefCell`] has permission to access the component mutably /// - no other references to the component exist at the same time #[inline] pub unsafe fn get_mut(self) -> Option> { @@ -501,7 +501,7 @@ impl<'w> InteriorMutableEntityRef<'w> { /// # Safety /// It is the callers responsibility to ensure that - /// - the [`InteriorMutableEntityRef`] has permission to access the component mutably + /// - the [`UnsafeEntityRefCell`] has permission to access the component mutably /// - no other references to the component exist at the same time #[inline] pub(crate) unsafe fn get_mut_using_ticks( @@ -530,19 +530,19 @@ impl<'w> InteriorMutableEntityRef<'w> { } } -impl<'w> InteriorMutableEntityRef<'w> { +impl<'w> UnsafeEntityRefCell<'w> { /// Gets the component of the given [`ComponentId`] from the entity. /// /// **You should prefer to use the typed API where possible and only /// use this in cases where the actual component types are not known at /// compile time.** /// - /// Unlike [`InteriorMutableEntityRef::get`], this returns a raw pointer to the component, + /// Unlike [`UnsafeEntityRefCell::get`], this returns a raw pointer to the component, /// which is only valid while the `'w` borrow of the lifetime is active. /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`InteriorMutableEntityRef`] has permission to access the component + /// - the [`UnsafeEntityRefCell`] has permission to access the component /// - no other mutable references to the component exist at the same time #[inline] pub unsafe fn get_by_id(self, component_id: ComponentId) -> Option> { @@ -561,12 +561,12 @@ impl<'w> InteriorMutableEntityRef<'w> { /// Retrieves a mutable untyped reference to the given `entity`'s [Component] of the given [`ComponentId`]. /// Returns [None] if the `entity` does not have a [Component] of the given type. /// - /// **You should prefer to use the typed API [`InteriorMutableEntityRef::get_mut`] where possible and only + /// **You should prefer to use the typed API [`UnsafeEntityRefCell::get_mut`] where possible and only /// use this in cases where the actual types are not known at compile time.** /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`InteriorMutableEntityRef`] has permission to access the component mutably + /// - the [`UnsafeEntityRefCell`] has permission to access the component mutably /// - no other references to the component exist at the same time #[inline] pub unsafe fn get_mut_by_id(self, component_id: ComponentId) -> Option> { diff --git a/crates/bevy_ecs/src/world/world_cell.rs b/crates/bevy_ecs/src/world/world_cell.rs index dddf64c103aa5..9ea673ca02f34 100644 --- a/crates/bevy_ecs/src/world/world_cell.rs +++ b/crates/bevy_ecs/src/world/world_cell.rs @@ -14,12 +14,12 @@ use std::{ rc::Rc, }; -use super::interior_mutable_world::InteriorMutableWorld; +use super::unsafe_world_cell::UnsafeWorldCell; /// Exposes safe mutable access to multiple resources at a time in a World. Attempting to access /// World in a way that violates Rust's mutability rules will panic thanks to runtime checks. pub struct WorldCell<'w> { - pub(crate) world: InteriorMutableWorld<'w>, + pub(crate) world: UnsafeWorldCell<'w>, pub(crate) access: Rc>, } @@ -190,7 +190,7 @@ impl<'w> WorldCell<'w> { ); // world's ArchetypeComponentAccess is recycled to cut down on allocations Self { - world: world.as_interior_mutable(), + world: world.as_unsafe_world_cell(), access: Rc::new(RefCell::new(access)), } } From 118a822ca09cf8dbbe592f239b6e61d386af4b5b Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Tue, 24 Jan 2023 16:39:55 +0100 Subject: [PATCH 30/30] UnsafeEntityRefCell -> UnsafeWorldCellEntityRef --- crates/bevy_ecs/src/world/entity_ref.rs | 14 ++++---- .../bevy_ecs/src/world/unsafe_world_cell.rs | 34 +++++++++---------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index d57d3b52a34e4..7eff73711881a 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -13,7 +13,7 @@ use bevy_ptr::{OwningPtr, Ptr}; use bevy_utils::tracing::debug; use std::any::TypeId; -use super::unsafe_world_cell::UnsafeEntityRefCell; +use super::unsafe_world_cell::UnsafeWorldCellEntityRef; /// A read-only reference to a particular [`Entity`] and all of its components #[derive(Copy, Clone)] @@ -42,8 +42,8 @@ impl<'w> EntityRef<'w> { } } - fn as_unsafe_world_cell_readonly(&self) -> UnsafeEntityRefCell<'w> { - UnsafeEntityRefCell::new( + fn as_unsafe_world_cell_readonly(&self) -> UnsafeWorldCellEntityRef<'w> { + UnsafeWorldCellEntityRef::new( self.world.as_unsafe_world_cell_readonly(), self.entity, self.location, @@ -148,15 +148,15 @@ pub struct EntityMut<'w> { } impl<'w> EntityMut<'w> { - fn as_unsafe_world_cell_readonly(&self) -> UnsafeEntityRefCell<'_> { - UnsafeEntityRefCell::new( + fn as_unsafe_world_cell_readonly(&self) -> UnsafeWorldCellEntityRef<'_> { + UnsafeWorldCellEntityRef::new( self.world.as_unsafe_world_cell_readonly(), self.entity, self.location, ) } - fn as_unsafe_world_cell(&mut self) -> UnsafeEntityRefCell<'_> { - UnsafeEntityRefCell::new( + fn as_unsafe_world_cell(&mut self) -> UnsafeWorldCellEntityRef<'_> { + UnsafeWorldCellEntityRef::new( self.world.as_unsafe_world_cell(), self.entity, self.location, diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index c90385526c365..4f3539e75db39 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -135,11 +135,11 @@ impl<'w> UnsafeWorldCell<'w> { self.0.increment_change_tick() } - /// Retrieves an [`UnsafeEntityRefCell`] that exposes read and write operations for the given `entity`. + /// Retrieves an [`UnsafeWorldCellEntityRef`] that exposes read and write operations for the given `entity`. /// Similar to the [`UnsafeWorldCell`], you are in charge of making sure that no aliasing rules are violated. - pub fn get_entity(self, entity: Entity) -> Option> { + pub fn get_entity(self, entity: Entity) -> Option> { let location = self.0.entities.get(entity)?; - Some(UnsafeEntityRefCell::new(self, entity, location)) + Some(UnsafeWorldCellEntityRef::new(self, entity, location)) } /// Gets a reference to the resource of the given type if it exists @@ -356,19 +356,19 @@ impl<'w> UnsafeWorldCell<'w> { /// A interior-mutable reference to a particular [`Entity`] and all of its components #[derive(Copy, Clone)] -pub struct UnsafeEntityRefCell<'w> { +pub struct UnsafeWorldCellEntityRef<'w> { world: UnsafeWorldCell<'w>, entity: Entity, location: EntityLocation, } -impl<'w> UnsafeEntityRefCell<'w> { +impl<'w> UnsafeWorldCellEntityRef<'w> { pub(crate) fn new( world: UnsafeWorldCell<'w>, entity: Entity, location: EntityLocation, ) -> Self { - UnsafeEntityRefCell { + UnsafeWorldCellEntityRef { world, entity, location, @@ -413,7 +413,7 @@ impl<'w> UnsafeEntityRefCell<'w> { /// # Safety /// It is the callers responsibility to ensure that - /// - the [`UnsafeEntityRefCell`] has permission to access the component + /// - the [`UnsafeWorldCellEntityRef`] has permission to access the component /// - no other mutable references to the component exist at the same time #[inline] pub unsafe fn get(self) -> Option<&'w T> { @@ -439,7 +439,7 @@ impl<'w> UnsafeEntityRefCell<'w> { /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`UnsafeEntityRefCell`] has permission to access the component + /// - the [`UnsafeWorldCellEntityRef`] has permission to access the component /// - no other mutable references to the component exist at the same time #[inline] pub unsafe fn get_change_ticks(self) -> Option { @@ -459,13 +459,13 @@ impl<'w> UnsafeEntityRefCell<'w> { /// Retrieves the change ticks for the given [`ComponentId`]. This can be useful for implementing change /// detection in custom runtimes. /// - /// **You should prefer to use the typed API [`UnsafeEntityRefCell::get_change_ticks`] where possible and only + /// **You should prefer to use the typed API [`UnsafeWorldCellEntityRef::get_change_ticks`] where possible and only /// use this in cases where the actual component types are not known at /// compile time.** /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`UnsafeEntityRefCell`] has permission to access the component + /// - the [`UnsafeWorldCellEntityRef`] has permission to access the component /// - no other mutable references to the component exist at the same time #[inline] pub unsafe fn get_change_ticks_by_id( @@ -489,7 +489,7 @@ impl<'w> UnsafeEntityRefCell<'w> { /// # Safety /// It is the callers responsibility to ensure that - /// - the [`UnsafeEntityRefCell`] has permission to access the component mutably + /// - the [`UnsafeWorldCellEntityRef`] has permission to access the component mutably /// - no other references to the component exist at the same time #[inline] pub unsafe fn get_mut(self) -> Option> { @@ -501,7 +501,7 @@ impl<'w> UnsafeEntityRefCell<'w> { /// # Safety /// It is the callers responsibility to ensure that - /// - the [`UnsafeEntityRefCell`] has permission to access the component mutably + /// - the [`UnsafeWorldCellEntityRef`] has permission to access the component mutably /// - no other references to the component exist at the same time #[inline] pub(crate) unsafe fn get_mut_using_ticks( @@ -530,19 +530,19 @@ impl<'w> UnsafeEntityRefCell<'w> { } } -impl<'w> UnsafeEntityRefCell<'w> { +impl<'w> UnsafeWorldCellEntityRef<'w> { /// Gets the component of the given [`ComponentId`] from the entity. /// /// **You should prefer to use the typed API where possible and only /// use this in cases where the actual component types are not known at /// compile time.** /// - /// Unlike [`UnsafeEntityRefCell::get`], this returns a raw pointer to the component, + /// Unlike [`UnsafeWorldCellEntityRef::get`], this returns a raw pointer to the component, /// which is only valid while the `'w` borrow of the lifetime is active. /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`UnsafeEntityRefCell`] has permission to access the component + /// - the [`UnsafeWorldCellEntityRef`] has permission to access the component /// - no other mutable references to the component exist at the same time #[inline] pub unsafe fn get_by_id(self, component_id: ComponentId) -> Option> { @@ -561,12 +561,12 @@ impl<'w> UnsafeEntityRefCell<'w> { /// Retrieves a mutable untyped reference to the given `entity`'s [Component] of the given [`ComponentId`]. /// Returns [None] if the `entity` does not have a [Component] of the given type. /// - /// **You should prefer to use the typed API [`UnsafeEntityRefCell::get_mut`] where possible and only + /// **You should prefer to use the typed API [`UnsafeWorldCellEntityRef::get_mut`] where possible and only /// use this in cases where the actual types are not known at compile time.** /// /// # Safety /// It is the callers responsibility to ensure that - /// - the [`UnsafeEntityRefCell`] has permission to access the component mutably + /// - the [`UnsafeWorldCellEntityRef`] has permission to access the component mutably /// - no other references to the component exist at the same time #[inline] pub unsafe fn get_mut_by_id(self, component_id: ComponentId) -> Option> {