Klooni1010
Klooni1010 copied to clipboard
LonamiWebs
assets MUST NOT be loaded in static code
There are issues when loading asses in static code as that is exposed to a race condition. Loading assets with SkinLoader must not happen before class Klooni is initiated. In other cases Gdx.graphics will be null. It happens that this condition is accidently met for the current state of the code.
The order classes are loaded and initialized is subject to the class loader involved and that may change in the future, especially as this is an Android app and Google is experimenting much with it's runtime.
There are more reason to not load assets in static code, ie. error handling and the like.
Example of issues (initiate effects in static code from class Klooni):
SkinLoader static
Exception in thread "main" java.lang.ExceptionInInitializerError
at io.github.lonamiwebs.klooni.effects.WaterdropEffectFactory.
Is this still an issue after #43 got merged? (I see you moved the Waterdrop static initialization into the constructor which is nice).
Also regarding the FIXME you introduced:
https://github.com/LonamiWebs/Klooni1010/blob/ef2fb8fa84bc1fa2f6c5cea777a91ae64a8fff99/core/src/io/github/lonamiwebs/klooni/Klooni.java#L46-L47
Sure, it's static, but it's not loading anything. It defaults to null, and is loaded later on create:
https://github.com/LonamiWebs/Klooni1010/blob/ef2fb8fa84bc1fa2f6c5cea777a91ae64a8fff99/core/src/io/github/lonamiwebs/klooni/Klooni.java#L82-L95
So, is this still wrong?
The text is according to your TODO that have been there before. I did'n have any look into it beside the code of SkinLoader.
As to to the comment in Line 89 says it is a singleton, you should not allow public write access to it and use a public getter ... and have a look if that's really a performance hit or not ;)
As it isn't loaded that early there's no need for static at all. If other classes need access, they should simply know Klooni. This would lead to more refactoring, ie. making the cellTexture a parameter of Cell.draw or supplying it at cell creation and changing the parameter of set to the already resolved texture to having Cells and maybe other to not needing to know Klooni.
Alternatively you could have a private static variable to store the Klooni instance and supply it with a public static getter.
... much changes and I don't see a practical benefit now. As a big difference to effects you only have one Theme class and there's no need to change as long as that one size fits all.