Skip to content

Fix optional texture map lookup warnings #512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
belgianguy wants to merge 3 commits into
base: master
Choose a base branch
from
+17 −11
Open

Fix optional texture map lookup warnings #512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ead3d6c
Fix optional texture map lookup warnings
belgianguy Aug 1, 2026
a3fdcf3
Avoid duplicate load for optional texture maps
belgianguy Aug 2, 2026
8dff1b8
Suppress instead of try/catch
belgianguy Aug 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/main/java/toniarts/openkeeper/Main.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ public static void main(String[] args) throws InvocationTargetException, Interru

// set a better logging format
System.setProperty("java.util.logging.SimpleFormatter.format", "%1$tF %1$tT %4$s %2$s - %5$s%6$s%n");
suppressMissingAssetWarnings();

// Finally start it if everything went ok
if (checkSetup(app)) {
Expand All @@ -134,6 +135,13 @@ public static void main(String[] args) throws InvocationTargetException, Interru
}
}

private static void suppressMissingAssetWarnings() {
java.util.logging.Logger assetManagerLogger = java.util.logging.Logger.getLogger(AssetManager.class.getName());
java.util.logging.Filter existingFilter = assetManagerLogger.getFilter();
assetManagerLogger.setFilter(record -> !"Cannot locate resource: {0}".equals(record.getMessage())
&& (existingFilter == null || existingFilter.isLoggable(record)));
}

/**
* Parse application parameters
*
Expand Down
20 changes: 9 additions & 11 deletions src/main/java/toniarts/openkeeper/utils/AssetUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.function.Supplier;
import javax.imageio.ImageIO;
import toniarts.openkeeper.Main;
Expand All @@ -75,7 +75,7 @@ public final class AssetUtils {
private final static Object ASSET_LOCK = new Object();
private final static AssetCache ASSET_CACHE = new SimpleAssetCache();
private final static AssetCache WEAK_ASSET_CACHE = new WeakRefAssetCache();
private final static Map<String, Boolean> TEXTURE_MAP_CACHE = new HashMap<>();
private final static ConcurrentMap<String, Boolean> TEXTURE_MAP_CACHE = new ConcurrentHashMap<>();

// Custom model data keys
public final static String USER_DATA_KEY_REMOVABLE = "Removable";
Expand Down Expand Up @@ -234,21 +234,19 @@ public static void assignMapsToMaterial(AssetManager assetManager, Material mate

private static void assignMapToMaterial(AssetManager assetManager, Material material, String paramName, String textureName) {

// Try to locate the texture
Boolean found = TEXTURE_MAP_CACHE.get(textureName);
if (found == null) {
TextureKey textureKey = new TextureKey(textureName, false);

// See if it exists
AssetInfo assetInfo = assetManager.locateAsset(textureKey);
AssetInfo assetInfo = assetManager.locateAsset(new TextureKey(textureName, false));
found = (assetInfo != null);
TEXTURE_MAP_CACHE.put(textureName, found);
Boolean cached = TEXTURE_MAP_CACHE.putIfAbsent(textureName, found);
if (cached != null) {

@tonihele tonihele Aug 9, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic I also don't quite get? found = (assetInfo != null);, that is clear, but why

            Boolean cached = TEXTURE_MAP_CACHE.putIfAbsent(textureName, found);
            if (cached != null) {
                found = cached;
            }

Why is the added complexity there? I'm not saying it is wrong, I just don't get it. We already know whether the texture was found or not because we just checked it, then for some reason we see if it was already resolved earlier (concurrency) and use that value instead. Shouldn't the new value actually be the dominant one? Maybe somebody just added it, or removed it. Either way using simply PUT as opposed to putIfAbsent we kinda have the latest value? Of course this event is highly unlikely and if we worry about it too much, double checked locking is always a trusted friend and in many cases way better than the chaos brought by simply using synchronized collections.

Sometimes the algorithm is way heavier than the cost of just locking, so you end up loosing in the throughput in the end. But this here I am not proposing the locking, just rambling :D

So just a question why? For learning purposes

@Trass3r Trass3r Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also why not cache the Texture instead of a bool

@belgianguy belgianguy Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Trass3r ,

The AssetManager already caches loaded textures. This cache is mainly for remembering missing optional maps, which the asset cache cannot represent.

@belgianguy belgianguy Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tonihele: Regarding your question:

The putIfAbsent was intended to ensure that concurrent callers use one consistent cached result, but I agree that the implementation added unnecessary complexity. I’ve replaced the whole sequence with computeIfAbsent(), which expresses the intent more directly and performs the lookup once per texture name.
Assets aren’t expected to be added or removed at runtime. If that becomes supported, this cache would need explicit invalidation—using put() alone wouldn’t reliably represent the latest state during concurrent lookups either.

found = cached;
}
}

// Set it
if (found) {
TextureKey textureKey = new TextureKey(textureName, false);
material.setTexture(paramName, assetManager.loadTexture(textureKey));
material.setTexture(paramName, assetManager.loadTexture(new TextureKey(textureName, false)));
} else {
material.clearParam(paramName);
}
Expand Down
Loading