From 65c4cfda18908902fddbe2c6a60137f31656448b Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Tue, 13 Oct 2026 20:54:04 +0000 Subject: [PATCH] fix: require reviewed character ownership for legacy stations --- .../research/database/StationBackfill.java | 2 +- .../research/manager/ResearchManager.java | 17 ++--------- .../manager/MultipleStationsTest.java | 2 ++ .../research/manager/ResearchManagerTest.java | 29 ++++++++++++++----- .../research/manager/ResearchSessionTest.java | 6 ++-- 5 files changed, 29 insertions(+), 27 deletions(-) diff --git a/src/main/java/net/tfminecraft/research/database/StationBackfill.java b/src/main/java/net/tfminecraft/research/database/StationBackfill.java index f5d0516..7b398dc 100644 --- a/src/main/java/net/tfminecraft/research/database/StationBackfill.java +++ b/src/main/java/net/tfminecraft/research/database/StationBackfill.java @@ -30,7 +30,7 @@ * as 6a did for locks and vehicles. The plan ({@code plan.json}) is written before any station changes and * reused after a crash; each changed station's original file is kept in {@code backup/}; {@code done.json} * records the result, after which later boots do nothing. Accounts without a heir keep their stations - * unassigned (the next character on the account claims them) and are listed for staff. + * unassigned and unusable, and are listed for staff. */ public final class StationBackfill { static final String FOLDER = "data/backfill-character-v1"; diff --git a/src/main/java/net/tfminecraft/research/manager/ResearchManager.java b/src/main/java/net/tfminecraft/research/manager/ResearchManager.java index c0eaa46..309c21a 100644 --- a/src/main/java/net/tfminecraft/research/manager/ResearchManager.java +++ b/src/main/java/net/tfminecraft/research/manager/ResearchManager.java @@ -173,7 +173,6 @@ public void onStationInteract(PlayerInteractEvent event) { player.sendMessage(Messages.get("station.in_use")); return; } - claimForActiveCharacter(station, player); openMainGui(player, station); return; } @@ -739,20 +738,8 @@ private boolean isOwnedBy(ResearchStation station, Player player) { } String ownerCharacterId = station.getOwnerCharacterId(); String activeCharacterId = playerManager.getActiveCharacterId(player); - return activeCharacterId != null - && (ownerCharacterId == null || ownerCharacterId.equalsIgnoreCase(activeCharacterId)); - } - - /** Stations saved before characters were recorded belong to the next character to open them. */ - private void claimForActiveCharacter(ResearchStation station, Player player) { - if (station.getOwnerCharacterId() != null) { - return; - } - String characterId = playerManager.getActiveCharacterId(player); - if (characterId != null) { - station.setOwnerCharacterId(characterId); - StationStore.saveStation(station); - } + return ownerCharacterId != null && activeCharacterId != null + && ownerCharacterId.equalsIgnoreCase(activeCharacterId); } private boolean isMenuFor(InventoryView view, ResearchStation station) { diff --git a/src/test/java/net/tfminecraft/research/manager/MultipleStationsTest.java b/src/test/java/net/tfminecraft/research/manager/MultipleStationsTest.java index 166e6b7..15db976 100644 --- a/src/test/java/net/tfminecraft/research/manager/MultipleStationsTest.java +++ b/src/test/java/net/tfminecraft/research/manager/MultipleStationsTest.java @@ -41,6 +41,8 @@ void scrapConfirmActsOnTheStationWhoseMenuIsOpen() { Location second = new Location(world, 20, 64, 20); ResearchStation firstStation = new ResearchStation(first, ownerId, null); ResearchStation secondStation = new ResearchStation(second, ownerId, null); + firstStation.setOwnerCharacterId(session.ref().characterId().toString()); + secondStation.setOwnerCharacterId(session.ref().characterId().toString()); Inventory top = mock(Inventory.class); InventoryView view = mock(InventoryView.class); diff --git a/src/test/java/net/tfminecraft/research/manager/ResearchManagerTest.java b/src/test/java/net/tfminecraft/research/manager/ResearchManagerTest.java index 377cac2..b47b08f 100644 --- a/src/test/java/net/tfminecraft/research/manager/ResearchManagerTest.java +++ b/src/test/java/net/tfminecraft/research/manager/ResearchManagerTest.java @@ -199,6 +199,7 @@ void setUp() throws Exception { when(lectern.getLocation()).thenReturn(location); project = new ActiveProject("input", "output", "vanilla.DIAMOND"); station = new ResearchStation(location, owner, project); + station.setOwnerCharacterId("alice"); points = mock(PlayerManager.class); when(points.getActiveCharacterId(player)).thenReturn("alice"); MockedStatic characters = scoped(mockStatic(Characters.class)); @@ -391,21 +392,33 @@ void startingCharacterKeepsWorkingOnItsProject() { assertEquals(2, project.getAspectPoints("fire")); } - @Test - void olderStationsBelongToTheNextCharacterToOpenThem() { + @ParameterizedTest + @ValueSource(strings = {"alice", "bob"}) + void unassignedStationsCannotBeClaimedByAnActiveCharacter(String character) { + station.setOwnerCharacterId(null); when(points.getActiveCharacterId(player)).thenReturn(null); manager.onStationInteract(interact(Action.RIGHT_CLICK_BLOCK, lectern, EquipmentSlot.HAND)); assertNull(station.getOwnerCharacterId()); store.verify(() -> StationStore.saveStation(any()), never()); - when(points.getActiveCharacterId(player)).thenReturn("alice"); + when(points.getActiveCharacterId(player)).thenReturn(character); manager.onStationInteract(interact(Action.RIGHT_CLICK_BLOCK, lectern, EquipmentSlot.HAND)); - assertEquals("alice", station.getOwnerCharacterId()); - store.verify(() -> StationStore.saveStation(station)); - manager.onStationInteract(interact(Action.RIGHT_CLICK_BLOCK, lectern, EquipmentSlot.HAND)); - store.verify(() -> StationStore.saveStation(station), times(1)); - verify(menus, times(2)).openMain(player, station); + assertNull(station.getOwnerCharacterId()); + store.verify(() -> StationStore.saveStation(any()), never()); + verifyNoInteractions(menus); + } + + @Test + void unassignedStationCannotSpendFocusThroughAnAlreadyOpenMenu() { + station.setOwnerCharacterId(null); + prepareExperiment("fire", "water", Map.of("fire", 10, "water", 10)); + + confirm(); + + verify(points, never()).trySpendMentalPoints(any(), anyInt()); + assertEquals(0, project.getAspectPoints("fire")); + store.verify(() -> StationStore.saveStation(any()), never()); } @ParameterizedTest diff --git a/src/test/java/net/tfminecraft/research/manager/ResearchSessionTest.java b/src/test/java/net/tfminecraft/research/manager/ResearchSessionTest.java index 84e3dc5..13d6801 100644 --- a/src/test/java/net/tfminecraft/research/manager/ResearchSessionTest.java +++ b/src/test/java/net/tfminecraft/research/manager/ResearchSessionTest.java @@ -143,7 +143,7 @@ void endReturnsExperimentOnceBeforeIncomingSessionAndKeepsProgress(CharacterSess } @Test - void legacyOwnershipIsClaimedOnlyWhenReadyAndExistingCasingIsPreserved() { + void unassignedOwnershipCannotBeClaimedAndExistingCasingIsPreserved() { String original = station.getOwnerCharacterId(); interact(); assertEquals(original, station.getOwnerCharacterId()); @@ -156,8 +156,8 @@ void legacyOwnershipIsClaimedOnlyWhenReadyAndExistingCasingIsPreserved() { assertEquals("Character is loading.", player.nextMessage()); current = ready; interact(); - assertEquals(ready.ref().characterId().toString(), station.getOwnerCharacterId()); - store.verify(() -> StationStore.saveStation(station), times(1)); + assertNull(station.getOwnerCharacterId()); + store.verifyNoInteractions(); } @Test