There are issues when loading asses in static code as that is exposed to a race condit

Is this still an issue after <a class="issue-link js-issue-link" data-error-text="Fail

assets MUST NOT be loaded in static code about klooni1010 HOT 2 OPEN

lonamiwebs commented on May 23, 2024

assets MUST NOT be loaded in static code

from klooni1010.

Comments (2)

Lonami commented on May 23, 2024

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:

Klooni1010/core/src/io/github/lonamiwebs/klooni/Klooni.java

Lines 46 to 47 in ef2fb8f

    
           // FIXME theme should NOT be static as it might load textures which will expose it to the race condition iff GDX got initialized before or not 
        
           public static Theme theme;

Sure, it's static, but it's not loading anything. It defaults to null, and is loaded later on create:

Klooni1010/core/src/io/github/lonamiwebs/klooni/Klooni.java

Lines 82 to 95 in ef2fb8f

    
           public void create() { 
        
               onDesktop = Gdx.app.getType().equals(Application.ApplicationType.Desktop); 
        
               prefs = Gdx.app.getPreferences("io.github.lonamiwebs.klooni.game"); 
        
               // Load the best match for the skin (depending on the device screen dimensions) 
        
               skin = SkinLoader.loadSkin(); 
        
               // Use only one instance for the theme, so anyone using it uses the most up-to-date 
        
               Theme.skin = skin; // Not the best idea 
        
               final String themeName = prefs.getString("themeName", "default"); 
        
               if (Theme.exists(themeName)) 
        
                   theme = Theme.getTheme(themeName); 
        
               else 
        
                   theme = Theme.getTheme("default");

So, is this still wrong?

from klooni1010.

andidevi commented on May 23, 2024

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.

from klooni1010.

assets MUST NOT be loaded in static code about klooni1010 HOT 2 OPEN

Comments (2)

Related Issues (20)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	// FIXME theme should NOT be static as it might load textures which will expose it to the race condition iff GDX got initialized before or not
	public static Theme theme;

	public void create() {
	onDesktop = Gdx.app.getType().equals(Application.ApplicationType.Desktop);
	prefs = Gdx.app.getPreferences("io.github.lonamiwebs.klooni.game");

	// Load the best match for the skin (depending on the device screen dimensions)
	skin = SkinLoader.loadSkin();

	// Use only one instance for the theme, so anyone using it uses the most up-to-date
	Theme.skin = skin; // Not the best idea
	final String themeName = prefs.getString("themeName", "default");
	if (Theme.exists(themeName))
	theme = Theme.getTheme(themeName);
	else
	theme = Theme.getTheme("default");