fix: store the hidden book glint on the books, not in packets

README.md

@@ -14,7 +14,7 @@ These features give other TF-Minecraft plugins common building blocks while also
 - **Player utilities** — shared player commands and resource-pack delivery support everyday server use.
 - **Resource-pack texture guard** — warns the console and staff when the ItemsAdder pack holds a texture over 16384 px, which stops the whole pack loading on most Intel graphics.
 - **Required pack on join** — opt-in (`resource-pack.require-on-join`); sends the pack as required and publishes `tfmccore:required-pack` player metadata (`loading`, `ready`, `failed`) for RPCharacters. Staff retry a failed load with `/tcore packretry <player>`.
-- **Book appearance** — optionally hides the default glint on signed books through ProtocolLib.
+- **Book appearance** — optionally hides the default glint on signed books by storing a glint override on the books (`hide-book-glint`).
 - **Book copies** — written books can be copied with a Book and Quill at a crafting table, even though MMOItems blocks vanilla recipes that use a disable-crafting MMOItem (`allow-book-copies`).
 - **Cross-plugin statistics** — records supported vehicle, character, crafting, skill, and faction events for staff queries.
 

src/main/java/net/tfminecraft/tfmccore/TFMCCore.java

@@ -15,6 +15,7 @@
 import net.tfminecraft.tlibs.database.SqliteProvider;
 import net.tfminecraft.tfmccore.books.BookCopyUnblocker;
 import net.tfminecraft.tfmccore.books.BookGlintHider;
+import net.tfminecraft.tfmccore.books.BookGlintStamper;
 import net.tfminecraft.tfmccore.commands.CoreCommands;
 import net.tfminecraft.tfmccore.commands.CoreTabCompletion;
 import net.tfminecraft.tfmccore.commands.SilentPermissionCommand;
@@ -331,6 +332,7 @@ public void registerListeners() {
         getServer().getPluginManager().registerEvents(new XaeroFairPlayListener(this), this);
         // Covers a plugin reload; on a normal start nobody is online yet
         getServer().getOnlinePlayers().forEach(XaeroFairPlayListener::send);
+        BookGlintStamper.register(this);
         BookGlintHider.register(this);
         bookCopies = BookCopyUnblocker.register(this);
     }

src/main/java/net/tfminecraft/tfmccore/books/BookGlintHider.java

@@ -23,21 +23,18 @@
 import net.tfminecraft.tfmccore.cache.Cache;
 
 /**
- * Hides the enchantment glint signed books have by default. Only the item packets
- * sent to players change; the books stored on the server keep their data.
+ * Hides the enchantment glint on signed books shown on entities (dropped items, item frames,
+ * displays and held or worn books) that {@link BookGlintStamper} has not reached. Only entity
+ * packets change: clients hash the stacks they click in inventories, so an inventory item
+ * changed in packets would never match the server's copy.
  */
 public final class BookGlintHider extends PacketAdapter {
 
     BookGlintHider(JavaPlugin plugin) {
         // HIGHEST: run after plugins that rewrite items so their copy is the one changed
         super(plugin, ListenerPriority.HIGHEST,
-                PacketType.Play.Server.SET_SLOT,
-                PacketType.Play.Server.WINDOW_ITEMS,
-                PacketType.Play.Server.SET_CURSOR_ITEM,
-                PacketType.Play.Server.SET_PLAYER_INVENTORY,
                 PacketType.Play.Server.ENTITY_EQUIPMENT,
-                PacketType.Play.Server.ENTITY_METADATA,
-                PacketType.Play.Client.SET_CREATIVE_SLOT);
+                PacketType.Play.Server.ENTITY_METADATA);
     }
 
     public static void register(JavaPlugin plugin) {
@@ -53,21 +50,7 @@ public void onPacketSending(PacketEvent event) {
         if (!Cache.hideBookGlint) return;
         PacketContainer packet = event.getPacket();
         PacketType type = event.getPacketType();
-        if (type == PacketType.Play.Server.WINDOW_ITEMS) {
-            StructureModifier<List<ItemStack>> lists = packet.getItemListModifier();
-            List<ItemStack> items = new ArrayList<>(lists.read(0));
-            boolean changed = false;
-            for (int i = 0; i < items.size(); i++) {
-                ItemStack hidden = hide(items.get(i));
-                if (hidden != null) {
-                    items.set(i, hidden);
-                    changed = true;
-                }
-            }
-            if (changed) lists.write(0, items);
-            // The item held on the cursor
-            hideItem(packet);
-        } else if (type == PacketType.Play.Server.ENTITY_EQUIPMENT) {
+        if (type == PacketType.Play.Server.ENTITY_EQUIPMENT) {
             StructureModifier<List<Pair<EnumWrappers.ItemSlot, ItemStack>>> slots = packet.getSlotStackPairLists();
             List<Pair<EnumWrappers.ItemSlot, ItemStack>> pairs = slots.read(0);
             boolean changed = false;
@@ -79,8 +62,8 @@ public void onPacketSending(PacketEvent event) {
                 }
             }
             if (changed) slots.write(0, pairs);
-        } else if (type == PacketType.Play.Server.ENTITY_METADATA) {
-            // Dropped items, item frames and item displays
+        } else {
+            // ENTITY_METADATA: dropped items, item frames and item displays
             StructureModifier<List<WrappedDataValue>> data = packet.getDataValueCollectionModifier();
             List<WrappedDataValue> values = new ArrayList<>(data.read(0));
             boolean changed = false;
@@ -95,26 +78,9 @@ public void onPacketSending(PacketEvent event) {
                 }
             }
             if (changed) data.write(0, values);
-        } else {
-            hideItem(packet);
         }
     }
 
