Allow using short type names for asset processors.

[andriyDev]

Objective

• Previously, asset processors needed to be fully specified. This meant writing met files by hand was incredibly cumbersome.
• In addition, using core::any::type_name is generally undesirable since it is only intended for debugging.

Solution

• Require TypePath impl on AssetLoader, AssetTransformer, AssetSaver, and AssetProcessor.
• Register loaders and processors by their TypePath::type_path. This makes the path format stable.
• Additionally register the processors by their TypePath::short_type_path, and ensure there are no other processors with the same short_type_path. If a user tries to use an ambiguous short type path, we list out all full type paths.

Note: I left doing this with asset loaders as a future PR. There's more complexity there (since we have to deal with pre-registering asset loaders), and this seems like the bigger "bang for our buck".

Testing

• I ran the asset_processing example. No change.
• I changed the asset processing example to use the short type path. It still behaved as expected.
• I created a new AssetTransformer in asset_processing::sneaky::CoolTextTransformer and a new processor that looked identical to the regular one (but using the sneaky transformer). It printed an error, listing out the full paths of both processors!

[github-actions]

It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note.

Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes.

[github-actions]

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

Please review the instructions for writing migration guides, then expand or revise the content in the migration guides directory to reflect your changes.

[JMS55]

Code lgtm!

Please add a migration guide for the TypePath changes, and a release note entry showcasing this work.

Also, while I won't block on it, it would be great to have a unit test for ambiguous processor names. I'm worried that that is easy to break in future code changes.

[JMS55]

Also check the CI jobs. Looks like you need to add the TypePath derive to some more stuff.

[andriyDev]

Fixed up CI, added the migration guide + release notes, and also added tests for getting the processor by name. We definitely need more testing around asset processing itself (I'm only testing that getting the processor by name all works), but that should be a different PR.

[ChristopherBiscardi]

I think this is the first Bevy feature that will allow previously written assets (I guess .meta files in this case) to break when installing a new plugin. (ex: if that plugin also includes an ImageLoader, or CoolTextLoader in the example here). So the general guidance will probably become "namespace your loaders" like MyCrateImageLoader since this is now a global namespace, which doesn't really change the amount of handwritten code. We're moving the namespacing from the module path to a human-curated prefix.

This may also be introducing a difference in behavior between other types and loaders, ex: the scenes example asset currently requires full type paths for components, Resources, etc

I don't have a strong feeling either way (slight preference for adding new plugins not being able to break current applications), but I do see "why can I wrote loaders with shortnames but not components" coming up and we should have an answer for it.

[hukasu]

Deriving TypePath should already namespace the type, no?

[andriyDev]

Deriving TypePath should already namespace the type, no?

Only if you use long type names. If you use short type names, then there's a risk that something like ImageLoader may be aliased by some other plugin that adds its own ImageLoader. So if you write your meta file to point to ImageLoader, it's more at risk of failing with an error, but if you name your loader MyExtraSpecialImageLoader, there's a smaller risk.

I still think this is a win. I don't think it's desirable for users to have to specify the module path since that can include info that doesn't matter to them, like LoadTransformAndSave is stored in bevy_asset::processor.

[hukasu]

ah, so the comment was about namespacing it on the .meta files, right

[andriyDev]

The comment was that rather than calling their loaders ImageLoader the recommendation will be to "fake namespace" by saying MyExtraSpecialImageLoader. Which means the long name might look like: my_extra_special_crate::MyExtraSpecialImageLoader.

[hukasu]

Meanwhile, in my code

https://github.com/hukasu/bevy_ro_client/blob/49c8654d191728ada67ad7861324a2554cb254b6/bevy_crates/bevy_ragnarok_act/src/plugin/loader.rs#L44

https://github.com/hukasu/bevy_ro_client/blob/49c8654d191728ada67ad7861324a2554cb254b6/bevy_crates/bevy_ragnarok_gat/src/loader.rs#L13

https://github.com/hukasu/bevy_ro_client/blob/49c8654d191728ada67ad7861324a2554cb254b6/bevy_crates/bevy_ragnarok_pal/src/plugin/loader.rs#L8

https://github.com/hukasu/bevy_ro_client/blob/49c8654d191728ada67ad7861324a2554cb254b6/bevy_crates/bevy_ragnarok_rsm/src/plugin/loader.rs#L50

https://github.com/hukasu/bevy_ro_client/blob/49c8654d191728ada67ad7861324a2554cb254b6/bevy_crates/bevy_ragnarok_rsw/src/plugin/loader.rs#L36

https://github.com/hukasu/bevy_ro_client/blob/49c8654d191728ada67ad7861324a2554cb254b6/bevy_crates/bevy_ragnarok_spr/src/plugin/loader.rs#L13

[alice-i-cecile]

I'm similarly not fully convinced that this is the optimal design, but this is really well-made!

[JMS55]

@cart @alice-i-cecile and other maintainers - we need a final decision on this. Close, or merge?

[cart]

This is the right path forward. TypePath was implemented with this scenario in mind, and this approach is the way I imagined it would be used.