Add simple Disabled marker #17514
Merged
Add simple Disabled marker #17514
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BenjaminBrienen
added
C-Feature
|
I think that this should close that issue. Would you mind fixing the description? :) |
MrGVSV
reviewed
Jan 23, 2025
|
|
||
| let disabled = self.register_component::<Disabled>(); | ||
| let mut filters = DefaultQueryFilters::default(); | ||
| filters.set_disabled(disabled); |
There was a problem hiding this comment.
I'm a bit worried that users might do a
app.init_resource::<DefaultQueryFilters>()
and lose the Disabled functionality inadvertently..
There was a problem hiding this comment.
init_resource doesn't overwrite existing resources. You'd have to explicitly call .insert_resource(DefaultQueryFilters::default()) which is probably verbose enough to make it obvious what will happen. Currently it is intended that you can opt-out of entity disabling, but as the feature matures we could consider making it so that it is always there.
chescock
approved these changes
Jan 28, 2025
|
|
||
| /// A marker component for disabled entities. See [the module docs] for more info. | ||
| /// | ||
| /// [the module docs]: crate::entity_disabling |
There was a problem hiding this comment.
Should we update the module docs to mention that the Disabled component is disabled in the default DefaultQueryFilters? Right now they talk about it as a hypothetical.
| let original_world_dqf_id = self | ||
| .original_world | ||
| .components() | ||
| .get_resource_id(TypeId::of::<DefaultQueryFilters>()); |
There was a problem hiding this comment.
It will be nice when resources-as-components lets us use default query filters to exclude the default query filters resource from our resource query :).
cBournhonesque
approved these changes
Jan 30, 2025
MrGVSV
added
S-Ready-For-Final-Review
mrchantey
pushed a commit
to mrchantey/bevy
that referenced
this pull request
Feb 4, 2025
# Objective We have default query filters now, but there is no first-party marker for entity disabling yet Fixes bevyengine#17458 ## Solution Add the marker, cool recursive features and/or potential hook changes should be follow up work ## Testing Added a unit test to check that the new marker is enabled by default
joshua-holmes
pushed a commit
to joshua-holmes/bevy
that referenced
this pull request
Feb 5, 2025
# Objective We have default query filters now, but there is no first-party marker for entity disabling yet Fixes bevyengine#17458 ## Solution Add the marker, cool recursive features and/or potential hook changes should be follow up work ## Testing Added a unit test to check that the new marker is enabled by default
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 9, 2025
# Objective The entity disabling / default query filter work added in #17514 and #13120 is neat, but we don't teach users how it works! We should fix that before 0.16. ## Solution Write a simple example to teach the basics of entity disabling! ## Testing `cargo run --example entity_disabling` ## Showcase  --------- Co-authored-by: Zachary Harrold <[email protected]>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 11, 2025
…y filters (#17768) # Objective Currently, default query filters, as added in #13120 / #17514 are hardcoded to only use a single query filter. This is limiting, as multiple distinct disabling components can serve important distinct roles. I ran into this limitation when experimenting with a workflow for prefabs, which don't represent the same state as "an entity which is temporarily nonfunctional". ## Solution 1. Change `DefaultQueryFilters` to store a SmallVec of ComponentId, rather than an Option. 2. Expose methods on `DefaultQueryFilters`, `World` and `App` to actually configure this. 3. While we're here, improve the docs, write some tests, make use of FromWorld and make some method names more descriptive. ## Follow-up I'm not convinced that supporting sparse set disabling components is useful, given the hit to iteration performance and runtime checks incurred. That's disjoint from this PR though, so I'm not doing it here. The existing warnings are fine for now. ## Testing I've added both a doc test and an mid-level unit test to verify that this works!
mrchantey
pushed a commit
to mrchantey/bevy
that referenced
this pull request
Feb 17, 2025
# Objective The entity disabling / default query filter work added in bevyengine#17514 and bevyengine#13120 is neat, but we don't teach users how it works! We should fix that before 0.16. ## Solution Write a simple example to teach the basics of entity disabling! ## Testing `cargo run --example entity_disabling` ## Showcase  --------- Co-authored-by: Zachary Harrold <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
We have default query filters now, but there is no first-party marker for entity disabling yet
Fixes #17458
Solution
Add the marker, cool recursive features and/or potential hook changes should be follow up work
Testing
Added a unit test to check that the new marker is enabled by default