- resovles #15098

- implements consistently named AssertServer methods for
    asset, dependency, and recursive dependency load states
- adds error to DependencyLoadState::Failed and
    RecursiveDependencyLoadState::Failed
- fixes bug where successful dependency load overwrites
    previous dependency failed load state

Author: crvarner

crates/bevy_asset/src/lib.rs

@@ -1095,13 +1095,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);
@@ -1110,13 +1116,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_dependencies(a_id));
+            assert!(!asset_server.is_loaded_with_recursive_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",

crates/bevy_asset/src/server/info.rs

@@ -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 {
@@ -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);
                     }
@@ -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
                     }
@@ -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()) {
@@ -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) = {
@@ -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,
@@ -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;
                 }
@@ -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 => {
@@ -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,
             ))
@@ -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);
             }
         }
     }
@@ -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),
@@ -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);
         }
     }