Arc-ed assets
- Dominant language
- Rust
- Stars
- 48.2k
- Forks
- 4.8k
- Avg merge
- 3d 22h
- Merged PRs (30d)
- 161
Description
## What problem does this solve or what need does it fill?
Today, there is no way to access an asset from a thread/async block. The best you can do is clone the asset entirely, or make your asset a wrapper around an Arc with all the data. This is especially painful for https://github.com/bevyengine/bevy/issues/11216, which needs to be able to send assets (and their subassets) away to be serialized.
## What solution would you like?
Store assets as `Arc`! Now the asset can be passed to threads and read from there.
### Design proposal
1. In `assets::Entry`, instead of storing an `Option`, store an `Option>`.
2. Make `Assets::get` return an `Option>`
3. Remove `Assets::get_mut`.
4. Make `Assets::insert` and `Assets::add` take `impl Into>` instead of `impl Into`.
This is a loss in functionality (you can't pass in something that converts to an `A`), but I don't think it's a big loss.
5. Make `AssetEvent::Modified` occur whenever an asset is replaced.
### Optional additional steps
1. Make the render world take `Arc`s instead of cloning.
This improves the render asset situation *for assets that are both in `MAIN_WORLD` and `RENDER_WORLD`*. This does not matter for assets that must be only in the `RENDER_WORLD` since moving the assets has to happen anyway. There's a slight win in that moving the assets is now just a pointer copy, but there's also a slight loss in that creating `RenderAssets` may need to do an extra clone if they want to move parts of a vector or something to the render asset.
2. Implement `Assets::get_cloned_mut`.
Since assets are no longer mutable (due to the `Arc`), we can provide a utility function for assets that implement `Clone` that just calls `Arc::make_mut`. This allows users to seem like they are mutating an asset even though they're really cloning on write (when necessary; turns out `make_mut` will only clone if there are other `Arc`s, quite neat!).
3. Consider deleting `DenseAssetStorage`
Today, `DenseAssetStorage` is used because we store assets densely to improve cache hits and stuff. With `Arc`s this probably doesn't matter much (since accessing assets will require a pointer hop anyway). So we could replace `DenseAssetStorage` with a `HashMap`. In fact we could merge it with the Uuid `HashMap`, making `Assets` basically a wrapper around `HashMap>`. In this model, `AssetIndex` will probably just become a u64 that counts up.
## What alternative(s) have you considered?
Asset locking - #15346
### Advantages to `Arc`ing
- The "next steps" section shows many simplifications we can implement. Less code is preferable!
- Assets in an async context are the "front door" - just pass around the `Arc`!
### Disadvantages to `Arc`ing
- It becomes very easy for users to just clone the `Arc` and then never "poll" the `Assets` resource again. This makes it a hazard for hot-reloading or any case where you want mutable assets. Asset locking is a scary enough name that users are more likely to avoid it by default and only reach for it when necessary (and may also think about how to unlock the asset).
Contributor guide
Assessment
This issue has not been assessed yet.