From 165972df8a1ae74db8a146c813df74939320af5a Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Mon, 19 Oct 2026 20:01:36 +0200 Subject: [PATCH 1/4] fix: let written books be copied despite MMOItems Every Book and Quill on the server is the MMOItems BOOKS.WRITABLE_BOOK (MIReplacer), which has disable-crafting. MMOItems cancels any vanilla recipe using such an item, so the book copy showed in the result slot but could not be taken. BookCopyListener notes whether a book_cloning craft is cancelled at LOW, and at HIGH undoes the cancel only if it appeared in between (MMOItems runs at NORMAL) and an ingredient has MMOItems' disable-crafting tag. Cancels from earlier handlers and from HIGHEST handlers still apply. allow-book-copies in config.yml (default true) turns it off. Co-Authored-By: Claude Opus 5.5 --- README.md | 1 + .../net/tfminecraft/tfmccore/TFMCCore.java | 2 + .../tfmccore/books/BookCopyListener.java | 68 ++++++ .../net/tfminecraft/tfmccore/cache/Cache.java | 1 + .../tfmccore/loader/ConfigLoader.java | 1 + src/main/resources/config.yml | 3 + .../tfmccore/books/BookCopyListenerTest.java | 201 ++++++++++++++++++ .../loader/ConfigLoaderCoverageTest.java | 6 +- 8 files changed, 282 insertions(+), 1 deletion(-) create mode 100644 src/main/java/net/tfminecraft/tfmccore/books/BookCopyListener.java create mode 100644 src/test/java/net/tfminecraft/tfmccore/books/BookCopyListenerTest.java diff --git a/README.md b/README.md index 34b4568..f27a0c3 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,7 @@ These features give other TF-Minecraft plugins common building blocks while also - **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 `. - **Book appearance** — optionally hides the default glint on signed books through ProtocolLib. +- **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. TFMC Core works alongside the server's specialist plugins, connecting their systems with the smaller details that make the roleplay world feel consistent. diff --git a/src/main/java/net/tfminecraft/tfmccore/TFMCCore.java b/src/main/java/net/tfminecraft/tfmccore/TFMCCore.java index 01ce941..c75079a 100644 --- a/src/main/java/net/tfminecraft/tfmccore/TFMCCore.java +++ b/src/main/java/net/tfminecraft/tfmccore/TFMCCore.java @@ -13,6 +13,7 @@ import io.papermc.paper.plugin.lifecycle.event.types.LifecycleEvents; import net.tfminecraft.tlibs.database.SqliteProvider; +import net.tfminecraft.tfmccore.books.BookCopyListener; import net.tfminecraft.tfmccore.books.BookGlintHider; import net.tfminecraft.tfmccore.commands.CoreCommands; import net.tfminecraft.tfmccore.commands.CoreTabCompletion; @@ -329,6 +330,7 @@ public void registerListeners() { // Covers a plugin reload; on a normal start nobody is online yet getServer().getOnlinePlayers().forEach(XaeroFairPlayListener::send); BookGlintHider.register(this); + BookCopyListener.register(this); } public void createConfigs() { diff --git a/src/main/java/net/tfminecraft/tfmccore/books/BookCopyListener.java b/src/main/java/net/tfminecraft/tfmccore/books/BookCopyListener.java new file mode 100644 index 0000000..fbd1ce4 --- /dev/null +++ b/src/main/java/net/tfminecraft/tfmccore/books/BookCopyListener.java @@ -0,0 +1,68 @@ +package net.tfminecraft.tfmccore.books; + +import java.util.function.Predicate; + +import org.bukkit.Keyed; +import org.bukkit.NamespacedKey; +import org.bukkit.event.EventHandler; +import org.bukkit.event.EventPriority; +import org.bukkit.event.Listener; +import org.bukkit.event.inventory.CraftItemEvent; +import org.bukkit.inventory.ItemStack; +import org.bukkit.plugin.java.JavaPlugin; + +import io.lumine.mythic.lib.api.item.NBTItem; +import net.tfminecraft.tfmccore.cache.Cache; + +/** + * Lets players copy written books at a crafting table. MIReplacer makes every Book and + * Quill an MMOItem with disable-crafting, and MMOItems cancels any vanilla recipe that + * uses one, so the copy showed but could not be taken. Only that cancel is undone: the + * craft must be uncancelled before MMOItems' NORMAL handler, cancelled after it, and use + * an item with MMOItems' disable-crafting tag. HIGHEST handlers can still cancel it. + */ +public final class BookCopyListener implements Listener { + + static final NamespacedKey BOOK_CLONING = NamespacedKey.minecraft("book_cloning"); + static final String DISABLE_CRAFTING = "MMOITEMS_DISABLE_CRAFTING"; + + private final Predicate craftingDisabled; + private CraftItemEvent checked; + private boolean cancelledBefore; + + BookCopyListener(Predicate craftingDisabled) { + this.craftingDisabled = craftingDisabled; + } + + public static void register(JavaPlugin plugin) { + // MMOItems needs MythicLib, so without it nothing cancels book copies + if (!plugin.getServer().getPluginManager().isPluginEnabled("MythicLib")) return; + plugin.getServer().getPluginManager().registerEvents( + new BookCopyListener(BookCopyListener::mmoItemsDisablesCrafting), plugin); + } + + @EventHandler(priority = EventPriority.LOW) + public void beforeMmoItems(CraftItemEvent event) { + if (!Cache.allowBookCopies) return; + if (!(event.getRecipe() instanceof Keyed keyed) || !BOOK_CLONING.equals(keyed.getKey())) return; + checked = event; + cancelledBefore = event.isCancelled(); + } + + @EventHandler(priority = EventPriority.HIGH) + public void afterMmoItems(CraftItemEvent event) { + if (checked != event) return; + checked = null; + if (cancelledBefore || !event.isCancelled()) return; + for (ItemStack item : event.getInventory().getMatrix()) { + if (item != null && !item.isEmpty() && craftingDisabled.test(item)) { + event.setCancelled(false); + return; + } + } + } + + static boolean mmoItemsDisablesCrafting(ItemStack item) { + return NBTItem.get(item).getBoolean(DISABLE_CRAFTING); + } +} diff --git a/src/main/java/net/tfminecraft/tfmccore/cache/Cache.java b/src/main/java/net/tfminecraft/tfmccore/cache/Cache.java index b9adf04..6955e63 100644 --- a/src/main/java/net/tfminecraft/tfmccore/cache/Cache.java +++ b/src/main/java/net/tfminecraft/tfmccore/cache/Cache.java @@ -18,6 +18,7 @@ public class Cache { public static boolean xaeroFairPlay = true; // Read from packet threads public static volatile boolean hideBookGlint = true; + public static boolean allowBookCopies = true; public static int armourTime = 7; diff --git a/src/main/java/net/tfminecraft/tfmccore/loader/ConfigLoader.java b/src/main/java/net/tfminecraft/tfmccore/loader/ConfigLoader.java index 072fb36..ee5134f 100644 --- a/src/main/java/net/tfminecraft/tfmccore/loader/ConfigLoader.java +++ b/src/main/java/net/tfminecraft/tfmccore/loader/ConfigLoader.java @@ -36,6 +36,7 @@ public boolean loadConfig(File configFile) { Cache.dropsDebug = config.getBoolean("drops-debug", true); Cache.xaeroFairPlay = config.getBoolean("xaero-fair-play", true); Cache.hideBookGlint = config.getBoolean("hide-book-glint", true); + Cache.allowBookCopies = config.getBoolean("allow-book-copies", true); Cache.armourTime = config.getInt("armour-time", 7); diff --git a/src/main/resources/config.yml b/src/main/resources/config.yml index 45c6f7b..19dcc6f 100644 --- a/src/main/resources/config.yml +++ b/src/main/resources/config.yml @@ -27,6 +27,9 @@ 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-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. +allow-book-copies: true drops-debug: true diff --git a/src/test/java/net/tfminecraft/tfmccore/books/BookCopyListenerTest.java b/src/test/java/net/tfminecraft/tfmccore/books/BookCopyListenerTest.java new file mode 100644 index 0000000..3c13043 --- /dev/null +++ b/src/test/java/net/tfminecraft/tfmccore/books/BookCopyListenerTest.java @@ -0,0 +1,201 @@ +package net.tfminecraft.tfmccore.books; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +import io.lumine.mythic.lib.api.item.NBTItem; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Predicate; +import net.tfminecraft.tfmccore.cache.Cache; +import org.bukkit.Material; +import org.bukkit.NamespacedKey; +import org.bukkit.Server; +import org.bukkit.event.inventory.ClickType; +import org.bukkit.event.inventory.CraftItemEvent; +import org.bukkit.event.inventory.InventoryAction; +import org.bukkit.event.inventory.InventoryType; +import org.bukkit.inventory.ComplexRecipe; +import org.bukkit.inventory.CraftingInventory; +import org.bukkit.inventory.InventoryView; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.Recipe; +import org.bukkit.plugin.PluginManager; +import org.bukkit.plugin.java.JavaPlugin; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class BookCopyListenerTest { + + private final List tested = new ArrayList<>(); + private final Predicate disabled = + item -> { + tested.add(item); + return item.getType() == Material.WRITABLE_BOOK; + }; + private final BookCopyListener listener = new BookCopyListener(disabled); + private boolean previous; + + @BeforeEach + void enable() { + previous = Cache.allowBookCopies; + Cache.allowBookCopies = true; + } + + @AfterEach + void restore() { + Cache.allowBookCopies = previous; + } + + @Test + void undoesOnlyTheCancelMadeBetweenItsTwoHandlers() { + CraftItemEvent event = copy(item(Material.WRITTEN_BOOK), item(Material.WRITABLE_BOOK)); + + listener.beforeMmoItems(event); + event.setCancelled(true); + listener.afterMmoItems(event); + + assertFalse(event.isCancelled()); + } + + @Test + void keepsCancelsMadeBeforeMmoItems() { + CraftItemEvent event = copy(item(Material.WRITABLE_BOOK)); + event.setCancelled(true); + + listener.beforeMmoItems(event); + listener.afterMmoItems(event); + + assertTrue(event.isCancelled()); + assertTrue(tested.isEmpty()); + } + + @Test + void leavesAllowedCraftsAlone() { + CraftItemEvent event = copy(item(Material.WRITABLE_BOOK)); + + listener.beforeMmoItems(event); + listener.afterMmoItems(event); + + assertFalse(event.isCancelled()); + assertTrue(tested.isEmpty()); + } + + @Test + void keepsCancelsWithoutAnMmoItemsDisabledIngredient() { + ItemStack plain = item(Material.WRITTEN_BOOK); + CraftItemEvent event = copy(null, item(Material.AIR), plain); + + listener.beforeMmoItems(event); + event.setCancelled(true); + listener.afterMmoItems(event); + + assertTrue(event.isCancelled()); + assertEquals(List.of(plain), tested); + } + + @Test + void ignoresOtherRecipes() { + ComplexRecipe other = mock(ComplexRecipe.class); + when(other.getKey()).thenReturn(NamespacedKey.minecraft("map_cloning")); + for (Recipe recipe : List.of(other, mock(Recipe.class))) { + CraftItemEvent event = craft(recipe, item(Material.WRITABLE_BOOK)); + + listener.beforeMmoItems(event); + event.setCancelled(true); + listener.afterMmoItems(event); + + assertTrue(event.isCancelled()); + } + assertTrue(tested.isEmpty()); + } + + @Test + void canBeTurnedOff() { + Cache.allowBookCopies = false; + CraftItemEvent event = copy(item(Material.WRITABLE_BOOK)); + + listener.beforeMmoItems(event); + event.setCancelled(true); + listener.afterMmoItems(event); + + assertTrue(event.isCancelled()); + } + + @Test + void onlyChecksTheEventItSawAndOnlyOnce() { + CraftItemEvent first = copy(item(Material.WRITABLE_BOOK)); + CraftItemEvent second = copy(item(Material.WRITABLE_BOOK)); + + listener.beforeMmoItems(first); + second.setCancelled(true); + listener.afterMmoItems(second); + assertTrue(second.isCancelled()); + + first.setCancelled(true); + listener.afterMmoItems(first); + assertFalse(first.isCancelled()); + + first.setCancelled(true); + listener.afterMmoItems(first); + assertTrue(first.isCancelled()); + } + + @Test + void readsMmoItemsDisableCraftingTag() { + ItemStack item = item(Material.WRITABLE_BOOK); + NBTItem tags = mock(NBTItem.class); + try (var nbt = mockStatic(NBTItem.class)) { + nbt.when(() -> NBTItem.get(item)).thenReturn(tags); + + assertFalse(BookCopyListener.mmoItemsDisablesCrafting(item)); + when(tags.getBoolean("MMOITEMS_DISABLE_CRAFTING")).thenReturn(true); + assertTrue(BookCopyListener.mmoItemsDisablesCrafting(item)); + } + } + + @Test + void registersOnlyWithMythicLib() { + JavaPlugin plugin = mock(JavaPlugin.class); + Server server = mock(Server.class); + PluginManager plugins = mock(PluginManager.class); + when(plugin.getServer()).thenReturn(server); + when(server.getPluginManager()).thenReturn(plugins); + + BookCopyListener.register(plugin); + verify(plugins, never()).registerEvents(any(), any()); + + when(plugins.isPluginEnabled("MythicLib")).thenReturn(true); + BookCopyListener.register(plugin); + verify(plugins).registerEvents(any(BookCopyListener.class), eq(plugin)); + } + + private static ItemStack item(Material type) { + ItemStack item = mock(ItemStack.class); + when(item.getType()).thenReturn(type); + when(item.isEmpty()).thenReturn(type == Material.AIR); + return item; + } + + private static CraftItemEvent copy(ItemStack... matrix) { + ComplexRecipe recipe = mock(ComplexRecipe.class); + when(recipe.getKey()).thenReturn(NamespacedKey.minecraft("book_cloning")); + return craft(recipe, matrix); + } + + private static CraftItemEvent craft(Recipe recipe, ItemStack... matrix) { + CraftingInventory inventory = mock(CraftingInventory.class); + when(inventory.getMatrix()).thenReturn(matrix); + InventoryView view = mock(InventoryView.class); + when(view.getTopInventory()).thenReturn(inventory); + return new CraftItemEvent( + recipe, + view, + InventoryType.SlotType.RESULT, + 0, + ClickType.LEFT, + InventoryAction.PICKUP_ALL); + } +} diff --git a/src/test/java/net/tfminecraft/tfmccore/loader/ConfigLoaderCoverageTest.java b/src/test/java/net/tfminecraft/tfmccore/loader/ConfigLoaderCoverageTest.java index 5d26eed..c9413c2 100644 --- a/src/test/java/net/tfminecraft/tfmccore/loader/ConfigLoaderCoverageTest.java +++ b/src/test/java/net/tfminecraft/tfmccore/loader/ConfigLoaderCoverageTest.java @@ -106,7 +106,8 @@ protected Class loadClass(String name, boolean resolve) throws ClassNotFoundE "preventGolemScrape", "dropsDebug", "xaeroFairPlay", - "hideBookGlint")) { + "hideBookGlint", + "allowBookCopies")) { assertEquals(true, cache.getField(name).get(null), name); } assertEquals(false, cache.getField("limitShields").get(null)); @@ -132,6 +133,7 @@ void validConfigReplacesValuesAndMalformedReloadRetainsThem() throws Exception { drops-debug: false xaero-fair-play: false hide-book-glint: false + allow-book-copies: false armour-time: 12 blocked-consume: [GOLDEN_APPLE] blocked-crafts: [DIAMOND_SWORD] @@ -148,6 +150,7 @@ void validConfigReplacesValuesAndMalformedReloadRetainsThem() throws Exception { assertFalse(Cache.dropsDebug); assertFalse(Cache.xaeroFairPlay); assertFalse(Cache.hideBookGlint); + assertFalse(Cache.allowBookCopies); assertEquals(12, Cache.armourTime); assertEquals(List.of(Material.GOLDEN_APPLE), Cache.blockedConsume); assertEquals(List.of(Material.DIAMOND_SWORD), Cache.blockedCrafts); @@ -161,6 +164,7 @@ void validConfigReplacesValuesAndMalformedReloadRetainsThem() throws Exception { assertTrue(Cache.allowBoneMeal); assertTrue(Cache.compactResourcePackOverlays); assertTrue(Cache.hideBookGlint); + assertTrue(Cache.allowBookCopies); assertEquals(7, Cache.armourTime); assertTrue(Cache.blockedCrafts.isEmpty()); assertTrue(Cache.blockedConsume.isEmpty()); From 28a97e6d7a9fb4465f7e35f39bc81ce6e74d5354 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Mon, 19 Oct 2026 20:36:07 +0200 Subject: [PATCH 2/4] fix: only undo MMOItems' own book copy cancel Review: inferring the cause from timing and a tagged ingredient would also undo another plugin's NORMAL cancel. BookCopyUnblocker now wraps MMOItems' DisableInteractions registrations in the InventoryClickEvent handler list (one tick after startup, since MMOItems is not a dependency). The wrapper undoes a book copy cancel only when that MMOItems call made it, so cancels from every other plugin, at any priority, stay. No MythicLib NBT reading. Co-Authored-By: Claude Opus 5.5 --- .../net/tfminecraft/tfmccore/TFMCCore.java | 4 +- .../tfmccore/books/BookCopyListener.java | 68 ----- .../tfmccore/books/BookCopyUnblocker.java | 73 ++++++ .../listener/DisableInteractions.java | 6 + .../tfmccore/books/BookCopyListenerTest.java | 201 --------------- .../tfmccore/books/BookCopyUnblockerTest.java | 233 ++++++++++++++++++ 6 files changed, 314 insertions(+), 271 deletions(-) delete mode 100644 src/main/java/net/tfminecraft/tfmccore/books/BookCopyListener.java create mode 100644 src/main/java/net/tfminecraft/tfmccore/books/BookCopyUnblocker.java create mode 100644 src/test/java/net/Indyuce/mmoitems/listener/DisableInteractions.java delete mode 100644 src/test/java/net/tfminecraft/tfmccore/books/BookCopyListenerTest.java create mode 100644 src/test/java/net/tfminecraft/tfmccore/books/BookCopyUnblockerTest.java diff --git a/src/main/java/net/tfminecraft/tfmccore/TFMCCore.java b/src/main/java/net/tfminecraft/tfmccore/TFMCCore.java index c75079a..49918b5 100644 --- a/src/main/java/net/tfminecraft/tfmccore/TFMCCore.java +++ b/src/main/java/net/tfminecraft/tfmccore/TFMCCore.java @@ -13,7 +13,7 @@ import io.papermc.paper.plugin.lifecycle.event.types.LifecycleEvents; import net.tfminecraft.tlibs.database.SqliteProvider; -import net.tfminecraft.tfmccore.books.BookCopyListener; +import net.tfminecraft.tfmccore.books.BookCopyUnblocker; import net.tfminecraft.tfmccore.books.BookGlintHider; import net.tfminecraft.tfmccore.commands.CoreCommands; import net.tfminecraft.tfmccore.commands.CoreTabCompletion; @@ -330,7 +330,7 @@ public void registerListeners() { // Covers a plugin reload; on a normal start nobody is online yet getServer().getOnlinePlayers().forEach(XaeroFairPlayListener::send); BookGlintHider.register(this); - BookCopyListener.register(this); + BookCopyUnblocker.register(this); } public void createConfigs() { diff --git a/src/main/java/net/tfminecraft/tfmccore/books/BookCopyListener.java b/src/main/java/net/tfminecraft/tfmccore/books/BookCopyListener.java deleted file mode 100644 index fbd1ce4..0000000 --- a/src/main/java/net/tfminecraft/tfmccore/books/BookCopyListener.java +++ /dev/null @@ -1,68 +0,0 @@ -package net.tfminecraft.tfmccore.books; - -import java.util.function.Predicate; - -import org.bukkit.Keyed; -import org.bukkit.NamespacedKey; -import org.bukkit.event.EventHandler; -import org.bukkit.event.EventPriority; -import org.bukkit.event.Listener; -import org.bukkit.event.inventory.CraftItemEvent; -import org.bukkit.inventory.ItemStack; -import org.bukkit.plugin.java.JavaPlugin; - -import io.lumine.mythic.lib.api.item.NBTItem; -import net.tfminecraft.tfmccore.cache.Cache; - -/** - * Lets players copy written books at a crafting table. MIReplacer makes every Book and - * Quill an MMOItem with disable-crafting, and MMOItems cancels any vanilla recipe that - * uses one, so the copy showed but could not be taken. Only that cancel is undone: the - * craft must be uncancelled before MMOItems' NORMAL handler, cancelled after it, and use - * an item with MMOItems' disable-crafting tag. HIGHEST handlers can still cancel it. - */ -public final class BookCopyListener implements Listener { - - static final NamespacedKey BOOK_CLONING = NamespacedKey.minecraft("book_cloning"); - static final String DISABLE_CRAFTING = "MMOITEMS_DISABLE_CRAFTING"; - - private final Predicate craftingDisabled; - private CraftItemEvent checked; - private boolean cancelledBefore; - - BookCopyListener(Predicate craftingDisabled) { - this.craftingDisabled = craftingDisabled; - } - - public static void register(JavaPlugin plugin) { - // MMOItems needs MythicLib, so without it nothing cancels book copies - if (!plugin.getServer().getPluginManager().isPluginEnabled("MythicLib")) return; - plugin.getServer().getPluginManager().registerEvents( - new BookCopyListener(BookCopyListener::mmoItemsDisablesCrafting), plugin); - } - - @EventHandler(priority = EventPriority.LOW) - public void beforeMmoItems(CraftItemEvent event) { - if (!Cache.allowBookCopies) return; - if (!(event.getRecipe() instanceof Keyed keyed) || !BOOK_CLONING.equals(keyed.getKey())) return; - checked = event; - cancelledBefore = event.isCancelled(); - } - - @EventHandler(priority = EventPriority.HIGH) - public void afterMmoItems(CraftItemEvent event) { - if (checked != event) return; - checked = null; - if (cancelledBefore || !event.isCancelled()) return; - for (ItemStack item : event.getInventory().getMatrix()) { - if (item != null && !item.isEmpty() && craftingDisabled.test(item)) { - event.setCancelled(false); - return; - } - } - } - - static boolean mmoItemsDisablesCrafting(ItemStack item) { - return NBTItem.get(item).getBoolean(DISABLE_CRAFTING); - } -} diff --git a/src/main/java/net/tfminecraft/tfmccore/books/BookCopyUnblocker.java b/src/main/java/net/tfminecraft/tfmccore/books/BookCopyUnblocker.java new file mode 100644 index 0000000..0e82b19 --- /dev/null +++ b/src/main/java/net/tfminecraft/tfmccore/books/BookCopyUnblocker.java @@ -0,0 +1,73 @@ +package net.tfminecraft.tfmccore.books; + +import org.bukkit.Keyed; +import org.bukkit.NamespacedKey; +import org.bukkit.event.Cancellable; +import org.bukkit.event.Event; +import org.bukkit.event.HandlerList; +import org.bukkit.event.inventory.CraftItemEvent; +import org.bukkit.event.inventory.InventoryClickEvent; +import org.bukkit.plugin.EventExecutor; +import org.bukkit.plugin.RegisteredListener; +import org.bukkit.plugin.java.JavaPlugin; + +import net.tfminecraft.tfmccore.cache.Cache; + +/** + * Lets players copy written books at a crafting table. MIReplacer makes every Book and + * Quill an MMOItem with disable-crafting, and MMOItems cancels any vanilla recipe that + * uses one, so the copy showed but could not be taken. MMOItems' own click handlers are + * wrapped so that only a book copy cancel made by that handler call is undone; cancels + * from every other plugin, at any priority, stay. + */ +public final class BookCopyUnblocker { + + static final NamespacedKey BOOK_CLONING = NamespacedKey.minecraft("book_cloning"); + static final String MMOITEMS_LISTENER = "net.Indyuce.mmoitems.listener.DisableInteractions"; + + private BookCopyUnblocker() { + } + + public static void register(JavaPlugin plugin) { + // MMOItems is not a dependency, so it may enable after TFMCCore: wait until startup ends + plugin.getServer().getScheduler().runTask(plugin, () -> { + if (!plugin.getServer().getPluginManager().isPluginEnabled("MMOItems")) return; + int wrapped = wrap(InventoryClickEvent.getHandlerList()); + if (wrapped == 0) { + plugin.getLogger().warning("MMOItems craft handler not found; book copies stay blocked"); + } else { + plugin.getLogger().info("Book copies: wrapped " + wrapped + " MMOItems click handlers"); + } + }); + } + + /** Replaces each MMOItems DisableInteractions registration with one that undoes its book copy cancels. */ + static int wrap(HandlerList handlers) { + int wrapped = 0; + for (RegisteredListener original : handlers.getRegisteredListeners()) { + if (!original.getListener().getClass().getName().equals(MMOITEMS_LISTENER)) continue; + handlers.unregister(original); + handlers.register(new RegisteredListener(original.getListener(), executor(original), + original.getPriority(), original.getPlugin(), original.isIgnoringCancelled())); + wrapped++; + } + return wrapped; + } + + static EventExecutor executor(RegisteredListener original) { + return (listener, event) -> { + boolean copy = Cache.allowBookCopies && isBookCopy(event); + boolean cancelledBefore = copy && ((Cancellable) event).isCancelled(); + original.callEvent(event); + if (copy && !cancelledBefore && ((Cancellable) event).isCancelled()) { + ((Cancellable) event).setCancelled(false); + } + }; + } + + private static boolean isBookCopy(Event event) { + return event instanceof CraftItemEvent craft + && craft.getRecipe() instanceof Keyed keyed + && BOOK_CLONING.equals(keyed.getKey()); + } +} diff --git a/src/test/java/net/Indyuce/mmoitems/listener/DisableInteractions.java b/src/test/java/net/Indyuce/mmoitems/listener/DisableInteractions.java new file mode 100644 index 0000000..48300e5 --- /dev/null +++ b/src/test/java/net/Indyuce/mmoitems/listener/DisableInteractions.java @@ -0,0 +1,6 @@ +package net.Indyuce.mmoitems.listener; + +import org.bukkit.event.Listener; + +/** Stands in for MMOItems' listener, which BookCopyUnblocker finds by class name. */ +public class DisableInteractions implements Listener {} diff --git a/src/test/java/net/tfminecraft/tfmccore/books/BookCopyListenerTest.java b/src/test/java/net/tfminecraft/tfmccore/books/BookCopyListenerTest.java deleted file mode 100644 index 3c13043..0000000 --- a/src/test/java/net/tfminecraft/tfmccore/books/BookCopyListenerTest.java +++ /dev/null @@ -1,201 +0,0 @@ -package net.tfminecraft.tfmccore.books; - -import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.ArgumentMatchers.*; -import static org.mockito.Mockito.*; - -import io.lumine.mythic.lib.api.item.NBTItem; -import java.util.ArrayList; -import java.util.List; -import java.util.function.Predicate; -import net.tfminecraft.tfmccore.cache.Cache; -import org.bukkit.Material; -import org.bukkit.NamespacedKey; -import org.bukkit.Server; -import org.bukkit.event.inventory.ClickType; -import org.bukkit.event.inventory.CraftItemEvent; -import org.bukkit.event.inventory.InventoryAction; -import org.bukkit.event.inventory.InventoryType; -import org.bukkit.inventory.ComplexRecipe; -import org.bukkit.inventory.CraftingInventory; -import org.bukkit.inventory.InventoryView; -import org.bukkit.inventory.ItemStack; -import org.bukkit.inventory.Recipe; -import org.bukkit.plugin.PluginManager; -import org.bukkit.plugin.java.JavaPlugin; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -class BookCopyListenerTest { - - private final List tested = new ArrayList<>(); - private final Predicate disabled = - item -> { - tested.add(item); - return item.getType() == Material.WRITABLE_BOOK; - }; - private final BookCopyListener listener = new BookCopyListener(disabled); - private boolean previous; - - @BeforeEach - void enable() { - previous = Cache.allowBookCopies; - Cache.allowBookCopies = true; - } - - @AfterEach - void restore() { - Cache.allowBookCopies = previous; - } - - @Test - void undoesOnlyTheCancelMadeBetweenItsTwoHandlers() { - CraftItemEvent event = copy(item(Material.WRITTEN_BOOK), item(Material.WRITABLE_BOOK)); - - listener.beforeMmoItems(event); - event.setCancelled(true); - listener.afterMmoItems(event); - - assertFalse(event.isCancelled()); - } - - @Test - void keepsCancelsMadeBeforeMmoItems() { - CraftItemEvent event = copy(item(Material.WRITABLE_BOOK)); - event.setCancelled(true); - - listener.beforeMmoItems(event); - listener.afterMmoItems(event); - - assertTrue(event.isCancelled()); - assertTrue(tested.isEmpty()); - } - - @Test - void leavesAllowedCraftsAlone() { - CraftItemEvent event = copy(item(Material.WRITABLE_BOOK)); - - listener.beforeMmoItems(event); - listener.afterMmoItems(event); - - assertFalse(event.isCancelled()); - assertTrue(tested.isEmpty()); - } - - @Test - void keepsCancelsWithoutAnMmoItemsDisabledIngredient() { - ItemStack plain = item(Material.WRITTEN_BOOK); - CraftItemEvent event = copy(null, item(Material.AIR), plain); - - listener.beforeMmoItems(event); - event.setCancelled(true); - listener.afterMmoItems(event); - - assertTrue(event.isCancelled()); - assertEquals(List.of(plain), tested); - } - - @Test - void ignoresOtherRecipes() { - ComplexRecipe other = mock(ComplexRecipe.class); - when(other.getKey()).thenReturn(NamespacedKey.minecraft("map_cloning")); - for (Recipe recipe : List.of(other, mock(Recipe.class))) { - CraftItemEvent event = craft(recipe, item(Material.WRITABLE_BOOK)); - - listener.beforeMmoItems(event); - event.setCancelled(true); - listener.afterMmoItems(event); - - assertTrue(event.isCancelled()); - } - assertTrue(tested.isEmpty()); - } - - @Test - void canBeTurnedOff() { - Cache.allowBookCopies = false; - CraftItemEvent event = copy(item(Material.WRITABLE_BOOK)); - - listener.beforeMmoItems(event); - event.setCancelled(true); - listener.afterMmoItems(event); - - assertTrue(event.isCancelled()); - } - - @Test - void onlyChecksTheEventItSawAndOnlyOnce() { - CraftItemEvent first = copy(item(Material.WRITABLE_BOOK)); - CraftItemEvent second = copy(item(Material.WRITABLE_BOOK)); - - listener.beforeMmoItems(first); - second.setCancelled(true); - listener.afterMmoItems(second); - assertTrue(second.isCancelled()); - - first.setCancelled(true); - listener.afterMmoItems(first); - assertFalse(first.isCancelled()); - - first.setCancelled(true); - listener.afterMmoItems(first); - assertTrue(first.isCancelled()); - } - - @Test - void readsMmoItemsDisableCraftingTag() { - ItemStack item = item(Material.WRITABLE_BOOK); - NBTItem tags = mock(NBTItem.class); - try (var nbt = mockStatic(NBTItem.class)) { - nbt.when(() -> NBTItem.get(item)).thenReturn(tags); - - assertFalse(BookCopyListener.mmoItemsDisablesCrafting(item)); - when(tags.getBoolean("MMOITEMS_DISABLE_CRAFTING")).thenReturn(true); - assertTrue(BookCopyListener.mmoItemsDisablesCrafting(item)); - } - } - - @Test - void registersOnlyWithMythicLib() { - JavaPlugin plugin = mock(JavaPlugin.class); - Server server = mock(Server.class); - PluginManager plugins = mock(PluginManager.class); - when(plugin.getServer()).thenReturn(server); - when(server.getPluginManager()).thenReturn(plugins); - - BookCopyListener.register(plugin); - verify(plugins, never()).registerEvents(any(), any()); - - when(plugins.isPluginEnabled("MythicLib")).thenReturn(true); - BookCopyListener.register(plugin); - verify(plugins).registerEvents(any(BookCopyListener.class), eq(plugin)); - } - - private static ItemStack item(Material type) { - ItemStack item = mock(ItemStack.class); - when(item.getType()).thenReturn(type); - when(item.isEmpty()).thenReturn(type == Material.AIR); - return item; - } - - private static CraftItemEvent copy(ItemStack... matrix) { - ComplexRecipe recipe = mock(ComplexRecipe.class); - when(recipe.getKey()).thenReturn(NamespacedKey.minecraft("book_cloning")); - return craft(recipe, matrix); - } - - private static CraftItemEvent craft(Recipe recipe, ItemStack... matrix) { - CraftingInventory inventory = mock(CraftingInventory.class); - when(inventory.getMatrix()).thenReturn(matrix); - InventoryView view = mock(InventoryView.class); - when(view.getTopInventory()).thenReturn(inventory); - return new CraftItemEvent( - recipe, - view, - InventoryType.SlotType.RESULT, - 0, - ClickType.LEFT, - InventoryAction.PICKUP_ALL); - } -} diff --git a/src/test/java/net/tfminecraft/tfmccore/books/BookCopyUnblockerTest.java b/src/test/java/net/tfminecraft/tfmccore/books/BookCopyUnblockerTest.java new file mode 100644 index 0000000..8730c3f --- /dev/null +++ b/src/test/java/net/tfminecraft/tfmccore/books/BookCopyUnblockerTest.java @@ -0,0 +1,233 @@ +package net.tfminecraft.tfmccore.books; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +import java.util.logging.Logger; +import net.Indyuce.mmoitems.listener.DisableInteractions; +import net.tfminecraft.tfmccore.cache.Cache; +import org.bukkit.NamespacedKey; +import org.bukkit.Server; +import org.bukkit.event.Cancellable; +import org.bukkit.event.EventPriority; +import org.bukkit.event.HandlerList; +import org.bukkit.event.Listener; +import org.bukkit.event.inventory.ClickType; +import org.bukkit.event.inventory.CraftItemEvent; +import org.bukkit.event.inventory.InventoryAction; +import org.bukkit.event.inventory.InventoryClickEvent; +import org.bukkit.event.inventory.InventoryType; +import org.bukkit.inventory.ComplexRecipe; +import org.bukkit.inventory.CraftingInventory; +import org.bukkit.inventory.InventoryView; +import org.bukkit.inventory.Recipe; +import org.bukkit.plugin.Plugin; +import org.bukkit.plugin.PluginManager; +import org.bukkit.plugin.RegisteredListener; +import org.bukkit.plugin.java.JavaPlugin; +import org.bukkit.scheduler.BukkitScheduler; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +class BookCopyUnblockerTest { + + private final Plugin mmoItems = mock(Plugin.class); + private final Plugin other = mock(Plugin.class); + private final DisableInteractions disable = new DisableInteractions(); + private HandlerList handlers; + private boolean previous; + + @BeforeEach + void setUp() { + previous = Cache.allowBookCopies; + Cache.allowBookCopies = true; + handlers = new HandlerList(); + } + + @AfterEach + void restore() { + Cache.allowBookCopies = previous; + handlers.unregisterAll(mmoItems); + handlers.unregisterAll(other); + InventoryClickEvent.getHandlerList().unregisterAll(mmoItems); + } + + @Test + void undoesTheBookCopyCancelMadeByMmoItems() { + mmoItemsCancels(handlers); + assertEquals(1, BookCopyUnblocker.wrap(handlers)); + + CraftItemEvent event = craft(bookCloning()); + dispatch(event); + + assertFalse(event.isCancelled()); + } + + @Test + void keepsWhatTheWrappedRegistrationLooksLike() { + mmoItemsCancels(handlers); + BookCopyUnblocker.wrap(handlers); + + RegisteredListener wrapped = handlers.getRegisteredListeners()[0]; + assertEquals(1, handlers.getRegisteredListeners().length); + assertSame(disable, wrapped.getListener()); + assertSame(mmoItems, wrapped.getPlugin()); + assertEquals(EventPriority.NORMAL, wrapped.getPriority()); + assertFalse(wrapped.isIgnoringCancelled()); + } + + @Test + void keepsCancelsFromOtherPluginsAtEveryPriority() { + for (EventPriority priority : + new EventPriority[] {EventPriority.LOW, EventPriority.NORMAL, EventPriority.HIGHEST}) { + HandlerList list = new HandlerList(); + // Another NORMAL plugin registered before MMOItems, and one registered after it. + if (priority == EventPriority.NORMAL) otherCancels(list, priority); + mmoItemsCancels(list); + otherCancels(list, priority); + BookCopyUnblocker.wrap(list); + + CraftItemEvent event = craft(bookCloning()); + for (RegisteredListener registered : list.getRegisteredListeners()) call(registered, event); + + assertTrue(event.isCancelled(), priority.name()); + list.unregisterAll(mmoItems); + list.unregisterAll(other); + } + } + + @Test + void keepsAnOtherPluginsCancelThatComesAfterMmoItemsInTheSameSlot() { + mmoItemsCancels(handlers); + BookCopyUnblocker.wrap(handlers); + // Registered after wrapping, so it runs after the wrapped MMOItems handler. + otherCancels(handlers, EventPriority.NORMAL); + + CraftItemEvent event = craft(bookCloning()); + dispatch(event); + + assertTrue(event.isCancelled()); + } + + @Test + void leavesOtherRecipesAndClicksCancelled() { + mmoItemsCancels(handlers); + BookCopyUnblocker.wrap(handlers); + ComplexRecipe map = mock(ComplexRecipe.class); + when(map.getKey()).thenReturn(NamespacedKey.minecraft("map_cloning")); + + for (Recipe recipe : new Recipe[] {map, mock(Recipe.class)}) { + CraftItemEvent event = craft(recipe); + dispatch(event); + assertTrue(event.isCancelled()); + } + InventoryClickEvent click = + new InventoryClickEvent( + view(), InventoryType.SlotType.CONTAINER, 3, ClickType.LEFT, InventoryAction.PICKUP_ALL); + dispatch(click); + assertTrue(click.isCancelled()); + } + + @Test + void leavesAllowedCraftsAndCanBeTurnedOff() { + RegisteredListener allows = + new RegisteredListener(disable, (l, e) -> {}, EventPriority.NORMAL, mmoItems, false); + handlers.register(allows); + BookCopyUnblocker.wrap(handlers); + CraftItemEvent allowed = craft(bookCloning()); + dispatch(allowed); + assertFalse(allowed.isCancelled()); + + handlers.unregisterAll(mmoItems); + mmoItemsCancels(handlers); + BookCopyUnblocker.wrap(handlers); + Cache.allowBookCopies = false; + CraftItemEvent off = craft(bookCloning()); + dispatch(off); + assertTrue(off.isCancelled()); + } + + @Test + void onlyWrapsMmoItemsDisableInteractions() { + handlers.register( + new RegisteredListener(new Listener() {}, (l, e) -> {}, EventPriority.NORMAL, other, false)); + + assertEquals(0, BookCopyUnblocker.wrap(handlers)); + } + + @Test + void registersOneTickLaterWhenMmoItemsIsEnabled() { + JavaPlugin plugin = mock(JavaPlugin.class); + Server server = mock(Server.class); + PluginManager plugins = mock(PluginManager.class); + BukkitScheduler scheduler = mock(BukkitScheduler.class); + Logger logger = mock(Logger.class); + when(plugin.getServer()).thenReturn(server); + when(plugin.getLogger()).thenReturn(logger); + when(server.getPluginManager()).thenReturn(plugins); + when(server.getScheduler()).thenReturn(scheduler); + + BookCopyUnblocker.register(plugin); + ArgumentCaptor task = ArgumentCaptor.forClass(Runnable.class); + verify(scheduler).runTask(eq(plugin), task.capture()); + + task.getValue().run(); + verifyNoInteractions(logger); + + when(plugins.isPluginEnabled("MMOItems")).thenReturn(true); + task.getValue().run(); + verify(logger).warning("MMOItems craft handler not found; book copies stay blocked"); + + mmoItemsCancels(InventoryClickEvent.getHandlerList()); + task.getValue().run(); + verify(logger).info("Book copies: wrapped 1 MMOItems click handlers"); + } + + private void mmoItemsCancels(HandlerList list) { + list.register( + new RegisteredListener( + disable, + (l, e) -> ((Cancellable) e).setCancelled(true), + EventPriority.NORMAL, + mmoItems, + false)); + } + + private void otherCancels(HandlerList list, EventPriority priority) { + list.register( + new RegisteredListener( + new Listener() {}, (l, e) -> ((Cancellable) e).setCancelled(true), priority, other, false)); + } + + private void dispatch(InventoryClickEvent event) { + for (RegisteredListener registered : handlers.getRegisteredListeners()) call(registered, event); + } + + private static void call(RegisteredListener registered, InventoryClickEvent event) { + try { + registered.callEvent(event); + } catch (Exception e) { + throw new AssertionError(e); + } + } + + private static ComplexRecipe bookCloning() { + ComplexRecipe recipe = mock(ComplexRecipe.class); + when(recipe.getKey()).thenReturn(BookCopyUnblocker.BOOK_CLONING); + return recipe; + } + + private static InventoryView view() { + InventoryView view = mock(InventoryView.class); + when(view.getTopInventory()).thenReturn(mock(CraftingInventory.class)); + return view; + } + + private static CraftItemEvent craft(Recipe recipe) { + return new CraftItemEvent( + recipe, view(), InventoryType.SlotType.RESULT, 0, ClickType.LEFT, InventoryAction.PICKUP_ALL); + } +} From acaf23a482017778f1f0cf08d2191cd62648be9a Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Mon, 19 Oct 2026 21:39:16 +0200 Subject: [PATCH 3/4] fix: make the MMOItems wrap reload-safe and catch a late MMOItems Review: wrap on MMOItems' PluginEnableEvent as well as at TFMCCore enable (instead of a one-shot next-tick task), never wrap a wrapper, and put MMOItems' own registrations back in onDisable so a TFMCCore reload keeps no old classloader alive or stacks executors. Registrations that MMOItems dropped itself are not restored. Co-Authored-By: Claude Opus 5.5 --- .../net/tfminecraft/tfmccore/TFMCCore.java | 4 +- .../tfmccore/books/BookCopyUnblocker.java | 88 ++++++++++---- .../tfmccore/PluginLifecycleCoverageTest.java | 4 +- .../tfmccore/books/BookCopyUnblockerTest.java | 112 ++++++++++++++---- 4 files changed, 160 insertions(+), 48 deletions(-) diff --git a/src/main/java/net/tfminecraft/tfmccore/TFMCCore.java b/src/main/java/net/tfminecraft/tfmccore/TFMCCore.java index 49918b5..64468fd 100644 --- a/src/main/java/net/tfminecraft/tfmccore/TFMCCore.java +++ b/src/main/java/net/tfminecraft/tfmccore/TFMCCore.java @@ -77,6 +77,7 @@ public class TFMCCore extends JavaPlugin{ private net.tfminecraft.tfmccore.resourcepack.MultipartPackService multipartPacks; private net.tfminecraft.tfmccore.resourcepack.PackTextureGuard textureGuard; private net.tfminecraft.tfmccore.resourcepack.RequiredPackGate requiredPacks; + private BookCopyUnblocker bookCopies; @Override public void onEnable() { @@ -182,6 +183,7 @@ public void onDisable() { if (requiredPacks != null) requiredPacks.close(); if (multipartPacks != null) multipartPacks.close(); if (textureGuard != null) textureGuard.close(); + if (bookCopies != null) bookCopies.unwrap(); if (stoneListener != null) { stoneListener.refundAll(); } @@ -330,7 +332,7 @@ public void registerListeners() { // Covers a plugin reload; on a normal start nobody is online yet getServer().getOnlinePlayers().forEach(XaeroFairPlayListener::send); BookGlintHider.register(this); - BookCopyUnblocker.register(this); + bookCopies = BookCopyUnblocker.register(this); } public void createConfigs() { diff --git a/src/main/java/net/tfminecraft/tfmccore/books/BookCopyUnblocker.java b/src/main/java/net/tfminecraft/tfmccore/books/BookCopyUnblocker.java index 0e82b19..97141a2 100644 --- a/src/main/java/net/tfminecraft/tfmccore/books/BookCopyUnblocker.java +++ b/src/main/java/net/tfminecraft/tfmccore/books/BookCopyUnblocker.java @@ -1,12 +1,19 @@ package net.tfminecraft.tfmccore.books; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + import org.bukkit.Keyed; import org.bukkit.NamespacedKey; import org.bukkit.event.Cancellable; import org.bukkit.event.Event; +import org.bukkit.event.EventHandler; import org.bukkit.event.HandlerList; +import org.bukkit.event.Listener; import org.bukkit.event.inventory.CraftItemEvent; import org.bukkit.event.inventory.InventoryClickEvent; +import org.bukkit.event.server.PluginEnableEvent; import org.bukkit.plugin.EventExecutor; import org.bukkit.plugin.RegisteredListener; import org.bukkit.plugin.java.JavaPlugin; @@ -20,38 +27,77 @@ * wrapped so that only a book copy cancel made by that handler call is undone; cancels * from every other plugin, at any priority, stay. */ -public final class BookCopyUnblocker { +public final class BookCopyUnblocker implements Listener { static final NamespacedKey BOOK_CLONING = NamespacedKey.minecraft("book_cloning"); static final String MMOITEMS_LISTENER = "net.Indyuce.mmoitems.listener.DisableInteractions"; - private BookCopyUnblocker() { + private final JavaPlugin plugin; + private final HandlerList handlers; + private final List wrapped = new ArrayList<>(); + + BookCopyUnblocker(JavaPlugin plugin, HandlerList handlers) { + this.plugin = plugin; + this.handlers = handlers; } - public static void register(JavaPlugin plugin) { - // MMOItems is not a dependency, so it may enable after TFMCCore: wait until startup ends - plugin.getServer().getScheduler().runTask(plugin, () -> { - if (!plugin.getServer().getPluginManager().isPluginEnabled("MMOItems")) return; - int wrapped = wrap(InventoryClickEvent.getHandlerList()); - if (wrapped == 0) { - plugin.getLogger().warning("MMOItems craft handler not found; book copies stay blocked"); - } else { - plugin.getLogger().info("Book copies: wrapped " + wrapped + " MMOItems click handlers"); - } - }); + public static BookCopyUnblocker register(JavaPlugin plugin) { + BookCopyUnblocker unblocker = new BookCopyUnblocker(plugin, InventoryClickEvent.getHandlerList()); + plugin.getServer().getPluginManager().registerEvents(unblocker, plugin); + // MMOItems is not a dependency: it registers its handlers in its own onEnable, before or after ours + if (plugin.getServer().getPluginManager().isPluginEnabled("MMOItems")) unblocker.wrapAndLog(); + return unblocker; + } + + @EventHandler + public void onPluginEnable(PluginEnableEvent event) { + if (event.getPlugin().getName().equals("MMOItems")) wrapAndLog(); + } + + void wrapAndLog() { + int count = wrap(); + if (wrapped.isEmpty()) { + plugin.getLogger().warning("MMOItems craft handler not found; book copies stay blocked"); + } else if (count > 0) { + plugin.getLogger().info("Book copies: wrapped " + count + " MMOItems click handlers"); + } } - /** Replaces each MMOItems DisableInteractions registration with one that undoes its book copy cancels. */ - static int wrap(HandlerList handlers) { - int wrapped = 0; + /** Replaces each unwrapped MMOItems DisableInteractions registration with one that undoes its book copy cancels. */ + int wrap() { + int count = 0; for (RegisteredListener original : handlers.getRegisteredListeners()) { - if (!original.getListener().getClass().getName().equals(MMOITEMS_LISTENER)) continue; + if (original instanceof Wrapped + || !original.getListener().getClass().getName().equals(MMOITEMS_LISTENER)) continue; + Wrapped wrapper = new Wrapped(original); handlers.unregister(original); - handlers.register(new RegisteredListener(original.getListener(), executor(original), - original.getPriority(), original.getPlugin(), original.isIgnoringCancelled())); - wrapped++; + handlers.register(wrapper); + wrapped.add(wrapper); + count++; + } + return count; + } + + /** Puts MMOItems' own registrations back, so a TFMCCore disable or reload leaves nothing of ours behind. */ + public void unwrap() { + List current = Arrays.asList(handlers.getRegisteredListeners()); + for (Wrapped wrapper : wrapped) { + // Gone when MMOItems was disabled meanwhile; its original must not come back then + if (!current.contains(wrapper)) continue; + handlers.unregister(wrapper); + handlers.register(wrapper.original); + } + wrapped.clear(); + } + + static final class Wrapped extends RegisteredListener { + final RegisteredListener original; + + Wrapped(RegisteredListener original) { + super(original.getListener(), executor(original), original.getPriority(), + original.getPlugin(), original.isIgnoringCancelled()); + this.original = original; } - return wrapped; } static EventExecutor executor(RegisteredListener original) { diff --git a/src/test/java/net/tfminecraft/tfmccore/PluginLifecycleCoverageTest.java b/src/test/java/net/tfminecraft/tfmccore/PluginLifecycleCoverageTest.java index 4768823..146be0f 100644 --- a/src/test/java/net/tfminecraft/tfmccore/PluginLifecycleCoverageTest.java +++ b/src/test/java/net/tfminecraft/tfmccore/PluginLifecycleCoverageTest.java @@ -54,6 +54,7 @@ import net.tfminecraft.tfmccore.stats.categories.skills.SkillsStatConfig; import net.tfminecraft.tfmccore.stats.categories.vehicles.VehiclesStatConfig; import net.tfminecraft.tfmccore.stones.LorestoneConfigLoader; +import net.tfminecraft.tfmccore.books.BookCopyUnblocker; import net.tfminecraft.tfmccore.stones.StoneItems; import net.tfminecraft.tfmccore.stones.StoneListener; import net.tfminecraft.tfmccore.testsupport.TestRegistryAccess; @@ -293,7 +294,8 @@ void startupRegistersCommandsListenersAndTheActualLifecycleAndQuitCallbacks() th verify(rig.coreCommand).setTabCompleter(rig.tabCompletion); verify(rig.silentCommand).setExecutor(any(SilentPermissionCommand.class)); verify(rig.silentCommand).setTabCompleter(any(SilentPermissionCommand.class)); - assertEquals(9, rig.listeners.size()); + assertEquals(10, rig.listeners.size()); + assertTrue(rig.listeners.stream().anyMatch(BookCopyUnblocker.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()); diff --git a/src/test/java/net/tfminecraft/tfmccore/books/BookCopyUnblockerTest.java b/src/test/java/net/tfminecraft/tfmccore/books/BookCopyUnblockerTest.java index 8730c3f..611344f 100644 --- a/src/test/java/net/tfminecraft/tfmccore/books/BookCopyUnblockerTest.java +++ b/src/test/java/net/tfminecraft/tfmccore/books/BookCopyUnblockerTest.java @@ -18,6 +18,7 @@ import org.bukkit.event.inventory.InventoryAction; import org.bukkit.event.inventory.InventoryClickEvent; import org.bukkit.event.inventory.InventoryType; +import org.bukkit.event.server.PluginEnableEvent; import org.bukkit.inventory.ComplexRecipe; import org.bukkit.inventory.CraftingInventory; import org.bukkit.inventory.InventoryView; @@ -26,11 +27,9 @@ import org.bukkit.plugin.PluginManager; import org.bukkit.plugin.RegisteredListener; import org.bukkit.plugin.java.JavaPlugin; -import org.bukkit.scheduler.BukkitScheduler; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; class BookCopyUnblockerTest { @@ -58,7 +57,7 @@ void restore() { @Test void undoesTheBookCopyCancelMadeByMmoItems() { mmoItemsCancels(handlers); - assertEquals(1, BookCopyUnblocker.wrap(handlers)); + assertEquals(1, unblocker(handlers).wrap()); CraftItemEvent event = craft(bookCloning()); dispatch(event); @@ -69,7 +68,7 @@ void undoesTheBookCopyCancelMadeByMmoItems() { @Test void keepsWhatTheWrappedRegistrationLooksLike() { mmoItemsCancels(handlers); - BookCopyUnblocker.wrap(handlers); + unblocker(handlers).wrap(); RegisteredListener wrapped = handlers.getRegisteredListeners()[0]; assertEquals(1, handlers.getRegisteredListeners().length); @@ -88,7 +87,7 @@ void keepsCancelsFromOtherPluginsAtEveryPriority() { if (priority == EventPriority.NORMAL) otherCancels(list, priority); mmoItemsCancels(list); otherCancels(list, priority); - BookCopyUnblocker.wrap(list); + unblocker(list).wrap(); CraftItemEvent event = craft(bookCloning()); for (RegisteredListener registered : list.getRegisteredListeners()) call(registered, event); @@ -102,7 +101,7 @@ void keepsCancelsFromOtherPluginsAtEveryPriority() { @Test void keepsAnOtherPluginsCancelThatComesAfterMmoItemsInTheSameSlot() { mmoItemsCancels(handlers); - BookCopyUnblocker.wrap(handlers); + unblocker(handlers).wrap(); // Registered after wrapping, so it runs after the wrapped MMOItems handler. otherCancels(handlers, EventPriority.NORMAL); @@ -115,7 +114,7 @@ void keepsAnOtherPluginsCancelThatComesAfterMmoItemsInTheSameSlot() { @Test void leavesOtherRecipesAndClicksCancelled() { mmoItemsCancels(handlers); - BookCopyUnblocker.wrap(handlers); + unblocker(handlers).wrap(); ComplexRecipe map = mock(ComplexRecipe.class); when(map.getKey()).thenReturn(NamespacedKey.minecraft("map_cloning")); @@ -136,14 +135,14 @@ void leavesAllowedCraftsAndCanBeTurnedOff() { RegisteredListener allows = new RegisteredListener(disable, (l, e) -> {}, EventPriority.NORMAL, mmoItems, false); handlers.register(allows); - BookCopyUnblocker.wrap(handlers); + unblocker(handlers).wrap(); CraftItemEvent allowed = craft(bookCloning()); dispatch(allowed); assertFalse(allowed.isCancelled()); handlers.unregisterAll(mmoItems); mmoItemsCancels(handlers); - BookCopyUnblocker.wrap(handlers); + unblocker(handlers).wrap(); Cache.allowBookCopies = false; CraftItemEvent off = craft(bookCloning()); dispatch(off); @@ -155,37 +154,100 @@ void onlyWrapsMmoItemsDisableInteractions() { handlers.register( new RegisteredListener(new Listener() {}, (l, e) -> {}, EventPriority.NORMAL, other, false)); - assertEquals(0, BookCopyUnblocker.wrap(handlers)); + assertEquals(0, unblocker(handlers).wrap()); } @Test - void registersOneTickLaterWhenMmoItemsIsEnabled() { - JavaPlugin plugin = mock(JavaPlugin.class); - Server server = mock(Server.class); - PluginManager plugins = mock(PluginManager.class); - BukkitScheduler scheduler = mock(BukkitScheduler.class); + void neverWrapsAWrapperAndUnwrapRestoresMmoItemsOwnRegistration() { + mmoItemsCancels(handlers); + RegisteredListener original = handlers.getRegisteredListeners()[0]; + BookCopyUnblocker unblocker = unblocker(handlers); + + assertEquals(1, unblocker.wrap()); + assertEquals(0, unblocker.wrap()); + assertEquals(1, handlers.getRegisteredListeners().length); + assertNotSame(original, handlers.getRegisteredListeners()[0]); + + unblocker.unwrap(); + assertArrayEquals(new RegisteredListener[] {original}, handlers.getRegisteredListeners()); + CraftItemEvent event = craft(bookCloning()); + dispatch(event); + assertTrue(event.isCancelled()); + + // After a TFMCCore reload the original is found and wrapped again. + assertEquals(1, unblocker(handlers).wrap()); + } + + @Test + void unwrapLeavesOutHandlersOfADisabledMmoItems() { + mmoItemsCancels(handlers); + BookCopyUnblocker unblocker = unblocker(handlers); + unblocker.wrap(); + handlers.unregisterAll(mmoItems); + + unblocker.unwrap(); + + assertEquals(0, handlers.getRegisteredListeners().length); + } + + @Test + void wrapsWhenMmoItemsEnablesLater() { Logger logger = mock(Logger.class); - when(plugin.getServer()).thenReturn(server); - when(plugin.getLogger()).thenReturn(logger); - when(server.getPluginManager()).thenReturn(plugins); - when(server.getScheduler()).thenReturn(scheduler); + BookCopyUnblocker unblocker = new BookCopyUnblocker(plugin(logger, mock(PluginManager.class)), handlers); + when(other.getName()).thenReturn("Other"); + when(mmoItems.getName()).thenReturn("MMOItems"); - BookCopyUnblocker.register(plugin); - ArgumentCaptor task = ArgumentCaptor.forClass(Runnable.class); - verify(scheduler).runTask(eq(plugin), task.capture()); + unblocker.onPluginEnable(enabled(other)); + verifyNoInteractions(logger); + + mmoItemsCancels(handlers); + unblocker.onPluginEnable(enabled(mmoItems)); + verify(logger).info("Book copies: wrapped 1 MMOItems click handlers"); + + // Nothing new to wrap: no second line, and no warning. + unblocker.onPluginEnable(enabled(mmoItems)); + verify(logger).info(anyString()); + verify(logger, never()).warning(anyString()); + } + + @Test + void registersItsListenerAndWrapsWhenMmoItemsIsAlreadyEnabled() { + Logger logger = mock(Logger.class); + PluginManager plugins = mock(PluginManager.class); + JavaPlugin plugin = plugin(logger, plugins); - task.getValue().run(); + BookCopyUnblocker first = BookCopyUnblocker.register(plugin); + verify(plugins).registerEvents(first, plugin); verifyNoInteractions(logger); when(plugins.isPluginEnabled("MMOItems")).thenReturn(true); - task.getValue().run(); + BookCopyUnblocker.register(plugin); verify(logger).warning("MMOItems craft handler not found; book copies stay blocked"); mmoItemsCancels(InventoryClickEvent.getHandlerList()); - task.getValue().run(); + BookCopyUnblocker.register(plugin).unwrap(); verify(logger).info("Book copies: wrapped 1 MMOItems click handlers"); } + private static JavaPlugin plugin(Logger logger, PluginManager plugins) { + JavaPlugin plugin = mock(JavaPlugin.class); + Server server = mock(Server.class); + when(plugin.getServer()).thenReturn(server); + when(plugin.getLogger()).thenReturn(logger); + when(server.getPluginManager()).thenReturn(plugins); + return plugin; + } + + private static PluginEnableEvent enabled(Plugin plugin) { + PluginEnableEvent event = mock(PluginEnableEvent.class); + when(event.getPlugin()).thenReturn(plugin); + return event; + } + + private static BookCopyUnblocker unblocker(HandlerList list) { + return new BookCopyUnblocker(mock(JavaPlugin.class), list); + } + private void mmoItemsCancels(HandlerList list) { list.register( new RegisteredListener( From 4c571052251e64524c2acaa9b735497949331f8c Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Mon, 19 Oct 2026 22:42:37 +0200 Subject: [PATCH 4/4] fix: keep MMOItems' handler in its place when wrapping Review: unregister + register moved the wrapped MMOItems handler to the end of its NORMAL slot. HandlerList only appends, so the listeners after it in that slot are now taken out and re-added behind the replacement, for both wrap and unwrap. Co-Authored-By: Claude Opus 5.5 --- .../tfmccore/books/BookCopyUnblocker.java | 26 +++++++++++++--- .../tfmccore/books/BookCopyUnblockerTest.java | 30 +++++++++++++++++-- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/tfminecraft/tfmccore/books/BookCopyUnblocker.java b/src/main/java/net/tfminecraft/tfmccore/books/BookCopyUnblocker.java index 97141a2..5fe88f6 100644 --- a/src/main/java/net/tfminecraft/tfmccore/books/BookCopyUnblocker.java +++ b/src/main/java/net/tfminecraft/tfmccore/books/BookCopyUnblocker.java @@ -70,22 +70,40 @@ int wrap() { if (original instanceof Wrapped || !original.getListener().getClass().getName().equals(MMOITEMS_LISTENER)) continue; Wrapped wrapper = new Wrapped(original); - handlers.unregister(original); - handlers.register(wrapper); + replace(original, wrapper); wrapped.add(wrapper); count++; } return count; } + /** + * Swaps one registration for another in the same place. HandlerList only appends, so the + * listeners after it in its priority slot are taken out and added back behind the replacement. + */ + private void replace(RegisteredListener old, RegisteredListener replacement) { + List later = new ArrayList<>(); + boolean found = false; + for (RegisteredListener registered : handlers.getRegisteredListeners()) { + if (registered == old) { + found = true; + } else if (found && registered.getPriority() == old.getPriority()) { + later.add(registered); + } + } + handlers.unregister(old); + later.forEach(handlers::unregister); + handlers.register(replacement); + later.forEach(handlers::register); + } + /** Puts MMOItems' own registrations back, so a TFMCCore disable or reload leaves nothing of ours behind. */ public void unwrap() { List current = Arrays.asList(handlers.getRegisteredListeners()); for (Wrapped wrapper : wrapped) { // Gone when MMOItems was disabled meanwhile; its original must not come back then if (!current.contains(wrapper)) continue; - handlers.unregister(wrapper); - handlers.register(wrapper.original); + replace(wrapper, wrapper.original); } wrapped.clear(); } diff --git a/src/test/java/net/tfminecraft/tfmccore/books/BookCopyUnblockerTest.java b/src/test/java/net/tfminecraft/tfmccore/books/BookCopyUnblockerTest.java index 611344f..10bdff9 100644 --- a/src/test/java/net/tfminecraft/tfmccore/books/BookCopyUnblockerTest.java +++ b/src/test/java/net/tfminecraft/tfmccore/books/BookCopyUnblockerTest.java @@ -101,9 +101,8 @@ void keepsCancelsFromOtherPluginsAtEveryPriority() { @Test void keepsAnOtherPluginsCancelThatComesAfterMmoItemsInTheSameSlot() { mmoItemsCancels(handlers); - unblocker(handlers).wrap(); - // Registered after wrapping, so it runs after the wrapped MMOItems handler. otherCancels(handlers, EventPriority.NORMAL); + unblocker(handlers).wrap(); CraftItemEvent event = craft(bookCloning()); dispatch(event); @@ -178,6 +177,33 @@ void neverWrapsAWrapperAndUnwrapRestoresMmoItemsOwnRegistration() { assertEquals(1, unblocker(handlers).wrap()); } + @Test + void wrapAndUnwrapKeepEveryListenersPlace() { + otherCancels(handlers, EventPriority.LOW); + otherCancels(handlers, EventPriority.NORMAL); + mmoItemsCancels(handlers); + otherCancels(handlers, EventPriority.NORMAL); + otherCancels(handlers, EventPriority.NORMAL); + otherCancels(handlers, EventPriority.HIGH); + RegisteredListener[] before = handlers.getRegisteredListeners(); + BookCopyUnblocker unblocker = unblocker(handlers); + + unblocker.wrap(); + RegisteredListener[] wrapped = handlers.getRegisteredListeners(); + assertEquals(before.length, wrapped.length); + for (int i = 0; i < before.length; i++) { + if (i == 2) { + assertNotSame(before[i], wrapped[i]); + assertSame(disable, wrapped[i].getListener()); + } else { + assertSame(before[i], wrapped[i], "position " + i); + } + } + + unblocker.unwrap(); + assertArrayEquals(before, handlers.getRegisteredListeners()); + } + @Test void unwrapLeavesOutHandlersOfADisabledMmoItems() { mmoItemsCancels(handlers);