Skip to content

Commit 612897b

Browse files
authored
AssetServer LoadState API consistency (#15237)
# Objective - implements consistently named AssertServer methods for asset, dependency, and recursive dependency load states - returns relevant LoadState when required, including error information for failed loads - resolves #15098 ## Solution - implement consistently named LoadState accessor methods: - load_state, dependency_load_state, recursive_dependency_load_state (return unwrapped load states) - get_load_state, get_dependency_load_state, get_recursive_dependency_load_state (return Option) - is_loaded, is_loaded_with_dependencies, is_loaded_with_recursive_dependencies (return bool) - adds AssetLoadError to DependencyLoadState::Failed and RecursiveDependencyLoadState::Failed ## Testing - Added coverage to existing unit tests
1 parent 106db47 commit 612897b

File tree

3 files changed

+244
-41
lines changed

3 files changed

+244
-41
lines changed

crates/bevy_asset/src/lib.rs

+129-5
Original file line numberDiff line numberDiff line change
@@ -1246,13 +1246,19 @@ mod tests {
12461246

12471247
assert!(d_text.is_none());
12481248
assert!(matches!(d_load, LoadState::Failed(_)));
1249-
assert_eq!(d_deps, DependencyLoadState::Failed);
1250-
assert_eq!(d_rec_deps, RecursiveDependencyLoadState::Failed);
1249+
assert!(matches!(d_deps, DependencyLoadState::Failed(_)));
1250+
assert!(matches!(
1251+
d_rec_deps,
1252+
RecursiveDependencyLoadState::Failed(_)
1253+
));
12511254

12521255
assert_eq!(a_text.text, "a");
12531256
assert_eq!(a_load, LoadState::Loaded);
12541257
assert_eq!(a_deps, DependencyLoadState::Loaded);
1255-
assert_eq!(a_rec_deps, RecursiveDependencyLoadState::Failed);
1258+
assert!(matches!(
1259+
a_rec_deps,
1260+
RecursiveDependencyLoadState::Failed(_)
1261+
));
12561262

12571263
assert_eq!(b_text.text, "b");
12581264
assert_eq!(b_load, LoadState::Loaded);
@@ -1261,13 +1267,131 @@ mod tests {
12611267

12621268
assert_eq!(c_text.text, "c");
12631269
assert_eq!(c_load, LoadState::Loaded);
1264-
assert_eq!(c_deps, DependencyLoadState::Failed);
1265-
assert_eq!(c_rec_deps, RecursiveDependencyLoadState::Failed);
1270+
assert!(matches!(c_deps, DependencyLoadState::Failed(_)));
1271+
assert!(matches!(
1272+
c_rec_deps,
1273+
RecursiveDependencyLoadState::Failed(_)
1274+
));
1275+
1276+
assert_eq!(asset_server.load_state(a_id), LoadState::Loaded);
1277+
assert_eq!(
1278+
asset_server.dependency_load_state(a_id),
1279+
DependencyLoadState::Loaded
1280+
);
1281+
assert!(matches!(
1282+
asset_server.recursive_dependency_load_state(a_id),
1283+
RecursiveDependencyLoadState::Failed(_)
1284+
));
1285+
1286+
assert!(asset_server.is_loaded(a_id));
1287+
assert!(asset_server.is_loaded_with_direct_dependencies(a_id));
1288+
assert!(!asset_server.is_loaded_with_dependencies(a_id));
12661289

12671290
Some(())
12681291
});
12691292
}
12701293

