bevyengine / bevyengine/bevy

Adding the same asset label multiple times to `LoadContext` does not return an error.

Open
#19,026 1 comment 0 reactions 0 assignees View on GitHub
A-Assets C-Bug S-Needs-Design
Dominant language
Rust
Stars
48.2k
Forks
4.8k
Avg merge
3d 22h
Merged PRs (30d)
161

Description

## The problem

Calling `LoadContext::add_labeled_asset` multiple times with the same label is not considered an error. This case is kinda trivial, but the more tricky case is when using `LoadContext::add_loaded_labeled_asset`. Calling this multiple times can have subassets of those subassets conflict.

## Prior attempts

In #15481, I flattened the subasset hierarchy, and in #18013, I took advantage of this to return an error on duplicate labels. However these were reverted in #18567 due to #18010. The TL;DR is that multiple nested asset loads with subassets can have overlapping subasset names. For example, if you have an asset loader that does two nested asset loads for GLTF files, both are likely to contain "Mesh0/Primitive0" subassets. Since nested asset loads currently are loaded as `LoadedAsset`, that means that their subassets were getting flattened, resulting in them being detected as duplicates.

## Basic Example

The following test fails since no error is returned by the loader. One of the subassets just silently gets dropped (which is bad since they contain different data).

```rust
#[test]
fn error_on_duplicate_subasset_names() {
let mut app = App::new();

let dir = Dir::default();
dir.insert_asset_text(Path::new("a.txt"), "");

app.register_asset_source(
AssetSourceId::Default,
AssetSource::build()
.with_reader(move || Box::new(MemoryAssetReader { root: dir.clone() })),
)
.add_plugins((TaskPoolPlugin::default(), AssetPlugin::default()));

struct DuplicateSubassetLoader;

impl AssetLoader for DuplicateSubassetLoader {
type Asset = TestAsset;
type Error = std::io::Error;
type Settings = ();

async fn load(
&self,
_: &mut dyn Reader,
_: &Self::Settings,
load_context: &mut LoadContext<'_>,
) -> Result {
load_context.add_labeled_asset("A".into(), SubText { text: "one".into() });
load_context.add_labeled_asset("A".into(), SubText { text: "two".into() });
Ok(TestAsset)
}
}

app.init_asset::()
.init_asset::()
.register_asset_loader(DuplicateSubassetLoader);

let asset_server = app.world().resource::().clone();
let handle = asset_server.load::("a.txt");

run_app_until(&mut app, |_world| match asset_server.load_state(&handle) {
LoadState::Loading => None,
LoadState::Failed(err) => {
// TODO: Check the error message matches here.
Some(())
}
state => panic!("Unexpected asset state: {state:?}"),
});
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.