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

Stop automatically generating meta files for assets while using asset processing. #17216

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

andriyDev
Copy link
Contributor

@andriyDev andriyDev commented Jan 7, 2025

Objective

  • Today, enabling asset processing can generate many meta files. This makes it a painful transition for users as they get a "mega commit" containing tons of meta files.

Solution

  • Stop automatically generating meta files! Users can just leave the meta files defaulted.
  • Add a function AssetServer::write_default_meta_file_for_path

Testing

  • Tested this manually on the asset_processing example (by removing the meta files for the assets that had default meta files).
  • I did not add a unit test for the write_default_meta_file_for_path since we don't have an in-memory asset writer. Writing one could be useful in the future.

Showcase

Asset processing no longer automatically generates meta files! This makes it much easier to transition to using asset processing since you don't suddenly get many meta files when turning it on.

You can still manually generate meta files using the new AssetServer::write_default_meta_file_for_path function.

@IQuick143 IQuick143 added A-Assets Load files from disk to use for things like images, models, and sounds M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 7, 2025
Copy link
Contributor

github-actions bot commented Jan 7, 2025

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

@andriyDev
Copy link
Contributor Author

I don't think this PR needs a migration guide since users don't need to change any existing stuff for their assets to work. New assets will also continue to work, though new meta files will have to be created using the provided function or manually created.

@BenjaminBrienen BenjaminBrienen removed the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Jan 7, 2025
@alice-i-cecile alice-i-cecile requested a review from viridia January 7, 2025 18:45
@alice-i-cecile alice-i-cecile added the C-Bug An unexpected or incorrect behavior label Jan 7, 2025
Copy link
Contributor

@viridia viridia left a comment

Choose a reason for hiding this comment

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

Looks fine to me. Do we want to wait for cart to weigh in on this?

@viridia
Copy link
Contributor

viridia commented Jan 7, 2025

In terms of guides, I can write up something once all changes are in and the dust has settled. Not a migration guide so much as a description of the new workflow.

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-Bug An unexpected or incorrect behavior D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants