Fix untyped labeled asset loading #10514
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cart
added
the
A-Assets
|
@Selene-Amanita, can you test and review this please? |
cart
commented
Nov 11, 2023
|
Looks good to me |
NiklasEi
approved these changes
Nov 13, 2023
alice-i-cecile
approved these changes
Nov 14, 2023
alice-i-cecile
added
C-Bug
cart
added a commit
that referenced
this pull request
Nov 30, 2023
# Objective Fixes #10436 Alternative to #10465 ## Solution `load_untyped_async` / `load_internal` currently has a bug. In `load_untyped_async`, we pass None into `load_internal` for the `UntypedHandle` of the labeled asset path. This results in a call to `get_or_create_path_handle_untyped` with `loader.asset_type_id()` This is a new code path that wasn't hit prior to the newly added `load_untyped` because `load_untyped_async` was a private method only used in the context of the `load_folder` impl (which doesn't have labels) The fix required some refactoring to catch that case and defer handle retrieval. I have also made `load_untyped_async` public as it is now "ready for public use" and unlocks new scenarios.
rdrpenguin04
pushed a commit
to rdrpenguin04/bevy
that referenced
this pull request
Jan 9, 2024
# Objective Fixes bevyengine#10436 Alternative to bevyengine#10465 ## Solution `load_untyped_async` / `load_internal` currently has a bug. In `load_untyped_async`, we pass None into `load_internal` for the `UntypedHandle` of the labeled asset path. This results in a call to `get_or_create_path_handle_untyped` with `loader.asset_type_id()` This is a new code path that wasn't hit prior to the newly added `load_untyped` because `load_untyped_async` was a private method only used in the context of the `load_folder` impl (which doesn't have labels) The fix required some refactoring to catch that case and defer handle retrieval. I have also made `load_untyped_async` public as it is now "ready for public use" and unlocks new scenarios.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
Fixes #10436
Alternative to #10465
Solution
load_untyped_async/load_internalcurrently has a bug. Inload_untyped_async, we pass None intoload_internalfor theUntypedHandleof the labeled asset path. This results in a call toget_or_create_path_handle_untypedwithloader.asset_type_id()This is a new code path that wasn't hit prior to the newly added
load_untypedbecauseload_untyped_asyncwas a private method only used in the context of theload_folderimpl (which doesn't have labels)The fix required some refactoring to catch that case and defer handle retrieval. I have also made
load_untyped_asyncpublic as it is now "ready for public use" and unlocks new scenarios.