Fix GLTF scene dependencies and make full scene renders predictable (#…

…10745)

# Objective

Fixes #10688

There were a number of issues at play:

1. The GLTF loader was not registering Scene dependencies properly. They
were being registered at the root instead of on the scene assets. This
made `LoadedWithDependencies` fire immediately on load.
2. Recursive labeled assets _inside_ of labeled assets were not being
loaded. This only became relevant for scenes after fixing (1) because we
now add labeled assets to the nested scene `LoadContext` instead of the
root load context. I'm surprised nobody has hit this yet. I'm glad I
caught it before somebody hit it.
3. Accessing "loaded with dependencies" state on the Asset Server is
boilerplatey + error prone (because you need to manually query two
states).

## Solution

1. In GltfLoader, use a nested LoadContext for scenes and load
dependencies through that context.
2. In the `AssetServer`, load labeled assets recursively.
3. Added a simple `asset_server.is_loaded_with_dependencies(id)`

I also added some docs to `LoadContext` to help prevent this problem in
the future.

---

## Changelog

- Added `AssetServer::is_loaded_with_dependencies`
- Fixed GLTF Scene dependencies
- Fixed nested labeled assets not being loaded

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

crates/bevy_asset/src/loader.rs

@@ -353,6 +353,13 @@ impl<'a> LoadContext<'a> {
 
     /// This will add the given `asset` as a "labeled [`Asset`]" with the `label` label.
     ///
+    /// # Warning
+    ///
+    /// This will not assign dependencies to the given `asset`. If adding an asset
+    /// with dependencies generated from calls such as [`LoadContext::load`], use
+    /// [`LoadContext::labeled_asset_scope`] or [`LoadContext::begin_labeled_asset`] to generate a
+    /// new [`LoadContext`] to track the dependencies for the labeled asset.
+    ///
     /// See [`AssetPath`] for more on labeled assets.
     pub fn add_labeled_asset<A: Asset>(&mut self, label: String, asset: A) -> Handle<A> {
         self.labeled_asset_scope(label, |_| asset)

crates/bevy_asset/src/server/info.rs

@@ -260,6 +260,10 @@ impl AssetInfos {
         self.infos.get(&id)
     }
 
+    pub(crate) fn contains_key(&self, id: UntypedAssetId) -> bool {
+        self.infos.contains_key(&id)
+    }
+
     pub(crate) fn get_mut(&mut self, id: UntypedAssetId) -> Option<&mut AssetInfo> {
         self.infos.get_mut(&id)
     }

crates/bevy_asset/src/server/mod.rs

@@ -482,7 +482,7 @@ impl AssetServer {
             .load_with_meta_loader_and_reader(&base_path, meta, &*loader, &mut *reader, true, false)
             .await
         {
-            Ok(mut loaded_asset) => {
+            Ok(loaded_asset) => {
                 let final_handle = if let Some(label) = path.label_cow() {
                     match loaded_asset.labeled_assets.get(&label) {
                         Some(labeled_asset) => labeled_asset.handle.clone(),
@@ -498,17 +498,7 @@ impl AssetServer {
                     handle.unwrap()
                 };
 
-                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_handle.id(),
-                    loaded_asset,
-                });
+                self.send_loaded_asset(base_handle.id(), loaded_asset);
                 Ok(final_handle)
             }
             Err(err) => {
@@ -520,6 +510,16 @@ impl AssetServer {
         }
     }
 
+    /// Sends a load event for the given `loaded_asset` and does the same recursively for all
+    /// labeled assets.
+    fn send_loaded_asset(&self, id: UntypedAssetId, mut loaded_asset: ErasedLoadedAsset) {
+        for (_, labeled_asset) in loaded_asset.labeled_assets.drain() {
+            self.send_loaded_asset(labeled_asset.handle.id(), labeled_asset.asset);
+        }
+
+        self.send_asset_event(InternalAssetEvent::Loaded { id, loaded_asset });
+    }
+
     /// Kicks off a reload of the asset stored at the given path. This will only reload the asset if it currently loaded.
     pub fn reload<'a>(&self, path: impl Into<AssetPath<'a>>) {
         let server = self.clone();
@@ -707,6 +707,9 @@ impl AssetServer {
     }
 
     /// Retrieves the main [`LoadState`] of a given asset `id`.
+    ///
+    /// Note that this is "just" the root asset load state. To check if an asset _and_ its recursive
+    /// dependencies have loaded, see [`AssetServer::is_loaded_with_dependencies`].
     pub fn get_load_state(&self, id: impl Into<UntypedAssetId>) -> Option<LoadState> {
         self.data.infos.read().get(id.into()).map(|i| i.load_state)
     }
@@ -737,6 +740,13 @@ impl AssetServer {
             .unwrap_or(RecursiveDependencyLoadState::NotLoaded)
     }
 
+    /// Returns true if the asset and all of its dependencies (recursive) have been loaded.
+    pub fn is_loaded_with_dependencies(&self, id: impl Into<UntypedAssetId>) -> bool {
+        let id = id.into();
+        self.load_state(id) == LoadState::Loaded
+            && self.recursive_dependency_load_state(id) == RecursiveDependencyLoadState::Loaded
+    }
+
     /// Returns an active handle for the given path, if the asset at the given path has already started loading,
     /// or is still "alive".
     pub fn get_handle<'a, A: Asset>(&self, path: impl Into<AssetPath<'a>>) -> Option<Handle<A>> {
@@ -752,6 +762,12 @@ impl AssetServer {
         self.data.infos.read().get_id_handle(id)
     }
 
+    /// Returns `true` if the given `id` corresponds to an asset that is managed by this [`AssetServer`].
+    /// Otherwise, returns false.
+    pub fn is_managed(&self, id: impl Into<UntypedAssetId>) -> bool {
+        self.data.infos.read().contains_key(id.into())
+    }
+
     /// Returns an active untyped handle for the given path, if the asset at the given path has already started loading,
     /// or is still "alive".
     pub fn get_handle_untyped<'a>(&self, path: impl Into<AssetPath<'a>>) -> Option<UntypedHandle> {

crates/bevy_gltf/src/loader.rs

@@ -545,7 +545,7 @@ async fn load_gltf<'a, 'b, 'c>(
         let mut world = World::default();
         let mut node_index_to_entity_map = HashMap::new();
         let mut entity_to_skin_index_map = HashMap::new();
-
+        let mut scene_load_context = load_context.begin_labeled_asset();
         world
             .spawn(SpatialBundle::INHERITED_IDENTITY)
             .with_children(|parent| {
@@ -554,6 +554,7 @@ async fn load_gltf<'a, 'b, 'c>(
                         &node,
                         parent,
                         load_context,
+                        &mut scene_load_context,
                         &mut node_index_to_entity_map,
                         &mut entity_to_skin_index_map,
                         &mut active_camera_found,
@@ -606,8 +607,8 @@ async fn load_gltf<'a, 'b, 'c>(
                 joints: joint_entities,
             });
         }
-
-        let scene_handle = load_context.add_labeled_asset(scene_label(&scene), Scene::new(world));
+        let loaded_scene = scene_load_context.finish(Scene::new(world), None);
+        let scene_handle = load_context.add_loaded_labeled_asset(scene_label(&scene), loaded_scene);
 
         if let Some(name) = scene.name() {
             named_scenes.insert(name.to_string(), scene_handle.clone());
@@ -853,9 +854,11 @@ fn load_material(
 }
 
 /// Loads a glTF node.
+#[allow(clippy::too_many_arguments)]
 fn load_node(
     gltf_node: &gltf::Node,
     world_builder: &mut WorldChildBuilder,
+    root_load_context: &LoadContext,
     load_context: &mut LoadContext,
     node_index_to_entity_map: &mut HashMap<usize, Entity>,
     entity_to_skin_index_map: &mut HashMap<Entity, usize>,
@@ -942,7 +945,9 @@ fn load_node(
                 // added when iterating over all the gltf materials (since the default material is
                 // not explicitly listed in the gltf).
                 // It also ensures an inverted scale copy is instantiated if required.
-                if !load_context.has_labeled_asset(&material_label) {
+                if !root_load_context.has_labeled_asset(&material_label)
+                    && !load_context.has_labeled_asset(&material_label)
+                {
                     load_material(&material, load_context, is_scale_inverted);
                 }
 
@@ -1074,6 +1079,7 @@ fn load_node(
             if let Err(err) = load_node(
                 &child,
                 parent,
+                root_load_context,
                 load_context,
                 node_index_to_entity_map,
                 entity_to_skin_index_map,