-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
LoadTransformAndSave Drops Asset Dependencies #11606
Labels
Milestone
Comments
RyanSpaker
added
C-Feature
A new feature, making something new possible
S-Needs-Triage
This issue needs to be labelled
labels
Jan 29, 2024
alice-i-cecile
added
C-Bug
An unexpected or incorrect behavior
A-Assets
Load files from disk to use for things like images, models, and sounds
and removed
C-Feature
A new feature, making something new possible
S-Needs-Triage
This issue needs to be labelled
labels
Jan 29, 2024
github-merge-queue bot
pushed a commit
that referenced
this issue
Feb 2, 2024
# Objective - `AssetTransformer` provides an input asset, and output an asset, but provides no access to the `LabeledAsset`'s created by the `AssetLoader`. Labeled sub assets are an extremely important piece of many assets, Gltf in particular, and without them the amount of transformation on an asset is limited. In order for `AssetTransformer`'s to be useful, they need to have access to these sub assets. - LabeledAsset's loaded by `AssetLoader`s are provided to `AssetSaver`s in the `LoadAndSave` process, but the `LoadTransformAndSave` process drops these values in the transform stage, and so `AssetSaver` is given none. - Fixes #11606 Ideally the AssetTransformer should not ignore labeled sub assets, and they should be kept at least for the AssetSaver ## Solution - I created a new struct similar to `SavedAsset` named `TransformedAsset` which holds the input asset, and the HashMap of `LabeledAsset`s. The transform function now takes as input a `TransformedAsset`, and returns a `TransformedAsset::<AssetOutput>`. This gives the transform function access to the labeled sub assets created by the `AssetLoader`. - I also created `TransformedSubAsset` which holds mutable references to a sub asset and that sub assets HashMap of `LabeledAsset`s. This allows you to travers the Tree of `LabeledAsset`s by reference relatively easily. - The `LoadTransformAndSave` processor was then reworked to use the new structs, stopping the `LabeledAsset`s from being dropped. --- ## Changelog - Created TransformedAsset struct and TransformedSubAsset struct. - Changed `get_untyped_handle` to return a `UntypedHandle` directly rather than a reference and added `get_handle` as a typed variant in SavedAsset and TransformedAsset - Added `SavedAsset::from_transformed` as a constructor from a `TransformedAsset` - Switched LoadTransformAndSave process code to work with new `TransformedAsset` type - Added a `ProcessError` for `AssetTransformer` in process.rs - Switched `AssetTransformer::transform` to use `TransformedAsset` as input and output. - Switched `AssetTransformer` to use a `BoxedFuture` like `AssetLoader` and `AssetSaver` to allow for async transformation code. - Updated AssetTransformer example to use new structure.
tjamaan
pushed a commit
to tjamaan/bevy
that referenced
this issue
Feb 6, 2024
# Objective - `AssetTransformer` provides an input asset, and output an asset, but provides no access to the `LabeledAsset`'s created by the `AssetLoader`. Labeled sub assets are an extremely important piece of many assets, Gltf in particular, and without them the amount of transformation on an asset is limited. In order for `AssetTransformer`'s to be useful, they need to have access to these sub assets. - LabeledAsset's loaded by `AssetLoader`s are provided to `AssetSaver`s in the `LoadAndSave` process, but the `LoadTransformAndSave` process drops these values in the transform stage, and so `AssetSaver` is given none. - Fixes bevyengine#11606 Ideally the AssetTransformer should not ignore labeled sub assets, and they should be kept at least for the AssetSaver ## Solution - I created a new struct similar to `SavedAsset` named `TransformedAsset` which holds the input asset, and the HashMap of `LabeledAsset`s. The transform function now takes as input a `TransformedAsset`, and returns a `TransformedAsset::<AssetOutput>`. This gives the transform function access to the labeled sub assets created by the `AssetLoader`. - I also created `TransformedSubAsset` which holds mutable references to a sub asset and that sub assets HashMap of `LabeledAsset`s. This allows you to travers the Tree of `LabeledAsset`s by reference relatively easily. - The `LoadTransformAndSave` processor was then reworked to use the new structs, stopping the `LabeledAsset`s from being dropped. --- ## Changelog - Created TransformedAsset struct and TransformedSubAsset struct. - Changed `get_untyped_handle` to return a `UntypedHandle` directly rather than a reference and added `get_handle` as a typed variant in SavedAsset and TransformedAsset - Added `SavedAsset::from_transformed` as a constructor from a `TransformedAsset` - Switched LoadTransformAndSave process code to work with new `TransformedAsset` type - Added a `ProcessError` for `AssetTransformer` in process.rs - Switched `AssetTransformer::transform` to use `TransformedAsset` as input and output. - Switched `AssetTransformer` to use a `BoxedFuture` like `AssetLoader` and `AssetSaver` to allow for async transformation code. - Updated AssetTransformer example to use new structure.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What problem does this solve or what need does it fill?
The new LoadTransformAndSave processor drops Sub-Assets loaded by the Loader, not giving them to either the Transformer or the Saver. LoadAndSave kept these dependencies, putting them in the SavedAsset struct and passing them into the Saver, but the new process drops these sub-assets when it takes the Loaded Asset out of the ErasedLoadedAsset struct.
What solution would you like?
LoadTransformAndSave should be altered so that the Asset dependecies are kept around to at least pass into the SavedAsset Struct for the Asset Saver. The Transformer trait should probably also be rewritten to take a struct containing both the Asset type and the Sub-Assets, similar to SavedAsset.
The text was updated successfully, but these errors were encountered: