Skip to content

Commit 0294373

Browse files
cartkillercup
andauthored
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 <[email protected]>
1 parent 49d5c6b commit 0294373

File tree

2 files changed

+48
-30
lines changed

2 files changed

+48
-30
lines changed

crates/bevy_asset/src/path.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,12 @@ impl<'a> AssetPath<'a> {
203203
self.label.as_deref()
204204
}
205205

206+
/// Gets the "sub-asset label".
207+
#[inline]
208+
pub fn label_cow(&self) -> Option<CowArc<'a, str>> {
209+
self.label.clone()
210+
}
211+
206212
/// Gets the path to the asset in the "virtual filesystem".
207213
#[inline]
208214
pub fn path(&self) -> &Path {

crates/bevy_asset/src/server/mod.rs

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -257,21 +257,18 @@ impl AssetServer {
257257
path: impl Into<AssetPath<'a>>,
258258
meta_transform: Option<MetaTransform>,
259259
) -> Handle<A> {
260-
let mut path = path.into().into_owned();
260+
let path = path.into().into_owned();
261261
let (handle, should_load) = self.data.infos.write().get_or_create_path_handle::<A>(
262262
path.clone(),
263263
HandleLoadingMode::Request,
264264
meta_transform,
265265
);
266266

267267
if should_load {
268-
let mut owned_handle = Some(handle.clone().untyped());
268+
let owned_handle = Some(handle.clone().untyped());
269269
let server = self.clone();
270270
IoTaskPool::get()
271271
.spawn(async move {
272-
if path.take_label().is_some() {
273-
owned_handle = None;
274-
}
275272
if let Err(err) = server.load_internal(owned_handle, path, false, None).await {
276273
error!("{}", err);
277274
}
@@ -291,14 +288,18 @@ impl AssetServer {
291288
self.load_internal(None, path, false, None).await
292289
}
293290

291+
/// Performs an async asset load.
292+
///
293+
/// `input_handle` must only be [`Some`] if `should_load` was true when retrieving `input_handle`. This is an optimization to
294+
/// avoid looking up `should_load` twice, but it means you _must_ be sure a load is necessary when calling this function with [`Some`].
294295
async fn load_internal<'a>(
295296
&self,
296297
input_handle: Option<UntypedHandle>,
297298
path: AssetPath<'a>,
298299
force: bool,
299300
meta_transform: Option<MetaTransform>,
300301
) -> Result<UntypedHandle, AssetLoadError> {
301-
let mut path = path.into_owned();
302+
let path = path.into_owned();
302303
let path_clone = path.clone();
303304
let (mut meta, loader, mut reader) = self
304305
.get_meta_loader_and_reader(&path_clone)
@@ -312,18 +313,8 @@ impl AssetServer {
312313
e
313314
})?;
314315

315-
let has_label = path.label().is_some();
316-
317316
let (handle, should_load) = match input_handle {
318317
Some(handle) => {
319-
if !has_label && handle.type_id() != loader.asset_type_id() {
320-
return Err(AssetLoadError::RequestedHandleTypeMismatch {
321-
path: path.into_owned(),
322-
requested: handle.type_id(),
323-
actual_asset_name: loader.asset_type_name(),
324-
loader_name: loader.type_name(),
325-
});
326-
}
327318
// if a handle was passed in, the "should load" check was already done
328319
(handle, true)
329320
}
@@ -339,51 +330,67 @@ impl AssetServer {
339330
}
340331
};
341332

333+
if path.label().is_none() && handle.type_id() != loader.asset_type_id() {
334+
return Err(AssetLoadError::RequestedHandleTypeMismatch {
335+
path: path.into_owned(),
336+
requested: handle.type_id(),
337+
actual_asset_name: loader.asset_type_name(),
338+
loader_name: loader.type_name(),
339+
});
340+
}
341+
342342
if !should_load && !force {
343343
return Ok(handle);
344344
}
345-
let base_asset_id = if has_label {
346-
path.remove_label();
347-
// If the path has a label, the current id does not match the asset root type.
348-
// We need to get the actual asset id
345+
346+
let (base_handle, base_path) = if path.label().is_some() {
349347
let mut infos = self.data.infos.write();
350-
let (actual_handle, _) = infos.get_or_create_path_handle_untyped(
351-
path.clone(),
348+
let base_path = path.without_label().into_owned();
349+
let (base_handle, _) = infos.get_or_create_path_handle_untyped(
350+
base_path.clone(),
352351
loader.asset_type_id(),
353352
loader.asset_type_name(),
354-
// ignore current load state ... we kicked off this sub asset load because it needed to be loaded but
355-
// does not currently exist
356353
HandleLoadingMode::Force,
357354
None,
358355
);
359-
actual_handle.id()
356+
(base_handle, base_path)
360357
} else {
361-
handle.id()
358+
(handle.clone(), path.clone())
362359
};
363360

364-
if let Some(meta_transform) = handle.meta_transform() {
361+
if let Some(meta_transform) = base_handle.meta_transform() {
365362
(*meta_transform)(&mut *meta);
366363
}
367364

368365
match self
369-
.load_with_meta_loader_and_reader(&path, meta, &*loader, &mut *reader, true, false)
366+
.load_with_meta_loader_and_reader(&base_path, meta, &*loader, &mut *reader, true, false)
370367
.await
371368
{
372369
Ok(mut loaded_asset) => {
370+
if let Some(label) = path.label_cow() {
371+
if !loaded_asset.labeled_assets.contains_key(&label) {
372+
return Err(AssetLoadError::MissingLabel {
373+
base_path,
374+
label: label.to_string(),
375+
});
376+
}
377+
}
373378
for (_, labeled_asset) in loaded_asset.labeled_assets.drain() {
374379
self.send_asset_event(InternalAssetEvent::Loaded {
375380
id: labeled_asset.handle.id(),
376381
loaded_asset: labeled_asset.asset,
377382
});
378383
}
379384
self.send_asset_event(InternalAssetEvent::Loaded {
380-
id: base_asset_id,
385+
id: base_handle.id(),
381386
loaded_asset,
382387
});
383388
Ok(handle)
384389
}
385390
Err(err) => {
386-
self.send_asset_event(InternalAssetEvent::Failed { id: base_asset_id });
391+
self.send_asset_event(InternalAssetEvent::Failed {
392+
id: base_handle.id(),
393+
});
387394
Err(err)
388395
}
389396
}
@@ -935,6 +942,11 @@ pub enum AssetLoadError {
935942
loader_name: &'static str,
936943
error: Box<dyn std::error::Error + Send + Sync + 'static>,
937944
},
945+
#[error("The file at '{base_path}' does not contain the labeled asset '{label}'.")]
946+
MissingLabel {
947+
base_path: AssetPath<'static>,
948+
label: String,
949+
},
938950
}
939951

940952
/// An error that occurs when an [`AssetLoader`] is not registered for a given extension.

0 commit comments

Comments
 (0)