Skip to content

Commit

Permalink
AssetServer LoadState API consistency (#15237)
Browse files Browse the repository at this point in the history
# 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
  • Loading branch information
crvarner authored Sep 19, 2024
1 parent 106db47 commit 612897b
Show file tree
Hide file tree
Showing 3 changed files with 244 additions and 41 deletions.
134 changes: 129 additions & 5 deletions crates/bevy_asset/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1246,13 +1246,19 @@ mod tests {

assert!(d_text.is_none());
assert!(matches!(d_load, LoadState::Failed(_)));
assert_eq!(d_deps, DependencyLoadState::Failed);
assert_eq!(d_rec_deps, RecursiveDependencyLoadState::Failed);
assert!(matches!(d_deps, DependencyLoadState::Failed(_)));
assert!(matches!(
d_rec_deps,
RecursiveDependencyLoadState::Failed(_)
));

assert_eq!(a_text.text, "a");
assert_eq!(a_load, LoadState::Loaded);
assert_eq!(a_deps, DependencyLoadState::Loaded);
assert_eq!(a_rec_deps, RecursiveDependencyLoadState::Failed);
assert!(matches!(
a_rec_deps,
RecursiveDependencyLoadState::Failed(_)
));

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

assert_eq!(c_text.text, "c");
assert_eq!(c_load, LoadState::Loaded);
assert_eq!(c_deps, DependencyLoadState::Failed);
assert_eq!(c_rec_deps, RecursiveDependencyLoadState::Failed);
assert!(matches!(c_deps, DependencyLoadState::Failed(_)));
assert!(matches!(
c_rec_deps,
RecursiveDependencyLoadState::Failed(_)
));

assert_eq!(asset_server.load_state(a_id), LoadState::Loaded);
assert_eq!(
asset_server.dependency_load_state(a_id),
DependencyLoadState::Loaded
);
assert!(matches!(
asset_server.recursive_dependency_load_state(a_id),
RecursiveDependencyLoadState::Failed(_)
));

assert!(asset_server.is_loaded(a_id));
assert!(asset_server.is_loaded_with_direct_dependencies(a_id));
assert!(!asset_server.is_loaded_with_dependencies(a_id));

Some(())
});
}

#[test]
fn dependency_load_states() {
// The particular usage of GatedReader in this test will cause deadlocking if running single-threaded
#[cfg(not(feature = "multi_threaded"))]
panic!("This test requires the \"multi_threaded\" feature, otherwise it will deadlock.\ncargo test --package bevy_asset --features multi_threaded");

let a_path = "a.cool.ron";
let a_ron = r#"
(
text: "a",
dependencies: [
"b.cool.ron",
"c.cool.ron",
],
embedded_dependencies: [],
sub_texts: []
)"#;
let b_path = "b.cool.ron";
let b_ron = r#"
(
text: "b",
dependencies: [],
MALFORMED
embedded_dependencies: [],
sub_texts: []
)"#;

let c_path = "c.cool.ron";
let c_ron = r#"
(
text: "c",
dependencies: [],
embedded_dependencies: [],
sub_texts: []
)"#;

let dir = Dir::default();
dir.insert_asset_text(Path::new(a_path), a_ron);
dir.insert_asset_text(Path::new(b_path), b_ron);
dir.insert_asset_text(Path::new(c_path), c_ron);

let (mut app, gate_opener) = test_app(dir);
app.init_asset::<CoolText>()
.register_asset_loader(CoolTextLoader);
let asset_server = app.world().resource::<AssetServer>().clone();
let handle: Handle<CoolText> = asset_server.load(a_path);
let a_id = handle.id();
app.world_mut().spawn(handle);

gate_opener.open(a_path);
run_app_until(&mut app, |world| {
let _a_text = get::<CoolText>(world, a_id)?;
let (a_load, a_deps, a_rec_deps) = asset_server.get_load_states(a_id).unwrap();
assert_eq!(a_load, LoadState::Loaded);
assert_eq!(a_deps, DependencyLoadState::Loading);
assert_eq!(a_rec_deps, RecursiveDependencyLoadState::Loading);
Some(())
});

gate_opener.open(b_path);
run_app_until(&mut app, |world| {
let a_text = get::<CoolText>(world, a_id)?;
let b_id = a_text.dependencies[0].id();

let (b_load, _b_deps, _b_rec_deps) = asset_server.get_load_states(b_id).unwrap();
if !matches!(b_load, LoadState::Failed(_)) {
// wait until b fails
return None;
}

let (a_load, a_deps, a_rec_deps) = asset_server.get_load_states(a_id).unwrap();
assert_eq!(a_load, LoadState::Loaded);
assert!(matches!(a_deps, DependencyLoadState::Failed(_)));
assert!(matches!(
a_rec_deps,
RecursiveDependencyLoadState::Failed(_)
));
Some(())
});

gate_opener.open(c_path);
run_app_until(&mut app, |world| {
let a_text = get::<CoolText>(world, a_id)?;
let c_id = a_text.dependencies[1].id();
// wait until c loads
let _c_text = get::<CoolText>(world, c_id)?;

let (a_load, a_deps, a_rec_deps) = asset_server.get_load_states(a_id).unwrap();
assert_eq!(a_load, LoadState::Loaded);
assert!(
matches!(a_deps, DependencyLoadState::Failed(_)),
"Successful dependency load should not overwrite a previous failure"
);
assert!(
matches!(a_rec_deps, RecursiveDependencyLoadState::Failed(_)),
"Successful dependency load should not overwrite a previous failure"
);
Some(())
});
}

