Skip to content

Add Allows filter to bypass DefaultQueryFilters #18192

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
merged 1 commit into from
May 5, 2025

Conversation

NiseVoid
Copy link
Contributor

@NiseVoid NiseVoid commented Mar 7, 2025

Objective

Fixes #17803

Solution

  • Add an Allows<T> QueryFilter that adds archetypal access for T
  • Fix access merging to include archetypal from both sides

Testing

  • Added a case to the unit test for the application of DefaultQueryFilters

@NiseVoid NiseVoid added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples D-Unsafe Touches with unsafe code in some way labels Mar 7, 2025
@@ -2232,6 +2232,10 @@ mod tests {
let mut query = QueryState::<Has<C>>::new(&mut world);
assert_eq!(3, query.iter(&world).count());

// Allows should bypass the filter entirely
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to write or update a test or example explaining why allows is different than has?

Copy link
Contributor

@Carter0 Carter0 left a comment

Choose a reason for hiding this comment

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

Could use an example, but I think the implementation is good!

@chescock
Copy link
Contributor

I don't think this use of "archetypal" access matches the original intent. (Although I don't know whether anyone is actually using it for its original purpose!)

From #11700,

It would be useful to be able to inspect a QueryState's accesses so we can detect when the data it accesses changes without having to iterate it.

Add the notion of "archetypal" accesses, which are not accessed directly, but whose presence in an archetype affects a query result.

Allows<T> is sort of the opposite, as it means T no longer affects the query result, even though it does by default on other queries.

I'd gently vote for making this a new kind of access. If we do want to use "archetypal" for this, then we should probably update the doc comment to say something other than "whose presence in an archetype may affect query results".

@alice-i-cecile alice-i-cecile added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Mar 18, 2025
@alice-i-cecile alice-i-cecile added this to the 0.17 milestone Mar 28, 2025
@alice-i-cecile
Copy link
Member

I'd gently vote for making this a new kind of access. If we do want to use "archetypal" for this, then we should probably update the doc comment to say something other than "whose presence in an archetype may affect query results".

I'm sympathetic to this, but I'm reluctant to block this particular PR over it, or to add more complexity for a very niche feature currently.

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 30, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 5, 2025
Merged via the queue into bevyengine:main with commit 02d569d May 5, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples D-Unsafe Touches with unsafe code in some way S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a Includes query filter for working with disabling components
4 participants