Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Merged by Bors] - remove QF generics from all Query/State methods and types #5170

Closed
wants to merge 2 commits into from
Closed

[Merged by Bors] - remove QF generics from all Query/State methods and types #5170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c03dfa
remove QF generics from all `Query/State` methods
BoxyUwU Jul 4, 2022
5dad4fe
add warning to not do stuff with `Q` or `F` >:)
BoxyUwU Jul 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 25 additions & 5 deletions crates/bevy_ecs/src/query/fetch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ use std::{cell::UnsafeCell, marker::PhantomData};
/// ```
/// # Safety
///
/// component access of `ROQueryFetch<Self>` should be a subset of `QueryFetch<Self>`
/// component access of `ROQueryFetch<Self>` must be a subset of `QueryFetch<Self>`
/// and `ROQueryFetch<Self>` must match exactly the same archetypes/tables as `QueryFetch<Self>`
pub unsafe trait WorldQuery: for<'w> WorldQueryGats<'w, _State = Self::State> {
type ReadOnly: ReadOnlyWorldQuery<State = Self::State>;
type State: FetchState;
Expand Down Expand Up @@ -1608,6 +1609,25 @@ macro_rules! impl_anytuple_fetch {
all_tuples!(impl_tuple_fetch, 0, 15, F, S);
all_tuples!(impl_anytuple_fetch, 0, 15, F, S);

/// [`WorldQuery`] used to nullify queries by turning `Query<Q>` into `Query<NopWorldQuery<Q>>`
///
/// This will rarely be useful to consumers of `bevy_ecs`.
pub struct NopWorldQuery<Q: WorldQuery>(PhantomData<Q>);
BoxyUwU marked this conversation as resolved.
Show resolved Hide resolved

/// SAFETY: `Self::ReadOnly` is `Self`
unsafe impl<Q: WorldQuery> WorldQuery for NopWorldQuery<Q> {
type ReadOnly = Self;
type State = Q::State;

fn shrink<'wlong: 'wshort, 'wshort>(_: ()) {}
}
impl<'a, Q: WorldQuery> WorldQueryGats<'a> for NopWorldQuery<Q> {
type Fetch = NopFetch<QueryFetch<'a, Q>>;
type _State = <Q as WorldQueryGats<'a>>::_State;
}
/// SAFETY: `NopFetch` never accesses any data
unsafe impl<Q: WorldQuery> ReadOnlyWorldQuery for NopWorldQuery<Q> {}

/// [`Fetch`] that does not actually fetch anything
///
/// Mostly useful when something is generic over the Fetch and you don't want to fetch as you will discard the result
Expand All @@ -1616,18 +1636,18 @@ pub struct NopFetch<State> {
}

// SAFETY: NopFetch doesnt access anything
unsafe impl<'w, State: FetchState> Fetch<'w> for NopFetch<State> {
unsafe impl<'w, F: Fetch<'w>> Fetch<'w> for NopFetch<F> {
type Item = ();
type State = State;
type State = F::State;

const IS_DENSE: bool = true;
const IS_DENSE: bool = F::IS_DENSE;
cart marked this conversation as resolved.
Show resolved Hide resolved

const IS_ARCHETYPAL: bool = true;

#[inline(always)]
unsafe fn init(
_world: &'w World,
_state: &State,
_state: &F::State,
_last_change_tick: u32,
_change_tick: u32,
) -> Self {
Expand Down
86 changes: 32 additions & 54 deletions crates/bevy_ecs/src/query/iter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,14 @@ use super::{QueryFetch, QueryItem, ReadOnlyWorldQuery};
///
/// This struct is created by the [`Query::iter`](crate::system::Query::iter) and
/// [`Query::iter_mut`](crate::system::Query::iter_mut) methods.
pub struct QueryIter<'w, 's, Q: WorldQuery, QF: Fetch<'w, State = Q::State>, F: WorldQuery> {
pub struct QueryIter<'w, 's, Q: WorldQuery, F: WorldQuery> {
tables: &'w Tables,
archetypes: &'w Archetypes,
query_state: &'s QueryState<Q, F>,
cursor: QueryIterationCursor<'w, 's, Q, QF, F>,
cursor: QueryIterationCursor<'w, 's, Q, F>,
}

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

impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery> Iterator for QueryIter<'w, 's, Q, QF, F>
where
QF: Fetch<'w, State = Q::State>,
{
type Item = QF::Item;
impl<'w, 's, Q: WorldQuery, F: WorldQuery> Iterator for QueryIter<'w, 's, Q, F> {
type Item = QueryItem<'w, Q>;

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

let archetype_query = F::Fetch::IS_ARCHETYPAL && QF::IS_ARCHETYPAL;
let archetype_query = Q::Fetch::IS_ARCHETYPAL && F::Fetch::IS_ARCHETYPAL;
let min_size = if archetype_query { max_size } else { 0 };
(min_size, Some(max_size))
}
}

// This is correct as [`QueryIter`] always returns `None` once exhausted.
impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery> FusedIterator for QueryIter<'w, 's, Q, QF, F> where
QF: Fetch<'w, State = Q::State>
{
}
impl<'w, 's, Q: WorldQuery, F: WorldQuery> FusedIterator for QueryIter<'w, 's, Q, F> {}

/// An [`Iterator`] over query results of a [`Query`](crate::system::Query).
///
/// This struct is created by the [`Query::iter_many`](crate::system::Query::iter_many) method.
pub struct QueryManyIter<
'w,
's,
Q: WorldQuery,
QF: Fetch<'w, State = Q::State>,
F: WorldQuery,
I: Iterator,
> where
pub struct QueryManyIter<'w, 's, Q: WorldQuery, F: WorldQuery, I: Iterator>
where
I::Item: Borrow<Entity>,
{
entity_iter: I,
entities: &'w Entities,
tables: &'w Tables,
archetypes: &'w Archetypes,
fetch: QF,
fetch: QueryFetch<'w, Q>,
filter: QueryFetch<'w, F>,
query_state: &'s QueryState<Q, F>,
}

impl<'w, 's, Q: WorldQuery, QF: Fetch<'w, State = Q::State>, F: WorldQuery, I: Iterator>
QueryManyIter<'w, 's, Q, QF, F, I>
impl<'w, 's, Q: WorldQuery, F: WorldQuery, I: Iterator> QueryManyIter<'w, 's, Q, F, I>
where
I::Item: Borrow<Entity>,
{
Expand All @@ -119,14 +103,14 @@ where
entity_list: EntityList,
last_change_tick: u32,
change_tick: u32,
) -> QueryManyIter<'w, 's, Q, QF, F, I> {
let fetch = QF::init(
) -> QueryManyIter<'w, 's, Q, F, I> {
let fetch = Q::Fetch::init(
world,
&query_state.fetch_state,
last_change_tick,
change_tick,
);
let filter = QueryFetch::<F>::init(
let filter = F::Fetch::init(
world,
&query_state.filter_state,
last_change_tick,
Expand All @@ -144,12 +128,11 @@ where
}
}

impl<'w, 's, Q: WorldQuery, QF: Fetch<'w, State = Q::State>, F: WorldQuery, I: Iterator> Iterator
for QueryManyIter<'w, 's, Q, QF, F, I>
impl<'w, 's, Q: WorldQuery, F: WorldQuery, I: Iterator> Iterator for QueryManyIter<'w, 's, Q, F, I>
where
I::Item: Borrow<Entity>,
{
type Item = QF::Item;
type Item = QueryItem<'w, Q>;

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

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

let mut array: MaybeUninit<[QueryIterationCursor<'w, 's, Q, QueryFetch<'w, Q>, F>; K]> =
MaybeUninit::uninit();
let mut array: MaybeUninit<[QueryIterationCursor<'w, 's, Q, F>; K]> = MaybeUninit::uninit();
let ptr = array
.as_mut_ptr()
.cast::<QueryIterationCursor<'w, 's, Q, QueryFetch<'w, Q>, F>>();
.cast::<QueryIterationCursor<'w, 's, Q, F>>();
if K != 0 {
ptr.write(QueryIterationCursor::init(
world,
Expand Down Expand Up @@ -367,10 +349,9 @@ where
}
}

impl<'w, 's, Q: WorldQuery, QF, F> ExactSizeIterator for QueryIter<'w, 's, Q, QF, F>
impl<'w, 's, Q: WorldQuery, F: WorldQuery> ExactSizeIterator for QueryIter<'w, 's, Q, F>
where
QF: Fetch<'w, State = Q::State>,
F: WorldQuery + ArchetypeFilter,
F: ArchetypeFilter,
{
fn len(&self) -> usize {
self.query_state
Expand Down Expand Up @@ -405,21 +386,21 @@ where
{
}

struct QueryIterationCursor<'w, 's, Q: WorldQuery, QF: Fetch<'w, State = Q::State>, F: WorldQuery> {
struct QueryIterationCursor<'w, 's, Q: WorldQuery, F: WorldQuery> {
table_id_iter: std::slice::Iter<'s, TableId>,
archetype_id_iter: std::slice::Iter<'s, ArchetypeId>,
fetch: QF,
fetch: QueryFetch<'w, Q>,
filter: QueryFetch<'w, F>,
// length of the table table or length of the archetype, depending on whether both `Q`'s and `F`'s fetches are dense
current_len: usize,
// either table row or archetype index, depending on whether both `Q`'s and `F`'s fetches are dense
current_index: usize,
phantom: PhantomData<(&'w (), Q)>,
phantom: PhantomData<Q>,
}

impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery> Clone for QueryIterationCursor<'w, 's, Q, QF, F>
impl<'w, 's, Q: WorldQuery, F: WorldQuery> Clone for QueryIterationCursor<'w, 's, Q, F>
where
QF: Fetch<'w, State = Q::State> + Clone,
QueryFetch<'w, Q>: Clone,
QueryFetch<'w, F>: Clone,
{
fn clone(&self) -> Self {
Expand All @@ -435,11 +416,8 @@ where
}
}

impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery> QueryIterationCursor<'w, 's, Q, QF, F>
where
QF: Fetch<'w, State = Q::State>,
{
const IS_DENSE: bool = QF::IS_DENSE && <QueryFetch<'static, F>>::IS_DENSE;
impl<'w, 's, Q: WorldQuery, F: WorldQuery> QueryIterationCursor<'w, 's, Q, F> {
const IS_DENSE: bool = Q::Fetch::IS_DENSE && F::Fetch::IS_DENSE;
BoxyUwU marked this conversation as resolved.
Show resolved Hide resolved

unsafe fn init_empty(
world: &'w World,
Expand All @@ -460,13 +438,13 @@ where
last_change_tick: u32,
change_tick: u32,
) -> Self {
let fetch = QF::init(
let fetch = Q::Fetch::init(
world,
&query_state.fetch_state,
last_change_tick,
change_tick,
);
let filter = QueryFetch::<F>::init(
let filter = F::Fetch::init(
world,
&query_state.filter_state,
last_change_tick,
Expand All @@ -485,7 +463,7 @@ where

/// retrieve item returned from most recent `next` call again.
#[inline]
unsafe fn peek_last(&mut self) -> Option<QF::Item> {
unsafe fn peek_last(&mut self) -> Option<QueryItem<'w, Q>> {
if self.current_index > 0 {
if Self::IS_DENSE {
Some(self.fetch.table_fetch(self.current_index - 1))
Expand All @@ -509,7 +487,7 @@ where
tables: &'w Tables,
archetypes: &'w Archetypes,
query_state: &'s QueryState<Q, F>,
) -> Option<QF::Item> {
) -> Option<QueryItem<'w, Q>> {
if Self::IS_DENSE {
loop {
// we are on the beginning of the query, or finished processing a table, so skip to the next
Expand Down
60 changes: 50 additions & 10 deletions crates/bevy_ecs/src/query/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ pub(crate) unsafe fn debug_checked_unreachable() -> ! {
mod tests {
use super::WorldQuery;
use crate::prelude::{AnyOf, Entity, Or, QueryState, With, Without};
use crate::query::{ArchetypeFilter, QueryCombinationIter, QueryFetch, ReadOnlyWorldQuery};
use crate::system::{IntoSystem, Query, System};
use crate::query::{ArchetypeFilter, QueryCombinationIter, QueryFetch};
use crate::system::{IntoSystem, Query, System, SystemState};
use crate::{self as bevy_ecs, component::Component, world::World};
use std::any::type_name;
use std::collections::HashSet;
Expand Down Expand Up @@ -67,10 +67,11 @@ mod tests {
}
fn assert_combination<Q, F, const K: usize>(world: &mut World, expected_size: usize)
where
Q: ReadOnlyWorldQuery,
F: ReadOnlyWorldQuery + ArchetypeFilter,
for<'w> QueryFetch<'w, Q>: Clone,
for<'w> QueryFetch<'w, F>: Clone,
Q: WorldQuery,
F: WorldQuery,
F::ReadOnly: ArchetypeFilter,
for<'w> QueryFetch<'w, Q::ReadOnly>: Clone,
for<'w> QueryFetch<'w, F::ReadOnly>: Clone,
{
let mut query = world.query_filtered::<Q, F>();
let iter = query.iter_combinations::<K>(world);
Expand All @@ -79,10 +80,11 @@ mod tests {
}
fn assert_all_sizes_equal<Q, F>(world: &mut World, expected_size: usize)
where
Q: ReadOnlyWorldQuery,
F: ReadOnlyWorldQuery + ArchetypeFilter,
for<'w> QueryFetch<'w, Q>: Clone,
for<'w> QueryFetch<'w, F>: Clone,
Q: WorldQuery,
F: WorldQuery,
F::ReadOnly: ArchetypeFilter,
for<'w> QueryFetch<'w, Q::ReadOnly>: Clone,
for<'w> QueryFetch<'w, F::ReadOnly>: Clone,
{
let mut query = world.query_filtered::<Q, F>();
let iter = query.iter(world);
Expand Down Expand Up @@ -653,4 +655,42 @@ count(): {count}"#
system.run((), &mut world);
}
}

#[test]
fn mut_to_immut_query_methods_have_immut_item() {
#[derive(Component)]
struct Foo;

let mut world = World::new();
let e = world.spawn().insert(Foo).id();

// state
let mut q = world.query::<&mut Foo>();
let _: Option<&Foo> = q.iter(&world).next();
let _: Option<[&Foo; 2]> = q.iter_combinations::<2>(&world).next();
let _: Option<&Foo> = q.iter_manual(&world).next();
let _: Option<&Foo> = q.iter_many(&world, [e]).next();
q.for_each(&world, |_: &Foo| ());

let _: Option<&Foo> = q.get(&world, e).ok();
let _: Option<&Foo> = q.get_manual(&world, e).ok();
let _: Option<[&Foo; 1]> = q.get_many(&world, [e]).ok();
let _: Option<&Foo> = q.get_single(&world).ok();
let _: &Foo = q.single(&world);

// system param
let mut q = SystemState::<Query<&mut Foo>>::new(&mut world);
let q = q.get_mut(&mut world);
let _: Option<&Foo> = q.iter().next();
let _: Option<[&Foo; 2]> = q.iter_combinations::<2>().next();
let _: Option<&Foo> = q.iter_many([e]).next();
q.for_each(|_: &Foo| ());

let _: Option<&Foo> = q.get(e).ok();
let _: Option<&Foo> = q.get_component(e).ok();
let _: Option<[&Foo; 1]> = q.get_many([e]).ok();
let _: Option<&Foo> = q.get_single().ok();
let _: [&Foo; 1] = q.many([e]);
let _: &Foo = q.single();
}
}
Loading