Skip to content

Commit

Permalink
return type-erased Errors in AssetLoader::load
Browse files Browse the repository at this point in the history
this fixes #10350

Previous change made in #10003
resulted in inability for users to have ?Sized errors, which caused
usability issues with crates like anyhow.
  • Loading branch information
rlidwka committed Nov 8, 2023
1 parent 0cc1179 commit a5f15ba
Show file tree
Hide file tree
Showing 17 changed files with 37 additions and 62 deletions.
8 changes: 3 additions & 5 deletions crates/bevy_asset/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub enum AssetMode {
///
/// When developing an app, you should enable the `asset_processor` cargo feature, which will run the asset processor at startup. This should generally
/// be used in combination with the `file_watcher` cargo feature, which enables hot-reloading of assets that have changed. When both features are enabled,
/// changes to "original/source assets" will be detected, the asset will be re-processed, and then the final processed asset will be hot-reloaded in the app.
/// changes to "original/source assets" will be detected, the asset will be re-processed, and then the final processed asset will be hot-reloaded in the app.
///
/// [`AssetMeta`]: crate::meta::AssetMeta
/// [`AssetSource`]: crate::io::AssetSource
Expand Down Expand Up @@ -459,22 +459,20 @@ mod tests {

type Settings = ();

type Error = CoolTextLoaderError;

fn load<'a>(
&'a self,
reader: &'a mut Reader,
_settings: &'a Self::Settings,
load_context: &'a mut LoadContext,
) -> BoxedFuture<'a, Result<Self::Asset, Self::Error>> {
) -> BoxedFuture<'a, Result<Self::Asset, bevy_asset::Error>> {
Box::pin(async move {
let mut bytes = Vec::new();
reader.read_to_end(&mut bytes).await?;
let mut ron: CoolTextRon = ron::de::from_bytes(&bytes)?;
let mut embedded = String::new();
for dep in ron.embedded_dependencies {
let loaded = load_context.load_direct(&dep).await.map_err(|_| {
Self::Error::CannotLoadDependency {
CoolTextLoaderError::CannotLoadDependency {
dependency: dep.into(),
}
})?;
Expand Down
20 changes: 7 additions & 13 deletions crates/bevy_asset/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,22 @@ use std::{
};
use thiserror::Error;

pub type Error = Box<dyn std::error::Error + Send + Sync>;

/// Loads an [`Asset`] from a given byte [`Reader`]. This can accept [`AssetLoader::Settings`], which configure how the [`Asset`]
/// should be loaded.
pub trait AssetLoader: Send + Sync + 'static {
/// The top level [`Asset`] loaded by this [`AssetLoader`].
type Asset: crate::Asset;
/// The settings type used by this [`AssetLoader`].
type Settings: Settings + Default + Serialize + for<'a> Deserialize<'a>;
/// The type of [error](`std::error::Error`) which could be encountered by this loader.
type Error: std::error::Error + Send + Sync + 'static;
/// Asynchronously loads [`AssetLoader::Asset`] (and any other labeled assets) from the bytes provided by [`Reader`].
fn load<'a>(
&'a self,
reader: &'a mut Reader,
settings: &'a Self::Settings,
load_context: &'a mut LoadContext,
) -> BoxedFuture<'a, Result<Self::Asset, Self::Error>>;
) -> BoxedFuture<'a, Result<Self::Asset, Error>>;

/// Returns a list of extensions supported by this asset loader, without the preceding dot.
fn extensions(&self) -> &[&str];
Expand All @@ -49,10 +49,7 @@ pub trait ErasedAssetLoader: Send + Sync + 'static {
reader: &'a mut Reader,
meta: Box<dyn AssetMetaDyn>,
load_context: LoadContext<'a>,
) -> BoxedFuture<
'a,
Result<ErasedLoadedAsset, Box<dyn std::error::Error + Send + Sync + 'static>>,
>;
) -> BoxedFuture<'a, Result<ErasedLoadedAsset, Error>>;

