Skip to content

Weak handle migration #17695

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
merged 2 commits into from
Feb 5, 2025
Merged

Conversation

SludgePhD
Copy link
Contributor

Objective

Solution

  • Migrate bevy from Handle::weak_from_u128 to the new weak_handle! macro that takes a random UUID
  • Deprecate Handle::weak_from_u128, since there are no remaining use cases that can't also be addressed by constructing the type manually

Testing

  • cargo run -p ci -- test

Migration Guide

Replace Handle::weak_from_u128 with weak_handle! and a random UUID.

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Nice! Do we need to note that the UUIDs associated with these handles has changed in the migration guide?

@alice-i-cecile alice-i-cecile added A-Assets Load files from disk to use for things like images, models, and sounds C-Code-Quality A section of code that is hard to understand or change S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Feb 5, 2025
@SludgePhD
Copy link
Contributor Author

Do we need to note that the UUIDs associated with these handles has changed in the migration guide?

I hope that anyone who's been using these internal handles has done so through the constants and not by copying the raw ID, but in case someone hasn't, they should be getting a decent error message about the asset not being found already, so I don't think we need to point this out specifically

@alice-i-cecile alice-i-cecile 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 Feb 5, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Feb 5, 2025
Merged via the queue into bevyengine:main with commit 989f547 Feb 5, 2025
33 checks passed
@SludgePhD SludgePhD deleted the weak-handle-migration branch February 6, 2025 01:14
mrchantey pushed a commit to mrchantey/bevy that referenced this pull request Feb 17, 2025
# Objective

- Make use of the new `weak_handle!` macro added in
bevyengine#17384

## Solution

- Migrate bevy from `Handle::weak_from_u128` to the new `weak_handle!`
macro that takes a random UUID
- Deprecate `Handle::weak_from_u128`, since there are no remaining use
cases that can't also be addressed by constructing the type manually

## Testing

- `cargo run -p ci -- test`

---

## Migration Guide

Replace `Handle::weak_from_u128` with `weak_handle!` and a random UUID.
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-Code-Quality A section of code that is hard to understand or change 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.

3 participants