From 4da449344911f9eb83d393eb06ff1cb08f5d8035 Mon Sep 17 00:00:00 2001 From: Vitaliy Sapronenko Date: Thu, 4 Apr 2024 17:04:27 +0300 Subject: [PATCH] Error info has been added to LoadState::Failed (#12709) # Objective Fixes #12667. ## Solution - Stored [AssetLoadError](https://docs.rs/bevy/latest/bevy/asset/enum.AssetLoadError.html) inside of [LoadState::Failed](https://docs.rs/bevy/latest/bevy/asset/enum.LoadState.html) as a Box to avoid bloating the size of all variants of LoadState. - Fixed dependent code ## Migration guide Added [AssetLoadError](https://docs.rs/bevy/latest/bevy/asset/enum.AssetLoadError.html) to [LoadState::Failed](https://docs.rs/bevy/latest/bevy/asset/enum.LoadState.html) option Removed `Copy`, `Ord` and `PartialOrd` implementations for [LoadState](https://docs.rs/bevy/latest/bevy/asset/enum.LoadState.html) enum Added `Eq` and `PartialEq` implementations for [MissingAssetSourceError](https://docs.rs/bevy/latest/bevy/asset/io/struct.MissingAssetSourceError.html), [MissingProcessedAssetReaderError](https://docs.rs/bevy/latest/bevy/asset/io/struct.MissingProcessedAssetReaderError.html), [DeserializeMetaError](https://docs.rs/bevy/latest/bevy/asset/enum.DeserializeMetaError.html), [LoadState](https://docs.rs/bevy/latest/bevy/asset/enum.LoadState.html), [AssetLoadError](https://docs.rs/bevy/latest/bevy/asset/enum.AssetLoadError.html), [MissingAssetLoaderForTypeNameError](https://docs.rs/bevy/latest/bevy/asset/struct.MissingAssetLoaderForTypeNameError.html) and [MissingAssetLoaderForTypeIdError](https://docs.rs/bevy/latest/bevy/asset/struct.MissingAssetLoaderForTypeIdError.html) --- crates/bevy_asset/src/io/mod.rs | 15 +++++++ crates/bevy_asset/src/io/source.rs | 4 +- crates/bevy_asset/src/lib.rs | 6 +-- crates/bevy_asset/src/loader.rs | 2 +- crates/bevy_asset/src/processor/mod.rs | 8 ++-- crates/bevy_asset/src/server/info.rs | 9 ++-- crates/bevy_asset/src/server/mod.rs | 62 ++++++++++++++++++-------- 7 files changed, 72 insertions(+), 34 deletions(-) diff --git a/crates/bevy_asset/src/io/mod.rs b/crates/bevy_asset/src/io/mod.rs index 1a1c680ecd..df17c4d6cd 100644 --- a/crates/bevy_asset/src/io/mod.rs +++ b/crates/bevy_asset/src/io/mod.rs @@ -51,6 +51,21 @@ pub enum AssetReaderError { HttpError(u16), } +impl PartialEq for AssetReaderError { + /// Equality comparison for `AssetReaderError::Io` is not full (only through `ErrorKind` of inner error) + #[inline] + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (Self::NotFound(path), Self::NotFound(other_path)) => path == other_path, + (Self::Io(error), Self::Io(other_error)) => error.kind() == other_error.kind(), + (Self::HttpError(code), Self::HttpError(other_code)) => code == other_code, + _ => false, + } + } +} + +impl Eq for AssetReaderError {} + impl From for AssetReaderError { fn from(value: std::io::Error) -> Self { Self::Io(Arc::new(value)) diff --git a/crates/bevy_asset/src/io/source.rs b/crates/bevy_asset/src/io/source.rs index 21bd29294e..cd42d31f01 100644 --- a/crates/bevy_asset/src/io/source.rs +++ b/crates/bevy_asset/src/io/source.rs @@ -584,7 +584,7 @@ impl AssetSources { } /// An error returned when an [`AssetSource`] does not exist for a given id. -#[derive(Error, Debug, Clone)] +#[derive(Error, Debug, Clone, PartialEq, Eq)] #[error("Asset Source '{0}' does not exist")] pub struct MissingAssetSourceError(AssetSourceId<'static>); @@ -594,7 +594,7 @@ pub struct MissingAssetSourceError(AssetSourceId<'static>); pub struct MissingAssetWriterError(AssetSourceId<'static>); /// An error returned when a processed [`AssetReader`] does not exist for a given id. -#[derive(Error, Debug, Clone)] +#[derive(Error, Debug, Clone, PartialEq, Eq)] #[error("Asset Source '{0}' does not have a processed AssetReader.")] pub struct MissingProcessedAssetReaderError(AssetSourceId<'static>); diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index 239ea361f8..4b3be75ecd 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -1071,13 +1071,13 @@ mod tests { let d_id = c_text.dependencies[0].id(); let d_text = get::(world, d_id); let (d_load, d_deps, d_rec_deps) = asset_server.get_load_states(d_id).unwrap(); - if d_load != LoadState::Failed { + if !matches!(d_load, LoadState::Failed(_)) { // wait until d has exited the loading state return None; } assert!(d_text.is_none()); - assert_eq!(d_load, LoadState::Failed); + assert!(matches!(d_load, LoadState::Failed(_))); assert_eq!(d_deps, DependencyLoadState::Failed); assert_eq!(d_rec_deps, RecursiveDependencyLoadState::Failed); @@ -1384,7 +1384,7 @@ mod tests { // Check what just failed for error in errors.read() { let (load_state, _, _) = server.get_load_states(error.id).unwrap(); - assert_eq!(load_state, LoadState::Failed); + assert!(matches!(load_state, LoadState::Failed(_))); assert_eq!(*error.path.source(), AssetSourceId::Name("unstable".into())); match &error.error { AssetLoadError::AssetReaderError(read_error) => match read_error { diff --git a/crates/bevy_asset/src/loader.rs b/crates/bevy_asset/src/loader.rs index ae3f08511a..8630ffea8f 100644 --- a/crates/bevy_asset/src/loader.rs +++ b/crates/bevy_asset/src/loader.rs @@ -257,7 +257,7 @@ pub struct LoadDirectError { } /// An error that occurs while deserializing [`AssetMeta`]. -#[derive(Error, Debug, Clone)] +#[derive(Error, Debug, Clone, PartialEq, Eq)] pub enum DeserializeMetaError { #[error("Failed to deserialize asset meta: {0:?}")] DeserializeSettings(#[from] SpannedError), diff --git a/crates/bevy_asset/src/processor/mod.rs b/crates/bevy_asset/src/processor/mod.rs index f3d373da06..cf2b34ef04 100644 --- a/crates/bevy_asset/src/processor/mod.rs +++ b/crates/bevy_asset/src/processor/mod.rs @@ -1203,10 +1203,8 @@ impl ProcessorAssetInfos { Err(err) => { error!("Failed to process asset {asset_path}: {err}"); // if this failed because a dependency could not be loaded, make sure it is reprocessed if that dependency is reprocessed - if let ProcessError::AssetLoadError(AssetLoadError::AssetLoaderError { - path: dependency, - .. - }) = err + if let ProcessError::AssetLoadError(AssetLoadError::AssetLoaderError(dependency)) = + err { let info = self.get_mut(&asset_path).expect("info should exist"); info.processed_info = Some(ProcessedInfo { @@ -1214,7 +1212,7 @@ impl ProcessorAssetInfos { full_hash: AssetHash::default(), process_dependencies: vec![], }); - self.add_dependant(&dependency, asset_path.to_owned()); + self.add_dependant(dependency.path(), asset_path.to_owned()); } let info = self.get_mut(&asset_path).expect("info should exist"); diff --git a/crates/bevy_asset/src/server/info.rs b/crates/bevy_asset/src/server/info.rs index dd967a9b92..76eb43bc21 100644 --- a/crates/bevy_asset/src/server/info.rs +++ b/crates/bevy_asset/src/server/info.rs @@ -212,8 +212,7 @@ impl AssetInfos { let mut should_load = false; if loading_mode == HandleLoadingMode::Force || (loading_mode == HandleLoadingMode::Request - && (info.load_state == LoadState::NotLoaded - || info.load_state == LoadState::Failed)) + && matches!(info.load_state, LoadState::NotLoaded | LoadState::Failed(_))) { info.load_state = LoadState::Loading; info.dep_load_state = DependencyLoadState::Loading; @@ -412,7 +411,7 @@ impl AssetInfos { // If dependency is loaded, reduce our count by one false } - LoadState::Failed => { + LoadState::Failed(_) => { failed_deps.insert(*dep_id); false } @@ -582,12 +581,12 @@ impl AssetInfos { } } - pub(crate) fn process_asset_fail(&mut self, failed_id: UntypedAssetId) { + pub(crate) fn process_asset_fail(&mut self, failed_id: UntypedAssetId, error: AssetLoadError) { let (dependants_waiting_on_load, dependants_waiting_on_rec_load) = { let info = self .get_mut(failed_id) .expect("Asset info should always exist at this point"); - info.load_state = LoadState::Failed; + info.load_state = LoadState::Failed(Box::new(error)); info.dep_load_state = DependencyLoadState::Failed; info.rec_dep_load_state = RecursiveDependencyLoadState::Failed; ( diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index 26b057bddf..0e4ae6aab1 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -26,7 +26,7 @@ use futures_lite::StreamExt; use info::*; use loaders::*; use parking_lot::RwLock; -use std::path::PathBuf; +use std::{any::Any, path::PathBuf}; use std::{any::TypeId, path::Path, sync::Arc}; use thiserror::Error; @@ -754,7 +754,7 @@ impl AssetServer { .infos .read() .get(id.into()) - .map(|i| (i.load_state, i.dep_load_state, i.rec_dep_load_state)) + .map(|i| (i.load_state.clone(), i.dep_load_state, i.rec_dep_load_state)) } /// Retrieves the main [`LoadState`] of a given asset `id`. @@ -762,7 +762,11 @@ impl AssetServer { /// Note that this is "just" the root asset load state. To check if an asset _and_ its recursive /// dependencies have loaded, see [`AssetServer::is_loaded_with_dependencies`]. pub fn get_load_state(&self, id: impl Into) -> Option { - self.data.infos.read().get(id.into()).map(|i| i.load_state) + self.data + .infos + .read() + .get(id.into()) + .map(|i| i.load_state.clone()) } /// Retrieves the [`RecursiveDependencyLoadState`] of a given asset `id`. @@ -1041,11 +1045,11 @@ impl AssetServer { let load_context = LoadContext::new(self, asset_path.clone(), load_dependencies, populate_hashes); loader.load(reader, meta, load_context).await.map_err(|e| { - AssetLoadError::AssetLoaderError { + AssetLoadError::AssetLoaderError(AssetLoaderError { path: asset_path.clone_owned(), loader_name: loader.type_name(), error: e.into(), - } + }) }) } } @@ -1073,7 +1077,7 @@ pub fn handle_internal_asset_events(world: &mut World) { sender(world, id); } InternalAssetEvent::Failed { id, path, error } => { - infos.process_asset_fail(id); + infos.process_asset_fail(id, error.clone()); // Send untyped failure event untyped_failures.push(UntypedAssetLoadFailedEvent { @@ -1190,7 +1194,7 @@ pub(crate) enum InternalAssetEvent { } /// The load state of an asset. -#[derive(Component, Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd)] +#[derive(Component, Clone, Debug, PartialEq, Eq)] pub enum LoadState { /// The asset has not started loading yet NotLoaded, @@ -1199,7 +1203,7 @@ pub enum LoadState { /// The asset has been loaded and has been added to the [`World`] Loaded, /// The asset failed to load. - Failed, + Failed(Box), } /// The load state of an asset's dependencies. @@ -1229,7 +1233,7 @@ pub enum RecursiveDependencyLoadState { } /// An error that occurs during an [`Asset`] load. -#[derive(Error, Debug, Clone)] +#[derive(Error, Debug, Clone, PartialEq, Eq)] pub enum AssetLoadError { #[error("Requested handle of type {requested:?} for asset '{path}' does not match actual asset type '{actual_asset_name}', which used loader '{loader_name}'")] RequestedHandleTypeMismatch { @@ -1268,12 +1272,8 @@ pub enum AssetLoadError { CannotLoadProcessedAsset { path: AssetPath<'static> }, #[error("Asset '{path}' is configured to be ignored. It cannot be loaded.")] CannotLoadIgnoredAsset { path: AssetPath<'static> }, - #[error("Failed to load asset '{path}' with asset loader '{loader_name}': {error}")] - AssetLoaderError { - path: AssetPath<'static>, - loader_name: &'static str, - error: Arc, - }, + #[error(transparent)] + AssetLoaderError(#[from] AssetLoaderError), #[error("The file at '{}' does not contain the labeled asset '{}'; it contains the following {} assets: {}", base_path, label, @@ -1286,22 +1286,48 @@ pub enum AssetLoadError { }, } -/// An error that occurs when an [`AssetLoader`] is not registered for a given extension. #[derive(Error, Debug, Clone)] +#[error("Failed to load asset '{path}' with asset loader '{loader_name}': {error}")] +pub struct AssetLoaderError { + path: AssetPath<'static>, + loader_name: &'static str, + error: Arc, +} + +impl PartialEq for AssetLoaderError { + /// Equality comparison for `AssetLoaderError::error` is not full (only through `TypeId`) + #[inline] + fn eq(&self, other: &Self) -> bool { + self.path == other.path + && self.loader_name == other.loader_name + && self.error.type_id() == other.error.type_id() + } +} + +impl Eq for AssetLoaderError {} + +impl AssetLoaderError { + pub fn path(&self) -> &AssetPath<'static> { + &self.path + } +} + +/// An error that occurs when an [`AssetLoader`] is not registered for a given extension. +#[derive(Error, Debug, Clone, PartialEq, Eq)] #[error("no `AssetLoader` found{}", format_missing_asset_ext(.extensions))] pub struct MissingAssetLoaderForExtensionError { extensions: Vec, } /// An error that occurs when an [`AssetLoader`] is not registered for a given [`std::any::type_name`]. -#[derive(Error, Debug, Clone)] +#[derive(Error, Debug, Clone, PartialEq, Eq)] #[error("no `AssetLoader` found with the name '{type_name}'")] pub struct MissingAssetLoaderForTypeNameError { type_name: String, } /// An error that occurs when an [`AssetLoader`] is not registered for a given [`Asset`] [`TypeId`]. -#[derive(Error, Debug, Clone)] +#[derive(Error, Debug, Clone, PartialEq, Eq)] #[error("no `AssetLoader` found with the ID '{type_id:?}'")] pub struct MissingAssetLoaderForTypeIdError { pub type_id: TypeId,