fix(tyrant): synchronize rejected captured-mob transfers
Release / release (push) Successful in 3m14s
CI / build (push) Successful in 1m36s

This commit is contained in:
dmg
2026-09-12 09:25:04 -04:00
parent 695c78ea16
commit f69378fb2b
5 changed files with 455 additions and 10 deletions
@@ -31,6 +31,19 @@ public final class CapturedMobInventoryService {
} }
} }
Set<UUID> retained = new HashSet<>(); Set<UUID> retained = new HashSet<>();
ItemStack cursor = player.getItemOnCursor();
if (items.isCapturedMob(cursor)) {
UUID captureId = items.captureId(cursor).orElseThrow();
if (items.owner(cursor).filter(state.playerId()::equals).isPresent()
&& stored.containsKey(captureId) && retained.add(captureId)) {
if (cursor.getAmount() > 1) {
cursor.setAmount(1);
player.setItemOnCursor(cursor);
}
} else {
player.setItemOnCursor(null);
}
}
ItemStack[] contents = player.getInventory().getContents(); ItemStack[] contents = player.getInventory().getContents();
for (int index = 0; index < contents.length; index++) { for (int index = 0; index < contents.length; index++) {
ItemStack item = contents[index]; ItemStack item = contents[index];
@@ -42,6 +55,9 @@ public final class CapturedMobInventoryService {
&& stored.containsKey(captureId) && retained.add(captureId); && stored.containsKey(captureId) && retained.add(captureId);
if (!valid) { if (!valid) {
player.getInventory().setItem(index, null); player.getInventory().setItem(index, null);
} else if (item.getAmount() > 1) {
item.setAmount(1);
player.getInventory().setItem(index, item);
} }
} }
for (Map.Entry<UUID, CapturedMob> entry : stored.entrySet()) { for (Map.Entry<UUID, CapturedMob> entry : stored.entrySet()) {
@@ -205,7 +205,7 @@ public final class SpigotTyrantPlugin extends JavaPlugin {
getServer().getPluginManager().registerEvents( getServer().getPluginManager().registerEvents(
new TamerListener( new TamerListener(
stateManager, abilityItems, capturedMobItems, capturedMobs, stateManager, abilityItems, capturedMobItems, capturedMobs,
settings, getServer() settings, getServer(), task -> getServer().getScheduler().runTask(this, task)
), ),
this this
); );
@@ -33,6 +33,8 @@ public final class TamerListener implements Listener {
private final CapturedMobService capturedMobs; private final CapturedMobService capturedMobs;
private final PluginSettings settings; private final PluginSettings settings;
private final Server server; private final Server server;
private final java.util.function.Consumer<Runnable> nextTick;
private final java.util.Set<UUID> pendingInventoryRefresh = new java.util.HashSet<>();
public TamerListener( public TamerListener(
TyrantStateManager stateManager, TyrantStateManager stateManager,
@@ -40,7 +42,8 @@ public final class TamerListener implements Listener {
CapturedMobItemService capturedItems, CapturedMobItemService capturedItems,
CapturedMobService capturedMobs, CapturedMobService capturedMobs,
PluginSettings settings, PluginSettings settings,
Server server Server server,
java.util.function.Consumer<Runnable> nextTick
) { ) {
this.stateManager = stateManager; this.stateManager = stateManager;
this.abilityItems = abilityItems; this.abilityItems = abilityItems;
@@ -48,6 +51,7 @@ public final class TamerListener implements Listener {
this.capturedMobs = capturedMobs; this.capturedMobs = capturedMobs;
this.settings = settings; this.settings = settings;
this.server = server; this.server = server;
this.nextTick = nextTick;
} }
@EventHandler(priority = EventPriority.HIGH) @EventHandler(priority = EventPriority.HIGH)
@@ -162,15 +166,82 @@ public final class TamerListener implements Listener {
@EventHandler @EventHandler
public void onInventoryClick(InventoryClickEvent event) { public void onInventoryClick(InventoryClickEvent event) {
Player player = event.getWhoClicked() instanceof Player clicked ? clicked : null;
ItemStack swapSource = null;
if (player != null) {
if (event.getHotbarButton() >= 0 && event.getHotbarButton() < 9) {
swapSource = player.getInventory().getItem(event.getHotbarButton());
} else if (event.getClick() == org.bukkit.event.inventory.ClickType.SWAP_OFFHAND) {
swapSource = player.getInventory().getItemInOffHand();
}
}
if (capturedItems.isCapturedMob(event.getCurrentItem()) if (capturedItems.isCapturedMob(event.getCurrentItem())
|| capturedItems.isCapturedMob(event.getCursor())) { || capturedItems.isCapturedMob(event.getCursor()) || capturedItems.isCapturedMob(swapSource)) {
if (!(event.getWhoClicked() instanceof Player player) var action = event.getAction();
|| event.isShiftClick() if (player == null || event.isCancelled() || event.isShiftClick()
|| action == org.bukkit.event.inventory.InventoryAction.MOVE_TO_OTHER_INVENTORY
|| action == org.bukkit.event.inventory.InventoryAction.CLONE_STACK
|| action == org.bukkit.event.inventory.InventoryAction.COLLECT_TO_CURSOR
|| action == org.bukkit.event.inventory.InventoryAction.UNKNOWN
|| event.getClickedInventory() == null || event.getClickedInventory() == null
|| !event.getClickedInventory().equals(player.getInventory())) { || !event.getClickedInventory().equals(player.getInventory())
|| !ownedBy(player, event.getCurrentItem()) || !ownedBy(player, event.getCursor())
|| !ownedBy(player, swapSource)) {
event.setCancelled(true);
if (player != null) {
refreshInventory(player);
}
}
}
}
@EventHandler
public void onInventoryDrag(org.bukkit.event.inventory.InventoryDragEvent event) {
if (!capturedItems.isCapturedMob(event.getOldCursor())
&& event.getNewItems().values().stream().noneMatch(capturedItems::isCapturedMob)) {
return;
}
Player player = event.getWhoClicked() instanceof Player dragged ? dragged : null;
if (player == null || event.isCancelled() || !ownedBy(player, event.getOldCursor())
|| event.getNewItems().values().stream().anyMatch(item -> !ownedBy(player, item))
|| event.getRawSlots().stream().anyMatch(raw ->
!player.getInventory().equals(event.getView().getInventory(raw)))) {
event.setCancelled(true);
if (player != null) {
refreshInventory(player);
}
}
}
@EventHandler
public void onInventoryMove(org.bukkit.event.inventory.InventoryMoveItemEvent event) {
if (capturedItems.isCapturedMob(event.getItem())) {
event.setCancelled(true); event.setCancelled(true);
} }
} }
private boolean ownedBy(Player player, ItemStack item) {
return !capturedItems.isCapturedMob(item)
|| capturedItems.owner(item).filter(player.getUniqueId()::equals).isPresent();
}
private void refreshInventory(Player player) {
UUID id = player.getUniqueId();
if (!pendingInventoryRefresh.add(id)) {
return;
}
// The native click transaction must finish before forcing its authoritative contents to the client.
try {
nextTick.accept(() -> {
pendingInventoryRefresh.remove(id);
if (player.isOnline()) {
player.updateInventory();
}
});
} catch (RuntimeException exception) {
pendingInventoryRefresh.remove(id);
throw exception;
}
} }
@EventHandler @EventHandler
@@ -0,0 +1,358 @@
package games.dmg.spigottyrant;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.*;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicReference;
import java.util.logging.Logger;
import org.bukkit.NamespacedKey;
import org.bukkit.Server;
import org.bukkit.entity.Player;
import org.bukkit.event.inventory.ClickType;
import org.bukkit.event.inventory.InventoryAction;
import org.bukkit.event.inventory.InventoryClickEvent;
import org.bukkit.event.inventory.InventoryType;
import org.bukkit.inventory.Inventory;
import org.bukkit.inventory.InventoryView;
import org.bukkit.inventory.ItemStack;
import org.bukkit.inventory.PlayerInventory;
import org.bukkit.inventory.meta.ItemMeta;
import org.bukkit.persistence.PersistentDataContainer;
import org.bukkit.persistence.PersistentDataType;
import org.bukkit.plugin.Plugin;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
class CapturedMobInventorySynchronizationTest {
@TempDir Path directory;
private final UUID owner = UUID.randomUUID(), captureId = UUID.randomUUID();
private final Player player = mock(Player.class);
private final PlayerInventory inventory = mock(PlayerInventory.class);
private final Inventory top = mock(Inventory.class);
private final InventoryView view = mock(InventoryView.class);
private final Server server = mock(Server.class);
private final ItemStack[] contents = new ItemStack[41];
private final AtomicReference<ItemStack> cursor = new AtomicReference<>();
private final List<Runnable> scheduled = new ArrayList<>();
private TyrantStateManager manager;
private CapturedMobItemService items;
private TamerListener listener;
private ItemStack egg;
private Path stateFile;
@BeforeEach
void setup() throws Exception {
when(player.getUniqueId()).thenReturn(owner);
when(player.getName()).thenReturn("Tamer");
when(player.isOnline()).thenReturn(true);
when(player.getInventory()).thenReturn(inventory);
when(player.getItemOnCursor()).thenAnswer(call -> cursor.get());
doAnswer(call -> { cursor.set(call.getArgument(0)); return null; })
.when(player).setItemOnCursor(nullable(ItemStack.class));
when(inventory.getContents()).thenAnswer(call -> contents.clone());
when(inventory.getItem(anyInt())).thenAnswer(call -> contents[call.getArgument(0, Integer.class)]);
doAnswer(call -> { contents[call.getArgument(0, Integer.class)] = call.getArgument(1); return null; })
.when(inventory).setItem(anyInt(), nullable(ItemStack.class));
when(view.getPlayer()).thenReturn(player);
when(view.getBottomInventory()).thenReturn(inventory);
when(view.getTopInventory()).thenReturn(top);
when(top.getSize()).thenReturn(27);
when(top.getContents()).thenReturn(new ItemStack[27]);
when(view.getInventory(anyInt())).thenAnswer(call -> {
int raw = call.getArgument(0);
return raw < 0 || raw >= 68 ? null : raw < 27 ? top : inventory;
});
when(view.convertSlot(anyInt())).thenAnswer(call -> call.getArgument(0, Integer.class) - 27);
when(view.getItem(anyInt())).thenAnswer(call -> {
int raw = call.getArgument(0);
return raw >= 27 && raw < 68 ? contents[raw - 27] : null;
});
when(view.getCursor()).thenAnswer(call -> cursor.get());
Plugin plugin = mock(Plugin.class);
when(plugin.getName()).thenReturn("SpigotTyrant");
items = new BukkitCapturedMobItemService(plugin, PluginSettings.from(Map.of()));
egg = egg(owner, captureId);
contents[3] = egg;
stateFile = directory.resolve("state.yml");
manager = new TyrantStateManager(new YamlTyrantStateRepository(stateFile), Logger.getAnonymousLogger());
var mob = new CapturedMob("COW", Map.of("capture-id", captureId.toString(), "snapshot", "stored-cow"));
var state = new PlayerState(owner, "Tamer", Optional.empty(), Optional.empty(), TyrantClass.TAMER,
Optional.empty(), Map.of(), Set.of(), List.of(mob));
manager.updatePlayer(owner, "Tamer", ignored -> state);
assertTrue(manager.saveIfDirty());
listener = new TamerListener(manager, mock(AbilityItemService.class), items,
new CapturedMobService(), PluginSettings.from(Map.of()), server, scheduled::add);
}
@Test
void rejectedShiftKeepsAuthoritativeEggAndCustodyAndResendsAfterTransaction() throws Exception {
var before = manager.snapshot();
byte[] bytes = Files.readAllBytes(stateFile);
var event = click(30, ClickType.SHIFT_LEFT, InventoryAction.MOVE_TO_OTHER_INVENTORY, -1);
listener.onInventoryClick(event);
assertTrue(event.isCancelled());
assertSame(egg, contents[3]);
assertTrue(java.util.Arrays.stream(top.getContents()).allMatch(java.util.Objects::isNull));
verify(player, never()).updateInventory();
assertEquals(1, scheduled.size(), "A rejected transfer needs a post-transaction full inventory refresh");
scheduled.remove(0).run();
verify(player).updateInventory();
assertEquals(before, manager.snapshot());
assertArrayEquals(bytes, Files.readAllBytes(stateFile));
assertEquals(1, new YamlTyrantStateRepository(stateFile).load().players().get(owner).capturedMobs().size());
verify(server, never()).getEntityFactory();
verify(inventory, never()).setItem(anyInt(), nullable(ItemStack.class));
}
@Test
void repeatedRejectedTransfersCoalesceRefreshWithoutChangingEggOrCustody() throws Exception {
var before = manager.snapshot();
byte[] bytes = Files.readAllBytes(stateFile);
for (int tick = 0; tick < 3; tick++) {
for (int attempt = 0; attempt < 5; attempt++) {
var event = click(30, ClickType.SHIFT_LEFT, InventoryAction.MOVE_TO_OTHER_INVENTORY, -1);
listener.onInventoryClick(event);
assertTrue(event.isCancelled());
assertSame(egg, contents[3]);
}
assertEquals(1, scheduled.size(), "Repeated rejects in one tick need one authoritative refresh");
scheduled.remove(0).run();
}
verify(player, times(3)).updateInventory();
assertEquals(before, manager.snapshot());
assertArrayEquals(bytes, Files.readAllBytes(stateFile));
assertEquals(1, egg.getAmount());
assertTrue(java.util.Arrays.stream(top.getContents()).allMatch(java.util.Objects::isNull));
}
@Test
void logoutBeforeRefreshDoesNotMutateOrReissueAnything() {
var before = manager.snapshot();
listener.onInventoryClick(click(30, ClickType.SHIFT_LEFT, InventoryAction.MOVE_TO_OTHER_INVENTORY, -1));
when(player.isOnline()).thenReturn(false);
scheduled.remove(0).run();
verify(player, never()).updateInventory();
assertSame(egg, contents[3]);
assertEquals(before, manager.snapshot());
}
@Test
void rejectedSchedulingDoesNotPermanentlySuppressLaterRefresh() {
int[] attempts = {0};
listener = new TamerListener(manager, mock(AbilityItemService.class), items,
new CapturedMobService(), PluginSettings.from(Map.of()), server, task -> {
if (attempts[0]++ == 0) { throw new IllegalStateException("scheduler unavailable"); }
scheduled.add(task);
});
var first = click(30, ClickType.SHIFT_LEFT, InventoryAction.MOVE_TO_OTHER_INVENTORY, -1);
assertThrows(IllegalStateException.class, () -> listener.onInventoryClick(first));
assertTrue(first.isCancelled());
listener.onInventoryClick(click(30, ClickType.SHIFT_LEFT, InventoryAction.MOVE_TO_OTHER_INVENTORY, -1));
assertEquals(1, scheduled.size());
scheduled.remove(0).run();
verify(player).updateInventory();
}
@Test
void recoveryDoesNotDuplicateAnEggHeldOnTheCursor() {
contents[3] = null;
cursor.set(egg);
when(inventory.addItem(any(ItemStack[].class))).thenAnswer(call -> {
ItemStack[] added = (ItemStack[]) call.getRawArguments()[0];
for (ItemStack item : added) {
for (int index = 0; index < contents.length; index++) {
if (contents[index] == null) { contents[index] = item; break; }
}
}
return new java.util.HashMap<Integer, ItemStack>();
});
var before = manager.snapshot();
var recovery = new CapturedMobInventoryService(items, PluginSettings.from(Map.of()));
try (var constructed = mockConstruction(ItemStack.class, (item, context) -> markEgg(item, owner, captureId))) {
for (int attempt = 0; attempt < 3; attempt++) {
recovery.reconcile(player, manager.player(owner, "Tamer"));
}
assertEquals(0, constructed.constructed().size(), "Cursor-held egg already represents the stored mob");
assertTrue(java.util.Arrays.stream(contents).allMatch(java.util.Objects::isNull));
assertSame(egg, cursor.get());
assertEquals(before, manager.snapshot());
}
}
@Test
void reconciliationKeepsCursorCopyAndRemovesOnlyDuplicateEggs() {
cursor.set(egg);
egg.setAmount(2);
contents[3] = egg(owner, captureId);
ItemStack ordinary = mock(ItemStack.class);
contents[4] = ordinary;
var before = manager.snapshot();
new CapturedMobInventoryService(items, PluginSettings.from(Map.of()))
.reconcile(player, manager.player(owner, "Tamer"));
assertSame(egg, cursor.get());
assertEquals(1, egg.getAmount());
assertNull(contents[3]);
assertSame(ordinary, contents[4]);
assertEquals(before, manager.snapshot());
}
@Test
void cursorEggIsRemovedOnlyWhenCustodyIsGoneOrOwnershipIsInvalid() {
contents[3] = null;
cursor.set(egg);
var released = new CapturedMobService().release(manager.player(owner, "Tamer"), captureId);
var recovery = new CapturedMobInventoryService(items, PluginSettings.from(Map.of()));
recovery.reconcile(player, released);
assertNull(cursor.get(), "A released mob must not leave a cursor token behind");
contents[3] = egg;
cursor.set(egg(UUID.randomUUID(), captureId));
recovery.reconcile(player, manager.player(owner, "Tamer"));
assertNull(cursor.get(), "Foreign items cannot be retained");
assertSame(egg, contents[3]);
}
@Test
void hiddenHotbarTransferIsRejectedAndResynchronized() {
contents[3] = null;
contents[2] = egg;
var event = click(0, ClickType.NUMBER_KEY, InventoryAction.HOTBAR_SWAP, 2);
listener.onInventoryClick(event);
assertTrue(event.isCancelled());
assertEquals(1, scheduled.size());
scheduled.remove(0).run();
verify(player).updateInventory();
assertSame(egg, contents[2]);
}
@Test
void hiddenOffhandTransferIsRejectedAndResynchronized() {
contents[3] = null;
contents[40] = egg;
when(inventory.getItemInOffHand()).thenReturn(egg);
var event = click(0, ClickType.SWAP_OFFHAND, InventoryAction.HOTBAR_SWAP, -1);
listener.onInventoryClick(event);
assertTrue(event.isCancelled());
assertEquals(1, scheduled.size());
}
@Test
void crossInventoryDragIsRejectedAndResynchronized() {
cursor.set(egg);
contents[3] = null;
var event = new org.bukkit.event.inventory.InventoryDragEvent(view, null, egg, false, Map.of(0, egg, 30, egg));
listener.onInventoryDrag(event);
assertTrue(event.isCancelled());
assertEquals(1, scheduled.size());
assertSame(egg, cursor.get());
}
@Test
void automatedTransferCannotMoveCapturedEggs() {
var hopper = new org.bukkit.event.inventory.InventoryMoveItemEvent(top, egg, mock(Inventory.class), true);
listener.onInventoryMove(hopper);
assertTrue(hopper.isCancelled());
assertTrue(scheduled.isEmpty(), "Automated moves have no predicting player to refresh");
}
@Test
void cloningCannotCreateAdditionalEggCopies() {
var clone = click(30, ClickType.MIDDLE, InventoryAction.CLONE_STACK, -1);
listener.onInventoryClick(clone);
assertTrue(clone.isCancelled());
assertEquals(1, scheduled.size());
}
@Test
void ordinaryItemsRemainUnaffectedWithoutRefreshTraffic() {
ItemStack ordinary = mock(ItemStack.class);
when(ordinary.clone()).thenReturn(ordinary);
contents[3] = ordinary;
var shift = click(30, ClickType.SHIFT_LEFT, InventoryAction.MOVE_TO_OTHER_INVENTORY, -1);
listener.onInventoryClick(shift);
assertFalse(shift.isCancelled());
var drag = new org.bukkit.event.inventory.InventoryDragEvent(view, null, ordinary, false, Map.of(0, ordinary));
listener.onInventoryDrag(drag);
assertFalse(drag.isCancelled());
var hopper = new org.bukkit.event.inventory.InventoryMoveItemEvent(top, ordinary, mock(Inventory.class), true);
listener.onInventoryMove(hopper);
assertFalse(hopper.isCancelled());
assertTrue(scheduled.isEmpty());
}
@Test
void safeInternalCursorMovementDoesNotTriggerRejectionOrRefresh() {
var pickup = click(30, ClickType.LEFT, InventoryAction.PICKUP_ALL, -1);
listener.onInventoryClick(pickup);
assertFalse(pickup.isCancelled());
contents[3] = null;
cursor.set(egg);
var place = click(31, ClickType.LEFT, InventoryAction.PLACE_ALL, -1);
listener.onInventoryClick(place);
assertFalse(place.isCancelled());
var drag = new org.bukkit.event.inventory.InventoryDragEvent(view, null, egg, false, Map.of(31, egg));
listener.onInventoryDrag(drag);
assertFalse(drag.isCancelled());
assertTrue(scheduled.isEmpty());
new CapturedMobInventoryService(items, PluginSettings.from(Map.of()))
.reconcile(player, manager.player(owner, "Tamer"));
assertSame(egg, cursor.get());
verify(inventory, never()).addItem(any(ItemStack[].class));
}
@Test
void cursorTransferAndExistingCancellationPreserveCustodyAndResynchronize() throws Exception {
var before = manager.snapshot();
byte[] bytes = Files.readAllBytes(stateFile);
contents[3] = null;
cursor.set(egg);
var external = click(0, ClickType.LEFT, InventoryAction.PLACE_ALL, -1);
listener.onInventoryClick(external);
assertTrue(external.isCancelled());
var cancelled = click(31, ClickType.LEFT, InventoryAction.PLACE_ALL, -1);
cancelled.setCancelled(true);
listener.onInventoryClick(cancelled);
assertTrue(cancelled.isCancelled());
assertEquals(1, scheduled.size());
scheduled.remove(0).run();
assertSame(egg, cursor.get());
assertEquals(before, manager.snapshot());
assertArrayEquals(bytes, Files.readAllBytes(stateFile));
verify(player).updateInventory();
verify(top, never()).setItem(anyInt(), nullable(ItemStack.class));
}
private InventoryClickEvent click(int raw, ClickType click, InventoryAction action, int button) {
return new InventoryClickEvent(view, InventoryType.SlotType.CONTAINER, raw, click, action, button);
}
private ItemStack egg(UUID id, UUID captured) {
ItemStack item = mock(ItemStack.class);
markEgg(item, id, captured);
return item;
}
private void markEgg(ItemStack item, UUID id, UUID captured) {
ItemMeta meta = mock(ItemMeta.class);
PersistentDataContainer data = mock(PersistentDataContainer.class);
when(item.clone()).thenReturn(item);
when(item.hasItemMeta()).thenReturn(true);
when(item.getItemMeta()).thenReturn(meta);
var amount = new java.util.concurrent.atomic.AtomicInteger(1);
when(item.getAmount()).thenAnswer(call -> amount.get());
doAnswer(call -> { amount.set(call.getArgument(0)); return null; }).when(item).setAmount(anyInt());
when(meta.getPersistentDataContainer()).thenReturn(data);
when(data.get(new NamespacedKey("spigottyrant", "captured-owner"), PersistentDataType.STRING))
.thenReturn(id.toString());
when(data.get(new NamespacedKey("spigottyrant", "captured-id"), PersistentDataType.STRING))
.thenReturn(captured.toString());
}
}
@@ -49,7 +49,7 @@ final class TamerListenerTest {
CapturedMobItemService capturedItems = mock(CapturedMobItemService.class); CapturedMobItemService capturedItems = mock(CapturedMobItemService.class);
TamerListener listener = new TamerListener( TamerListener listener = new TamerListener(
manager, abilityItems, capturedItems, new CapturedMobService(), manager, abilityItems, capturedItems, new CapturedMobService(),
PluginSettings.from(Map.of()), mock(Server.class) PluginSettings.from(Map.of()), mock(Server.class), Runnable::run
); );
listener.onCapture(event); listener.onCapture(event);