Skip to content

Combine Query and QueryLens using a type parameter for state #18162

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

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
e943852
Make Query generic over the State type so it can use either owned or …
chescock Feb 18, 2025
f7ff2b3
Make QueryLens a type alias.
chescock Feb 19, 2025
0a68186
Update doc link now that `StorageId` is `pub`.
chescock Mar 5, 2025
49ae0f3
Box QueryState.
chescock Mar 5, 2025
2d2d78d
Rename QueryStateBorrow to QueryStateDeref.
chescock Mar 5, 2025
4c367db
Make QueryStateDeref an unsafe trait.
chescock Mar 5, 2025
556eced
Introduce a TrustedEntityQueryData trait so we can make a blanket `im…
chescock Mar 5, 2025
c81671d
Doc fixes.
chescock Mar 5, 2025
e94970c
Better docs for `TrustedEntityQueryData` from @Victoronz
chescock Mar 6, 2025
39c8c16
Use pointer casts for `Box<QueryState>::into_readonly()` to avoid nee…
chescock Mar 11, 2025
3d2bba9
Merge remote-tracking branch 'remotes/origin/main' into query-owned-s…
chescock Apr 4, 2025
ddd30a5
Fix other references to `TrustedEntityBorrow`.
chescock Apr 4, 2025
0e61022
Rename `TrustedEntityQueryData` to `EntityEquivalentQueryData`.
chescock Apr 4, 2025
3e83269
Merge remote-tracking branch 'remotes/origin/main' into query-owned-s…
chescock Apr 4, 2025
de74769
Add migration guide.
chescock May 7, 2025
1751fa0
Merge branch 'main' into query-owned-state
chescock May 7, 2025
fccca4d
bevy_platform_support was renamed bevy_platform.
chescock May 7, 2025
1bc9f75
Remove `'s` lifetime from `WorldQuery::Fetch`.
chescock Jun 17, 2025
66751f0
Pass `&'s QueryState` to `fetch` and `filter_fetch`.
chescock Jun 17, 2025
391665f
Merge branch 'no-state-lifetime-in-fetch' into query-owned-state-merged
chescock Jun 18, 2025
b9fc1ff
Rework to support query data that borrows from state.
chescock Jun 18, 2025
ebf1b12
Fix clippy lints.
chescock Jun 18, 2025
bde90e1
Merge remote-tracking branch 'remotes/origin/main' into query-owned-s…
chescock Jun 19, 2025
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
37 changes: 36 additions & 1 deletion crates/bevy_ecs/src/query/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{
bundle::Bundle,
change_detection::{MaybeLocation, Ticks, TicksMut},
component::{Component, ComponentId, Components, Mutable, StorageType, Tick},
entity::{Entities, Entity, EntityLocation},
entity::{Entities, Entity, EntityEquivalent, EntityLocation},
query::{Access, DebugCheckedUnwrap, FilteredAccess, WorldQuery},
storage::{ComponentSparseSet, Table, TableRow},
world::{
Expand Down Expand Up @@ -340,6 +340,20 @@ pub unsafe trait QueryData: WorldQuery {
/// This must only be implemented for read-only [`QueryData`]'s.
pub unsafe trait ReadOnlyQueryData: QueryData<ReadOnly = Self> {}

/// A [`QueryData`] type that produces a [`EntityEquivalent`] item
/// equaling the `Entity` it is addressed by.
///
/// # Safety
///
/// [`<Self as QueryData>::fetch`](QueryData::fetch) must always return an item whose `Entity`
/// equals the one the function was called with.
/// I.e.: `Self::fetch(fetch, entity, table_row).entity() == entity` always holds.
pub unsafe trait EntityEquivalentQueryData: QueryData
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about leaving out the Equivalent from the trait name?
It seems redundant, the bound on Item defines what this trait sees as an entity, and plain EntityQueryData would be easier to parse/understand imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think about leaving out the Equivalent from the trait name? It seems redundant, the bound on Item defines what this trait sees as an entity, and plain EntityQueryData would be easier to parse/understand imo.

I don't have any especially strong opinions here. Most people won't need to look at this trait at all; they'll expect QueryIter<Entity> to be usable as an EntitySet, and it will be, so they won't worry about how that was implemented.

The trickiest part here is that MainEntity and RenderEntity are EntityEquivalent + QueryData, but can't be EntityEquivalentQueryData because they return a different entity. So maybe EntityEquivalentQueryData is misleading on that count? But EntityQueryData seems like it has the same confusion. Maybe something math-y? No, ReflexiveEntityQueryData just sounds awkward.

Since the stakes are low and none of the options seem perfect, I'm inclined to leave it alone for now, but if you argue more then I'll probably change it rather than argue back :).

where
for<'w, 's> Self: QueryData<Item<'w, 's>: EntityEquivalent>,
{
}

/// The item type returned when a [`WorldQuery`] is iterated over
pub type QueryItem<'w, 's, Q> = <Q as QueryData>::Item<'w, 's>;
/// The read-only variant of the item type returned when a [`QueryData`] is iterated over immutably
Expand Down Expand Up @@ -435,6 +449,9 @@ unsafe impl QueryData for Entity {
/// SAFETY: access is read only
unsafe impl ReadOnlyQueryData for Entity {}

/// SAFETY: `entity()` returns `self`, and `fetch` returns `entity`
unsafe impl EntityEquivalentQueryData for Entity {}

impl ReleaseStateQueryData for Entity {
fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> {
item
Expand Down Expand Up @@ -814,6 +831,9 @@ unsafe impl<'a> QueryData for EntityRef<'a> {
/// SAFETY: access is read only
unsafe impl ReadOnlyQueryData for EntityRef<'_> {}

/// SAFETY: `entity()` returns the `entity` used to get the `UnsafeEntityCell`, and `fetch` constructs that from `entity`
unsafe impl EntityEquivalentQueryData for EntityRef<'_> {}

impl ReleaseStateQueryData for EntityRef<'_> {
fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> {
item
Expand Down Expand Up @@ -914,6 +934,9 @@ unsafe impl<'a> QueryData for EntityMut<'a> {
}
}

/// SAFETY: `entity()` returns the `entity` used to get the `UnsafeEntityCell`, and `fetch` constructs that from `entity`
unsafe impl EntityEquivalentQueryData for EntityMut<'_> {}

impl ReleaseStateQueryData for EntityMut<'_> {
fn release_state<'w>(item: Self::Item<'w, '_>) -> Self::Item<'w, 'static> {
item
Expand Down Expand Up @@ -1044,6 +1067,9 @@ unsafe impl<'a> QueryData for FilteredEntityRef<'a> {
/// SAFETY: Access is read-only.
unsafe impl ReadOnlyQueryData for FilteredEntityRef<'_> {}

/// SAFETY: `entity()` returns the `entity` used to get the `UnsafeEntityCell`, and `fetch` constructs that from `entity`
unsafe impl EntityEquivalentQueryData for FilteredEntityRef<'_> {}

/// SAFETY: The accesses of `Self::ReadOnly` are a subset of the accesses of `Self`
unsafe impl<'a> WorldQuery for FilteredEntityMut<'a> {
type Fetch<'w> = (EntityFetch<'w>, Access<ComponentId>);
Expand Down Expand Up @@ -1163,6 +1189,9 @@ unsafe impl<'a> QueryData for FilteredEntityMut<'a> {
}
}

/// SAFETY: `entity()` returns the `entity` used to get the `UnsafeEntityCell`, and `fetch` constructs that from `entity`
unsafe impl EntityEquivalentQueryData for FilteredEntityMut<'_> {}

/// SAFETY: `EntityRefExcept` guards access to all components in the bundle `B`
/// and populates `Access` values so that queries that conflict with this access
/// are rejected.
Expand Down Expand Up @@ -1273,6 +1302,9 @@ where
/// components.
unsafe impl<'a, B> ReadOnlyQueryData for EntityRefExcept<'a, B> where B: Bundle {}

/// SAFETY: `entity()` returns the `entity` used to get the `UnsafeEntityCell`, and `fetch` constructs that from `entity`
unsafe impl<B: Bundle> EntityEquivalentQueryData for EntityRefExcept<'_, B> {}

/// SAFETY: `EntityMutExcept` guards access to all components in the bundle `B`
/// and populates `Access` values so that queries that conflict with this access
/// are rejected.
Expand Down Expand Up @@ -1380,6 +1412,9 @@ where
}
}

/// SAFETY: `entity()` returns the `entity` used to get the `UnsafeEntityCell`, and `fetch` constructs that from `entity`
unsafe impl<B: Bundle> EntityEquivalentQueryData for EntityMutExcept<'_, B> {}

/// SAFETY:
/// `update_component_access` does nothing.
/// This is sound because `fetch` does not access components.
Expand Down
Loading