/// Returns a list of extensions supported by this asset loader, without the preceding dot.
fn extensions(&self) -> &[&str];
Expand Down Expand Up @@ -80,10 +77,7 @@ where
reader: &'a mut Reader,
meta: Box<dyn AssetMetaDyn>,
mut load_context: LoadContext<'a>,
) -> BoxedFuture<
'a,
Result<ErasedLoadedAsset, Box<dyn std::error::Error + Send + Sync + 'static>>,
> {
) -> BoxedFuture<'a, Result<ErasedLoadedAsset, Error>> {
Box::pin(async move {
let settings = meta
.loader_settings()
Expand Down Expand Up @@ -446,7 +440,7 @@ impl<'a> LoadContext<'a> {
/// Retrieves a handle for the asset at the given path and adds that path as a dependency of the asset.
/// If the current context is a normal [`AssetServer::load`], an actual asset load will be kicked off immediately, which ensures the load happens
/// as soon as possible.
/// "Normal loads" kicked from within a normal Bevy App will generally configure the context to kick off loads immediately.
/// "Normal loads" kicked from within a normal Bevy App will generally configure the context to kick off loads immediately.
/// If the current context is configured to not load dependencies automatically (ex: [`AssetProcessor`](crate::processor::AssetProcessor)),
/// a load will not be kicked off automatically. It is then the calling context's responsibility to begin a load if necessary.
pub fn load<'b, A: Asset>(&mut self, path: impl Into<AssetPath<'b>>) -> Handle<A> {
Expand All @@ -462,7 +456,7 @@ impl<'a> LoadContext<'a> {

/// Loads the [`Asset`] of type `A` at the given `path` with the given [`AssetLoader::Settings`] settings `S`. This is a "deferred"
/// load. If the settings type `S` does not match the settings expected by `A`'s asset loader, an error will be printed to the log
/// and the asset load will fail.
/// and the asset load will fail.
pub fn load_with_settings<'b, A: Asset, S: Settings + Default>(
&mut self,
path: impl Into<AssetPath<'b>>,
Expand Down
3 changes: 1 addition & 2 deletions crates/bevy_asset/src/meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,12 @@ impl VisitAssetDependencies for () {
impl AssetLoader for () {
type Asset = ();
type Settings = ();
type Error = std::io::Error;
fn load<'a>(
&'a self,
_reader: &'a mut crate::io::Reader,
_settings: &'a Self::Settings,
_load_context: &'a mut crate::LoadContext,
) -> bevy_utils::BoxedFuture<'a, Result<Self::Asset, Self::Error>> {
) -> bevy_utils::BoxedFuture<'a, Result<Self::Asset, bevy_asset::Error>> {
unreachable!();
}

Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_asset/src/processor/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ pub enum ProcessError {
#[error("The wrong meta type was passed into a processor. This is probably an internal implementation error.")]
WrongMetaType,
#[error("Encountered an error while saving the asset: {0}")]
AssetSaveError(#[from] Box<dyn std::error::Error + Send + Sync + 'static>),
AssetSaveError(#[from] crate::Error),
#[error("Assets without extensions are not supported.")]
ExtensionRequired,
}
Expand Down Expand Up @@ -138,7 +138,7 @@ impl<Loader: AssetLoader, Saver: AssetSaver<Asset = Loader::Asset>> Process
.saver
.save(writer, saved_asset, &settings.saver_settings)
.await
.map_err(|error| ProcessError::AssetSaveError(Box::new(error)))?;
.map_err(ProcessError::AssetSaveError)?;
Ok(output_settings)
})
}
Expand Down
12 changes: 5 additions & 7 deletions crates/bevy_asset/src/saver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,27 @@ pub trait AssetSaver: Send + Sync + 'static {
type Settings: Settings + Default + Serialize + for<'a> Deserialize<'a>;
/// The type of [`AssetLoader`] used to load this [`Asset`]
type OutputLoader: AssetLoader;
/// The type of [error](`std::error::Error`) which could be encountered by this saver.
type Error: std::error::Error + Send + Sync + 'static;

/// Saves the given runtime [`Asset`] by writing it to a byte format using `writer`. The passed in `settings` can influence how the
/// `asset` is saved.
/// `asset` is saved.
fn save<'a>(
&'a self,
writer: &'a mut Writer,
asset: SavedAsset<'a, Self::Asset>,
settings: &'a Self::Settings,
) -> BoxedFuture<'a, Result<<Self::OutputLoader as AssetLoader>::Settings, Self::Error>>;
) -> BoxedFuture<'a, Result<<Self::OutputLoader as AssetLoader>::Settings, crate::Error>>;
}

