-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Automatic registration of ECS types #12332
Open
james7132
wants to merge
31
commits into
bevyengine:main
Choose a base branch
from
james7132:automatic-ecs-registration
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+264
−104
Open
Changes from 30 commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
b14896a
Automatically register Component types on initialization
james7132 3d5f60d
Make AppTypeRegistry use the World internal one
james7132 5a9c04b
Merge branch 'main' into automatic-ecs-registration
james7132 fc1b308
Fix dynamic_scene_builder tests
james7132 9c388a5
Formatting
james7132 91354fd
Fix typo.
james7132 c19bda8
Cleanup proc macro code
james7132 cc7c253
Target reflect(Component) instead
james7132 51a2f08
Support automatically registering resources
james7132 b615535
Fix up tests
james7132 512593e
Cleanup + disable generics
james7132 20dde15
Minimize what we're reexporting from the private module
james7132 5fdea45
Merge branch 'main' into automatic-ecs-registration
james7132 3e1b0b1
Fix tests
james7132 ccd8405
Fix deadlock by not panicking if already registered.
james7132 20e18ac
Fix tests
james7132 3013029
Fix more tests
james7132 01725d9
Merge branch 'main' into automatic-ecs-registration
james7132 4558439
Remove unnecessary registrations
james7132 5aa2fa9
Cleanup generated code with a shim
james7132 a8950a6
Cleanup has_reflect_attr
james7132 47ef511
Cleanup macro logic
james7132 54809cf
More cleanup
james7132 8e44f11
then_some
james7132 4f07b1f
Update example for scenes
james7132 9a42c8b
Provide crate level docs
james7132 89bddcb
AppTypeRegistry is already in bevy_ecs
james7132 807f947
Make AppTypeRegistry only constructable via FromWorld
james7132 000f939
Add registration test for resources
james7132 91ad713
Fix tests
james7132 9fda301
Merge branch 'main' into automatic-ecs-registration
james7132 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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 file contains 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 file contains 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 file contains 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 file contains 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 file contains 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 file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This requirement is so that we don't automatically register to the World non-ECS-related Reflect types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was requested so that users could opt out of this, such as avoiding serialization in scenes. I personally think we should find other ways of opting out of each use case and just move towards registration by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also consider an opt-out attribute. There could be other reasons to exclude a type from the registry besides opting out of scenes. For larger projects, you may only want to register types you know you’ll need (to save on memory and reduce startup times). You might also be using a third party crate that uses the registry for things you want to opt out of as well. Or maybe you don't want to "leak" type information to downstream crates.
Whatever the reason, we need a way to opt out of automatic registration since registration is intentionally a one-way process: you can't undo or remove a registration. This is so that guarantees about the registered type are never broken without the user being aware of it (unless, of course, a mischievous plugin goes nuclear and replaces the entire
AppTypeRegistry
resource but that's a whole separate issue haha).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for the opt-out attribute. It's more explicit, and allows this to work even when the user forgets to reflect
Component
/Resource