diff --git a/src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java b/src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java index a171867..a90feb8 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java +++ b/src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java @@ -15,10 +15,12 @@ import net.tfminecraft.birdmessenger.mail.MailService; import net.tfminecraft.birdmessenger.mail.MailStore; import net.tfminecraft.birdmessenger.session.SelectedTarget; +import net.tfminecraft.birdmessenger.session.SendSession; import net.tfminecraft.birdmessenger.session.SendSessionManager; import net.tfminecraft.birdmessenger.letters.LetterFeature; import java.util.List; +import java.util.Objects; public final class BirdMessenger extends JavaPlugin { @@ -107,18 +109,18 @@ public void openPicker(Player player) { if (player == null || !player.isOnline()) { return; } + SendSession session = sessions.get(player.getUniqueId()); + if (session == null || session.getLetter() == null || session.isConfirmed()) { + return; + } + if (player.getOpenInventory().getTopInventory().getHolder() instanceof CharacterPickerGui) { + return; + } if (!Bukkit.getPluginManager().isPluginEnabled("RPCharacters")) { sessions.returnLetter(player, false); player.sendMessage(config.msgRpcMissing()); return; } - RPCharacters.refreshMailTargetTexturesAsync(() -> openPickerAfterTextures(player)); - } - - private void openPickerAfterTextures(Player player) { - if (player == null || !player.isOnline()) { - return; - } List targets = CharacterPickerGui.loadTargets(); if (targets.isEmpty()) { sessions.returnLetter(player, false); @@ -131,7 +133,35 @@ private void openPickerAfterTextures(Player player) { player.sendMessage(config.msgCannotSendToSelf()); return; } + CharacterPickerGui picker = new CharacterPickerGui(this, targets); player.sendMessage(config.msgPickerSelect()); - player.openInventory(new CharacterPickerGui(this, targets).getInventory()); + player.openInventory(picker.getInventory()); + // Skin lookups can take minutes, so fill in missing heads after the picker is open. + RPCharacters.refreshMailTargetTexturesAsync(() -> refreshPickerHeads(player, picker)); + } + + private void refreshPickerHeads(Player player, CharacterPickerGui picker) { + if (!player.isOnline() + || player.getOpenInventory().getTopInventory().getHolder() != picker) { + return; + } + SendSession session = sessions.get(player.getUniqueId()); + if (session == null) { + return; + } + List targets = CharacterPickerGui.loadTargets(); + targets.removeIf(t -> player.getUniqueId().equals(t.getOwnerUuid())); + picker.setTargets(targets); + SelectedTarget selected = session.getSelected(); + if (selected != null) { + // Drop a selection whose recipient is no longer listed. + session.setSelected(targets.stream() + .filter(t -> selected.getCharacterId().equals(t.getCharacterId()) + && Objects.equals(selected.getOwnerUuid(), t.getOwnerUuid())) + .findFirst() + .orElse(null)); + } + session.setPickerPage(Math.min(session.getPickerPage(), picker.maxPage())); + CharacterPickerGui.applyPage(session, picker); } } diff --git a/src/main/java/net/tfminecraft/birdmessenger/gui/CharacterPickerGui.java b/src/main/java/net/tfminecraft/birdmessenger/gui/CharacterPickerGui.java index 96450c4..bba48d9 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/gui/CharacterPickerGui.java +++ b/src/main/java/net/tfminecraft/birdmessenger/gui/CharacterPickerGui.java @@ -31,7 +31,7 @@ public final class CharacterPickerGui implements InventoryHolder { public static final int SLOT_NEXT = 53; private final BirdMessenger plugin; - private final List targets; + private List targets; private final Inventory inventory; // Keep the existing legacy text representation, formatting, and exact-string comparisons. @@ -47,6 +47,11 @@ public List targets() { return targets; } + /** Swap in a reloaded target list, for example once missing skins arrive. */ + public void setTargets(List targets) { + this.targets = targets; + } + // Keep the existing legacy text representation, formatting, and exact-string comparisons. @SuppressWarnings("deprecation") public void render(int page, SelectedTarget selected) { diff --git a/src/main/java/net/tfminecraft/birdmessenger/listener/GuiListener.java b/src/main/java/net/tfminecraft/birdmessenger/listener/GuiListener.java index fe971fa..f23e3a0 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/listener/GuiListener.java +++ b/src/main/java/net/tfminecraft/birdmessenger/listener/GuiListener.java @@ -82,6 +82,8 @@ public void onClose(InventoryCloseEvent event) { return; } event.getInventory().setItem(LetterGui.LETTER_SLOT, null); + // A second letter must not overwrite one that is still waiting for a recipient. + plugin.sessions().returnLetter(player, false); SendSession session = plugin.sessions().getOrCreate(player.getUniqueId()); session.setLetter(placed.clone()); session.setSelected(null); @@ -114,7 +116,12 @@ private void handleLetterClick(InventoryClickEvent event, Player player) { return; } if (LetterItems.isLetter(plugin.config(), cursor)) { - Bukkit.getScheduler().runTaskLater(plugin, () -> player.closeInventory(), 3L); + Bukkit.getScheduler().runTaskLater(plugin, () -> { + // The player may already have closed this GUI and moved on to another one. + if (player.getOpenInventory().getTopInventory() == top) { + player.closeInventory(); + } + }, 3L); } return; } diff --git a/src/test/java/net/tfminecraft/birdmessenger/PickerOpenTest.java b/src/test/java/net/tfminecraft/birdmessenger/PickerOpenTest.java new file mode 100644 index 0000000..af7fe86 --- /dev/null +++ b/src/test/java/net/tfminecraft/birdmessenger/PickerOpenTest.java @@ -0,0 +1,186 @@ +package net.tfminecraft.birdmessenger; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; + +import org.bukkit.Bukkit; +import org.bukkit.configuration.file.YamlConfiguration; +import org.bukkit.entity.Player; +import org.bukkit.inventory.Inventory; +import org.bukkit.inventory.InventoryHolder; +import org.bukkit.inventory.InventoryView; +import org.bukkit.inventory.ItemStack; +import org.bukkit.plugin.PluginManager; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedConstruction; +import org.mockito.MockedStatic; + +import net.tfminecraft.birdmessenger.gui.CharacterPickerGui; +import net.tfminecraft.birdmessenger.session.SendSession; +import net.tfminecraft.birdmessenger.session.SendSessionManager; +import net.tfminecraft.rpcharacters.RPCharacters; +import net.tfminecraft.rpcharacters.api.CharacterSkull; +import net.tfminecraft.rpcharacters.mail.CharacterMailTarget; + +class PickerOpenTest { + private final UUID self = UUID.randomUUID(); + private final UUID other = UUID.randomUUID(); + private final List refreshCallbacks = new ArrayList<>(); + private final List directory = new ArrayList<>(); + private BirdMessenger plugin; + private SendSessionManager sessions; + private Player player; + private InventoryHolder openHolder; + private int opens; + + @BeforeEach void setup() throws Exception { + refreshCallbacks.clear(); + directory.clear(); + opens = 0; + openHolder = null; + plugin = mock(BirdMessenger.class, CALLS_REAL_METHODS); + doReturn(new YamlConfiguration()).when(plugin).getConfig(); + sessions = new SendSessionManager(plugin); + setField("config", new BirdConfig(plugin)); + setField("sessions", sessions); + player = mock(Player.class); + when(player.getUniqueId()).thenReturn(self); + when(player.isOnline()).thenReturn(true); + InventoryView view = mock(InventoryView.class); + Inventory top = mock(Inventory.class); + when(player.getOpenInventory()).thenReturn(view); + when(view.getTopInventory()).thenReturn(top); + when(top.getHolder()).thenAnswer(call -> openHolder); + when(player.openInventory(any(Inventory.class))).thenAnswer(call -> { + opens++; + openHolder = ((Inventory) call.getArgument(0)).getHolder(); + return view; + }); + } + + @Test void pickerOpensBeforeSkinRefreshFinishesAndUpdatesAfterwards() { + directory.add(target(other, "a", "Alice")); + pendingLetter(); + try (var mocks = new Mocks()) { + plugin.openPicker(player); + assertEquals(1, opens, "picker must not wait for skin lookups"); + CharacterPickerGui picker = assertInstanceOf(CharacterPickerGui.class, openHolder); + assertEquals(1, picker.targets().size()); + assertEquals(1, refreshCallbacks.size()); + + directory.add(target(other, "b", "Bob")); + refreshCallbacks.getFirst().run(); + assertEquals(1, opens, "refresh re-renders the open picker instead of reopening it"); + assertEquals(2, picker.targets().size()); + } + } + + @Test void refreshDropsASelectedRecipientWhoIsNoLongerListed() { + CharacterMailTarget alice = target(other, "a", "Alice"); + CharacterMailTarget bob = target(other, "b", "Bob"); + directory.add(alice); + directory.add(bob); + pendingLetter(); + try (var mocks = new Mocks()) { + plugin.openPicker(player); + CharacterPickerGui picker = (CharacterPickerGui) openHolder; + SendSession session = sessions.get(self); + session.setSelected(picker.targets().get(1)); + refreshCallbacks.getFirst().run(); + assertEquals("b", session.getSelected().getCharacterId(), "a listed selection is kept"); + + directory.remove(bob); + refreshCallbacks.getFirst().run(); + assertNull(session.getSelected()); + + session.setSelected(picker.targets().get(0)); + directory.clear(); + clearInvocations(picker.getInventory()); + refreshCallbacks.getFirst().run(); + assertTrue(picker.targets().isEmpty(), "an emptied list is redrawn, not left stale"); + verify(picker.getInventory()).clear(); + verify(picker.getInventory()).setItem(eq(CharacterPickerGui.SLOT_CANCEL), any()); + verify(picker.getInventory()).setItem(eq(CharacterPickerGui.SLOT_CONFIRM), any()); + assertNull(session.getSelected()); + } + } + + @Test void refreshIgnoresPickerThePlayerHasClosed() { + directory.add(target(other, "a", "Alice")); + pendingLetter(); + try (var mocks = new Mocks()) { + plugin.openPicker(player); + CharacterPickerGui picker = (CharacterPickerGui) openHolder; + openHolder = null; + directory.add(target(other, "b", "Bob")); + refreshCallbacks.getFirst().run(); + assertEquals(1, picker.targets().size()); + } + } + + @Test void doesNotOpenWithoutPendingLetterOrOverAnOpenPicker() { + directory.add(target(other, "a", "Alice")); + try (var mocks = new Mocks()) { + plugin.openPicker(player); + assertEquals(0, opens); + + pendingLetter(); + plugin.openPicker(player); + plugin.openPicker(player); + assertEquals(1, opens); + } + } + + private void pendingLetter() { + SendSession session = sessions.getOrCreate(self); + session.setLetter(mock(ItemStack.class)); + } + + private CharacterMailTarget target(UUID owner, String id, String name) { + return new CharacterMailTarget(owner, id, name, name, null, null, null, null); + } + + private void setField(String name, Object value) throws Exception { + Field field = BirdMessenger.class.getDeclaredField(name); + field.setAccessible(true); + field.set(plugin, value); + } + + private final class Mocks implements AutoCloseable { + private final MockedStatic bukkit = mockStatic(Bukkit.class); + private final MockedStatic rpc = mockStatic(RPCharacters.class); + private final MockedStatic skulls = mockStatic(CharacterSkull.class); + private final MockedConstruction items = mockConstruction(ItemStack.class); + + Mocks() { + PluginManager manager = mock(PluginManager.class); + when(manager.isPluginEnabled("RPCharacters")).thenReturn(true); + bukkit.when(Bukkit::getPluginManager).thenReturn(manager); + bukkit.when(() -> Bukkit.createInventory(any(InventoryHolder.class), anyInt(), anyString())) + .thenAnswer(call -> { + Inventory inventory = mock(Inventory.class); + InventoryHolder holder = call.getArgument(0); + when(inventory.getHolder()).thenReturn(holder); + return inventory; + }); + rpc.when(RPCharacters::listMailTargets).thenAnswer(call -> new ArrayList<>(directory)); + rpc.when(() -> RPCharacters.refreshMailTargetTexturesAsync(any())) + .thenAnswer(call -> refreshCallbacks.add(call.getArgument(0))); + skulls.when(() -> CharacterSkull.fromTextures(any(), any())) + .thenAnswer(call -> mock(ItemStack.class)); + } + + @Override public void close() { + items.close(); + skulls.close(); + rpc.close(); + bukkit.close(); + } + } +}