From 5190b0d05cb6ab05c68f0e348191f88ca787ad5d Mon Sep 17 00:00:00 2001 From: bobhenl <18662086+bobhenl@users.noreply.github.com> Date: Thu, 29 Oct 2026 10:07:54 +0100 Subject: [PATCH 1/2] fix(playerui): build heads from the live player profile Skull profiles without textures trigger a Mojang lookup on every build, which never succeeds for offline-mode UUIDs. Use the player's own profile instead and keep setOwningPlayer only for v4 UUIDs. --- .../screen/playerui/HeadCache.java | 81 +++++++--- .../screen/playerui/HeadCacheTest.java | 143 ++++++++++++++++++ 2 files changed, 204 insertions(+), 20 deletions(-) diff --git a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/screen/playerui/HeadCache.java b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/screen/playerui/HeadCache.java index fab688f..1c074c0 100644 --- a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/screen/playerui/HeadCache.java +++ b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/screen/playerui/HeadCache.java @@ -1,5 +1,7 @@ package dev.cyr1en.promptpaper.screen.playerui; +import com.destroystokyo.paper.profile.PlayerProfile; +import com.destroystokyo.paper.profile.ProfileProperty; import dev.cyr1en.promptpaper.CommandPrompter; import dev.cyr1en.promptpaper.hook.HookContainer; import dev.cyr1en.promptpaper.hook.hooks.FilterHook; @@ -12,7 +14,9 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import net.kyori.adventure.text.serializer.plain.PlainTextComponentSerializer; @@ -42,6 +46,8 @@ public class HeadCache implements Listener { private final Scheduler scheduler; private final int maxCacheSize; private final Map> cache; + // UUIDs whose cached head was built without skin textures; always a subset of the cache keys. + private final Set untextured = ConcurrentHashMap.newKeySet(); private final List filters; public HeadCache(CommandPrompter plugin, Scheduler scheduler) { @@ -54,7 +60,9 @@ public HeadCache(CommandPrompter plugin, Scheduler scheduler) { @Override protected boolean removeEldestEntry(Map.Entry> eldest) { int max = maxCacheSize; - return max > 0 && size() > max; + boolean evict = max > 0 && size() > max; + if (evict) untextured.remove(eldest.getKey()); + return evict; } }); this.filters = new ArrayList<>(); @@ -151,25 +159,55 @@ private TokenMatch longestMatchAt(String key, int pos) { */ public Optional getHeadFor(Player player) { if (isVanished(player)) return Optional.empty(); - return cache.computeIfAbsent( - player.getUniqueId(), - uuid -> { - if (!Bukkit.getOnlinePlayers().contains(player)) return Optional.empty(); - var skull = new ItemStack(Material.PLAYER_HEAD); - var meta = (SkullMeta) Bukkit.getItemFactory().getItemMeta(Material.PLAYER_HEAD); - if (meta != null) { - meta.setOwningPlayer(player); - var promptConfig = plugin.getConfigLoader().getPromptConfig(); - var format = promptConfig.skullNameFormat(); - var cmData = promptConfig.skullCustomModelData(); - meta.displayName(ComponentUtil.mini("" + format.formatted(player.getName()))); - if (cmData != 0) { - applyCustomModelData(meta, cmData); - } - skull.setItemMeta(meta); - } - return Optional.of(skull); - }); + var uuid = player.getUniqueId(); + // A head cached before the skin was applied (e.g. by a skin plugin shortly after join) is + // rebuilt once the live profile carries textures. Every other cached head is reused as-is. + if (untextured.contains(uuid) && hasTextures(player.getPlayerProfile())) invalidate(player); + var head = cache.computeIfAbsent(uuid, ignored -> createHead(player)); + // Not cached: the player is not online, so there is nothing to show and no entry to clean up. + return head != null ? head : Optional.empty(); + } + + /** + * Builds the head for an online player, or returns null (not cached) if the player is offline. + * + *

