Remove labeled_assets from LoadedAsset and ErasedLoadedAsset (b…

…evyengine#15481)

# Objective

Fixes bevyengine#15417.

## Solution

- Remove the `labeled_assets` fields from `LoadedAsset` and
`ErasedLoadedAsset`.
- Created new structs `CompleteLoadedAsset` and
`CompleteErasedLoadedAsset` to hold the `labeled_subassets`.
- When a subasset is `LoadContext::finish`ed, it produces a
`CompleteLoadedAsset`.
- When a `CompleteLoadedAsset` is added to a `LoadContext` (as a
subasset), their `labeled_assets` are merged, reporting any overlaps.

One important detail to note: nested subassets with overlapping names
could in theory have been used in the past for the purposes of asset
preprocessing. Even though there was no way to access these "shadowed"
nested subassets, asset preprocessing does get access to these nested
subassets. This does not seem like a case we should support though. It
is confusing at best.

## Testing

- This is just a refactor.

---

## Migration Guide

- Most uses of `LoadedAsset` and `ErasedLoadedAsset` should be replaced
with `CompleteLoadedAsset` and `CompleteErasedLoadedAsset` respectively.

crates/bevy_asset/Cargo.toml

@@ -22,6 +22,7 @@ trace = []
 bevy_app = { path = "../bevy_app", version = "0.16.0-dev" }
 bevy_asset_macros = { path = "macros", version = "0.16.0-dev" }
 bevy_ecs = { path = "../bevy_ecs", version = "0.16.0-dev" }
+bevy_log = { path = "../bevy_log", version = "0.16.0-dev" }
 bevy_reflect = { path = "../bevy_reflect", version = "0.16.0-dev", features = [
   "uuid",
 ] }
@@ -69,9 +70,6 @@ uuid = { version = "1.13.1", default-features = false, features = ["js"] }
 [target.'cfg(not(target_arch = "wasm32"))'.dependencies]
 notify-debouncer-full = { version = "0.5.0", optional = true }
 
-[dev-dependencies]
-bevy_log = { path = "../bevy_log", version = "0.16.0-dev" }
-
 [lints]
 workspace = true
 

crates/bevy_asset/src/lib.rs

@@ -725,7 +725,7 @@ mod tests {
                     .map_err(|_| Self::Error::CannotLoadDependency {
                         dependency: dep.into(),
                     })?;
-                let cool = loaded.get();
+                let cool = loaded.get_asset().get();
                 embedded.push_str(&cool.text);
             }
             Ok(CoolText {

crates/bevy_asset/src/loader.rs

@@ -13,6 +13,7 @@ use alloc::{
 };
 use atomicow::CowArc;
 use bevy_ecs::world::World;
+use bevy_log::warn;
 use bevy_platform_support::collections::{HashMap, HashSet};
 use bevy_tasks::{BoxedFuture, ConditionalSendFuture};
 use core::any::{Any, TypeId};
@@ -60,7 +61,7 @@ pub trait ErasedAssetLoader: Send + Sync + 'static {
         load_context: LoadContext<'a>,
     ) -> BoxedFuture<
         'a,
-        Result<ErasedLoadedAsset, Box<dyn core::error::Error + Send + Sync + 'static>>,
+        Result<CompleteErasedLoadedAsset, Box<dyn core::error::Error + Send + Sync + 'static>>,
     >;
 
     /// Returns a list of extensions supported by this asset loader, without the preceding dot.
@@ -91,7 +92,7 @@ where
         mut load_context: LoadContext<'a>,
     ) -> BoxedFuture<
         'a,
-        Result<ErasedLoadedAsset, Box<dyn core::error::Error + Send + Sync + 'static>>,
+        Result<CompleteErasedLoadedAsset, Box<dyn core::error::Error + Send + Sync + 'static>>,
     > {
         Box::pin(async move {
             let settings = meta
@@ -152,7 +153,6 @@ pub struct LoadedAsset<A: Asset> {
     pub(crate) value: A,
     pub(crate) dependencies: HashSet<UntypedAssetId>,
     pub(crate) loader_dependencies: HashMap<AssetPath<'static>, AssetHash>,
-    pub(crate) labeled_assets: HashMap<CowArc<'static, str>, LabeledAsset>,
 }
 
 impl<A: Asset> LoadedAsset<A> {
@@ -166,7 +166,6 @@ impl<A: Asset> LoadedAsset<A> {
             value,
             dependencies,
             loader_dependencies: HashMap::default(),
-            labeled_assets: HashMap::default(),
         }
     }
 
@@ -179,19 +178,6 @@ impl<A: Asset> LoadedAsset<A> {
     pub fn get(&self) -> &A {
         &self.value
     }
-
-    /// Returns the [`ErasedLoadedAsset`] for the given label, if it exists.
-    pub fn get_labeled(
-        &self,
-        label: impl Into<CowArc<'static, str>>,
-    ) -> Option<&ErasedLoadedAsset> {
-        self.labeled_assets.get(&label.into()).map(|a| &a.asset)
-    }
-
-    /// Iterate over all labels for "labeled assets" in the loaded asset
-    pub fn iter_labels(&self) -> impl Iterator<Item = &str> {
-        self.labeled_assets.keys().map(|s| &**s)
-    }
 }
 
 impl<A: Asset> From<A> for LoadedAsset<A> {
@@ -205,7 +191,6 @@ pub struct ErasedLoadedAsset {
     pub(crate) value: Box<dyn AssetContainer>,
     pub(crate) dependencies: HashSet<UntypedAssetId>,
     pub(crate) loader_dependencies: HashMap<AssetPath<'static>, AssetHash>,
-    pub(crate) labeled_assets: HashMap<CowArc<'static, str>, LabeledAsset>,
 }
 
 impl<A: Asset> From<LoadedAsset<A>> for ErasedLoadedAsset {
@@ -214,7 +199,6 @@ impl<A: Asset> From<LoadedAsset<A>> for ErasedLoadedAsset {
             value: Box::new(asset.value),
             dependencies: asset.dependencies,
             loader_dependencies: asset.loader_dependencies,
-            labeled_assets: asset.labeled_assets,
         }
     }
 }
@@ -241,19 +225,6 @@ impl ErasedLoadedAsset {
         self.value.asset_type_name()
     }
 
-    /// Returns the [`ErasedLoadedAsset`] for the given label, if it exists.
-    pub fn get_labeled(
-        &self,
-        label: impl Into<CowArc<'static, str>>,
-    ) -> Option<&ErasedLoadedAsset> {
-        self.labeled_assets.get(&label.into()).map(|a| &a.asset)
-    }
-
-    /// Iterate over all labels for "labeled assets" in the loaded asset
-    pub fn iter_labels(&self) -> impl Iterator<Item = &str> {
-        self.labeled_assets.keys().map(|s| &**s)
-    }
-
     /// Cast this loaded asset as the given type. If the type does not match,
     /// the original type-erased asset is returned.
     pub fn downcast<A: Asset>(mut self) -> Result<LoadedAsset<A>, ErasedLoadedAsset> {
@@ -262,7 +233,6 @@ impl ErasedLoadedAsset {
                 value: *value,
                 dependencies: self.dependencies,
                 loader_dependencies: self.loader_dependencies,
-                labeled_assets: self.labeled_assets,
             }),
             Err(value) => {
                 self.value = value;
@@ -290,6 +260,100 @@ impl<A: Asset> AssetContainer for A {
     }
 }
 
+/// A loaded asset and all its loaded subassets.
+pub struct CompleteLoadedAsset<A: Asset> {
+    /// The loaded asset.
+    pub(crate) asset: LoadedAsset<A>,
+    /// The subassets by their label.
+    pub(crate) labeled_assets: HashMap<CowArc<'static, str>, LabeledAsset>,
+}
+
+impl<A: Asset> CompleteLoadedAsset<A> {
+    /// Take ownership of the stored [`Asset`] value.
+    pub fn take(self) -> A {
+        self.asset.value
+    }
+
+    /// Returns the stored asset.
+    pub fn get_asset(&self) -> &LoadedAsset<A> {
+        &self.asset
+    }
+
+    /// Returns the [`ErasedLoadedAsset`] for the given label, if it exists.
+    pub fn get_labeled(
+        &self,
+        label: impl Into<CowArc<'static, str>>,
+    ) -> Option<&ErasedLoadedAsset> {
+        self.labeled_assets.get(&label.into()).map(|a| &a.asset)
+    }
+
+    /// Iterate over all labels for "labeled assets" in the loaded asset
+    pub fn iter_labels(&self) -> impl Iterator<Item = &str> {
+        self.labeled_assets.keys().map(|s| &**s)
+    }
+}
+
+/// A "type erased / boxed" counterpart to [`CompleteLoadedAsset`]. This is used in places where the
+/// loaded type is not statically known.
+pub struct CompleteErasedLoadedAsset {
+    /// The loaded asset.
+    pub(crate) asset: ErasedLoadedAsset,
+    /// The subassets by their label.
+    pub(crate) labeled_assets: HashMap<CowArc<'static, str>, LabeledAsset>,
+}
+
+impl CompleteErasedLoadedAsset {
+    /// Cast (and take ownership) of the [`Asset`] value of the given type. This will return
+    /// [`Some`] if the stored type matches `A` and [`None`] if it does not.
+    pub fn take<A: Asset>(self) -> Option<A> {
+        self.asset.take()
+    }
+
+    /// Returns the stored asset.
+    pub fn get_asset(&self) -> &ErasedLoadedAsset {
+        &self.asset
+    }
+
+    /// Returns the [`ErasedLoadedAsset`] for the given label, if it exists.
+    pub fn get_labeled(
+        &self,
+        label: impl Into<CowArc<'static, str>>,
+    ) -> Option<&ErasedLoadedAsset> {
+        self.labeled_assets.get(&label.into()).map(|a| &a.asset)
+    }
+
+    /// Iterate over all labels for "labeled assets" in the loaded asset
+    pub fn iter_labels(&self) -> impl Iterator<Item = &str> {
+        self.labeled_assets.keys().map(|s| &**s)
+    }
+
+    /// Cast this loaded asset as the given type. If the type does not match,
+    /// the original type-erased asset is returned.
+    pub fn downcast<A: Asset>(
+        mut self,
+    ) -> Result<CompleteLoadedAsset<A>, CompleteErasedLoadedAsset> {
+        match self.asset.downcast::<A>() {
+            Ok(asset) => Ok(CompleteLoadedAsset {
+                asset,
+                labeled_assets: self.labeled_assets,
+            }),
+            Err(asset) => {
+                self.asset = asset;
+                Err(self)
+            }
+        }
+    }
+}
+
+impl<A: Asset> From<CompleteLoadedAsset<A>> for CompleteErasedLoadedAsset {
+    fn from(value: CompleteLoadedAsset<A>) -> Self {
+        Self {
+            asset: value.asset.into(),
+            labeled_assets: value.labeled_assets,
+        }
+    }
+}
+
 /// An error that occurs when attempting to call [`NestedLoader::load`] which
 /// is configured to work [immediately].
 ///
@@ -397,8 +461,8 @@ impl<'a> LoadContext<'a> {
     ) -> Handle<A> {
         let mut context = self.begin_labeled_asset();
         let asset = load(&mut context);
-        let loaded_asset = context.finish(asset);
-        self.add_loaded_labeled_asset(label, loaded_asset)
+        let complete_asset = context.finish(asset);
+        self.add_loaded_labeled_asset(label, complete_asset)
     }
 
     /// This will add the given `asset` as a "labeled [`Asset`]" with the `label` label.
@@ -423,10 +487,14 @@ impl<'a> LoadContext<'a> {
     pub fn add_loaded_labeled_asset<A: Asset>(
         &mut self,
         label: impl Into<CowArc<'static, str>>,
-        loaded_asset: LoadedAsset<A>,
+        loaded_asset: CompleteLoadedAsset<A>,
     ) -> Handle<A> {
         let label = label.into();
-        let loaded_asset: ErasedLoadedAsset = loaded_asset.into();
+        let CompleteLoadedAsset {
+            asset,
+            labeled_assets,
+        } = loaded_asset;
+        let loaded_asset: ErasedLoadedAsset = asset.into();
         let labeled_path = self.asset_path.clone().with_label(label.clone());
         let handle = self
             .asset_server
@@ -438,6 +506,11 @@ impl<'a> LoadContext<'a> {
                 handle: handle.clone().untyped(),
             },
         );
+        for (label, asset) in labeled_assets {
+            if self.labeled_assets.insert(label.clone(), asset).is_some() {
+                warn!("A labeled asset with the label \"{label}\" already exists. Replacing with the new asset.");
+            }
+        }
         handle
     }
 
@@ -450,11 +523,13 @@ impl<'a> LoadContext<'a> {
     }
 
     /// "Finishes" this context by populating the final [`Asset`] value.
-    pub fn finish<A: Asset>(self, value: A) -> LoadedAsset<A> {
-        LoadedAsset {
-            value,
-            dependencies: self.dependencies,
-            loader_dependencies: self.loader_dependencies,
+    pub fn finish<A: Asset>(self, value: A) -> CompleteLoadedAsset<A> {
+        CompleteLoadedAsset {
+            asset: LoadedAsset {
+                value,
+                dependencies: self.dependencies,
+                loader_dependencies: self.loader_dependencies,
+            },
             labeled_assets: self.labeled_assets,
         }
     }
@@ -525,8 +600,8 @@ impl<'a> LoadContext<'a> {
         meta: &dyn AssetMetaDyn,
         loader: &dyn ErasedAssetLoader,
         reader: &mut dyn Reader,
-    ) -> Result<ErasedLoadedAsset, LoadDirectError> {
-        let loaded_asset = self
+    ) -> Result<CompleteErasedLoadedAsset, LoadDirectError> {
+        let complete_asset = self
             .asset_server
             .load_with_meta_loader_and_reader(
                 &path,
@@ -544,7 +619,7 @@ impl<'a> LoadContext<'a> {
         let info = meta.processed_info().as_ref();
         let hash = info.map(|i| i.full_hash).unwrap_or_default();
         self.loader_dependencies.insert(path, hash);
-        Ok(loaded_asset)
+        Ok(complete_asset)
     }
 
     /// Create a builder for loading nested assets in this context.

crates/bevy_asset/src/loader_builders.rs

@@ -4,8 +4,8 @@
 use crate::{
     io::Reader,
     meta::{meta_transform_settings, AssetMetaDyn, MetaTransform, Settings},
-    Asset, AssetLoadError, AssetPath, ErasedAssetLoader, ErasedLoadedAsset, Handle, LoadContext,
-    LoadDirectError, LoadedAsset, LoadedUntypedAsset, UntypedHandle,
+    Asset, AssetLoadError, AssetPath, CompleteErasedLoadedAsset, CompleteLoadedAsset,
+    ErasedAssetLoader, Handle, LoadContext, LoadDirectError, LoadedUntypedAsset, UntypedHandle,
 };
 use alloc::{borrow::ToOwned, boxed::Box, sync::Arc};
 use core::any::TypeId;
@@ -57,11 +57,11 @@ impl ReaderRef<'_> {
 ///   If you know the type ID of the asset at runtime, but not at compile time,
 ///   use [`with_dynamic_type`] followed by [`load`] to start loading an asset
 ///   of that type. This lets you get an [`UntypedHandle`] (via [`Deferred`]),
-///   or a [`ErasedLoadedAsset`] (via [`Immediate`]).
+///   or a [`CompleteErasedLoadedAsset`] (via [`Immediate`]).
 ///
 /// - in [`UnknownTyped`]: loading either a type-erased version of the asset
-///   ([`ErasedLoadedAsset`]), or a handle *to a handle* of the actual asset
-///   ([`LoadedUntypedAsset`]).
+///   ([`CompleteErasedLoadedAsset`]), or a handle *to a handle* of the actual
+///   asset ([`LoadedUntypedAsset`]).
 ///
 ///   If you have no idea what type of asset you will be loading (not even at
 ///   runtime with a [`TypeId`]), use this.
@@ -386,7 +386,7 @@ impl<'builder, 'reader, T> NestedLoader<'_, '_, T, Immediate<'builder, 'reader>>
         self,
         path: &AssetPath<'static>,
         asset_type_id: Option<TypeId>,
-    ) -> Result<(Arc<dyn ErasedAssetLoader>, ErasedLoadedAsset), LoadDirectError> {
+    ) -> Result<(Arc<dyn ErasedAssetLoader>, CompleteErasedLoadedAsset), LoadDirectError> {
         let (mut meta, loader, mut reader) = if let Some(reader) = self.mode.reader {
             let loader = if let Some(asset_type_id) = asset_type_id {
                 self.load_context
@@ -448,7 +448,7 @@ impl NestedLoader<'_, '_, StaticTyped, Immediate<'_, '_>> {
     pub async fn load<'p, A: Asset>(
         self,
         path: impl Into<AssetPath<'p>>,
-    ) -> Result<LoadedAsset<A>, LoadDirectError> {
+    ) -> Result<CompleteLoadedAsset<A>, LoadDirectError> {
         let path = path.into().into_owned();
         self.load_internal(&path, Some(TypeId::of::<A>()))
             .await
@@ -476,7 +476,7 @@ impl NestedLoader<'_, '_, DynamicTyped, Immediate<'_, '_>> {
     pub async fn load<'p>(
         self,
         path: impl Into<AssetPath<'p>>,
-    ) -> Result<ErasedLoadedAsset, LoadDirectError> {
+    ) -> Result<CompleteErasedLoadedAsset, LoadDirectError> {
         let path = path.into().into_owned();
         let asset_type_id = Some(self.typing.asset_type_id);
         self.load_internal(&path, asset_type_id)
@@ -492,7 +492,7 @@ impl NestedLoader<'_, '_, UnknownTyped, Immediate<'_, '_>> {
     pub async fn load<'p>(
         self,
         path: impl Into<AssetPath<'p>>,
-    ) -> Result<ErasedLoadedAsset, LoadDirectError> {
+    ) -> Result<CompleteErasedLoadedAsset, LoadDirectError> {
         let path = path.into().into_owned();
         self.load_internal(&path, None)
             .await