-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Subassets of subassets do not work. #15417
Copy link
Copy link
Closed
Labels
A-AssetsLoad files from disk to use for things like images, models, and soundsLoad files from disk to use for things like images, models, and soundsC-UsabilityA targeted quality-of-life change that makes Bevy easier to useA targeted quality-of-life change that makes Bevy easier to useD-ModestA "normal" level of difficulty; suitable for simple features or challenging fixesA "normal" level of difficulty; suitable for simple features or challenging fixesM-Migration-GuideA breaking change to Bevy's public API that needs to be noted in a migration guideA breaking change to Bevy's public API that needs to be noted in a migration guideS-Needs-DesignThis issue requires design work to think about how it would best be accomplishedThis issue requires design work to think about how it would best be accomplishedX-Needs-SMEThis type of work requires an SME to approve it.This type of work requires an SME to approve it.
Metadata
Metadata
Assignees
Labels
A-AssetsLoad files from disk to use for things like images, models, and soundsLoad files from disk to use for things like images, models, and soundsC-UsabilityA targeted quality-of-life change that makes Bevy easier to useA targeted quality-of-life change that makes Bevy easier to useD-ModestA "normal" level of difficulty; suitable for simple features or challenging fixesA "normal" level of difficulty; suitable for simple features or challenging fixesM-Migration-GuideA breaking change to Bevy's public API that needs to be noted in a migration guideA breaking change to Bevy's public API that needs to be noted in a migration guideS-Needs-DesignThis issue requires design work to think about how it would best be accomplishedThis issue requires design work to think about how it would best be accomplishedX-Needs-SMEThis type of work requires an SME to approve it.This type of work requires an SME to approve it.
Problem
Today,
ErasedLoadedAssetholds the asset asBox<dyn AssetContainer>, and holds subassets asHashMap<String, ErasedLoadedAsset>(not really, but for simplicity, this is true). This means that subassets can themselves hold subassets, and so on, and so on. This makes it very confusing when you find out that these "nested subassets" get "flattened" when they get inserted by theAssetServer. For example, consider the following loader:The
ErasedLoadedAssetfor this looks like:{ value: SomeAsset(4), labeled_assets: { "A": ErasedLoadedAsset { value: SomeAsset(1), }, "B": ErasedLoadedAsset { value: SomeAsset(3), labeled_assets: { "A": ErasedLoadedAsset { value: SomeAsset(2) } } } } }The resulting Assets would be:
{ "my_asset.blah": SomeAsset(4), "my_asset.blah#A": SomeAsset(2), "my_asset.blah#B": SomeAsset(3), }What solution would you like?
Remove
labeled_assetsfromErasedLoadedAsset/LoadedAssetand propagate it separately. The hashmap of labeled assets should continue to just useErasedLoadedAsset.Make the
LoadContext::labeled_asset_scope(and related) merge the current and the new set of subassets.Make
AssetServer::send_loaded_assetno longer recursive.What alternative(s) have you considered?
Do nothing. Keep the confusing behavior. This is only an issue if, in an asset loader, in a
LoadContext::labeled_asset_scope(or equivalent), a subasset shares a name with a previously added subasset.Another alternative: Make them actually work! We could create some notation to support nested subassets in
AssetPath(for example, "my_asset.blah#A#B#C" or maybe "my_asset.blah#A/B/C"). It's not clear that this is something anyone wants or needs, so the complexity seems unwarranted.