From 80810b3ba15a89abb915451dca67f4b2ffa33124 Mon Sep 17 00:00:00 2001 From: Dylan Garvis Date: Fri, 21 Aug 2026 21:47:42 -0400 Subject: [PATCH] feat(tamer): limit custody to one mob --- design/log.md | 12 ++++ .../us-007-capture-and-place-mobs.md | 8 ++- .../CapturedMobCustodyRelease.java | 3 +- .../dmg/spigottyrant/CapturedMobService.java | 5 ++ .../DefaultCapturedMobCustodyRelease.java | 20 +++++- .../spigottyrant/FormerTamerReleaseTask.java | 8 ++- .../games/dmg/spigottyrant/TamerListener.java | 5 ++ .../spigottyrant/CapturedMobServiceTest.java | 15 +++++ .../DefaultCapturedMobCustodyReleaseTest.java | 22 +++++++ .../FormerTamerReleaseTaskTest.java | 26 +++++++- .../dmg/spigottyrant/TamerListenerTest.java | 66 +++++++++++++++++++ 11 files changed, 182 insertions(+), 8 deletions(-) create mode 100644 src/test/java/games/dmg/spigottyrant/TamerListenerTest.java diff --git a/design/log.md b/design/log.md index 191f1aa..0e2c60e 100644 --- a/design/log.md +++ b/design/log.md @@ -6,6 +6,18 @@ description: Chronological record of material decisions affecting the Spigot Tyr # Spigot Tyrant Design Log +## 2026-08-21 — Single-mob Tamer custody completed + +- Completed US-007 with an authoritative one-mob custody limit that rejects a second capture before inventory, entity, or state mutation and tells the Tamer to release the held mob first. +- Releasing or dropping custody permits another capture immediately, while durable state preserves the limit across logout, restart, recovery, and repeated interaction. +- Legacy excess custody retains the oldest mob and safely releases extras; offline or failed releases remain durable for periodic retry. +- Verified domain rejection, listener ordering, legacy reconciliation, compiler warnings, tests, and packaging with `./gradlew clean check jar`. + +## 2026-08-21 — Single-mob Tamer custody started + +- US-007 begins a test-first one-mob custody limit for active Tamers. +- New captures will be rejected before mutation while custody exists, and legacy excess custody will be released safely with failed releases retained for retry. + ## 2026-08-21 — Former Tamer automatic release completed - Completed US-004 and US-007 so every captured mob is released automatically after its holder loses the Tamer class, regardless of the class-removal path. diff --git a/design/user-stories/us-007-capture-and-place-mobs.md b/design/user-stories/us-007-capture-and-place-mobs.md index 0320f9e..c1504b4 100644 --- a/design/user-stories/us-007-capture-and-place-mobs.md +++ b/design/user-stories/us-007-capture-and-place-mobs.md @@ -28,10 +28,16 @@ As the **Tamer**, I want to capture a mob and release it elsewhere so that I can - [x] If the former Tamer is offline or no safe release location is available, release is deferred without losing the captured mob. - [x] A deferred release is retried when the former Tamer next logs in or reaches a safe location. - [x] Tyrant death, class reassignment, opt-out, administration, and other Tamer-removal paths use the same release behavior. +- [x] A Tamer can hold custody of at most one captured mob at a time. +- [x] Attempting to capture another mob while one is held is rejected before changing either mob. +- [x] The Tamer is told to release the currently captured mob before capturing another. +- [x] Releasing or dropping the held mob immediately allows another capture. +- [x] The one-mob limit survives logout, restart, item recovery, and repeated or concurrent interaction. +- [x] If legacy state contains multiple captured mobs, extras are safely released when possible; custody is retained until release succeeds so no mob is lost. ## Validation -Automated tests verify that only former Tamers are processed, offline custody remains deferred, successful releases remove only their corresponding custody records, and failed releases remain available for retry. The complete `./gradlew clean check jar` lifecycle passes. +Automated tests verify that only former Tamers are processed, offline custody remains deferred, successful releases remove only their corresponding custody records, and failed releases remain available for retry. Additional tests verify pre-mutation rejection of a second capture, clear player guidance, and safe reduction of legacy excess custody. The complete `./gradlew clean check jar` lifecycle passes. ## Related diff --git a/src/main/java/games/dmg/spigottyrant/CapturedMobCustodyRelease.java b/src/main/java/games/dmg/spigottyrant/CapturedMobCustodyRelease.java index 013c085..ef80e25 100644 --- a/src/main/java/games/dmg/spigottyrant/CapturedMobCustodyRelease.java +++ b/src/main/java/games/dmg/spigottyrant/CapturedMobCustodyRelease.java @@ -2,7 +2,8 @@ package games.dmg.spigottyrant; import org.bukkit.entity.Player; -@FunctionalInterface public interface CapturedMobCustodyRelease { PlayerState releaseAll(Player player, PlayerState state); + + PlayerState releaseExcess(Player player, PlayerState state, int custodyLimit); } diff --git a/src/main/java/games/dmg/spigottyrant/CapturedMobService.java b/src/main/java/games/dmg/spigottyrant/CapturedMobService.java index 2eae524..a3c6dfa 100644 --- a/src/main/java/games/dmg/spigottyrant/CapturedMobService.java +++ b/src/main/java/games/dmg/spigottyrant/CapturedMobService.java @@ -16,6 +16,11 @@ public final class CapturedMobService { if (player.tyrantClass() != TyrantClass.TAMER) { throw new IllegalStateException("only a Tamer can capture mobs"); } + if (!player.capturedMobs().isEmpty()) { + throw new IllegalStateException( + "release the currently captured mob before capturing another" + ); + } if ("ENDER_DRAGON".equalsIgnoreCase(entityType)) { throw new IllegalArgumentException("Ender Dragons cannot be captured"); } diff --git a/src/main/java/games/dmg/spigottyrant/DefaultCapturedMobCustodyRelease.java b/src/main/java/games/dmg/spigottyrant/DefaultCapturedMobCustodyRelease.java index 76967a5..09395b0 100644 --- a/src/main/java/games/dmg/spigottyrant/DefaultCapturedMobCustodyRelease.java +++ b/src/main/java/games/dmg/spigottyrant/DefaultCapturedMobCustodyRelease.java @@ -13,13 +13,27 @@ public final class DefaultCapturedMobCustodyRelease implements CapturedMobCustod @Override public PlayerState releaseAll(Player player, PlayerState state) { + return releaseExcess(player, state, 0); + } + + @Override + public PlayerState releaseExcess( + Player player, + PlayerState state, + int custodyLimit + ) { + if (custodyLimit < 0) { + throw new IllegalArgumentException("custodyLimit must not be negative"); + } List retained = new ArrayList<>(); - for (CapturedMob mob : state.capturedMobs()) { - if (!spawner.spawn(player, mob)) { + List custody = state.capturedMobs(); + for (int index = 0; index < custody.size(); index++) { + CapturedMob mob = custody.get(index); + if (index < custodyLimit || !spawner.spawn(player, mob)) { retained.add(mob); } } - if (retained.size() == state.capturedMobs().size()) { + if (retained.equals(custody)) { return state; } return new PlayerState( diff --git a/src/main/java/games/dmg/spigottyrant/FormerTamerReleaseTask.java b/src/main/java/games/dmg/spigottyrant/FormerTamerReleaseTask.java index 527d994..6fe4612 100644 --- a/src/main/java/games/dmg/spigottyrant/FormerTamerReleaseTask.java +++ b/src/main/java/games/dmg/spigottyrant/FormerTamerReleaseTask.java @@ -37,14 +37,18 @@ public final class FormerTamerReleaseTask implements Runnable { Map online = server.getOnlinePlayers().stream() .collect(Collectors.toMap(Player::getUniqueId, player -> player)); for (PlayerState state : stateManager.players().values()) { - if (state.tyrantClass() == TyrantClass.TAMER || state.capturedMobs().isEmpty()) { + if (state.capturedMobs().isEmpty() + || state.tyrantClass() == TyrantClass.TAMER + && state.capturedMobs().size() == 1) { continue; } Player player = online.get(state.playerId()); if (player == null || player.isDead()) { continue; } - PlayerState released = release.releaseAll(player, state); + PlayerState released = state.tyrantClass() == TyrantClass.TAMER + ? release.releaseExcess(player, state, 1) + : release.releaseAll(player, state); if (released.equals(state)) { continue; } diff --git a/src/main/java/games/dmg/spigottyrant/TamerListener.java b/src/main/java/games/dmg/spigottyrant/TamerListener.java index eeb573c..1f4bd5a 100644 --- a/src/main/java/games/dmg/spigottyrant/TamerListener.java +++ b/src/main/java/games/dmg/spigottyrant/TamerListener.java @@ -63,6 +63,11 @@ public final class TamerListener implements Listener { return; } PlayerState state = stateManager.player(player.getUniqueId(), player.getName()); + if (!state.capturedMobs().isEmpty()) { + player.sendMessage(ChatColor.RED + + "Release the currently captured mob before capturing another."); + return; + } Entity target = event.getRightClicked(); if (!canCapture(state, target)) { player.sendMessage(ChatColor.RED + "That mob cannot be captured."); diff --git a/src/test/java/games/dmg/spigottyrant/CapturedMobServiceTest.java b/src/test/java/games/dmg/spigottyrant/CapturedMobServiceTest.java index e41c5ca..59704ff 100644 --- a/src/test/java/games/dmg/spigottyrant/CapturedMobServiceTest.java +++ b/src/test/java/games/dmg/spigottyrant/CapturedMobServiceTest.java @@ -28,6 +28,21 @@ final class CapturedMobServiceTest { assertEquals(java.util.List.of(), released.capturedMobs()); } + @Test + void tamerMustReleaseHeldMobBeforeCapturingAnother() { + PlayerState holding = service.capture( + tamer(), UUID.randomUUID(), "COW", "cow-snapshot" + ); + + IllegalStateException exception = assertThrows(IllegalStateException.class, () -> + service.capture(holding, UUID.randomUUID(), "SHEEP", "sheep-snapshot") + ); + + assertEquals("release the currently captured mob before capturing another", + exception.getMessage()); + assertEquals(1, holding.capturedMobs().size()); + } + @Test void enderDragonCanNeverBeCapturedAndMissingReleaseDoesNotMutate() { assertThrows(IllegalArgumentException.class, () -> service.capture( diff --git a/src/test/java/games/dmg/spigottyrant/DefaultCapturedMobCustodyReleaseTest.java b/src/test/java/games/dmg/spigottyrant/DefaultCapturedMobCustodyReleaseTest.java index 371ad29..ddeb7b7 100644 --- a/src/test/java/games/dmg/spigottyrant/DefaultCapturedMobCustodyReleaseTest.java +++ b/src/test/java/games/dmg/spigottyrant/DefaultCapturedMobCustodyReleaseTest.java @@ -12,6 +12,28 @@ import org.bukkit.entity.Player; import org.junit.jupiter.api.Test; final class DefaultCapturedMobCustodyReleaseTest { + @Test + void legacyExcessReleaseRetainsOneAndAnyFailedExtra() { + UUID playerId = UUID.fromString("33333333-3333-3333-3333-333333333333"); + CapturedMob retained = mob("COW"); + CapturedMob releasedExtra = mob("SHEEP"); + CapturedMob deferredExtra = mob("PIG"); + PlayerState state = new PlayerState( + playerId, "Tamer", Optional.empty(), Optional.empty(), TyrantClass.TAMER, + Optional.empty(), Map.of(), java.util.Set.of(), + List.of(retained, releasedExtra, deferredExtra) + ); + CapturedMobSpawner spawner = mock(CapturedMobSpawner.class); + Player player = mock(Player.class); + when(spawner.spawn(player, releasedExtra)).thenReturn(true); + when(spawner.spawn(player, deferredExtra)).thenReturn(false); + DefaultCapturedMobCustodyRelease service = new DefaultCapturedMobCustodyRelease(spawner); + + PlayerState result = service.releaseExcess(player, state, 1); + + assertEquals(List.of(retained, deferredExtra), result.capturedMobs()); + } + @Test void removesOnlyCustodyThatSpawnedSuccessfully() { UUID playerId = UUID.fromString("33333333-3333-3333-3333-333333333333"); diff --git a/src/test/java/games/dmg/spigottyrant/FormerTamerReleaseTaskTest.java b/src/test/java/games/dmg/spigottyrant/FormerTamerReleaseTaskTest.java index acba306..ae31d1f 100644 --- a/src/test/java/games/dmg/spigottyrant/FormerTamerReleaseTaskTest.java +++ b/src/test/java/games/dmg/spigottyrant/FormerTamerReleaseTaskTest.java @@ -31,6 +31,28 @@ final class FormerTamerReleaseTaskTest { verify(release, never()).releaseAll(any(), any()); } + @Test + void safelyReleasesLegacyCustodyAboveOneForActiveTamer() { + UUID tamerId = UUID.fromString("44444444-4444-4444-4444-444444444444"); + PlayerState once = withMob(withClass( + PlayerState.newPlayer(tamerId, "Tamer"), TyrantClass.TAMER + )); + PlayerState legacy = withMob(once); + TyrantStateManager manager = mock(TyrantStateManager.class); + when(manager.players()).thenReturn(Map.of(tamerId, legacy)); + Player tamer = mock(Player.class); + when(tamer.getUniqueId()).thenReturn(tamerId); + Server server = mock(Server.class); + doReturn(Set.of(tamer)).when(server).getOnlinePlayers(); + CapturedMobCustodyRelease release = mock(CapturedMobCustodyRelease.class); + when(release.releaseExcess(tamer, legacy, 1)).thenReturn(legacy); + + new FormerTamerReleaseTask(manager, server, release).run(); + + verify(release).releaseExcess(tamer, legacy, 1); + verify(release, never()).releaseAll(any(), any()); + } + @Test void releasesCustodyOnlyAfterPlayerLosesTamerClass() { UUID formerId = UUID.fromString("33333333-3333-3333-3333-333333333333"); @@ -61,10 +83,12 @@ final class FormerTamerReleaseTaskTest { CapturedMob mob = new CapturedMob("COW", Map.of( "capture-id", UUID.randomUUID().toString(), "snapshot", "snapshot" )); + java.util.List custody = new java.util.ArrayList<>(player.capturedMobs()); + custody.add(mob); return new PlayerState( player.playerId(), player.latestName(), player.lastLogin(), player.optedOutUntil(), player.tyrantClass(), player.followerOf(), player.cooldownEnds(), - player.readyAbilityItems(), java.util.List.of(mob) + player.readyAbilityItems(), custody ); } diff --git a/src/test/java/games/dmg/spigottyrant/TamerListenerTest.java b/src/test/java/games/dmg/spigottyrant/TamerListenerTest.java new file mode 100644 index 0000000..849530f --- /dev/null +++ b/src/test/java/games/dmg/spigottyrant/TamerListenerTest.java @@ -0,0 +1,66 @@ +package games.dmg.spigottyrant; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.UUID; +import org.bukkit.Server; +import org.bukkit.entity.Entity; +import org.bukkit.entity.Player; +import org.bukkit.event.player.PlayerInteractEntityEvent; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.PlayerInventory; +import org.junit.jupiter.api.Test; + +final class TamerListenerTest { + @Test + void heldMobRejectsAnotherCaptureBeforeTargetOrInventoryChanges() { + UUID playerId = UUID.fromString("33333333-3333-3333-3333-333333333333"); + CapturedMob heldMob = new CapturedMob("COW", Map.of( + "capture-id", UUID.randomUUID().toString(), "snapshot", "cow" + )); + PlayerState state = new PlayerState( + playerId, "Tamer", Optional.empty(), Optional.empty(), TyrantClass.TAMER, + Optional.empty(), Map.of(), Set.of(Ability.TAMER_CAPTURE), + java.util.List.of(heldMob) + ); + Player player = mock(Player.class); + when(player.getUniqueId()).thenReturn(playerId); + when(player.getName()).thenReturn("Tamer"); + PlayerInventory inventory = mock(PlayerInventory.class); + when(player.getInventory()).thenReturn(inventory); + ItemStack lead = mock(ItemStack.class); + when(inventory.getItem(org.bukkit.inventory.EquipmentSlot.HAND)).thenReturn(lead); + Entity target = mock(Entity.class); + PlayerInteractEntityEvent event = mock(PlayerInteractEntityEvent.class); + when(event.getPlayer()).thenReturn(player); + when(event.getHand()).thenReturn(org.bukkit.inventory.EquipmentSlot.HAND); + when(event.getRightClicked()).thenReturn(target); + AbilityItemService abilityItems = mock(AbilityItemService.class); + when(abilityItems.ability(lead)).thenReturn(Optional.of(Ability.TAMER_CAPTURE)); + when(abilityItems.owner(lead)).thenReturn(Optional.of(playerId)); + TyrantStateManager manager = mock(TyrantStateManager.class); + when(manager.player(playerId, "Tamer")).thenReturn(state); + CapturedMobItemService capturedItems = mock(CapturedMobItemService.class); + TamerListener listener = new TamerListener( + manager, abilityItems, capturedItems, new CapturedMobService(), + PluginSettings.from(Map.of()), mock(Server.class) + ); + + listener.onCapture(event); + + verify(player).sendMessage(org.mockito.ArgumentMatchers.contains( + "Release the currently captured mob" + )); + verify(target, never()).createSnapshot(); + verify(capturedItems, never()).create( + org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any(), + org.mockito.ArgumentMatchers.anyString() + ); + } +}