Skip to content

Use type aliases for Query item types in Query and QueryState methods #4297

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

Closed
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
8 changes: 8 additions & 0 deletions crates/bevy_ecs/src/query/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,16 @@ pub trait WorldQuery {
+ ReadOnlyFetch;
}

/// The item type returned when you iterate a [`Query`](crate::system::Query) or [`QueryState`](crate::query::QueryState)
///
/// Unlike [`ReadOnlyQueryItem`], this type may allow mutable access to the component data.
pub type QueryItem<'w, 's, Q> = <<Q as WorldQuery>::Fetch as Fetch<'w, 's>>::Item;

/// The item type returned when you iterate a [`Query`](crate::system::Query) or [`QueryState`](crate::query::QueryState)
///
/// Unlike [`QueryItem`], this type only permits immutable access to the component data.
pub type ReadOnlyQueryItem<'w, 's, Q> = <<Q as WorldQuery>::ReadOnlyFetch as Fetch<'w, 's>>::Item;

/// Types that implement this trait are responsible for fetching query items from tables or
/// archetypes.
///
Expand Down
32 changes: 11 additions & 21 deletions crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
entity::Entity,
query::{
Access, Fetch, FetchState, FilterFetch, FilteredAccess, NopFetch, QueryCombinationIter,
QueryIter, WorldQuery,
QueryItem, QueryIter, ReadOnlyQueryItem, WorldQuery,
},
storage::TableId,
world::{World, WorldId},
Expand Down Expand Up @@ -140,7 +140,7 @@ where
&'s mut self,
world: &'w World,
entity: Entity,
) -> Result<<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item, QueryEntityError> {
) -> Result<ReadOnlyQueryItem<'w, 's, Q>, QueryEntityError> {
self.update_archetypes(world);
// SAFETY: query is read only
unsafe {
Expand All @@ -159,7 +159,7 @@ where
&'s mut self,
world: &'w mut World,
entity: Entity,
) -> Result<<Q::Fetch as Fetch<'w, 's>>::Item, QueryEntityError> {
) -> Result<QueryItem<'w, 's, Q>, QueryEntityError> {
self.update_archetypes(world);
// SAFETY: query has unique world access
unsafe {
Expand All @@ -177,7 +177,7 @@ where
&'s self,
world: &'w World,
entity: Entity,
) -> Result<<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item, QueryEntityError> {
) -> Result<ReadOnlyQueryItem<'w, 's, Q>, QueryEntityError> {
self.validate_world(world);
// SAFETY: query is read only and world is validated
unsafe {
Expand All @@ -201,7 +201,7 @@ where
&'s mut self,
world: &'w World,
entity: Entity,
) -> Result<<Q::Fetch as Fetch<'w, 's>>::Item, QueryEntityError> {
) -> Result<QueryItem<'w, 's, Q>, QueryEntityError> {
self.update_archetypes(world);
self.get_unchecked_manual::<Q::Fetch>(
world,
Expand Down Expand Up @@ -427,7 +427,7 @@ where
///
/// This can only be called for read-only queries, see [`Self::for_each_mut`] for write-queries.
#[inline]
pub fn for_each<'w, 's, FN: FnMut(<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item)>(
pub fn for_each<'w, 's, FN: FnMut(ReadOnlyQueryItem<'w, 's, Q>)>(
&'s mut self,
world: &'w World,
func: FN,
Expand All @@ -447,7 +447,7 @@ where
/// Runs `func` on each query result for the given [`World`]. This is faster than the equivalent
/// `iter_mut()` method, but cannot be chained like a normal [`Iterator`].
#[inline]
pub fn for_each_mut<'w, 's, FN: FnMut(<Q::Fetch as Fetch<'w, 's>>::Item)>(
pub fn for_each_mut<'w, 's, FN: FnMut(QueryItem<'w, 's, Q>)>(
&'s mut self,
world: &'w mut World,
func: FN,
Expand All @@ -467,14 +467,12 @@ where
/// Runs `func` on each query result for the given [`World`]. This is faster than the equivalent
/// iter() method, but cannot be chained like a normal [`Iterator`].
///
/// This can only be called for read-only queries.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment was blatantly false, and seems to just be a mistake. It contradicts the safety comment, and doesn't seem to make sense with the API.

///
/// # Safety
///
/// This does not check for mutable query correctness. To be safe, make sure mutable queries
/// have unique access to the components they query.
#[inline]
pub unsafe fn for_each_unchecked<'w, 's, FN: FnMut(<Q::Fetch as Fetch<'w, 's>>::Item)>(
pub unsafe fn for_each_unchecked<'w, 's, FN: FnMut(QueryItem<'w, 's, Q>)>(
&'s mut self,
world: &'w World,
func: FN,
Expand All @@ -493,11 +491,7 @@ where
/// This can only be called for read-only queries, see [`Self::par_for_each_mut`] for
/// write-queries.
#[inline]
pub fn par_for_each<
'w,
's,
FN: Fn(<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item) + Send + Sync + Clone,
>(
pub fn par_for_each<'w, 's, FN: Fn(ReadOnlyQueryItem<'w, 's, Q>) + Send + Sync + Clone>(
&'s mut self,
world: &'w World,
task_pool: &TaskPool,
Expand All @@ -520,11 +514,7 @@ where

/// Runs `func` on each query result in parallel using the given `task_pool`.
#[inline]
pub fn par_for_each_mut<
'w,
's,
FN: Fn(<Q::Fetch as Fetch<'w, 's>>::Item) + Send + Sync + Clone,
>(
pub fn par_for_each_mut<'w, 's, FN: Fn(QueryItem<'w, 's, Q>) + Send + Sync + Clone>(
&'s mut self,
world: &'w mut World,
task_pool: &TaskPool,
Expand Down Expand Up @@ -557,7 +547,7 @@ where
pub unsafe fn par_for_each_unchecked<
'w,
's,
FN: Fn(<Q::Fetch as Fetch<'w, 's>>::Item) + Send + Sync + Clone,
FN: Fn(QueryItem<'w, 's, Q>) + Send + Sync + Clone,
>(
&'s mut self,
world: &'w World,
Expand Down
41 changes: 14 additions & 27 deletions crates/bevy_ecs/src/system/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use crate::{
component::Component,
entity::Entity,
query::{
Fetch, FilterFetch, NopFetch, QueryCombinationIter, QueryEntityError, QueryIter,
QueryState, ReadOnlyFetch, WorldQuery,
FilterFetch, NopFetch, QueryCombinationIter, QueryEntityError, QueryItem, QueryIter,
QueryState, ReadOnlyFetch, ReadOnlyQueryItem, WorldQuery,
},
world::{Mut, World},
};
Expand Down Expand Up @@ -454,10 +454,7 @@ where
/// # bevy_ecs::system::assert_is_system(report_names_system);
/// ```
#[inline]
pub fn for_each<'this>(
&'this self,
f: impl FnMut(<Q::ReadOnlyFetch as Fetch<'this, 's>>::Item),
) {
pub fn for_each<'this>(&'this self, f: impl FnMut(ReadOnlyQueryItem<'this, 's, Q>)) {
// SAFE: system runs without conflicts with other systems.
// same-system queries have runtime borrow checks when they conflict
unsafe {
Expand Down Expand Up @@ -492,7 +489,7 @@ where
/// # bevy_ecs::system::assert_is_system(gravity_system);
/// ```
#[inline]
pub fn for_each_mut<'a, FN: FnMut(<Q::Fetch as Fetch<'a, 'a>>::Item)>(&'a mut self, f: FN) {
pub fn for_each_mut<'a, FN: FnMut(QueryItem<'a, 'a, Q>)>(&'a mut self, f: FN) {
// SAFE: system runs without conflicts with other systems. same-system queries have runtime
// borrow checks when they conflict
unsafe {
Expand Down Expand Up @@ -530,7 +527,7 @@ where
&'this self,
task_pool: &TaskPool,
batch_size: usize,
f: impl Fn(<Q::ReadOnlyFetch as Fetch<'this, 's>>::Item) + Send + Sync + Clone,
f: impl Fn(ReadOnlyQueryItem<'this, 's, Q>) + Send + Sync + Clone,
) {
// SAFE: system runs without conflicts with other systems. same-system queries have runtime
// borrow checks when they conflict
Expand All @@ -550,7 +547,7 @@ where
/// Runs `f` on each query result in parallel using the given [`TaskPool`].
/// See [`Self::par_for_each`] for more details.
#[inline]
pub fn par_for_each_mut<'a, FN: Fn(<Q::Fetch as Fetch<'a, 'a>>::Item) + Send + Sync + Clone>(
pub fn par_for_each_mut<'a, FN: Fn(QueryItem<'a, 'a, Q>) + Send + Sync + Clone>(
&'a mut self,
task_pool: &TaskPool,
batch_size: usize,
Expand Down Expand Up @@ -602,10 +599,7 @@ where
/// # bevy_ecs::system::assert_is_system(print_selected_character_name_system);
/// ```
#[inline]
pub fn get(
&self,
entity: Entity,
) -> Result<<Q::ReadOnlyFetch as Fetch<'_, 's>>::Item, QueryEntityError> {
pub fn get(&self, entity: Entity) -> Result<ReadOnlyQueryItem<Q>, QueryEntityError> {
// SAFE: system runs without conflicts with other systems.
// same-system queries have runtime borrow checks when they conflict
unsafe {
Expand Down Expand Up @@ -643,10 +637,7 @@ where
/// # bevy_ecs::system::assert_is_system(poison_system);
/// ```
#[inline]
pub fn get_mut(
&mut self,
entity: Entity,
) -> Result<<Q::Fetch as Fetch>::Item, QueryEntityError> {
pub fn get_mut(&mut self, entity: Entity) -> Result<QueryItem<Q>, QueryEntityError> {
// SAFE: system runs without conflicts with other systems.
// same-system queries have runtime borrow checks when they conflict
unsafe {
Expand All @@ -672,7 +663,7 @@ where
pub unsafe fn get_unchecked(
&'s self,
entity: Entity,
) -> Result<<Q::Fetch as Fetch<'w, 's>>::Item, QueryEntityError> {
) -> Result<QueryItem<Q>, QueryEntityError> {
// SEMI-SAFE: system runs without conflicts with other systems.
// same-system queries have runtime borrow checks when they conflict
self.state.get_unchecked_manual::<Q::Fetch>(
Expand Down Expand Up @@ -836,7 +827,7 @@ where
/// Panics if the number of query results is not exactly one. Use
/// [`get_single`](Self::get_single) to return a `Result` instead of panicking.
#[track_caller]
pub fn single(&self) -> <Q::ReadOnlyFetch as Fetch<'_, 's>>::Item {
pub fn single(&self) -> ReadOnlyQueryItem<Q> {
self.get_single().unwrap()
}

Expand Down Expand Up @@ -871,9 +862,7 @@ where
/// }
/// # bevy_ecs::system::assert_is_system(player_scoring_system);
/// ```
pub fn get_single(
&self,
) -> Result<<Q::ReadOnlyFetch as Fetch<'_, 's>>::Item, QuerySingleError> {
pub fn get_single(&self) -> Result<ReadOnlyQueryItem<Q>, QuerySingleError> {
let mut query = self.iter();
let first = query.next();
let extra = query.next().is_some();
Expand Down Expand Up @@ -912,7 +901,7 @@ where
/// Panics if the number of query results is not exactly one. Use
/// [`get_single_mut`](Self::get_single_mut) to return a `Result` instead of panicking.
#[track_caller]
pub fn single_mut(&mut self) -> <Q::Fetch as Fetch<'_, '_>>::Item {
pub fn single_mut(&mut self) -> QueryItem<Q> {
self.get_single_mut().unwrap()
}

Expand All @@ -938,9 +927,7 @@ where
/// }
/// # bevy_ecs::system::assert_is_system(regenerate_player_health_system);
/// ```
pub fn get_single_mut(
&mut self,
) -> Result<<Q::Fetch as Fetch<'_, '_>>::Item, QuerySingleError> {
pub fn get_single_mut(&mut self) -> Result<QueryItem<Q>, QuerySingleError> {
let mut query = self.iter_mut();
let first = query.next();
let extra = query.next().is_some();
Expand Down Expand Up @@ -1081,7 +1068,7 @@ where
pub fn get_inner(
&'s self,
entity: Entity,
) -> Result<<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item, QueryEntityError> {
) -> Result<ReadOnlyQueryItem<'w, 's, Q>, QueryEntityError> {
// SAFE: system runs without conflicts with other systems.
// same-system queries have runtime borrow checks when they conflict
unsafe {
Expand Down