Skip to content

Commit f176448

Browse files
authored
Remove labeled_assets from LoadedAsset and ErasedLoadedAsset (#15481)
# Objective Fixes #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.
1 parent 232824c commit f176448

File tree

10 files changed

+173
-199
lines changed

10 files changed

+173
-199
lines changed

crates/bevy_asset/Cargo.toml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ trace = []
2222
bevy_app = { path = "../bevy_app", version = "0.16.0-dev" }
2323
bevy_asset_macros = { path = "macros", version = "0.16.0-dev" }
2424
bevy_ecs = { path = "../bevy_ecs", version = "0.16.0-dev" }
25+
bevy_log = { path = "../bevy_log", version = "0.16.0-dev" }
2526
bevy_reflect = { path = "../bevy_reflect", version = "0.16.0-dev", features = [
2627
"uuid",
2728
] }
@@ -69,9 +70,6 @@ uuid = { version = "1.13.1", default-features = false, features = ["js"] }
6970
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
7071
notify-debouncer-full = { version = "0.5.0", optional = true }
7172

72-
[dev-dependencies]
73-
bevy_log = { path = "../bevy_log", version = "0.16.0-dev" }
74-
7573
[lints]
7674
workspace = true
7775

crates/bevy_asset/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -725,7 +725,7 @@ mod tests {
725725
.map_err(|_| Self::Error::CannotLoadDependency {
726726
dependency: dep.into(),
727727
})?;
728-
let cool = loaded.get();
728+
let cool = loaded.get_asset().get();
729729
embedded.push_str(&cool.text);
730730
}
731731
Ok(CoolText {

crates/bevy_asset/src/loader.rs

Lines changed: 120 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use alloc::{
1313
};
1414
use atomicow::CowArc;
1515
use bevy_ecs::world::World;
16+
use bevy_log::warn;
1617
use bevy_platform_support::collections::{HashMap, HashSet};
1718
use bevy_tasks::{BoxedFuture, ConditionalSendFuture};
1819
use core::any::{Any, TypeId};
@@ -60,7 +61,7 @@ pub trait ErasedAssetLoader: Send + Sync + 'static {
6061
load_context: LoadContext<'a>,
6162
) -> BoxedFuture<
6263
'a,
63-
Result<ErasedLoadedAsset, Box<dyn core::error::Error + Send + Sync + 'static>>,
64+
Result<CompleteErasedLoadedAsset, Box<dyn core::error::Error + Send + Sync + 'static>>,
6465
>;
6566

6667
/// Returns a list of extensions supported by this asset loader, without the preceding dot.
@@ -91,7 +92,7 @@ where
9192
mut load_context: LoadContext<'a>,
9293
) -> BoxedFuture<
9394
'a,
94-
Result<ErasedLoadedAsset, Box<dyn core::error::Error + Send + Sync + 'static>>,
95+
Result<CompleteErasedLoadedAsset, Box<dyn core::error::Error + Send + Sync + 'static>>,
9596
> {
9697
Box::pin(async move {
9798
let settings = meta
@@ -152,7 +153,6 @@ pub struct LoadedAsset<A: Asset> {
152153
pub(crate) value: A,
153154
pub(crate) dependencies: HashSet<UntypedAssetId>,
154155
pub(crate) loader_dependencies: HashMap<AssetPath<'static>, AssetHash>,
155-
pub(crate) labeled_assets: HashMap<CowArc<'static, str>, LabeledAsset>,
156156
}
157157

158158
impl<A: Asset> LoadedAsset<A> {
@@ -166,7 +166,6 @@ impl<A: Asset> LoadedAsset<A> {
166166
value,
167167
dependencies,
168168
loader_dependencies: HashMap::default(),
169-
labeled_assets: HashMap::default(),
170169
}
171170
}
172171

@@ -179,19 +178,6 @@ impl<A: Asset> LoadedAsset<A> {
179178
pub fn get(&self) -> &A {
180179
&self.value
181180
}
182-
183-
/// Returns the [`ErasedLoadedAsset`] for the given label, if it exists.
184-
pub fn get_labeled(
185-
&self,
186-
label: impl Into<CowArc<'static, str>>,
187-
) -> Option<&ErasedLoadedAsset> {
188-
self.labeled_assets.get(&label.into()).map(|a| &a.asset)
189-
}
190-
191-
/// Iterate over all labels for "labeled assets" in the loaded asset
192-
pub fn iter_labels(&self) -> impl Iterator<Item = &str> {
193-
self.labeled_assets.keys().map(|s| &**s)
194-
}
195181
}
196182

197183
impl<A: Asset> From<A> for LoadedAsset<A> {
@@ -205,7 +191,6 @@ pub struct ErasedLoadedAsset {
205191
pub(crate) value: Box<dyn AssetContainer>,
206192
pub(crate) dependencies: HashSet<UntypedAssetId>,
207193
pub(crate) loader_dependencies: HashMap<AssetPath<'static>, AssetHash>,
208-
pub(crate) labeled_assets: HashMap<CowArc<'static, str>, LabeledAsset>,
209194
}
210195

211196
impl<A: Asset> From<LoadedAsset<A>> for ErasedLoadedAsset {
@@ -214,7 +199,6 @@ impl<A: Asset> From<LoadedAsset<A>> for ErasedLoadedAsset {
214199
value: Box::new(asset.value),
215200
dependencies: asset.dependencies,
216201
loader_dependencies: asset.loader_dependencies,
217-
labeled_assets: asset.labeled_assets,
218202
}
219203
}
220204
}
@@ -241,19 +225,6 @@ impl ErasedLoadedAsset {
241225
self.value.asset_type_name()
242226
}
243227

244-
/// Returns the [`ErasedLoadedAsset`] for the given label, if it exists.
245-
pub fn get_labeled(
246-
&self,
247-
label: impl Into<CowArc<'static, str>>,
248-
) -> Option<&ErasedLoadedAsset> {
249-
self.labeled_assets.get(&label.into()).map(|a| &a.asset)
250-
}
251-
252-
/// Iterate over all labels for "labeled assets" in the loaded asset
253-
pub fn iter_labels(&self) -> impl Iterator<Item = &str> {
254-
self.labeled_assets.keys().map(|s| &**s)
255-
}
256-
257228
/// Cast this loaded asset as the given type. If the type does not match,
258229
/// the original type-erased asset is returned.
259230
pub fn downcast<A: Asset>(mut self) -> Result<LoadedAsset<A>, ErasedLoadedAsset> {
@@ -262,7 +233,6 @@ impl ErasedLoadedAsset {
262233
value: *value,
263234
dependencies: self.dependencies,
264235
loader_dependencies: self.loader_dependencies,
265-
labeled_assets: self.labeled_assets,
266236
}),
267237
Err(value) => {
268238
self.value = value;
@@ -290,6 +260,100 @@ impl<A: Asset> AssetContainer for A {
290260
}
291261
}
292262

263+
/// A loaded asset and all its loaded subassets.
264+
pub struct CompleteLoadedAsset<A: Asset> {
265+
/// The loaded asset.
266+
pub(crate) asset: LoadedAsset<A>,
267+
/// The subassets by their label.
268+
pub(crate) labeled_assets: HashMap<CowArc<'static, str>, LabeledAsset>,
269+
}
270+
271+
impl<A: Asset> CompleteLoadedAsset<A> {
272+
/// Take ownership of the stored [`Asset`] value.
273+
pub fn take(self) -> A {
274+
self.asset.value
275+
}
276+
277+
/// Returns the stored asset.
278+
pub fn get_asset(&self) -> &LoadedAsset<A> {
279+
&self.asset
280+
}
281+
282+
/// Returns the [`ErasedLoadedAsset`] for the given label, if it exists.
283+
pub fn get_labeled(
284+
&self,
285+
label: impl Into<CowArc<'static, str>>,
286+
) -> Option<&ErasedLoadedAsset> {
287+
self.labeled_assets.get(&label.into()).map(|a| &a.asset)
288+
}
289+
290+
/// Iterate over all labels for "labeled assets" in the loaded asset
291+
pub fn iter_labels(&self) -> impl Iterator<Item = &str> {
292+
self.labeled_assets.keys().map(|s| &**s)
293+
}
294+
}
295+
296+
/// A "type erased / boxed" counterpart to [`CompleteLoadedAsset`]. This is used in places where the
297+
/// loaded type is not statically known.
298+
pub struct CompleteErasedLoadedAsset {
299+
/// The loaded asset.
300+
pub(crate) asset: ErasedLoadedAsset,
301+
/// The subassets by their label.
302+
pub(crate) labeled_assets: HashMap<CowArc<'static, str>, LabeledAsset>,
303+
}
304+
305+
impl CompleteErasedLoadedAsset {
306+
/// Cast (and take ownership) of the [`Asset`] value of the given type. This will return
307+
/// [`Some`] if the stored type matches `A` and [`None`] if it does not.
308+
pub fn take<A: Asset>(self) -> Option<A> {
309+
self.asset.take()
310+
}
311+
312+
/// Returns the stored asset.
313+
pub fn get_asset(&self) -> &ErasedLoadedAsset {
314+
&self.asset
315+
}
316+
317+
/// Returns the [`ErasedLoadedAsset`] for the given label, if it exists.
318+
pub fn get_labeled(
319+
&self,
320+
label: impl Into<CowArc<'static, str>>,
321+
) -> Option<&ErasedLoadedAsset> {
322+
self.labeled_assets.get(&label.into()).map(|a| &a.asset)
323+
}
324+
325+
/// Iterate over all labels for "labeled assets" in the loaded asset
326+
pub fn iter_labels(&self) -> impl Iterator<Item = &str> {
327+
self.labeled_assets.keys().map(|s| &**s)
328+
}
329+
330+
/// Cast this loaded asset as the given type. If the type does not match,
331+
/// the original type-erased asset is returned.
332+
pub fn downcast<A: Asset>(
333+
mut self,
334+
) -> Result<CompleteLoadedAsset<A>, CompleteErasedLoadedAsset> {
335+
match self.asset.downcast::<A>() {
336+
Ok(asset) => Ok(CompleteLoadedAsset {
337+
asset,
338+
labeled_assets: self.labeled_assets,
339+
}),
340+
Err(asset) => {
341+
self.asset = asset;
342+
Err(self)
343+
}
344+
}
345+
}
346+
}
347+
348+
impl<A: Asset> From<CompleteLoadedAsset<A>> for CompleteErasedLoadedAsset {
349+
fn from(value: CompleteLoadedAsset<A>) -> Self {
350+
Self {
351+
asset: value.asset.into(),
352+
labeled_assets: value.labeled_assets,
353+
}
354+
}
355+
}
356+
293357
/// An error that occurs when attempting to call [`NestedLoader::load`] which
294358
/// is configured to work [immediately].
295359
///
@@ -397,8 +461,8 @@ impl<'a> LoadContext<'a> {
397461
) -> Handle<A> {
398462
let mut context = self.begin_labeled_asset();
399463
let asset = load(&mut context);
400-
let loaded_asset = context.finish(asset);
401-
self.add_loaded_labeled_asset(label, loaded_asset)
464+
let complete_asset = context.finish(asset);
465+
self.add_loaded_labeled_asset(label, complete_asset)
402466
}
403467

404468
/// This will add the given `asset` as a "labeled [`Asset`]" with the `label` label.
@@ -423,10 +487,14 @@ impl<'a> LoadContext<'a> {
423487
pub fn add_loaded_labeled_asset<A: Asset>(
424488
&mut self,
425489
label: impl Into<CowArc<'static, str>>,
426-
loaded_asset: LoadedAsset<A>,
490+
loaded_asset: CompleteLoadedAsset<A>,
427491
) -> Handle<A> {
428492
let label = label.into();
429-
let loaded_asset: ErasedLoadedAsset = loaded_asset.into();
493+
let CompleteLoadedAsset {
494+
asset,
495+
labeled_assets,
496+
} = loaded_asset;
497+
let loaded_asset: ErasedLoadedAsset = asset.into();
430498
let labeled_path = self.asset_path.clone().with_label(label.clone());
431499
let handle = self
432500
.asset_server
@@ -438,6 +506,11 @@ impl<'a> LoadContext<'a> {
438506
handle: handle.clone().untyped(),
439507
},
440508
);
509+
for (label, asset) in labeled_assets {
510+
if self.labeled_assets.insert(label.clone(), asset).is_some() {
511+
warn!("A labeled asset with the label \"{label}\" already exists. Replacing with the new asset.");
512+
}
513+
}
441514
handle
442515
}
443516

@@ -450,11 +523,13 @@ impl<'a> LoadContext<'a> {
450523
}
451524

452525
/// "Finishes" this context by populating the final [`Asset`] value.
453-
pub fn finish<A: Asset>(self, value: A) -> LoadedAsset<A> {
454-
LoadedAsset {
455-
value,
456-
dependencies: self.dependencies,
457-
loader_dependencies: self.loader_dependencies,
526+
pub fn finish<A: Asset>(self, value: A) -> CompleteLoadedAsset<A> {
527+
CompleteLoadedAsset {
528+
asset: LoadedAsset {
529+
value,
530+
dependencies: self.dependencies,
531+
loader_dependencies: self.loader_dependencies,
532+
},
458533
labeled_assets: self.labeled_assets,
459534
}
460535
}
@@ -525,8 +600,8 @@ impl<'a> LoadContext<'a> {
525600
meta: &dyn AssetMetaDyn,
526601
loader: &dyn ErasedAssetLoader,
527602
reader: &mut dyn Reader,
528-
) -> Result<ErasedLoadedAsset, LoadDirectError> {
529-
let loaded_asset = self
603+
) -> Result<CompleteErasedLoadedAsset, LoadDirectError> {
604+
let complete_asset = self
530605
.asset_server
531606
.load_with_meta_loader_and_reader(
532607
&path,
@@ -544,7 +619,7 @@ impl<'a> LoadContext<'a> {
544619
let info = meta.processed_info().as_ref();
545620
let hash = info.map(|i| i.full_hash).unwrap_or_default();
546621
self.loader_dependencies.insert(path, hash);
547-
Ok(loaded_asset)
622+
Ok(complete_asset)
548623
}
549624

550625
/// Create a builder for loading nested assets in this context.

crates/bevy_asset/src/loader_builders.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
use crate::{
55
io::Reader,
66
meta::{meta_transform_settings, AssetMetaDyn, MetaTransform, Settings},
7-
Asset, AssetLoadError, AssetPath, ErasedAssetLoader, ErasedLoadedAsset, Handle, LoadContext,
8-
LoadDirectError, LoadedAsset, LoadedUntypedAsset, UntypedHandle,
7+
Asset, AssetLoadError, AssetPath, CompleteErasedLoadedAsset, CompleteLoadedAsset,
8+
ErasedAssetLoader, Handle, LoadContext, LoadDirectError, LoadedUntypedAsset, UntypedHandle,
99
};
1010
use alloc::{borrow::ToOwned, boxed::Box, sync::Arc};
1111
use core::any::TypeId;
@@ -57,11 +57,11 @@ impl ReaderRef<'_> {
5757
/// If you know the type ID of the asset at runtime, but not at compile time,
5858
/// use [`with_dynamic_type`] followed by [`load`] to start loading an asset
5959
/// of that type. This lets you get an [`UntypedHandle`] (via [`Deferred`]),
60-
/// or a [`ErasedLoadedAsset`] (via [`Immediate`]).
60+
/// or a [`CompleteErasedLoadedAsset`] (via [`Immediate`]).
6161
///
6262
/// - in [`UnknownTyped`]: loading either a type-erased version of the asset
63-
/// ([`ErasedLoadedAsset`]), or a handle *to a handle* of the actual asset
64-
/// ([`LoadedUntypedAsset`]).
63+
/// ([`CompleteErasedLoadedAsset`]), or a handle *to a handle* of the actual
64+
/// asset ([`LoadedUntypedAsset`]).
6565
///
6666
/// If you have no idea what type of asset you will be loading (not even at
6767
/// runtime with a [`TypeId`]), use this.
@@ -386,7 +386,7 @@ impl<'builder, 'reader, T> NestedLoader<'_, '_, T, Immediate<'builder, 'reader>>
386386
self,
387387
path: &AssetPath<'static>,
388388
asset_type_id: Option<TypeId>,
389-
) -> Result<(Arc<dyn ErasedAssetLoader>, ErasedLoadedAsset), LoadDirectError> {
389+
) -> Result<(Arc<dyn ErasedAssetLoader>, CompleteErasedLoadedAsset), LoadDirectError> {
390390
let (mut meta, loader, mut reader) = if let Some(reader) = self.mode.reader {
391391
let loader = if let Some(asset_type_id) = asset_type_id {
392392
self.load_context
@@ -448,7 +448,7 @@ impl NestedLoader<'_, '_, StaticTyped, Immediate<'_, '_>> {
448448
pub async fn load<'p, A: Asset>(
449449
self,
450450
path: impl Into<AssetPath<'p>>,
451-
) -> Result<LoadedAsset<A>, LoadDirectError> {
451+
) -> Result<CompleteLoadedAsset<A>, LoadDirectError> {
452452
let path = path.into().into_owned();
453453
self.load_internal(&path, Some(TypeId::of::<A>()))
454454
.await
@@ -476,7 +476,7 @@ impl NestedLoader<'_, '_, DynamicTyped, Immediate<'_, '_>> {
476476
pub async fn load<'p>(
477477
self,
478478
path: impl Into<AssetPath<'p>>,
479-
) -> Result<ErasedLoadedAsset, LoadDirectError> {
479+
) -> Result<CompleteErasedLoadedAsset, LoadDirectError> {
480480
let path = path.into().into_owned();
481481
let asset_type_id = Some(self.typing.asset_type_id);
482482
self.load_internal(&path, asset_type_id)
@@ -492,7 +492,7 @@ impl NestedLoader<'_, '_, UnknownTyped, Immediate<'_, '_>> {
492492
pub async fn load<'p>(
493493
self,
494494
path: impl Into<AssetPath<'p>>,
495-
) -> Result<ErasedLoadedAsset, LoadDirectError> {
495+
) -> Result<CompleteErasedLoadedAsset, LoadDirectError> {
496496
let path = path.into().into_owned();
497497
self.load_internal(&path, None)
498498
.await

0 commit comments

Comments
 (0)