diff --git a/serverfeatures-api/src/main/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHook.java b/serverfeatures-api/src/main/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHook.java index b90206a4..a0a96119 100644 --- a/serverfeatures-api/src/main/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHook.java +++ b/serverfeatures-api/src/main/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHook.java @@ -2,18 +2,53 @@ import org.bukkit.Bukkit; import org.bukkit.entity.Player; +import org.bukkit.plugin.RegisteredServiceProvider; +import org.bukkit.plugin.java.JavaPlugin; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.TreeSet; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.BiConsumer; import java.util.function.BiFunction; +import java.util.function.BooleanSupplier; +import java.util.function.LongSupplier; import java.util.function.Predicate; +import java.util.logging.Level; +import java.util.regex.Matcher; +import java.util.regex.Pattern; -public class PlaceholderAPIHook { +/** Safe boundary around PlaceholderAPI and third-party expansions. */ +public final class PlaceholderAPIHook { + + private static final Pattern PLACEHOLDER_PATTERN = + Pattern.compile("%([A-Za-z0-9]+)(?:_[^%]*)?%"); + private static final Pattern VAULT_ECONOMY_PATTERN = + Pattern.compile("(?i)%vault_eco_[^%]+%"); + private static final long WARNING_INTERVAL_NANOS = TimeUnit.MINUTES.toNanos(5); + private static final int MAX_WARNING_KEYS = 2_048; + private static final ConcurrentMap LAST_WARNINGS = new ConcurrentHashMap<>(); + + private PlaceholderAPIHook() { + } public static String applyPlaceholders(String text, Player player) { return applyPlaceholders( text, player, pluginName -> Bukkit.getPluginManager().isPluginEnabled(pluginName), - me.clip.placeholderapi.PlaceholderAPI::setPlaceholders + me.clip.placeholderapi.PlaceholderAPI::setPlaceholders, + PlaceholderAPIHook::isVaultEconomyAvailable, + PlaceholderAPIHook::logWarning, + System::nanoTime ); } @@ -23,11 +58,254 @@ static String applyPlaceholders( Predicate pluginEnabled, BiFunction resolver ) { - String output = text; - if (player != null && pluginEnabled.test("PlaceholderAPI")) { - output = resolver.apply(player, text); + return applyPlaceholders( + text, + player, + pluginEnabled, + resolver, + () -> true, + (message, failure) -> { }, + System::nanoTime + ); + } + + static String applyPlaceholders( + String text, + Player player, + Predicate pluginEnabled, + BiFunction resolver, + BooleanSupplier vaultEconomyAvailable, + BiConsumer warningSink, + LongSupplier nanoTime + ) { + Objects.requireNonNull(pluginEnabled, "pluginEnabled"); + Objects.requireNonNull(resolver, "resolver"); + Objects.requireNonNull(vaultEconomyAvailable, "vaultEconomyAvailable"); + Objects.requireNonNull(warningSink, "warningSink"); + Objects.requireNonNull(nanoTime, "nanoTime"); + if (text == null || player == null) { + return text; + } + + try { + if (!pluginEnabled.test("PlaceholderAPI")) { + return text; + } + } catch (RuntimeException | LinkageError failure) { + warnRateLimited(text, player, failure, warningSink, nanoTime.getAsLong()); + return text; + } + + MaskedText input = MaskedText.unchanged(text); + if (VAULT_ECONOMY_PATTERN.matcher(text).find() && !safeEconomyAvailability(vaultEconomyAvailable)) { + input = MaskedText.maskVaultEconomy(text); + warnRateLimited(text, player, null, warningSink, nanoTime.getAsLong()); + } + + try { + String resolved = resolver.apply(player, input.masked()); + if (resolved == null) { + throw new IllegalStateException("PlaceholderAPI returned null."); + } + return input.restore(resolved); + } catch (RuntimeException | LinkageError failure) { + warnRateLimited(text, player, failure, warningSink, nanoTime.getAsLong()); + return text; + } + } + + private static boolean safeEconomyAvailability(BooleanSupplier availability) { + try { + return availability.getAsBoolean(); + } catch (RuntimeException | LinkageError ignored) { + return false; + } + } + + private static boolean isVaultEconomyAvailable() { + try { + if (!Bukkit.getPluginManager().isPluginEnabled("Vault")) { + return false; + } + Class economyClass = Class.forName( + "net.milkbowl.vault.economy.Economy", + false, + PlaceholderAPIHook.class.getClassLoader() + ); + RegisteredServiceProvider registration = economyRegistration(economyClass); + if (registration == null || registration.getProvider() == null + || registration.getPlugin() == null || !registration.getPlugin().isEnabled()) { + return false; + } + Object provider = registration.getProvider(); + try { + JavaPlugin providingPlugin = JavaPlugin.getProvidingPlugin(provider.getClass()); + if (!providingPlugin.isEnabled()) { + return false; + } + } catch (IllegalArgumentException ignored) { + // Generated providers can use a bridge class loader; the service owner check above remains valid. + } + return providerReportsEnabled(provider); + } catch (ClassNotFoundException | RuntimeException | LinkageError ignored) { + return false; } - return output; } + @SuppressWarnings({"rawtypes", "unchecked"}) + private static RegisteredServiceProvider economyRegistration(Class economyClass) { + return Bukkit.getServicesManager().getRegistration((Class) economyClass); + } + + private static boolean providerReportsEnabled(Object provider) { + try { + Method method = provider.getClass().getMethod("isEnabled"); + Object result = method.invoke(provider); + return !(result instanceof Boolean enabled) || enabled; + } catch (NoSuchMethodException ignored) { + return true; + } catch (IllegalAccessException | InvocationTargetException | RuntimeException | LinkageError ignored) { + return false; + } + } + + private static void warnRateLimited( + String text, + Player player, + Throwable failure, + BiConsumer warningSink, + long now + ) { + WarningKey key = new WarningKey( + playerKey(player), + expansionKey(text), + text.hashCode(), + text.length() + ); + AtomicBoolean emit = new AtomicBoolean(); + LAST_WARNINGS.compute(key, (ignored, previous) -> { + if (previous == null || now - previous >= WARNING_INTERVAL_NANOS) { + emit.set(true); + return now; + } + return previous; + }); + trimWarningState(now); + if (!emit.get()) { + return; + } + + String playerName = safePlayerName(player); + String expansions = key.expansions(); + String message = failure == null + ? "Vault economy placeholders were left unresolved for player '" + playerName + + "' because no enabled economy provider is available (expansions: " + expansions + ")." + : "Placeholder resolution failed for player '" + playerName + "' (expansions: " + expansions + + "); keeping the original unresolved message."; + warningSink.accept(message, failure); + } + + private static void trimWarningState(long now) { + if (LAST_WARNINGS.size() <= MAX_WARNING_KEYS) { + return; + } + long staleBefore = now - WARNING_INTERVAL_NANOS * 2; + LAST_WARNINGS.entrySet().removeIf(entry -> entry.getValue() < staleBefore); + if (LAST_WARNINGS.size() <= MAX_WARNING_KEYS) { + return; + } + int toRemove = LAST_WARNINGS.size() - MAX_WARNING_KEYS; + for (WarningKey key : LAST_WARNINGS.keySet()) { + if (toRemove-- <= 0) { + break; + } + LAST_WARNINGS.remove(key); + } + } + + private static String expansionKey(String text) { + Set expansions = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + Matcher matcher = PLACEHOLDER_PATTERN.matcher(text); + while (matcher.find()) { + expansions.add(matcher.group(1).toLowerCase(java.util.Locale.ROOT)); + } + return expansions.isEmpty() ? "unknown" : String.join(",", expansions); + } + + private static String playerKey(Player player) { + try { + UUID uniqueId = player.getUniqueId(); + if (uniqueId != null) { + return uniqueId.toString(); + } + } catch (RuntimeException ignored) { + // Fall through to a stable best-effort key. + } + String name = safePlayerName(player); + return name + "@" + Integer.toUnsignedString(System.identityHashCode(player)); + } + + private static String safePlayerName(Player player) { + try { + String name = player.getName(); + if (name != null && !name.isBlank()) { + return name; + } + } catch (RuntimeException ignored) { + // Use a non-identifying fallback. + } + return "unknown"; + } + + private static void logWarning(String message, Throwable failure) { + if (failure == null) { + Bukkit.getLogger().warning("[ServerFeatures] " + message); + } else { + Bukkit.getLogger().log(Level.WARNING, "[ServerFeatures] " + message, failure); + } + } + + static void clearWarningStateForTests() { + LAST_WARNINGS.clear(); + } + + private record WarningKey( + String player, + String expansions, + int messageHash, + int messageLength + ) { + } + + private record MaskedText(String masked, Map replacements) { + private static MaskedText unchanged(String text) { + return new MaskedText(text, Map.of()); + } + + private static MaskedText maskVaultEconomy(String text) { + Matcher matcher = VAULT_ECONOMY_PATTERN.matcher(text); + StringBuffer masked = new StringBuffer(text.length()); + Map replacements = new LinkedHashMap<>(); + int index = 0; + while (matcher.find()) { + String token; + do { + token = "__SERVERFEATURES_VAULT_ECONOMY_" + index++ + "_" + + Integer.toUnsignedString(text.hashCode()) + "__"; + } while (text.contains(token) || replacements.containsKey(token)); + replacements.put(token, matcher.group()); + matcher.appendReplacement(masked, Matcher.quoteReplacement(token)); + } + matcher.appendTail(masked); + return new MaskedText(masked.toString(), Map.copyOf(replacements)); + } + + private String restore(String resolved) { + String restored = resolved; + for (Map.Entry replacement : replacements.entrySet()) { + restored = restored.replace(replacement.getKey(), replacement.getValue()); + } + return restored; + } + } } diff --git a/serverfeatures-api/src/test/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHookTest.java b/serverfeatures-api/src/test/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHookTest.java index b849050f..5f4336ee 100644 --- a/serverfeatures-api/src/test/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHookTest.java +++ b/serverfeatures-api/src/test/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHookTest.java @@ -2,10 +2,15 @@ import nl.hauntedmc.serverfeatures.util.InterfaceProxy; import org.bukkit.entity.Player; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.util.Map; +import java.util.UUID; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -13,6 +18,11 @@ class PlaceholderAPIHookTest { + @BeforeEach + void resetWarningLimiter() { + PlaceholderAPIHook.clearWarningStateForTests(); + } + @Test void returnsOriginalTextWhenPlayerIsNull() { AtomicBoolean resolverCalled = new AtomicBoolean(false); @@ -33,14 +43,14 @@ void returnsOriginalTextWhenPlayerIsNull() { @Test void returnsOriginalTextWhenPluginIsDisabled() { - Player player = InterfaceProxy.of(Player.class, Map.of()); + Player player = player("Remy"); AtomicBoolean resolverCalled = new AtomicBoolean(false); String output = PlaceholderAPIHook.applyPlaceholders( "hello", player, name -> false, - (p, text) -> { + (ignored, text) -> { resolverCalled.set(true); return "changed"; } @@ -52,16 +62,125 @@ void returnsOriginalTextWhenPluginIsDisabled() { @Test void appliesResolverWhenPluginIsEnabledAndPlayerExists() { - Player player = InterfaceProxy.of(Player.class, Map.of("getName", args -> "Remy")); + Player player = player("Remy"); String output = PlaceholderAPIHook.applyPlaceholders( "hello %player%", player, name -> "PlaceholderAPI".equals(name), - (p, text) -> text.replace("%player%", p.getName()) + (resolvedPlayer, text) -> text.replace("%player%", resolvedPlayer.getName()) ); assertEquals("hello Remy", output); assertTrue(output.contains("Remy")); } + + @Test + void preservesOriginalMessageAndRateLimitsExpansionFailures() { + Player player = player("Remy"); + AtomicInteger warnings = new AtomicInteger(); + AtomicLong now = new AtomicLong(); + String input = "Balance: %vault_eco_balance%"; + + for (int attempt = 0; attempt < 2; attempt++) { + assertEquals(input, failingResolution(input, player, warnings, now)); + } + assertEquals(1, warnings.get()); + + now.addAndGet(TimeUnit.MINUTES.toNanos(5)); + assertEquals(input, failingResolution(input, player, warnings, now)); + assertEquals(2, warnings.get()); + } + + @Test + void containsBinaryIncompatibilityErrorsFromExpansions() { + Player player = player("Remy"); + AtomicInteger warnings = new AtomicInteger(); + String input = "Balance: %vault_eco_balance%"; + + String output = PlaceholderAPIHook.applyPlaceholders( + input, + player, + name -> true, + (ignored, text) -> { + throw new NoSuchMethodError("incompatible expansion"); + }, + () -> true, + (message, failure) -> warnings.incrementAndGet(), + System::nanoTime + ); + + assertEquals(input, output); + assertEquals(1, warnings.get()); + } + + @Test + void leavesVaultEconomyPlaceholdersUnresolvedWhenProviderIsUnavailable() { + Player player = player("Remy"); + AtomicBoolean resolverSawVaultPlaceholder = new AtomicBoolean(); + AtomicInteger warnings = new AtomicInteger(); + + String output = PlaceholderAPIHook.applyPlaceholders( + "%player_name% has %vault_eco_balance%", + player, + name -> true, + (ignored, text) -> { + resolverSawVaultPlaceholder.set(text.contains("%vault_eco_")); + return text.replace("%player_name%", "Remy"); + }, + () -> false, + (message, failure) -> warnings.incrementAndGet(), + System::nanoTime + ); + + assertEquals("Remy has %vault_eco_balance%", output); + assertFalse(resolverSawVaultPlaceholder.get()); + assertEquals(1, warnings.get()); + } + + @Test + void resolvesVaultEconomyPlaceholdersWhenProviderIsAvailable() { + Player player = player("Remy"); + + String output = PlaceholderAPIHook.applyPlaceholders( + "%player_name% has %vault_eco_balance%", + player, + name -> true, + (ignored, text) -> text + .replace("%player_name%", "Remy") + .replace("%vault_eco_balance%", "125.00"), + () -> true, + (message, failure) -> { }, + System::nanoTime + ); + + assertEquals("Remy has 125.00", output); + } + + private static String failingResolution( + String input, + Player player, + AtomicInteger warnings, + AtomicLong now + ) { + return PlaceholderAPIHook.applyPlaceholders( + input, + player, + name -> true, + (ignored, text) -> { + throw new IllegalStateException("broken expansion"); + }, + () -> true, + (message, failure) -> warnings.incrementAndGet(), + now::get + ); + } + + private static Player player(String name) { + UUID uniqueId = UUID.nameUUIDFromBytes(name.getBytes(java.nio.charset.StandardCharsets.UTF_8)); + return InterfaceProxy.of(Player.class, Map.of( + "getName", arguments -> name, + "getUniqueId", arguments -> uniqueId + )); + } } diff --git a/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/Scoreboard.java b/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/Scoreboard.java index cd143e75..7c5b24b9 100644 --- a/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/Scoreboard.java +++ b/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/Scoreboard.java @@ -1,9 +1,9 @@ package nl.hauntedmc.serverfeatures.features.scoreboard; -import nl.hauntedmc.serverfeatures.features.FeatureContext; import nl.hauntedmc.serverfeatures.api.io.config.ConfigMap; import nl.hauntedmc.serverfeatures.api.io.localization.MessageMap; import nl.hauntedmc.serverfeatures.features.BukkitBaseFeature; +import nl.hauntedmc.serverfeatures.features.FeatureContext; import nl.hauntedmc.serverfeatures.features.scoreboard.internal.ScoreboardHandler; import nl.hauntedmc.serverfeatures.features.scoreboard.listener.PlayerJoinListener; import nl.hauntedmc.serverfeatures.features.scoreboard.meta.Meta; @@ -23,7 +23,6 @@ public ConfigMap getDefaultConfig() { defaults.put("enabled", false); defaults.put("refresh_interval", 100); return defaults; - } @Override @@ -48,20 +47,18 @@ public MessageMap getDefaultMessages() { return messages; } - @Override public void initialize() { scoreboardHandler = new ScoreboardHandler(this); scoreboardHandler.startUpdater(); getLifecycleManager().getListenerManager().registerListener(new PlayerJoinListener(scoreboardHandler)); - // Initialize the scoreboard for all currently online players. - Bukkit.getOnlinePlayers().forEach(scoreboardHandler::updateScoreboardContent); + // Initialize each online player independently so one broken expansion cannot abort the feature. + Bukkit.getOnlinePlayers().forEach(scoreboardHandler::updateScoreboardSafely); } @Override public void disable() { scoreboardHandler.removeAllPlayers(); } - -} \ No newline at end of file +} diff --git a/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/internal/ScoreboardHandler.java b/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/internal/ScoreboardHandler.java index 6a447c34..7f3b3ebd 100644 --- a/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/internal/ScoreboardHandler.java +++ b/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/internal/ScoreboardHandler.java @@ -12,17 +12,24 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.BiConsumer; +import java.util.function.Consumer; +import java.util.logging.Level; -/** - * Generates the localized lines each tick and delegates to ScoreboardManager. - */ +/** Generates localized scoreboard content and isolates third-party rendering failures. */ public class ScoreboardHandler { private static final int MAX_LINES = 15; + private static final long WARNING_INTERVAL_NANOS = TimeUnit.MINUTES.toNanos(5); private final Scoreboard feature; private final LocalizationHandler i18n; - private final Map> lastScoreboardLines = new ConcurrentHashMap<>(); + private final Map lastScoreboards = new ConcurrentHashMap<>(); + private final ConcurrentMap lastWarnings = new ConcurrentHashMap<>(); private final int refreshInterval; public ScoreboardHandler(Scoreboard feature) { @@ -31,54 +38,167 @@ public ScoreboardHandler(Scoreboard feature) { this.refreshInterval = (int) feature.getConfigHandler().get("refresh_interval"); } - /** - * Immediately recalculates and pushes the sidebar for one player. - */ + /** Immediately recalculates and pushes the sidebar for one player. */ public void updateScoreboardContent(Player player) { - Component title = i18n.getMessage("scoreboard.title").forAudience(player).build(); + Component title = renderMessageSafely("scoreboard.title", player); + if (title == null) { + title = Component.empty(); + } + List lines = new ArrayList<>(); - for (int i = 1; i <= MAX_LINES; i++) { - Component c = i18n.getMessage("scoreboard.line" + i).forAudience(player).build(); - String s = ComponentFormatter.serialize(c).format(ComponentFormatter.Serializer.Format.PLAIN).build(); - if (s.startsWith("")) break; - lines.add(c); + for (int lineNumber = 1; lineNumber <= MAX_LINES; lineNumber++) { + String messageKey = "scoreboard.line" + lineNumber; + Component line = renderMessageSafely(messageKey, player); + if (line == null) { + continue; + } + String plain; + try { + plain = ComponentFormatter.serialize(line) + .format(ComponentFormatter.Serializer.Format.PLAIN) + .build(); + } catch (RuntimeException | LinkageError failure) { + reportFailure(player, messageKey + ".serialize", failure); + continue; + } + if (plain.startsWith("")) { + break; + } + lines.add(line); } - List oldLines = lastScoreboardLines.get(player); - if (oldLines != null && oldLines.equals(lines)) { + UUID playerId = player.getUniqueId(); + ScoreboardSnapshot previous = lastScoreboards.get(playerId); + ScoreboardSnapshot next = new ScoreboardSnapshot(title, List.copyOf(lines)); + if (next.equals(previous)) { return; } - lastScoreboardLines.put(player, new ArrayList<>(lines)); - ScoreboardManager.updateSidebar(player, title, lines, oldLines); + + ScoreboardManager.updateSidebar( + player, + title, + lines, + previous == null ? null : previous.lines() + ); + lastScoreboards.put(playerId, next); } - /** - * Runs forceUpdate once every `refreshInterval` ticks for all online players - */ + /** Updates one player without allowing a rendering or provider failure to escape to an event. */ + public void updateScoreboardSafely(Player player) { + try { + updateScoreboardContent(player); + } catch (RuntimeException | LinkageError failure) { + reportFailure(player, "scoreboard.update", failure); + } + } + + /** Runs one independently guarded update for every online player. */ public void startUpdater() { feature.getLifecycleManager().getTaskManager() - .scheduleRepeatingTask(() -> - Bukkit.getOnlinePlayers().forEach(this::updateScoreboardContent), + .scheduleRepeatingTask(() -> updatePlayersIndependently( + Bukkit.getOnlinePlayers(), + this::updateScoreboardContent, + (player, failure) -> reportFailure(player, "scoreboard.update", failure) + ), BukkitTime.ticks(0L), BukkitTime.ticks(refreshInterval)); } - /** - * Removes the player's scoreboard from the handler. - * - * @param player the player to remove - */ + /** Removes the player's scoreboard from the handler. */ public void removePlayer(Player player) { - lastScoreboardLines.remove(player); - ScoreboardManager.removeSidebar(player); + UUID playerId = player.getUniqueId(); + lastScoreboards.remove(playerId); + lastWarnings.keySet().removeIf(key -> key.playerId().equals(playerId)); + try { + ScoreboardManager.removeSidebar(player); + } catch (RuntimeException | LinkageError failure) { + reportFailure(player, "scoreboard.remove", failure); + } } - /** - * Removes all players from scoreboard tracking and resets their scoreboards to the main scoreboard. - */ + /** Removes all tracked sidebars while isolating failures per player. */ public void removeAllPlayers() { - lastScoreboardLines.clear(); - for (Player player : Bukkit.getOnlinePlayers()) { - ScoreboardManager.removeSidebar(player); + lastScoreboards.clear(); + lastWarnings.clear(); + updatePlayersIndependently( + Bukkit.getOnlinePlayers(), + ScoreboardManager::removeSidebar, + (player, failure) -> reportFailure(player, "scoreboard.remove", failure) + ); + } + + private Component renderMessageSafely(String messageKey, Player player) { + try { + return i18n.getMessage(messageKey).forAudience(player).build(); + } catch (RuntimeException | LinkageError failure) { + reportFailure(player, messageKey, failure); + return null; + } + } + + private void reportFailure(Player player, String messageKey, Throwable failure) { + UUID playerId = safePlayerId(player); + FailureKey key = new FailureKey(playerId, messageKey, failure.getClass().getName()); + long now = System.nanoTime(); + AtomicBoolean emit = new AtomicBoolean(); + lastWarnings.compute(key, (ignored, previous) -> { + if (previous == null || now - previous >= WARNING_INTERVAL_NANOS) { + emit.set(true); + return now; + } + return previous; + }); + if (!emit.get()) { + return; + } + feature.getPlugin().getLogger().log( + Level.WARNING, + "[Scoreboard] Failed to render '" + messageKey + "' for player '" + + safePlayerName(player) + "'; the remaining scoreboard update will continue.", + failure + ); + } + + private static UUID safePlayerId(Player player) { + try { + UUID uniqueId = player.getUniqueId(); + if (uniqueId != null) { + return uniqueId; + } + } catch (RuntimeException ignored) { + // Use an instance-derived UUID solely for warning suppression. + } + return new UUID(0L, Integer.toUnsignedLong(System.identityHashCode(player))); + } + + private static String safePlayerName(Player player) { + try { + String name = player.getName(); + if (name != null && !name.isBlank()) { + return name; + } + } catch (RuntimeException ignored) { + // Use a non-identifying fallback. } + return "unknown"; + } + + static void updatePlayersIndependently( + Iterable players, + Consumer updater, + BiConsumer failureHandler + ) { + for (Player player : players) { + try { + updater.accept(player); + } catch (RuntimeException | LinkageError failure) { + failureHandler.accept(player, failure); + } + } + } + + private record ScoreboardSnapshot(Component title, List lines) { + } + + private record FailureKey(UUID playerId, String messageKey, String failureType) { } } diff --git a/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/listener/PlayerJoinListener.java b/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/listener/PlayerJoinListener.java index 81d22caf..3a876aed 100644 --- a/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/listener/PlayerJoinListener.java +++ b/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/listener/PlayerJoinListener.java @@ -17,12 +17,11 @@ public PlayerJoinListener(ScoreboardHandler scoreboardHandler) { @EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true) public void onPlayerJoin(PlayerJoinEvent event) { - scoreboardHandler.updateScoreboardContent(event.getPlayer()); + scoreboardHandler.updateScoreboardSafely(event.getPlayer()); } @EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true) public void onPlayerQuit(PlayerQuitEvent event) { scoreboardHandler.removePlayer(event.getPlayer()); } - } diff --git a/serverfeatures-platform-paper/src/test/java/nl/hauntedmc/serverfeatures/features/scoreboard/internal/ScoreboardHandlerTest.java b/serverfeatures-platform-paper/src/test/java/nl/hauntedmc/serverfeatures/features/scoreboard/internal/ScoreboardHandlerTest.java new file mode 100644 index 00000000..027f5cc8 --- /dev/null +++ b/serverfeatures-platform-paper/src/test/java/nl/hauntedmc/serverfeatures/features/scoreboard/internal/ScoreboardHandlerTest.java @@ -0,0 +1,44 @@ +package nl.hauntedmc.serverfeatures.features.scoreboard.internal; + +import nl.hauntedmc.serverfeatures.util.InterfaceProxy; +import org.bukkit.entity.Player; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class ScoreboardHandlerTest { + + @Test + void continuesUpdatingOtherPlayersAfterRuntimeAndLinkageFailures() { + Player runtimeFailure = player("runtime"); + Player successful = player("successful"); + Player linkageFailure = player("linkage"); + AtomicInteger updates = new AtomicInteger(); + AtomicInteger failures = new AtomicInteger(); + + ScoreboardHandler.updatePlayersIndependently( + List.of(runtimeFailure, successful, linkageFailure), + player -> { + if (player == runtimeFailure) { + throw new IllegalStateException("broken expansion"); + } + if (player == linkageFailure) { + throw new NoSuchMethodError("incompatible expansion"); + } + updates.incrementAndGet(); + }, + (player, failure) -> failures.incrementAndGet() + ); + + assertEquals(1, updates.get()); + assertEquals(2, failures.get()); + } + + private static Player player(String name) { + return InterfaceProxy.of(Player.class, Map.of("getName", arguments -> name)); + } +}