Skip to content

Commit 13de06f

Browse files
BoxyUwUItsDoot
authored andcommitted
remove QF generics from all Query/State methods and types (bevyengine#5170)
# Objective remove `QF` generics from a bunch of types and methods on query related items. this has a few benefits: - simplifies type signatures `fn iter(&self) -> QueryIter<'_, 's, Q::ReadOnly, F::ReadOnly>` is (imo) conceptually simpler than `fn iter(&self) -> QueryIter<'_, 's, Q, ROQueryFetch<'_, Q>, F>` - `Fetch` is mostly an implementation detail but previously we had to expose it on every `iter` `get` etc method - Allows us to potentially in the future simplify the `WorldQuery` trait hierarchy by removing the `Fetch` trait ## Solution remove the `QF` generic and add a way to (unsafely) turn `&QueryState<Q1, F1>` into `&QueryState<Q2, F2>` --- ## Changelog/Migration Guide The `QF` generic was removed from various `Query` iterator types and some methods, you should update your code to use the type of the corresponding worldquery of the fetch type that was being used, or call `as_readonly`/`as_nop` to convert a querystate to the appropriate type. For example: `.get_single_unchecked_manual::<ROQueryFetch<Q>>(..)` -> `.as_readonly().get_single_unchecked_manual(..)` `my_field: QueryIter<'w, 's, Q, ROQueryFetch<'w, Q>, F>` -> `my_field: QueryIter<'w, 's, Q::ReadOnly, F::ReadOnly>`
1 parent 06e2944 commit 13de06f

File tree

8 files changed

+273
-206
lines changed

8 files changed

+273
-206
lines changed

crates/bevy_ecs/src/query/fetch.rs

+25-5
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,8 @@ use std::{cell::UnsafeCell, marker::PhantomData};
318318
/// ```
319319
/// # Safety
320320
///
321-
/// component access of `ROQueryFetch<Self>` should be a subset of `QueryFetch<Self>`
321+
/// component access of `ROQueryFetch<Self>` must be a subset of `QueryFetch<Self>`
322+
/// and `ROQueryFetch<Self>` must match exactly the same archetypes/tables as `QueryFetch<Self>`
322323
pub unsafe trait WorldQuery: for<'w> WorldQueryGats<'w, _State = Self::State> {
323324
type ReadOnly: ReadOnlyWorldQuery<State = Self::State>;
324325
type State: FetchState;
@@ -1608,6 +1609,25 @@ macro_rules! impl_anytuple_fetch {
16081609
all_tuples!(impl_tuple_fetch, 0, 15, F, S);
16091610
all_tuples!(impl_anytuple_fetch, 0, 15, F, S);
16101611

1612+
/// [`WorldQuery`] used to nullify queries by turning `Query<Q>` into `Query<NopWorldQuery<Q>>`
1613+
///
1614+
/// This will rarely be useful to consumers of `bevy_ecs`.
1615+
pub struct NopWorldQuery<Q: WorldQuery>(PhantomData<Q>);
1616+
1617+
/// SAFETY: `Self::ReadOnly` is `Self`
1618+
unsafe impl<Q: WorldQuery> WorldQuery for NopWorldQuery<Q> {
1619+
type ReadOnly = Self;
1620+
type State = Q::State;
1621+
1622+
fn shrink<'wlong: 'wshort, 'wshort>(_: ()) {}
1623+
}
1624+
impl<'a, Q: WorldQuery> WorldQueryGats<'a> for NopWorldQuery<Q> {
1625+
type Fetch = NopFetch<QueryFetch<'a, Q>>;
1626+
type _State = <Q as WorldQueryGats<'a>>::_State;
1627+
}
1628+
/// SAFETY: `NopFetch` never accesses any data
1629+
unsafe impl<Q: WorldQuery> ReadOnlyWorldQuery for NopWorldQuery<Q> {}
1630+
16111631
/// [`Fetch`] that does not actually fetch anything
16121632
///
16131633
/// Mostly useful when something is generic over the Fetch and you don't want to fetch as you will discard the result
@@ -1616,18 +1636,18 @@ pub struct NopFetch<State> {
16161636
}
16171637

16181638
// SAFETY: NopFetch doesnt access anything
1619-
unsafe impl<'w, State: FetchState> Fetch<'w> for NopFetch<State> {
1639+
unsafe impl<'w, F: Fetch<'w>> Fetch<'w> for NopFetch<F> {
16201640
type Item = ();
1621-
type State = State;
1641+
type State = F::State;
16221642

1623-
const IS_DENSE: bool = true;
1643+
const IS_DENSE: bool = F::IS_DENSE;
16241644

16251645
const IS_ARCHETYPAL: bool = true;
16261646

16271647
#[inline(always)]
16281648
unsafe fn init(
16291649
_world: &'w World,
1630-
_state: &State,
1650+
_state: &F::State,
16311651
_last_change_tick: u32,
16321652
_change_tick: u32,
16331653
) -> Self {

crates/bevy_ecs/src/query/iter.rs

+32-54
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,14 @@ use super::{QueryFetch, QueryItem, ReadOnlyWorldQuery};
1313
///
1414
/// This struct is created by the [`Query::iter`](crate::system::Query::iter) and
1515
/// [`Query::iter_mut`](crate::system::Query::iter_mut) methods.
16-
pub struct QueryIter<'w, 's, Q: WorldQuery, QF: Fetch<'w, State = Q::State>, F: WorldQuery> {
16+
pub struct QueryIter<'w, 's, Q: WorldQuery, F: WorldQuery> {
1717
tables: &'w Tables,
1818
archetypes: &'w Archetypes,
1919
query_state: &'s QueryState<Q, F>,
20-
cursor: QueryIterationCursor<'w, 's, Q, QF, F>,
20+
cursor: QueryIterationCursor<'w, 's, Q, F>,
2121
}
2222

23-
impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery> QueryIter<'w, 's, Q, QF, F>
24-
where
25-
QF: Fetch<'w, State = Q::State>,
26-
{
23+
impl<'w, 's, Q: WorldQuery, F: WorldQuery> QueryIter<'w, 's, Q, F> {
2724
/// # Safety
2825
/// This does not check for mutable query correctness. To be safe, make sure mutable queries
2926
/// have unique access to the components they query.
@@ -44,11 +41,8 @@ where
4441
}
4542
}
4643

47-
impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery> Iterator for QueryIter<'w, 's, Q, QF, F>
48-
where
49-
QF: Fetch<'w, State = Q::State>,
50-
{
51-
type Item = QF::Item;
44+
impl<'w, 's, Q: WorldQuery, F: WorldQuery> Iterator for QueryIter<'w, 's, Q, F> {
45+
type Item = QueryItem<'w, Q>;
5246

5347
#[inline(always)]
5448
fn next(&mut self) -> Option<Self::Item> {
@@ -69,42 +63,32 @@ where
6963
.map(|id| self.archetypes[*id].len())
7064
.sum();
7165

72-
let archetype_query = F::Fetch::IS_ARCHETYPAL && QF::IS_ARCHETYPAL;
66+
let archetype_query = Q::Fetch::IS_ARCHETYPAL && F::Fetch::IS_ARCHETYPAL;
7367
let min_size = if archetype_query { max_size } else { 0 };
7468
(min_size, Some(max_size))
7569
}
7670
}
7771

7872
// This is correct as [`QueryIter`] always returns `None` once exhausted.
79-
impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery> FusedIterator for QueryIter<'w, 's, Q, QF, F> where
80-
QF: Fetch<'w, State = Q::State>
81-
{
82-
}
73+
impl<'w, 's, Q: WorldQuery, F: WorldQuery> FusedIterator for QueryIter<'w, 's, Q, F> {}
8374

8475
/// An [`Iterator`] over query results of a [`Query`](crate::system::Query).
8576
///
8677
/// This struct is created by the [`Query::iter_many`](crate::system::Query::iter_many) method.
87-
pub struct QueryManyIter<
88-
'w,
89-
's,
90-
Q: WorldQuery,
91-
QF: Fetch<'w, State = Q::State>,
92-
F: WorldQuery,
93-
I: Iterator,
94-
> where
78+
pub struct QueryManyIter<'w, 's, Q: WorldQuery, F: WorldQuery, I: Iterator>
79+
where
9580
I::Item: Borrow<Entity>,
9681
{
9782
entity_iter: I,
9883
entities: &'w Entities,
9984
tables: &'w Tables,
10085
archetypes: &'w Archetypes,
101-
fetch: QF,
86+
fetch: QueryFetch<'w, Q>,
10287
filter: QueryFetch<'w, F>,
10388
query_state: &'s QueryState<Q, F>,
10489
}
10590

106-
impl<'w, 's, Q: WorldQuery, QF: Fetch<'w, State = Q::State>, F: WorldQuery, I: Iterator>
107-
QueryManyIter<'w, 's, Q, QF, F, I>
91+
impl<'w, 's, Q: WorldQuery, F: WorldQuery, I: Iterator> QueryManyIter<'w, 's, Q, F, I>
10892
where
10993
I::Item: Borrow<Entity>,
11094
{
@@ -119,14 +103,14 @@ where
119103
entity_list: EntityList,
120104
last_change_tick: u32,
121105
change_tick: u32,
122-
) -> QueryManyIter<'w, 's, Q, QF, F, I> {
123-
let fetch = QF::init(
106+
) -> QueryManyIter<'w, 's, Q, F, I> {
107+
let fetch = Q::Fetch::init(
124108
world,
125109
&query_state.fetch_state,
126110
last_change_tick,
127111
change_tick,
128112
);
129-
let filter = QueryFetch::<F>::init(
113+
let filter = F::Fetch::init(
130114
world,
131115
&query_state.filter_state,
132116
last_change_tick,
@@ -144,12 +128,11 @@ where
144128
}
145129
}
146130

147-
impl<'w, 's, Q: WorldQuery, QF: Fetch<'w, State = Q::State>, F: WorldQuery, I: Iterator> Iterator
148-
for QueryManyIter<'w, 's, Q, QF, F, I>
131+
impl<'w, 's, Q: WorldQuery, F: WorldQuery, I: Iterator> Iterator for QueryManyIter<'w, 's, Q, F, I>
149132
where
150133
I::Item: Borrow<Entity>,
151134
{
152-
type Item = QF::Item;
135+
type Item = QueryItem<'w, Q>;
153136

154137
#[inline(always)]
155138
fn next(&mut self) -> Option<Self::Item> {
@@ -201,7 +184,7 @@ pub struct QueryCombinationIter<'w, 's, Q: WorldQuery, F: WorldQuery, const K: u
201184
tables: &'w Tables,
202185
archetypes: &'w Archetypes,
203186
query_state: &'s QueryState<Q, F>,
204-
cursors: [QueryIterationCursor<'w, 's, Q, QueryFetch<'w, Q>, F>; K],
187+
cursors: [QueryIterationCursor<'w, 's, Q, F>; K],
205188
}
206189

207190
impl<'w, 's, Q: WorldQuery, F: WorldQuery, const K: usize> QueryCombinationIter<'w, 's, Q, F, K> {
@@ -219,11 +202,10 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery, const K: usize> QueryCombinationIter<
219202
// Initialize array with cursors.
220203
// There is no FromIterator on arrays, so instead initialize it manually with MaybeUninit
221204

222-
let mut array: MaybeUninit<[QueryIterationCursor<'w, 's, Q, QueryFetch<'w, Q>, F>; K]> =
223-
MaybeUninit::uninit();
205+
let mut array: MaybeUninit<[QueryIterationCursor<'w, 's, Q, F>; K]> = MaybeUninit::uninit();
224206
let ptr = array
225207
.as_mut_ptr()
226-
.cast::<QueryIterationCursor<'w, 's, Q, QueryFetch<'w, Q>, F>>();
208+
.cast::<QueryIterationCursor<'w, 's, Q, F>>();
227209
if K != 0 {
228210
ptr.write(QueryIterationCursor::init(
229211
world,
@@ -367,10 +349,9 @@ where
367349
}
368350
}
369351

370-
impl<'w, 's, Q: WorldQuery, QF, F> ExactSizeIterator for QueryIter<'w, 's, Q, QF, F>
352+
impl<'w, 's, Q: WorldQuery, F: WorldQuery> ExactSizeIterator for QueryIter<'w, 's, Q, F>
371353
where
372-
QF: Fetch<'w, State = Q::State>,
373-
F: WorldQuery + ArchetypeFilter,
354+
F: ArchetypeFilter,
374355
{
375356
fn len(&self) -> usize {
376357
self.query_state
@@ -405,21 +386,21 @@ where
405386
{
406387
}
407388

408-
struct QueryIterationCursor<'w, 's, Q: WorldQuery, QF: Fetch<'w, State = Q::State>, F: WorldQuery> {
389+
struct QueryIterationCursor<'w, 's, Q: WorldQuery, F: WorldQuery> {
409390
table_id_iter: std::slice::Iter<'s, TableId>,
410391
archetype_id_iter: std::slice::Iter<'s, ArchetypeId>,
411-
fetch: QF,
392+
fetch: QueryFetch<'w, Q>,
412393
filter: QueryFetch<'w, F>,
413394
// length of the table table or length of the archetype, depending on whether both `Q`'s and `F`'s fetches are dense
414395
current_len: usize,
415396
// either table row or archetype index, depending on whether both `Q`'s and `F`'s fetches are dense
416397
current_index: usize,
417-
phantom: PhantomData<(&'w (), Q)>,
398+
phantom: PhantomData<Q>,
418399
}
419400

420-
impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery> Clone for QueryIterationCursor<'w, 's, Q, QF, F>
401+
impl<'w, 's, Q: WorldQuery, F: WorldQuery> Clone for QueryIterationCursor<'w, 's, Q, F>
421402
where
422-
QF: Fetch<'w, State = Q::State> + Clone,
403+
QueryFetch<'w, Q>: Clone,
423404
QueryFetch<'w, F>: Clone,
424405
{
425406
fn clone(&self) -> Self {
@@ -435,11 +416,8 @@ where
435416
}
436417
}
437418

438-
impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery> QueryIterationCursor<'w, 's, Q, QF, F>
439-
where
440-
QF: Fetch<'w, State = Q::State>,
441-
{
442-
const IS_DENSE: bool = QF::IS_DENSE && <QueryFetch<'static, F>>::IS_DENSE;
419+
impl<'w, 's, Q: WorldQuery, F: WorldQuery> QueryIterationCursor<'w, 's, Q, F> {
420+
const IS_DENSE: bool = Q::Fetch::IS_DENSE && F::Fetch::IS_DENSE;
443421

444422
unsafe fn init_empty(
445423
world: &'w World,
@@ -460,13 +438,13 @@ where
460438
last_change_tick: u32,
461439
change_tick: u32,
462440
) -> Self {
463-
let fetch = QF::init(
441+
let fetch = Q::Fetch::init(
464442
world,
465443
&query_state.fetch_state,
466444
last_change_tick,
467445
change_tick,
468446
);
469-
let filter = QueryFetch::<F>::init(
447+
let filter = F::Fetch::init(
470448
world,
471449
&query_state.filter_state,
472450
last_change_tick,
@@ -485,7 +463,7 @@ where
485463

486464
/// retrieve item returned from most recent `next` call again.
487465
#[inline]
488-
unsafe fn peek_last(&mut self) -> Option<QF::Item> {
466+
unsafe fn peek_last(&mut self) -> Option<QueryItem<'w, Q>> {
489467
if self.current_index > 0 {
490468
if Self::IS_DENSE {
491469
Some(self.fetch.table_fetch(self.current_index - 1))
@@ -509,7 +487,7 @@ where
509487
tables: &'w Tables,
510488
archetypes: &'w Archetypes,
511489
query_state: &'s QueryState<Q, F>,
512-
) -> Option<QF::Item> {
490+
) -> Option<QueryItem<'w, Q>> {
513491
if Self::IS_DENSE {
514492
loop {
515493
// we are on the beginning of the query, or finished processing a table, so skip to the next

crates/bevy_ecs/src/query/mod.rs

+50-10
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ pub(crate) unsafe fn debug_checked_unreachable() -> ! {
2121
mod tests {
2222
use super::WorldQuery;
2323
use crate::prelude::{AnyOf, Entity, Or, QueryState, With, Without};
24-
use crate::query::{ArchetypeFilter, QueryCombinationIter, QueryFetch, ReadOnlyWorldQuery};
25-
use crate::system::{IntoSystem, Query, System};
24+
use crate::query::{ArchetypeFilter, QueryCombinationIter, QueryFetch};
25+
use crate::system::{IntoSystem, Query, System, SystemState};
2626
use crate::{self as bevy_ecs, component::Component, world::World};
2727
use std::any::type_name;
2828
use std::collections::HashSet;
@@ -67,10 +67,11 @@ mod tests {
6767
}
6868
fn assert_combination<Q, F, const K: usize>(world: &mut World, expected_size: usize)
6969
where
70-
Q: ReadOnlyWorldQuery,
71-
F: ReadOnlyWorldQuery + ArchetypeFilter,
72-
for<'w> QueryFetch<'w, Q>: Clone,
73-
for<'w> QueryFetch<'w, F>: Clone,
70+
Q: WorldQuery,
71+
F: WorldQuery,
72+
F::ReadOnly: ArchetypeFilter,
73+
for<'w> QueryFetch<'w, Q::ReadOnly>: Clone,
74+
for<'w> QueryFetch<'w, F::ReadOnly>: Clone,
7475
{
7576
let mut query = world.query_filtered::<Q, F>();
7677
let iter = query.iter_combinations::<K>(world);
@@ -79,10 +80,11 @@ mod tests {
7980
}
8081
fn assert_all_sizes_equal<Q, F>(world: &mut World, expected_size: usize)
8182
where
82-
Q: ReadOnlyWorldQuery,
83-
F: ReadOnlyWorldQuery + ArchetypeFilter,
84-
for<'w> QueryFetch<'w, Q>: Clone,
85-
for<'w> QueryFetch<'w, F>: Clone,
83+
Q: WorldQuery,
84+
F: WorldQuery,
85+
F::ReadOnly: ArchetypeFilter,
86+
for<'w> QueryFetch<'w, Q::ReadOnly>: Clone,
87+
for<'w> QueryFetch<'w, F::ReadOnly>: Clone,
8688
{
8789
let mut query = world.query_filtered::<Q, F>();
8890
let iter = query.iter(world);
@@ -653,4 +655,42 @@ count(): {count}"#
653655
system.run((), &mut world);
654656
}
655657
}
658+
659+
#[test]
660+
fn mut_to_immut_query_methods_have_immut_item() {
661+
#[derive(Component)]
662+
struct Foo;
663+
664+
let mut world = World::new();
665+
let e = world.spawn().insert(Foo).id();
666+
667+
// state
668+
let mut q = world.query::<&mut Foo>();
669+
let _: Option<&Foo> = q.iter(&world).next();
670+
let _: Option<[&Foo; 2]> = q.iter_combinations::<2>(&world).next();
671+
let _: Option<&Foo> = q.iter_manual(&world).next();
672+
let _: Option<&Foo> = q.iter_many(&world, [e]).next();
673+
q.for_each(&world, |_: &Foo| ());
674+
675+
let _: Option<&Foo> = q.get(&world, e).ok();
676+
let _: Option<&Foo> = q.get_manual(&world, e).ok();
677+
let _: Option<[&Foo; 1]> = q.get_many(&world, [e]).ok();
678+
let _: Option<&Foo> = q.get_single(&world).ok();
679+
let _: &Foo = q.single(&world);
680+
681+
// system param
682+
let mut q = SystemState::<Query<&mut Foo>>::new(&mut world);
683+
let q = q.get_mut(&mut world);
684+
let _: Option<&Foo> = q.iter().next();
685+
let _: Option<[&Foo; 2]> = q.iter_combinations::<2>().next();
686+
let _: Option<&Foo> = q.iter_many([e]).next();
687+
q.for_each(|_: &Foo| ());
688+
689+
let _: Option<&Foo> = q.get(e).ok();
690+
let _: Option<&Foo> = q.get_component(e).ok();
691+
let _: Option<[&Foo; 1]> = q.get_many([e]).ok();
692+
let _: Option<&Foo> = q.get_single().ok();
693+
let _: [&Foo; 1] = q.many([e]);
694+
let _: &Foo = q.single();
695+
}
656696
}

0 commit comments

Comments
 (0)