Make QueryState::transmute&co validate the world of the &Components used (#14631)
# Objective - Fix #14629 ## Solution - Make `QueryState::transmute`, `QueryState::transmute_filtered`, `QueryState::join` and `QueryState::join_filtered` take a `impl Into<UnsafeWorldCell>` instead of a `&Components` and validate their `WorldId` ## Migration Guide - `QueryState::transmute`, `QueryState::transmute_filtered`, `QueryState::join` and `QueryState::join_filtered` now take a `impl Into<UnsafeWorldCell>` instead of a `&Components` --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
This commit is contained in:
Giacomo Stevanato
GitHub
parent
fc2f564c6f
commit
68ec6f4f50
@ -375,8 +375,7 @@ fn observer_system_runner<E: Event, B: Bundle>(
|
||||
};
|
||||
|
||||
// TODO: Move this check into the observer cache to avoid dynamic dispatch
|
||||
// SAFETY: We only access world metadata
|
||||
let last_trigger = unsafe { world.world_metadata() }.last_trigger_id();
|
||||
let last_trigger = world.last_trigger_id();
|
||||
if state.last_trigger_id == last_trigger {
|
||||
return;
|
||||
}
|
||||
|
||||
@ -362,9 +362,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> {
|
||||
|
||||
let world = self.world;
|
||||
|
||||
let query_lens_state = self
|
||||
.query_state
|
||||
.transmute_filtered::<(L, Entity), F>(world.components());
|
||||
let query_lens_state = self.query_state.transmute_filtered::<(L, Entity), F>(world);
|
||||
|
||||
// SAFETY:
|
||||
// `self.world` has permission to access the required components.
|
||||
@ -456,9 +454,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> {
|
||||
|
||||
let world = self.world;
|
||||
|
||||
let query_lens_state = self
|
||||
.query_state
|
||||
.transmute_filtered::<(L, Entity), F>(world.components());
|
||||
let query_lens_state = self.query_state.transmute_filtered::<(L, Entity), F>(world);
|
||||
|
||||
// SAFETY:
|
||||
// `self.world` has permission to access the required components.
|
||||
@ -558,9 +554,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> {
|
||||
|
||||
let world = self.world;
|
||||
|
||||
let query_lens_state = self
|
||||
.query_state
|
||||
.transmute_filtered::<(L, Entity), F>(world.components());
|
||||
let query_lens_state = self.query_state.transmute_filtered::<(L, Entity), F>(world);
|
||||
|
||||
// SAFETY:
|
||||
// `self.world` has permission to access the required components.
|
||||
@ -626,9 +620,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> {
|
||||
|
||||
let world = self.world;
|
||||
|
||||
let query_lens_state = self
|
||||
.query_state
|
||||
.transmute_filtered::<(L, Entity), F>(world.components());
|
||||
let query_lens_state = self.query_state.transmute_filtered::<(L, Entity), F>(world);
|
||||
|
||||
// SAFETY:
|
||||
// `self.world` has permission to access the required components.
|
||||
@ -757,9 +749,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> {
|
||||
|
||||
let world = self.world;
|
||||
|
||||
let query_lens_state = self
|
||||
.query_state
|
||||
.transmute_filtered::<(L, Entity), F>(world.components());
|
||||
let query_lens_state = self.query_state.transmute_filtered::<(L, Entity), F>(world);
|
||||
|
||||
// SAFETY:
|
||||
// `self.world` has permission to access the required components.
|
||||
@ -828,9 +818,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> {
|
||||
|
||||
let world = self.world;
|
||||
|
||||
let query_lens_state = self
|
||||
.query_state
|
||||
.transmute_filtered::<(L, Entity), F>(world.components());
|
||||
let query_lens_state = self.query_state.transmute_filtered::<(L, Entity), F>(world);
|
||||
|
||||
// SAFETY:
|
||||
// `self.world` has permission to access the required components.
|
||||
@ -900,9 +888,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> {
|
||||
|
||||
let world = self.world;
|
||||
|
||||
let query_lens_state = self
|
||||
.query_state
|
||||
.transmute_filtered::<(L, Entity), F>(world.components());
|
||||
let query_lens_state = self.query_state.transmute_filtered::<(L, Entity), F>(world);
|
||||
|
||||
// SAFETY:
|
||||
// `self.world` has permission to access the required components.
|
||||
|
||||
@ -1,7 +1,7 @@
|
||||
use crate::{
|
||||
archetype::{Archetype, ArchetypeComponentId, ArchetypeGeneration, ArchetypeId},
|
||||
batching::BatchingStrategy,
|
||||
component::{ComponentId, Components, Tick},
|
||||
component::{ComponentId, Tick},
|
||||
entity::Entity,
|
||||
prelude::FromWorld,
|
||||
query::{
|
||||
@ -476,21 +476,27 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
|
||||
/// You might end up with a mix of archetypes that only matched the original query + archetypes that only match
|
||||
/// the new [`QueryState`]. Most of the safe methods on [`QueryState`] call [`QueryState::update_archetypes`] internally, so this
|
||||
/// best used through a [`Query`](crate::system::Query).
|
||||
pub fn transmute<NewD: QueryData>(&self, components: &Components) -> QueryState<NewD> {
|
||||
self.transmute_filtered::<NewD, ()>(components)
|
||||
pub fn transmute<'a, NewD: QueryData>(
|
||||
&self,
|
||||
world: impl Into<UnsafeWorldCell<'a>>,
|
||||
) -> QueryState<NewD> {
|
||||
self.transmute_filtered::<NewD, ()>(world.into())
|
||||
}
|
||||
|
||||
/// Creates a new [`QueryState`] with the same underlying [`FilteredAccess`], matched tables and archetypes
|
||||
/// as self but with a new type signature.
|
||||
///
|
||||
/// Panics if `NewD` or `NewF` require accesses that this query does not have.
|
||||
pub fn transmute_filtered<NewD: QueryData, NewF: QueryFilter>(
|
||||
pub fn transmute_filtered<'a, NewD: QueryData, NewF: QueryFilter>(
|
||||
&self,
|
||||
components: &Components,
|
||||
world: impl Into<UnsafeWorldCell<'a>>,
|
||||
) -> QueryState<NewD, NewF> {
|
||||
let world = world.into();
|
||||
self.validate_world(world.id());
|
||||
|
||||
let mut component_access = FilteredAccess::default();
|
||||
let mut fetch_state = NewD::get_state(components).expect("Could not create fetch_state, Please initialize all referenced components before transmuting.");
|
||||
let filter_state = NewF::get_state(components).expect("Could not create filter_state, Please initialize all referenced components before transmuting.");
|
||||
let mut fetch_state = NewD::get_state(world.components()).expect("Could not create fetch_state, Please initialize all referenced components before transmuting.");
|
||||
let filter_state = NewF::get_state(world.components()).expect("Could not create filter_state, Please initialize all referenced components before transmuting.");
|
||||
|
||||
NewD::set_access(&mut fetch_state, &self.component_access);
|
||||
NewD::update_component_access(&fetch_state, &mut component_access);
|
||||
@ -542,12 +548,12 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
|
||||
/// ## Panics
|
||||
///
|
||||
/// Will panic if `NewD` contains accesses not in `Q` or `OtherQ`.
|
||||
pub fn join<OtherD: QueryData, NewD: QueryData>(
|
||||
pub fn join<'a, OtherD: QueryData, NewD: QueryData>(
|
||||
&self,
|
||||
components: &Components,
|
||||
world: impl Into<UnsafeWorldCell<'a>>,
|
||||
other: &QueryState<OtherD>,
|
||||
) -> QueryState<NewD, ()> {
|
||||
self.join_filtered::<_, (), NewD, ()>(components, other)
|
||||
self.join_filtered::<_, (), NewD, ()>(world, other)
|
||||
}
|
||||
|
||||
/// Use this to combine two queries. The data accessed will be the intersection
|
||||
@ -557,23 +563,28 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
|
||||
///
|
||||
/// Will panic if `NewD` or `NewF` requires accesses not in `Q` or `OtherQ`.
|
||||
pub fn join_filtered<
|
||||
'a,
|
||||
OtherD: QueryData,
|
||||
OtherF: QueryFilter,
|
||||
NewD: QueryData,
|
||||
NewF: QueryFilter,
|
||||
>(
|
||||
&self,
|
||||
components: &Components,
|
||||
world: impl Into<UnsafeWorldCell<'a>>,
|
||||
other: &QueryState<OtherD, OtherF>,
|
||||
) -> QueryState<NewD, NewF> {
|
||||
if self.world_id != other.world_id {
|
||||
panic!("Joining queries initialized on different worlds is not allowed.");
|
||||
}
|
||||
|
||||
let world = world.into();
|
||||
|
||||
self.validate_world(world.id());
|
||||
|
||||
let mut component_access = FilteredAccess::default();
|
||||
let mut new_fetch_state = NewD::get_state(components)
|
||||
let mut new_fetch_state = NewD::get_state(world.components())
|
||||
.expect("Could not create fetch_state, Please initialize all referenced components before transmuting.");
|
||||
let new_filter_state = NewF::get_state(components)
|
||||
let new_filter_state = NewF::get_state(world.components())
|
||||
.expect("Could not create filter_state, Please initialize all referenced components before transmuting.");
|
||||
|
||||
NewD::set_access(&mut new_fetch_state, &self.component_access);
|
||||
@ -1779,7 +1790,7 @@ mod tests {
|
||||
world.spawn((A(1), B(0)));
|
||||
|
||||
let query_state = world.query::<(&A, &B)>();
|
||||
let mut new_query_state = query_state.transmute::<&A>(world.components());
|
||||
let mut new_query_state = query_state.transmute::<&A>(&world);
|
||||
assert_eq!(new_query_state.iter(&world).len(), 1);
|
||||
let a = new_query_state.single(&world);
|
||||
|
||||
@ -1793,7 +1804,7 @@ mod tests {
|
||||
world.spawn((A(1), B(0), C(0)));
|
||||
|
||||
let query_state = world.query_filtered::<(&A, &B), Without<C>>();
|
||||
let mut new_query_state = query_state.transmute::<&A>(world.components());
|
||||
let mut new_query_state = query_state.transmute::<&A>(&world);
|
||||
// even though we change the query to not have Without<C>, we do not get the component with C.
|
||||
let a = new_query_state.single(&world);
|
||||
|
||||
@ -1807,7 +1818,7 @@ mod tests {
|
||||
let entity = world.spawn(A(10)).id();
|
||||
|
||||
let q = world.query::<()>();
|
||||
let mut q = q.transmute::<Entity>(world.components());
|
||||
let mut q = q.transmute::<Entity>(&world);
|
||||
assert_eq!(q.single(&world), entity);
|
||||
}
|
||||
|
||||
@ -1817,11 +1828,11 @@ mod tests {
|
||||
world.spawn(A(10));
|
||||
|
||||
let q = world.query::<&A>();
|
||||
let mut new_q = q.transmute::<Ref<A>>(world.components());
|
||||
let mut new_q = q.transmute::<Ref<A>>(&world);
|
||||
assert!(new_q.single(&world).is_added());
|
||||
|
||||
let q = world.query::<Ref<A>>();
|
||||
let _ = q.transmute::<&A>(world.components());
|
||||
let _ = q.transmute::<&A>(&world);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@ -1830,8 +1841,8 @@ mod tests {
|
||||
world.spawn(A(0));
|
||||
|
||||
let q = world.query::<&mut A>();
|
||||
let _ = q.transmute::<Ref<A>>(world.components());
|
||||
let _ = q.transmute::<&A>(world.components());
|
||||
let _ = q.transmute::<Ref<A>>(&world);
|
||||
let _ = q.transmute::<&A>(&world);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@ -1840,7 +1851,7 @@ mod tests {
|
||||
world.spawn(A(0));
|
||||
|
||||
let q: QueryState<EntityMut<'_>> = world.query::<EntityMut>();
|
||||
let _ = q.transmute::<EntityRef>(world.components());
|
||||
let _ = q.transmute::<EntityRef>(&world);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@ -1849,8 +1860,8 @@ mod tests {
|
||||
world.spawn((A(0), B(0)));
|
||||
|
||||
let query_state = world.query::<(Option<&A>, &B)>();
|
||||
let _ = query_state.transmute::<Option<&A>>(world.components());
|
||||
let _ = query_state.transmute::<&B>(world.components());
|
||||
let _ = query_state.transmute::<Option<&A>>(&world);
|
||||
let _ = query_state.transmute::<&B>(&world);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@ -1864,7 +1875,7 @@ mod tests {
|
||||
world.spawn(A(0));
|
||||
|
||||
let query_state = world.query::<&A>();
|
||||
let mut _new_query_state = query_state.transmute::<(&A, &B)>(world.components());
|
||||
let mut _new_query_state = query_state.transmute::<(&A, &B)>(&world);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@ -1876,7 +1887,7 @@ mod tests {
|
||||
world.spawn(A(0));
|
||||
|
||||
let query_state = world.query::<&A>();
|
||||
let mut _new_query_state = query_state.transmute::<&mut A>(world.components());
|
||||
let mut _new_query_state = query_state.transmute::<&mut A>(&world);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@ -1888,7 +1899,7 @@ mod tests {
|
||||
world.spawn(C(0));
|
||||
|
||||
let query_state = world.query::<Option<&A>>();
|
||||
let mut new_query_state = query_state.transmute::<&A>(world.components());
|
||||
let mut new_query_state = query_state.transmute::<&A>(&world);
|
||||
let x = new_query_state.single(&world);
|
||||
assert_eq!(x.0, 1234);
|
||||
}
|
||||
@ -1902,15 +1913,15 @@ mod tests {
|
||||
world.init_component::<A>();
|
||||
|
||||
let q = world.query::<EntityRef>();
|
||||
let _ = q.transmute::<&A>(world.components());
|
||||
let _ = q.transmute::<&A>(&world);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn can_transmute_filtered_entity() {
|
||||
let mut world = World::new();
|
||||
let entity = world.spawn((A(0), B(1))).id();
|
||||
let query = QueryState::<(Entity, &A, &B)>::new(&mut world)
|
||||
.transmute::<FilteredEntityRef>(world.components());
|
||||
let query =
|
||||
QueryState::<(Entity, &A, &B)>::new(&mut world).transmute::<FilteredEntityRef>(&world);
|
||||
|
||||
let mut query = query;
|
||||
// Our result is completely untyped
|
||||
@ -1927,7 +1938,7 @@ mod tests {
|
||||
let entity_a = world.spawn(A(0)).id();
|
||||
|
||||
let mut query = QueryState::<(Entity, &A, Has<B>)>::new(&mut world)
|
||||
.transmute_filtered::<(Entity, Has<B>), Added<A>>(world.components());
|
||||
.transmute_filtered::<(Entity, Has<B>), Added<A>>(&world);
|
||||
|
||||
assert_eq!((entity_a, false), query.single(&world));
|
||||
|
||||
@ -1947,7 +1958,7 @@ mod tests {
|
||||
let entity_a = world.spawn(A(0)).id();
|
||||
|
||||
let mut detection_query = QueryState::<(Entity, &A)>::new(&mut world)
|
||||
.transmute_filtered::<Entity, Changed<A>>(world.components());
|
||||
.transmute_filtered::<Entity, Changed<A>>(&world);
|
||||
|
||||
let mut change_query = QueryState::<&mut A>::new(&mut world);
|
||||
assert_eq!(entity_a, detection_query.single(&world));
|
||||
@ -1970,7 +1981,20 @@ mod tests {
|
||||
world.init_component::<A>();
|
||||
world.init_component::<B>();
|
||||
let query = QueryState::<&A>::new(&mut world);
|
||||
let _new_query = query.transmute_filtered::<Entity, Changed<B>>(world.components());
|
||||
let _new_query = query.transmute_filtered::<Entity, Changed<B>>(&world);
|
||||
}
|
||||
|
||||
// Regression test for #14629
|
||||
#[test]
|
||||
#[should_panic]
|
||||
fn transmute_with_different_world() {
|
||||
let mut world = World::new();
|
||||
world.spawn((A(1), B(2)));
|
||||
|
||||
let mut world2 = World::new();
|
||||
world2.init_component::<B>();
|
||||
|
||||
world.query::<(&A, &B)>().transmute::<&B>(&world2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@ -1983,8 +2007,7 @@ mod tests {
|
||||
|
||||
let query_1 = QueryState::<&A, Without<C>>::new(&mut world);
|
||||
let query_2 = QueryState::<&B, Without<C>>::new(&mut world);
|
||||
let mut new_query: QueryState<Entity, ()> =
|
||||
query_1.join_filtered(world.components(), &query_2);
|
||||
let mut new_query: QueryState<Entity, ()> = query_1.join_filtered(&world, &query_2);
|
||||
|
||||
assert_eq!(new_query.single(&world), entity_ab);
|
||||
}
|
||||
@ -1999,8 +2022,7 @@ mod tests {
|
||||
|
||||
let query_1 = QueryState::<&A>::new(&mut world);
|
||||
let query_2 = QueryState::<&B, Without<C>>::new(&mut world);
|
||||
let mut new_query: QueryState<Entity, ()> =
|
||||
query_1.join_filtered(world.components(), &query_2);
|
||||
let mut new_query: QueryState<Entity, ()> = query_1.join_filtered(&world, &query_2);
|
||||
|
||||
assert!(new_query.get(&world, entity_ab).is_ok());
|
||||
// should not be able to get entity with c.
|
||||
@ -2016,7 +2038,7 @@ mod tests {
|
||||
world.init_component::<C>();
|
||||
let query_1 = QueryState::<&A>::new(&mut world);
|
||||
let query_2 = QueryState::<&B>::new(&mut world);
|
||||
let _query: QueryState<&C> = query_1.join(world.components(), &query_2);
|
||||
let _query: QueryState<&C> = query_1.join(&world, &query_2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@ -2030,6 +2052,6 @@ mod tests {
|
||||
let mut world = World::new();
|
||||
let query_1 = QueryState::<&A, Without<C>>::new(&mut world);
|
||||
let query_2 = QueryState::<&B, Without<C>>::new(&mut world);
|
||||
let _: QueryState<Entity, Changed<C>> = query_1.join_filtered(world.components(), &query_2);
|
||||
let _: QueryState<Entity, Changed<C>> = query_1.join_filtered(&world, &query_2);
|
||||
}
|
||||
}
|
||||
|
||||
@ -1368,8 +1368,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> {
|
||||
pub fn transmute_lens_filtered<NewD: QueryData, NewF: QueryFilter>(
|
||||
&mut self,
|
||||
) -> QueryLens<'_, NewD, NewF> {
|
||||
let components = self.world.components();
|
||||
let state = self.state.transmute_filtered::<NewD, NewF>(components);
|
||||
let state = self.state.transmute_filtered::<NewD, NewF>(self.world);
|
||||
QueryLens {
|
||||
world: self.world,
|
||||
state,
|
||||
@ -1460,10 +1459,9 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> {
|
||||
&mut self,
|
||||
other: &mut Query<OtherD, OtherF>,
|
||||
) -> QueryLens<'_, NewD, NewF> {
|
||||
let components = self.world.components();
|
||||
let state = self
|
||||
.state
|
||||
.join_filtered::<OtherD, OtherF, NewD, NewF>(components, other.state);
|
||||
.join_filtered::<OtherD, OtherF, NewD, NewF>(self.world, other.state);
|
||||
QueryLens {
|
||||
world: self.world,
|
||||
state,
|
||||
|
||||
@ -83,6 +83,18 @@ unsafe impl Send for UnsafeWorldCell<'_> {}
|
||||
// SAFETY: `&World` and `&mut World` are both `Sync`
|
||||
unsafe impl Sync for UnsafeWorldCell<'_> {}
|
||||
|
||||
impl<'w> From<&'w mut World> for UnsafeWorldCell<'w> {
|
||||
fn from(value: &'w mut World) -> Self {
|
||||
value.as_unsafe_world_cell()
|
||||
}
|
||||
}
|
||||
|
||||
impl<'w> From<&'w World> for UnsafeWorldCell<'w> {
|
||||
fn from(value: &'w World) -> Self {
|
||||
value.as_unsafe_world_cell_readonly()
|
||||
}
|
||||
}
|
||||
|
||||
impl<'w> UnsafeWorldCell<'w> {
|
||||
/// Creates a [`UnsafeWorldCell`] that can be used to access everything immutably
|
||||
#[inline]
|
||||
@ -257,6 +269,15 @@ impl<'w> UnsafeWorldCell<'w> {
|
||||
unsafe { self.world_metadata() }.read_change_tick()
|
||||
}
|
||||
|
||||
/// Returns the id of the last ECS event that was fired.
|
||||
/// Used internally to ensure observers don't trigger multiple times for the same event.
|
||||
#[inline]
|
||||
pub fn last_trigger_id(&self) -> u32 {
|
||||
// SAFETY:
|
||||
// - we only access world metadata
|
||||
unsafe { self.world_metadata() }.last_trigger_id()
|
||||
}
|
||||
|
||||
/// Returns the [`Tick`] indicating the last time that [`World::clear_trackers`] was called.
|
||||
///
|
||||
/// If this `UnsafeWorldCell` was created from inside of an exclusive system (a [`System`] that
|
||||
|
||||
Loading…
Reference in New Issue
Block a user