Skip to content
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

Add LoadContext::load_direct_with_settings or similar #12963

Closed
Zoomulator opened this issue Apr 14, 2024 · 0 comments · Fixed by #13415
Closed

Add LoadContext::load_direct_with_settings or similar #12963

Zoomulator opened this issue Apr 14, 2024 · 0 comments · Fixed by #13415
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Enhancement A new feature

Comments

@Zoomulator
Copy link
Contributor

What problem does this solve or what need does it fill?

There's currently no way to pass loader settings for assets you wish to fetch via LoadContext::load_direct. I wanted to load a Gltf dependency with GltfLoaderSettings::include_source and traverse it myself in an AssetLoader.

What solution would you like?

I'd be fine with a simple addition of load_direct_with_settings, though perhaps it gets a bit much with all the permutations of arguments if load_direct_with_settings_from_reader were to be added as well.

I would therefore suggest that there are only two load functions returning handles (LoadContext::load, LoadContext::load_from_reader) with an Option argument for settings in both, and then have one async LoadContext::get_direct(Handle) -> Result<ErasedLoadedAsset, E>, with the added benefit of being able to await any asset Handle.

What alternative(s) have you considered?

Afaikt, the alternative is to wait for the dependency to load by listening to Events in a system. It's doable, but quite clunky compared to the async provided by the AssetLoader interface.

@Zoomulator Zoomulator added C-Enhancement A new feature S-Needs-Triage This issue needs to be labelled labels Apr 14, 2024
@ItsDoot ItsDoot added A-Assets Load files from disk to use for things like images, models, and sounds and removed S-Needs-Triage This issue needs to be labelled labels Apr 18, 2024
github-merge-queue bot pushed a commit that referenced this issue May 21, 2024
# Objective
- Introduce variants of `LoadContext::load_direct` which allow picking
asset type & configuring settings.
- Fixes #12963.

## Solution
- Implements `ErasedLoadedAsset::downcast` and adds some accessors to
`LoadedAsset<A>`.
- Changes `load_direct`/`load_direct_with_reader` to be typed, and
introduces `load_direct_untyped`/`load_direct_untyped_with_reader`.
- Introduces `load_direct_with_settings` and
`load_direct_with_reader_and_settings`.

## Testing
- I've run cargo test and played with the examples which use
`load_direct`.
- I also extended the `asset_processing` example to use the new typed
version of `load_direct` and use `load_direct_with_settings`.

---

## Changelog
- Introduced new `load_direct` methods in `LoadContext` to allow
specifying type & settings

## Migration Guide
- `LoadContext::load_direct` has been renamed to
`LoadContext::load_direct_untyped`. You may find the new `load_direct`
is more appropriate for your use case (and the migration may only be
moving one type parameter).
- `LoadContext::load_direct_with_reader` has been renamed to
`LoadContext::load_direct_untyped_with_reader`.

---

This might not be an obvious win as a solution because it introduces
quite a few new `load_direct` alternatives - but it does follow the
existing pattern pretty well. I'm very open to alternatives.
:sweat_smile:
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-Enhancement A new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants