[Krom/HL] Implement asset loading failed callbacks for most cases #1441
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.
This PR implements the
failed
callback on Krom and HL targets in most situations. This means that the "Could not read hxBytes of undefined" errors are finally gone in most cases – they didn't tell anyone what asset caused the problem and they were difficult to debug from remote if some Armory users for example simply shared just that error message. More importantly, it can no longer happen on Krom or HL thatImage
objects are returned to the user that neither contain a texture or a render target, thus crashing the application at a later point in time.A few remarks:
I didn't implement the
failed
callback in HL for loading videos because I wasn't sure how to handle things inkinc_video_init()
(e.g. video.cpp.h for Windows) which doesn't have a return value at the moment and on Windows simply ignores the HRESULT values returned from the functions it calls. I don't know if you want to slightly change Kinc's API, but I think this is required in order to get error information out of this method. Preferably there would be some more stuff in place that in case of an error would print the actual reason to the console (e.g. theHRESULT
value if!= S_OK
). I think this would have a very minor impact on performance but would be an tremendously helpful change.The same holds true for loading sounds:
kinc_affirm()
and not passed to Kha. Instead the user either sees "Unknown Error" or the application just crashes.new kha.korehl.Sound()
in a try/catch, because otherwise I don't think there is a nice way of calling a failed callback in the current API.As for loading images in HL, I changed
Image.fromFile()
so that it returns null in the error case. If you don't want to do this, I can instead move that code into the LoaderImpl, but that would be a rather hacky workaround and I think the current way is better since users will notice any issues before the application fails in mysterious ways at a later point in time. It probably makes sense to change the return type ofImage.fromFile()
toNull<Image>
to make the new behaviour clear and to satisfy Haxe's null safety checks, but it would introduce more inconsistency for the Kha API since it's not really used anywhere else in Kha, so I'm not sure whether you'd want that.Loading non-existing sounds still crashes on Kode/Krom. Fixing this is pretty simple (see armory3d/armorcore#56), but I don't want to open PRs for untested code. Please see below on why I can't test for Kode/Krom.
The current state of things when attempting to load files that don't exist (✔ meaning the
failed
callback is called):Please note that I could test my Krom-related changes only with Armorcore and not with Kode/Krom, since the latter fails to build with a linker error due to
kickstart()
being disabled by an#if 0
block: