Isolate component registration#17671
Merged
alice-i-cecile merged 16 commits intobevyengine:mainfrom Feb 5, 2025
Merged
Conversation
This was moved over from the original PR, and I copied this change in the wrong place.
SpecificProtagonist
approved these changes
Feb 4, 2025
Contributor
SpecificProtagonist
left a comment
SpecificProtagonist
left a comment
There was a problem hiding this comment.
Makes sense to me, and the unsafe code is still sound (as far as I can tell).
Note that SparseSets are no longer present for all registered sparse set components, only those that have been spawned.
Maybe also add a comment to that effect on Storages.sparse_sets?
Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net>
…lliottjPierce/bevy into isolate-component-registration
Contributor
Author
Done! Thanks @SpecificProtagonist for the review and suggestions. |
Github doesn't run fmt or course for commits from a review.
chescock
approved these changes
Feb 4, 2025
Contributor
chescock
left a comment
chescock
left a comment
There was a problem hiding this comment.
Looks good! Thanks for pulling this out of the larger PR!
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
…lliottjPierce/bevy into isolate-component-registration
chescock
added
A-ECS
cart
reviewed
Feb 4, 2025
per Cart's suggestion.
These were fixes from changes caused by bevyengine#17679
Member
|
Good stuff; I think this is a worthwhile cleanup regardless of the final design choice. |
mrchantey
pushed a commit
to mrchantey/bevy
that referenced
this pull request
Feb 17, 2025
# Objective Progresses bevyengine#17569. The end goal here is to synchronize component registration. See the other PR for details for the motivation behind that. For this PR specifically, the objective is to decouple `Components` from `Storages`. What components are registered etc should have nothing to do with what Storages looks like. Storages should only care about what entity archetypes have been spawned. ## Solution Previously, this was used to create sparse sets for relevant components when those components were registered. Now, we do that when the component is inserted/spawned. This PR proposes doing that in `BundleInfo::new`, but there may be a better place. ## Testing In theory, this shouldn't have changed any functionality, so no new tests were created. I'm not aware of any examples that make heavy use of sparse set components either. ## Migration Guide - Remove storages from functions where it is no longer needed. - Note that SparseSets are no longer present for all registered sparse set components, only those that have been spawned. --------- Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net> Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
This was referenced Mar 4, 2025
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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
Progresses #17569. The end goal here is to synchronize component registration. See the other PR for details for the motivation behind that.
For this PR specifically, the objective is to decouple
ComponentsfromStorages. What components are registered etc should have nothing to do with what Storages looks like. Storages should only care about what entity archetypes have been spawned.Solution
Previously, this was used to create sparse sets for relevant components when those components were registered. Now, we do that when the component is inserted/spawned.
This PR proposes doing that in
BundleInfo::new, but there may be a better place.Testing
In theory, this shouldn't have changed any functionality, so no new tests were created. I'm not aware of any examples that make heavy use of sparse set components either.
Migration Guide