From 17a5c665752454948becfb38bd2b7d2ab98b7aab Mon Sep 17 00:00:00 2001 From: Dylan Garvis Date: Fri, 4 Sep 2026 22:01:52 -0400 Subject: [PATCH] fix(arena): move winners outside after victory --- design/log.md | 6 +++ .../us-019-claim-vigilante-in-arena.md | 1 + .../us-020-claim-tyrant-in-arena.md | 1 + .../dmg/spigottyrant/RoleArenaController.java | 26 ++++++++- .../spigottyrant/RoleArenaControllerTest.java | 53 +++++++++++++++++++ 5 files changed, 85 insertions(+), 2 deletions(-) diff --git a/design/log.md b/design/log.md index 4953ac2..78fa05e 100644 --- a/design/log.md +++ b/design/log.md @@ -6,6 +6,12 @@ description: Chronological record of material decisions affecting the Spigot Tyr # Spigot Tyrant Design Log +## 2026-09-04 — Arena winners safely extracted + +- Corrected shared-role arena completion so a successful Tyrant or Vigilante challenger is moved safely outside immediately after assignment and barrier removal. +- Prevented the next challenge's containment enforcement from treating the previous winner as an unauthorized entrant inside the arena. +- Verified both role-victory paths, 121 automated tests, compiler warnings, and packaging with `./gradlew clean check jar`. + ## 2026-08-23 — Arena containment and idle boss corrected - Corrected shared-arena containment with a validated 12-block barrier wall extending below the arena floor and active-fight block placement and breaking protection. diff --git a/design/user-stories/us-019-claim-vigilante-in-arena.md b/design/user-stories/us-019-claim-vigilante-in-arena.md index 5d2b23c..c2d56a0 100644 --- a/design/user-stories/us-019-claim-vigilante-in-arena.md +++ b/design/user-stories/us-019-claim-vigilante-in-arena.md @@ -26,6 +26,7 @@ As an **opted-in participant**, I want a visible one-player boss challenge for a - [x] Challenger death, disconnect, or leaving by any mechanism ends the fight and resets the boss to full health at its spawn; death counts as leaving and the same player may retry immediately. - [x] Block placement and breaking are denied inside the arena during an active fight so players cannot build over or modify the boundary. - [x] Killing the boss assigns the challenger as Vigilante, removes the boss and barriers, and restores the passive particle boundary. +- [x] After winning, the new Vigilante is moved safely outside the arena before normal arena enforcement resumes. - [x] Pausing, resetting, restarting, reloading, and repeated or concurrent events cannot duplicate the boss, barriers, challenger, or Vigilante assignment. - [x] If the arena is not configured when a challenge should open, the Vigilante remains vacant and administrators receive a clear warning. diff --git a/design/user-stories/us-020-claim-tyrant-in-arena.md b/design/user-stories/us-020-claim-tyrant-in-arena.md index d392bf0..aa5b875 100644 --- a/design/user-stories/us-020-claim-tyrant-in-arena.md +++ b/design/user-stories/us-020-claim-tyrant-in-arena.md @@ -18,6 +18,7 @@ As an **opted-in participant**, I want to earn a vacant Tyrant role through the - [x] The Tyrant challenge uses the existing configured arena, boss balance, tall visible one-challenger barrier, block-change protection, idle-center boss behavior, containment, reset, notification, and missing-location rules. - [x] An eligible opted-in participant, including the former Tyrant, may challenge for the vacant Tyrant role; an active Tyrant cannot enter any arena fight. - [x] Defeating the Tyrant challenge boss assigns the challenger as Tyrant with level zero, no purchases, and exactly one starting unlock choice. +- [x] After winning, the new Tyrant is moved safely outside before any subsequent Vigilante challenge opens. - [x] Completing the Tyrant challenge immediately permits the blue Vigilante challenge to open at the same location when the Vigilante role is vacant. - [x] Legacy pending Tyrant replacement state is cleared into an arena vacancy without granting a role. - [x] Pausing, resetting, restarting, reloading, and repeated or concurrent events cannot duplicate either role boss, challenger, barrier, or assignment. diff --git a/src/main/java/games/dmg/spigottyrant/RoleArenaController.java b/src/main/java/games/dmg/spigottyrant/RoleArenaController.java index dc4a7e5..10e4c88 100644 --- a/src/main/java/games/dmg/spigottyrant/RoleArenaController.java +++ b/src/main/java/games/dmg/spigottyrant/RoleArenaController.java @@ -400,7 +400,7 @@ public final class RoleArenaController implements Listener, Runnable { && !zombie.isDead() ? zombie : null; } - private void assignRole(UUID playerId, ArenaRole role) { + void assignRole(UUID playerId, ArenaRole role) { LifecycleState assigned; try { assigned = role == ArenaRole.TYRANT @@ -414,13 +414,35 @@ public final class RoleArenaController implements Listener, Runnable { stateManager.saveIfDirty(); challengerId = null; barrier.clear(); - String name = Optional.ofNullable(server.getPlayer(playerId)) + Player winner = server.getPlayer(playerId); + moveWinnerOutside(winner); + String name = Optional.ofNullable(winner) .map(Player::getName).orElse(playerId.toString()); String message = ChatColor.YELLOW + name + " defeated the arena boss and is the " + readable(role) + "!"; server.getOnlinePlayers().forEach(player -> player.sendMessage(message)); } + private void moveWinnerOutside(Player winner) { + Optional configured = locations.location(); + if (winner == null || configured.isEmpty()) { + return; + } + ArenaLocation arena = configured.orElseThrow(); + Location winnerLocation = winner.getLocation(); + if (!contains(arena, winnerLocation)) { + return; + } + relocating.add(winner.getUniqueId()); + try { + winner.teleport( + outside(arena, winnerLocation), PlayerTeleportEvent.TeleportCause.PLUGIN + ); + } finally { + relocating.remove(winner.getUniqueId()); + } + } + private void announceVacancy(ArenaLocation arena, ArenaRole role) { String message = vacancyMessage(arena, role); for (Player player : server.getOnlinePlayers()) { diff --git a/src/test/java/games/dmg/spigottyrant/RoleArenaControllerTest.java b/src/test/java/games/dmg/spigottyrant/RoleArenaControllerTest.java index e609856..2ce39d1 100644 --- a/src/test/java/games/dmg/spigottyrant/RoleArenaControllerTest.java +++ b/src/test/java/games/dmg/spigottyrant/RoleArenaControllerTest.java @@ -4,6 +4,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.mockito.ArgumentMatchers.contains; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -16,7 +19,10 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; import org.bukkit.Color; +import org.bukkit.Location; import org.bukkit.Server; +import org.bukkit.World; +import org.bukkit.block.Block; import org.bukkit.entity.Player; import org.bukkit.event.player.PlayerJoinEvent; import org.junit.jupiter.api.Test; @@ -38,6 +44,16 @@ final class RoleArenaControllerTest { ); } + @Test + void tyrantArenaWinnerIsMovedOutsideAfterAssignment() { + assertWinnerMovedOutside(ArenaRole.TYRANT, tyrantVacancy()); + } + + @Test + void vigilanteArenaWinnerIsMovedOutsideAfterAssignment() { + assertWinnerMovedOutside(ArenaRole.VIGILANTE, openGame()); + } + @Test void optedInPlayerJoiningDuringVacancyReceivesArenaCoordinates() { Player player = mock(Player.class); @@ -102,6 +118,43 @@ final class RoleArenaControllerTest { verify(player, never()).sendMessage(org.mockito.ArgumentMatchers.anyString()); } + private static void assertWinnerMovedOutside(ArenaRole role, GameState game) { + ArenaLocation arena = new ArenaLocation("world", 10.0, 64.0, -5.0, 0.0F, 0.0F); + World world = mock(World.class); + when(world.getName()).thenReturn("world"); + when(world.getMinHeight()).thenReturn(-64); + when(world.getMaxHeight()).thenReturn(320); + when(world.getHighestBlockYAt(org.mockito.ArgumentMatchers.anyInt(), + org.mockito.ArgumentMatchers.anyInt())).thenReturn(63); + when(world.getBlockAt(org.mockito.ArgumentMatchers.anyInt(), + org.mockito.ArgumentMatchers.anyInt(), + org.mockito.ArgumentMatchers.anyInt())).thenReturn(mock(Block.class)); + Player player = mock(Player.class); + when(player.getUniqueId()).thenReturn(PLAYER); + when(player.getName()).thenReturn("Player"); + when(player.getLocation()).thenReturn(new Location(world, arena.x(), arena.y(), arena.z())); + PlayerState playerState = PlayerState.newPlayer(PLAYER, "Player"); + TyrantStateManager manager = mock(TyrantStateManager.class); + when(manager.snapshot()).thenReturn(new PersistentState(game, Map.of(PLAYER, playerState))); + Server server = mock(Server.class); + when(server.getPlayer(PLAYER)).thenReturn(player); + when(server.getWorld("world")).thenReturn(world); + doReturn(Set.of(player)).when(server).getOnlinePlayers(); + ArenaLocationStore locations = mock(ArenaLocationStore.class); + when(locations.location()).thenReturn(Optional.of(arena)); + RoleArenaController controller = new RoleArenaController( + manager, server, locations, PluginSettings.from(Map.of()) + ); + + controller.assignRole(PLAYER, role); + + verify(manager).replaceState(org.mockito.ArgumentMatchers.any(LifecycleState.class)); + verify(player).teleport(argThat((Location destination) -> + !ArenaGeometry.contains(arena, 10.0, "world", + destination.getX(), destination.getY(), destination.getZ()) + ), eq(org.bukkit.event.player.PlayerTeleportEvent.TeleportCause.PLUGIN)); + } + private static PlayerJoinEvent mockJoin(Player player) { PlayerJoinEvent event = mock(PlayerJoinEvent.class); when(event.getPlayer()).thenReturn(player);