Hide UnsafeWorldCell::unsafe_world (#9741)

# Objective

We've done a lot of work to remove the pattern of a `&World` with
interior mutability (#6404, #8833). However, this pattern still persists
within `bevy_ecs` via the `unsafe_world` method.

## Solution

* Make `unsafe_world` private. Adjust any callsites to use
`UnsafeWorldCell` for interior mutability.
* Add `UnsafeWorldCell::removed_components`, since it is always safe to
access the removed components collection through `UnsafeWorldCell`.

## Future Work

Remove/hide `UnsafeWorldCell::world_metadata`, once we have provided
safe ways of accessing all world metadata.

---

## Changelog

+ Added `UnsafeWorldCell::removed_components`, which provides read-only
access to a world's collection of removed components.
This commit is contained in:
Joseph 2023-10-02 05:46:43 -07:00 committed by GitHub
parent 450328d15e
commit 8cc255c2f0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 29 additions and 24 deletions

View File

@ -427,8 +427,7 @@ macro_rules! impl_tick_filter {
table_ticks: None, table_ticks: None,
sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet) sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet)
.then(|| { .then(|| {
world.unsafe_world() world.storages()
.storages()
.sparse_sets .sparse_sets
.get(id) .get(id)
.debug_checked_unwrap() .debug_checked_unwrap()

View File

@ -338,10 +338,12 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
) -> Result<[ROQueryItem<'w, Q>; N], QueryEntityError> { ) -> Result<[ROQueryItem<'w, Q>; N], QueryEntityError> {
self.update_archetypes(world); self.update_archetypes(world);
// SAFETY: update_archetypes validates the `World` matches // SAFETY:
// - We have read-only access to the entire world.
// - `update_archetypes` validates that the `World` matches.
unsafe { unsafe {
self.get_many_read_only_manual( self.get_many_read_only_manual(
world, world.as_unsafe_world_cell_readonly(),
entities, entities,
world.last_change_tick(), world.last_change_tick(),
world.read_change_tick(), world.read_change_tick(),
@ -633,11 +635,13 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
/// ///
/// # Safety /// # Safety
/// ///
/// This must be called on the same `World` that the `Query` was generated from: /// * `world` must have permission to read all of the components returned from this call.
/// No mutable references may coexist with any of the returned references.
/// * This must be called on the same `World` that the `Query` was generated from:
/// use `QueryState::validate_world` to verify this. /// use `QueryState::validate_world` to verify this.
pub(crate) unsafe fn get_many_read_only_manual<'w, const N: usize>( pub(crate) unsafe fn get_many_read_only_manual<'w, const N: usize>(
&self, &self,
world: &'w World, world: UnsafeWorldCell<'w>,
entities: [Entity; N], entities: [Entity; N],
last_run: Tick, last_run: Tick,
this_run: Tick, this_run: Tick,
@ -647,12 +651,9 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
for (value, entity) in std::iter::zip(&mut values, entities) { for (value, entity) in std::iter::zip(&mut values, entities) {
// SAFETY: fetch is read-only // SAFETY: fetch is read-only
// and world must be validated // and world must be validated
let item = self.as_readonly().get_unchecked_manual( let item = self
world.as_unsafe_world_cell_readonly(), .as_readonly()
entity, .get_unchecked_manual(world, entity, last_run, this_run)?;
last_run,
this_run,
)?;
*value = MaybeUninit::new(item); *value = MaybeUninit::new(item);
} }

View File

@ -259,8 +259,7 @@ impl<'w, 's, T: Component> RemovedComponents<'w, 's, T> {
// SAFETY: Only reads World removed component events // SAFETY: Only reads World removed component events
unsafe impl<'a> ReadOnlySystemParam for &'a RemovedComponentEvents {} unsafe impl<'a> ReadOnlySystemParam for &'a RemovedComponentEvents {}
// SAFETY: no component value access, removed component events can be read in parallel and are // SAFETY: no component value access.
// never mutably borrowed during system execution
unsafe impl<'a> SystemParam for &'a RemovedComponentEvents { unsafe impl<'a> SystemParam for &'a RemovedComponentEvents {
type State = (); type State = ();
type Item<'w, 's> = &'w RemovedComponentEvents; type Item<'w, 's> = &'w RemovedComponentEvents;
@ -274,6 +273,6 @@ unsafe impl<'a> SystemParam for &'a RemovedComponentEvents {
world: UnsafeWorldCell<'w>, world: UnsafeWorldCell<'w>,
_change_tick: Tick, _change_tick: Tick,
) -> Self::Item<'w, 's> { ) -> Self::Item<'w, 's> {
world.world_metadata().removed_components() world.removed_components()
} }
} }

View File

@ -874,14 +874,12 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> {
&self, &self,
entities: [Entity; N], entities: [Entity; N],
) -> Result<[ROQueryItem<'_, Q>; N], QueryEntityError> { ) -> Result<[ROQueryItem<'_, Q>; N], QueryEntityError> {
// SAFETY: it is the scheduler's responsibility to ensure that `Query` is never handed out on the wrong `World`. // SAFETY:
// - `&self` ensures there is no mutable access to any components accessible to this query.
// - `self.world` matches `self.state`.
unsafe { unsafe {
self.state.get_many_read_only_manual( self.state
self.world.unsafe_world(), .get_many_read_only_manual(self.world, entities, self.last_run, self.this_run)
entities,
self.last_run,
self.this_run,
)
} }
} }

View File

@ -12,6 +12,7 @@ use crate::{
}, },
entity::{Entities, Entity, EntityLocation}, entity::{Entities, Entity, EntityLocation},
prelude::Component, prelude::Component,
removal_detection::RemovedComponentEvents,
storage::{Column, ComponentSparseSet, Storages}, storage::{Column, ComponentSparseSet, Storages},
system::Resource, system::Resource,
}; };
@ -156,7 +157,7 @@ impl<'w> UnsafeWorldCell<'w> {
// - caller ensures there is no `&mut World` this makes it okay to make a `&World` // - caller ensures there is no `&mut World` this makes it okay to make a `&World`
// - caller ensures there is no mutable borrows of world data, this means the caller cannot // - caller ensures there is no mutable borrows of world data, this means the caller cannot
// misuse the returned `&World` // misuse the returned `&World`
unsafe { &*self.0 } unsafe { self.unsafe_world() }
} }
/// Gets a reference to the [`World`] this [`UnsafeWorldCell`] belong to. /// Gets a reference to the [`World`] this [`UnsafeWorldCell`] belong to.
@ -185,7 +186,7 @@ impl<'w> UnsafeWorldCell<'w> {
/// - must not be used in a way that would conflict with any /// - must not be used in a way that would conflict with any
/// live exclusive borrows on world data /// live exclusive borrows on world data
#[inline] #[inline]
pub(crate) unsafe fn unsafe_world(self) -> &'w World { unsafe fn unsafe_world(self) -> &'w World {
// SAFETY: // SAFETY:
// - caller ensures that the returned `&World` is not used in a way that would conflict // - caller ensures that the returned `&World` is not used in a way that would conflict
// with any existing mutable borrows of world data // with any existing mutable borrows of world data
@ -224,6 +225,13 @@ impl<'w> UnsafeWorldCell<'w> {
&unsafe { self.world_metadata() }.components &unsafe { self.world_metadata() }.components
} }
/// Retrieves this world's collection of [removed components](RemovedComponentEvents).
pub fn removed_components(self) -> &'w RemovedComponentEvents {
// SAFETY:
// - we only access world metadata
&unsafe { self.world_metadata() }.removed_components
}
/// Retrieves this world's [`Bundles`] collection. /// Retrieves this world's [`Bundles`] collection.
#[inline] #[inline]
pub fn bundles(self) -> &'w Bundles { pub fn bundles(self) -> &'w Bundles {