You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently, getRootTileAvailableEvent never fires if the underlying loader is invalid. This happens in cases where invalid asset IDs are passed in, and the loader rightfully prevents itself from making a bogus request.
I ran into this in Unreal because of how statistics work. It requires loading the tileset's metadata schema after the root tile is available. However, if the promise never resolves, then we just keep waiting for something that will never happen. This resulted in the Editor never being able to close a level that was attempting to load inaccessible tileset metadata. Even if no root tile is actually loaded, the event should resolve when it's clear nothing can be loaded (similar to failed URL loads).
It doesn't feel to me like getRootTileAvailableEvent should resolve if there is no root tile, because that means that the root tile is not actually available. Would it be possible to reject the future instead?
@azrogers Totally agree with you, this just happens to be the pattern that we've established in other parts of the code. In a similar issue in #1299, @kring explained:
[The promise] only rejects in the catch handler, which is when something quite catastrophic goes wrong. A normal load failure (e.g., request for the tileset.json returns a 404, or JSON parsing of the tileset.json response fails) will not invoke that catch continuation.
So the promise not rejecting, even for typical load failures, is intentional at least for now. It might be nice to change that behavior in the future, or at least to rename getRootTileAvailableEvent for that possibility (resolveRootTileEvent?). I'm happy to open an issue for this so we can keep mental tabs on it and avoid confusion later.
We should probably have the getRootTileAvailableEvent return a Result with the pointer to the root tile or something to make explicit the implicit requirement being that the subscribers should check to see if the root tile is nullptr when the event fires. But that's a refactor for another time.
This file contains hidden or 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
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.
Currently,
getRootTileAvailableEventnever fires if the underlying loader is invalid. This happens in cases where invalid asset IDs are passed in, and the loader rightfully prevents itself from making a bogus request.I ran into this in Unreal because of how statistics work. It requires loading the tileset's metadata schema after the root tile is available. However, if the promise never resolves, then we just keep waiting for something that will never happen. This resulted in the Editor never being able to close a level that was attempting to load inaccessible tileset metadata. Even if no root tile is actually loaded, the event should resolve when it's clear nothing can be loaded (similar to failed URL loads).