-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Removed anyhow
#10003
Removed anyhow
#10003
Conversation
…into RemoveAnyhow
Fixed Typos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is much nicer. Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
anyhow
in the engine #8140Solution
AssetLoader
andAssetSaver
, which were the last instances ofanyhow
in use across Bevy.Changelog
Error
toAssetLoader
andAssetSaver
for use with theload
andsave
methods respectively.ErasedAssetLoader
andErasedAssetSaver
load
andsave
methods to useBox<dyn Error + Send + Sync + 'static>
to allow for arbitraryError
types from the non-erased trait variants. Note the strict requirements match the pre-existing requirements aroundanyhow::Error
.Migration Guide
anyhow
is no longer exported bybevy_asset
; Add it to your own project (if required).AssetLoader
andAssetSaver
have 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::Error
is a drop-in replacement.AssetLoaderError
has 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
Error
type 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 removeanyhow
entirely. End users are still encouraged to useanyhow
if that is their preferred error handling style. Arguably, adding theError
associated type gives more freedom to end-users to decide whether they want more or less explicit error handling (anyhow
vsthiserror
).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.