1294+
#[test]
1295+
fn dependency_load_states() {
1296+
// The particular usage of GatedReader in this test will cause deadlocking if running single-threaded
1297+
#[cfg(not(feature = "multi_threaded"))]
1298+
panic!("This test requires the \"multi_threaded\" feature, otherwise it will deadlock.\ncargo test --package bevy_asset --features multi_threaded");
1299+
1300+
let a_path = "a.cool.ron";
1301+
let a_ron = r#"
1302+
(
1303+
text: "a",
1304+
dependencies: [
1305+
"b.cool.ron",
1306+
"c.cool.ron",
1307+
],
1308+
embedded_dependencies: [],
1309+
sub_texts: []
1310+
)"#;
1311+
let b_path = "b.cool.ron";
1312+
let b_ron = r#"
1313+
(
1314+
text: "b",
1315+
dependencies: [],
1316+
MALFORMED
1317+
embedded_dependencies: [],
1318+
sub_texts: []
1319+
)"#;
1320+
1321+
let c_path = "c.cool.ron";
1322+
let c_ron = r#"
1323+
(
1324+
text: "c",
1325+
dependencies: [],
1326+
embedded_dependencies: [],
1327+
sub_texts: []
1328+
)"#;
1329+
1330+
let dir = Dir::default();
1331+
dir.insert_asset_text(Path::new(a_path), a_ron);
1332+
dir.insert_asset_text(Path::new(b_path), b_ron);
1333+
dir.insert_asset_text(Path::new(c_path), c_ron);
1334+
1335+
let (mut app, gate_opener) = test_app(dir);
1336+
app.init_asset::<CoolText>()
1337+
.register_asset_loader(CoolTextLoader);
1338+
let asset_server = app.world().resource::<AssetServer>().clone();
1339+
let handle: Handle<CoolText> = asset_server.load(a_path);
1340+
let a_id = handle.id();
1341+
app.world_mut().spawn(handle);
1342+
1343+
gate_opener.open(a_path);
1344+
run_app_until(&mut app, |world| {
1345+
let _a_text = get::<CoolText>(world, a_id)?;
1346+
let (a_load, a_deps, a_rec_deps) = asset_server.get_load_states(a_id).unwrap();
1347+
assert_eq!(a_load, LoadState::Loaded);
1348+
assert_eq!(a_deps, DependencyLoadState::Loading);
1349+
assert_eq!(a_rec_deps, RecursiveDependencyLoadState::Loading);
1350+
Some(())
1351+
});
1352+
1353+
gate_opener.open(b_path);
1354+
run_app_until(&mut app, |world| {
1355+
let a_text = get::<CoolText>(world, a_id)?;
1356+
let b_id = a_text.dependencies[0].id();
1357+
1358+
let (b_load, _b_deps, _b_rec_deps) = asset_server.get_load_states(b_id).unwrap();
1359+
if !matches!(b_load, LoadState::Failed(_)) {
1360+
// wait until b fails
1361+
return None;
1362+
}
1363+
1364+
let (a_load, a_deps, a_rec_deps) = asset_server.get_load_states(a_id).unwrap();
1365+
assert_eq!(a_load, LoadState::Loaded);
1366+
assert!(matches!(a_deps, DependencyLoadState::Failed(_)));
1367+
assert!(matches!(
1368+
a_rec_deps,
1369+
RecursiveDependencyLoadState::Failed(_)
1370+
));
1371+
Some(())
1372+
});
1373+
1374+
gate_opener.open(c_path);
1375+
run_app_until(&mut app, |world| {
1376+
let a_text = get::<CoolText>(world, a_id)?;
1377+
let c_id = a_text.dependencies[1].id();
1378+
// wait until c loads
1379+
let _c_text = get::<CoolText>(world, c_id)?;
1380+
1381+
let (a_load, a_deps, a_rec_deps) = asset_server.get_load_states(a_id).unwrap();
1382+
assert_eq!(a_load, LoadState::Loaded);
1383+
assert!(
1384+
matches!(a_deps, DependencyLoadState::Failed(_)),
1385+
"Successful dependency load should not overwrite a previous failure"
1386+
);
1387+
assert!(
1388+
matches!(a_rec_deps, RecursiveDependencyLoadState::Failed(_)),
1389+
"Successful dependency load should not overwrite a previous failure"
1390+
);
1391+
Some(())
1392+
});
1393+
}
1394+
12711395
const SIMPLE_TEXT: &str = r#"
12721396
(
12731397
text: "dep",

crates/bevy_asset/src/server/info.rs

+31-16
Original file line numberDiff line numberDiff line change
@@ -388,8 +388,10 @@ impl AssetInfos {
388388
loaded_asset.value.insert(loaded_asset_id, world);
389389
let mut loading_deps = loaded_asset.dependencies;
390390
let mut failed_deps = HashSet::new();
391+
let mut dep_error = None;
391392
let mut loading_rec_deps = loading_deps.clone();
392393
let mut failed_rec_deps = HashSet::new();
394+
let mut rec_dep_error = None;
393395
loading_deps.retain(|dep_id| {
394396
if let Some(dep_info) = self.get_mut(*dep_id) {
395397
match dep_info.rec_dep_load_state {
@@ -404,7 +406,10 @@ impl AssetInfos {
404406
// If dependency is loaded, reduce our count by one
405407
loading_rec_deps.remove(dep_id);
406408
}
407-
RecursiveDependencyLoadState::Failed => {
409+
RecursiveDependencyLoadState::Failed(ref error) => {
410+
if rec_dep_error.is_none() {
411+
rec_dep_error = Some(error.clone());
412+
}
408413
failed_rec_deps.insert(*dep_id);
409414
loading_rec_deps.remove(dep_id);
410415
}
@@ -419,7 +424,10 @@ impl AssetInfos {
419424
// If dependency is loaded, reduce our count by one
420425
false
421426
}
422-
LoadState::Failed(_) => {
427+
LoadState::Failed(ref error) => {
428+
if dep_error.is_none() {
429+
dep_error = Some(error.clone());
430+
}
423431
failed_deps.insert(*dep_id);
424432
false
425433
}
@@ -437,7 +445,7 @@ impl AssetInfos {
437445
let dep_load_state = match (loading_deps.len(), failed_deps.len()) {
438446
(0, 0) => DependencyLoadState::Loaded,
439447
(_loading, 0) => DependencyLoadState::Loading,
440-
(_loading, _failed) => DependencyLoadState::Failed,
448+
(_loading, _failed) => DependencyLoadState::Failed(dep_error.unwrap()),
441449
};
442450

443451
let rec_dep_load_state = match (loading_rec_deps.len(), failed_rec_deps.len()) {
@@ -450,7 +458,7 @@ impl AssetInfos {
450458
RecursiveDependencyLoadState::Loaded
451459
}
452460
(_loading, 0) => RecursiveDependencyLoadState::Loading,
453-
(_loading, _failed) => RecursiveDependencyLoadState::Failed,
461+
(_loading, _failed) => RecursiveDependencyLoadState::Failed(rec_dep_error.unwrap()),
454462
};
455463

456464
let (dependants_waiting_on_load, dependants_waiting_on_rec_load) = {
@@ -480,14 +488,14 @@ impl AssetInfos {
480488
info.failed_rec_dependencies = failed_rec_deps;
481489
info.load_state = LoadState::Loaded;
482490
info.dep_load_state = dep_load_state;
483-
info.rec_dep_load_state = rec_dep_load_state;
491+
info.rec_dep_load_state = rec_dep_load_state.clone();
484492
if watching_for_changes {
485493
info.loader_dependencies = loaded_asset.loader_dependencies;
486494
}
487495

488496
let dependants_waiting_on_rec_load = if matches!(
489497
rec_dep_load_state,
490-
RecursiveDependencyLoadState::Loaded | RecursiveDependencyLoadState::Failed
498+
RecursiveDependencyLoadState::Loaded | RecursiveDependencyLoadState::Failed(_)
491499
) {
492500
Some(std::mem::take(
493501
&mut info.dependants_waiting_on_recursive_dep_load,
@@ -505,7 +513,9 @@ impl AssetInfos {
505513
for id in dependants_waiting_on_load {
506514
if let Some(info) = self.get_mut(id) {
507515
info.loading_dependencies.remove(&loaded_asset_id);
508-
if info.loading_dependencies.is_empty() {
516+
if info.loading_dependencies.is_empty()
517+
&& !matches!(info.dep_load_state, DependencyLoadState::Failed(_))
518+
{
509519
// send dependencies loaded event
510520
info.dep_load_state = DependencyLoadState::Loaded;
511521
}
@@ -519,9 +529,9 @@ impl AssetInfos {
519529
Self::propagate_loaded_state(self, loaded_asset_id, dep_id, sender);
520530
}
521531
}
522-
RecursiveDependencyLoadState::Failed => {
532+
RecursiveDependencyLoadState::Failed(ref error) => {
523533
for dep_id in dependants_waiting_on_rec_load {
524-
Self::propagate_failed_state(self, loaded_asset_id, dep_id);
534+
Self::propagate_failed_state(self, loaded_asset_id, dep_id, error);
525535
}
526536
}
527537
RecursiveDependencyLoadState::Loading | RecursiveDependencyLoadState::NotLoaded => {
@@ -570,11 +580,12 @@ impl AssetInfos {
570580
infos: &mut AssetInfos,
571581
failed_id: UntypedAssetId,
572582
waiting_id: UntypedAssetId,
583+
error: &Arc<AssetLoadError>,
573584
) {
574585
let dependants_waiting_on_rec_load = if let Some(info) = infos.get_mut(waiting_id) {
575586
info.loading_rec_dependencies.remove(&failed_id);
576587
info.failed_rec_dependencies.insert(failed_id);
577-
info.rec_dep_load_state = RecursiveDependencyLoadState::Failed;
588+
info.rec_dep_load_state = RecursiveDependencyLoadState::Failed(error.clone());
578589
Some(std::mem::take(
579590
&mut info.dependants_waiting_on_recursive_dep_load,
580591
))
@@ -584,7 +595,7 @@ impl AssetInfos {
584595

585596
if let Some(dependants_waiting_on_rec_load) = dependants_waiting_on_rec_load {
586597
for dep_id in dependants_waiting_on_rec_load {
587-
Self::propagate_failed_state(infos, waiting_id, dep_id);
598+
Self::propagate_failed_state(infos, waiting_id, dep_id, error);
588599
}
589600
}
590601
}
@@ -595,14 +606,15 @@ impl AssetInfos {
595606
return;
596607
}
597608

609+
let error = Arc::new(error);
598610
let (dependants_waiting_on_load, dependants_waiting_on_rec_load) = {
599611
let Some(info) = self.get_mut(failed_id) else {
600612
// The asset was already dropped.
601613
return;
602614
};
603-
info.load_state = LoadState::Failed(Box::new(error));
604-
info.dep_load_state = DependencyLoadState::Failed;
605-
info.rec_dep_load_state = RecursiveDependencyLoadState::Failed;
615+
info.load_state = LoadState::Failed(error.clone());
616+
info.dep_load_state = DependencyLoadState::Failed(error.clone());
617+
info.rec_dep_load_state = RecursiveDependencyLoadState::Failed(error.clone());
606618
(
607619
std::mem::take(&mut info.dependants_waiting_on_load),
608620
std::mem::take(&mut info.dependants_waiting_on_recursive_dep_load),
@@ -613,12 +625,15 @@ impl AssetInfos {
613625
if let Some(info) = self.get_mut(waiting_id) {
614626
info.loading_dependencies.remove(&failed_id);
615627
info.failed_dependencies.insert(failed_id);
616-
info.dep_load_state = DependencyLoadState::Failed;
628+
// don't overwrite DependencyLoadState if already failed to preserve first error
629+
if !(matches!(info.dep_load_state, DependencyLoadState::Failed(_))) {
630+
info.dep_load_state = DependencyLoadState::Failed(error.clone());
631+
}
617632
}
618633
}
619634

620635
for waiting_id in dependants_waiting_on_rec_load {
621-
Self::propagate_failed_state(self, failed_id, waiting_id);
636+
Self::propagate_failed_state(self, failed_id, waiting_id, &error);
622637
}
623638
}
624639

0 commit comments

Comments
 (0)