From 66d222dfbf26629af3ac845a1891ec211e9761ef Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Fri, 27 Jan 2023 00:12:13 +0000 Subject: [PATCH] add `UnsafeWorldCell` abstraction (#6404) alternative to #5922, implements #5956 builds on top of https://github.com/bevyengine/bevy/pull/6402 # Objective https://github.com/bevyengine/bevy/issues/5956 goes into more detail, but the TLDR is: - bevy systems ensure disjoint accesses to resources and components, and for that to work there are methods `World::get_resource_unchecked_mut(&self)`, ..., `EntityRef::get_mut_unchecked(&self)` etc. - we don't have these unchecked methods for `by_id` variants, so third-party crate authors cannot build their own safe disjoint-access abstractions with these - having `_unchecked_mut` methods is not great, because in their presence safe code can accidentally violate subtle invariants. Having to go through `world.as_unsafe_world_cell().unsafe_method()` forces you to stop and think about what you want to write in your `// SAFETY` comment. The alternative is to keep exposing `_unchecked_mut` variants for every operation that we want third-party crates to build upon, but we'd prefer to avoid using these methods alltogether: https://github.com/bevyengine/bevy/pull/5922#issuecomment-1241954543 Also, this is something that **cannot be implemented outside of bevy**, so having either this PR or #5922 as an escape hatch with lots of discouraging comments would be great. ## Solution - add `UnsafeWorldCell` with `unsafe fn get_resource(&self)`, `unsafe fn get_resource_mut(&self)` - add `fn World::as_unsafe_world_cell(&mut self) -> UnsafeWorldCell<'_>` (and `as_unsafe_world_cell_readonly(&self)`) - add `UnsafeWorldCellEntityRef` with `unsafe fn get`, `unsafe fn get_mut` and the other utilities on `EntityRef` (no methods for spawning, despawning, insertion) - use the `UnsafeWorldCell` abstraction in `ReflectComponent`, `ReflectResource` and `ReflectAsset`, so these APIs are easier to reason about - remove `World::get_resource_mut_unchecked`, `EntityRef::get_mut_unchecked` and use `unsafe { world.as_unsafe_world_cell().get_mut() }` and `unsafe { world.as_unsafe_world_cell().get_entity(entity)?.get_mut() }` instead This PR does **not** make use of `UnsafeWorldCell` for anywhere else in `bevy_ecs` such as `SystemParam` or `Query`. That is a much larger change, and I am convinced that having `UnsafeWorldCell` is already useful for third-party crates. Implemented API: ```rust struct World { .. } impl World { fn as_unsafe_world_cell(&self) -> UnsafeWorldCell<'_>; } struct UnsafeWorldCell<'w>(&'w World); impl<'w> UnsafeWorldCell { unsafe fn world(&self) -> &World; fn get_entity(&self) -> UnsafeWorldCellEntityRef<'w>; // returns 'w which is `'self` of the `World::as_unsafe_world_cell(&'w self)` unsafe fn get_resource(&self) -> Option<&'w T>; unsafe fn get_resource_by_id(&self, ComponentId) -> Option<&'w T>; unsafe fn get_resource_mut(&self) -> Option>; unsafe fn get_resource_mut_by_id(&self) -> Option>; unsafe fn get_non_send_resource(&self) -> Option<&'w T>; unsafe fn get_non_send_resource_mut(&self) -> Option>>; // not included: remove, remove_resource, despawn, anything that might change archetypes } struct UnsafeWorldCellEntityRef<'w> { .. } impl UnsafeWorldCellEntityRef<'w> { unsafe fn get(&self, Entity) -> Option<&'w T>; unsafe fn get_by_id(&self, Entity, ComponentId) -> Option>; unsafe fn get_mut(&self, Entity) -> Option>; unsafe fn get_mut_by_id(&self, Entity, ComponentId) -> Option>; unsafe fn get_change_ticks(&self, Entity) -> Option>; // fn id, archetype, contains, contains_id, containts_type_id } ```
UnsafeWorldCell docs 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 [`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. ```rust use bevy_ecs::world::World; use bevy_ecs::change_detection::Mut; use bevy_ecs::system::Resource; use bevy_ecs::world::unsafe_world_cell_world::UnsafeWorldCell; // 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>(UnsafeWorldCell<'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>(UnsafeWorldCell<'w>); impl<'w> OnlyResourceAccessWorld<'w> { fn get_resource_mut(&mut self) -> Option> { // SAFETY: resource access is allowed through this UnsafeWorldCell 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_unsafe_world_cell() }); let component_access = OnlyComponentAccessWorld(unsafe { world.as_unsafe_world_cell() }); (resource_access, component_access) } ```
--- crates/bevy_asset/src/reflect.rs | 39 +- crates/bevy_ecs/src/reflect.rs | 49 +- crates/bevy_ecs/src/system/query.rs | 3 +- crates/bevy_ecs/src/system/system_param.rs | 6 +- crates/bevy_ecs/src/world/entity_ref.rs | 215 ++----- crates/bevy_ecs/src/world/mod.rs | 135 ++-- .../bevy_ecs/src/world/unsafe_world_cell.rs | 595 ++++++++++++++++++ crates/bevy_ecs/src/world/world_cell.rs | 67 +- 8 files changed, 774 insertions(+), 335 deletions(-) create mode 100644 crates/bevy_ecs/src/world/unsafe_world_cell.rs diff --git a/crates/bevy_asset/src/reflect.rs b/crates/bevy_asset/src/reflect.rs index 3d01ac148d635a..27a0d8ce24cea2 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::{unsafe_world_cell::UnsafeWorldCell, World}; use bevy_reflect::{FromReflect, FromType, Reflect, Uuid}; use crate::{Asset, Assets, Handle, HandleId, HandleUntyped}; @@ -18,8 +18,10 @@ 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(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, @@ -54,10 +56,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_unsafe_world_cell(), handle) } } - /// Equivalent of [`Assets::get_mut`], but does not require a mutable reference to the world. + /// 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, @@ -67,11 +70,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 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 /// @@ -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 [`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: &'w World, + world: UnsafeWorldCell<'w>, handle: HandleUntyped, ) -> Option<&'w mut dyn Reflect> { // SAFETY: requirements are deferred to the caller @@ -140,18 +143,10 @@ 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 - .get_resource_unchecked_mut::>() - .unwrap() - .into_inner() - }; + // 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()); 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 88e25dfaf86b58..b865844731b66b 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::{unsafe_world_cell::UnsafeWorldCell, FromWorld, World}, }; use bevy_reflect::{ impl_from_reflect_value, impl_reflect_value, FromType, Reflect, ReflectDeserialize, @@ -52,7 +52,10 @@ 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>, + /// + /// # Safety + /// 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), } @@ -117,20 +120,20 @@ impl ReflectComponent { entity: Entity, ) -> Option> { // SAFETY: unique world access - unsafe { (self.0.reflect_mut)(world, 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 in an exclusive system to avoid sharing across threads (or use a - /// scheduler that enforces safe memory access). + /// * 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: &'a World, + world: UnsafeWorldCell<'a>, entity: Entity, ) -> Option> { + // SAFETY: safety requirements deferred to caller (self.0.reflect_mut)(world, entity) } @@ -209,16 +212,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 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_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, + }) } }, }) @@ -265,7 +266,10 @@ 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>, + /// + /// # Safety + /// 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), } @@ -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_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 in an exclusive system to avoid sharing across threads (or use a - /// scheduler that enforces safe memory access). + /// * 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<'a>( + pub unsafe fn reflect_unchecked_mut<'w>( &self, - world: &'a World, - ) -> Option> { + world: UnsafeWorldCell<'w>, + ) -> Option> { // SAFETY: caller promises to uphold uniqueness guarantees (self.0.reflect_unchecked_mut)(world) } @@ -385,7 +388,7 @@ 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 { + world.get_resource_mut::().map(|res| Mut { value: res.value as &mut dyn Reflect, ticks: res.ticks, }) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index a86d40e62b038b..4d3a12ed49bc7c 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_unsafe_world_cell_migration_internal() .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/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 48df3c9f4032bc..4dc3a85bdbf1b4 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_unsafe_world_cell_migration_internal() + .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_unsafe_world_cell_migration_internal() + .get_resource_mut_with_id(component_id) .map(|value| ResMut { value: value.value, ticks: TicksMut { diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index a1656785169876..7eff73711881a9 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::unsafe_world_cell::UnsafeWorldCellEntityRef; + /// A read-only reference to a particular [`Entity`] and all of its components #[derive(Copy, Clone)] pub struct EntityRef<'w> { @@ -40,6 +42,14 @@ impl<'w> EntityRef<'w> { } } + fn as_unsafe_world_cell_readonly(&self) -> UnsafeWorldCellEntityRef<'w> { + UnsafeWorldCellEntityRef::new( + self.world.as_unsafe_world_cell_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 { @@ -78,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_unsafe_world_cell_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_unsafe_world_cell_readonly().get_change_ticks::() } } /// Retrieves the change ticks for the given [`ComponentId`]. This can be useful for implementing change @@ -121,56 +108,12 @@ 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_unsafe_world_cell_readonly() + .get_change_ticks_by_id(component_id) } } - - /// 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> { @@ -184,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_unsafe_world_cell_readonly().get_by_id(component_id) } } } @@ -216,6 +148,21 @@ pub struct EntityMut<'w> { } impl<'w> EntityMut<'w> { + 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) -> UnsafeWorldCellEntityRef<'_> { + UnsafeWorldCellEntityRef::new( + self.world.as_unsafe_world_cell(), + self.entity, + self.location, + ) + } + /// # Safety /// /// - `entity` must be valid for `world`: the generation should match that of the entity at the same index. @@ -271,45 +218,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_unsafe_world_cell_readonly().get::() } } #[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::() } + // SAFETY: &mut self implies exclusive access for duration of returned value + unsafe { self.as_unsafe_world_cell().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_unsafe_world_cell_readonly().get_change_ticks::() } } /// Retrieves the change ticks for the given [`ComponentId`]. This can be useful for implementing change @@ -320,54 +244,13 @@ 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_unsafe_world_cell_readonly() + .get_change_ticks_by_id(component_id) } } - /// 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 +595,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 +607,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, @@ -857,7 +744,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::(), @@ -888,7 +775,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 { diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index dd09a5341aaa4e..87913d2b59ddbb 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1,5 +1,6 @@ mod entity_ref; mod spawn_batch; +pub mod unsafe_world_cell; mod world_cell; pub use crate::change_detection::{Mut, Ref, CHECK_TICK_THRESHOLD}; @@ -17,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, @@ -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}, }; @@ -33,6 +34,8 @@ mod identifier; pub use identifier::WorldId; +use self::unsafe_world_cell::UnsafeWorldCell; + /// Stores and exposes operations on [entities](Entity), [components](Component), resources, /// and their associated metadata. /// @@ -60,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, @@ -105,6 +108,23 @@ impl World { self.id } + /// 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 [`UnsafeWorldCell`] view with only read access to everything. + pub fn as_unsafe_world_cell_readonly(&self) -> UnsafeWorldCell<'_> { + UnsafeWorldCell::new(self) + } + + /// 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_unsafe_world_cell_migration_internal(&self) -> UnsafeWorldCell<'_> { + UnsafeWorldCell::new(self) + } + /// Retrieves this world's [Entities] collection #[inline] pub fn entities(&self) -> &Entities { @@ -991,7 +1011,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_unsafe_world_cell().get_resource_mut() } } /// Gets a mutable reference to the resource of type `T` if it exists, @@ -1024,18 +1044,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 @@ -1100,22 +1108,7 @@ 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_unsafe_world_cell().get_non_send_resource_mut() } } // Shorthand helper function for getting the data and change ticks for a resource. @@ -1398,25 +1391,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] @@ -1433,30 +1407,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 @@ -1655,7 +1605,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 +1616,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. /// @@ -1674,20 +1624,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_unsafe_world_cell() + .get_resource_mut_by_id(component_id) + } } /// Gets a `!Send` resource to the resource with the id [`ComponentId`] if it exists. @@ -1811,7 +1752,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. @@ -1830,7 +1771,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` @@ -1862,7 +1803,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` @@ -1882,7 +1823,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` @@ -1909,7 +1850,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` @@ -1929,7 +1870,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` diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs new file mode 100644 index 00000000000000..4f3539e75db390 --- /dev/null +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -0,0 +1,595 @@ +#![warn(unsafe_op_in_unsafe_fn)] + +use super::{entity_ref, Mut, World}; +use crate::{ + archetype::{Archetype, Archetypes}, + bundle::Bundles, + change_detection::{MutUntyped, TicksMut}, + component::{ComponentId, ComponentStorage, ComponentTicks, Components}, + entity::{Entities, Entity, EntityLocation}, + prelude::Component, + storage::Storages, + system::Resource, +}; +use bevy_ptr::Ptr; +use std::any::TypeId; + +/// 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 +/// 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 +/// +/// [`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. +/// +/// ``` +/// use bevy_ecs::world::World; +/// use bevy_ecs::change_detection::Mut; +/// use bevy_ecs::system::Resource; +/// 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>(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>(UnsafeWorldCell<'w>); +/// +/// impl<'w> OnlyResourceAccessWorld<'w> { +/// fn get_resource_mut(&mut self) -> Option> { +/// // SAFETY: resource access is allowed through this UnsafeWorldCell +/// unsafe { self.0.get_resource_mut::() } +/// } +/// } +/// // impl<'w> OnlyComponentAccessWorld<'w> { +/// // ... +/// // } +/// +/// // 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 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 UnsafeWorldCell<'w>(&'w World); + +impl<'w> UnsafeWorldCell<'w> { + pub(crate) fn new(world: &'w World) -> Self { + UnsafeWorldCell(world) + } + + /// 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 [`UnsafeWorldCell`] 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 + } + + /// 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 + } + + /// 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 [`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> { + let location = self.0.entities.get(entity)?; + Some(UnsafeWorldCellEntityRef::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 [`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> { + 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 [`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 [`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> { + self.0.get_resource_by_id(component_id) + } + + /// Gets a reference to the non-send resource of the given type if it exists + /// + /// # Safety + /// It is the callers responsibility to ensure that + /// - 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> { + 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 [`UnsafeWorldCell::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 [`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> { + 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 + /// It is the callers responsibility to ensure that + /// - 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> { + 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.get_resource_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 [`UnsafeWorldCell`] is still valid. + /// + /// **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 [`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( + &self, + component_id: ComponentId, + ) -> Option> { + 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 { + TicksMut::from_tick_cells(ticks, self.last_change_tick(), self.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 mutable reference to the non-send resource of the given type if it exists + /// + /// # Safety + /// It is the callers responsibility to ensure that + /// - 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> { + 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) } + } + + /// 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 [`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 + /// 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 [`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( + &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> UnsafeWorldCell<'w> { + /// # Safety + /// It is the callers responsibility to ensure that + /// - `component_id` must be assigned to a component of type `R` + /// - 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( + &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.last_change_tick(), self.read_change_tick()) + }; + + Some(Mut { + // SAFETY: caller ensures aliasing rules, ptr is of type `R` + value: unsafe { ptr.assert_unique().deref_mut() }, + ticks, + }) + } + + /// # Safety + /// It is the callers responsibility to ensure that + /// - `component_id` must be assigned to a component of type `R`. + /// - 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( + &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.last_change_tick(), self.read_change_tick()) + }, + }) + } +} + +/// A interior-mutable reference to a particular [`Entity`] and all of its components +#[derive(Copy, Clone)] +pub struct UnsafeWorldCellEntityRef<'w> { + world: UnsafeWorldCell<'w>, + entity: Entity, + location: EntityLocation, +} + +impl<'w> UnsafeWorldCellEntityRef<'w> { + pub(crate) fn new( + world: UnsafeWorldCell<'w>, + entity: Entity, + location: EntityLocation, + ) -> Self { + UnsafeWorldCellEntityRef { + 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 { + self.entity + } + + #[inline] + pub fn location(self) -> EntityLocation { + self.location + } + + #[inline] + pub fn archetype(self) -> &'w Archetype { + &self.world.0.archetypes[self.location.archetype_id] + } + + #[inline] + pub fn world(self) -> UnsafeWorldCell<'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 + /// It is the callers responsibility to ensure that + /// - 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> { + // SAFETY: + // - entity location is valid + // - proper world access is promised by caller + 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 + /// It is the callers responsibility to ensure that + /// - 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 { + // SAFETY: + // - entity location is valid + // - proper world acess is promised by caller + unsafe { + self.world.0.get_ticks_with_type( + TypeId::of::(), + T::Storage::STORAGE_TYPE, + self.entity, + self.location, + ) + } + } + + /// 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 [`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 [`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( + &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 [`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> { + // SAFETY: same safety requirements + unsafe { + self.get_mut_using_ticks(self.world.last_change_tick(), self.world.read_change_tick()) + } + } + + /// # Safety + /// It is the callers responsibility to ensure that + /// - 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( + &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> 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 [`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 [`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> { + 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 [`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 [`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> { + 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.last_change_tick(), + self.world.read_change_tick(), + ), + }) + } + } +} diff --git a/crates/bevy_ecs/src/world/world_cell.rs b/crates/bevy_ecs/src/world/world_cell.rs index 8e6699e0766946..9ea673ca02f34f 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::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: &'w mut World, + pub(crate) world: UnsafeWorldCell<'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_unsafe_world_cell(), 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,13 +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.get_resource_unchecked_mut_with_id(component_id) }, + // SAFETY: access is checked by WorldBorrowMut + || unsafe { self.world.get_resource_mut::() }, archetype_component_id, self.access.clone(), ) @@ -253,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(), ) @@ -286,13 +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: ComponentId matches TypeId and access is checked by WorldBorrowMut - || unsafe { self.world.get_non_send_unchecked_mut_with_id(component_id) }, + // SAFETY: access is checked by WorldBorrowMut + || unsafe { self.world.get_non_send_resource_mut::() }, archetype_component_id, self.access.clone(), ) @@ -408,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),