-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[Merged by Bors] - add ReflectAsset
and ReflectHandle
#5923
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
Closed
Closed
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
266f7b6
add ReflectAsset and ReflectHandle
jakobhellermann de346a7
add register_asset_reflect function
jakobhellermann 443e816
register assets as reflect: StandardMaterial, ColorMaterial, Animatio…
jakobhellermann b46db74
add len, ids and remove
jakobhellermann de862d8
make Image reflect_value and register it as reflect asset
jakobhellermann 26d5c61
tweak wording
jakobhellermann 467d1c2
add a test
jakobhellermann cf44ca7
add is_empty
jakobhellermann ab4822a
silence clippy
jakobhellermann b2af704
expand docs
jakobhellermann d21c2f5
add handle type id to ReflectAsset as well
jakobhellermann d51bb6c
register texture atlas as reflect asset
jakobhellermann f862b37
add typed method
jakobhellermann c511ba7
Apply suggestions from code review
jakobhellermann e4e93ee
fix warnings and errors when compiling bevy_app with default_features…
jakobhellermann 592e136
more review feedback
jakobhellermann 48eb298
reflect(Debug)
jakobhellermann 51697dd
fix test
jakobhellermann 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
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.
I wonder if this should also perform the
add_asset
call internally. Then users only need to write one or the other, not both. In that case, I might also recommend bikeshedding this toadd_reflectable_asset
or something similar.Any thoughts on this?
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.
Hm. I think my preference is to have these two methods separately, because then you can search for
add_asset
globally and easily find all asset registration. Registering reflect also feels like something that is done "additionally", not like a completely different way of adding an asset.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.
Ah, true. I guess it also means you can quickly comment out the line if you need to test it without the reflection. Save on those keystrokes haha
I think I lean more towards keeping it as is now, but if anyone else has any thoughts, I'd love to hear it!