Skip to content

Commit

Permalink
Return an error when loading non-existent labels (bevyengine#9751)
Browse files Browse the repository at this point in the history 
# Objective

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

Referenced in bevyengine#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 <[email protected]>
  • Loading branch information
@cart @killercup @ameknite
2 people authored and ameknite committed Nov 6, 2023
1 parent aaaae25 commit d8b3dd9
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 30 deletions.
6 changes: 6 additions & 0 deletions crates/bevy_asset/src/path.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,12 @@ impl<'a> AssetPath<'a> {
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".
#[inline]
pub fn path(&self) -> &Path {
Expand Down
72 changes: 42 additions & 30 deletions crates/bevy_asset/src/server/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,21 +257,18 @@ impl AssetServer {
path: impl Into<AssetPath<'a>>,
meta_transform: Option<MetaTransform>,
) -> 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>(
path.clone(),
HandleLoadingMode::Request,
meta_transform,
);

if should_load {
let mut owned_handle = Some(handle.clone().untyped());
let owned_handle = Some(handle.clone().untyped());
let server = self.clone();
IoTaskPool::get()
.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 {
error!("{}", err);
}
Expand All @@ -291,14 +288,18 @@ impl AssetServer {
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>(
&self,
input_handle: Option<UntypedHandle>,
path: AssetPath<'a>,
force: bool,
meta_transform: Option<MetaTransform>,
) -> Result<UntypedHandle, AssetLoadError> {
let mut path = path.into_owned();
let path = path.into_owned();
let path_clone = path.clone();
let (mut meta, loader, mut reader) = self
.get_meta_loader_and_reader(&path_clone)
Expand All @@ -312,18 +313,8 @@ impl AssetServer {
e
})?;

let has_label = path.label().is_some();

let (handle, should_load) = match input_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
(handle, true)
}
Expand All @@ -339,51 +330,67 @@ 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 {
return Ok(handle);
}
let base_asset_id = if has_label {
path.remove_label();
// 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 (base_handle, base_path) = if path.label().is_some() {
let mut infos = self.data.infos.write();
let (actual_handle, _) = infos.get_or_create_path_handle_untyped(
path.clone(),
let base_path = path.without_label().into_owned();
let (base_handle, _) = infos.get_or_create_path_handle_untyped(
base_path.clone(),
loader.asset_type_id(),
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,
None,
);
actual_handle.id()
(base_handle, base_path)
} 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);
}

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
{
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() {
self.send_asset_event(InternalAssetEvent::Loaded {
id: labeled_asset.handle.id(),
loaded_asset: labeled_asset.asset,
});
}
self.send_asset_event(InternalAssetEvent::Loaded {
id: base_asset_id,
id: base_handle.id(),
loaded_asset,
});
Ok(handle)
}
Err(err) => {
self.send_asset_event(InternalAssetEvent::Failed { id: base_asset_id });
self.send_asset_event(InternalAssetEvent::Failed {
id: base_handle.id(),
});
Err(err)
}
}
Expand Down Expand Up @@ -935,6 +942,11 @@ pub enum AssetLoadError {
loader_name: &'static str,
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.
Expand Down

0 comments on commit d8b3dd9

Please sign in to comment.