Skip to content

Commit 794bf6a

Browse files
authored
Move implementations of Query methods from QueryState to Query. (#17822)
# Objective Simplify the API surface by removing duplicated functionality between `Query` and `QueryState`. Reduce the amount of `unsafe` code required in `QueryState`. This is a follow-up to #15858. ## Solution Move implementations of `Query` methods from `QueryState` to `Query`. Instead of the original methods being on `QueryState`, with `Query` methods calling them by passing the individual parameters, the original methods are now on `Query`, with `QueryState` methods calling them by constructing a `Query`. This also adds two `_inner` methods that were missed in #15858: `iter_many_unique_inner` and `single_inner`. One goal here is to be able to deprecate and eventually remove many of the methods on `QueryState`, reducing the overall API surface. (I expected to do that in this PR, but this change was large enough on its own!) Now that the `QueryState` methods each consist of a simple expression like `self.query(world).get_inner(entity)`, a future PR can deprecate some or all of them with simple migration instructions. The other goal is to reduce the amount of `unsafe` code. The current implementation of a read-only method like `QueryState::get` directly calls the `unsafe fn get_unchecked_manual` and needs to repeat the proof that `&World` has enough access. With this change, `QueryState::get` is entirely safe code, with the proof that `&World` has enough access done by the `query()` method and shared across all read-only operations. ## Future Work The next step will be to mark the `QueryState` methods as `#[deprecated]` and migrate callers to the methods on `Query`.
1 parent 0a32450 commit 794bf6a

File tree

6 files changed

+481
-691
lines changed

6 files changed

+481
-691
lines changed

crates/bevy_ecs/src/query/error.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ pub enum QueryEntityError<'w> {
1717
NoSuchEntity(Entity, EntityDoesNotExistDetails),
1818
/// The [`Entity`] was requested mutably more than once.
1919
///
20-
/// See [`QueryState::get_many_mut`](crate::query::QueryState::get_many_mut) for an example.
20+
/// See [`Query::get_many_mut`](crate::system::Query::get_many_mut) for an example.
2121
AliasedMutability(Entity),
2222
}
2323

crates/bevy_ecs/src/query/iter.rs

+2
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> {
849849
// SAFETY:
850850
// `self.world` has permission to access the required components.
851851
// The original query iter has not been iterated on, so no items are aliased from it.
852+
// `QueryIter::new` ensures `world` is the same one used to initialize `query_state`.
852853
let query_lens = unsafe { query_lens_state.query_unchecked_manual(world) }.into_iter();
853854
let mut keyed_query: Vec<_> = query_lens
854855
.map(|(key, entity)| (key, NeutralOrd(entity)))
@@ -1683,6 +1684,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator<Item: EntityBorrow>>
16831684
// SAFETY:
16841685
// `self.world` has permission to access the required components.
16851686
// The original query iter has not been iterated on, so no items are aliased from it.
1687+
// `QueryIter::new` ensures `world` is the same one used to initialize `query_state`.
16861688
let query_lens = unsafe { query_lens_state.query_unchecked_manual(world) }
16871689
.iter_many_inner(self.entity_iter);
16881690
let mut keyed_query: Vec<_> = query_lens

crates/bevy_ecs/src/query/par_iter.rs

+12-26
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryParIter<'w, 's, D, F> {
9494
// at the same time.
9595
unsafe {
9696
self.state
97-
.iter_unchecked_manual(self.world, self.last_run, self.this_run)
97+
.query_unchecked_manual_with_ticks(self.world, self.last_run, self.this_run)
98+
.into_iter()
9899
.fold(init, func);
99100
}
100101
}
@@ -106,7 +107,8 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryParIter<'w, 's, D, F> {
106107
// SAFETY: See the safety comment above.
107108
unsafe {
108109
self.state
109-
.iter_unchecked_manual(self.world, self.last_run, self.this_run)
110+
.query_unchecked_manual_with_ticks(self.world, self.last_run, self.this_run)
111+
.into_iter()
110112
.fold(init, func);
111113
}
112114
} else {
@@ -262,12 +264,8 @@ impl<'w, 's, D: ReadOnlyQueryData, F: QueryFilter, E: EntityBorrow + Sync>
262264
// at the same time.
263265
unsafe {
264266
self.state
265-
.iter_many_unchecked_manual(
266-
&self.entity_list,
267-
self.world,
268-
self.last_run,
269-
self.this_run,
270-
)
267+
.query_unchecked_manual_with_ticks(self.world, self.last_run, self.this_run)
268+
.iter_many_inner(&self.entity_list)
271269
.fold(init, func);
272270
}
273271
}
@@ -279,12 +277,8 @@ impl<'w, 's, D: ReadOnlyQueryData, F: QueryFilter, E: EntityBorrow + Sync>
279277
// SAFETY: See the safety comment above.
280278
unsafe {
281279
self.state
282-
.iter_many_unchecked_manual(
283-
&self.entity_list,
284-
self.world,
285-
self.last_run,
286-
self.this_run,
287-
)
280+
.query_unchecked_manual_with_ticks(self.world, self.last_run, self.this_run)
281+
.iter_many_inner(&self.entity_list)
288282
.fold(init, func);
289283
}
290284
} else {
@@ -430,12 +424,8 @@ impl<'w, 's, D: QueryData, F: QueryFilter, E: TrustedEntityBorrow + Sync>
430424
// at the same time.
431425
unsafe {
432426
self.state
433-
.iter_many_unique_unchecked_manual(
434-
self.entity_list,
435-
self.world,
436-
self.last_run,
437-
self.this_run,
438-
)
427+
.query_unchecked_manual_with_ticks(self.world, self.last_run, self.this_run)
428+
.iter_many_unique_inner(self.entity_list)
439429
.fold(init, func);
440430
}
441431
}
@@ -447,12 +437,8 @@ impl<'w, 's, D: QueryData, F: QueryFilter, E: TrustedEntityBorrow + Sync>
447437
// SAFETY: See the safety comment above.
448438
unsafe {
449439
self.state
450-
.iter_many_unique_unchecked_manual(
451-
self.entity_list,
452-
self.world,
453-
self.last_run,
454-
self.this_run,
455-
)
440+
.query_unchecked_manual_with_ticks(self.world, self.last_run, self.this_run)
441+
.iter_many_unique_inner(self.entity_list)
456442
.fold(init, func);
457443
}
458444
} else {

0 commit comments

Comments
 (0)