The skull is given the player's own live {@link PlayerProfile}, which already carries the + * {@code textures} property (from Mojang in online mode, or from a skin plugin in offline mode), + * so no profile lookup is needed. A skull profile without textures makes the server call {@link + * PlayerProfile#update()}, a Mojang profile request that can never succeed for offline-mode + * (version 3) UUIDs. Such players get a plain head instead; only version 4 (Mojang) UUIDs keep + * the old lookup-based {@link SkullMeta#setOwningPlayer} fallback. + */ + private Optional createHead(Player player) { + if (!Bukkit.getOnlinePlayers().contains(player)) return null; + var skull = new ItemStack(Material.PLAYER_HEAD); + var meta = (SkullMeta) Bukkit.getItemFactory().getItemMeta(Material.PLAYER_HEAD); + if (meta != null) { + var profile = player.getPlayerProfile(); + if (hasTextures(profile)) { + meta.setPlayerProfile(profile); + } else { + untextured.add(player.getUniqueId()); + if (player.getUniqueId().version() == 4) meta.setOwningPlayer(player); + } + var promptConfig = plugin.getConfigLoader().getPromptConfig(); + var format = promptConfig.skullNameFormat(); + var cmData = promptConfig.skullCustomModelData(); + meta.displayName(ComponentUtil.mini("" + format.formatted(player.getName()))); + if (cmData != 0) { + applyCustomModelData(meta, cmData); + } + skull.setItemMeta(meta); + } + return Optional.of(skull); + } + + private static boolean hasTextures(PlayerProfile profile) { + if (profile == null) return false; + for (ProfileProperty property : profile.getProperties()) { + if ("textures".equals(property.getName()) && !property.getValue().isEmpty()) return true; + } + return false; } @SuppressWarnings("deprecation") @@ -185,6 +223,7 @@ private void applyCustomModelData(SkullMeta meta, int cmData) { public void invalidate(Player player) { cache.remove(player.getUniqueId()); + untextured.remove(player.getUniqueId()); } /** @@ -229,6 +268,7 @@ public int size() { */ public void buildCache(Runnable callback) { cache.clear(); + untextured.clear(); var players = new ArrayList(Bukkit.getOnlinePlayers()); players.removeIf(this::isVanished); processBatch(players, 0, callback, new AtomicBoolean()); @@ -288,6 +328,7 @@ private void continueBatch( public void reBuildCache() { cache.clear(); + untextured.clear(); } @EventHandler diff --git a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/screen/playerui/HeadCacheTest.java b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/screen/playerui/HeadCacheTest.java index aecebc7..23a32c8 100644 --- a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/screen/playerui/HeadCacheTest.java +++ b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/screen/playerui/HeadCacheTest.java @@ -3,20 +3,26 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import com.destroystokyo.paper.profile.PlayerProfile; +import com.destroystokyo.paper.profile.ProfileProperty; import dev.cyr1en.promptpaper.MockBukkitTest; import dev.cyr1en.promptpaper.config.PromptConfig; import dev.cyr1en.promptpaper.hook.HookContainer; import dev.cyr1en.promptpaper.hook.hooks.FilterHook; import dev.cyr1en.promptpaper.hook.hooks.VanishHook; import java.lang.reflect.Field; +import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.UUID; import java.util.concurrent.atomic.AtomicInteger; import java.util.regex.Pattern; @@ -27,8 +33,10 @@ import org.bukkit.event.player.PlayerJoinEvent; import org.bukkit.event.player.PlayerQuitEvent; import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.SkullMeta; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockbukkit.mockbukkit.entity.PlayerMock; /** * Regression tests for the PlayerUI head-cache empty-entry bug. @@ -465,6 +473,141 @@ String getParam() { } } + @Test + void getHeadForUsesLivePlayerProfileTextures() { + var player = offlinePlayer("Skinned"); + giveTextures(player, "c2tpbg=="); + + Optional head = headCache.getHeadFor(player); + + assertTrue(head.isPresent()); + var profile = ((SkullMeta) head.get().getItemMeta()).getPlayerProfile(); + assertNotNull(profile, "a textured player must get their own profile on the skull"); + assertEquals(player.getUniqueId(), profile.getId()); + assertEquals("c2tpbg==", texturesOf(profile), "the skull must carry the live skin textures"); + } + + @Test + void getHeadForOfflineUuidWithoutTexturesUsesPlainHead() { + var player = offlinePlayer("NoSkin"); + assertEquals(3, player.getUniqueId().version(), "fixture must use an offline-mode UUID"); + + Optional head = headCache.getHeadFor(player); + + assertTrue(head.isPresent()); + assertNull( + ((SkullMeta) head.get().getItemMeta()).getPlayerProfile(), + "an untextured offline-mode skull must not get a profile the server would look up"); + } + + @Test + void getHeadForMojangUuidWithoutTexturesKeepsOwningPlayer() { + var player = new PlayerMock(server, "OnlineMode", UUID.randomUUID()); + server.addPlayer(player); + + Optional head = headCache.getHeadFor(player); + + assertTrue(head.isPresent()); + var profile = ((SkullMeta) head.get().getItemMeta()).getPlayerProfile(); + assertNotNull(profile, "version 4 UUIDs keep the owning-player fallback"); + assertEquals(player.getUniqueId(), profile.getId()); + } + + @Test + void getHeadForReusesCachedHead() { + var player = offlinePlayer("Reused"); + giveTextures(player, "c2tpbg=="); + + var first = headCache.getHeadFor(player).orElseThrow(); + var second = headCache.getHeadFor(player).orElseThrow(); + + assertSame(first, second, "an already cached head must not be rebuilt"); + } + + @Test + void getHeadForRebuildsUntexturedHeadOnceTexturesArrive() { + var player = offlinePlayer("LateSkin"); + var plain = headCache.getHeadFor(player).orElseThrow(); + assertSame(plain, headCache.getHeadFor(player).orElseThrow()); + + giveTextures(player, "bGF0ZQ=="); + var skinned = headCache.getHeadFor(player).orElseThrow(); + + assertNotSame(plain, skinned, "the plain head must be replaced once the skin is applied"); + assertEquals("bGF0ZQ==", texturesOf(((SkullMeta) skinned.getItemMeta()).getPlayerProfile())); + assertSame(skinned, headCache.getHeadFor(player).orElseThrow()); + } + + @Test + void getHeadForDoesNotCacheOfflinePlayer() { + var player = createPlayer("Gone"); + player.disconnect(); + + assertTrue(headCache.getHeadFor(player).isEmpty()); + assertFalse( + readCacheMap().containsKey(player.getUniqueId()), + "an offline player must not leave an entry that no quit event would remove"); + } + + @Test + void onPlayerQuitDropsUntexturedMarker() { + var player = offlinePlayer("QuitPlain"); + headCache.onPlayerJoin(new PlayerJoinEvent(player, (Component) null)); + assertTrue(readUntextured().contains(player.getUniqueId())); + + headCache.onPlayerQuit( + new PlayerQuitEvent(player, (Component) null, PlayerQuitEvent.QuitReason.DISCONNECTED)); + + assertTrue(readUntextured().isEmpty(), "quit must drop every per-player entry"); + } + + @Test + void evictionDropsUntexturedMarker() { + when(promptCfg.cacheSize()).thenReturn(1); + headCache = new HeadCache(plugin, scheduler); + var first = offlinePlayer("EvictedPlain"); + var second = offlinePlayer("KeptPlain"); + + headCache.getHeadFor(first); + headCache.getHeadFor(second); + + assertFalse(readUntextured().contains(first.getUniqueId())); + assertTrue(readUntextured().contains(second.getUniqueId())); + } + + /** Online player with an offline-mode (version 3) UUID, as on an offline-mode server. */ + private PlayerMock offlinePlayer(String name) { + var uuid = UUID.nameUUIDFromBytes(("OfflinePlayer:" + name).getBytes(StandardCharsets.UTF_8)); + var player = new PlayerMock(server, name, uuid); + server.addPlayer(player); + return player; + } + + private static void giveTextures(PlayerMock player, String value) { + var profile = player.getPlayerProfile(); + profile.setProperty(new ProfileProperty("textures", value)); + player.setPlayerProfile(profile); + } + + private static String texturesOf(PlayerProfile profile) { + return profile.getProperties().stream() + .filter(p -> p.getName().equals("textures")) + .map(ProfileProperty::getValue) + .findFirst() + .orElse(null); + } + + @SuppressWarnings("unchecked") + private Set readUntextured() { + try { + Field field = HeadCache.class.getDeclaredField("untextured"); + field.setAccessible(true); + return (Set) field.get(headCache); + } catch (ReflectiveOperationException e) { + throw new RuntimeException("Failed to read untextured set", e); + } + } + /** * Installs a mocked vanish hook on the plugin's hook container that reports the given players as * vanished. From 7a9d8e5aada567db40ade3c68008456b79fe2ab8 Mon Sep 17 00:00:00 2001 From: bobhenl <18662086+bobhenl@users.noreply.github.com> Date: Wed, 11 Nov 2026 17:22:18 +0100 Subject: [PATCH 2/2] fix(playerui): keep plain heads selectable, single-map head cache Store the owner name in the head's PDC, mirror the server's skin/cape check, keep head and textures key in one entry and rebuild on skin change. --- .../screen/playerui/HeadCache.java | 120 +++++++++------ .../screen/playerui/PlayerUIScreen.java | 39 ++--- .../screen/playerui/HeadCacheTest.java | 145 ++++++++---------- .../screen/playerui/PlayerUIScreenTest.java | 43 ++++++ .../promptpaper/testutil/TestPlayerMock.java | 5 + 5 files changed, 200 insertions(+), 152 deletions(-) diff --git a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/screen/playerui/HeadCache.java b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/screen/playerui/HeadCache.java index 1c074c0..a8e0c92 100644 --- a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/screen/playerui/HeadCache.java +++ b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/screen/playerui/HeadCache.java @@ -1,7 +1,6 @@ package dev.cyr1en.promptpaper.screen.playerui; import com.destroystokyo.paper.profile.PlayerProfile; -import com.destroystokyo.paper.profile.ProfileProperty; import dev.cyr1en.promptpaper.CommandPrompter; import dev.cyr1en.promptpaper.hook.HookContainer; import dev.cyr1en.promptpaper.hook.hooks.FilterHook; @@ -13,15 +12,15 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; -import java.util.Set; import java.util.UUID; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import net.kyori.adventure.text.serializer.plain.PlainTextComponentSerializer; import org.bukkit.Bukkit; import org.bukkit.Material; +import org.bukkit.NamespacedKey; import org.bukkit.entity.Player; import org.bukkit.event.EventHandler; import org.bukkit.event.Listener; @@ -29,6 +28,7 @@ import org.bukkit.event.player.PlayerQuitEvent; import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.meta.SkullMeta; +import org.bukkit.persistence.PersistentDataType; /** * Maintains a bounded LRU cache of player-head {@link ItemStack}s used by {@link PlayerUIScreen} @@ -45,24 +45,29 @@ public class HeadCache implements Listener { private final CommandPrompter plugin; private final Scheduler scheduler; private final int maxCacheSize; - private final Map> cache; - // UUIDs whose cached head was built without skin textures; always a subset of the cache keys. - private final Set untextured = ConcurrentHashMap.newKeySet(); + private final NamespacedKey ownerKey; + // Compound operations hold the map's own lock (synchronized (cache)). + private final Map cache; private final List filters; + /** + * A built head plus the textures key it was built from ({@code null} for a head built without + * textures). Both live in one value, so there is no second structure to keep in sync. + */ + private record CachedHead(ItemStack item, String textures) {} + public HeadCache(CommandPrompter plugin, Scheduler scheduler) { this.plugin = plugin; this.scheduler = scheduler; this.maxCacheSize = plugin.getConfigLoader().getPromptConfig().cacheSize(); + this.ownerKey = new NamespacedKey(plugin, "head_owner"); this.cache = Collections.synchronizedMap( - new LinkedHashMap>(16, 0.75f, true) { + new LinkedHashMap(16, 0.75f, true) { @Override - protected boolean removeEldestEntry(Map.Entry> eldest) { + protected boolean removeEldestEntry(Map.Entry eldest) { int max = maxCacheSize; - boolean evict = max > 0 && size() > max; - if (evict) untextured.remove(eldest.getKey()); - return evict; + return max > 0 && size() > max; } }); this.filters = new ArrayList<>(); @@ -160,36 +165,57 @@ private TokenMatch longestMatchAt(String key, int pos) { public Optional getHeadFor(Player player) { if (isVanished(player)) return Optional.empty(); var uuid = player.getUniqueId(); - // A head cached before the skin was applied (e.g. by a skin plugin shortly after join) is - // rebuilt once the live profile carries textures. Every other cached head is reused as-is. - if (untextured.contains(uuid) && hasTextures(player.getPlayerProfile())) invalidate(player); - var head = cache.computeIfAbsent(uuid, ignored -> createHead(player)); - // Not cached: the player is not online, so there is nothing to show and no entry to clean up. - return head != null ? head : Optional.empty(); + var liveTextures = texturesOf(player.getPlayerProfile()); + synchronized (cache) { + // A head built from other textures (none yet, or a skin changed mid-session, e.g. by a skin + // plugin after join) is rebuilt. Every other cached head is reused as-is. + var cached = cache.get(uuid); + if (cached != null && !Objects.equals(cached.textures(), liveTextures)) cache.remove(uuid); + var head = cache.computeIfAbsent(uuid, ignored -> createHead(player, liveTextures)); + // Not cached: the player is not online, so there is nothing to show and no entry to clean up. + return head != null ? Optional.of(head.item()) : Optional.empty(); + } } /** - * Builds the head for an online player, or returns null (not cached) if the player is offline. + * Returns the name of the player a head from this cache belongs to, or null if the item is not + * such a head. The name is stored on the item itself, so heads without an owning profile can + * still be identified. + */ + public String getOwnerName(ItemStack head) { + if (head == null) return null; + var meta = head.getItemMeta(); + if (meta == null) return null; + var name = meta.getPersistentDataContainer().get(ownerKey, PersistentDataType.STRING); + if (name != null) return name; + if (meta instanceof SkullMeta skull && skull.getOwningPlayer() != null) { + return skull.getOwningPlayer().getName(); + } + return null; + } + + /** + * Builds the head for an online player, or returns null (nothing is cached) if the player is + * offline. * - *

The skull is given the player's own live {@link PlayerProfile}, which already carries the - * {@code textures} property (from Mojang in online mode, or from a skin plugin in offline mode), - * so no profile lookup is needed. A skull profile without textures makes the server call {@link - * PlayerProfile#update()}, a Mojang profile request that can never succeed for offline-mode - * (version 3) UUIDs. Such players get a plain head instead; only version 4 (Mojang) UUIDs keep - * the old lookup-based {@link SkullMeta#setOwningPlayer} fallback. + *

A skull whose profile has no skin and no cape makes the server call {@link + * PlayerProfile#update()}, an uncached Mojang profile request that can never succeed for + * non-Mojang UUIDs (such as offline-mode version 3 UUIDs). So a textured player gets their live + * profile, an untextured player with a non-version-4 UUID gets a plain head, and only untextured + * version 4 (Mojang) UUIDs keep the lookup-based {@link SkullMeta#setOwningPlayer} fallback. The + * player name is written to the item's persistent data so plain heads stay selectable. */ - private Optional createHead(Player player) { + private CachedHead createHead(Player player, String textures) { if (!Bukkit.getOnlinePlayers().contains(player)) return null; var skull = new ItemStack(Material.PLAYER_HEAD); var meta = (SkullMeta) Bukkit.getItemFactory().getItemMeta(Material.PLAYER_HEAD); if (meta != null) { - var profile = player.getPlayerProfile(); - if (hasTextures(profile)) { - meta.setPlayerProfile(profile); - } else { - untextured.add(player.getUniqueId()); - if (player.getUniqueId().version() == 4) meta.setOwningPlayer(player); + if (textures != null) { + meta.setPlayerProfile(player.getPlayerProfile()); + } else if (player.getUniqueId().version() == 4) { + meta.setOwningPlayer(player); } + meta.getPersistentDataContainer().set(ownerKey, PersistentDataType.STRING, player.getName()); var promptConfig = plugin.getConfigLoader().getPromptConfig(); var format = promptConfig.skullNameFormat(); var cmData = promptConfig.skullCustomModelData(); @@ -199,15 +225,18 @@ private Optional createHead(Player player) { } skull.setItemMeta(meta); } - return Optional.of(skull); + return new CachedHead(skull, textures); } - private static boolean hasTextures(PlayerProfile profile) { - if (profile == null) return false; - for (ProfileProperty property : profile.getProperties()) { - if ("textures".equals(property.getName()) && !property.getValue().isEmpty()) return true; - } - return false; + /** + * Returns the profile's skin and cape URLs as one key, or null if it has neither (the same check + * the server makes before looking a skull profile up). + */ + private static String texturesOf(PlayerProfile profile) { + if (profile == null) return null; + var textures = profile.getTextures(); + if (textures.isEmpty()) return null; + return textures.getSkin() + "|" + textures.getCape(); } @SuppressWarnings("deprecation") @@ -223,7 +252,6 @@ private void applyCustomModelData(SkullMeta meta, int cmData) { public void invalidate(Player player) { cache.remove(player.getUniqueId()); - untextured.remove(player.getUniqueId()); } /** @@ -231,14 +259,17 @@ public void invalidate(Player player) { * cached. */ public List getHeads() { - return cache.entrySet().stream() - .filter(entry -> entry.getValue().isPresent()) + List> entries; + synchronized (cache) { + entries = cache.entrySet().stream().map(e -> Map.entry(e.getKey(), e.getValue())).toList(); + } + return entries.stream() .filter( entry -> { var cachedPlayer = Bukkit.getPlayer(entry.getKey()); return cachedPlayer != null && !isVanished(cachedPlayer); }) - .map(entry -> entry.getValue().get()) + .map(entry -> entry.getValue().item()) .toList(); } @@ -258,8 +289,7 @@ public List getHeadsSorted() { } public int size() { - // Count only populated entries so empty/unloaded heads are detected as stale. - return (int) cache.values().stream().filter(Optional::isPresent).count(); + return cache.size(); } /** @@ -268,7 +298,6 @@ public int size() { */ public void buildCache(Runnable callback) { cache.clear(); - untextured.clear(); var players = new ArrayList(Bukkit.getOnlinePlayers()); players.removeIf(this::isVanished); processBatch(players, 0, callback, new AtomicBoolean()); @@ -328,7 +357,6 @@ private void continueBatch( public void reBuildCache() { cache.clear(); - untextured.clear(); } @EventHandler diff --git a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/screen/playerui/PlayerUIScreen.java b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/screen/playerui/PlayerUIScreen.java index a0263a8..3be4417 100644 --- a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/screen/playerui/PlayerUIScreen.java +++ b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/screen/playerui/PlayerUIScreen.java @@ -30,7 +30,6 @@ import org.bukkit.event.inventory.InventoryClickEvent; import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.meta.ItemMeta; -import org.bukkit.inventory.meta.SkullMeta; /** * Prompt screen that displays online players as clickable heads in a paginated chest GUI, with @@ -124,24 +123,7 @@ private void openInternal(long token) { ItemStack head = currentHeads.get(j).clone(); int x = (j - i) % 9; int y = (j - i) / 9; - page.addItem( - new GuiItem( - head, - event -> { - var meta = head.getItemMeta(); - if (meta instanceof SkullMeta skullMeta - && skullMeta.getOwningPlayer() != null) { - var name = skullMeta.getOwningPlayer().getName(); - if (name != null) { - close(); - if (callback != null) { - callback.accept(ScreenResult.answer(name)); - } - } - } - }), - x, - y); + page.addItem(new GuiItem(head, event -> selectHead(head)), x, y); } headPane.addPane(page); } @@ -242,6 +224,14 @@ private static void sortHeads(List heads) { String.CASE_INSENSITIVE_ORDER)); } + /** Answers the prompt with the player the clicked head belongs to, if it belongs to one. */ + void selectHead(ItemStack head) { + var name = plugin.getHeadCache().getOwnerName(head); + if (name == null) return; + close(); + if (callback != null) callback.accept(ScreenResult.answer(name)); + } + /** * Applies the display format of the first (leftmost) combined filter to the given heads, * returning cloned items so the shared head cache is never mutated. When the filter has no @@ -254,16 +244,15 @@ private List applyFirstFilterFormat( List heads, CacheFilter firstFilter, PromptConfig config) { var format = firstFilter.getFormat(config); if (format == null || format.isBlank() || "%s".equals(format)) return heads; + var headCache = plugin.getHeadCache(); var formatted = new ArrayList(heads.size()); for (ItemStack head : heads) { var clone = head.clone(); + var name = headCache.getOwnerName(clone); var meta = clone.getItemMeta(); - if (meta instanceof SkullMeta skull - && skull.getOwningPlayer() != null - && skull.getOwningPlayer().getName() != null) { - skull.displayName( - ComponentUtil.mini("" + format.formatted(skull.getOwningPlayer().getName()))); - clone.setItemMeta(skull); + if (name != null && meta != null) { + meta.displayName(ComponentUtil.mini("" + format.formatted(name))); + clone.setItemMeta(meta); } formatted.add(clone); } diff --git a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/screen/playerui/HeadCacheTest.java b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/screen/playerui/HeadCacheTest.java index 23a32c8..cb68db3 100644 --- a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/screen/playerui/HeadCacheTest.java +++ b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/screen/playerui/HeadCacheTest.java @@ -18,11 +18,12 @@ import dev.cyr1en.promptpaper.hook.hooks.FilterHook; import dev.cyr1en.promptpaper.hook.hooks.VanishHook; import java.lang.reflect.Field; +import java.net.MalformedURLException; +import java.net.URI; import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.Set; import java.util.UUID; import java.util.concurrent.atomic.AtomicInteger; import java.util.regex.Pattern; @@ -76,39 +77,13 @@ void sizeReturnsZeroForEmptyCache() { } @Test - void sizeIgnoresEmptyOptionalEntries() { - // Simulate the regression: cache holds only Optional.empty() because - // getHeadFor was called before the player joined. - UUID uuid = UUID.randomUUID(); - injectEmptyEntry(uuid); - - assertEquals( - 0, - headCache.size(), - "size() must ignore Optional.empty() entries so the staleness check fires"); - assertTrue(headCache.getHeads().isEmpty()); - } - - @Test - void sizeCountsNonEmptyEntries() { - var player = createPlayer("CachedOne"); - injectEntry(player.getUniqueId(), Optional.of(new ItemStack(org.bukkit.Material.PLAYER_HEAD))); + void sizeCountsCachedHeads() { + headCache.getHeadFor(createPlayer("CachedOne")); assertEquals(1, headCache.size()); assertEquals(1, headCache.getHeads().size()); } - @Test - void sizeCountsMixedEntries() { - var player = createPlayer("CachedMixed"); - UUID emptyUuid = UUID.randomUUID(); - injectEntry(player.getUniqueId(), Optional.of(new ItemStack(org.bukkit.Material.PLAYER_HEAD))); - injectEmptyEntry(emptyUuid); - - assertEquals(1, headCache.size(), "size() should only count the populated entry"); - assertEquals(1, headCache.getHeads().size()); - } - @Test void onPlayerJoinPopulatesCache() { var player = createPlayer("VisibleOne"); @@ -550,29 +525,58 @@ void getHeadForDoesNotCacheOfflinePlayer() { } @Test - void onPlayerQuitDropsUntexturedMarker() { - var player = offlinePlayer("QuitPlain"); - headCache.onPlayerJoin(new PlayerJoinEvent(player, (Component) null)); - assertTrue(readUntextured().contains(player.getUniqueId())); + void plainHeadOfOfflineUuidKeepsOwnerName() { + var player = offlinePlayer("PlainOwner"); - headCache.onPlayerQuit( - new PlayerQuitEvent(player, (Component) null, PlayerQuitEvent.QuitReason.DISCONNECTED)); + var head = headCache.getHeadFor(player).orElseThrow(); - assertTrue(readUntextured().isEmpty(), "quit must drop every per-player entry"); + assertNull(((SkullMeta) head.getItemMeta()).getOwningPlayer(), "fixture must be a plain head"); + assertEquals( + "PlainOwner", + headCache.getOwnerName(head), + "a plain head must still identify its player for selection"); } @Test - void evictionDropsUntexturedMarker() { - when(promptCfg.cacheSize()).thenReturn(1); - headCache = new HeadCache(plugin, scheduler); - var first = offlinePlayer("EvictedPlain"); - var second = offlinePlayer("KeptPlain"); + void texturesPropertyWithoutSkinOrCapeCountsAsUntextured() { + var player = offlinePlayer("EmptyTextures"); + var profile = player.getPlayerProfile(); + profile.setProperty(new ProfileProperty("textures", "e30=")); + player.setPlayerProfile(profile); + + var head = headCache.getHeadFor(player).orElseThrow(); + + assertNull( + ((SkullMeta) head.getItemMeta()).getPlayerProfile(), + "a profile with no skin and no cape is looked up by the server, so it must not be used"); + } + + @Test + void getHeadForRebuildsHeadWhenSkinChanges() { + var player = offlinePlayer("NewSkin"); + giveTextures(player, "b2xk"); + var oldHead = headCache.getHeadFor(player).orElseThrow(); + + giveTextures(player, "bmV3"); + var newHead = headCache.getHeadFor(player).orElseThrow(); - headCache.getHeadFor(first); - headCache.getHeadFor(second); + assertNotSame(oldHead, newHead, "a skin change mid-session must rebuild the head"); + assertEquals("bmV3", texturesOf(((SkullMeta) newHead.getItemMeta()).getPlayerProfile())); + assertEquals(1, headCache.size()); + } + + @Test + void getHeadForDoesNotCacheFailedBuild() { + var player = offlinePlayer("Broken"); + when(promptCfg.skullNameFormat()).thenThrow(new IllegalStateException("boom")); + + try { + headCache.getHeadFor(player); + } catch (IllegalStateException expected) { + // the build failed; nothing may be left behind + } - assertFalse(readUntextured().contains(first.getUniqueId())); - assertTrue(readUntextured().contains(second.getUniqueId())); + assertFalse(readCacheMap().containsKey(player.getUniqueId())); } /** Online player with an offline-mode (version 3) UUID, as on an offline-mode server. */ @@ -583,29 +587,25 @@ private PlayerMock offlinePlayer(String name) { return player; } + /** Gives the player a skin, as a real decoded {@code textures} property would. */ private static void giveTextures(PlayerMock player, String value) { var profile = player.getPlayerProfile(); - profile.setProperty(new ProfileProperty("textures", value)); + var textures = profile.getTextures(); + try { + textures.setSkin(URI.create("https://textures.minecraft.net/texture/" + value).toURL()); + } catch (MalformedURLException e) { + throw new IllegalStateException(e); + } + profile.setTextures(textures); player.setPlayerProfile(profile); } + /** Returns the skin id given by {@link #giveTextures}, or null if the profile has no skin. */ private static String texturesOf(PlayerProfile profile) { - return profile.getProperties().stream() - .filter(p -> p.getName().equals("textures")) - .map(ProfileProperty::getValue) - .findFirst() - .orElse(null); - } - - @SuppressWarnings("unchecked") - private Set readUntextured() { - try { - Field field = HeadCache.class.getDeclaredField("untextured"); - field.setAccessible(true); - return (Set) field.get(headCache); - } catch (ReflectiveOperationException e) { - throw new RuntimeException("Failed to read untextured set", e); - } + var skin = profile.getTextures().getSkin(); + if (skin == null) return null; + var path = skin.getPath(); + return path.substring(path.lastIndexOf('/') + 1); } /** @@ -632,31 +632,14 @@ private static Style findStyleWithColor(Component root) { return null; } - @SuppressWarnings({"unchecked", "PMD.AvoidAccessibilityAlteration"}) - private void injectEmptyEntry(UUID uuid) { - injectEntry(uuid, Optional.empty()); - } - @SuppressWarnings("unchecked") - private Map> readCacheMap() { + private Map readCacheMap() { try { Field field = HeadCache.class.getDeclaredField("cache"); field.setAccessible(true); - return (Map>) field.get(headCache); + return (Map) field.get(headCache); } catch (ReflectiveOperationException e) { throw new RuntimeException("Failed to read cache map", e); } } - - @SuppressWarnings({"unchecked", "PMD.AvoidAccessibilityAlteration"}) - private void injectEntry(UUID uuid, Optional value) { - try { - Field field = HeadCache.class.getDeclaredField("cache"); - field.setAccessible(true); - Map> map = (Map>) field.get(headCache); - map.put(uuid, value); - } catch (ReflectiveOperationException e) { - throw new RuntimeException("Failed to inject cache entry", e); - } - } } diff --git a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/screen/playerui/PlayerUIScreenTest.java b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/screen/playerui/PlayerUIScreenTest.java index 7b195a7..486eaa2 100644 --- a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/screen/playerui/PlayerUIScreenTest.java +++ b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/screen/playerui/PlayerUIScreenTest.java @@ -17,15 +17,18 @@ import dev.cyr1en.promptpaper.hook.hooks.VanishHook; import dev.cyr1en.promptpaper.preset.PlayerUiPrompt; import dev.cyr1en.promptpaper.preset.UIButton; +import dev.cyr1en.promptpaper.testutil.TestPlayerMock; import dev.cyr1en.promptui.AnvilInputScreen; import dev.cyr1en.promptui.ScreenProvider; import dev.cyr1en.promptui.ScreenResult; import dev.cyr1en.promptui.gui.GuiItem; import dev.cyr1en.promptui.pane.StaticPane; +import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.UUID; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.regex.Pattern; @@ -40,6 +43,7 @@ import org.bukkit.inventory.meta.SkullMeta; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockbukkit.mockbukkit.entity.PlayerMock; import org.mockito.ArgumentCaptor; class PlayerUIScreenTest extends MockBukkitTest { @@ -476,6 +480,45 @@ void parameterizedFilterCombinesWithBuiltin() throws Exception { assertEquals(1, skullNames.size(), "exactly one head must survive the intersection"); } + /** + * Regression: a plain head (offline-mode UUID, no skin) has no owning player, but clicking it + * must still answer the prompt with that player's name. + */ + @Test + void clickingPlainHeadSelectsItsPlayer() { + var realCache = stubRealCacheWithBuiltins(); + var plain = offlinePlayer("PlainClick"); + var head = realCache.getHeadFor(plain).orElseThrow(); + assertNull(((SkullMeta) head.getItemMeta()).getOwningPlayer(), "fixture must be a plain head"); + + screen.selectHead(head.clone()); + + assertNotNull(resultRef.get(), "clicking a plain head must answer the prompt"); + assertEquals("PlainClick", resultRef.get().answer()); + } + + /** Regression: filter formats must apply to plain heads too. */ + @Test + void filterFormatAppliesToPlainHeads() throws Exception { + stubRealCacheWithBuiltins(); + var plain = offlinePlayer("PlainNearby"); + teleportRelative(plain, 5, 0); + + var colors = skullDisplayColors(buildFilteredScreen("r10s")); + + assertEquals(1, colors.size(), "the plain head must be listed"); + assertEquals(NamedTextColor.RED, colors.get(0), "the radial format must apply to a plain head"); + } + + /** Online player with an offline-mode (version 3) UUID and no skin. */ + private PlayerMock offlinePlayer(String name) { + var uuid = UUID.nameUUIDFromBytes(("OfflinePlayer:" + name).getBytes(StandardCharsets.UTF_8)); + var offline = new TestPlayerMock(server, name, uuid); + server.addPlayer(offline); + offline.teleport(player.getLocation()); + return offline; + } + /** * Regression: for combined keys, the head display format must come from the FIRST (leftmost) * filter only — radial in {@code r10s} — while the remaining filters contribute player sets only. diff --git a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/testutil/TestPlayerMock.java b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/testutil/TestPlayerMock.java index 374ce42..f1e3aff 100644 --- a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/testutil/TestPlayerMock.java +++ b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/testutil/TestPlayerMock.java @@ -1,6 +1,7 @@ package dev.cyr1en.promptpaper.testutil; import io.papermc.paper.threadedregions.scheduler.EntityScheduler; +import java.util.UUID; import org.mockbukkit.mockbukkit.ServerMock; import org.mockbukkit.mockbukkit.entity.PlayerMock; @@ -13,6 +14,10 @@ public TestPlayerMock(ServerMock server, String name) { super(server, name); } + public TestPlayerMock(ServerMock server, String name, UUID uuid) { + super(server, name, uuid); + } + @Override public EntityScheduler getScheduler() { return scheduler;