-    @Override
-    public void onPacketReceiving(PacketEvent event) {
-        // Creative clients send back the stack they were shown. Drop the hidden flag so it
-        // is never saved, even after the option is turned off.
-        StructureModifier<ItemStack> items = event.getPacket().getItemModifier();
-        ItemStack restored = restore(items.read(0));
-        if (restored != null) items.write(0, restored);
-    }
-
-    private static void hideItem(PacketContainer packet) {
-        StructureModifier<ItemStack> items = packet.getItemModifier();
-        ItemStack hidden = hide(items.read(0));
-        if (hidden != null) items.write(0, hidden);
-    }
-
     /** A copy of a signed book with its glint turned off, or null when nothing changes. */
     static ItemStack hide(ItemStack item) {
         if (item == null || item.getType() != Material.WRITTEN_BOOK) return null;
@@ -128,15 +94,4 @@ static ItemStack hide(ItemStack item) {
         copy.setItemMeta(meta);
         return copy;
     }
-
-    /** A copy of a signed book without the hidden-glint flag, or null when nothing changes. */
-    static ItemStack restore(ItemStack item) {
-        if (item == null || item.getType() != Material.WRITTEN_BOOK) return null;
-        ItemMeta meta = item.getItemMeta();
-        if (meta == null || !meta.hasEnchantmentGlintOverride() || meta.getEnchantmentGlintOverride()) return null;
-        meta.setEnchantmentGlintOverride(null);
-        ItemStack copy = item.clone();
-        copy.setItemMeta(meta);
-        return copy;
-    }
 }

src/main/java/net/tfminecraft/tfmccore/books/BookGlintStamper.java

