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.

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

[Merged by Bors] - Query filter types must be ReadOnlyWorldQuery #6008

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ mod tests {
bundle::Bundle,
component::{Component, ComponentId},
entity::Entity,
query::{Added, ChangeTrackers, Changed, FilteredAccess, With, Without, WorldQuery},
query::{
Added, ChangeTrackers, Changed, FilteredAccess, ReadOnlyWorldQuery, With, Without,
},
system::Resource,
world::{Mut, World},
};
Expand Down Expand Up @@ -902,7 +904,7 @@ mod tests {
}
}

fn get_filtered<F: WorldQuery>(world: &mut World) -> Vec<Entity> {
fn get_filtered<F: ReadOnlyWorldQuery>(world: &mut World) -> Vec<Entity> {
world
.query_filtered::<Entity, F>()
.iter(world)
Expand Down
26 changes: 14 additions & 12 deletions crates/bevy_ecs/src/query/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +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, F: WorldQuery> {
pub struct QueryIter<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> {
tables: &'w Tables,
archetypes: &'w Archetypes,
query_state: &'s QueryState<Q, F>,
cursor: QueryIterationCursor<'w, 's, Q, F>,
}

impl<'w, 's, Q: WorldQuery, F: WorldQuery> QueryIter<'w, 's, Q, F> {
impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> 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 @@ -41,7 +41,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> QueryIter<'w, 's, Q, F> {
}
}

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

#[inline(always)]
Expand Down Expand Up @@ -70,12 +70,12 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Iterator for QueryIter<'w, 's, Q, F>
}

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

/// An [`Iterator`] over [`Query`](crate::system::Query) results of a list of [`Entity`]s.
///
/// This struct is created by the [`Query::iter_many`](crate::system::Query::iter_many) and [`Query::iter_many_mut`](crate::system::Query::iter_many_mut) methods.
pub struct QueryManyIter<'w, 's, Q: WorldQuery, F: WorldQuery, I: Iterator>
pub struct QueryManyIter<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery, I: Iterator>
where
I::Item: Borrow<Entity>,
{
Expand All @@ -88,7 +88,7 @@ where
query_state: &'s QueryState<Q, F>,
}

impl<'w, 's, Q: WorldQuery, F: WorldQuery, I: Iterator> QueryManyIter<'w, 's, Q, F, I>
impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery, I: Iterator> QueryManyIter<'w, 's, Q, F, I>
where
I::Item: Borrow<Entity>,
{
Expand Down Expand Up @@ -267,14 +267,16 @@ where
/// [`Query`]: crate::system::Query
/// [`Query::iter_combinations`]: crate::system::Query::iter_combinations
/// [`Query::iter_combinations_mut`]: crate::system::Query::iter_combinations_mut
pub struct QueryCombinationIter<'w, 's, Q: WorldQuery, F: WorldQuery, const K: usize> {
pub struct QueryCombinationIter<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery, const K: usize> {
tables: &'w Tables,
archetypes: &'w Archetypes,
query_state: &'s QueryState<Q, F>,
cursors: [QueryIterationCursor<'w, 's, Q, F>; K],
}

impl<'w, 's, Q: WorldQuery, F: WorldQuery, const K: usize> QueryCombinationIter<'w, 's, Q, F, K> {
impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery, const K: usize>
QueryCombinationIter<'w, 's, Q, F, K>
{
/// # 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 Down Expand Up @@ -436,7 +438,7 @@ where
}
}

impl<'w, 's, Q: WorldQuery, F: WorldQuery> ExactSizeIterator for QueryIter<'w, 's, Q, F>
impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> ExactSizeIterator for QueryIter<'w, 's, Q, F>
where
F: ArchetypeFilter,
{
Expand Down Expand Up @@ -473,7 +475,7 @@ where
{
}

struct QueryIterationCursor<'w, 's, Q: WorldQuery, F: WorldQuery> {
struct QueryIterationCursor<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> {
table_id_iter: std::slice::Iter<'s, TableId>,
archetype_id_iter: std::slice::Iter<'s, ArchetypeId>,
fetch: QueryFetch<'w, Q>,
Expand All @@ -485,7 +487,7 @@ struct QueryIterationCursor<'w, 's, Q: WorldQuery, F: WorldQuery> {
phantom: PhantomData<Q>,
}

impl<'w, 's, Q: WorldQuery, F: WorldQuery> Clone for QueryIterationCursor<'w, 's, Q, F>
impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Clone for QueryIterationCursor<'w, 's, Q, F>
where
QueryFetch<'w, Q>: Clone,
QueryFetch<'w, F>: Clone,
Expand All @@ -503,7 +505,7 @@ where
}
}

impl<'w, 's, Q: WorldQuery, F: WorldQuery> QueryIterationCursor<'w, 's, Q, F> {
impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's, Q, F> {
const IS_DENSE: bool = Q::IS_DENSE && F::IS_DENSE;

unsafe fn init_empty(
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_ecs/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub(crate) unsafe fn debug_checked_unreachable() -> ! {

#[cfg(test)]
mod tests {
use super::WorldQuery;
use super::{ReadOnlyWorldQuery, WorldQuery};
use crate::prelude::{AnyOf, Entity, Or, QueryState, With, Without};
use crate::query::{ArchetypeFilter, QueryCombinationIter, QueryFetch};
use crate::system::{IntoSystem, Query, System, SystemState};
Expand Down Expand Up @@ -68,7 +68,7 @@ mod tests {
fn assert_combination<Q, F, const K: usize>(world: &mut World, expected_size: usize)
where
Q: WorldQuery,
F: WorldQuery,
F: ReadOnlyWorldQuery,
F::ReadOnly: ArchetypeFilter,
for<'w> QueryFetch<'w, Q::ReadOnly>: Clone,
for<'w> QueryFetch<'w, F::ReadOnly>: Clone,
Expand All @@ -81,7 +81,7 @@ mod tests {
fn assert_all_sizes_equal<Q, F>(world: &mut World, expected_size: usize)
where
Q: WorldQuery,
F: WorldQuery,
F: ReadOnlyWorldQuery,
F::ReadOnly: ArchetypeFilter,
for<'w> QueryFetch<'w, Q::ReadOnly>: Clone,
for<'w> QueryFetch<'w, F::ReadOnly>: Clone,
Expand Down
12 changes: 6 additions & 6 deletions crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ use bevy_utils::tracing::Instrument;
use fixedbitset::FixedBitSet;
use std::{borrow::Borrow, fmt};

use super::{NopWorldQuery, QueryItem, QueryManyIter, ROQueryItem};
use super::{NopWorldQuery, QueryItem, QueryManyIter, ROQueryItem, ReadOnlyWorldQuery};

/// Provides scoped access to a [`World`] state according to a given [`WorldQuery`] and query filter.
#[repr(C)]
// SAFETY NOTE:
// Do not add any new fields that use the `Q` or `F` generic parameters as this may
// make `QueryState::as_transmuted_state` unsound if not done with care.
pub struct QueryState<Q: WorldQuery, F: WorldQuery = ()> {
pub struct QueryState<Q: WorldQuery, F: ReadOnlyWorldQuery = ()> {
world_id: WorldId,
pub(crate) archetype_generation: ArchetypeGeneration,
pub(crate) matched_tables: FixedBitSet,
Expand All @@ -35,13 +35,13 @@ pub struct QueryState<Q: WorldQuery, F: WorldQuery = ()> {
pub(crate) filter_state: F::State,
}

impl<Q: WorldQuery, F: WorldQuery> FromWorld for QueryState<Q, F> {
impl<Q: WorldQuery, F: ReadOnlyWorldQuery> FromWorld for QueryState<Q, F> {
fn from_world(world: &mut World) -> Self {
world.query_filtered()
}
}

impl<Q: WorldQuery, F: WorldQuery> QueryState<Q, F> {
impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
/// Converts this `QueryState` reference to a `QueryState` that does not access anything mutably.
pub fn as_readonly(&self) -> &QueryState<Q::ReadOnly, F::ReadOnly> {
// SAFETY: invariant on `WorldQuery` trait upholds that `Q::ReadOnly` and `F::ReadOnly`
Expand Down Expand Up @@ -71,15 +71,15 @@ impl<Q: WorldQuery, F: WorldQuery> QueryState<Q, F> {
/// `NewF` must have a subset of the access that `F` does and match the exact same archetypes/tables
pub(crate) unsafe fn as_transmuted_state<
NewQ: WorldQuery<State = Q::State>,
NewF: WorldQuery<State = F::State>,
NewF: ReadOnlyWorldQuery<State = F::State>,
>(
&self,
) -> &QueryState<NewQ, NewF> {
&*(self as *const QueryState<Q, F> as *const QueryState<NewQ, NewF>)
}
}

impl<Q: WorldQuery, F: WorldQuery> QueryState<Q, F> {
impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
/// Creates a new [`QueryState`] from a given [`World`] and inherits the result of `world.id()`.
pub fn new(world: &mut World) -> Self {
let fetch_state = Q::init_state(world);
Expand Down
10 changes: 5 additions & 5 deletions crates/bevy_ecs/src/system/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,14 +273,14 @@ use std::{any::TypeId, borrow::Borrow, fmt::Debug};
/// [`Table`]: crate::storage::Table
/// [`With`]: crate::query::With
/// [`Without`]: crate::query::Without
pub struct Query<'world, 'state, Q: WorldQuery, F: WorldQuery = ()> {
pub struct Query<'world, 'state, Q: WorldQuery, F: ReadOnlyWorldQuery = ()> {
pub(crate) world: &'world World,
pub(crate) state: &'state QueryState<Q, F>,
pub(crate) last_change_tick: u32,
pub(crate) change_tick: u32,
}

impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> {
impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> {
/// Creates a new query.
///
/// # Safety
Expand Down Expand Up @@ -1380,7 +1380,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> {
}
}

impl<'w, 's, Q: WorldQuery, F: WorldQuery> IntoIterator for &'w Query<'_, 's, Q, F> {
impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> IntoIterator for &'w Query<'_, 's, Q, F> {
type Item = ROQueryItem<'w, Q>;
type IntoIter = QueryIter<'w, 's, Q::ReadOnly, F::ReadOnly>;

Expand All @@ -1389,7 +1389,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> IntoIterator for &'w Query<'_, 's, Q,
}
}

impl<'w, 's, Q: WorldQuery, F: WorldQuery> IntoIterator for &'w mut Query<'_, 's, Q, F> {
impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> IntoIterator for &'w mut Query<'_, 's, Q, F> {
type Item = QueryItem<'w, Q>;
type IntoIter = QueryIter<'w, 's, Q, F>;

Expand Down Expand Up @@ -1434,7 +1434,7 @@ impl std::fmt::Display for QueryComponentError {
}
}

impl<'w, 's, Q: ReadOnlyWorldQuery, F: WorldQuery> Query<'w, 's, Q, F> {
impl<'w, 's, Q: ReadOnlyWorldQuery, F: ReadOnlyWorldQuery> Query<'w, 's, Q, F> {
/// Returns the query item for the given [`Entity`], with the actual "inner" world lifetime.
///
/// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is
Expand Down
17 changes: 11 additions & 6 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,16 +135,21 @@ pub trait SystemParamFetch<'world, 'state>: SystemParamState {
) -> Self::Item;
}

impl<'w, 's, Q: WorldQuery + 'static, F: WorldQuery + 'static> SystemParam for Query<'w, 's, Q, F> {
impl<'w, 's, Q: WorldQuery + 'static, F: ReadOnlyWorldQuery + 'static> SystemParam
for Query<'w, 's, Q, F>
{
type Fetch = QueryState<Q, F>;
}

// SAFETY: QueryState is constrained to read-only fetches, so it only reads World.
unsafe impl<Q: ReadOnlyWorldQuery, F: WorldQuery> ReadOnlySystemParamFetch for QueryState<Q, F> {}
unsafe impl<Q: ReadOnlyWorldQuery, F: ReadOnlyWorldQuery> ReadOnlySystemParamFetch
for QueryState<Q, F>
{
}

// SAFETY: Relevant query ComponentId and ArchetypeComponentId access is applied to SystemMeta. If
// this QueryState conflicts with any prior access, a panic will occur.
unsafe impl<Q: WorldQuery + 'static, F: WorldQuery + 'static> SystemParamState
unsafe impl<Q: WorldQuery + 'static, F: ReadOnlyWorldQuery + 'static> SystemParamState
for QueryState<Q, F>
{
fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self {
Expand Down Expand Up @@ -174,7 +179,7 @@ unsafe impl<Q: WorldQuery + 'static, F: WorldQuery + 'static> SystemParamState
}
}

impl<'w, 's, Q: WorldQuery + 'static, F: WorldQuery + 'static> SystemParamFetch<'w, 's>
impl<'w, 's, Q: WorldQuery + 'static, F: ReadOnlyWorldQuery + 'static> SystemParamFetch<'w, 's>
for QueryState<Q, F>
{
type Item = Query<'w, 's, Q, F>;
Expand Down Expand Up @@ -1595,7 +1600,7 @@ mod tests {
use super::SystemParam;
use crate::{
self as bevy_ecs, // Necessary for the `SystemParam` Derive when used inside `bevy_ecs`.
query::WorldQuery,
query::{ReadOnlyWorldQuery, WorldQuery},
system::Query,
};

Expand All @@ -1605,7 +1610,7 @@ mod tests {
'w,
's,
Q: WorldQuery + Send + Sync + 'static,
F: WorldQuery + Send + Sync + 'static = (),
F: ReadOnlyWorldQuery + Send + Sync + 'static = (),
> {
_query: Query<'w, 's, Q, F>,
}
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::{
StorageType,
},
entity::{AllocAtWithoutReplacement, Entities, Entity},
query::{QueryState, WorldQuery},
query::{QueryState, ReadOnlyWorldQuery, WorldQuery},
storage::{Column, SparseSet, Storages},
system::Resource,
};
Expand Down Expand Up @@ -610,7 +610,7 @@ impl World {
/// assert_eq!(matching_entities, vec![e2]);
/// ```
#[inline]
pub fn query_filtered<Q: WorldQuery, F: WorldQuery>(&mut self) -> QueryState<Q, F> {
pub fn query_filtered<Q: WorldQuery, F: ReadOnlyWorldQuery>(&mut self) -> QueryState<Q, F> {
QueryState::new(self)
}

Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_reflect/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ pub mod __macro_exports {
}

#[cfg(test)]
#[allow(clippy::blacklisted_name, clippy::approx_constant)]
#[allow(clippy::disallowed_types, clippy::approx_constant)]
mod tests {
#[cfg(feature = "glam")]
use ::glam::{vec3, Vec3};
Expand Down Expand Up @@ -188,7 +188,7 @@ mod tests {
}

#[test]
#[allow(clippy::blacklisted_name)]
#[allow(clippy::disallowed_types)]
fn reflect_unit_struct() {
#[derive(Reflect)]
struct Foo(u32, u64);
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_tasks/src/task_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ impl TaskPool {
// before this function returns. However, rust has no way of knowing
// this so we must convert to 'static here to appease the compiler as it is unable to
// validate safety.
let executor: &async_executor::Executor = &*self.executor;
let executor: &async_executor::Executor = &self.executor;
let executor: &'scope async_executor::Executor = unsafe { mem::transmute(executor) };
let local_executor: &'scope async_executor::LocalExecutor =
unsafe { mem::transmute(local_executor) };
Expand Down Expand Up @@ -287,7 +287,7 @@ impl<'scope, T: Send + 'scope> Scope<'scope, T> {
}

#[cfg(test)]
#[allow(clippy::blacklisted_name)]
#[allow(clippy::disallowed_types)]
mod tests {
use super::*;
use std::sync::{
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ui/src/flex/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{CalculatedSize, Node, Style, UiScale};
use bevy_ecs::{
entity::Entity,
event::EventReader,
query::{Changed, With, Without, WorldQuery},
query::{Changed, ReadOnlyWorldQuery, With, Without},
system::{Query, RemovedComponents, Res, ResMut, Resource},
};
use bevy_hierarchy::{Children, Parent};
Expand Down Expand Up @@ -234,7 +234,7 @@ pub fn flex_node_system(
update_changed(&mut *flex_surface, scale_factor, node_query);
}

fn update_changed<F: WorldQuery>(
fn update_changed<F: ReadOnlyWorldQuery>(
flex_surface: &mut FlexSurface,
scaling_factor: f64,
query: Query<(Entity, &Style, Option<&CalculatedSize>), F>,
Expand Down