From 0dcc09173db22974dc11841429a83851d95be769 Mon Sep 17 00:00:00 2001 From: Joseph <21144246+JoJoJet@users.noreply.github.com> Date: Sat, 9 Sep 2023 18:15:09 -0700 Subject: [PATCH 1/3] make unsafe_world private --- crates/bevy_ecs/src/query/filter.rs | 3 +-- crates/bevy_ecs/src/query/state.rs | 21 ++++++++++--------- crates/bevy_ecs/src/system/query.rs | 12 +++++------ .../bevy_ecs/src/world/unsafe_world_cell.rs | 2 +- 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 021b010021d70..eef17c314bfac 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -427,8 +427,7 @@ macro_rules! impl_tick_filter { table_ticks: None, sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet) .then(|| { - world.unsafe_world() - .storages() + world.storages() .sparse_sets .get(id) .debug_checked_unwrap() diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index c1214c55585db..3675c7449a288 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -320,10 +320,12 @@ impl QueryState { ) -> Result<[ROQueryItem<'w, Q>; N], QueryEntityError> { 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 { self.get_many_read_only_manual( - world, + world.as_unsafe_world_cell_readonly(), entities, world.last_change_tick(), world.read_change_tick(), @@ -511,11 +513,13 @@ impl QueryState { /// /// # 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. pub(crate) unsafe fn get_many_read_only_manual<'w, const N: usize>( &self, - world: &'w World, + world: UnsafeWorldCell<'w>, entities: [Entity; N], last_run: Tick, this_run: Tick, @@ -525,12 +529,9 @@ impl QueryState { for (value, entity) in std::iter::zip(&mut values, entities) { // SAFETY: fetch is read-only // and world must be validated - let item = self.as_readonly().get_unchecked_manual( - world.as_unsafe_world_cell_readonly(), - entity, - last_run, - this_run, - )?; + let item = self + .as_readonly() + .get_unchecked_manual(world, entity, last_run, this_run)?; *value = MaybeUninit::new(item); } diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index a96bdb3531e79..503cddbb44167 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -873,14 +873,12 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { &self, entities: [Entity; N], ) -> 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 { - self.state.get_many_read_only_manual( - self.world.unsafe_world(), - entities, - self.last_run, - self.this_run, - ) + self.state + .get_many_read_only_manual(self.world, entities, self.last_run, self.this_run) } } diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index a8f777a26a022..71dbe1dd3b24b 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -185,7 +185,7 @@ impl<'w> UnsafeWorldCell<'w> { /// - must not be used in a way that would conflict with any /// live exclusive borrows on world data #[inline] - pub(crate) unsafe fn unsafe_world(self) -> &'w World { + unsafe fn unsafe_world(self) -> &'w World { // SAFETY: // - caller ensures that the returned `&World` is not used in a way that would conflict // with any existing mutable borrows of world data From 623941307d2a224d296e2fbed3400875aa6f3dce Mon Sep 17 00:00:00 2001 From: Joseph <21144246+JoJoJet@users.noreply.github.com> Date: Sat, 9 Sep 2023 18:28:24 -0700 Subject: [PATCH 2/3] provide access to removed components --- crates/bevy_ecs/src/removal_detection.rs | 5 ++--- crates/bevy_ecs/src/world/unsafe_world_cell.rs | 8 ++++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/removal_detection.rs b/crates/bevy_ecs/src/removal_detection.rs index 0118ab8dedc55..4b1487f11d020 100644 --- a/crates/bevy_ecs/src/removal_detection.rs +++ b/crates/bevy_ecs/src/removal_detection.rs @@ -256,8 +256,7 @@ where // SAFETY: Only reads World removed component events unsafe impl<'a> ReadOnlySystemParam for &'a RemovedComponentEvents {} -// SAFETY: no component value access, removed component events can be read in parallel and are -// never mutably borrowed during system execution +// SAFETY: no component value access. unsafe impl<'a> SystemParam for &'a RemovedComponentEvents { type State = (); type Item<'w, 's> = &'w RemovedComponentEvents; @@ -271,6 +270,6 @@ unsafe impl<'a> SystemParam for &'a RemovedComponentEvents { world: UnsafeWorldCell<'w>, _change_tick: Tick, ) -> Self::Item<'w, 's> { - world.world_metadata().removed_components() + world.removed_components() } } diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index 71dbe1dd3b24b..093fdcc95a072 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -12,6 +12,7 @@ use crate::{ }, entity::{Entities, Entity, EntityLocation}, prelude::Component, + removal_detection::RemovedComponentEvents, storage::{Column, ComponentSparseSet, Storages}, system::Resource, }; @@ -224,6 +225,13 @@ impl<'w> UnsafeWorldCell<'w> { &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 #[inline] pub fn bundles(self) -> &'w Bundles { From 675990ceba285c7367a3ff286873d8c2e9a0796f Mon Sep 17 00:00:00 2001 From: Joseph <21144246+JoJoJet@users.noreply.github.com> Date: Sat, 9 Sep 2023 18:29:07 -0700 Subject: [PATCH 3/3] use unsafe_world to implement world --- crates/bevy_ecs/src/world/unsafe_world_cell.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index 093fdcc95a072..966c5cb913656 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -157,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 mutable borrows of world data, this means the caller cannot // misuse the returned `&World` - unsafe { &*self.0 } + unsafe { self.unsafe_world() } } /// Gets a reference to the [`World`] this [`UnsafeWorldCell`] belong to.