Skip to content
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

[Merged by Bors] - Rename Handle::as_weak() to cast_weak() #5321

Closed
wants to merge 1 commit into from

Conversation

manokara
Copy link
Contributor

Objective

Following discussion on #3536 and #3522, Handle::as_weak() takes a type U, reinterpreting the handle as of another asset type while keeping the same ID. This is mainly used today in font atlas code. This PR does two things:

  • Rename the method to cast_weak() to make its intent more clear
  • Actually change the type uuid in the handle if it's not an asset path variant.

Migration Guide

  • Rename Handle::as_weak uses to Handle::cast_weak

    The method now properly sets the associated type uuid if the handle is a direct reference (e.g. not a reference to an AssetPath), so adjust you code accordingly if you relied on the previous behavior.

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior 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 labels Jul 15, 2022
@mockersf
Copy link
Member

mockersf commented Jul 16, 2022

I really don't like the name cast_weak but it's what was suggested by Cart in http://github.com/bevyengine/bevy/pull/3536/commits/be09fc0c0e6b27b2023f156aa6bceb17251fc145#r882205748

@alice-i-cecile alice-i-cecile requested review from cart and james7132 July 16, 2022 18:21
@colepoirier
Copy link
Contributor

I don’t really understand this code so I don’t think I’m qualified to review it.

@james7132 james7132 removed the request for review from colepoirier October 28, 2022 21:48
Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think this API is a bad code smell. We should try to get rid of it's primary uses and migrate to a better alternative.

For now, this is a good solution to prevent further misuse. The code looks good to me.

@james7132 james7132 added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Oct 28, 2022
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Oct 28, 2022
# Objective

Following discussion on #3536 and #3522, `Handle::as_weak()` takes a type `U`, reinterpreting the handle as of another asset type while keeping the same ID. This is mainly used today in font atlas code. This PR does two things:

- Rename the method to `cast_weak()` to make its intent more clear
- Actually change the type uuid in the handle if it's not an asset path variant.

## Migration Guide

- Rename `Handle::as_weak` uses to `Handle::cast_weak`

    The method now properly sets the associated type uuid if the handle is a direct reference (e.g. not a reference to an `AssetPath`), so adjust you code accordingly if you relied on the previous behavior.
@bors bors bot changed the title Rename Handle::as_weak() to cast_weak() [Merged by Bors] - Rename Handle::as_weak() to cast_weak() Oct 28, 2022
@bors bors bot closed this Oct 28, 2022
@rparrett rparrett added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Oct 28, 2022
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

Following discussion on bevyengine#3536 and bevyengine#3522, `Handle::as_weak()` takes a type `U`, reinterpreting the handle as of another asset type while keeping the same ID. This is mainly used today in font atlas code. This PR does two things:

- Rename the method to `cast_weak()` to make its intent more clear
- Actually change the type uuid in the handle if it's not an asset path variant.

## Migration Guide

- Rename `Handle::as_weak` uses to `Handle::cast_weak`

    The method now properly sets the associated type uuid if the handle is a direct reference (e.g. not a reference to an `AssetPath`), so adjust you code accordingly if you relied on the previous behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior C-Usability A targeted quality-of-life change that makes Bevy easier to use 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants