Skip to content

Remove the Component trait implementation from Handle#15796

Merged
alice-i-cecile merged 6 commits intobevyengine:mainfrom
tim-blackbird:yeet-handles
Oct 9, 2024
Merged

Remove the Component trait implementation from Handle#15796
alice-i-cecile merged 6 commits intobevyengine:mainfrom
tim-blackbird:yeet-handles

Conversation

@tim-blackbird
Copy link
Copy Markdown
Contributor

@tim-blackbird tim-blackbird commented Oct 9, 2024 •
edited
Loading

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
Copy Markdown
Member

alice-i-cecile commented Oct 9, 2024

I've extended the migration guide for you :)

@JMS55
Copy link
Copy Markdown
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
@bushrat011899 bushrat011899 mentioned this pull request Oct 9, 2024
Closed
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

+1 more reviewer

@NiseVoid NiseVoid NiseVoid approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

0.15

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

@tim-blackbird @alice-i-cecile @JMS55 @NiseVoid