Skip to content

Comments

Fix failed sub-asset loads getting stuck in LoadState::Loading#22628

Merged
alice-i-cecile merged 9 commits intobevyengine:mainfrom
greeble-dev:test-asset-load-fail
Feb 2, 2026
Merged

Fix failed sub-asset loads getting stuck in LoadState::Loading#22628
alice-i-cecile merged 9 commits intobevyengine:mainfrom
greeble-dev:test-asset-load-fail

Conversation

@greeble-dev
Copy link
Contributor

@greeble-dev greeble-dev commented Jan 21, 2026

Objective

Fix #22607. Also add a test that reproduces the issues.

If it makes things clearer, I can split this into an "add a test" PR and a separate "fix the bug" PR.

Solution

There are three additions to AssetServer::load_internal that each fix a separate case. In source code order:

Case 1

// Sub-asset exists, but is not of type `WrongAssetType`.
asset_server.load::<WrongAssetType>("asset#subasset");

Previously this would silently fail - the asset would successfully load, but the handle can't actually resolve to that asset so it's stuck in limbo.

Fixed by adding a type check, then sending a AssetLoadError::RequestedHandleTypeMismatch event and returning it as an error.

Note that this doesn't prevent someone loading the asset with the correct type - the error is only associated with the <WrongAssetType> handle.

Case 2

// Sub-asset doesn't exist.
asset_server.load::<AssetType>("asset#non_existent_subasset");

Previously this was detected and would return a AssetLoadError::MissingLabel error from load_internal, but the load status for the handle wasn't set.

Fixed by sending an InternalAssetEvent::Failed.

Case 3

// Asset loader returns an error.
asset_server.load::<AssetType>("malformed_asset#subasset");

Previously this was detected, but the event was sent with the root asset's id (base_asset_id) so it wasn't associated with the sub-asset's handle.

Fixed by using the sub-asset's id.

Notes

Some of the events are only sent if asset_id is set. This might seem odd, but I think it's correct - asset_id can only be unset when using load_untyped with a sub-asset path, in which case there's no handle to associate with the error, and so there's no handle to pass to get_load_status.

Testing

cargo test -p bevy_asset
cargo test -p bevy_asset --features "multi_threaded"

@greeble-dev greeble-dev added C-Bug An unexpected or incorrect behavior A-Assets Load files from disk to use for things like images, models, and sounds labels Jan 21, 2026
@greeble-dev greeble-dev added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Jan 21, 2026
Comment on lines +3000 to +3006
for _ in 0..LARGE_ITERATION_COUNT {
app.update();
load_state = asset_server.get_load_state(&handle).unwrap().into();
if load_state == expected_load_state {
break;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just use run_app_until here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this initially, but found that it made debugging test failures harder - run_app_until will panic without any information about what failed. Doing it manually lets me print a clear "did X, expected Y, got Z".

I do think there's room for improvement. Maybe adding a run_app_until variant with more options, or even better a way to say "run until all loads have resolved". But don't think that would fit in this PR.

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Feb 2, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Feb 2, 2026
github-merge-queue bot pushed a commit that referenced this pull request Feb 2, 2026
)

## Objective

Fix #22607. Also add a test that reproduces the issues.

If it makes things clearer, I can split this into an "add a test" PR and
a separate "fix the bug" PR.

## Solution

There are three additions to `AssetServer::load_internal` that each fix
a separate case. In source code order:

### Case 1

```rust
// Sub-asset exists, but is not of type `WrongAssetType`.
asset_server.load::<WrongAssetType>("asset#subasset");
```

Previously this would silently fail - the asset would successfully load,
but the handle can't actually resolve to that asset so it's stuck in
limbo.

Fixed by adding a type check, then sending a
`AssetLoadError::RequestedHandleTypeMismatch` event and returning it as
an error.

Note that this doesn't prevent someone loading the asset with the
correct type - the error is only associated with the `<WrongAssetType>`
handle.

### Case 2

```rust
// Sub-asset doesn't exist.
asset_server.load::<AssetType>("asset#non_existent_subasset");
```

Previously this was detected and would return a
`AssetLoadError::MissingLabel` error from `load_internal`, but the load
status for the handle wasn't set.

Fixed by sending an `InternalAssetEvent::Failed`.

### Case 3
```rust
// Asset loader returns an error.
asset_server.load::<AssetType>("malformed_asset#subasset");
```

Previously this was detected, but the event was sent with the root
asset's id (`base_asset_id`) so it wasn't associated with the
sub-asset's handle.

Fixed by using the sub-asset's id.

### Notes

Some of the events are only sent if `asset_id` is set. This might seem
odd, but I think it's correct - `asset_id` can only be unset when using
`load_untyped` with a sub-asset path, in which case there's no handle to
associate with the error, and so there's no handle to pass to
`get_load_status`.

## Testing

```sh
cargo test -p bevy_asset
cargo test -p bevy_asset --features "multi_threaded"
```
Merged via the queue into bevyengine:main with commit f929d06 Feb 2, 2026
47 checks passed
viridia pushed a commit to viridia/bevy that referenced this pull request Feb 3, 2026
…yengine#22628)

## Objective

Fix bevyengine#22607. Also add a test that reproduces the issues.

If it makes things clearer, I can split this into an "add a test" PR and
a separate "fix the bug" PR.

## Solution

There are three additions to `AssetServer::load_internal` that each fix
a separate case. In source code order:

### Case 1

```rust
// Sub-asset exists, but is not of type `WrongAssetType`.
asset_server.load::<WrongAssetType>("asset#subasset");
```

Previously this would silently fail - the asset would successfully load,
but the handle can't actually resolve to that asset so it's stuck in
limbo.

Fixed by adding a type check, then sending a
`AssetLoadError::RequestedHandleTypeMismatch` event and returning it as
an error.

Note that this doesn't prevent someone loading the asset with the
correct type - the error is only associated with the `<WrongAssetType>`
handle.

### Case 2

```rust
// Sub-asset doesn't exist.
asset_server.load::<AssetType>("asset#non_existent_subasset");
```

Previously this was detected and would return a
`AssetLoadError::MissingLabel` error from `load_internal`, but the load
status for the handle wasn't set.

Fixed by sending an `InternalAssetEvent::Failed`.

### Case 3
```rust
// Asset loader returns an error.
asset_server.load::<AssetType>("malformed_asset#subasset");
```

Previously this was detected, but the event was sent with the root
asset's id (`base_asset_id`) so it wasn't associated with the
sub-asset's handle.

Fixed by using the sub-asset's id.

### Notes

Some of the events are only sent if `asset_id` is set. This might seem
odd, but I think it's correct - `asset_id` can only be unset when using
`load_untyped` with a sub-asset path, in which case there's no handle to
associate with the error, and so there's no handle to pass to
`get_load_status`.

## Testing

```sh
cargo test -p bevy_asset
cargo test -p bevy_asset --features "multi_threaded"
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failed sub-asset loads can get stuck in LoadState::Loading

3 participants