@@ -0,0 +1,133 @@
+package net.tfminecraft.tfmccore.books;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import org.bukkit.Bukkit;
+import org.bukkit.Material;
+import org.bukkit.NamespacedKey;
+import org.bukkit.block.DoubleChest;
+import org.bukkit.block.Lectern;
+import org.bukkit.entity.Entity;
+import org.bukkit.entity.HumanEntity;
+import org.bukkit.entity.Player;
+import org.bukkit.event.EventHandler;
+import org.bukkit.event.EventPriority;
+import org.bukkit.event.Listener;
+import org.bukkit.event.inventory.InventoryOpenEvent;
+import org.bukkit.event.player.PlayerEditBookEvent;
+import org.bukkit.event.player.PlayerInteractEvent;
+import org.bukkit.event.player.PlayerQuitEvent;
+import org.bukkit.inventory.BlockInventoryHolder;
+import org.bukkit.inventory.Inventory;
+import org.bukkit.inventory.InventoryHolder;
+import org.bukkit.inventory.ItemStack;
+import org.bukkit.inventory.meta.ItemMeta;
+import org.bukkit.persistence.PersistentDataContainer;
+import org.bukkit.persistence.PersistentDataType;
+import org.bukkit.plugin.java.JavaPlugin;
+
+import net.tfminecraft.tfmccore.cache.Cache;
+
+/**
+ * Hides the enchantment glint signed books have by default by storing the override on the
+ * books themselves. Clients report a hash of every stack they click; a glint changed only in
+ * packets never matches the server's book, and the resync that follows can arrive after the
+ * next click and leave a ghost copy on the cursor. Books this sets are tagged, so turning the
+ * option off puts the glint back on only those.
+ */
+public final class BookGlintStamper implements Listener {
+
+    static final long SWEEP_TICKS = 20;
+    /** Book handlers compare the book with their own copy a tick later; leave it alone meanwhile. */
+    static final int QUIET_TICKS = 5;
+
+    static final NamespacedKey KEY = new NamespacedKey("tfmccore", "hidden_glint");
+
+    private final Map<UUID, Integer> quietUntil = new HashMap<>();
+
+    public static BookGlintStamper register(JavaPlugin plugin) {
+        BookGlintStamper stamper = new BookGlintStamper();
+        plugin.getServer().getPluginManager().registerEvents(stamper, plugin);
+        plugin.getServer().getScheduler().runTaskTimer(plugin, stamper::sweep, SWEEP_TICKS, SWEEP_TICKS);
+        return stamper;
+    }
+
+    /** Brings online players' own inventories in line with the option. */
+    void sweep() {
+        int now = Bukkit.getCurrentTick();
+        quietUntil.values().removeIf(until -> until <= now);
+        for (Player player : Bukkit.getOnlinePlayers()) {
+            if (!quietUntil.containsKey(player.getUniqueId())) update(player.getInventory());
+        }
+    }
+
+    @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true)
+    public void onOpen(InventoryOpenEvent event) {
+        Inventory top = event.getView().getTopInventory();
+        if (isStorage(top)) update(top);
+    }
+
+    @EventHandler(priority = EventPriority.MONITOR)
+    public void onEdit(PlayerEditBookEvent event) {
+        quiet(event.getPlayer());
+    }
+
+    @EventHandler(priority = EventPriority.MONITOR)
+    public void onInteract(PlayerInteractEvent event) {
+        ItemStack item = event.getItem();
+        if (item != null && item.getType() == Material.WRITTEN_BOOK) quiet(event.getPlayer());
+    }
+
+    @EventHandler
+    public void onQuit(PlayerQuitEvent event) {
+        quietUntil.remove(event.getPlayer().getUniqueId());
+    }
+
+    private void quiet(Player player) {
+        quietUntil.put(player.getUniqueId(), Bukkit.getCurrentTick() + QUIET_TICKS);
+    }
+
+    /** Containers in the world. Plugin menus may match their own icons exactly, so skip them. */
+    static boolean isStorage(Inventory inventory) {
+        InventoryHolder holder = inventory.getHolder(false);
+        // A lectern resets to the first page when its book is set
+        if (holder instanceof Lectern) return false;
+        // Menus are often opened with the player as holder; of a player's own, only the ender chest
+        if (holder instanceof HumanEntity human) return inventory.equals(human.getEnderChest());
+        return holder instanceof BlockInventoryHolder || holder instanceof DoubleChest || holder instanceof Entity;
+    }
+
+    void update(Inventory inventory) {
+        ItemStack[] contents = inventory.getContents();
+        for (int slot = 0; slot < contents.length; slot++) {
+            ItemStack updated = updated(contents[slot]);
+            if (updated != null) inventory.setItem(slot, updated);
+        }
+    }
+
+    /** A copy of a signed book matching the option, or null when nothing changes. */
+    ItemStack updated(ItemStack item) {
+        if (item == null || item.getType() != Material.WRITTEN_BOOK) return null;
+        ItemMeta meta = item.getItemMeta();
+        if (meta == null) return null;
+        PersistentDataContainer data = meta.getPersistentDataContainer();
+        if (data.has(KEY, PersistentDataType.BYTE)) {
+            if (Cache.hideBookGlint) return null;
+            data.remove(KEY);
+            if (meta.hasEnchantmentGlintOverride() && !meta.getEnchantmentGlintOverride()) {
+                meta.setEnchantmentGlintOverride(null);
+            }
+        } else {
+            // Glint true is the written book default and is never stored, so only a book
+            // that already turned it off has an override
+            if (!Cache.hideBookGlint || meta.hasEnchantmentGlintOverride()) return null;
+            meta.setEnchantmentGlintOverride(false);
+            data.set(KEY, PersistentDataType.BYTE, (byte) 1);
+        }
+        ItemStack copy = item.clone();
+        copy.setItemMeta(meta);
+        return copy;
+    }
+}

