Conversation
alice-i-cecile
left a comment
There was a problem hiding this comment.
This is much nicer. Thank you!
james7132
left a comment
There was a problem hiding this comment.
Thanks for diving into this. This has been bothering me since I started using Bevy. It's notably more verbose, but also much more explicit. Much more Rust-like this way. Glad to see it's finally being addressed.
|
Looks like I missed one docstring in the |
# Objective - Fixes bevyengine#8140 ## Solution - Added Explicit Error Typing for `AssetLoader` and `AssetSaver`, which were the last instances of `anyhow` in use across Bevy. --- ## Changelog - Added an associated type `Error` to `AssetLoader` and `AssetSaver` for use with the `load` and `save` methods respectively. - Changed `ErasedAssetLoader` and `ErasedAssetSaver` `load` and `save` methods to use `Box<dyn Error + Send + Sync + 'static>` to allow for arbitrary `Error` types from the non-erased trait variants. Note the strict requirements match the pre-existing requirements around `anyhow::Error`. ## Migration Guide - `anyhow` is no longer exported by `bevy_asset`; Add it to your own project (if required). - `AssetLoader` and `AssetSaver` have an associated type `Error`; Define an appropriate error type (e.g., using `thiserror`), or use a pre-made error type (e.g., `anyhow::Error`). Note that using `anyhow::Error` is a drop-in replacement. - `AssetLoaderError` has been removed; Define a new error type, or use an alternative (e.g., `anyhow::Error`) - All the first-party `AssetLoader`'s and `AssetSaver`'s now return relevant (and narrow) error types instead of a single ambiguous type; Match over the specific error type, or encapsulate (`Box<dyn>`, `thiserror`, `anyhow`, etc.) ## Notes A simpler PR to resolve this issue would simply define a Bevy `Error` type defined as `Box<dyn std::error::Error + Send + Sync + 'static>`, but I think this type of error handling should be discouraged when possible. Since only 2 traits required the use of `anyhow`, it isn't a substantive body of work to solidify these error types, and remove `anyhow` entirely. End users are still encouraged to use `anyhow` if that is their preferred error handling style. Arguably, adding the `Error` associated type gives more freedom to end-users to decide whether they want more or less explicit error handling (`anyhow` vs `thiserror`). As an aside, I didn't perform any testing on Android or WASM. CI passed locally, but there may be mistakes for those platforms I missed.
|
These AssetLoadError::CannotLoadDependency errors don't make sense to me:
|
|
We're also no longer including meta deserialization failures / throwing them in the generic CannotLoadDependency bucket. |
|
I'm just going to fix this in Multiple Asset Sources as it touches this code. |
Not sure if a comment is the right place to put this, but this section of this PR's migration guide is incorrect. |
# Objective - Fixes bevyengine#8140 ## Solution - Added Explicit Error Typing for `AssetLoader` and `AssetSaver`, which were the last instances of `anyhow` in use across Bevy. --- ## Changelog - Added an associated type `Error` to `AssetLoader` and `AssetSaver` for use with the `load` and `save` methods respectively. - Changed `ErasedAssetLoader` and `ErasedAssetSaver` `load` and `save` methods to use `Box<dyn Error + Send + Sync + 'static>` to allow for arbitrary `Error` types from the non-erased trait variants. Note the strict requirements match the pre-existing requirements around `anyhow::Error`. ## Migration Guide - `anyhow` is no longer exported by `bevy_asset`; Add it to your own project (if required). - `AssetLoader` and `AssetSaver` have an associated type `Error`; Define an appropriate error type (e.g., using `thiserror`), or use a pre-made error type (e.g., `anyhow::Error`). Note that using `anyhow::Error` is a drop-in replacement. - `AssetLoaderError` has been removed; Define a new error type, or use an alternative (e.g., `anyhow::Error`) - All the first-party `AssetLoader`'s and `AssetSaver`'s now return relevant (and narrow) error types instead of a single ambiguous type; Match over the specific error type, or encapsulate (`Box<dyn>`, `thiserror`, `anyhow`, etc.) ## Notes A simpler PR to resolve this issue would simply define a Bevy `Error` type defined as `Box<dyn std::error::Error + Send + Sync + 'static>`, but I think this type of error handling should be discouraged when possible. Since only 2 traits required the use of `anyhow`, it isn't a substantive body of work to solidify these error types, and remove `anyhow` entirely. End users are still encouraged to use `anyhow` if that is their preferred error handling style. Arguably, adding the `Error` associated type gives more freedom to end-users to decide whether they want more or less explicit error handling (`anyhow` vs `thiserror`). As an aside, I didn't perform any testing on Android or WASM. CI passed locally, but there may be mistakes for those platforms I missed.
this fixes bevyengine#10350 Previous change made in bevyengine#10003 resulted in inability for users to have ?Sized errors, which caused usability issues with crates like anyhow.
this fixes bevyengine#10350 Previous change made in bevyengine#10003 resulted in inability for users to have ?Sized errors, which caused usability issues with crates like anyhow.
# Objective - Fixes bevyengine#8140 ## Solution - Added Explicit Error Typing for `AssetLoader` and `AssetSaver`, which were the last instances of `anyhow` in use across Bevy. --- ## Changelog - Added an associated type `Error` to `AssetLoader` and `AssetSaver` for use with the `load` and `save` methods respectively. - Changed `ErasedAssetLoader` and `ErasedAssetSaver` `load` and `save` methods to use `Box<dyn Error + Send + Sync + 'static>` to allow for arbitrary `Error` types from the non-erased trait variants. Note the strict requirements match the pre-existing requirements around `anyhow::Error`. ## Migration Guide - `anyhow` is no longer exported by `bevy_asset`; Add it to your own project (if required). - `AssetLoader` and `AssetSaver` have an associated type `Error`; Define an appropriate error type (e.g., using `thiserror`), or use a pre-made error type (e.g., `anyhow::Error`). Note that using `anyhow::Error` is a drop-in replacement. - `AssetLoaderError` has been removed; Define a new error type, or use an alternative (e.g., `anyhow::Error`) - All the first-party `AssetLoader`'s and `AssetSaver`'s now return relevant (and narrow) error types instead of a single ambiguous type; Match over the specific error type, or encapsulate (`Box<dyn>`, `thiserror`, `anyhow`, etc.) ## Notes A simpler PR to resolve this issue would simply define a Bevy `Error` type defined as `Box<dyn std::error::Error + Send + Sync + 'static>`, but I think this type of error handling should be discouraged when possible. Since only 2 traits required the use of `anyhow`, it isn't a substantive body of work to solidify these error types, and remove `anyhow` entirely. End users are still encouraged to use `anyhow` if that is their preferred error handling style. Arguably, adding the `Error` associated type gives more freedom to end-users to decide whether they want more or less explicit error handling (`anyhow` vs `thiserror`). As an aside, I didn't perform any testing on Android or WASM. CI passed locally, but there may be mistakes for those platforms I missed.
Objective
anyhowin the engine #8140Solution
AssetLoaderandAssetSaver, which were the last instances ofanyhowin use across Bevy.Changelog
ErrortoAssetLoaderandAssetSaverfor use with theloadandsavemethods respectively.ErasedAssetLoaderandErasedAssetSaverloadandsavemethods to useBox<dyn Error + Send + Sync + 'static>to allow for arbitraryErrortypes from the non-erased trait variants. Note the strict requirements match the pre-existing requirements aroundanyhow::Error.Migration Guide
anyhowis no longer exported bybevy_asset; Add it to your own project (if required).AssetLoaderandAssetSaverhave an associated typeError; Define an appropriate error type (e.g., usingthiserror), or use a pre-made error type (e.g.,anyhow::Error). Note that usinganyhow::Erroris a drop-in replacement.AssetLoaderErrorhas been removed; Define a new error type, or use an alternative (e.g.,anyhow::Error)AssetLoader's andAssetSaver's now return relevant (and narrow) error types instead of a single ambiguous type; Match over the specific error type, or encapsulate (Box<dyn>,thiserror,anyhow, etc.)Notes
A simpler PR to resolve this issue would simply define a Bevy
Errortype defined asBox<dyn std::error::Error + Send + Sync + 'static>, but I think this type of error handling should be discouraged when possible. Since only 2 traits required the use ofanyhow, it isn't a substantive body of work to solidify these error types, and removeanyhowentirely. End users are still encouraged to useanyhowif that is their preferred error handling style. Arguably, adding theErrorassociated type gives more freedom to end-users to decide whether they want more or less explicit error handling (anyhowvsthiserror).As an aside, I didn't perform any testing on Android or WASM. CI passed locally, but there may be mistakes for those platforms I missed.