Skip to content

Replace UntypedHandle from ReflectAsset with impl Into<UntypedAssetId>. #19606

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 15, 2025
Merged

Replace UntypedHandle from ReflectAsset with impl Into<UntypedAssetId>. #19606

merged 3 commits into from
Jun 15, 2025

Conversation

andriyDev
Copy link
Contributor

@andriyDev andriyDev commented Jun 13, 2025 •
edited
Loading

Objective

  • A step towards "Always Strong" Asset Handles / Remove asset weak handles #19024.
  • Allow ReflectAsset to work with any AssetId not just Handle.
  • ReflectAsset::ids() returns an iterator of AssetIds, but then there's no way to use these ids, since all the other APIs in ReflectAsset require a handle (and we don't have a reflect way to get the handle).

Solution

  • Replace the UntypedHandle argument in ReflectAsset methods with impl Into<UntypedAssetId>.
  • This matches the regular asset API.
  • This allows ReflectAsset::ids() to be more useful.

Testing

  • None.

@alice-i-cecile alice-i-cecile added A-Assets Load files from disk to use for things like images, models, and sounds C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Reflection Runtime information about types X-Uncontroversial This work is generally agreed upon S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 13, 2025
@alice-i-cecile alice-i-cecile added M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Jun 13, 2025
@alice-i-cecile
Copy link
Member

Sensible choice, and very nice migration guide.

@andriyDev andriyDev added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 15, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 15, 2025
Merged via the queue into bevyengine:main with commit 98c14e5 Jun 15, 2025
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@Austreelis Austreelis Austreelis approved these changes

Assignees
No one assigned
Labels
A-Assets Load files from disk to use for things like images, models, and sounds A-Reflection Runtime information about types C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andriyDev @alice-i-cecile @Austreelis