const SIMPLE_TEXT: &str = r#"
(
text: "dep",
Expand Down
47 changes: 31 additions & 16 deletions crates/bevy_asset/src/server/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,8 +388,10 @@ impl AssetInfos {
loaded_asset.value.insert(loaded_asset_id, world);
let mut loading_deps = loaded_asset.dependencies;
let mut failed_deps = HashSet::new();
let mut dep_error = None;
let mut loading_rec_deps = loading_deps.clone();
let mut failed_rec_deps = HashSet::new();
let mut rec_dep_error = None;
loading_deps.retain(|dep_id| {
if let Some(dep_info) = self.get_mut(*dep_id) {
match dep_info.rec_dep_load_state {
Expand All @@ -404,7 +406,10 @@ impl AssetInfos {
// If dependency is loaded, reduce our count by one
loading_rec_deps.remove(dep_id);
}
RecursiveDependencyLoadState::Failed => {
RecursiveDependencyLoadState::Failed(ref error) => {
if rec_dep_error.is_none() {
rec_dep_error = Some(error.clone());
}
failed_rec_deps.insert(*dep_id);
loading_rec_deps.remove(dep_id);
}
Expand All @@ -419,7 +424,10 @@ impl AssetInfos {
// If dependency is loaded, reduce our count by one
false
}
LoadState::Failed(_) => {
LoadState::Failed(ref error) => {
if dep_error.is_none() {
dep_error = Some(error.clone());
}
failed_deps.insert(*dep_id);
false
}
Expand All @@ -437,7 +445,7 @@ impl AssetInfos {
let dep_load_state = match (loading_deps.len(), failed_deps.len()) {
(0, 0) => DependencyLoadState::Loaded,
(_loading, 0) => DependencyLoadState::Loading,
(_loading, _failed) => DependencyLoadState::Failed,
(_loading, _failed) => DependencyLoadState::Failed(dep_error.unwrap()),
};

let rec_dep_load_state = match (loading_rec_deps.len(), failed_rec_deps.len()) {
Expand All @@ -450,7 +458,7 @@ impl AssetInfos {
RecursiveDependencyLoadState::Loaded
}
(_loading, 0) => RecursiveDependencyLoadState::Loading,
(_loading, _failed) => RecursiveDependencyLoadState::Failed,
(_loading, _failed) => RecursiveDependencyLoadState::Failed(rec_dep_error.unwrap()),
};

let (dependants_waiting_on_load, dependants_waiting_on_rec_load) = {
Expand Down Expand Up @@ -480,14 +488,14 @@ impl AssetInfos {
info.failed_rec_dependencies = failed_rec_deps;
info.load_state = LoadState::Loaded;
info.dep_load_state = dep_load_state;
info.rec_dep_load_state = rec_dep_load_state;
info.rec_dep_load_state = rec_dep_load_state.clone();
if watching_for_changes {
info.loader_dependencies = loaded_asset.loader_dependencies;
}

let dependants_waiting_on_rec_load = if matches!(
rec_dep_load_state,
RecursiveDependencyLoadState::Loaded | RecursiveDependencyLoadState::Failed
RecursiveDependencyLoadState::Loaded | RecursiveDependencyLoadState::Failed(_)
) {
Some(std::mem::take(
&mut info.dependants_waiting_on_recursive_dep_load,
Expand All @@ -505,7 +513,9 @@ impl AssetInfos {
for id in dependants_waiting_on_load {
if let Some(info) = self.get_mut(id) {
info.loading_dependencies.remove(&loaded_asset_id);
if info.loading_dependencies.is_empty() {
if info.loading_dependencies.is_empty()
&& !matches!(info.dep_load_state, DependencyLoadState::Failed(_))
{
// send dependencies loaded event
info.dep_load_state = DependencyLoadState::Loaded;
}
Expand All @@ -519,9 +529,9 @@ impl AssetInfos {
Self::propagate_loaded_state(self, loaded_asset_id, dep_id, sender);
}
}
RecursiveDependencyLoadState::Failed => {
RecursiveDependencyLoadState::Failed(ref error) => {
for dep_id in dependants_waiting_on_rec_load {
Self::propagate_failed_state(self, loaded_asset_id, dep_id);
Self::propagate_failed_state(self, loaded_asset_id, dep_id, error);
}
}
RecursiveDependencyLoadState::Loading | RecursiveDependencyLoadState::NotLoaded => {
Expand Down Expand Up @@ -570,11 +580,12 @@ impl AssetInfos {
infos: &mut AssetInfos,
failed_id: UntypedAssetId,
waiting_id: UntypedAssetId,
error: &Arc<AssetLoadError>,
) {
let dependants_waiting_on_rec_load = if let Some(info) = infos.get_mut(waiting_id) {
info.loading_rec_dependencies.remove(&failed_id);
info.failed_rec_dependencies.insert(failed_id);
info.rec_dep_load_state = RecursiveDependencyLoadState::Failed;
info.rec_dep_load_state = RecursiveDependencyLoadState::Failed(error.clone());
Some(std::mem::take(
&mut info.dependants_waiting_on_recursive_dep_load,
))
Expand All @@ -584,7 +595,7 @@ impl AssetInfos {

if let Some(dependants_waiting_on_rec_load) = dependants_waiting_on_rec_load {
for dep_id in dependants_waiting_on_rec_load {
Self::propagate_failed_state(infos, waiting_id, dep_id);
Self::propagate_failed_state(infos, waiting_id, dep_id, error);
}
}
}
Expand All @@ -595,14 +606,15 @@ impl AssetInfos {
return;
}

let error = Arc::new(error);
let (dependants_waiting_on_load, dependants_waiting_on_rec_load) = {
let Some(info) = self.get_mut(failed_id) else {
// The asset was already dropped.
return;
};
info.load_state = LoadState::Failed(Box::new(error));
info.dep_load_state = DependencyLoadState::Failed;
info.rec_dep_load_state = RecursiveDependencyLoadState::Failed;
info.load_state = LoadState::Failed(error.clone());
info.dep_load_state = DependencyLoadState::Failed(error.clone());
info.rec_dep_load_state = RecursiveDependencyLoadState::Failed(error.clone());
(
std::mem::take(&mut info.dependants_waiting_on_load),
std::mem::take(&mut info.dependants_waiting_on_recursive_dep_load),
Expand All @@ -613,12 +625,15 @@ impl AssetInfos {
if let Some(info) = self.get_mut(waiting_id) {
info.loading_dependencies.remove(&failed_id);
info.failed_dependencies.insert(failed_id);
info.dep_load_state = DependencyLoadState::Failed;
// don't overwrite DependencyLoadState if already failed to preserve first error
if !(matches!(info.dep_load_state, DependencyLoadState::Failed(_))) {
info.dep_load_state = DependencyLoadState::Failed(error.clone());
}
}
}

for waiting_id in dependants_waiting_on_rec_load {
Self::propagate_failed_state(self, failed_id, waiting_id);
Self::propagate_failed_state(self, failed_id, waiting_id, &error);
}
}

Expand Down
Loading

0 comments on commit 612897b

Please sign in to comment.