-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Reflect auto registration #15030
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
Open
eugineerd
wants to merge
66
commits into
bevyengine:main
Choose a base branch
from
eugineerd:reflect-auto-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.
Open
Reflect auto registration #15030
Changes from all commits
Commits
Show all changes
66 commits
Select commit
Hold shift + click to select a range
4b48e23
minimal example auto registration for reflect types
eugineerd 89c52dd
implement auto registration for the rest of supported `#[derive(Refle…
eugineerd f3614b8
add `no_auto_register` reflect attribute to allow opting out of autom…
eugineerd 7367345
added `reflect_auto_register` feature to allow enabling/disabling aut…
eugineerd 16ee7cf
reduce wasm overhead
eugineerd 35f0591
run `cargo run -p build-templated-pages -- update features`
eugineerd 119a598
fix not registering `TypeData`
eugineerd 53696f7
add doc test and remove feature-gating
eugineerd 7d761e1
remove `dbg!`
eugineerd d8eb597
update examples for automatic reflect type registration
eugineerd 1da577a
fix typo
eugineerd 4485798
remove outdated comment from reflect Cargo.toml
eugineerd ef6bb2d
remove needless borrow
eugineerd 5fa1572
fix from check-doc
eugineerd d2d3290
move calling automatic type registration to `AppTypeRegistry`.
eugineerd 878e8d3
more clippy fixes
eugineerd b518d5e
Move automatic types registration to app creation
eugineerd edf5f87
revert changes to examples that use `TypeRegistry`
eugineerd e9f67f0
hide automatic reflect registration in `__macro_exports`
eugineerd 301b60a
add note about `no_auto_register` to `Reflect` derive doc
eugineerd 6ad0a23
made `inventory` and `wasm-init` platform specific deps
eugineerd 1279d62
tried to abstract platform-dependent code away
eugineerd 74337f7
update `bevy_reflect`'s module-level doc's "Manual Registration" section
eugineerd 7723de5
Merge remote-tracking branch 'upstream/main' into reflect-auto-regist…
eugineerd b18b645
Merge remote-tracking branch 'upstream/main' into reflect-auto-regist…
eugineerd 257af97
Merge branch 'main' into reflect-auto-registration
eugineerd 943dde9
added test for ignored auto reflect registration
eugineerd 36f49f0
clippy
eugineerd e50610b
apply suggested doc fix
eugineerd b0f2438
implement reflect auto register for opaque types
eugineerd 2ae4bb9
add test for auto reflect registration on all supported types
eugineerd 7c7bd79
put automatic type registration behind feature gate
eugineerd 03ce543
add reason to `no_auto_register` allow
eugineerd 67f4d02
use `impl_is_generic` instead of converting to token stream.
eugineerd 1548654
Merge remote-tracking branch 'upstream/main' into reflect-auto-regist…
eugineerd 69bbc17
fix missing `[package]` in `bevy_reflect/Cargo.toml` (How did that ev…
eugineerd e82d33c
Merge branch 'main' into reflect-auto-registration
eugineerd 58b847c
add `no_std` support on wasm
eugineerd 95593d1
feature-gate `no_auto_register` attribute getter
eugineerd e18aaef
Merge branch 'main' of https://github.com/bevyengine/bevy into reflec…
eugineerd 42db5e0
Merge branch 'main' of https://github.com/bevyengine/bevy into reflec…
eugineerd 1bb9ee8
remove `wasm-init` and just use `inventory` instead
eugineerd c6be4fa
Merge branch 'main' of https://github.com/bevyengine/bevy into reflec…
eugineerd 7756f4b
Merge branch 'main' of https://github.com/bevyengine/bevy into reflec…
eugineerd 2eb5d1b
remove empty enum from tests
eugineerd 1448ee6
Disable auto registration by default
eugineerd 54ac872
update features
eugineerd 098cff9
Revert "remove empty enum from tests"
eugineerd eb66a7b
Merge branch 'main' into reflect-auto-registration-staticlib
eugineerd 21a5ba0
implement automatic type registrations without relying on `inventory`
eugineerd fa60c51
add an example for static automatic registration
eugineerd 2958700
Add `auto_register_inventory` feature to `bevy_reflect`
eugineerd 2433fd6
example types fixes
eugineerd 4d2a9c3
Merge branch 'main' of https://github.com/bevyengine/bevy into reflec…
eugineerd ff78027
fix `auto_register_static` example
eugineerd 8ba6dad
enable `auto_register_inventory` by default for `bevy_reflect`
eugineerd da7768b
update examples readme
eugineerd 22e6f27
fix markdownlint errors
eugineerd 9671223
typos
eugineerd ded6b15
Merge branch 'main' of https://github.com/bevyengine/bevy into reflec…
eugineerd e257d75
fix trailing spaces
eugineerd 7c60aa8
fix `wasm_support` path
eugineerd 202e59f
use proper calling convention for extern fns
eugineerd d6a254f
remove some unsafe from `auto_register_static`
eugineerd 567683a
add release notes
eugineerd b49063d
markdown lints
eugineerd 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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
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 being enabled by default means that we are unlikely to catch new types that we forget to register. I am certain that over time Bevy's type registry completeness would degrade.
Likewise, plugin authors will be insulated from this by default, so I suspect the ecosystem would effectively just not care about this. This isn't the kind of problem you can solve by adding a "please register your types manually" to the "Bevy plugin guidelines" doc. This problem will be hidden from pretty much everyone, and I suspect most people that are aware will choose the path of least resistance.
Given that, I am uncomfortable backing us into this corner when we don't know how this approach will fare on future platforms. The entire ecosystem falling apart when we add, say, PS5 support doesn't feel stomach-able to me. Related: would this throw a wrench in @bushrat011899's no_std / niche platform work? (as in, does auto-registration work on the platforms targeted by the current no_std work, and therefore would the "ecosystem and internal Bevy breakage problem" apply there)
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.
Re.
no_std
:inventory
relies on linker tricks which are added on a platform-by-platform basis in theinventory
crate. That is, there's no way for an end user on an unsupported platform to add that support themselves, short of upstream efforts ininventory
. At least, that's my understanding of the crate.As for what platforms
inventory
currently supports, it appears to be WebAssembly, and most operating systems. I don't see any support for no-OS platforms like embedded or retro consoles.That's fine IMO, because not every Bevy feature needs to work on embedded. But as Cart points out, this does have the potential side effect of making libraries "lazy", breaking their reflection support on
no_std
or just not bothering withno_std
at all.Personally, I would like this feature to be included, but as an option. It's a lot easier to recommend users making a game enable this feature than it is to recommend library authors disable it.
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.
Removed
reflect_auto_register
from default features and reverted documentation and examples changes using it.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.
Added
static
approach as a fallback for platforms that aren't supported byinventory
, this should be fine to enable by default now.