src/main/resources/config.yml

@@ -24,8 +24,10 @@ horse-archery: false
 prevent-golem-scrape: true
 # Send Xaero's Minimap fair-play code (no entity radar or cave mode) on every join.
 xaero-fair-play: true
-# Hide the enchantment glint on signed books (letters and book skins too). Only changes
-# what players see; needs ProtocolLib. After a reload, items update as they are resent.
+# Hide the enchantment glint on signed books (letters and book skins too). Books are
+# updated within a second in player inventories and when their container is opened.
+# Updated books are tagged, so false brings back the glint on those the same way.
+# ProtocolLib hides it on books not yet updated in item frames or on the ground.
 hide-book-glint: true
 # Let written books be copied with a Book and Quill at a crafting table. MMOItems
 # otherwise blocks the vanilla recipe, because every Book and Quill is a disable-crafting MMOItem.

src/test/java/net/tfminecraft/tfmccore/PluginLifecycleCoverageTest.java

@@ -55,6 +55,7 @@
 import net.tfminecraft.tfmccore.stats.categories.vehicles.VehiclesStatConfig;
 import net.tfminecraft.tfmccore.stones.LorestoneConfigLoader;
 import net.tfminecraft.tfmccore.books.BookCopyUnblocker;
+import net.tfminecraft.tfmccore.books.BookGlintStamper;
 import net.tfminecraft.tfmccore.stones.StoneItems;
 import net.tfminecraft.tfmccore.stones.StoneListener;
 import net.tfminecraft.tfmccore.testsupport.TestRegistryAccess;
@@ -261,8 +262,8 @@
           .when(Bukkit::getUnsafe)
           .thenReturn(mock(org.bukkit.UnsafeValues.class, RETURNS_DEEP_STUBS));
       services
           .when(() -> net.kyori.adventure.util.Services.service(PluginLoader.class))
           .thenReturn(Optional.of(mock(PluginLoader.class)));
       TestPluginLoader loader = new TestPluginLoader(description, server, logger, directory);
 
       JavaPlugin plugin =
@@ -294,8 +295,9 @@
       verify(rig.coreCommand).setTabCompleter(rig.tabCompletion);
       verify(rig.silentCommand).setExecutor(any(SilentPermissionCommand.class));
       verify(rig.silentCommand).setTabCompleter(any(SilentPermissionCommand.class));
-      assertEquals(10, rig.listeners.size());
+      assertEquals(11, rig.listeners.size());
       assertTrue(rig.listeners.stream().anyMatch(BookCopyUnblocker.class::isInstance));
+      assertTrue(rig.listeners.stream().anyMatch(BookGlintStamper.class::isInstance));
       assertTrue(rig.listeners.contains(rig.whistles.constructed().getFirst()));
       assertTrue(rig.listeners.contains(rig.stones.constructed().getFirst()));
       assertSame(rig.stones.constructed().getFirst(), TFMCCore.getStoneListener());