/// A type-erased dynamic variant of [`AssetSaver`] that allows callers to save assets without knowing the actual type of the [`AssetSaver`].
pub trait ErasedAssetSaver: Send + Sync + 'static {
/// Saves the given runtime [`ErasedLoadedAsset`] by writing it to a byte format using `writer`. The passed in `settings` can influence how the
/// `asset` is saved.
/// `asset` is saved.
fn save<'a>(
&'a self,
writer: &'a mut Writer,
asset: &'a ErasedLoadedAsset,
settings: &'a dyn Settings,
) -> BoxedFuture<'a, Result<(), Box<dyn std::error::Error + Send + Sync + 'static>>>;
) -> BoxedFuture<'a, Result<(), crate::Error>>;

/// The type name of the [`AssetSaver`].
fn type_name(&self) -> &'static str;
Expand All @@ -47,7 +45,7 @@ impl<S: AssetSaver> ErasedAssetSaver for S {
writer: &'a mut Writer,
asset: &'a ErasedLoadedAsset,
settings: &'a dyn Settings,
) -> BoxedFuture<'a, Result<(), Box<dyn std::error::Error + Send + Sync + 'static>>> {
) -> BoxedFuture<'a, Result<(), crate::Error>> {
Box::pin(async move {
let settings = settings
.downcast_ref::<S::Settings>()
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_asset/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use thiserror::Error;
/// The general process to load an asset is:
/// 1. Initialize a new [`Asset`] type with the [`AssetServer`] via [`AssetApp::init_asset`], which will internally call [`AssetServer::register_asset`]
/// and set up related ECS [`Assets`] storage and systems.
/// 2. Register one or more [`AssetLoader`]s for that asset with [`AssetApp::init_asset_loader`]
/// 2. Register one or more [`AssetLoader`]s for that asset with [`AssetApp::init_asset_loader`]
/// 3. Add the asset to your asset folder (defaults to `assets`).
/// 4. Call [`AssetServer::load`] with a path to your asset.
///
Expand Down Expand Up @@ -957,7 +957,7 @@ enum MaybeAssetLoader {
},
}

/// Internal events for asset load results
/// Internal events for asset load results
#[allow(clippy::large_enum_variant)]
pub(crate) enum InternalAssetEvent {
Loaded {
Expand Down Expand Up @@ -1046,7 +1046,7 @@ pub enum AssetLoadError {
AssetLoaderError {
path: AssetPath<'static>,
loader_name: &'static str,
error: Box<dyn std::error::Error + Send + Sync + 'static>,
error: crate::Error,
},
#[error("The file at '{base_path}' does not contain the labeled asset '{label}'.")]
MissingLabel {
Expand Down
5 changes: 2 additions & 3 deletions crates/bevy_audio/src/audio_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,13 @@ pub struct AudioLoader;
impl AssetLoader for AudioLoader {
type Asset = AudioSource;
type Settings = ();
type Error = std::io::Error;

fn load<'a>(
&'a self,
reader: &'a mut Reader,
_settings: &'a Self::Settings,
_load_context: &'a mut LoadContext,
) -> BoxedFuture<'a, Result<AudioSource, Self::Error>> {
) -> BoxedFuture<'a, Result<AudioSource, bevy_asset::Error>> {
Box::pin(async move {
let mut bytes = Vec::new();
reader.read_to_end(&mut bytes).await?;
Expand Down Expand Up @@ -113,7 +112,7 @@ pub trait AddAudioSource {
/// so that it can be converted to a [`rodio::Source`] type,
/// and [`Asset`], so that it can be registered as an asset.
/// To use this method on [`App`][bevy_app::App],
/// the [audio][super::AudioPlugin] and [asset][bevy_asset::AssetPlugin] plugins must be added first.
/// the [audio][super::AudioPlugin] and [asset][bevy_asset::AssetPlugin] plugins must be added first.
fn add_audio_source<T>(&mut self) -> &mut Self
where
T: Decodable + Asset,
Expand Down
5 changes: 2 additions & 3 deletions crates/bevy_gltf/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,17 +108,16 @@ pub struct GltfLoader {
impl AssetLoader for GltfLoader {
type Asset = Gltf;
type Settings = ();
type Error = GltfError;
fn load<'a>(
&'a self,
reader: &'a mut Reader,
_settings: &'a (),
load_context: &'a mut LoadContext,
) -> bevy_utils::BoxedFuture<'a, Result<Gltf, Self::Error>> {
) -> bevy_utils::BoxedFuture<'a, Result<Gltf, bevy_asset::Error>> {
Box::pin(async move {
let mut bytes = Vec::new();
reader.read_to_end(&mut bytes).await?;
load_gltf(self, &bytes, load_context).await
Ok(load_gltf(self, &bytes, load_context).await?)
})
}

Expand Down
3 changes: 1 addition & 2 deletions crates/bevy_render/src/render_resource/shader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,13 +250,12 @@ pub enum ShaderLoaderError {
impl AssetLoader for ShaderLoader {
type Asset = Shader;
type Settings = ();
type Error = ShaderLoaderError;
fn load<'a>(
&'a self,
reader: &'a mut Reader,
_settings: &'a Self::Settings,
load_context: &'a mut LoadContext,
) -> BoxedFuture<'a, Result<Shader, Self::Error>> {
) -> BoxedFuture<'a, Result<Shader, bevy_asset::Error>> {
Box::pin(async move {
let ext = load_context.path().extension().unwrap().to_str().unwrap();

Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_render/src/texture/compressed_image_saver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ impl AssetSaver for CompressedImageSaver {

type Settings = ();
type OutputLoader = ImageLoader;
type Error = CompressedImageSaverError;

fn save<'a>(
&'a self,
writer: &'a mut bevy_asset::io::Writer,
image: SavedAsset<'a, Self::Asset>,
_settings: &'a Self::Settings,
) -> bevy_utils::BoxedFuture<'a, std::result::Result<ImageLoaderSettings, Self::Error>> {
) -> bevy_utils::BoxedFuture<'a, std::result::Result<ImageLoaderSettings, bevy_asset::Error>>
{
// PERF: this should live inside the future, but CompressorParams and Compressor are not Send / can't be owned by the BoxedFuture (which _is_ Send)
let mut compressor_params = basis_universal::CompressorParams::new();
compressor_params.set_basis_format(basis_universal::BasisTextureFormat::UASTC4x4);
Expand Down
3 changes: 1 addition & 2 deletions crates/bevy_render/src/texture/exr_texture_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,13 @@ pub enum ExrTextureLoaderError {
impl AssetLoader for ExrTextureLoader {
type Asset = Image;
type Settings = ();
type Error = ExrTextureLoaderError;

fn load<'a>(
&'a self,
reader: &'a mut Reader,
_settings: &'a Self::Settings,
_load_context: &'a mut LoadContext,
) -> BoxedFuture<'a, Result<Image, Self::Error>> {
) -> BoxedFuture<'a, Result<Image, bevy_asset::Error>> {
Box::pin(async move {
let format = TextureFormat::Rgba32Float;
debug_assert_eq!(
Expand Down
3 changes: 1 addition & 2 deletions crates/bevy_render/src/texture/hdr_texture_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,12 @@ pub enum HdrTextureLoaderError {
impl AssetLoader for HdrTextureLoader {
type Asset = Image;
type Settings = ();
type Error = HdrTextureLoaderError;
fn load<'a>(
&'a self,
reader: &'a mut Reader,
_settings: &'a (),
_load_context: &'a mut LoadContext,
) -> bevy_utils::BoxedFuture<'a, Result<Image, Self::Error>> {
) -> bevy_utils::BoxedFuture<'a, Result<Image, bevy_asset::Error>> {
Box::pin(async move {
let format = TextureFormat::Rgba32Float;
debug_assert_eq!(
Expand Down
3 changes: 1 addition & 2 deletions crates/bevy_render/src/texture/image_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,12 @@ pub enum ImageLoaderError {
impl AssetLoader for ImageLoader {
type Asset = Image;
type Settings = ImageLoaderSettings;
type Error = ImageLoaderError;
fn load<'a>(
&'a self,
reader: &'a mut Reader,
settings: &'a ImageLoaderSettings,
load_context: &'a mut LoadContext,
) -> bevy_utils::BoxedFuture<'a, Result<Image, Self::Error>> {
) -> bevy_utils::BoxedFuture<'a, Result<Image, bevy_asset::Error>> {
Box::pin(async move {
// use the file extension for the image type
let ext = load_context.path().extension().unwrap().to_str().unwrap();
Expand Down
3 changes: 1 addition & 2 deletions crates/bevy_scene/src/scene_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,13 @@ pub enum SceneLoaderError {
impl AssetLoader for SceneLoader {
type Asset = DynamicScene;
type Settings = ();
type Error = SceneLoaderError;

fn load<'a>(
&'a self,
reader: &'a mut Reader,
_settings: &'a (),
_load_context: &'a mut LoadContext,
) -> BoxedFuture<'a, Result<Self::Asset, Self::Error>> {
) -> BoxedFuture<'a, Result<Self::Asset, bevy_asset::Error>> {
Box::pin(async move {
let mut bytes = Vec::new();
reader.read_to_end(&mut bytes).await?;
Expand Down
3 changes: 1 addition & 2 deletions crates/bevy_text/src/font_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,12 @@ pub enum FontLoaderError {
impl AssetLoader for FontLoader {
type Asset = Font;
type Settings = ();
type Error = FontLoaderError;
fn load<'a>(
&'a self,
reader: &'a mut Reader,
_settings: &'a (),
_load_context: &'a mut LoadContext,
) -> bevy_utils::BoxedFuture<'a, Result<Font, Self::Error>> {
) -> bevy_utils::BoxedFuture<'a, Result<Font, bevy_asset::Error>> {
Box::pin(async move {
let mut bytes = Vec::new();
reader.read_to_end(&mut bytes).await?;
Expand Down
3 changes: 1 addition & 2 deletions examples/asset/custom_asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,12 @@ pub enum CustomAssetLoaderError {
impl AssetLoader for CustomAssetLoader {
type Asset = CustomAsset;
type Settings = ();
type Error = CustomAssetLoaderError;
fn load<'a>(
&'a self,
reader: &'a mut Reader,
_settings: &'a (),
_load_context: &'a mut LoadContext,
) -> BoxedFuture<'a, Result<Self::Asset, Self::Error>> {
) -> BoxedFuture<'a, Result<Self::Asset, bevy::asset::Error>> {
Box::pin(async move {
let mut bytes = Vec::new();
reader.read_to_end(&mut bytes).await?;
Expand Down
Loading

0 comments on commit a5f15ba

Please sign in to comment.