Skip to content

Commit d4babaf

Browse files
committed
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
1 parent 3600c5a commit d4babaf

File tree

3 files changed

+25
-16
lines changed

3 files changed

+25
-16
lines changed

crates/bevy_ecs/src/system/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1210,7 +1210,7 @@ mod tests {
12101210
let world2 = World::new();
12111211
let qstate = world1.query::<()>();
12121212
// SAFETY: doesnt access anything
1213-
let query = unsafe { Query::new(&world2, &qstate, 0, 0) };
1213+
let query = unsafe { Query::new(&world2, &qstate, 0, 0, false) };
12141214
query.iter();
12151215
}
12161216
}

crates/bevy_ecs/src/system/query.rs

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -274,16 +274,16 @@ use std::{any::TypeId, borrow::Borrow, fmt::Debug};
274274
/// [`With`]: crate::query::With
275275
/// [`Without`]: crate::query::Without
276276
pub struct Query<'world, 'state, Q: WorldQuery, F: ReadOnlyWorldQuery = ()> {
277-
pub(crate) world: &'world World,
278-
pub(crate) state: &'state QueryState<Q, F>,
279-
pub(crate) last_change_tick: u32,
280-
pub(crate) change_tick: u32,
277+
world: &'world World,
278+
state: &'state QueryState<Q, F>,
279+
last_change_tick: u32,
280+
change_tick: u32,
281281
// SAFETY: This is used to ensure that `get_component_mut::<C>` properly fails when a Query writes C
282282
// and gets converted to a read-only query using `to_readonly`. Without checking this, `get_component_mut` relies on
283283
// QueryState's archetype_component_access, which will continue allowing write access to C after being cast to
284284
// the read-only variant. This whole situation is confusing and error prone. Ideally this is a temporary hack
285285
// until we sort out a cleaner alternative.
286-
pub(crate) force_read_only_component_access: bool,
286+
force_read_only_component_access: bool,
287287
}
288288

289289
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> {
309309
state: &'s QueryState<Q, F>,
310310
last_change_tick: u32,
311311
change_tick: u32,
312+
force_read_only_component_access: bool,
312313
) -> Self {
313314
state.validate_world(world);
314315

315316
Self {
316-
force_read_only_component_access: false,
317+
force_read_only_component_access,
317318
world,
318319
state,
319320
last_change_tick,
@@ -329,14 +330,16 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> {
329330
pub fn to_readonly(&self) -> Query<'_, 's, Q::ReadOnly, F::ReadOnly> {
330331
let new_state = self.state.as_readonly();
331332
// SAFETY: This is memory safe because it turns the query immutable.
332-
Query {
333-
// SAFETY: this must be set to true or `get_component_mut` will be unsound. See the comments
334-
// on this field for more details
335-
force_read_only_component_access: true,
336-
world: self.world,
337-
state: new_state,
338-
last_change_tick: self.last_change_tick,
339-
change_tick: self.change_tick,
333+
unsafe {
334+
Query::new(
335+
self.world,
336+
new_state,
337+
self.last_change_tick,
338+
self.change_tick,
339+
// SAFETY: this must be set to true or `get_component_mut` will be unsound. See the comments
340+
// on this field for more details
341+
true,
342+
)
340343
}
341344
}
342345

crates/bevy_ecs/src/system/system_param.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,13 @@ unsafe impl<Q: WorldQuery + 'static, F: ReadOnlyWorldQuery + 'static> SystemPara
223223
world: &'w World,
224224
change_tick: u32,
225225
) -> Self::Item<'w, 's> {
226-
Query::new(world, state, system_meta.last_change_tick, change_tick)
226+
Query::new(
227+
world,
228+
state,
229+
system_meta.last_change_tick,
230+
change_tick,
231+
false,
232+
)
227233
}
228234
}
229235

0 commit comments

Comments
 (0)