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.

Sign up for GitHub

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

Jump to bottom

Remove the meta field from LoadedAsset and ErasedLoadedAsset. #15487

Merged
merged 1 commit into from
Dec 13, 2024
Merged

Remove the meta field from LoadedAsset and ErasedLoadedAsset. #15487

merged 1 commit into from
Dec 13, 2024

Conversation

andriyDev
Copy link
Contributor

Objective

Fixes #15485.

Solution

Deletes the field! The meta field had no way to access or mutate it.

Testing

  • It builds!

Migration Guide

  • ErasedAssetLoader now takes a borrow to AssetMetaDyn instead of a Box.
  • LoadedAsset::new_with_dependencies no longer requires a meta argument.
  • LoadContext::finish no longer requires a meta argument.

@alice-i-cecile alice-i-cecile requested a review from cart September 28, 2024 01:30
@BenjaminBrienen BenjaminBrienen added A-Assets Load files from disk to use for things like images, models, and sounds X-Contentious There are nontrivial implications that should be thought through D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 28, 2024
@andriyDev
Copy link
Contributor Author

No change. Just had to resolve a conflict where two changes lines are right next to each other.

cart
cart approved these changes Dec 13, 2024
View reviewed changes
Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

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

Iirc I had a future reason to include this, but we can always add it back if we need it.

@cart cart added this pull request to the merge queue Dec 13, 2024
Merged via the queue into bevyengine:main with commit 4ba47ed Dec 13, 2024
28 checks passed
This was referenced Dec 14, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenjaminBrienen BenjaminBrienen BenjaminBrienen approved these changes

@cart cart cart approved these changes

Assignees
No one assigned
Labels
A-Assets Load files from disk to use for things like images, models, and sounds D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove LoadedAsset::meta
3 participants
@andriyDev @cart @BenjaminBrienen