bevy_reflect: Use active_types instead of active_fields where appropriate
#19922
+13
−19
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
This could have been part of bevyengine#19876, which deduplicated the fields. It's also simpler and more efficient to keep the (short-lived) active types as an `IndexSet<Type>`, and avoid converting them to a `Vec<Type>` and then a `Box<[Type]>`.
Instead of `active_fields`. This makes it match `ReflectStruct`, and avoids redundant calls within the generated `register_type_dependencies` when the same type is used in multiple enum variant fields.
nnethercote
changed the title
active_types instead of active_fields where appropriate
|
Best reviewed one commit at a time. It looks like this repo might squash-and-rebase to land PRs? I often do multi-commit PRs so I'll give it a try here, see how it pans out. |
tychedelia
added
C-Performance
|
Yep, we use squash-on-merge, so the multi-commit approach is really helpful here. |
alice-i-cecile
approved these changes
Jul 2, 2025
It's a shame that squashing loses the separation in the history. It also decreases the value of bisection, if bugs are introduced. |
nnethercote
changed the title
active_types instead of active_fields where appropriateactive_types instead of active_fields where appropriate
|
Indeed :) However, it is much easier on novice contributors, and it allows us to ensure that every commit in the history compiles via CI checks, allowing for easier bisection and examination of history. |
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
Follow on fixes for #19876, using
active_typesinstead ofactive_fieldswhere appropriate.Helps a tiny bit with #19873.
Solution
Testing
I checked the output of
cargo expandand rancargo run -p ci.