Skip to content

Arc-ed assets #15723

Open
Open
@andriyDev

Description

@andriyDev

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 #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<A>! Now the asset can be passed to threads and read from there.

Design proposal

  1. In assets::Entry<A>, instead of storing an Option<A>, store an Option<Arc<A>>.

  2. Make Assets<A>::get return an Option<Arc<A>>

  3. Remove Assets<A>::get_mut.

  4. Make Assets<A>::insert and Assets<A>::add take impl Into<Arc<A>> instead of impl Into<A>.

    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 Arcs 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<A>::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 Arcs, quite neat!).

  3. Consider deleting DenseAssetStorage

    Today, DenseAssetStorage is used because we store assets densely to improve cache hits and stuff. With Arcs 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<AssetId, Arc<A>>. In this model, AssetIndex will probably just become a u64 that counts up.

What alternative(s) have you considered?

Asset locking - #15346

Advantages to Arcing

  • 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 Arcing

  • 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).

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-AssetsLoad files from disk to use for things like images, models, and soundsC-FeatureA new feature, making something new possibleD-ModestA "normal" level of difficulty; suitable for simple features or challenging fixesS-Ready-For-ImplementationThis issue is ready for an implementation PR. Go for it!

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions