Make Query fields private (#7149)
`Query`'s fields being `pub(crate)` means that the struct can be constructed via safe code from anywhere in `bevy_ecs` . This is Not Good since it is intended that all construction of this type goes through `Query::new` which is an `unsafe fn` letting various `Query` methods rely on those invariants holding even though they can be trivially bypassed. This has no user facing impact
This commit is contained in:
parent
3600c5a340
commit
d4babafe81
@ -1210,7 +1210,7 @@ mod tests {
|
|||||||
let world2 = World::new();
|
let world2 = World::new();
|
||||||
let qstate = world1.query::<()>();
|
let qstate = world1.query::<()>();
|
||||||
// SAFETY: doesnt access anything
|
// SAFETY: doesnt access anything
|
||||||
let query = unsafe { Query::new(&world2, &qstate, 0, 0) };
|
let query = unsafe { Query::new(&world2, &qstate, 0, 0, false) };
|
||||||
query.iter();
|
query.iter();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -274,16 +274,16 @@ use std::{any::TypeId, borrow::Borrow, fmt::Debug};
|
|||||||
/// [`With`]: crate::query::With
|
/// [`With`]: crate::query::With
|
||||||
/// [`Without`]: crate::query::Without
|
/// [`Without`]: crate::query::Without
|
||||||
pub struct Query<'world, 'state, Q: WorldQuery, F: ReadOnlyWorldQuery = ()> {
|
pub struct Query<'world, 'state, Q: WorldQuery, F: ReadOnlyWorldQuery = ()> {
|
||||||
pub(crate) world: &'world World,
|
world: &'world World,
|
||||||
pub(crate) state: &'state QueryState<Q, F>,
|
state: &'state QueryState<Q, F>,
|
||||||
pub(crate) last_change_tick: u32,
|
last_change_tick: u32,
|
||||||
pub(crate) change_tick: u32,
|
change_tick: u32,
|
||||||
// SAFETY: This is used to ensure that `get_component_mut::<C>` properly fails when a Query writes C
|
// SAFETY: This is used to ensure that `get_component_mut::<C>` properly fails when a Query writes C
|
||||||
// and gets converted to a read-only query using `to_readonly`. Without checking this, `get_component_mut` relies on
|
// and gets converted to a read-only query using `to_readonly`. Without checking this, `get_component_mut` relies on
|
||||||
// QueryState's archetype_component_access, which will continue allowing write access to C after being cast to
|
// QueryState's archetype_component_access, which will continue allowing write access to C after being cast to
|
||||||
// the read-only variant. This whole situation is confusing and error prone. Ideally this is a temporary hack
|
// the read-only variant. This whole situation is confusing and error prone. Ideally this is a temporary hack
|
||||||
// until we sort out a cleaner alternative.
|
// until we sort out a cleaner alternative.
|
||||||
pub(crate) force_read_only_component_access: bool,
|
force_read_only_component_access: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> std::fmt::Debug for Query<'w, 's, Q, F> {
|
impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> std::fmt::Debug for Query<'w, 's, Q, F> {
|
||||||
@ -309,11 +309,12 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> {
|
|||||||
state: &'s QueryState<Q, F>,
|
state: &'s QueryState<Q, F>,
|
||||||
last_change_tick: u32,
|
last_change_tick: u32,
|
||||||
change_tick: u32,
|
change_tick: u32,
|
||||||
|
force_read_only_component_access: bool,
|
||||||
) -> Self {
|
) -> Self {
|
||||||
state.validate_world(world);
|
state.validate_world(world);
|
||||||
|
|
||||||
Self {
|
Self {
|
||||||
force_read_only_component_access: false,
|
force_read_only_component_access,
|
||||||
world,
|
world,
|
||||||
state,
|
state,
|
||||||
last_change_tick,
|
last_change_tick,
|
||||||
@ -329,14 +330,16 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> {
|
|||||||
pub fn to_readonly(&self) -> Query<'_, 's, Q::ReadOnly, F::ReadOnly> {
|
pub fn to_readonly(&self) -> Query<'_, 's, Q::ReadOnly, F::ReadOnly> {
|
||||||
let new_state = self.state.as_readonly();
|
let new_state = self.state.as_readonly();
|
||||||
// SAFETY: This is memory safe because it turns the query immutable.
|
// SAFETY: This is memory safe because it turns the query immutable.
|
||||||
Query {
|
unsafe {
|
||||||
// SAFETY: this must be set to true or `get_component_mut` will be unsound. See the comments
|
Query::new(
|
||||||
// on this field for more details
|
self.world,
|
||||||
force_read_only_component_access: true,
|
new_state,
|
||||||
world: self.world,
|
self.last_change_tick,
|
||||||
state: new_state,
|
self.change_tick,
|
||||||
last_change_tick: self.last_change_tick,
|
// SAFETY: this must be set to true or `get_component_mut` will be unsound. See the comments
|
||||||
change_tick: self.change_tick,
|
// on this field for more details
|
||||||
|
true,
|
||||||
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -223,7 +223,13 @@ unsafe impl<Q: WorldQuery + 'static, F: ReadOnlyWorldQuery + 'static> SystemPara
|
|||||||
world: &'w World,
|
world: &'w World,
|
||||||
change_tick: u32,
|
change_tick: u32,
|
||||||
) -> Self::Item<'w, 's> {
|
) -> Self::Item<'w, 's> {
|
||||||
Query::new(world, state, system_meta.last_change_tick, change_tick)
|
Query::new(
|
||||||
|
world,
|
||||||
|
state,
|
||||||
|
system_meta.last_change_tick,
|
||||||
|
change_tick,
|
||||||
|
false,
|
||||||
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user