From 9e3bf14ffaead5e388993d9c925acf6a6a00a6e2 Mon Sep 17 00:00:00 2001 From: Klesti Selimaj Date: Fri, 25 Sep 2026 14:00:29 +0200 Subject: [PATCH] Stop per-frame player lookups and make shared state thread-safe SaturnPlayer stored non-Saturn players as null, which looked the same as "not fetched yet", so every render re-queued them and flooded the server with get_player requests. Remember negative lookups and re-check them after 60 s, retry failed lookups after 5 s, queue each player at most once, skip lookups while disconnected, and replace the hand-rolled worker thread with a single-thread executor. getPlayer now returns null only when the server says the player isn't on Saturn and throws on failure, so failures are retried instead of cached. Make state shared between network callbacks and the render thread safe: ConcurrentHashMap for players, CopyOnWriteArrayList for owned cosmetics, volatile session/uuid and cosmetic fields, and run emote notifications on the client thread. Refs saturnclientmc/saturnclient#2, saturnclientmc/saturnclient#6 Co-Authored-By: Claude Opus 5.5 --- .../saturnclient/client/ServiceClient.java | 43 +++---- .../client/player/SaturnPlayer.java | 111 ++++++++++-------- .../org/saturnclient/cosmetics/Cloaks.java | 4 +- .../java/org/saturnclient/cosmetics/Hats.java | 5 +- 4 files changed, 88 insertions(+), 75 deletions(-) diff --git a/src/main/java/org/saturnclient/client/ServiceClient.java b/src/main/java/org/saturnclient/client/ServiceClient.java index 8f4e4cb..885026c 100644 --- a/src/main/java/org/saturnclient/client/ServiceClient.java +++ b/src/main/java/org/saturnclient/client/ServiceClient.java @@ -13,8 +13,8 @@ import org.saturnclient.cosmetics.Hats; import dev.selimaj.session.Session; public class ServiceClient { - private static Session session; - public static UUID uuid; + private static volatile Session session; + public static volatile UUID uuid; public static void initialize() { new Thread(() -> { @@ -28,6 +28,10 @@ public class ServiceClient { }).start(); } + public static boolean isConnected() { + return session != null; + } + public static boolean connectTimeout() { try { session = Session.connect("wss://saturn-server.selimaj.dev", 2, TimeUnit.MINUTES); @@ -220,11 +224,10 @@ public class ServiceClient { return; } - if (data.emote() != null && !data.emote().isEmpty()) { - Providers.saturn.playEmote(from, data.emote()); - } else { - Providers.saturn.playEmote(from, null); - } + String emote = data.emote() != null && !data.emote().isEmpty() ? data.emote() : null; + + // Notifications arrive on a background thread. + Providers.saturn.getClient().executeOnThread(() -> Providers.saturn.playEmote(from, emote)); }); session.onNotification(ServiceMethods.Player, (player) -> { @@ -233,21 +236,21 @@ public class ServiceClient { }); } - public static SaturnPlayer getPlayer(UUID uuid, String name) { + /** + * Returns the player's Saturn data, or null if the server says they aren't + * using Saturn. Throws if the lookup itself fails, so callers can retry. + */ + public static SaturnPlayer getPlayer(UUID uuid, String name) throws Exception { + Session session = ServiceClient.session; + if (session == null) + throw new IllegalStateException("Not connected to the Saturn server"); + + ServiceMethods.Types.Player player = session.request(ServiceMethods.GetPlayer, uuid.toString()).join(); + + if (player == null) return null; - try { - ServiceMethods.Types.Player player = session.request(ServiceMethods.GetPlayer, uuid.toString()).join(); - - if (player == null) - return null; - - return player.toSaturnPlayer(uuid, name); - } catch (Exception e) { - Providers.saturn.logError("Failed to get player", e); - } - - return null; + return player.toSaturnPlayer(uuid, name); } } diff --git a/src/main/java/org/saturnclient/client/player/SaturnPlayer.java b/src/main/java/org/saturnclient/client/player/SaturnPlayer.java index eee47ce..23d4a2f 100644 --- a/src/main/java/org/saturnclient/client/player/SaturnPlayer.java +++ b/src/main/java/org/saturnclient/client/player/SaturnPlayer.java @@ -1,24 +1,39 @@ package org.saturnclient.client.player; -import java.util.HashMap; import java.util.Map; -import java.util.Queue; +import java.util.Set; import java.util.UUID; -import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import org.saturnclient.client.ServiceClient; import org.saturnclient.common.provider.Providers; public class SaturnPlayer { - private static final Map PLAYERS = new HashMap<>(); + /** Players known to be using Saturn. */ + private static final Map PLAYERS = new ConcurrentHashMap<>(); - private static final Queue QUEUE = new ConcurrentLinkedQueue<>(); + /** + * Earliest time (ms) a player without a Saturn entry may be looked up again, + * so non-Saturn players aren't re-requested every frame. + */ + private static final Map NEXT_LOOKUP = new ConcurrentHashMap<>(); - private static volatile boolean RUNNING = false; - private static Thread WORKER; + /** Players queued or being looked up. */ + private static final Set PENDING = ConcurrentHashMap.newKeySet(); - public String cloak = ""; - public String hat = ""; + private static final long NOT_SATURN_RECHECK_MS = 60_000; + private static final long FAILED_LOOKUP_RETRY_MS = 5_000; + + private static final ExecutorService LOOKUPS = Executors.newSingleThreadExecutor(runnable -> { + Thread thread = new Thread(runnable, "SaturnPlayer-Worker"); + thread.setDaemon(true); + return thread; + }); + + public volatile String cloak = ""; + public volatile String hat = ""; public final UUID uuid; public final String name; @@ -30,12 +45,18 @@ public class SaturnPlayer { } public static SaturnPlayer get() { - if (ServiceClient.uuid == null) { + UUID self = ServiceClient.uuid; + if (self == null) { return null; } - return PLAYERS.get(ServiceClient.uuid); + return PLAYERS.get(self); } + /** + * Returns the player if they are known to use Saturn, otherwise null. An + * unknown player is looked up in the background, at most once at a time and + * no more often than {@link #NEXT_LOOKUP} allows. Safe to call every frame. + */ public static SaturnPlayer get(String name, UUID uuid) { if (uuid == null) return null; @@ -43,9 +64,7 @@ public class SaturnPlayer { SaturnPlayer player = PLAYERS.get(uuid); if (player == null) { - PLAYERS.put(uuid, null); - QUEUE.add(uuid); - startPlayerThread(); + requestLookup(name, uuid); } return player; @@ -61,58 +80,46 @@ public class SaturnPlayer { public static void set(SaturnPlayer player) { PLAYERS.put(player.uuid, player); + NEXT_LOOKUP.remove(player.uuid); } public static String[] getExternalUUIDAsString() { - return PLAYERS.keySet().stream().filter(id -> ServiceClient.uuid == null || !id.equals(ServiceClient.uuid)) + UUID self = ServiceClient.uuid; + + return PLAYERS.keySet().stream().filter(id -> self == null || !id.equals(self)) .map(UUID::toString) .toArray(String[]::new); } - public static synchronized void startPlayerThread() { - if (RUNNING) + private static void requestLookup(String name, UUID uuid) { + if (!ServiceClient.isConnected()) { return; + } - RUNNING = true; + Long next = NEXT_LOOKUP.get(uuid); + if (next != null && System.currentTimeMillis() < next) { + return; + } - WORKER = new Thread(() -> { - long lastWorkTime = System.currentTimeMillis(); + if (!PENDING.add(uuid)) { + return; + } - while (true) { - UUID uuid = QUEUE.poll(); + LOOKUPS.execute(() -> { + try { + SaturnPlayer player = ServiceClient.getPlayer(uuid, name); - if (uuid != null) { - try { - String name = Providers.saturn.getClient().getPlayerListEntry(uuid); - - SaturnPlayer player = ServiceClient.getPlayer(uuid, name); - - // Apply on main thread - Providers.saturn.getClient().executeOnThread(() -> { - PLAYERS.put(uuid, player); - }); - - } catch (Exception e) { - Providers.saturn.logError("Failed to fetch player " + uuid, e); - } - - lastWorkTime = System.currentTimeMillis(); + if (player != null) { + set(player); } else { - if (System.currentTimeMillis() - lastWorkTime > 5000) { - break; - } - - try { - Thread.sleep(50); - } catch (InterruptedException ignored) { - } + NEXT_LOOKUP.put(uuid, System.currentTimeMillis() + NOT_SATURN_RECHECK_MS); } + } catch (Exception e) { + Providers.saturn.logError("Failed to fetch player " + uuid, e); + NEXT_LOOKUP.put(uuid, System.currentTimeMillis() + FAILED_LOOKUP_RETRY_MS); + } finally { + PENDING.remove(uuid); } - - RUNNING = false; - }, "SaturnPlayer-Worker"); - - WORKER.setDaemon(true); - WORKER.start(); + }); } } diff --git a/src/main/java/org/saturnclient/cosmetics/Cloaks.java b/src/main/java/org/saturnclient/cosmetics/Cloaks.java index 959f0dd..df17902 100644 --- a/src/main/java/org/saturnclient/cosmetics/Cloaks.java +++ b/src/main/java/org/saturnclient/cosmetics/Cloaks.java @@ -5,6 +5,7 @@ import org.saturnclient.client.player.SaturnPlayer; import org.saturnclient.common.ref.asset.IdentifierRef; import java.util.*; +import java.util.concurrent.CopyOnWriteArrayList; public class Cloaks { public static final String[] ALL_CLOAKS = { @@ -30,7 +31,8 @@ public class Cloaks { "black_hole_flame", "black_hole_white" }; - public static final List availableCloaks = new ArrayList<>(); + // Written by network callbacks, read by the UI thread. + public static final List availableCloaks = new CopyOnWriteArrayList<>(); public static void initialize() { availableCloaks.add(0, ""); diff --git a/src/main/java/org/saturnclient/cosmetics/Hats.java b/src/main/java/org/saturnclient/cosmetics/Hats.java index 9aff673..8941c0e 100644 --- a/src/main/java/org/saturnclient/cosmetics/Hats.java +++ b/src/main/java/org/saturnclient/cosmetics/Hats.java @@ -1,7 +1,7 @@ package org.saturnclient.cosmetics; -import java.util.ArrayList; import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; import org.saturnclient.client.ServiceClient; import org.saturnclient.client.player.SaturnPlayer; @@ -9,7 +9,8 @@ import org.saturnclient.client.player.SaturnPlayer; public class Hats { public static final String[] ALL_HATS = { "horns_black", "horns_white", "halo_white", "halo_black", "horns_end", "halo_end", "bucket_black", "bucket_end" }; - public static List availableHats = new ArrayList<>(); + // Written by network callbacks, read by the UI thread. + public static List availableHats = new CopyOnWriteArrayList<>(); public static void initialize() { availableHats.add(0, "");