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 #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
-
In
assets::Entry<A>
, instead of storing anOption<A>
, store anOption<Arc<A>>
. -
Make
Assets<A>::get
return anOption<Arc<A>>
-
Remove
Assets<A>::get_mut
. -
Make
Assets<A>::insert
andAssets<A>::add
takeimpl Into<Arc<A>>
instead ofimpl 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. -
Make
AssetEvent::Modified
occur whenever an asset is replaced.
Optional additional steps
-
Make the render world take
Arc
s instead of cloning.This improves the render asset situation for assets that are both in
MAIN_WORLD
andRENDER_WORLD
. This does not matter for assets that must be only in theRENDER_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 creatingRenderAssets
may need to do an extra clone if they want to move parts of a vector or something to the render asset. -
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 implementClone
that just callsArc::make_mut
. This allows users to seem like they are mutating an asset even though they're really cloning on write (when necessary; turns outmake_mut
will only clone if there are otherArc
s, quite neat!). -
Consider deleting
DenseAssetStorage
Today,
DenseAssetStorage
is used because we store assets densely to improve cache hits and stuff. WithArc
s this probably doesn't matter much (since accessing assets will require a pointer hop anyway). So we could replaceDenseAssetStorage
with aHashMap
. In fact we could merge it with the UuidHashMap
, makingAssets
basically a wrapper aroundHashMap<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 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" theAssets
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).