Skip to content

Commit db8d365

Browse files
authored
Migrate the rest of the engine to UnsafeWorldCell (#8833)
# Objective Follow-up to #6404 and #8292. Mutating the world through a shared reference is surprising, and it makes the meaning of `&World` unclear: sometimes it gives read-only access to the entire world, and sometimes it gives interior mutable access to only part of it. This is an up-to-date version of #6972. ## Solution Use `UnsafeWorldCell` for all interior mutability. Now, `&World` *always* gives you read-only access to the entire world. --- ## Changelog TODO - do we still care about changelogs? ## Migration Guide Mutating any world data using `&World` is now considered unsound -- the type `UnsafeWorldCell` must be used to achieve interior mutability. The following methods now accept `UnsafeWorldCell` instead of `&World`: - `QueryState`: `get_unchecked`, `iter_unchecked`, `iter_combinations_unchecked`, `for_each_unchecked`, `get_single_unchecked`, `get_single_unchecked_manual`. - `SystemState`: `get_unchecked_manual` ```rust let mut world = World::new(); let mut query = world.query::<&mut T>(); // Before: let t1 = query.get_unchecked(&world, entity_1); let t2 = query.get_unchecked(&world, entity_2); // After: let world_cell = world.as_unsafe_world_cell(); let t1 = query.get_unchecked(world_cell, entity_1); let t2 = query.get_unchecked(world_cell, entity_2); ``` The methods `QueryState::validate_world` and `SystemState::matches_world` now take a `WorldId` instead of `&World`: ```rust // Before: query_state.validate_world(&world); // After: query_state.validate_world(world.id()); ``` The methods `QueryState::update_archetypes` and `SystemState::update_archetypes` now take `UnsafeWorldCell` instead of `&World`: ```rust // Before: query_state.update_archetypes(&world); // After: query_state.update_archetypes(world.as_unsafe_world_cell_readonly()); ```
1 parent f7aa83a commit db8d365

File tree

13 files changed

+350
-217
lines changed

13 files changed

+350
-217
lines changed

benches/benches/bevy_ecs/world/world_get.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,9 +271,10 @@ pub fn query_get_component_simple(criterion: &mut Criterion) {
271271
let entity = world.spawn(A(0.0)).id();
272272
let mut query = world.query::<&mut A>();
273273

274+
let world_cell = world.as_unsafe_world_cell();
274275
bencher.iter(|| {
275276
for _x in 0..100000 {
276-
let mut a = unsafe { query.get_unchecked(&world, entity).unwrap() };
277+
let mut a = unsafe { query.get_unchecked(world_cell, entity).unwrap() };
277278
a.0 += 1.0;
278279
}
279280
});

crates/bevy_ecs/macros/src/fetch.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
257257
}
258258

259259
unsafe fn init_fetch<'__w>(
260-
_world: &'__w #path::world::World,
260+
_world: #path::world::unsafe_world_cell::UnsafeWorldCell<'__w>,
261261
state: &Self::State,
262262
_last_run: #path::component::Tick,
263263
_this_run: #path::component::Tick,

crates/bevy_ecs/src/query/fetch.rs

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::{
55
entity::Entity,
66
query::{Access, DebugCheckedUnwrap, FilteredAccess},
77
storage::{ComponentSparseSet, Table, TableRow},
8-
world::{Mut, Ref, World},
8+
world::{unsafe_world_cell::UnsafeWorldCell, Mut, Ref, World},
99
};
1010
pub use bevy_ecs_macros::WorldQuery;
1111
use bevy_ptr::{ThinSlicePtr, UnsafeCellDeref};
@@ -335,10 +335,11 @@ pub unsafe trait WorldQuery {
335335
///
336336
/// # Safety
337337
///
338-
/// `state` must have been initialized (via [`WorldQuery::init_state`]) using the same `world` passed
339-
/// in to this function.
338+
/// - `world` must have permission to access any of the components specified in `Self::update_archetype_component_access`.
339+
/// - `state` must have been initialized (via [`WorldQuery::init_state`]) using the same `world` passed
340+
/// in to this function.
340341
unsafe fn init_fetch<'w>(
341-
world: &'w World,
342+
world: UnsafeWorldCell<'w>,
342343
state: &Self::State,
343344
last_run: Tick,
344345
this_run: Tick,
@@ -372,8 +373,10 @@ pub unsafe trait WorldQuery {
372373
///
373374
/// # Safety
374375
///
375-
/// `archetype` and `tables` must be from the [`World`] [`WorldQuery::init_state`] was called on. `state` must
376-
/// be the [`Self::State`] this was initialized with.
376+
/// - `archetype` and `tables` must be from the same [`World`] that [`WorldQuery::init_state`] was called on.
377+
/// - [`Self::update_archetype_component_access`] must have been previously called with `archetype`.
378+
/// - `table` must correspond to `archetype`.
379+
/// - `state` must be the [`State`](Self::State) that `fetch` was initialized with.
377380
unsafe fn set_archetype<'w>(
378381
fetch: &mut Self::Fetch<'w>,
379382
state: &Self::State,
@@ -386,8 +389,10 @@ pub unsafe trait WorldQuery {
386389
///
387390
/// # Safety
388391
///
389-
/// `table` must be from the [`World`] [`WorldQuery::init_state`] was called on. `state` must be the
390-
/// [`Self::State`] this was initialized with.
392+
/// - `table` must be from the same [`World`] that [`WorldQuery::init_state`] was called on.
393+
/// - `table` must belong to an archetype that was previously registered with
394+
/// [`Self::update_archetype_component_access`].
395+
/// - `state` must be the [`State`](Self::State) that `fetch` was initialized with.
391396
unsafe fn set_table<'w>(fetch: &mut Self::Fetch<'w>, state: &Self::State, table: &'w Table);
392397

393398
/// Fetch [`Self::Item`](`WorldQuery::Item`) for either the given `entity` in the current [`Table`],
@@ -475,7 +480,7 @@ unsafe impl WorldQuery for Entity {
475480
const IS_ARCHETYPAL: bool = true;
476481

477482
unsafe fn init_fetch<'w>(
478-
_world: &'w World,
483+
_world: UnsafeWorldCell<'w>,
479484
_state: &Self::State,
480485
_last_run: Tick,
481486
_this_run: Tick,
@@ -558,7 +563,7 @@ unsafe impl<T: Component> WorldQuery for &T {
558563

559564
#[inline]
560565
unsafe fn init_fetch<'w>(
561-
world: &'w World,
566+
world: UnsafeWorldCell<'w>,
562567
&component_id: &ComponentId,
563568
_last_run: Tick,
564569
_this_run: Tick,
@@ -567,6 +572,11 @@ unsafe impl<T: Component> WorldQuery for &T {
567572
table_components: None,
568573
sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| {
569574
world
575+
// SAFETY: The underlying type associated with `component_id` is `T`,
576+
// which we are allowed to access since we registered it in `update_archetype_component_access`.
577+
// Note that we do not actually access any components in this function, we just get a shared
578+
// reference to the sparse set, which is used to access the components in `Self::fetch`.
579+
.unsafe_world()
570580
.storages()
571581
.sparse_sets
572582
.get(component_id)
@@ -704,7 +714,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> {
704714

705715
#[inline]
706716
unsafe fn init_fetch<'w>(
707-
world: &'w World,
717+
world: UnsafeWorldCell<'w>,
708718
&component_id: &ComponentId,
709719
last_run: Tick,
710720
this_run: Tick,
@@ -713,6 +723,8 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> {
713723
table_data: None,
714724
sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| {
715725
world
726+
// SAFETY: See &T::init_fetch.
727+
.unsafe_world()
716728
.storages()
717729
.sparse_sets
718730
.get(component_id)
@@ -866,7 +878,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T {
866878

867879
#[inline]
868880
unsafe fn init_fetch<'w>(
869-
world: &'w World,
881+
world: UnsafeWorldCell<'w>,
870882
&component_id: &ComponentId,
871883
last_run: Tick,
872884
this_run: Tick,
@@ -875,6 +887,8 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T {
875887
table_data: None,
876888
sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| {
877889
world
890+
// SAFETY: See &T::init_fetch.
891+
.unsafe_world()
878892
.storages()
879893
.sparse_sets
880894
.get(component_id)
@@ -1011,7 +1025,7 @@ unsafe impl<T: WorldQuery> WorldQuery for Option<T> {
10111025

10121026
#[inline]
10131027
unsafe fn init_fetch<'w>(
1014-
world: &'w World,
1028+
world: UnsafeWorldCell<'w>,
10151029
state: &T::State,
10161030
last_run: Tick,
10171031
this_run: Tick,
@@ -1116,7 +1130,7 @@ macro_rules! impl_tuple_fetch {
11161130

11171131
#[inline]
11181132
#[allow(clippy::unused_unit)]
1119-
unsafe fn init_fetch<'w>(_world: &'w World, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> {
1133+
unsafe fn init_fetch<'w>(_world: UnsafeWorldCell<'w>, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> {
11201134
let ($($name,)*) = state;
11211135
($($name::init_fetch(_world, $name, _last_run, _this_run),)*)
11221136
}
@@ -1226,7 +1240,7 @@ macro_rules! impl_anytuple_fetch {
12261240

12271241
#[inline]
12281242
#[allow(clippy::unused_unit)]
1229-
unsafe fn init_fetch<'w>(_world: &'w World, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> {
1243+
unsafe fn init_fetch<'w>(_world: UnsafeWorldCell<'w>, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> {
12301244
let ($($name,)*) = state;
12311245
($(($name::init_fetch(_world, $name, _last_run, _this_run), false),)*)
12321246
}
@@ -1350,7 +1364,13 @@ unsafe impl<Q: WorldQuery> WorldQuery for NopWorldQuery<Q> {
13501364
const IS_ARCHETYPAL: bool = true;
13511365

13521366
#[inline(always)]
1353-
unsafe fn init_fetch(_world: &World, _state: &Q::State, _last_run: Tick, _this_run: Tick) {}
1367+
unsafe fn init_fetch(
1368+
_world: UnsafeWorldCell,
1369+
_state: &Q::State,
1370+
_last_run: Tick,
1371+
_this_run: Tick,
1372+
) {
1373+
}
13541374

13551375
unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {}
13561376

@@ -1408,7 +1428,7 @@ unsafe impl<T: ?Sized> WorldQuery for PhantomData<T> {
14081428
fn shrink<'wlong: 'wshort, 'wshort>(_item: Self::Item<'wlong>) -> Self::Item<'wshort> {}
14091429

14101430
unsafe fn init_fetch<'w>(
1411-
_world: &'w World,
1431+
_world: UnsafeWorldCell<'w>,
14121432
_state: &Self::State,
14131433
_last_run: Tick,
14141434
_this_run: Tick,

crates/bevy_ecs/src/query/filter.rs

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::{
44
entity::Entity,
55
query::{Access, DebugCheckedUnwrap, FilteredAccess, WorldQuery},
66
storage::{Column, ComponentSparseSet, Table, TableRow},
7-
world::World,
7+
world::{unsafe_world_cell::UnsafeWorldCell, World},
88
};
99
use bevy_ptr::{ThinSlicePtr, UnsafeCellDeref};
1010
use bevy_utils::all_tuples;
@@ -51,7 +51,13 @@ unsafe impl<T: Component> WorldQuery for With<T> {
5151
fn shrink<'wlong: 'wshort, 'wshort>(_: Self::Item<'wlong>) -> Self::Item<'wshort> {}
5252

5353
#[inline]
54-
unsafe fn init_fetch(_world: &World, _state: &ComponentId, _last_run: Tick, _this_run: Tick) {}
54+
unsafe fn init_fetch(
55+
_world: UnsafeWorldCell,
56+
_state: &ComponentId,
57+
_last_run: Tick,
58+
_this_run: Tick,
59+
) {
60+
}
5561

5662
unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {}
5763

@@ -148,7 +154,13 @@ unsafe impl<T: Component> WorldQuery for Without<T> {
148154
fn shrink<'wlong: 'wshort, 'wshort>(_: Self::Item<'wlong>) -> Self::Item<'wshort> {}
149155

150156
#[inline]
151-
unsafe fn init_fetch(_world: &World, _state: &ComponentId, _last_run: Tick, _this_run: Tick) {}
157+
unsafe fn init_fetch(
158+
_world: UnsafeWorldCell,
159+
_state: &ComponentId,
160+
_last_run: Tick,
161+
_this_run: Tick,
162+
) {
163+
}
152164

153165
unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {}
154166

@@ -268,7 +280,7 @@ macro_rules! impl_query_filter_tuple {
268280
const IS_ARCHETYPAL: bool = true $(&& $filter::IS_ARCHETYPAL)*;
269281

270282
#[inline]
271-
unsafe fn init_fetch<'w>(world: &'w World, state: &Self::State, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> {
283+
unsafe fn init_fetch<'w>(world: UnsafeWorldCell<'w>, state: &Self::State, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> {
272284
let ($($filter,)*) = state;
273285
($(OrFetch {
274286
fetch: $filter::init_fetch(world, $filter, last_run, this_run),
@@ -412,12 +424,18 @@ macro_rules! impl_tick_filter {
412424
}
413425

414426
#[inline]
415-
unsafe fn init_fetch<'w>(world: &'w World, &id: &ComponentId, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> {
427+
unsafe fn init_fetch<'w>(
428+
world: UnsafeWorldCell<'w>,
429+
&id: &ComponentId,
430+
last_run: Tick,
431+
this_run: Tick
432+
) -> Self::Fetch<'w> {
416433
Self::Fetch::<'w> {
417434
table_ticks: None,
418435
sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet)
419436
.then(|| {
420-
world.storages()
437+
world.unsafe_world()
438+
.storages()
421439
.sparse_sets
422440
.get(id)
423441
.debug_checked_unwrap()

crates/bevy_ecs/src/query/iter.rs

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ use crate::{
22
archetype::{ArchetypeEntity, ArchetypeId, Archetypes},
33
component::Tick,
44
entity::{Entities, Entity},
5-
prelude::World,
65
query::{ArchetypeFilter, DebugCheckedUnwrap, QueryState, WorldQuery},
76
storage::{TableId, TableRow, Tables},
7+
world::unsafe_world_cell::UnsafeWorldCell,
88
};
99
use std::{borrow::Borrow, iter::FusedIterator, marker::PhantomData, mem::MaybeUninit};
1010

@@ -23,20 +23,19 @@ pub struct QueryIter<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> {
2323

2424
impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIter<'w, 's, Q, F> {
2525
/// # Safety
26-
/// This does not check for mutable query correctness. To be safe, make sure mutable queries
27-
/// have unique access to the components they query.
28-
/// This does not validate that `world.id()` matches `query_state.world_id`. Calling this on a `world`
29-
/// with a mismatched [`WorldId`](crate::world::WorldId) is unsound.
26+
/// - `world` must have permission to access any of the components registered in `query_state`.
27+
/// - `world` must be the same one used to initialize `query_state`.
3028
pub(crate) unsafe fn new(
31-
world: &'w World,
29+
world: UnsafeWorldCell<'w>,
3230
query_state: &'s QueryState<Q, F>,
3331
last_run: Tick,
3432
this_run: Tick,
3533
) -> Self {
3634
QueryIter {
3735
query_state,
38-
tables: &world.storages().tables,
39-
archetypes: &world.archetypes,
36+
// SAFETY: We only access table data that has been registered in `query_state`.
37+
tables: &world.unsafe_world().storages().tables,
38+
archetypes: world.archetypes(),
4039
cursor: QueryIterationCursor::init(world, query_state, last_run, this_run),
4140
}
4241
}
@@ -91,12 +90,10 @@ where
9190
I::Item: Borrow<Entity>,
9291
{
9392
/// # Safety
94-
/// This does not check for mutable query correctness. To be safe, make sure mutable queries
95-
/// have unique access to the components they query.
96-
/// This does not validate that `world.id()` matches `query_state.world_id`. Calling this on a `world`
97-
/// with a mismatched [`WorldId`](crate::world::WorldId) is unsound.
93+
/// - `world` must have permission to access any of the components registered in `query_state`.
94+
/// - `world` must be the same one used to initialize `query_state`.
9895
pub(crate) unsafe fn new<EntityList: IntoIterator<IntoIter = I>>(
99-
world: &'w World,
96+
world: UnsafeWorldCell<'w>,
10097
query_state: &'s QueryState<Q, F>,
10198
entity_list: EntityList,
10299
last_run: Tick,
@@ -106,9 +103,11 @@ where
106103
let filter = F::init_fetch(world, &query_state.filter_state, last_run, this_run);
107104
QueryManyIter {
108105
query_state,
109-
entities: &world.entities,
110-
archetypes: &world.archetypes,
111-
tables: &world.storages.tables,
106+
entities: world.entities(),
107+
archetypes: world.archetypes(),
108+
// SAFETY: We only access table data that has been registered in `query_state`.
109+
// This means `world` has permission to access the data we use.
110+
tables: &world.unsafe_world().storages.tables,
112111
fetch,
113112
filter,
114113
entity_iter: entity_list.into_iter(),
@@ -282,12 +281,10 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery, const K: usize>
282281
QueryCombinationIter<'w, 's, Q, F, K>
283282
{
284283
/// # Safety
285-
/// This does not check for mutable query correctness. To be safe, make sure mutable queries
286-
/// have unique access to the components they query.
287-
/// This does not validate that `world.id()` matches `query_state.world_id`. Calling this on a
288-
/// `world` with a mismatched [`WorldId`](crate::world::WorldId) is unsound.
284+
/// - `world` must have permission to access any of the components registered in `query_state`.
285+
/// - `world` must be the same one used to initialize `query_state`.
289286
pub(crate) unsafe fn new(
290-
world: &'w World,
287+
world: UnsafeWorldCell<'w>,
291288
query_state: &'s QueryState<Q, F>,
292289
last_run: Tick,
293290
this_run: Tick,
@@ -318,8 +315,9 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery, const K: usize>
318315

319316
QueryCombinationIter {
320317
query_state,
321-
tables: &world.storages().tables,
322-
archetypes: &world.archetypes,
318+
// SAFETY: We only access table data that has been registered in `query_state`.
319+
tables: &world.unsafe_world().storages().tables,
320+
archetypes: world.archetypes(),
323321
cursors: array.assume_init(),
324322
}
325323
}
@@ -485,7 +483,7 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's,
485483
const IS_DENSE: bool = Q::IS_DENSE && F::IS_DENSE;
486484

487485
unsafe fn init_empty(
488-
world: &'w World,
486+
world: UnsafeWorldCell<'w>,
489487
query_state: &'s QueryState<Q, F>,
490488
last_run: Tick,
491489
this_run: Tick,
@@ -497,8 +495,11 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's,
497495
}
498496
}
499497

498+
/// # Safety
499+
/// - `world` must have permission to access any of the components registered in `query_state`.
500+
/// - `world` must be the same one used to initialize `query_state`.
500501
unsafe fn init(
501-
world: &'w World,
502+
world: UnsafeWorldCell<'w>,
502503
query_state: &'s QueryState<Q, F>,
503504
last_run: Tick,
504505
this_run: Tick,

0 commit comments

Comments
 (0)