Return an error when loading non-existent labels (#9751)

# Objective

Calling `asset_server.load("scene.gltf#SomeLabel")` will silently fail
if `SomeLabel` does not exist.

Referenced in #9714 

## Solution

We now detect this case and return an error. I also slightly refactored
`load_internal` to make the logic / dataflow much clearer.

---------

Co-authored-by: Pascal Hertleif <killercup@gmail.com>
This commit is contained in:
Carter Anderson 2023-10-15 11:36:51 -07:00 committed by GitHub
parent 49d5c6b8a3
commit 02943737b2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 48 additions and 30 deletions

View file

@ -203,6 +203,12 @@ impl<'a> AssetPath<'a> {
self.label.as_deref() self.label.as_deref()
} }
/// Gets the "sub-asset label".
#[inline]
pub fn label_cow(&self) -> Option<CowArc<'a, str>> {
self.label.clone()
}
/// Gets the path to the asset in the "virtual filesystem". /// Gets the path to the asset in the "virtual filesystem".
#[inline] #[inline]
pub fn path(&self) -> &Path { pub fn path(&self) -> &Path {

View file

@ -257,7 +257,7 @@ impl AssetServer {
path: impl Into<AssetPath<'a>>, path: impl Into<AssetPath<'a>>,
meta_transform: Option<MetaTransform>, meta_transform: Option<MetaTransform>,
) -> Handle<A> { ) -> Handle<A> {
let mut path = path.into().into_owned(); let path = path.into().into_owned();
let (handle, should_load) = self.data.infos.write().get_or_create_path_handle::<A>( let (handle, should_load) = self.data.infos.write().get_or_create_path_handle::<A>(
path.clone(), path.clone(),
HandleLoadingMode::Request, HandleLoadingMode::Request,
@ -265,13 +265,10 @@ impl AssetServer {
); );
if should_load { if should_load {
let mut owned_handle = Some(handle.clone().untyped()); let owned_handle = Some(handle.clone().untyped());
let server = self.clone(); let server = self.clone();
IoTaskPool::get() IoTaskPool::get()
.spawn(async move { .spawn(async move {
if path.take_label().is_some() {
owned_handle = None;
}
if let Err(err) = server.load_internal(owned_handle, path, false, None).await { if let Err(err) = server.load_internal(owned_handle, path, false, None).await {
error!("{}", err); error!("{}", err);
} }
@ -291,6 +288,10 @@ impl AssetServer {
self.load_internal(None, path, false, None).await self.load_internal(None, path, false, None).await
} }
/// Performs an async asset load.
///
/// `input_handle` must only be [`Some`] if `should_load` was true when retrieving `input_handle`. This is an optimization to
/// avoid looking up `should_load` twice, but it means you _must_ be sure a load is necessary when calling this function with [`Some`].
async fn load_internal<'a>( async fn load_internal<'a>(
&self, &self,
input_handle: Option<UntypedHandle>, input_handle: Option<UntypedHandle>,
@ -298,7 +299,7 @@ impl AssetServer {
force: bool, force: bool,
meta_transform: Option<MetaTransform>, meta_transform: Option<MetaTransform>,
) -> Result<UntypedHandle, AssetLoadError> { ) -> Result<UntypedHandle, AssetLoadError> {
let mut path = path.into_owned(); let path = path.into_owned();
let path_clone = path.clone(); let path_clone = path.clone();
let (mut meta, loader, mut reader) = self let (mut meta, loader, mut reader) = self
.get_meta_loader_and_reader(&path_clone) .get_meta_loader_and_reader(&path_clone)
@ -312,18 +313,8 @@ impl AssetServer {
e e
})?; })?;
let has_label = path.label().is_some();
let (handle, should_load) = match input_handle { let (handle, should_load) = match input_handle {
Some(handle) => { Some(handle) => {
if !has_label && handle.type_id() != loader.asset_type_id() {
return Err(AssetLoadError::RequestedHandleTypeMismatch {
path: path.into_owned(),
requested: handle.type_id(),
actual_asset_name: loader.asset_type_name(),
loader_name: loader.type_name(),
});
}
// if a handle was passed in, the "should load" check was already done // if a handle was passed in, the "should load" check was already done
(handle, true) (handle, true)
} }
@ -339,37 +330,51 @@ impl AssetServer {
} }
}; };
if path.label().is_none() && handle.type_id() != loader.asset_type_id() {
return Err(AssetLoadError::RequestedHandleTypeMismatch {
path: path.into_owned(),
requested: handle.type_id(),
actual_asset_name: loader.asset_type_name(),
loader_name: loader.type_name(),
});
}
if !should_load && !force { if !should_load && !force {
return Ok(handle); return Ok(handle);
} }
let base_asset_id = if has_label {
path.remove_label(); let (base_handle, base_path) = if path.label().is_some() {
// If the path has a label, the current id does not match the asset root type.
// We need to get the actual asset id
let mut infos = self.data.infos.write(); let mut infos = self.data.infos.write();
let (actual_handle, _) = infos.get_or_create_path_handle_untyped( let base_path = path.without_label().into_owned();
path.clone(), let (base_handle, _) = infos.get_or_create_path_handle_untyped(
base_path.clone(),
loader.asset_type_id(), loader.asset_type_id(),
loader.asset_type_name(), loader.asset_type_name(),
// ignore current load state ... we kicked off this sub asset load because it needed to be loaded but
// does not currently exist
HandleLoadingMode::Force, HandleLoadingMode::Force,
None, None,
); );
actual_handle.id() (base_handle, base_path)
} else { } else {
handle.id() (handle.clone(), path.clone())
}; };
if let Some(meta_transform) = handle.meta_transform() { if let Some(meta_transform) = base_handle.meta_transform() {
(*meta_transform)(&mut *meta); (*meta_transform)(&mut *meta);
} }
match self match self
.load_with_meta_loader_and_reader(&path, meta, &*loader, &mut *reader, true, false) .load_with_meta_loader_and_reader(&base_path, meta, &*loader, &mut *reader, true, false)
.await .await
{ {
Ok(mut loaded_asset) => { Ok(mut loaded_asset) => {
if let Some(label) = path.label_cow() {
if !loaded_asset.labeled_assets.contains_key(&label) {
return Err(AssetLoadError::MissingLabel {
base_path,
label: label.to_string(),
});
}
}
for (_, labeled_asset) in loaded_asset.labeled_assets.drain() { for (_, labeled_asset) in loaded_asset.labeled_assets.drain() {
self.send_asset_event(InternalAssetEvent::Loaded { self.send_asset_event(InternalAssetEvent::Loaded {
id: labeled_asset.handle.id(), id: labeled_asset.handle.id(),
@ -377,13 +382,15 @@ impl AssetServer {
}); });
} }
self.send_asset_event(InternalAssetEvent::Loaded { self.send_asset_event(InternalAssetEvent::Loaded {
id: base_asset_id, id: base_handle.id(),
loaded_asset, loaded_asset,
}); });
Ok(handle) Ok(handle)
} }
Err(err) => { Err(err) => {
self.send_asset_event(InternalAssetEvent::Failed { id: base_asset_id }); self.send_asset_event(InternalAssetEvent::Failed {
id: base_handle.id(),
});
Err(err) Err(err)
} }
} }
@ -935,6 +942,11 @@ pub enum AssetLoadError {
loader_name: &'static str, loader_name: &'static str,
error: Box<dyn std::error::Error + Send + Sync + 'static>, error: Box<dyn std::error::Error + Send + Sync + 'static>,
}, },
#[error("The file at '{base_path}' does not contain the labeled asset '{label}'.")]
MissingLabel {
base_path: AssetPath<'static>,
label: String,
},
} }
/// An error that occurs when an [`AssetLoader`] is not registered for a given extension. /// An error that occurs when an [`AssetLoader`] is not registered for a given extension.