diff --git a/src/main/java/net/tfminecraft/birdmessenger/letters/LetterListener.java b/src/main/java/net/tfminecraft/birdmessenger/letters/LetterListener.java index fc6196c..cd7445a 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/letters/LetterListener.java +++ b/src/main/java/net/tfminecraft/birdmessenger/letters/LetterListener.java @@ -4,15 +4,16 @@ import org.bukkit.Bukkit; import org.bukkit.Material; -import org.bukkit.block.Block; +import org.bukkit.Statistic; import org.bukkit.entity.Player; import org.bukkit.event.EventHandler; +import org.bukkit.event.EventPriority; import org.bukkit.event.Listener; -import org.bukkit.event.block.Action; import org.bukkit.event.player.PlayerEditBookEvent; -import org.bukkit.event.player.PlayerInteractEvent; +import org.bukkit.event.player.PlayerStatisticIncrementEvent; import org.bukkit.inventory.EquipmentSlot; import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.PlayerInventory; import org.bukkit.inventory.meta.BookMeta; import net.tfminecraft.birdmessenger.BirdMessenger; @@ -78,23 +79,26 @@ public void onBookSign(PlayerEditBookEvent event) { }); } - @EventHandler - public void onBookOpen(PlayerInteractEvent event) { - Action action = event.getAction(); - if (action != Action.RIGHT_CLICK_AIR && action != Action.RIGHT_CLICK_BLOCK) return; - Block clicked = event.getClickedBlock(); - if (clicked != null && (clicked.getType() == Material.CHISELED_BOOKSHELF || clicked.getType() == Material.LECTERN)) return; - // No guard on which hand was used - off-hand letters open too - but the swap below - // must go back into that same hand, or the other hand's item gets overwritten. - EquipmentSlot hand = event.getHand(); - if (hand != EquipmentSlot.HAND && hand != EquipmentSlot.OFF_HAND) return; - ItemStack item = event.getItem(); + // Not ignoreCancelled: a cancelled count still means the book opened. + @EventHandler(priority = EventPriority.MONITOR) + public void onBookOpen(PlayerStatisticIncrementEvent event) { + // Vanilla counts a written book as used right after it opens the reading screen, + // and nowhere else. Clicks that open a barrel, use a door, place the book on a + // lectern or are denied by another plugin never reach this, so the seal holds. + if (event.getStatistic() != Statistic.USE_ITEM || event.getMaterial() != Material.WRITTEN_BOOK) return; + Player player = event.getPlayer(); + PlayerInventory inventory = player.getInventory(); + // The client only tries the off hand when the main hand item does nothing, and a + // written book always opens, so a main-hand written book is the one being read. + // The swap below must go back into that same hand, or the other hand's item gets overwritten. + ItemStack mainHand = inventory.getItemInMainHand(); + EquipmentSlot hand = mainHand.getType() == Material.WRITTEN_BOOK ? EquipmentSlot.HAND : EquipmentSlot.OFF_HAND; + ItemStack item = hand == EquipmentSlot.HAND ? mainHand : inventory.getItemInOffHand(); if (!items.isSealedLetter(item)) return; ItemStack original = item.clone(); BookMeta current = (BookMeta) item.getItemMeta(); if (current == null) return; - Player player = event.getPlayer(); CharacterSession session = Characters.currentSession(player).orElse(null); if (session == null && Characters.available()) return; ItemStack opened = items.createOpenedLetter(current); @@ -103,16 +107,16 @@ public void onBookOpen(PlayerInteractEvent event) { sendMessage(player, LetterConfig.openFailedMessage); return; } - // Not cancelled - the book still opens for reading, so swap the item on the next tick. + // The book is already open for reading, so swap the item on the next tick. Bukkit.getScheduler().runTask(plugin, () -> { if (!isCurrent(player, session)) return; ItemStack held = hand == EquipmentSlot.OFF_HAND - ? player.getInventory().getItemInOffHand() : player.getInventory().getItemInMainHand(); + ? inventory.getItemInOffHand() : inventory.getItemInMainHand(); if (!unchangedOnOpen(original, held)) return; if (hand == EquipmentSlot.OFF_HAND) { - player.getInventory().setItemInOffHand(opened); + inventory.setItemInOffHand(opened); } else { - player.getInventory().setItemInMainHand(opened); + inventory.setItemInMainHand(opened); } sendMessage(player, LetterConfig.openedMessage); }); diff --git a/src/test/java/net/tfminecraft/birdmessenger/letters/LetterListenerTest.java b/src/test/java/net/tfminecraft/birdmessenger/letters/LetterListenerTest.java index b154873..005e8c5 100644 --- a/src/test/java/net/tfminecraft/birdmessenger/letters/LetterListenerTest.java +++ b/src/test/java/net/tfminecraft/birdmessenger/letters/LetterListenerTest.java @@ -13,10 +13,12 @@ import org.bukkit.Bukkit; import org.bukkit.Material; +import org.bukkit.Statistic; import org.bukkit.entity.Player; -import org.bukkit.event.block.Action; +import org.bukkit.event.EventHandler; +import org.bukkit.event.EventPriority; import org.bukkit.event.player.PlayerEditBookEvent; -import org.bukkit.event.player.PlayerInteractEvent; +import org.bukkit.event.player.PlayerStatisticIncrementEvent; import org.bukkit.inventory.EquipmentSlot; import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.PlayerInventory; @@ -189,7 +191,7 @@ class LetterListenerTest { for (EquipmentSlot hand : List.of(EquipmentSlot.HAND, EquipmentSlot.OFF_HAND)) { for (String change : List.of("replaced", "mutated", "amount", "offline")) { setup(); - PlayerInteractEvent event = openEvent(hand); + PlayerStatisticIncrementEvent event = openEvent(hand); try (var bukkit = mockBukkit(); var factories = mockConstruction(LetterItems.class, (items, context) -> { when(items.isSealedLetter(original)).thenReturn(true); @@ -214,7 +216,7 @@ class LetterListenerTest { @Test void openingStillReplacesOnlyTheRecordedHand() { for (EquipmentSlot hand : List.of(EquipmentSlot.HAND, EquipmentSlot.OFF_HAND)) { setup(); - PlayerInteractEvent event = openEvent(hand); + PlayerStatisticIncrementEvent event = openEvent(hand); try (var bukkit = mockBukkit(); var factories = mockConstruction(LetterItems.class, (items, context) -> { when(items.isSealedLetter(original)).thenReturn(true); @@ -235,10 +237,49 @@ class LetterListenerTest { } } + @Test void onlyAnOpenedWrittenBookBreaksTheSeal() { + // A click that opens a barrel, uses a door or fills a lectern never counts the book as used. + for (var counted : List.of( + Map.entry(Statistic.DROP, Material.WRITTEN_BOOK), + Map.entry(Statistic.PICKUP, Material.WRITTEN_BOOK), + Map.entry(Statistic.USE_ITEM, Material.WRITABLE_BOOK), + Map.entry(Statistic.USE_ITEM, Material.BARREL))) { + setup(); + PlayerStatisticIncrementEvent event = statisticEvent(counted.getKey(), counted.getValue(), EquipmentSlot.HAND); + try (var bukkit = mockBukkit(); var factories = successfulFactories()) { + new LetterListener(plugin).onBookOpen(event); + verifyNoInteractions(factories.constructed().getFirst()); + assertTrue(pending.isEmpty()); + verifyNoWrites(); + } + } + } + + @Test void aMainHandWrittenBookIsTheOneReadNotAnOffHandLetter() { + PlayerStatisticIncrementEvent event = openEvent(EquipmentSlot.OFF_HAND); + ItemStack book = stackOf(Material.WRITTEN_BOOK); + held(EquipmentSlot.HAND, book); + try (var bukkit = mockBukkit(); var factories = successfulFactories()) { + new LetterListener(plugin).onBookOpen(event); + LetterItems factory = factories.constructed().getFirst(); + verify(factory).isSealedLetter(book); + verify(factory, never()).isSealedLetter(original); + assertTrue(pending.isEmpty()); + verifyNoWrites(); + } + } + + @Test void openingListensLastAndAlsoToCancelledCounts() throws NoSuchMethodException { + EventHandler handler = LetterListener.class + .getMethod("onBookOpen", PlayerStatisticIncrementEvent.class).getAnnotation(EventHandler.class); + assertEquals(EventPriority.MONITOR, handler.priority()); + assertFalse(handler.ignoreCancelled()); + } + @Test void openingAllowsOnlyTheVanillaResolvedFlagToChange() { for (String change : List.of("resolved", "pages", "pdc", "name", "type", "amount", "already-resolved")) { setup(); - PlayerInteractEvent event = openEvent(EquipmentSlot.HAND); + PlayerStatisticIncrementEvent event = openEvent(EquipmentSlot.HAND); BookMeta beforeMeta = mock(BookMeta.class); Map before = new HashMap<>(Map.of( "meta-type", "BOOK_SIGNED", "pages", List.of("original text"), @@ -256,7 +297,6 @@ class LetterListenerTest { case "pages" -> after.put("pages", List.of("other letter")); case "pdc" -> after.put("PublicBukkitValues", Map.of("tfmccore:sealed_letter", 2)); case "name" -> after.put("display-name", "Another name"); - case "type" -> when(original.getType()).thenReturn(Material.WRITABLE_BOOK); case "amount" -> when(original.getAmount()).thenReturn(2); case "already-resolved" -> before.put("resolved", true); } @@ -267,6 +307,8 @@ class LetterListenerTest { })) { bukkit.when(Bukkit::getScheduler).thenReturn(scheduler); new LetterListener(plugin).onBookOpen(event); + // The held type decides which hand was read, so it may only change after the open. + if (change.equals("type")) when(original.getType()).thenReturn(Material.WRITABLE_BOOK); pending.getLast().run(); if (change.equals("resolved")) verify(inventory).setItemInMainHand(replacement); else verify(inventory, never()).setItemInMainHand(any()); @@ -291,7 +333,7 @@ class LetterListenerTest { } @Test void failedOpeningKeepsTheOriginalSealAndReportsTheFailure() { - PlayerInteractEvent event = openEvent(EquipmentSlot.OFF_HAND); + PlayerStatisticIncrementEvent event = openEvent(EquipmentSlot.OFF_HAND); try (var bukkit = mockBukkit(); var factories = mockConstruction(LetterItems.class, (items, context) -> { when(items.isSealedLetter(original)).thenReturn(true); @@ -367,7 +409,7 @@ void pendingCharactersCannotRewriteLettersAreToldWhyAndMayStillReadThem(String o try (var bukkit = mockBukkit(); var factories = successfulFactories()) { LetterListener listener = new LetterListener(plugin); if (operation.equals("open")) { - PlayerInteractEvent event = openEvent(EquipmentSlot.HAND); + PlayerStatisticIncrementEvent event = openEvent(EquipmentSlot.HAND); listener.onBookOpen(event); verify(event, never()).setCancelled(anyBoolean()); } else { @@ -400,7 +442,7 @@ private org.mockito.MockedConstruction successfulFactories() { private void schedule(String operation) { LetterListener listener = new LetterListener(plugin); if (operation.equals("open")) { - PlayerInteractEvent event = openEvent(EquipmentSlot.HAND); + PlayerStatisticIncrementEvent event = openEvent(EquipmentSlot.HAND); listener.onBookOpen(event); verify(event, never()).setCancelled(anyBoolean()); } else { @@ -439,17 +481,28 @@ private PlayerEditBookEvent editEvent(int slot, boolean signing) { return event; } - private PlayerInteractEvent openEvent(EquipmentSlot hand) { - PlayerInteractEvent event = mock(PlayerInteractEvent.class); - when(event.getAction()).thenReturn(Action.RIGHT_CLICK_AIR); + private PlayerStatisticIncrementEvent openEvent(EquipmentSlot hand) { + return statisticEvent(Statistic.USE_ITEM, Material.WRITTEN_BOOK, hand); + } + + private PlayerStatisticIncrementEvent statisticEvent(Statistic statistic, Material material, EquipmentSlot hand) { + PlayerStatisticIncrementEvent event = mock(PlayerStatisticIncrementEvent.class); + when(event.getStatistic()).thenReturn(statistic); + when(event.getMaterial()).thenReturn(material); when(event.getPlayer()).thenReturn(player); - when(event.getHand()).thenReturn(hand); - when(event.getItem()).thenReturn(original); + when(original.getType()).thenReturn(Material.WRITTEN_BOOK); when(original.getItemMeta()).thenReturn(edited); held(hand, original); + if (hand == EquipmentSlot.OFF_HAND) held(EquipmentSlot.HAND, stackOf(Material.AIR)); return event; } + private static ItemStack stackOf(Material type) { + ItemStack stack = mock(ItemStack.class); + when(stack.getType()).thenReturn(type); + return stack; + } + private void held(EquipmentSlot hand, ItemStack stack) { if (hand == EquipmentSlot.HAND) when(inventory.getItemInMainHand()).thenReturn(stack); else when(inventory.getItemInOffHand()).thenReturn(stack);