Skip to content

Remove the Component trait implementation from Handle #15796

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 6 commits into from
Oct 9, 2024

Conversation

tim-blackbird
Copy link
Contributor

@tim-blackbird tim-blackbird commented Oct 9, 2024

Objective

Solution

  • Replace Handle<MeshletMesh> with a new MeshletMesh3d component
  • As expected there were some random things that needed fixing:
    • A couple tests were storing handles just to prevent them from being dropped I believe, which seems to have been unnecessary in some.
    • The SpriteBundle still had a Handle<Image> field. I've removed this.
    • Tests in bevy_sprite incorrectly added a Handle<Image> field outside of the Sprite component.
    • A few examples were still inserting Handles, switched those to their corresponding wrappers.
    • 2 examples that were still querying for Handle<Image> were changed to query Sprite

Testing

  • I've verified that the changed examples work now

Migration Guide

Handle can no longer be used as a Component. All existing Bevy types using this pattern have been wrapped in their own semantically meaningful type. You should do the same for any custom Handle components your project needs.

The Handle<MeshletMesh> component is now MeshletMesh3d.

The WithMeshletMesh type alias has been removed. Use With<MeshletMesh3d> instead.

@tim-blackbird tim-blackbird added A-Cross-Cutting Impacts the entire engine D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 9, 2024
@tim-blackbird tim-blackbird added this to the 0.15 milestone Oct 9, 2024
@alice-i-cecile alice-i-cecile added the X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers label Oct 9, 2024
@alice-i-cecile
Copy link
Member

I've extended the migration guide for you :)

@JMS55
Copy link
Contributor

JMS55 commented Oct 9, 2024

Thanks for doing this! Small nit, can we remove the WithMeshletMesh type alias? I never liked it to begin with, and it's not even that complex now that handle is gone. Feels uneeded.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 9, 2024
Merged via the queue into bevyengine:main with commit 3da0ef0 Oct 9, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Cross-Cutting Impacts the entire engine D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stop using Handle<MeshletMesh> as a component Remove Component trait impl from Handle<T>
4 participants