diff --git a/benches/benches/bevy_ecs/world/world_get.rs b/benches/benches/bevy_ecs/world/world_get.rs index ee63498a095f1..725e9b2cf39f5 100644 --- a/benches/benches/bevy_ecs/world/world_get.rs +++ b/benches/benches/bevy_ecs/world/world_get.rs @@ -271,9 +271,10 @@ pub fn query_get_component_simple(criterion: &mut Criterion) { let entity = world.spawn(A(0.0)).id(); let mut query = world.query::<&mut A>(); + let world_cell = world.as_unsafe_world_cell(); bencher.iter(|| { for _x in 0..100000 { - let mut a = unsafe { query.get_unchecked(&world, entity).unwrap() }; + let mut a = unsafe { query.get_unchecked(world_cell, entity).unwrap() }; a.0 += 1.0; } }); diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 6383aa6fa306a..c2d925a56b94d 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -257,7 +257,7 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { } unsafe fn init_fetch<'__w>( - _world: &'__w #path::world::World, + _world: #path::world::unsafe_world_cell::UnsafeWorldCell<'__w>, state: &Self::State, _last_run: #path::component::Tick, _this_run: #path::component::Tick, diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 4540909f61685..cd0bb152d32ce 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -5,7 +5,7 @@ use crate::{ entity::Entity, query::{Access, DebugCheckedUnwrap, FilteredAccess}, storage::{ComponentSparseSet, Table, TableRow}, - world::{Mut, Ref, World}, + world::{unsafe_world_cell::UnsafeWorldCell, Mut, Ref, World}, }; pub use bevy_ecs_macros::WorldQuery; use bevy_ptr::{ThinSlicePtr, UnsafeCellDeref}; @@ -335,10 +335,11 @@ pub unsafe trait WorldQuery { /// /// # Safety /// - /// `state` must have been initialized (via [`WorldQuery::init_state`]) using the same `world` passed - /// in to this function. + /// - `world` must have permission to access any of the components specified in `Self::update_archetype_component_access`. + /// - `state` must have been initialized (via [`WorldQuery::init_state`]) using the same `world` passed + /// in to this function. unsafe fn init_fetch<'w>( - world: &'w World, + world: UnsafeWorldCell<'w>, state: &Self::State, last_run: Tick, this_run: Tick, @@ -372,8 +373,10 @@ pub unsafe trait WorldQuery { /// /// # Safety /// - /// `archetype` and `tables` must be from the [`World`] [`WorldQuery::init_state`] was called on. `state` must - /// be the [`Self::State`] this was initialized with. + /// - `archetype` and `tables` must be from the same [`World`] that [`WorldQuery::init_state`] was called on. + /// - [`Self::update_archetype_component_access`] must have been previously called with `archetype`. + /// - `table` must correspond to `archetype`. + /// - `state` must be the [`State`](Self::State) that `fetch` was initialized with. unsafe fn set_archetype<'w>( fetch: &mut Self::Fetch<'w>, state: &Self::State, @@ -386,8 +389,10 @@ pub unsafe trait WorldQuery { /// /// # Safety /// - /// `table` must be from the [`World`] [`WorldQuery::init_state`] was called on. `state` must be the - /// [`Self::State`] this was initialized with. + /// - `table` must be from the same [`World`] that [`WorldQuery::init_state`] was called on. + /// - `table` must belong to an archetype that was previously registered with + /// [`Self::update_archetype_component_access`]. + /// - `state` must be the [`State`](Self::State) that `fetch` was initialized with. unsafe fn set_table<'w>(fetch: &mut Self::Fetch<'w>, state: &Self::State, table: &'w Table); /// Fetch [`Self::Item`](`WorldQuery::Item`) for either the given `entity` in the current [`Table`], @@ -475,7 +480,7 @@ unsafe impl WorldQuery for Entity { const IS_ARCHETYPAL: bool = true; unsafe fn init_fetch<'w>( - _world: &'w World, + _world: UnsafeWorldCell<'w>, _state: &Self::State, _last_run: Tick, _this_run: Tick, @@ -558,7 +563,7 @@ unsafe impl WorldQuery for &T { #[inline] unsafe fn init_fetch<'w>( - world: &'w World, + world: UnsafeWorldCell<'w>, &component_id: &ComponentId, _last_run: Tick, _this_run: Tick, @@ -567,6 +572,11 @@ unsafe impl WorldQuery for &T { table_components: None, sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| { world + // SAFETY: The underlying type associated with `component_id` is `T`, + // which we are allowed to access since we registered it in `update_archetype_component_access`. + // Note that we do not actually access any components in this function, we just get a shared + // reference to the sparse set, which is used to access the components in `Self::fetch`. + .unsafe_world() .storages() .sparse_sets .get(component_id) @@ -704,7 +714,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { #[inline] unsafe fn init_fetch<'w>( - world: &'w World, + world: UnsafeWorldCell<'w>, &component_id: &ComponentId, last_run: Tick, this_run: Tick, @@ -713,6 +723,8 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { table_data: None, sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| { world + // SAFETY: See &T::init_fetch. + .unsafe_world() .storages() .sparse_sets .get(component_id) @@ -866,7 +878,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { #[inline] unsafe fn init_fetch<'w>( - world: &'w World, + world: UnsafeWorldCell<'w>, &component_id: &ComponentId, last_run: Tick, this_run: Tick, @@ -875,6 +887,8 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { table_data: None, sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| { world + // SAFETY: See &T::init_fetch. + .unsafe_world() .storages() .sparse_sets .get(component_id) @@ -1011,7 +1025,7 @@ unsafe impl WorldQuery for Option { #[inline] unsafe fn init_fetch<'w>( - world: &'w World, + world: UnsafeWorldCell<'w>, state: &T::State, last_run: Tick, this_run: Tick, @@ -1116,7 +1130,7 @@ macro_rules! impl_tuple_fetch { #[inline] #[allow(clippy::unused_unit)] - unsafe fn init_fetch<'w>(_world: &'w World, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> { + unsafe fn init_fetch<'w>(_world: UnsafeWorldCell<'w>, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> { let ($($name,)*) = state; ($($name::init_fetch(_world, $name, _last_run, _this_run),)*) } @@ -1226,7 +1240,7 @@ macro_rules! impl_anytuple_fetch { #[inline] #[allow(clippy::unused_unit)] - unsafe fn init_fetch<'w>(_world: &'w World, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> { + unsafe fn init_fetch<'w>(_world: UnsafeWorldCell<'w>, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> { let ($($name,)*) = state; ($(($name::init_fetch(_world, $name, _last_run, _this_run), false),)*) } @@ -1350,7 +1364,13 @@ unsafe impl WorldQuery for NopWorldQuery { const IS_ARCHETYPAL: bool = true; #[inline(always)] - unsafe fn init_fetch(_world: &World, _state: &Q::State, _last_run: Tick, _this_run: Tick) {} + unsafe fn init_fetch( + _world: UnsafeWorldCell, + _state: &Q::State, + _last_run: Tick, + _this_run: Tick, + ) { + } unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {} @@ -1408,7 +1428,7 @@ unsafe impl WorldQuery for PhantomData { fn shrink<'wlong: 'wshort, 'wshort>(_item: Self::Item<'wlong>) -> Self::Item<'wshort> {} unsafe fn init_fetch<'w>( - _world: &'w World, + _world: UnsafeWorldCell<'w>, _state: &Self::State, _last_run: Tick, _this_run: Tick, diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 996ef81b12ea0..b3f039566eb0f 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -4,7 +4,7 @@ use crate::{ entity::Entity, query::{Access, DebugCheckedUnwrap, FilteredAccess, WorldQuery}, storage::{Column, ComponentSparseSet, Table, TableRow}, - world::World, + world::{unsafe_world_cell::UnsafeWorldCell, World}, }; use bevy_ptr::{ThinSlicePtr, UnsafeCellDeref}; use bevy_utils::all_tuples; @@ -51,7 +51,13 @@ unsafe impl WorldQuery for With { fn shrink<'wlong: 'wshort, 'wshort>(_: Self::Item<'wlong>) -> Self::Item<'wshort> {} #[inline] - unsafe fn init_fetch(_world: &World, _state: &ComponentId, _last_run: Tick, _this_run: Tick) {} + unsafe fn init_fetch( + _world: UnsafeWorldCell, + _state: &ComponentId, + _last_run: Tick, + _this_run: Tick, + ) { + } unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {} @@ -148,7 +154,13 @@ unsafe impl WorldQuery for Without { fn shrink<'wlong: 'wshort, 'wshort>(_: Self::Item<'wlong>) -> Self::Item<'wshort> {} #[inline] - unsafe fn init_fetch(_world: &World, _state: &ComponentId, _last_run: Tick, _this_run: Tick) {} + unsafe fn init_fetch( + _world: UnsafeWorldCell, + _state: &ComponentId, + _last_run: Tick, + _this_run: Tick, + ) { + } unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {} @@ -268,7 +280,7 @@ macro_rules! impl_query_filter_tuple { const IS_ARCHETYPAL: bool = true $(&& $filter::IS_ARCHETYPAL)*; #[inline] - unsafe fn init_fetch<'w>(world: &'w World, state: &Self::State, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> { + unsafe fn init_fetch<'w>(world: UnsafeWorldCell<'w>, state: &Self::State, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> { let ($($filter,)*) = state; ($(OrFetch { fetch: $filter::init_fetch(world, $filter, last_run, this_run), @@ -412,12 +424,18 @@ macro_rules! impl_tick_filter { } #[inline] - unsafe fn init_fetch<'w>(world: &'w World, &id: &ComponentId, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> { + unsafe fn init_fetch<'w>( + world: UnsafeWorldCell<'w>, + &id: &ComponentId, + last_run: Tick, + this_run: Tick + ) -> Self::Fetch<'w> { Self::Fetch::<'w> { table_ticks: None, sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet) .then(|| { - world.storages() + world.unsafe_world() + .storages() .sparse_sets .get(id) .debug_checked_unwrap() diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 893df47aac3a3..2663a83a8da3f 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -2,9 +2,9 @@ use crate::{ archetype::{ArchetypeEntity, ArchetypeId, Archetypes}, component::Tick, entity::{Entities, Entity}, - prelude::World, query::{ArchetypeFilter, DebugCheckedUnwrap, QueryState, WorldQuery}, storage::{TableId, TableRow, Tables}, + world::unsafe_world_cell::UnsafeWorldCell, }; use std::{borrow::Borrow, iter::FusedIterator, marker::PhantomData, mem::MaybeUninit}; @@ -23,20 +23,19 @@ pub struct QueryIter<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> { impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIter<'w, 's, Q, F> { /// # Safety - /// This does not check for mutable query correctness. To be safe, make sure mutable queries - /// have unique access to the components they query. - /// This does not validate that `world.id()` matches `query_state.world_id`. Calling this on a `world` - /// with a mismatched [`WorldId`](crate::world::WorldId) is unsound. + /// - `world` must have permission to access any of the components registered in `query_state`. + /// - `world` must be the same one used to initialize `query_state`. pub(crate) unsafe fn new( - world: &'w World, + world: UnsafeWorldCell<'w>, query_state: &'s QueryState, last_run: Tick, this_run: Tick, ) -> Self { QueryIter { query_state, - tables: &world.storages().tables, - archetypes: &world.archetypes, + // SAFETY: We only access table data that has been registered in `query_state`. + tables: &world.unsafe_world().storages().tables, + archetypes: world.archetypes(), cursor: QueryIterationCursor::init(world, query_state, last_run, this_run), } } @@ -91,12 +90,10 @@ where I::Item: Borrow, { /// # Safety - /// This does not check for mutable query correctness. To be safe, make sure mutable queries - /// have unique access to the components they query. - /// This does not validate that `world.id()` matches `query_state.world_id`. Calling this on a `world` - /// with a mismatched [`WorldId`](crate::world::WorldId) is unsound. + /// - `world` must have permission to access any of the components registered in `query_state`. + /// - `world` must be the same one used to initialize `query_state`. pub(crate) unsafe fn new>( - world: &'w World, + world: UnsafeWorldCell<'w>, query_state: &'s QueryState, entity_list: EntityList, last_run: Tick, @@ -106,9 +103,11 @@ where let filter = F::init_fetch(world, &query_state.filter_state, last_run, this_run); QueryManyIter { query_state, - entities: &world.entities, - archetypes: &world.archetypes, - tables: &world.storages.tables, + entities: world.entities(), + archetypes: world.archetypes(), + // SAFETY: We only access table data that has been registered in `query_state`. + // This means `world` has permission to access the data we use. + tables: &world.unsafe_world().storages.tables, fetch, filter, entity_iter: entity_list.into_iter(), @@ -282,12 +281,10 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery, const K: usize> QueryCombinationIter<'w, 's, Q, F, K> { /// # Safety - /// This does not check for mutable query correctness. To be safe, make sure mutable queries - /// have unique access to the components they query. - /// This does not validate that `world.id()` matches `query_state.world_id`. Calling this on a - /// `world` with a mismatched [`WorldId`](crate::world::WorldId) is unsound. + /// - `world` must have permission to access any of the components registered in `query_state`. + /// - `world` must be the same one used to initialize `query_state`. pub(crate) unsafe fn new( - world: &'w World, + world: UnsafeWorldCell<'w>, query_state: &'s QueryState, last_run: Tick, this_run: Tick, @@ -318,8 +315,9 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery, const K: usize> QueryCombinationIter { query_state, - tables: &world.storages().tables, - archetypes: &world.archetypes, + // SAFETY: We only access table data that has been registered in `query_state`. + tables: &world.unsafe_world().storages().tables, + archetypes: world.archetypes(), cursors: array.assume_init(), } } @@ -485,7 +483,7 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's, const IS_DENSE: bool = Q::IS_DENSE && F::IS_DENSE; unsafe fn init_empty( - world: &'w World, + world: UnsafeWorldCell<'w>, query_state: &'s QueryState, last_run: Tick, this_run: Tick, @@ -497,8 +495,11 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's, } } + /// # Safety + /// - `world` must have permission to access any of the components registered in `query_state`. + /// - `world` must be the same one used to initialize `query_state`. unsafe fn init( - world: &'w World, + world: UnsafeWorldCell<'w>, query_state: &'s QueryState, last_run: Tick, this_run: Tick, diff --git a/crates/bevy_ecs/src/query/par_iter.rs b/crates/bevy_ecs/src/query/par_iter.rs index 77353bfb11fd0..ea54fa1aa4242 100644 --- a/crates/bevy_ecs/src/query/par_iter.rs +++ b/crates/bevy_ecs/src/query/par_iter.rs @@ -1,4 +1,4 @@ -use crate::{component::Tick, world::World}; +use crate::{component::Tick, world::unsafe_world_cell::UnsafeWorldCell}; use bevy_tasks::ComputeTaskPool; use std::ops::Range; @@ -82,7 +82,7 @@ impl BatchingStrategy { /// This struct is created by the [`Query::par_iter`](crate::system::Query::par_iter) and /// [`Query::par_iter_mut`](crate::system::Query::par_iter_mut) methods. pub struct QueryParIter<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> { - pub(crate) world: &'w World, + pub(crate) world: UnsafeWorldCell<'w>, pub(crate) state: &'s QueryState, pub(crate) last_run: Tick, pub(crate) this_run: Tick, @@ -178,7 +178,8 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryParIter<'w, 's, Q, F> { "Attempted to run parallel iteration over a query with an empty TaskPool" ); let max_size = if Q::IS_DENSE && F::IS_DENSE { - let tables = &self.world.storages().tables; + // SAFETY: We only access table metadata. + let tables = unsafe { &self.world.world_metadata().storages().tables }; self.state .matched_table_ids .iter() diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index d2bc1cc8468c6..267c7903893ba 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -8,7 +8,7 @@ use crate::{ QueryIter, QueryParIter, WorldQuery, }, storage::{TableId, TableRow}, - world::{World, WorldId}, + world::{unsafe_world_cell::UnsafeWorldCell, World, WorldId}, }; use bevy_tasks::ComputeTaskPool; #[cfg(feature = "trace")] @@ -134,20 +134,45 @@ impl QueryState { // SAFETY: NopFetch does not access any members while &self ensures no one has exclusive access unsafe { self.as_nop() - .iter_unchecked_manual(world, last_run, this_run) + .iter_unchecked_manual(world.as_unsafe_world_cell_readonly(), last_run, this_run) .next() .is_none() } } - /// Takes a query for the given [`World`], checks if the given world is the same as the query, and - /// updates the [`QueryState`]'s view of the [`World`] with any newly-added archetypes. + /// Updates the state's internal view of the [`World`]'s archetypes. If this is not called before querying data, + /// the results may not accurately reflect what is in the `world`. + /// + /// This is only required if a `manual` method (such as [`Self::get_manual`]) is being called, and it only needs to + /// be called if the `world` has been structurally mutated (i.e. added/removed a component or resource). Users using + /// non-`manual` methods such as [`QueryState::get`] do not need to call this as it will be automatically called for them. + /// + /// If you have an [`UnsafeWorldCell`] instead of `&World`, consider using [`QueryState::update_archetypes_unsafe_world_cell`]. /// /// # Panics /// /// If `world` does not match the one used to call `QueryState::new` for this instance. + #[inline] pub fn update_archetypes(&mut self, world: &World) { - self.validate_world(world); + self.update_archetypes_unsafe_world_cell(world.as_unsafe_world_cell_readonly()); + } + + /// Updates the state's internal view of the `world`'s archetypes. If this is not called before querying data, + /// the results may not accurately reflect what is in the `world`. + /// + /// This is only required if a `manual` method (such as [`Self::get_manual`]) is being called, and it only needs to + /// be called if the `world` has been structurally mutated (i.e. added/removed a component or resource). Users using + /// non-`manual` methods such as [`QueryState::get`] do not need to call this as it will be automatically called for them. + /// + /// # Note + /// + /// This method only accesses world metadata. + /// + /// # Panics + /// + /// If `world` does not match the one used to call `QueryState::new` for this instance. + pub fn update_archetypes_unsafe_world_cell(&mut self, world: UnsafeWorldCell) { + self.validate_world(world.id()); let archetypes = world.archetypes(); let new_generation = archetypes.generation(); let old_generation = std::mem::replace(&mut self.archetype_generation, new_generation); @@ -160,14 +185,14 @@ impl QueryState { /// # Panics /// - /// If `world` does not match the one used to call `QueryState::new` for this instance. + /// If `world_id` does not match the [`World`] used to call `QueryState::new` for this instance. /// /// Many unsafe query methods require the world to match for soundness. This function is the easiest /// way of ensuring that it matches. #[inline] - pub fn validate_world(&self, world: &World) { + pub fn validate_world(&self, world_id: WorldId) { assert!( - world.id() == self.world_id, + world_id == self.world_id, "Attempted to use {} with a mismatched World. QueryStates can only be used with the World they were created from.", std::any::type_name::(), ); @@ -217,7 +242,7 @@ impl QueryState { // SAFETY: query is read only unsafe { self.as_readonly().get_unchecked_manual( - world, + world.as_unsafe_world_cell_readonly(), entity, world.last_change_tick(), world.read_change_tick(), @@ -285,8 +310,16 @@ impl QueryState { ) -> Result, QueryEntityError> { self.update_archetypes(world); let change_tick = world.change_tick(); + let last_change_tick = world.last_change_tick(); // SAFETY: query has unique world access - unsafe { self.get_unchecked_manual(world, entity, world.last_change_tick(), change_tick) } + unsafe { + self.get_unchecked_manual( + world.as_unsafe_world_cell(), + entity, + last_change_tick, + change_tick, + ) + } } /// Returns the query results for the given array of [`Entity`]. @@ -336,10 +369,16 @@ impl QueryState { self.update_archetypes(world); let change_tick = world.change_tick(); + let last_change_tick = world.last_change_tick(); // SAFETY: method requires exclusive world access // and world has been validated via update_archetypes unsafe { - self.get_many_unchecked_manual(world, entities, world.last_change_tick(), change_tick) + self.get_many_unchecked_manual( + world.as_unsafe_world_cell(), + entities, + last_change_tick, + change_tick, + ) } } @@ -360,11 +399,11 @@ impl QueryState { world: &'w World, entity: Entity, ) -> Result, QueryEntityError> { - self.validate_world(world); + self.validate_world(world.id()); // SAFETY: query is read only and world is validated unsafe { self.as_readonly().get_unchecked_manual( - world, + world.as_unsafe_world_cell_readonly(), entity, world.last_change_tick(), world.read_change_tick(), @@ -381,16 +420,11 @@ impl QueryState { #[inline] pub unsafe fn get_unchecked<'w>( &mut self, - world: &'w World, + world: UnsafeWorldCell<'w>, entity: Entity, ) -> Result, QueryEntityError> { - self.update_archetypes(world); - self.get_unchecked_manual( - world, - entity, - world.last_change_tick(), - world.read_change_tick(), - ) + self.update_archetypes_unsafe_world_cell(world); + self.get_unchecked_manual(world, entity, world.last_change_tick(), world.change_tick()) } /// Gets the query result for the given [`World`] and [`Entity`], where the last change and @@ -405,13 +439,13 @@ impl QueryState { /// use `QueryState::validate_world` to verify this. pub(crate) unsafe fn get_unchecked_manual<'w>( &self, - world: &'w World, + world: UnsafeWorldCell<'w>, entity: Entity, last_run: Tick, this_run: Tick, ) -> Result, QueryEntityError> { let location = world - .entities + .entities() .get(entity) .ok_or(QueryEntityError::NoSuchEntity(entity))?; if !self @@ -421,13 +455,14 @@ impl QueryState { return Err(QueryEntityError::QueryDoesNotMatch(entity)); } let archetype = world - .archetypes + .archetypes() .get(location.archetype_id) .debug_checked_unwrap(); let mut fetch = Q::init_fetch(world, &self.fetch_state, last_run, this_run); let mut filter = F::init_fetch(world, &self.filter_state, last_run, this_run); let table = world + .unsafe_world() .storages() .tables .get(location.table_id) @@ -461,9 +496,12 @@ 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, entity, last_run, this_run)?; + let item = self.as_readonly().get_unchecked_manual( + world.as_unsafe_world_cell_readonly(), + entity, + last_run, + this_run, + )?; *value = MaybeUninit::new(item); } @@ -483,7 +521,7 @@ impl QueryState { /// use `QueryState::validate_world` to verify this. pub(crate) unsafe fn get_many_unchecked_manual<'w, const N: usize>( &self, - world: &'w World, + world: UnsafeWorldCell<'w>, entities: [Entity; N], last_run: Tick, this_run: Tick, @@ -520,7 +558,7 @@ impl QueryState { // SAFETY: query is read only unsafe { self.as_readonly().iter_unchecked_manual( - world, + world.as_unsafe_world_cell_readonly(), world.last_change_tick(), world.read_change_tick(), ) @@ -530,10 +568,13 @@ impl QueryState { /// Returns an [`Iterator`] over the query results for the given [`World`]. #[inline] pub fn iter_mut<'w, 's>(&'s mut self, world: &'w mut World) -> QueryIter<'w, 's, Q, F> { - let change_tick = world.change_tick(); self.update_archetypes(world); + let change_tick = world.change_tick(); + let last_change_tick = world.last_change_tick(); // SAFETY: query has unique world access - unsafe { self.iter_unchecked_manual(world, world.last_change_tick(), change_tick) } + unsafe { + self.iter_unchecked_manual(world.as_unsafe_world_cell(), last_change_tick, change_tick) + } } /// Returns an [`Iterator`] over the query results for the given [`World`] without updating the query's archetypes. @@ -545,11 +586,11 @@ impl QueryState { &'s self, world: &'w World, ) -> QueryIter<'w, 's, Q::ReadOnly, F::ReadOnly> { - self.validate_world(world); + self.validate_world(world.id()); // SAFETY: query is read only and world is validated unsafe { self.as_readonly().iter_unchecked_manual( - world, + world.as_unsafe_world_cell_readonly(), world.last_change_tick(), world.read_change_tick(), ) @@ -586,7 +627,7 @@ impl QueryState { // SAFETY: query is read only unsafe { self.as_readonly().iter_combinations_unchecked_manual( - world, + world.as_unsafe_world_cell_readonly(), world.last_change_tick(), world.read_change_tick(), ) @@ -615,11 +656,16 @@ impl QueryState { &'s mut self, world: &'w mut World, ) -> QueryCombinationIter<'w, 's, Q, F, K> { - let change_tick = world.change_tick(); self.update_archetypes(world); + let change_tick = world.change_tick(); + let last_change_tick = world.last_change_tick(); // SAFETY: query has unique world access unsafe { - self.iter_combinations_unchecked_manual(world, world.last_change_tick(), change_tick) + self.iter_combinations_unchecked_manual( + world.as_unsafe_world_cell(), + last_change_tick, + change_tick, + ) } } @@ -645,7 +691,7 @@ impl QueryState { unsafe { self.as_readonly().iter_many_unchecked_manual( entities, - world, + world.as_unsafe_world_cell_readonly(), world.last_change_tick(), world.read_change_tick(), ) @@ -675,12 +721,12 @@ impl QueryState { where EntityList::Item: Borrow, { - self.validate_world(world); + self.validate_world(world.id()); // SAFETY: query is read only, world id is validated unsafe { self.as_readonly().iter_many_unchecked_manual( entities, - world, + world.as_unsafe_world_cell_readonly(), world.last_change_tick(), world.read_change_tick(), ) @@ -702,9 +748,15 @@ impl QueryState { { self.update_archetypes(world); let change_tick = world.change_tick(); + let last_change_tick = world.last_change_tick(); // SAFETY: Query has unique world access. unsafe { - self.iter_many_unchecked_manual(entities, world, world.last_change_tick(), change_tick) + self.iter_many_unchecked_manual( + entities, + world.as_unsafe_world_cell(), + last_change_tick, + change_tick, + ) } } @@ -717,10 +769,10 @@ impl QueryState { #[inline] pub unsafe fn iter_unchecked<'w, 's>( &'s mut self, - world: &'w World, + world: UnsafeWorldCell<'w>, ) -> QueryIter<'w, 's, Q, F> { - self.update_archetypes(world); - self.iter_unchecked_manual(world, world.last_change_tick(), world.read_change_tick()) + self.update_archetypes_unsafe_world_cell(world); + self.iter_unchecked_manual(world, world.last_change_tick(), world.change_tick()) } /// Returns an [`Iterator`] over all possible combinations of `K` query results for the @@ -734,13 +786,13 @@ impl QueryState { #[inline] pub unsafe fn iter_combinations_unchecked<'w, 's, const K: usize>( &'s mut self, - world: &'w World, + world: UnsafeWorldCell<'w>, ) -> QueryCombinationIter<'w, 's, Q, F, K> { - self.update_archetypes(world); + self.update_archetypes_unsafe_world_cell(world); self.iter_combinations_unchecked_manual( world, world.last_change_tick(), - world.read_change_tick(), + world.change_tick(), ) } @@ -756,7 +808,7 @@ impl QueryState { #[inline] pub(crate) unsafe fn iter_unchecked_manual<'w, 's>( &'s self, - world: &'w World, + world: UnsafeWorldCell<'w>, last_run: Tick, this_run: Tick, ) -> QueryIter<'w, 's, Q, F> { @@ -777,7 +829,7 @@ impl QueryState { pub(crate) unsafe fn iter_many_unchecked_manual<'w, 's, EntityList: IntoIterator>( &'s self, entities: EntityList, - world: &'w World, + world: UnsafeWorldCell<'w>, last_run: Tick, this_run: Tick, ) -> QueryManyIter<'w, 's, Q, F, EntityList::IntoIter> @@ -800,7 +852,7 @@ impl QueryState { #[inline] pub(crate) unsafe fn iter_combinations_unchecked_manual<'w, 's, const K: usize>( &'s self, - world: &'w World, + world: UnsafeWorldCell<'w>, last_run: Tick, this_run: Tick, ) -> QueryCombinationIter<'w, 's, Q, F, K> { @@ -817,7 +869,7 @@ impl QueryState { // SAFETY: query is read only unsafe { self.as_readonly().for_each_unchecked_manual( - world, + world.as_unsafe_world_cell_readonly(), func, world.last_change_tick(), world.read_change_tick(), @@ -829,11 +881,17 @@ impl QueryState { /// `iter_mut()` method, but cannot be chained like a normal [`Iterator`]. #[inline] pub fn for_each_mut<'w, FN: FnMut(Q::Item<'w>)>(&mut self, world: &'w mut World, func: FN) { - let change_tick = world.change_tick(); self.update_archetypes(world); + let change_tick = world.change_tick(); + let last_change_tick = world.last_change_tick(); // SAFETY: query has unique world access unsafe { - self.for_each_unchecked_manual(world, func, world.last_change_tick(), change_tick); + self.for_each_unchecked_manual( + world.as_unsafe_world_cell(), + func, + last_change_tick, + change_tick, + ); } } @@ -847,16 +905,11 @@ impl QueryState { #[inline] pub unsafe fn for_each_unchecked<'w, FN: FnMut(Q::Item<'w>)>( &mut self, - world: &'w World, + world: UnsafeWorldCell<'w>, func: FN, ) { - self.update_archetypes(world); - self.for_each_unchecked_manual( - world, - func, - world.last_change_tick(), - world.read_change_tick(), - ); + self.update_archetypes_unsafe_world_cell(world); + self.for_each_unchecked_manual(world, func, world.last_change_tick(), world.change_tick()); } /// Returns a parallel iterator over the query results for the given [`World`]. @@ -871,7 +924,7 @@ impl QueryState { ) -> QueryParIter<'w, 's, Q::ReadOnly, F::ReadOnly> { self.update_archetypes(world); QueryParIter { - world, + world: world.as_unsafe_world_cell_readonly(), state: self.as_readonly(), last_run: world.last_change_tick(), this_run: world.read_change_tick(), @@ -888,10 +941,11 @@ impl QueryState { pub fn par_iter_mut<'w, 's>(&'s mut self, world: &'w mut World) -> QueryParIter<'w, 's, Q, F> { self.update_archetypes(world); let this_run = world.change_tick(); + let last_run = world.last_change_tick(); QueryParIter { - world, + world: world.as_unsafe_world_cell(), state: self, - last_run: world.last_change_tick(), + last_run, this_run, batching_strategy: BatchingStrategy::new(), } @@ -909,7 +963,7 @@ impl QueryState { /// with a mismatched [`WorldId`] is unsound. pub(crate) unsafe fn for_each_unchecked_manual<'w, FN: FnMut(Q::Item<'w>)>( &self, - world: &'w World, + world: UnsafeWorldCell<'w>, mut func: FN, last_run: Tick, this_run: Tick, @@ -919,7 +973,7 @@ impl QueryState { let mut fetch = Q::init_fetch(world, &self.fetch_state, last_run, this_run); let mut filter = F::init_fetch(world, &self.filter_state, last_run, this_run); - let tables = &world.storages().tables; + let tables = &world.unsafe_world().storages().tables; if Q::IS_DENSE && F::IS_DENSE { for table_id in &self.matched_table_ids { let table = tables.get(*table_id).debug_checked_unwrap(); @@ -937,7 +991,7 @@ impl QueryState { } } } else { - let archetypes = &world.archetypes; + let archetypes = world.archetypes(); for archetype_id in &self.matched_archetype_ids { let archetype = archetypes.get(*archetype_id).debug_checked_unwrap(); let table = tables.get(archetype.table_id()).debug_checked_unwrap(); @@ -983,7 +1037,7 @@ impl QueryState { FN: Fn(Q::Item<'w>) + Send + Sync + Clone, >( &self, - world: &'w World, + world: UnsafeWorldCell<'w>, batch_size: usize, func: FN, last_run: Tick, @@ -993,7 +1047,8 @@ impl QueryState { // QueryIter, QueryIterationCursor, QueryManyIter, QueryCombinationIter, QueryState::for_each_unchecked_manual, QueryState::par_for_each_unchecked_manual ComputeTaskPool::get().scope(|scope| { if Q::IS_DENSE && F::IS_DENSE { - let tables = &world.storages().tables; + // SAFETY: We only access table data that has been registered in `self.archetype_component_access`. + let tables = &world.unsafe_world().storages().tables; for table_id in &self.matched_table_ids { let table = &tables[*table_id]; if table.is_empty() { @@ -1009,7 +1064,7 @@ impl QueryState { Q::init_fetch(world, &self.fetch_state, last_run, this_run); let mut filter = F::init_fetch(world, &self.filter_state, last_run, this_run); - let tables = &world.storages().tables; + let tables = &world.unsafe_world().storages().tables; let table = tables.get(*table_id).debug_checked_unwrap(); let entities = table.entities(); Q::set_table(&mut fetch, &self.fetch_state, table); @@ -1037,7 +1092,7 @@ impl QueryState { } } } else { - let archetypes = &world.archetypes; + let archetypes = world.archetypes(); for archetype_id in &self.matched_archetype_ids { let mut offset = 0; let archetype = &archetypes[*archetype_id]; @@ -1053,9 +1108,9 @@ impl QueryState { Q::init_fetch(world, &self.fetch_state, last_run, this_run); let mut filter = F::init_fetch(world, &self.filter_state, last_run, this_run); - let tables = &world.storages().tables; + let tables = &world.unsafe_world().storages().tables; let archetype = - world.archetypes.get(*archetype_id).debug_checked_unwrap(); + world.archetypes().get(*archetype_id).debug_checked_unwrap(); let table = tables.get(archetype.table_id()).debug_checked_unwrap(); Q::set_archetype(&mut fetch, &self.fetch_state, archetype, table); F::set_archetype(&mut filter, &self.filter_state, archetype, table); @@ -1130,7 +1185,7 @@ impl QueryState { // SAFETY: query is read only unsafe { self.as_readonly().get_single_unchecked_manual( - world, + world.as_unsafe_world_cell_readonly(), world.last_change_tick(), world.read_change_tick(), ) @@ -1164,8 +1219,15 @@ impl QueryState { self.update_archetypes(world); let change_tick = world.change_tick(); + let last_change_tick = world.last_change_tick(); // SAFETY: query has unique world access - unsafe { self.get_single_unchecked_manual(world, world.last_change_tick(), change_tick) } + unsafe { + self.get_single_unchecked_manual( + world.as_unsafe_world_cell(), + last_change_tick, + change_tick, + ) + } } /// Returns a query result when there is exactly one entity matching the query. @@ -1180,10 +1242,10 @@ impl QueryState { #[inline] pub unsafe fn get_single_unchecked<'w>( &mut self, - world: &'w World, + world: UnsafeWorldCell<'w>, ) -> Result, QuerySingleError> { - self.update_archetypes(world); - self.get_single_unchecked_manual(world, world.last_change_tick(), world.read_change_tick()) + self.update_archetypes_unsafe_world_cell(world); + self.get_single_unchecked_manual(world, world.last_change_tick(), world.change_tick()) } /// Returns a query result when there is exactly one entity matching the query, @@ -1199,7 +1261,7 @@ impl QueryState { #[inline] pub unsafe fn get_single_unchecked_manual<'w>( &self, - world: &'w World, + world: UnsafeWorldCell<'w>, last_run: Tick, this_run: Tick, ) -> Result, QuerySingleError> { @@ -1269,11 +1331,11 @@ mod tests { // as it is shared and unsafe // We don't care about aliased mutability for the read-only equivalent - // SAFETY: mutable access is not checked, but we own the world and don't use the query results + // SAFETY: Query does not access world data. assert!(unsafe { query_state .get_many_unchecked_manual::<10>( - &world, + world.as_unsafe_world_cell_readonly(), entities.clone().try_into().unwrap(), last_change_tick, change_tick, @@ -1282,11 +1344,11 @@ mod tests { }); assert_eq!( - // SAFETY: mutable access is not checked, but we own the world and don't use the query results + // SAFETY: Query does not access world data. unsafe { query_state .get_many_unchecked_manual( - &world, + world.as_unsafe_world_cell_readonly(), [entities[0], entities[0]], last_change_tick, change_tick, @@ -1297,11 +1359,11 @@ mod tests { ); assert_eq!( - // SAFETY: mutable access is not checked, but we own the world and don't use the query results + // SAFETY: Query does not access world data. unsafe { query_state .get_many_unchecked_manual( - &world, + world.as_unsafe_world_cell_readonly(), [entities[0], entities[1], entities[0]], last_change_tick, change_tick, @@ -1312,11 +1374,11 @@ mod tests { ); assert_eq!( - // SAFETY: mutable access is not checked, but we own the world and don't use the query results + // SAFETY: Query does not access world data. unsafe { query_state .get_many_unchecked_manual( - &world, + world.as_unsafe_world_cell_readonly(), [entities[9], entities[9]], last_change_tick, change_tick, diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 1e4647389a97e..e245f93716d78 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -183,19 +183,20 @@ impl SystemState { where Param: ReadOnlySystemParam, { - self.validate_world(world); + self.validate_world(world.id()); self.update_archetypes(world); - // SAFETY: Param is read-only and doesn't allow mutable access to World. It also matches the World this SystemState was created with. - unsafe { self.get_unchecked_manual(world) } + // SAFETY: Param is read-only and doesn't allow mutable access to World. + // It also matches the World this SystemState was created with. + unsafe { self.get_unchecked_manual(world.as_unsafe_world_cell_readonly()) } } /// Retrieve the mutable [`SystemParam`] values. #[inline] pub fn get_mut<'w, 's>(&'s mut self, world: &'w mut World) -> SystemParamItem<'w, 's, Param> { - self.validate_world(world); + self.validate_world(world.id()); self.update_archetypes(world); // SAFETY: World is uniquely borrowed and matches the World this SystemState was created with. - unsafe { self.get_unchecked_manual(world) } + unsafe { self.get_unchecked_manual(world.as_unsafe_world_cell()) } } /// Applies all state queued up for [`SystemParam`] values. For example, this will apply commands queued up @@ -206,20 +207,20 @@ impl SystemState { Param::apply(&mut self.param_state, &self.meta, world); } - /// Returns `true` if `world` is the same one that was used to call [`SystemState::new`]. + /// Returns `true` if `world_id` matches the [`World`] that was used to call [`SystemState::new`]. /// Otherwise, this returns false. #[inline] - pub fn matches_world(&self, world: &World) -> bool { - self.world_id == world.id() + pub fn matches_world(&self, world_id: WorldId) -> bool { + self.world_id == world_id } - /// Asserts that the [`SystemState`] matches the provided [`World`]. + /// Asserts that the [`SystemState`] matches the provided world. #[inline] - fn validate_world(&self, world: &World) { - assert!(self.matches_world(world), "Encountered a mismatched World. A SystemState cannot be used with Worlds other than the one it was created with."); + fn validate_world(&self, world_id: WorldId) { + assert!(self.matches_world(world_id), "Encountered a mismatched World. A SystemState cannot be used with Worlds other than the one it was created with."); } - /// Updates the state's internal view of the `world`'s archetypes. If this is not called before fetching the parameters, + /// Updates the state's internal view of the [`World`]'s archetypes. If this is not called before fetching the parameters, /// the results may not accurately reflect what is in the `world`. /// /// This is only required if [`SystemState::get_manual`] or [`SystemState::get_manual_mut`] is being called, and it only needs to @@ -227,6 +228,21 @@ impl SystemState { /// [`SystemState::get`] or [`SystemState::get_mut`] do not need to call this as it will be automatically called for them. #[inline] pub fn update_archetypes(&mut self, world: &World) { + self.update_archetypes_unsafe_world_cell(world.as_unsafe_world_cell_readonly()); + } + + /// Updates the state's internal view of the `world`'s archetypes. If this is not called before fetching the parameters, + /// the results may not accurately reflect what is in the `world`. + /// + /// This is only required if [`SystemState::get_manual`] or [`SystemState::get_manual_mut`] is being called, and it only needs to + /// be called if the `world` has been structurally mutated (i.e. added/removed a component or resource). Users using + /// [`SystemState::get`] or [`SystemState::get_mut`] do not need to call this as it will be automatically called for them. + /// + /// # Note + /// + /// This method only accesses world metadata. + #[inline] + pub fn update_archetypes_unsafe_world_cell(&mut self, world: UnsafeWorldCell) { let archetypes = world.archetypes(); let new_generation = archetypes.generation(); let old_generation = std::mem::replace(&mut self.archetype_generation, new_generation); @@ -254,10 +270,11 @@ impl SystemState { where Param: ReadOnlySystemParam, { - self.validate_world(world); + self.validate_world(world.id()); let change_tick = world.read_change_tick(); - // SAFETY: Param is read-only and doesn't allow mutable access to World. It also matches the World this SystemState was created with. - unsafe { self.fetch(world, change_tick) } + // SAFETY: Param is read-only and doesn't allow mutable access to World. + // It also matches the World this SystemState was created with. + unsafe { self.fetch(world.as_unsafe_world_cell_readonly(), change_tick) } } /// Retrieve the mutable [`SystemParam`] values. This will not update the state's view of the world's archetypes @@ -272,10 +289,10 @@ impl SystemState { &'s mut self, world: &'w mut World, ) -> SystemParamItem<'w, 's, Param> { - self.validate_world(world); + self.validate_world(world.id()); let change_tick = world.change_tick(); // SAFETY: World is uniquely borrowed and matches the World this SystemState was created with. - unsafe { self.fetch(world, change_tick) } + unsafe { self.fetch(world.as_unsafe_world_cell(), change_tick) } } /// Retrieve the [`SystemParam`] values. This will not update archetypes automatically. @@ -287,7 +304,7 @@ impl SystemState { #[inline] pub unsafe fn get_unchecked_manual<'w, 's>( &'s mut self, - world: &'w World, + world: UnsafeWorldCell<'w>, ) -> SystemParamItem<'w, 's, Param> { let change_tick = world.increment_change_tick(); self.fetch(world, change_tick) @@ -300,15 +317,10 @@ impl SystemState { #[inline] unsafe fn fetch<'w, 's>( &'s mut self, - world: &'w World, + world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> SystemParamItem<'w, 's, Param> { - let param = Param::get_param( - &mut self.param_state, - &self.meta, - world.as_unsafe_world_cell_migration_internal(), - change_tick, - ); + let param = Param::get_param(&mut self.param_state, &self.meta, world, change_tick); self.meta.last_run = change_tick; param } diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index 63ed5235e3a05..06224a30d4525 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -1727,7 +1727,15 @@ mod tests { let world2 = World::new(); let qstate = world1.query::<()>(); // SAFETY: doesnt access anything - let query = unsafe { Query::new(&world2, &qstate, Tick::new(0), Tick::new(0), false) }; + let query = unsafe { + Query::new( + world2.as_unsafe_world_cell_readonly(), + &qstate, + Tick::new(0), + Tick::new(0), + false, + ) + }; query.iter(); } diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 1e18705dcede0..21d334338711e 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -5,7 +5,7 @@ use crate::{ BatchingStrategy, QueryCombinationIter, QueryEntityError, QueryIter, QueryManyIter, QueryParIter, QuerySingleError, QueryState, ROQueryItem, ReadOnlyWorldQuery, WorldQuery, }, - world::{Mut, World}, + world::{unsafe_world_cell::UnsafeWorldCell, Mut}, }; use std::{any::TypeId, borrow::Borrow, fmt::Debug}; @@ -24,6 +24,8 @@ use std::{any::TypeId, borrow::Borrow, fmt::Debug}; /// A set of conditions that determines whether query items should be kept or discarded. /// This type parameter is optional. /// +/// [`World`]: crate::world::World +/// /// # System parameter declaration /// /// A query should always be declared as a system parameter. @@ -274,7 +276,8 @@ use std::{any::TypeId, borrow::Borrow, fmt::Debug}; /// [`With`]: crate::query::With /// [`Without`]: crate::query::Without pub struct Query<'world, 'state, Q: WorldQuery, F: ReadOnlyWorldQuery = ()> { - world: &'world World, + // SAFETY: Must have access to the components registered in `state`. + world: UnsafeWorldCell<'world>, state: &'state QueryState, last_run: Tick, this_run: Tick, @@ -288,7 +291,12 @@ pub struct Query<'world, 'state, Q: WorldQuery, F: ReadOnlyWorldQuery = ()> { impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> std::fmt::Debug for Query<'w, 's, Q, F> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "Query {{ matched entities: {}, world: {:?}, state: {:?}, last_run: {:?}, this_run: {:?} }}", self.iter().count(), self.world, self.state, self.last_run, self.this_run) + write!( + f, "Query {{ matched entities: {}, world: {:?}, state: {:?}, last_run: {:?}, this_run: {:?} }}", + self.iter().count(), + // SAFETY: World's Debug implementation only accesses metadata. + unsafe { self.world.world_metadata() }, + self.state, self.last_run, self.this_run) } } @@ -305,13 +313,13 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { /// called in ways that ensure the queries have unique mutable access. #[inline] pub(crate) unsafe fn new( - world: &'w World, + world: UnsafeWorldCell<'w>, state: &'s QueryState, last_run: Tick, this_run: Tick, force_read_only_component_access: bool, ) -> Self { - state.validate_world(world); + state.validate_world(world.id()); Self { force_read_only_component_access, @@ -369,8 +377,9 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { /// - [`for_each`](Self::for_each) for the closure based alternative. #[inline] pub fn iter(&self) -> QueryIter<'_, 's, Q::ReadOnly, F::ReadOnly> { - // SAFETY: system runs without conflicts with other systems. - // same-system queries have runtime borrow checks when they conflict + // SAFETY: + // - `self.world` has permission to access the required components. + // - The query is read-only, so it can be aliased even if it was originaly mutable. unsafe { self.state .as_readonly() @@ -404,8 +413,7 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { /// - [`for_each_mut`](Self::for_each_mut) for the closure based alternative. #[inline] pub fn iter_mut(&mut self) -> QueryIter<'_, 's, Q, F> { - // SAFETY: system runs without conflicts with other systems. - // same-system queries have runtime borrow checks when they conflict + // SAFETY: `self.world` has permission to access the required components. unsafe { self.state .iter_unchecked_manual(self.world, self.last_run, self.this_run) @@ -435,8 +443,9 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { pub fn iter_combinations( &self, ) -> QueryCombinationIter<'_, 's, Q::ReadOnly, F::ReadOnly, K> { - // SAFETY: system runs without conflicts with other systems. - // same-system queries have runtime borrow checks when they conflict + // SAFETY: + // - `self.world` has permission to access the required components. + // - The query is read-only, so it can be aliased even if it was originaly mutable. unsafe { self.state.as_readonly().iter_combinations_unchecked_manual( self.world, @@ -469,8 +478,7 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { pub fn iter_combinations_mut( &mut self, ) -> QueryCombinationIter<'_, 's, Q, F, K> { - // SAFETY: system runs without conflicts with other systems. - // same-system queries have runtime borrow checks when they conflict + // SAFETY: `self.world` has permission to access the required components. unsafe { self.state .iter_combinations_unchecked_manual(self.world, self.last_run, self.this_run) @@ -521,8 +529,9 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { where EntityList::Item: Borrow, { - // SAFETY: system runs without conflicts with other systems. - // same-system queries have runtime borrow checks when they conflict + // SAFETY: + // - `self.world` has permission to access the required components. + // - The query is read-only, so it can be aliased even if it was originaly mutable. unsafe { self.state.as_readonly().iter_many_unchecked_manual( entities, @@ -574,8 +583,7 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { where EntityList::Item: Borrow, { - // SAFETY: system runs without conflicts with other systems. - // same-system queries have runtime borrow checks when they conflict + // SAFETY: `self.world` has permission to access the required components. unsafe { self.state.iter_many_unchecked_manual( entities, @@ -598,8 +606,9 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { /// - [`iter`](Self::iter) and [`iter_mut`](Self::iter_mut) for the safe versions. #[inline] pub unsafe fn iter_unsafe(&self) -> QueryIter<'_, 's, Q, F> { - // SEMI-SAFETY: system runs without conflicts with other systems. - // same-system queries have runtime borrow checks when they conflict + // SAFETY: + // - `self.world` has permission to access the required components. + // - The caller ensures that this operation will not result in any aliased mutable accesses. self.state .iter_unchecked_manual(self.world, self.last_run, self.this_run) } @@ -618,8 +627,9 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { pub unsafe fn iter_combinations_unsafe( &self, ) -> QueryCombinationIter<'_, 's, Q, F, K> { - // SEMI-SAFETY: system runs without conflicts with other systems. - // same-system queries have runtime borrow checks when they conflict + // SAFETY: + // - `self.world` has permission to access the required components. + // - The caller ensures that this operation will not result in any aliased mutable accesses. self.state .iter_combinations_unchecked_manual(self.world, self.last_run, self.this_run) } @@ -642,6 +652,9 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { where EntityList::Item: Borrow, { + // SAFETY: + // - `self.world` has permission to access the required components. + // - The caller ensures that this operation will not result in any aliased mutable accesses. self.state .iter_many_unchecked_manual(entities, self.world, self.last_run, self.this_run) } @@ -672,8 +685,9 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { /// - [`iter`](Self::iter) for the iterator based alternative. #[inline] pub fn for_each<'this>(&'this self, f: impl FnMut(ROQueryItem<'this, Q>)) { - // SAFETY: system runs without conflicts with other systems. - // same-system queries have runtime borrow checks when they conflict + // SAFETY: + // - `self.world` has permission to access the required components. + // - The query is read-only, so it can be aliased even if it was originaly mutable. unsafe { self.state.as_readonly().for_each_unchecked_manual( self.world, @@ -710,8 +724,7 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { /// - [`iter_mut`](Self::iter_mut) for the iterator based alternative. #[inline] pub fn for_each_mut<'a>(&'a mut self, f: impl FnMut(Q::Item<'a>)) { - // SAFETY: system runs without conflicts with other systems. same-system queries have runtime - // borrow checks when they conflict + // SAFETY: `self.world` has permission to access the required components. unsafe { self.state .for_each_unchecked_manual(self.world, f, self.last_run, self.this_run); @@ -723,6 +736,7 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { /// This can only be called for read-only queries, see [`par_iter_mut`] for write-queries. /// /// [`par_iter_mut`]: Self::par_iter_mut + /// [`World`]: crate::world::World #[inline] pub fn par_iter(&self) -> QueryParIter<'_, '_, Q::ReadOnly, F::ReadOnly> { QueryParIter { @@ -739,6 +753,7 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { /// This can only be called for mutable queries, see [`par_iter`] for read-only-queries. /// /// [`par_iter`]: Self::par_iter + /// [`World`]: crate::world::World #[inline] pub fn par_iter_mut(&mut self) -> QueryParIter<'_, '_, Q, F> { QueryParIter { @@ -812,8 +827,12 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { ) -> Result<[ROQueryItem<'_, Q>; N], QueryEntityError> { // SAFETY: it is the scheduler's responsibility to ensure that `Query` is never handed out on the wrong `World`. unsafe { - self.state - .get_many_read_only_manual(self.world, entities, self.last_run, self.this_run) + self.state.get_many_read_only_manual( + self.world.unsafe_world(), + entities, + self.last_run, + self.this_run, + ) } } @@ -1044,9 +1063,9 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { .archetype_component_access .has_read(archetype_component) { - entity_ref - .get::() - .ok_or(QueryComponentError::MissingComponent) + // SAFETY: `self.world` must have access to the component `T` for this entity, + // since it was registered in `self.state`'s archetype component access set. + unsafe { entity_ref.get::() }.ok_or(QueryComponentError::MissingComponent) } else { Err(QueryComponentError::MissingReadAccess) } @@ -1112,7 +1131,6 @@ 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 @@ -1301,8 +1319,12 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> { /// ``` #[inline] pub fn is_empty(&self) -> bool { - self.state - .is_empty(self.world, self.last_run, self.this_run) + self.state.is_empty( + // SAFETY: `QueryState::is_empty` does not access world data. + unsafe { self.world.unsafe_world() }, + self.last_run, + self.this_run, + ) } /// Returns `true` if the given [`Entity`] matches the query. diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 023657bcd9e7f..4a3be817b8dbb 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -198,16 +198,10 @@ unsafe impl SystemPara world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Self::Item<'w, 's> { - Query::new( - // SAFETY: We have registered all of the query's world accesses, - // so the caller ensures that `world` has permission to access any - // world data that the query needs. - world.unsafe_world(), - state, - system_meta.last_run, - change_tick, - false, - ) + // SAFETY: We have registered all of the query's world accesses, + // so the caller ensures that `world` has permission to access any + // world data that the query needs. + Query::new(world, state, system_meta.last_run, change_tick, false) } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 1e4598e1d72c0..0a2b4011ab3de 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -122,14 +122,6 @@ impl World { UnsafeWorldCell::new_readonly(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. - #[inline] - pub(crate) fn as_unsafe_world_cell_migration_internal(&self) -> UnsafeWorldCell<'_> { - UnsafeWorldCell::new_readonly(self) - } - /// Retrieves this world's [Entities] collection #[inline] pub fn entities(&self) -> &Entities { diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index 44cb91d6953f9..263974cbd9326 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -443,7 +443,9 @@ impl<'w> UnsafeWorldCell<'w> { }; Some(MutUntyped { - // SAFETY: This function has exclusive access to the world so nothing aliases `ptr`. + // SAFETY: + // - caller ensures that `self` has permission to access the resource + // - caller ensures that the resource is unaliased value: unsafe { ptr.assert_unique() }, ticks, })