-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Configurable Asset Storage #22015
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Configurable Asset Storage #22015
Conversation
|
|
||
| /// Implement the `Asset` trait. | ||
| #[proc_macro_derive(Asset, attributes(dependency))] | ||
| #[proc_macro_derive(Asset, attributes(dependency, asset_storage))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd personally prefer to hold off on asset storage changes until we've sorted out "assets as entities". I think we should try to reconcile that design with the "arc-ed assets" needs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this as a potential stopgap before Assets as Entities - which I’m excited for, but seems quite a ways out. The status quo of cloning heavy images and meshes into the render world is a bit rough.
The goal here is to provide a stepping stone that doesn't rock the boat much. At best, it could inform assets as entities, and at worst, it could be yeeted w/o complication when building Assets as Entities1.
Regarding the stepping stone comment, the storage abstractions here could potentially be used if assets end up as entities:
#[derive(Component)]
AssetComponent<A: Asset>(StoredAsset<A>) Footnotes
-
If it’d be helpful to delete HybridStorageStrategy that provides new async-specific APIs like
get_arcandget_arc_rwlocktoRes<Assets<A>>, I think that’d be fine. Assets could be arc-ed with ArcedStorageStrategy without the user knowing anything about the underlying implementation. ↩
|
It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note. Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes. |
This is an attempt at a minimally-invasive update to allow
Assettypes to define how they want to be stored withinRes<Assets<A>>. This is like Arc-ed assets (#15813), but more generic and less invasive.Showcase
Comparison
Arc-d assets #15813 - What’s different:
StackAssetStoragestrategy. No need to pay for atomics.AssetSnapshot<Self::SourceAsset>(a generic that derefences to&Self::SourceAsset) instead ofArc<Self::SourceAsset>. ForArcedAssetStorageandHybridAssetStorage, this isArc<Self::SourceAsset>. ForStackAssetStorage, this is effectivelyA.Asset locking #15359 - Similar concepts. Need to review this one in more detail to comment.
Work-in-Progress
This is more-or-less complete, though I want to clean up the documentation a little, review/cleanup the async approach in
HybridStorageStrategy, and add a migration guide. Open to feedback!