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] - make Handle::<T> field id private, and replace with a getter #6176

Closed
wants to merge 1 commit into from

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented Oct 5, 2022

Objective

Solution

  • Make the field private, and add a public getter

Opened after discussion in #6171. Pinging @zicklag


Migration Guide

  • If you were accessing the value handle.id, you can now do so with handle.id()

@mockersf mockersf added the A-Assets Load files from disk to use for things like images, models, and sounds label Oct 5, 2022
@mockersf mockersf added the C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide label Oct 5, 2022
@alice-i-cecile alice-i-cecile 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 6, 2022
@alice-i-cecile
Copy link
Member

bors r+

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

- Field `id` of `Handle<T>` is public: https://docs.rs/bevy/latest/bevy/asset/struct.Handle.html#structfield.id
- Changing the value of this field doesn't make sense as it could mean changing the previous handle without dropping it, breaking asset cleanup detection for the old handle and the new one

## Solution

- Make the field private, and add a public getter


Opened after discussion in #6171. Pinging @zicklag 

---

## Migration Guide

- If you were accessing the value `handle.id`, you can now do so with `handle.id()`
@bors
Copy link
Contributor

bors bot commented Oct 6, 2022

@bors bors bot changed the title make Handle::<T> field id private, and replace with a getter [Merged by Bors] - make Handle::<T> field id private, and replace with a getter Oct 6, 2022
@bors bors bot closed this Oct 6, 2022
@mockersf mockersf added the hacktoberfest-accepted A PR that was accepted for Hacktoberfest, an annual open source event label Oct 6, 2022
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 19, 2022
…ine#6176)

# Objective

- Field `id` of `Handle<T>` is public: https://docs.rs/bevy/latest/bevy/asset/struct.Handle.html#structfield.id
- Changing the value of this field doesn't make sense as it could mean changing the previous handle without dropping it, breaking asset cleanup detection for the old handle and the new one

## Solution

- Make the field private, and add a public getter


Opened after discussion in bevyengine#6171. Pinging @zicklag 

---

## Migration Guide

- If you were accessing the value `handle.id`, you can now do so with `handle.id()`
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
…ine#6176)

# Objective

- Field `id` of `Handle<T>` is public: https://docs.rs/bevy/latest/bevy/asset/struct.Handle.html#structfield.id
- Changing the value of this field doesn't make sense as it could mean changing the previous handle without dropping it, breaking asset cleanup detection for the old handle and the new one

## Solution

- Make the field private, and add a public getter


Opened after discussion in bevyengine#6171. Pinging @zicklag 

---

## Migration Guide

- If you were accessing the value `handle.id`, you can now do so with `handle.id()`
neocturne added a commit to neocturne/bevy that referenced this pull request Jan 2, 2023
# Objective

It is currently possible to break reference counting for assets by creating
a strong HandleUntyped and then modifying the `id` field before dropping
the handle. This should not be allowed.

## Solution

Change the `id` field visibility to private and add a getter instead.
The same change was previously done for Handle<T> in bevyengine#6176, but
HandleUntyped<T> was forgotten.
bors bot pushed a commit that referenced this pull request Jan 4, 2023
# Objective

It is currently possible to break reference counting for assets by creating a strong `HandleUntyped` and then modifying the `id` field before dropping the handle. This should not be allowed.

## Solution

Change the `id` field visibility to private and add a getter instead. The same change was previously done for `Handle<T>` in #6176, but `HandleUntyped` was forgotten.

---

## Migration Guide

- Instead of directly accessing the ID of a `HandleUntyped` as `handle.id`, use the new getter `handle.id()`.
alradish pushed a commit to alradish/bevy that referenced this pull request Jan 22, 2023
# Objective

It is currently possible to break reference counting for assets by creating a strong `HandleUntyped` and then modifying the `id` field before dropping the handle. This should not be allowed.

## Solution

Change the `id` field visibility to private and add a getter instead. The same change was previously done for `Handle<T>` in bevyengine#6176, but `HandleUntyped` was forgotten.

---

## Migration Guide

- Instead of directly accessing the ID of a `HandleUntyped` as `handle.id`, use the new getter `handle.id()`.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
…ine#6176)

# Objective

- Field `id` of `Handle<T>` is public: https://docs.rs/bevy/latest/bevy/asset/struct.Handle.html#structfield.id
- Changing the value of this field doesn't make sense as it could mean changing the previous handle without dropping it, breaking asset cleanup detection for the old handle and the new one

## Solution

- Make the field private, and add a public getter


Opened after discussion in bevyengine#6171. Pinging @zicklag 

---

## Migration Guide

- If you were accessing the value `handle.id`, you can now do so with `handle.id()`
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

It is currently possible to break reference counting for assets by creating a strong `HandleUntyped` and then modifying the `id` field before dropping the handle. This should not be allowed.

## Solution

Change the `id` field visibility to private and add a getter instead. The same change was previously done for `Handle<T>` in bevyengine#6176, but `HandleUntyped` was forgotten.

---

## Migration Guide

- Instead of directly accessing the ID of a `HandleUntyped` as `handle.id`, use the new getter `handle.id()`.
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-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide hacktoberfest-accepted A PR that was accepted for Hacktoberfest, an annual open source event 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.

None yet

5 participants