Skip to content

Commit 8cc255c

Browse files
authored
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.
1 parent 450328d commit 8cc255c

File tree

5 files changed

+29
-24
lines changed

5 files changed

+29
-24
lines changed

crates/bevy_ecs/src/query/filter.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -427,8 +427,7 @@ macro_rules! impl_tick_filter {
427427
table_ticks: None,
428428
sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet)
429429
.then(|| {
430-
world.unsafe_world()
431-
.storages()
430+
world.storages()
432431
.sparse_sets
433432
.get(id)
434433
.debug_checked_unwrap()

crates/bevy_ecs/src/query/state.rs

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -338,10 +338,12 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
338338
) -> Result<[ROQueryItem<'w, Q>; N], QueryEntityError> {
339339
self.update_archetypes(world);
340340

341-
// SAFETY: update_archetypes validates the `World` matches
341+
// SAFETY:
342+
// - We have read-only access to the entire world.
343+
// - `update_archetypes` validates that the `World` matches.
342344
unsafe {
343345
self.get_many_read_only_manual(
344-
world,
346+
world.as_unsafe_world_cell_readonly(),
345347
entities,
346348
world.last_change_tick(),
347349
world.read_change_tick(),
@@ -633,11 +635,13 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
633635
///
634636
/// # Safety
635637
///
636-
/// This must be called on the same `World` that the `Query` was generated from:
638+
/// * `world` must have permission to read all of the components returned from this call.
639+
/// No mutable references may coexist with any of the returned references.
640+
/// * This must be called on the same `World` that the `Query` was generated from:
637641
/// use `QueryState::validate_world` to verify this.
638642
pub(crate) unsafe fn get_many_read_only_manual<'w, const N: usize>(
639643
&self,
640-
world: &'w World,
644+
world: UnsafeWorldCell<'w>,
641645
entities: [Entity; N],
642646
last_run: Tick,
643647
this_run: Tick,
@@ -647,12 +651,9 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
647651
for (value, entity) in std::iter::zip(&mut values, entities) {
648652
// SAFETY: fetch is read-only
649653
// and world must be validated
650-
let item = self.as_readonly().get_unchecked_manual(
651-
world.as_unsafe_world_cell_readonly(),
652-
entity,
653-
last_run,
654-
this_run,
655-
)?;
654+
let item = self
655+
.as_readonly()
656+
.get_unchecked_manual(world, entity, last_run, this_run)?;
656657
*value = MaybeUninit::new(item);
657658
}
658659

crates/bevy_ecs/src/removal_detection.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -259,8 +259,7 @@ impl<'w, 's, T: Component> RemovedComponents<'w, 's, T> {
259259
// SAFETY: Only reads World removed component events
260260
unsafe impl<'a> ReadOnlySystemParam for &'a RemovedComponentEvents {}
261261

262-
// SAFETY: no component value access, removed component events can be read in parallel and are
263-
// never mutably borrowed during system execution
262+
// SAFETY: no component value access.
264263
unsafe impl<'a> SystemParam for &'a RemovedComponentEvents {
265264
type State = ();
266265
type Item<'w, 's> = &'w RemovedComponentEvents;
@@ -274,6 +273,6 @@ unsafe impl<'a> SystemParam for &'a RemovedComponentEvents {
274273
world: UnsafeWorldCell<'w>,
275274
_change_tick: Tick,
276275
) -> Self::Item<'w, 's> {
277-
world.world_metadata().removed_components()
276+
world.removed_components()
278277
}
279278
}

crates/bevy_ecs/src/system/query.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -874,14 +874,12 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> {
874874
&self,
875875
entities: [Entity; N],
876876
) -> Result<[ROQueryItem<'_, Q>; N], QueryEntityError> {
877-
// SAFETY: it is the scheduler's responsibility to ensure that `Query` is never handed out on the wrong `World`.
877+
// SAFETY:
878+
// - `&self` ensures there is no mutable access to any components accessible to this query.
879+
// - `self.world` matches `self.state`.
878880
unsafe {
879-
self.state.get_many_read_only_manual(
880-
self.world.unsafe_world(),
881-
entities,
882-
self.last_run,
883-
self.this_run,
884-
)
881+
self.state
882+
.get_many_read_only_manual(self.world, entities, self.last_run, self.this_run)
885883
}
886884
}
887885

crates/bevy_ecs/src/world/unsafe_world_cell.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use crate::{
1212
},
1313
entity::{Entities, Entity, EntityLocation},
1414
prelude::Component,
15+
removal_detection::RemovedComponentEvents,
1516
storage::{Column, ComponentSparseSet, Storages},
1617
system::Resource,
1718
};
@@ -156,7 +157,7 @@ impl<'w> UnsafeWorldCell<'w> {
156157
// - caller ensures there is no `&mut World` this makes it okay to make a `&World`
157158
// - caller ensures there is no mutable borrows of world data, this means the caller cannot
158159
// misuse the returned `&World`
159-
unsafe { &*self.0 }
160+
unsafe { self.unsafe_world() }
160161
}
161162

162163
/// Gets a reference to the [`World`] this [`UnsafeWorldCell`] belong to.
@@ -185,7 +186,7 @@ impl<'w> UnsafeWorldCell<'w> {
185186
/// - must not be used in a way that would conflict with any
186187
/// live exclusive borrows on world data
187188
#[inline]
188-
pub(crate) unsafe fn unsafe_world(self) -> &'w World {
189+
unsafe fn unsafe_world(self) -> &'w World {
189190
// SAFETY:
190191
// - caller ensures that the returned `&World` is not used in a way that would conflict
191192
// with any existing mutable borrows of world data
@@ -224,6 +225,13 @@ impl<'w> UnsafeWorldCell<'w> {
224225
&unsafe { self.world_metadata() }.components
225226
}
226227

228+
/// Retrieves this world's collection of [removed components](RemovedComponentEvents).
229+
pub fn removed_components(self) -> &'w RemovedComponentEvents {
230+
// SAFETY:
231+
// - we only access world metadata
232+
&unsafe { self.world_metadata() }.removed_components
233+
}
234+
227235
/// Retrieves this world's [`Bundles`] collection.
228236
#[inline]
229237
pub fn bundles(self) -> &'w Bundles {

0 commit comments

Comments
 (0)