From 9bea7e06f032de38b9389b4910a13cf767f8a9ae Mon Sep 17 00:00:00 2001 From: Stephen Goodhill Date: Wed, 22 Jul 2026 17:37:21 +1000 Subject: [PATCH 1/9] move player-wearer and armor-set tracking into pure domain registries Introduce PlayerArmorWearerRegistry (model.player) holding the PlayerId-to-ArmorSetWearer state and ArmorSetRegistry (model.set) holding the ArmorSetId-to-set mapping. Both are Bukkit-free, self-contained, and unit-tested to 100% line and branch. SetPlayerManager and SetManager keep their existing static public API but delegate to a held registry instance; getSetPlayer reconstructs the Bukkit SetPlayer on demand from the wearer record, and getSet guards null/empty names before ArmorSetId rejects them. The characterization tests are unchanged, proving observable behavior is preserved. --- AGENTS.md | 3 +- pom.xml | 2 + .../java/gg/steve/mc/ap/armor/SetManager.java | 16 +-- .../player/PlayerArmorWearerRegistry.java | 53 +++++++++ .../mc/ap/model/set/ArmorSetRegistry.java | 47 ++++++++ .../steve/mc/ap/player/SetPlayerManager.java | 29 +++-- .../player/PlayerArmorWearerRegistryTest.java | 102 ++++++++++++++++++ .../mc/ap/model/set/ArmorSetRegistryTest.java | 85 +++++++++++++++ 8 files changed, 312 insertions(+), 25 deletions(-) create mode 100644 src/main/java/gg/steve/mc/ap/model/player/PlayerArmorWearerRegistry.java create mode 100644 src/main/java/gg/steve/mc/ap/model/set/ArmorSetRegistry.java create mode 100644 src/test/java/gg/steve/mc/ap/model/player/PlayerArmorWearerRegistryTest.java create mode 100644 src/test/java/gg/steve/mc/ap/model/set/ArmorSetRegistryTest.java diff --git a/AGENTS.md b/AGENTS.md index 14ca093..44ef343 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,7 +5,7 @@ This file is the project's committed home for project-intrinsic agent knowledge: - Build with JDK 25 or newer - `maven-enforcer-plugin` (`requireJavaVersion [25,)`) fails `validate` on anything older. Emitted plugin bytecode is intentionally Java 8 (major version 52) via `8` in `pom.xml`; the target runtime is a Java 17+ Spigot server. Do not "modernize" the bytecode level. Build/verify: `JAVA_HOME= mvn -B clean verify`. - `de.tr7zw:functional-annotations` is pinned to `0.1-SNAPSHOT` because no released version has ever been published (CodeMC metadata lists only the SNAPSHOT). It is a real compile dependency (`de.tr7zw.annotations.FAUtil`, used by the `nbt` package), so it cannot be dropped. Revisit if upstream ever cuts a release. - Test harness: JUnit 5 + MockBukkit + Mockito + JaCoCo. `mvn verify` runs tests and produces `target/site/jacoco/` coverage report. JaCoCo excludes `**/nbt/**` (vendored) and `**/ArmorPlus.class` (bootstrap). -- Coverage gate (CORE set): JaCoCo `check-core` execution enforces 100% line AND branch on these pure-logic classes: `PlayerDirectionUtil`, `ColorUtil`, `Piece`, `SetType`, `SetDataType`, `ArmorHandItem`, `BasicDamageCalculator`, `model.id.TypedString`, `model.id.StringId`, `model.id.ItemHandle`, `model.id.TaskHandle`, `model.player.WornArmor`. Build fails if core coverage drops below 100%. Everything else remains report-only. Excluded from core: `LogUtil` (calls `ArmorPlus.get()` singleton), `nbt.NBTType` (vendored `**/nbt/**` exclusion). When modifying a core class, add tests in the same commit. +- Coverage gate (CORE set): JaCoCo `check-core` execution enforces 100% line AND branch on these pure-logic classes: `PlayerDirectionUtil`, `ColorUtil`, `Piece`, `SetType`, `SetDataType`, `ArmorHandItem`, `BasicDamageCalculator`, `model.id.TypedString`, `model.id.StringId`, `model.id.ItemHandle`, `model.id.TaskHandle`, `model.player.WornArmor`, `model.player.PlayerArmorWearerRegistry`, `model.set.ArmorSetRegistry`. Build fails if core coverage drops below 100%. Everything else remains report-only. Excluded from core: `LogUtil` (calls `ArmorPlus.get()` singleton), `nbt.NBTType` (vendored `**/nbt/**` exclusion). When modifying a core class, add tests in the same commit. - JDK/bytecode split: main code compiles to Java 8 (`maven.compiler.release=8`), test code compiles to Java 17 (`maven.compiler.testRelease=17`). The `testRelease` property is picked up automatically by maven-compiler-plugin's default-testCompile execution - no custom execution block needed. Both compile under the same JDK 25 build. MockBukkit + paper-api are Java 17 bytecode, so tests cannot target 8. - MockBukkit limitation: the plugin cannot be loaded via `MockBukkit.load(ArmorPlus.class)` because the vendored NBT-API performs reflection at enable time that MockBukkit does not model. Use pure-logic unit tests or Mockito for testing pieces that interact with Bukkit APIs. - Mockito on JDK 25: surefire must pass `-Dnet.bytebuddy.experimental=true` (configured in pom.xml ``). Without it, ByteBuddy fails to recognize the JDK 25 class file version. @@ -13,6 +13,7 @@ This file is the project's committed home for project-intrinsic agent knowledge: - Integration-only (not netted): `Set.isWearingSet` full integration path (NBT reflection in real server), `SetPlayerManager.init()` (iterates Bukkit.getOnlinePlayers), GUI rendering, PAPI expansion, scheduler-dependent abilities (Fairy/ColorWay/Lightning/Engineer/Traveller tick logic). - Domain model package: `gg.steve.mc.ap.model` with sub-packages `set`, `ability`, `combat`, `effect`, `notification`, `player`, `id`, `port`. Contains pure-Java value types with ZERO Bukkit/NMS/NBT imports. Uses Lombok (`@Value`, `@Builder`) for boilerplate elimination. The `lombok.config` at repo root sets `lombok.addLombokGeneratedAnnotation=true` so JaCoCo auto-excludes generated code. - Damage-calculation logic lives in `model.ability`: `BasicDamageCalculator` (attack damage multiplier, defense reduction + knockback) and `ArmorHandItem.calculateFinalDamage()` (hand-item combined multiplier). The old `BasicSetData` and `HandSetData` classes are now thin delegators - they translate Bukkit events into domain calls and apply results back. Do not add logic to those delegators; extend the domain classes instead. +- Wearer/set tracking state lives in pure registries: `model.player.PlayerArmorWearerRegistry` (`PlayerId` -> `ArmorSetWearer` map) and `model.set.ArmorSetRegistry` (`ArmorSetId` -> set, generic because no domain `ArmorSet` aggregate exists yet - config parsing owns it). `SetPlayerManager`/`SetManager` keep their static public API but delegate to a held registry instance; `SetPlayerManager.getSetPlayer` reconstructs the Bukkit `SetPlayer` on demand from the wearer record. `SetManager.getSet` guards null/empty names before `ArmorSetId.of` (which rejects empty). `getPiecesWearing`/`getSetFromHand` stay in the manager - they read live Bukkit inventory/NBT, not wearer-map state. Do not add tracking logic to the managers; extend the registries instead. - Domain model conventions: No class may use the `Manager` suffix (use Registry, Service, Store, Factory, Source, Resolver, etc.). Use the typed ID wrappers in model.id (PlayerId, ArmorSetId, DamageCauseId, PotionEffectId, SoundId) as identity currency - never reference Bukkit Player/ItemStack/World in the model layer. ID wrappers extend the `TypedString`/`StringId` base hierarchy: `TypedString` (abstract) rejects null via commons-lang3 `Validate.notNull` in its constructor and implements `equals`/`hashCode` with `getClass()` checks (different subtypes wrapping the same string are never equal); `StringId` extends it adding `Validate.notEmpty`. Leaf classes are final with a private constructor and an `of()` factory; `PlayerId` extends `TypedString` directly, wrapping a `UUID`. Use `@Value` + `@Builder` for multi-field value types. Annotate `List`/`Set`/`Map` fields with `@Singular` so Lombok's builder produces genuinely unmodifiable defensive copies at `build()` time. - Lombok dependency: `org.projectlombok:lombok:1.18.38` (provided scope). Annotation processor configured in maven-compiler-plugin's ``. Works with JDK 25 and emits Java 8 bytecode. - No-Bukkit-in-model rule: verified by grep (no ArchUnit yet). The `model` package must have zero imports of `org.bukkit.*`, `net.minecraft.*`, `de.tr7zw.*`, or any plugin class outside `model`. diff --git a/pom.xml b/pom.xml index 37c799b..d6c1ec5 100644 --- a/pom.xml +++ b/pom.xml @@ -121,6 +121,8 @@ gg.steve.mc.ap.model.id.ItemHandle gg.steve.mc.ap.model.id.TaskHandle gg.steve.mc.ap.model.player.WornArmor + gg.steve.mc.ap.model.player.PlayerArmorWearerRegistry + gg.steve.mc.ap.model.set.ArmorSetRegistry diff --git a/src/main/java/gg/steve/mc/ap/armor/SetManager.java b/src/main/java/gg/steve/mc/ap/armor/SetManager.java index a34e9a9..3e5af2d 100644 --- a/src/main/java/gg/steve/mc/ap/armor/SetManager.java +++ b/src/main/java/gg/steve/mc/ap/armor/SetManager.java @@ -2,6 +2,8 @@ import gg.steve.mc.ap.ArmorPlus; import gg.steve.mc.ap.managers.ConfigManager; +import gg.steve.mc.ap.model.id.ArmorSetId; +import gg.steve.mc.ap.model.set.ArmorSetRegistry; import gg.steve.mc.ap.utils.YamlFileUtil; import java.io.File; @@ -9,24 +11,24 @@ import java.util.Map; public class SetManager { - private static Map setConfigs; - private static Map sets; + private static final ArmorSetRegistry registry = new ArmorSetRegistry<>(); public static void loadSets() { - setConfigs = new HashMap<>(); - sets = new HashMap<>(); + registry.clear(); for (String set : ConfigManager.CONFIG.get().getStringList("loaded-sets")) { YamlFileUtil fileUtil = new YamlFileUtil("sets" + File.separator + set + ".yml", ArmorPlus.get()); - setConfigs.put(set, fileUtil); - sets.put(set, new Set(set, fileUtil)); + registry.register(ArmorSetId.of(set), new Set(set, fileUtil)); } } public static Map getSets() { + Map sets = new HashMap<>(); + registry.getAll().forEach((id, set) -> sets.put(id.toString(), set)); return sets; } public static Set getSet(String name) { - return sets.get(name); + if (name == null || name.isEmpty()) return null; + return registry.get(ArmorSetId.of(name)).orElse(null); } } diff --git a/src/main/java/gg/steve/mc/ap/model/player/PlayerArmorWearerRegistry.java b/src/main/java/gg/steve/mc/ap/model/player/PlayerArmorWearerRegistry.java new file mode 100644 index 0000000..741a081 --- /dev/null +++ b/src/main/java/gg/steve/mc/ap/model/player/PlayerArmorWearerRegistry.java @@ -0,0 +1,53 @@ +package gg.steve.mc.ap.model.player; + +import gg.steve.mc.ap.model.id.ArmorSetId; +import gg.steve.mc.ap.model.id.PlayerId; + +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; + +/** + * Pure in-memory record of which players are currently wearing which armor set. + * Holds the {@link PlayerId}-to-{@link ArmorSetWearer} state, with no reference to + * Bukkit types. Instances are mutable but self-contained, so they can be unit-tested + * without any server or static-state gymnastics. + */ +public final class PlayerArmorWearerRegistry { + private final Map wearers = new HashMap<>(); + + /** Record that a player is wearing a set, replacing any existing entry for that player. */ + public void add(ArmorSetWearer wearer) { + wearers.put(wearer.getPlayerId(), wearer); + } + + /** Convenience overload building the {@link ArmorSetWearer} from its identity parts. */ + public void add(PlayerId playerId, ArmorSetId setId) { + add(new ArmorSetWearer(playerId, setId)); + } + + /** Forget any set the player was recorded as wearing. */ + public void remove(PlayerId playerId) { + wearers.remove(playerId); + } + + /** Whether the player is currently recorded as wearing a set. */ + public boolean isWearing(PlayerId playerId) { + return wearers.containsKey(playerId); + } + + /** The wearer record for a player, if one exists. */ + public Optional get(PlayerId playerId) { + return Optional.ofNullable(wearers.get(playerId)); + } + + /** Number of players currently recorded as wearing a set. */ + public int count() { + return wearers.size(); + } + + /** Drop all wearer records. */ + public void clear() { + wearers.clear(); + } +} diff --git a/src/main/java/gg/steve/mc/ap/model/set/ArmorSetRegistry.java b/src/main/java/gg/steve/mc/ap/model/set/ArmorSetRegistry.java new file mode 100644 index 0000000..fb48862 --- /dev/null +++ b/src/main/java/gg/steve/mc/ap/model/set/ArmorSetRegistry.java @@ -0,0 +1,47 @@ +package gg.steve.mc.ap.model.set; + +import gg.steve.mc.ap.model.id.ArmorSetId; + +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Optional; + +/** + * Pure registry of the armor sets known to the plugin, keyed by {@link ArmorSetId}. + * Preserves registration order so callers that iterate see a deterministic sequence. + *

+ * Generic over the stored set type: the pure domain does not yet own an {@code ArmorSet} + * aggregate, so this registry stays value-type-agnostic and Bukkit-free while the + * platform layer parameterizes it over its own set representation. + * + * @param the stored representation of an armor set + */ +public final class ArmorSetRegistry { + private final Map sets = new LinkedHashMap<>(); + + /** Register (or replace) the set stored under the given id. */ + public void register(ArmorSetId id, T set) { + sets.put(id, set); + } + + /** The set stored under the given id, if one exists. */ + public Optional get(ArmorSetId id) { + return Optional.ofNullable(sets.get(id)); + } + + /** An unmodifiable view of every registered set, in registration order. */ + public Map getAll() { + return Collections.unmodifiableMap(sets); + } + + /** Number of registered sets. */ + public int count() { + return sets.size(); + } + + /** Drop all registered sets. */ + public void clear() { + sets.clear(); + } +} diff --git a/src/main/java/gg/steve/mc/ap/player/SetPlayerManager.java b/src/main/java/gg/steve/mc/ap/player/SetPlayerManager.java index 38f497b..0544907 100644 --- a/src/main/java/gg/steve/mc/ap/player/SetPlayerManager.java +++ b/src/main/java/gg/steve/mc/ap/player/SetPlayerManager.java @@ -3,22 +3,19 @@ import gg.steve.mc.ap.armor.Piece; import gg.steve.mc.ap.armor.Set; import gg.steve.mc.ap.armor.SetManager; +import gg.steve.mc.ap.model.id.ArmorSetId; +import gg.steve.mc.ap.model.id.PlayerId; +import gg.steve.mc.ap.model.player.PlayerArmorWearerRegistry; import gg.steve.mc.ap.nbt.NBTItem; -import gg.steve.mc.ap.utils.LogUtil; -import me.clip.placeholderapi.expansion.PlaceholderExpansion; import org.bukkit.Bukkit; import org.bukkit.Material; import org.bukkit.entity.Player; -import java.util.HashMap; -import java.util.Map; -import java.util.UUID; - public class SetPlayerManager { - private static Map playersWearingSets; + private static final PlayerArmorWearerRegistry registry = new PlayerArmorWearerRegistry(); public static void init() { - playersWearingSets = new HashMap<>(); + registry.clear(); for (Player player : Bukkit.getOnlinePlayers()) { for (Set set : SetManager.getSets().values()) { if (!set.isWearingSet(player, null, null)) continue; @@ -30,23 +27,21 @@ public static void init() { } public static void addSetPlayer(Player player, String setName) { - if (playersWearingSets == null) playersWearingSets = new HashMap<>(); - if (playersWearingSets.containsKey(player.getUniqueId())) removeSetPlayer(player); - playersWearingSets.put(player.getUniqueId(), new SetPlayer(player, setName)); + registry.add(PlayerId.of(player.getUniqueId()), ArmorSetId.of(setName)); } public static void removeSetPlayer(Player player) { - if (playersWearingSets == null) playersWearingSets = new HashMap<>(); - playersWearingSets.remove(player.getUniqueId()); + registry.remove(PlayerId.of(player.getUniqueId())); } public static boolean isWearingSet(Player player) { - if (playersWearingSets == null) playersWearingSets = new HashMap<>(); - return playersWearingSets.containsKey(player.getUniqueId()); + return registry.isWearing(PlayerId.of(player.getUniqueId())); } public static SetPlayer getSetPlayer(Player player) { - return playersWearingSets.get(player.getUniqueId()); + return registry.get(PlayerId.of(player.getUniqueId())) + .map(wearer -> new SetPlayer(player, wearer.getSetId().toString())) + .orElse(null); } public static int getPiecesWearing(Set set, Player player) { @@ -79,4 +74,4 @@ public static Set getSetFromHand(Player player) { if (hand.getString("armor+.set").equalsIgnoreCase("")) return null; return SetManager.getSet(hand.getString("armor+.set")); } -} \ No newline at end of file +} diff --git a/src/test/java/gg/steve/mc/ap/model/player/PlayerArmorWearerRegistryTest.java b/src/test/java/gg/steve/mc/ap/model/player/PlayerArmorWearerRegistryTest.java new file mode 100644 index 0000000..ac05153 --- /dev/null +++ b/src/test/java/gg/steve/mc/ap/model/player/PlayerArmorWearerRegistryTest.java @@ -0,0 +1,102 @@ +package gg.steve.mc.ap.model.player; + +import gg.steve.mc.ap.model.id.ArmorSetId; +import gg.steve.mc.ap.model.id.PlayerId; +import org.junit.jupiter.api.Test; + +import java.util.Optional; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.*; + +class PlayerArmorWearerRegistryTest { + + private final PlayerId player = PlayerId.of(UUID.fromString("00000000-0000-0000-0000-000000000001")); + private final PlayerId other = PlayerId.of(UUID.fromString("00000000-0000-0000-0000-000000000002")); + private final ArmorSetId dragon = ArmorSetId.of("dragon"); + private final ArmorSetId knight = ArmorSetId.of("knight"); + + @Test + void freshRegistryHasNoWearers() { + PlayerArmorWearerRegistry registry = new PlayerArmorWearerRegistry(); + + assertEquals(0, registry.count()); + assertFalse(registry.isWearing(player)); + assertEquals(Optional.empty(), registry.get(player)); + } + + @Test + void addFromParts_recordsWearer() { + PlayerArmorWearerRegistry registry = new PlayerArmorWearerRegistry(); + registry.add(player, dragon); + + assertTrue(registry.isWearing(player)); + assertEquals(1, registry.count()); + assertEquals(Optional.of(new ArmorSetWearer(player, dragon)), registry.get(player)); + } + + @Test + void addWearer_recordsWearer() { + PlayerArmorWearerRegistry registry = new PlayerArmorWearerRegistry(); + ArmorSetWearer wearer = new ArmorSetWearer(player, dragon); + registry.add(wearer); + + assertTrue(registry.isWearing(player)); + assertEquals(Optional.of(wearer), registry.get(player)); + } + + @Test + void add_samePlayerTwice_replacesExisting() { + PlayerArmorWearerRegistry registry = new PlayerArmorWearerRegistry(); + registry.add(player, dragon); + registry.add(player, knight); + + assertEquals(1, registry.count()); + assertEquals(Optional.of(new ArmorSetWearer(player, knight)), registry.get(player)); + } + + @Test + void remove_forgetsWearer() { + PlayerArmorWearerRegistry registry = new PlayerArmorWearerRegistry(); + registry.add(player, dragon); + registry.remove(player); + + assertFalse(registry.isWearing(player)); + assertEquals(Optional.empty(), registry.get(player)); + assertEquals(0, registry.count()); + } + + @Test + void remove_absentPlayer_isNoOp() { + PlayerArmorWearerRegistry registry = new PlayerArmorWearerRegistry(); + + assertDoesNotThrow(() -> registry.remove(player)); + assertEquals(0, registry.count()); + } + + @Test + void multiplePlayers_trackedIndependently() { + PlayerArmorWearerRegistry registry = new PlayerArmorWearerRegistry(); + registry.add(player, dragon); + registry.add(other, knight); + + assertEquals(2, registry.count()); + registry.remove(player); + + assertFalse(registry.isWearing(player)); + assertTrue(registry.isWearing(other)); + assertEquals(Optional.of(new ArmorSetWearer(other, knight)), registry.get(other)); + } + + @Test + void clear_dropsEveryWearer() { + PlayerArmorWearerRegistry registry = new PlayerArmorWearerRegistry(); + registry.add(player, dragon); + registry.add(other, knight); + registry.clear(); + + assertEquals(0, registry.count()); + assertFalse(registry.isWearing(player)); + assertFalse(registry.isWearing(other)); + } +} diff --git a/src/test/java/gg/steve/mc/ap/model/set/ArmorSetRegistryTest.java b/src/test/java/gg/steve/mc/ap/model/set/ArmorSetRegistryTest.java new file mode 100644 index 0000000..abf927a --- /dev/null +++ b/src/test/java/gg/steve/mc/ap/model/set/ArmorSetRegistryTest.java @@ -0,0 +1,85 @@ +package gg.steve.mc.ap.model.set; + +import gg.steve.mc.ap.model.id.ArmorSetId; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.*; + +class ArmorSetRegistryTest { + + private final ArmorSetId dragon = ArmorSetId.of("dragon"); + private final ArmorSetId knight = ArmorSetId.of("knight"); + private final ArmorSetId mage = ArmorSetId.of("mage"); + + @Test + void freshRegistryIsEmpty() { + ArmorSetRegistry registry = new ArmorSetRegistry<>(); + + assertEquals(0, registry.count()); + assertEquals(Optional.empty(), registry.get(dragon)); + assertTrue(registry.getAll().isEmpty()); + } + + @Test + void register_thenGet_returnsStoredSet() { + ArmorSetRegistry registry = new ArmorSetRegistry<>(); + registry.register(dragon, "dragon-set"); + + assertEquals(Optional.of("dragon-set"), registry.get(dragon)); + assertEquals(1, registry.count()); + } + + @Test + void get_unknownId_returnsEmpty() { + ArmorSetRegistry registry = new ArmorSetRegistry<>(); + registry.register(dragon, "dragon-set"); + + assertEquals(Optional.empty(), registry.get(knight)); + } + + @Test + void register_sameId_replacesExisting() { + ArmorSetRegistry registry = new ArmorSetRegistry<>(); + registry.register(dragon, "first"); + registry.register(dragon, "second"); + + assertEquals(Optional.of("second"), registry.get(dragon)); + assertEquals(1, registry.count()); + } + + @Test + void getAll_preservesRegistrationOrder() { + ArmorSetRegistry registry = new ArmorSetRegistry<>(); + registry.register(mage, "mage-set"); + registry.register(dragon, "dragon-set"); + registry.register(knight, "knight-set"); + + List order = new ArrayList<>(registry.getAll().keySet()); + assertEquals(List.of(mage, dragon, knight), order); + } + + @Test + void getAll_isUnmodifiable() { + ArmorSetRegistry registry = new ArmorSetRegistry<>(); + registry.register(dragon, "dragon-set"); + + Map all = registry.getAll(); + assertThrows(UnsupportedOperationException.class, () -> all.put(knight, "knight-set")); + } + + @Test + void clear_dropsEverySet() { + ArmorSetRegistry registry = new ArmorSetRegistry<>(); + registry.register(dragon, "dragon-set"); + registry.register(knight, "knight-set"); + registry.clear(); + + assertEquals(0, registry.count()); + assertEquals(Optional.empty(), registry.get(dragon)); + } +} From cd2ceb8c1c4f3500d3b6608b4eba54e8f4d9e94e Mon Sep 17 00:00:00 2001 From: Stephen Goodhill Date: Wed, 22 Jul 2026 21:47:11 +1000 Subject: [PATCH 2/9] no-mistakes(document): sync README coverage list with new domain registries --- README.md | 2 +- .../gg/steve/mc/ap/armor/SetManagerTest.java | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 src/test/java/gg/steve/mc/ap/armor/SetManagerTest.java diff --git a/README.md b/README.md index 195d8a9..c1624eb 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ For more information about the plugin, permissions, and commands please refer to ## Code Coverage JaCoCo runs on every CI build (`mvn verify`). -The build enforces 100% line and branch coverage on the pure-logic core classes (`PlayerDirectionUtil`, `ColorUtil`, `Piece`, `SetType`, `SetDataType`, `ArmorHandItem`, `BasicDamageCalculator`, `TypedString`, `StringId`, `ItemHandle`, `TaskHandle`, `WornArmor`); a coverage drop on any of these will fail the build. +The build enforces 100% line and branch coverage on the pure-logic core classes (`PlayerDirectionUtil`, `ColorUtil`, `Piece`, `SetType`, `SetDataType`, `ArmorHandItem`, `BasicDamageCalculator`, `TypedString`, `StringId`, `ItemHandle`, `TaskHandle`, `WornArmor`, `PlayerArmorWearerRegistry`, `ArmorSetRegistry`); a coverage drop on any of these will fail the build. The HTML coverage report is uploaded as the **jacoco-report** artifact on each workflow run - download it from the [Actions tab](https://github.com/nbdSteve/ArmorPlus/actions/workflows/ci.yml). ## Soft Dependencies diff --git a/src/test/java/gg/steve/mc/ap/armor/SetManagerTest.java b/src/test/java/gg/steve/mc/ap/armor/SetManagerTest.java new file mode 100644 index 0000000..69c06b4 --- /dev/null +++ b/src/test/java/gg/steve/mc/ap/armor/SetManagerTest.java @@ -0,0 +1,33 @@ +package gg.steve.mc.ap.armor; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Characterization tests for {@link SetManager#getSet(String)} pinning the exact + * null contract that the 4E registry extraction had to preserve. + *

+ * Real callers (GiveCmd, PlayerEquipListener, ArmorPlusExpansion) treat a null return as + * "no such set" and must never see an exception for a missing/blank name. Because the new + * {@code ArmorSetRegistry} keys on {@code ArmorSetId} - which rejects null/empty via + * commons-lang3 {@code Validate} - {@code getSet} guards those inputs before constructing the id. + * These tests prove that guard preserves the original {@code map.get()} behavior. + */ +class SetManagerTest { + + @Test + void getSet_nullName_returnsNullNotThrows() { + assertNull(SetManager.getSet(null)); + } + + @Test + void getSet_emptyName_returnsNullNotThrows() { + assertNull(SetManager.getSet("")); + } + + @Test + void getSet_unknownName_returnsNull() { + assertNull(SetManager.getSet("no-such-set")); + } +} From 1a314801f42cd9133f8e1af7cfc8b84d5cf0ba4e Mon Sep 17 00:00:00 2001 From: Stephen Goodhill Date: Thu, 23 Jul 2026 10:34:45 +1000 Subject: [PATCH 3/9] hold the armor-set registry as an instance field and return it directly SetManager now keeps its ArmorSetRegistry on a shared instance rather than in a static field, and getSets() returns the registry's own view instead of copying it into a fresh HashMap. The static entry points delegate to the shared instance so every caller and the characterization tests are unchanged; getSets() exposes ArmorSetId keys, so the one keySet() caller iterates those ids. --- .../java/gg/steve/mc/ap/armor/SetManager.java | 24 +++++++++++++------ .../mc/ap/listener/PlayerCommandListener.java | 5 ++-- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/main/java/gg/steve/mc/ap/armor/SetManager.java b/src/main/java/gg/steve/mc/ap/armor/SetManager.java index 3e5af2d..9d997e4 100644 --- a/src/main/java/gg/steve/mc/ap/armor/SetManager.java +++ b/src/main/java/gg/steve/mc/ap/armor/SetManager.java @@ -7,13 +7,25 @@ import gg.steve.mc.ap.utils.YamlFileUtil; import java.io.File; -import java.util.HashMap; import java.util.Map; public class SetManager { - private static final ArmorSetRegistry registry = new ArmorSetRegistry<>(); + private static final SetManager INSTANCE = new SetManager(); + + private final ArmorSetRegistry registry = new ArmorSetRegistry<>(); + + /** The shared instance the static entry points below delegate to, so the registry state is held by an instance rather than a static map. */ + public static SetManager get() { + return INSTANCE; + } + + /** The registry this instance owns, for callers that can take the instance directly. */ + public ArmorSetRegistry getRegistry() { + return registry; + } public static void loadSets() { + ArmorSetRegistry registry = INSTANCE.registry; registry.clear(); for (String set : ConfigManager.CONFIG.get().getStringList("loaded-sets")) { YamlFileUtil fileUtil = new YamlFileUtil("sets" + File.separator + set + ".yml", ArmorPlus.get()); @@ -21,14 +33,12 @@ public static void loadSets() { } } - public static Map getSets() { - Map sets = new HashMap<>(); - registry.getAll().forEach((id, set) -> sets.put(id.toString(), set)); - return sets; + public static Map getSets() { + return INSTANCE.registry.getAll(); } public static Set getSet(String name) { if (name == null || name.isEmpty()) return null; - return registry.get(ArmorSetId.of(name)).orElse(null); + return INSTANCE.registry.get(ArmorSetId.of(name)).orElse(null); } } diff --git a/src/main/java/gg/steve/mc/ap/listener/PlayerCommandListener.java b/src/main/java/gg/steve/mc/ap/listener/PlayerCommandListener.java index 9f42a01..8f7dd0b 100644 --- a/src/main/java/gg/steve/mc/ap/listener/PlayerCommandListener.java +++ b/src/main/java/gg/steve/mc/ap/listener/PlayerCommandListener.java @@ -5,6 +5,7 @@ import gg.steve.mc.ap.armor.SetManager; import gg.steve.mc.ap.message.CommandDebug; import gg.steve.mc.ap.message.MessageType; +import gg.steve.mc.ap.model.id.ArmorSetId; import gg.steve.mc.ap.permission.PermissionNode; import org.bukkit.Bukkit; import org.bukkit.entity.Player; @@ -19,9 +20,9 @@ public class PlayerCommandListener implements Listener { public void onCmd(PlayerCommandPreprocessEvent event) { String[] args = event.getMessage().split(" "); String setName = null; - for (String key : SetManager.getSets().keySet()) { + for (ArmorSetId key : SetManager.getSets().keySet()) { if (args[0].equalsIgnoreCase("/" + key)) { - setName = key; + setName = key.toString(); event.setCancelled(true); } } From f60d14d8626baf51142bca9a52c68cfdd8ec34c2 Mon Sep 17 00:00:00 2001 From: Stephen Goodhill Date: Thu, 23 Jul 2026 12:00:42 +1000 Subject: [PATCH 4/9] no-mistakes(document): docs already in sync with registry extraction --- .../listener/PlayerCommandListenerTest.java | 152 ++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java diff --git a/src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java b/src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java new file mode 100644 index 0000000..85b6043 --- /dev/null +++ b/src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java @@ -0,0 +1,152 @@ +package gg.steve.mc.ap.listener; + +import gg.steve.mc.ap.armor.Set; +import gg.steve.mc.ap.armor.SetManager; +import gg.steve.mc.ap.managers.FileManager; +import gg.steve.mc.ap.model.id.ArmorSetId; +import org.bukkit.configuration.file.YamlConfiguration; +import org.bukkit.entity.Player; +import org.bukkit.event.player.PlayerCommandPreprocessEvent; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +/** + * End-to-end style test of {@link PlayerCommandListener}, the only player-facing surface the + * 4E registry extraction changed. Before the change, {@code SetManager.getSets()} returned a + * {@code Map} and the listener iterated raw {@code String} keys; now it returns the + * registry's {@code Map} view and the listener iterates {@code ArmorSetId} keys, + * using {@code key.toString()} to recover the raw set-name string. + *

+ * These tests fire real {@link PlayerCommandPreprocessEvent}s (exactly what Bukkit delivers when a + * player types a command in chat) at the real listener, wired to the real shared {@link SetManager} + * registry, and assert the interception behaves identically: the set name is matched + * case-insensitively, the event is cancelled, the matching set's GUI is opened, and unrelated + * commands pass straight through. A transcript of each simulated keystroke and its outcome is + * written to the evidence directory so a reviewer can see the player experience directly. + */ +@ExtendWith(MockitoExtension.class) +class PlayerCommandListenerTest { + + private static final Path EVIDENCE = Path.of( + "/var/folders/xr/3t6rc9s511sfnzjbrjc1y_380000gr/T/no-mistakes-evidence/01KY69XZQMQHZWP62V198YSZ3P", + "player-command-intercept-transcript.txt"); + + @Mock private Player player; + @Mock private Set dragonSet; + @Mock private Set knightSet; + @Mock private YamlConfiguration permissionsConfig; + + private final PlayerCommandListener listener = new PlayerCommandListener(); + private final List transcript = new ArrayList<>(); + + @BeforeEach + void setUp() { + // Register two sets into the real shared registry, exactly as loadSets() would. + SetManager.get().getRegistry().clear(); + SetManager.get().getRegistry().register(ArmorSetId.of("dragon"), dragonSet); + SetManager.get().getRegistry().register(ArmorSetId.of("knight"), knightSet); + } + + @AfterEach + void tearDown() { + SetManager.get().getRegistry().clear(); + } + + /** Fire a command at the real listener and record the observable player-facing outcome. */ + private PlayerCommandPreprocessEvent type(String typed) { + // Three-arg constructor takes an explicit recipient set, avoiding the internal + // getServer().getOnlinePlayers() call the two-arg form makes. + PlayerCommandPreprocessEvent event = + new PlayerCommandPreprocessEvent(player, typed, Collections.emptySet()); + listener.onCmd(event); + transcript.add(String.format("player types %-22s -> intercepted=%s", "\"" + typed + "\"", event.isCancelled())); + return event; + } + + private void writeTranscript(String title) { + List lines = new ArrayList<>(); + lines.add("=== " + title + " ==="); + lines.add("Registry keys (ArmorSetId): " + new ArrayList<>(SetManager.getSets().keySet())); + lines.addAll(transcript); + try { + Files.createDirectories(EVIDENCE.getParent()); + Files.write(EVIDENCE, (String.join(System.lineSeparator(), lines) + System.lineSeparator()) + .getBytes(StandardCharsets.UTF_8)); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + @Test + void typingSetName_intercepted_opensThatSetsGui() { + when(player.hasPermission(anyString())).thenReturn(true); + try (MockedStatic fm = mockStatic(FileManager.class)) { + fm.when(() -> FileManager.get("PERMISSIONS")).thenReturn(permissionsConfig); + when(permissionsConfig.getString("command.gui")).thenReturn("armorplus.command.gui"); + + PlayerCommandPreprocessEvent event = type("/dragon"); + + assertTrue(event.isCancelled(), "typing a known set name must be intercepted"); + verify(dragonSet).openGui(player); + verify(knightSet, never()).openGui(any()); + writeTranscript("Player types a set command -> that set's GUI opens"); + } + } + + @Test + void typingSetName_isCaseInsensitive() { + when(player.hasPermission(anyString())).thenReturn(true); + try (MockedStatic fm = mockStatic(FileManager.class)) { + fm.when(() -> FileManager.get("PERMISSIONS")).thenReturn(permissionsConfig); + when(permissionsConfig.getString("command.gui")).thenReturn("armorplus.command.gui"); + + PlayerCommandPreprocessEvent event = type("/DRAGON"); + + assertTrue(event.isCancelled(), "matching is case-insensitive, as before"); + verify(dragonSet).openGui(player); + } + } + + @Test + void typingUnknownCommand_passesThroughUnintercepted() { + PlayerCommandPreprocessEvent event = type("/spawn"); + + assertFalse(event.isCancelled(), "commands that are not set names must not be intercepted"); + verify(dragonSet, never()).openGui(any()); + verify(knightSet, never()).openGui(any()); + } + + @Test + void eachSetNameRoutesToItsOwnSet() { + when(player.hasPermission(anyString())).thenReturn(true); + try (MockedStatic fm = mockStatic(FileManager.class)) { + fm.when(() -> FileManager.get("PERMISSIONS")).thenReturn(permissionsConfig); + when(permissionsConfig.getString("command.gui")).thenReturn("armorplus.command.gui"); + + type("/dragon"); + type("/knight"); + type("/spawn"); + + verify(dragonSet).openGui(player); + verify(knightSet).openGui(player); + writeTranscript("Each set command routes to its own set; unknown command passes through"); + } + } +} From d9474c04e1444599a928ceca616dab08a9cc1186 Mon Sep 17 00:00:00 2001 From: Stephen Goodhill Date: Thu, 23 Jul 2026 12:06:58 +1000 Subject: [PATCH 5/9] no-mistakes: apply CI fixes --- .../gg/steve/mc/ap/listener/PlayerCommandListenerTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java b/src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java index 85b6043..3aba995 100644 --- a/src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java +++ b/src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java @@ -45,7 +45,8 @@ class PlayerCommandListenerTest { private static final Path EVIDENCE = Path.of( - "/var/folders/xr/3t6rc9s511sfnzjbrjc1y_380000gr/T/no-mistakes-evidence/01KY69XZQMQHZWP62V198YSZ3P", + System.getProperty("java.io.tmpdir"), + "no-mistakes-evidence", "01KY69XZQMQHZWP62V198YSZ3P", "player-command-intercept-transcript.txt"); @Mock private Player player; From ea9ec594bb4b7579be1ac3a2b9805101cdb62f06 Mon Sep 17 00:00:00 2001 From: Stephen Goodhill Date: Thu, 23 Jul 2026 20:28:47 +1000 Subject: [PATCH 6/9] refactor: inject armor-set catalog and player service via Guice Introduce Google Guice as the plugin's dependency-injection framework and use it to replace the static SetManager/SetPlayerManager singletons with ordinary injected instances. - Add Guice 5.1.0 (JSR-330 javax.inject). Chosen over Dagger to avoid a second annotation processor alongside Lombok and for lower runtime ceremony on Spigot. Shade + relocate guice/guava/javax.inject/aopalliance under gg.steve.mc.ap.lib.* (mirrors the commons-lang3 relocation) and exclude annotation-only transitives from the shaded jar. - Stand up a composition root in ArmorPlus.onEnable: build one Injector from ArmorPlusModule and resolve the shared collaborators once. - Replace SetManager with ArmorSetCatalog and SetPlayerManager with PlayerArmorSetService - constructor-injected singletons holding the pure ArmorSetRegistry / PlayerArmorWearerRegistry. Drop the static maps and get() entry points; thread the instances to listeners, commands, the GUI, and the placeholder expansion. - SetPlayer takes a resolved Set directly instead of resolving a name through the old static manager. Behavior is preserved: the characterization suite is unchanged except for the manager-coupled tests, which now construct the injected instances instead of mocking static state (no behavioral assertions changed). mvn clean verify green on JDK 25 (Java 8 bytecode); check-core coverage gate still 100%. --- AGENTS.md | 7 +- pom.xml | 42 +++++++++++ src/main/java/gg/steve/mc/ap/ArmorPlus.java | 29 ++++++-- .../java/gg/steve/mc/ap/ArmorPlusModule.java | 36 +++++++++ .../gg/steve/mc/ap/armor/ArmorSetCatalog.java | 52 +++++++++++++ .../java/gg/steve/mc/ap/armor/SetManager.java | 44 ----------- src/main/java/gg/steve/mc/ap/cmd/ApCmd.java | 8 +- .../java/gg/steve/mc/ap/cmd/sub/GiveCmd.java | 8 +- src/main/java/gg/steve/mc/ap/gui/ApGui.java | 9 ++- .../mc/ap/listener/ArmorBuffListener.java | 33 +++++---- .../mc/ap/listener/PlayerCommandListener.java | 11 ++- .../mc/ap/listener/PlayerEquipListener.java | 19 +++-- .../mc/ap/listener/PlayerUnequipListener.java | 19 +++-- .../gg/steve/mc/ap/managers/SetupManager.java | 22 +++--- .../steve/mc/ap/papi/ArmorPlusExpansion.java | 62 ++++++++-------- ...anager.java => PlayerArmorSetService.java} | 46 ++++++++---- .../java/gg/steve/mc/ap/player/SetPlayer.java | 5 +- .../mc/ap/armor/ArmorSetCatalogTest.java | 50 +++++++++++++ .../gg/steve/mc/ap/armor/SetManagerTest.java | 33 --------- .../listener/PlayerCommandListenerTest.java | 41 +++++----- ...st.java => PlayerArmorSetServiceTest.java} | 74 +++++++++---------- 21 files changed, 409 insertions(+), 241 deletions(-) create mode 100644 src/main/java/gg/steve/mc/ap/ArmorPlusModule.java create mode 100644 src/main/java/gg/steve/mc/ap/armor/ArmorSetCatalog.java delete mode 100644 src/main/java/gg/steve/mc/ap/armor/SetManager.java rename src/main/java/gg/steve/mc/ap/player/{SetPlayerManager.java => PlayerArmorSetService.java} (57%) create mode 100644 src/test/java/gg/steve/mc/ap/armor/ArmorSetCatalogTest.java delete mode 100644 src/test/java/gg/steve/mc/ap/armor/SetManagerTest.java rename src/test/java/gg/steve/mc/ap/player/{SetPlayerManagerTest.java => PlayerArmorSetServiceTest.java} (68%) diff --git a/AGENTS.md b/AGENTS.md index 44ef343..7f0449e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,11 +9,12 @@ This file is the project's committed home for project-intrinsic agent knowledge: - JDK/bytecode split: main code compiles to Java 8 (`maven.compiler.release=8`), test code compiles to Java 17 (`maven.compiler.testRelease=17`). The `testRelease` property is picked up automatically by maven-compiler-plugin's default-testCompile execution - no custom execution block needed. Both compile under the same JDK 25 build. MockBukkit + paper-api are Java 17 bytecode, so tests cannot target 8. - MockBukkit limitation: the plugin cannot be loaded via `MockBukkit.load(ArmorPlus.class)` because the vendored NBT-API performs reflection at enable time that MockBukkit does not model. Use pure-logic unit tests or Mockito for testing pieces that interact with Bukkit APIs. - Mockito on JDK 25: surefire must pass `-Dnet.bytebuddy.experimental=true` (configured in pom.xml ``). Without it, ByteBuddy fails to recognize the JDK 25 class file version. -- Characterization test safety net (Phase 4A): `src/test/java/gg/steve/mc/ap/{data,player,armor,message}` contains characterization tests pinning current behavior of damage calc (`HandSetData.calculateFinalDamage`, `BasicSetData.onHit/onDamage`), wearer tracking (`SetPlayerManager`), warp safety (`WarpUtil.isSafe`), XP multiplier (`ExperienceSetData.onTargetDeath`), potion checks (`SetStatusEffectsManager.potionCheck`), set detection (`Set.isWearingSet/verifyPiece`), and messaging (`MessageType/CommandDebug`). Rearch phases 4B+ MUST keep these tests green - they prove behavior is preserved during extraction. Do NOT "fix" surprising behavior these tests pin; document it and defer to a separate bug-fix PR. -- Integration-only (not netted): `Set.isWearingSet` full integration path (NBT reflection in real server), `SetPlayerManager.init()` (iterates Bukkit.getOnlinePlayers), GUI rendering, PAPI expansion, scheduler-dependent abilities (Fairy/ColorWay/Lightning/Engineer/Traveller tick logic). +- Characterization test safety net: `src/test/java/gg/steve/mc/ap/{data,player,armor,message}` contains characterization tests pinning current behavior of damage calc (`HandSetData.calculateFinalDamage`, `BasicSetData.onHit/onDamage`), wearer tracking (`PlayerArmorSetServiceTest`), warp safety (`WarpUtil.isSafe`), XP multiplier (`ExperienceSetData.onTargetDeath`), potion checks (`SetStatusEffectsManager.potionCheck`), set detection (`Set.isWearingSet/verifyPiece`), and messaging (`MessageType/CommandDebug`). Later refactors MUST keep these tests green - they prove behavior is preserved during extraction. Do NOT "fix" surprising behavior these tests pin; document it and defer to a separate bug-fix PR. +- Integration-only (not netted): `Set.isWearingSet` full integration path (NBT reflection in real server), `PlayerArmorSetService.init()` (iterates Bukkit.getOnlinePlayers), GUI rendering, PAPI expansion, scheduler-dependent abilities (Fairy/ColorWay/Lightning/Engineer/Traveller tick logic). - Domain model package: `gg.steve.mc.ap.model` with sub-packages `set`, `ability`, `combat`, `effect`, `notification`, `player`, `id`, `port`. Contains pure-Java value types with ZERO Bukkit/NMS/NBT imports. Uses Lombok (`@Value`, `@Builder`) for boilerplate elimination. The `lombok.config` at repo root sets `lombok.addLombokGeneratedAnnotation=true` so JaCoCo auto-excludes generated code. - Damage-calculation logic lives in `model.ability`: `BasicDamageCalculator` (attack damage multiplier, defense reduction + knockback) and `ArmorHandItem.calculateFinalDamage()` (hand-item combined multiplier). The old `BasicSetData` and `HandSetData` classes are now thin delegators - they translate Bukkit events into domain calls and apply results back. Do not add logic to those delegators; extend the domain classes instead. -- Wearer/set tracking state lives in pure registries: `model.player.PlayerArmorWearerRegistry` (`PlayerId` -> `ArmorSetWearer` map) and `model.set.ArmorSetRegistry` (`ArmorSetId` -> set, generic because no domain `ArmorSet` aggregate exists yet - config parsing owns it). `SetPlayerManager`/`SetManager` keep their static public API but delegate to a held registry instance; `SetPlayerManager.getSetPlayer` reconstructs the Bukkit `SetPlayer` on demand from the wearer record. `SetManager.getSet` guards null/empty names before `ArmorSetId.of` (which rejects empty). `getPiecesWearing`/`getSetFromHand` stay in the manager - they read live Bukkit inventory/NBT, not wearer-map state. Do not add tracking logic to the managers; extend the registries instead. +- Wearer/set tracking state lives in pure registries: `model.player.PlayerArmorWearerRegistry` (`PlayerId` -> `ArmorSetWearer` map) and `model.set.ArmorSetRegistry` (`ArmorSetId` -> set, generic because no domain `ArmorSet` aggregate exists yet - config parsing owns it). The Bukkit-facing adapters over these registries are `armor.ArmorSetCatalog` (holds an `ArmorSetRegistry`; `loadSets`, `getSets`, `getSet`) and `player.PlayerArmorSetService` (holds a `PlayerArmorWearerRegistry` + the catalog; `addSetPlayer`/`removeSetPlayer`/`isWearingSet`/`getSetPlayer`/`getPiecesWearing`/`getSetFromHand`/`init`). Both are ordinary injected instances - NO static state, NO `get()` entry point (they replaced the former `SetManager`/`SetPlayerManager` singletons). `PlayerArmorSetService.getSetPlayer` reconstructs the Bukkit `SetPlayer` on demand from the wearer record via `catalog.getSet(...)`. `ArmorSetCatalog.getSet` guards null/empty names before `ArmorSetId.of` (which rejects empty). `getPiecesWearing`/`getSetFromHand` read live Bukkit inventory/NBT, not wearer-map state. Do not add tracking logic to the adapters; extend the registries instead. +- Dependency injection (Guice 5.1.0, JSR-330 `javax.inject`, chosen over Dagger to avoid a second annotation processor alongside Lombok and for lower Spigot-runtime ceremony): the composition root is `ArmorPlus.onEnable`, which builds one `Injector` from `ArmorPlusModule` and resolves the shared collaborators. `ArmorPlusModule` binds the two pure registries (no-arg construction, kept framework-annotation-free so the `model` layer imports zero DI types), `ArmorSetCatalog`, and `PlayerArmorSetService` as singletons; the catalog + service are constructor-injected (`@Inject com.google.inject.Inject`) and threaded to listeners/commands/expansion/GUI. Guice + guava + `javax.inject` + aopalliance are shaded and relocated under `gg.steve.mc.ap.lib.*` (mirrors the commons-lang3 relocation) to avoid classpath collisions on a shared server; annotation-only transitives (jsr305, checker-qual, error_prone_annotations, j2objc, listenablefuture) are excluded from the shaded jar. Remaining `ArmorPlus.get()` uses are deferred (scheduler/logger via `LogUtil`, ability tick tasks, `eco()`, `formatNumber()`, and the static `getApGui()` facade, which reads the injected catalog stored on the plugin). - Domain model conventions: No class may use the `Manager` suffix (use Registry, Service, Store, Factory, Source, Resolver, etc.). Use the typed ID wrappers in model.id (PlayerId, ArmorSetId, DamageCauseId, PotionEffectId, SoundId) as identity currency - never reference Bukkit Player/ItemStack/World in the model layer. ID wrappers extend the `TypedString`/`StringId` base hierarchy: `TypedString` (abstract) rejects null via commons-lang3 `Validate.notNull` in its constructor and implements `equals`/`hashCode` with `getClass()` checks (different subtypes wrapping the same string are never equal); `StringId` extends it adding `Validate.notEmpty`. Leaf classes are final with a private constructor and an `of()` factory; `PlayerId` extends `TypedString` directly, wrapping a `UUID`. Use `@Value` + `@Builder` for multi-field value types. Annotate `List`/`Set`/`Map` fields with `@Singular` so Lombok's builder produces genuinely unmodifiable defensive copies at `build()` time. - Lombok dependency: `org.projectlombok:lombok:1.18.38` (provided scope). Annotation processor configured in maven-compiler-plugin's ``. Works with JDK 25 and emits Java 8 bytecode. - No-Bukkit-in-model rule: verified by grep (no ArchUnit yet). The `model` package must have zero imports of `org.bukkit.*`, `net.minecraft.*`, `de.tr7zw.*`, or any plugin class outside `model`. diff --git a/pom.xml b/pom.xml index d6c1ec5..2c97e7d 100644 --- a/pom.xml +++ b/pom.xml @@ -157,7 +157,40 @@ org.apache.commons.lang3 gg.steve.mc.ap.lib.commons.lang3 + + + com.google.inject + gg.steve.mc.ap.lib.inject + + + com.google.common + gg.steve.mc.ap.lib.guava + + + com.google.thirdparty + gg.steve.mc.ap.lib.guava.thirdparty + + + javax.inject + gg.steve.mc.ap.lib.javax.inject + + + org.aopalliance + gg.steve.mc.ap.lib.aopalliance + + + + + com.google.code.findbugs:jsr305 + org.checkerframework:checker-qual + com.google.errorprone:error_prone_annotations + com.google.j2objc:j2objc-annotations + com.google.guava:listenablefuture + + @@ -255,6 +288,15 @@ 3.18.0 compile + + + com.google.inject + guice + 5.1.0 + compile + com.mojang authlib diff --git a/src/main/java/gg/steve/mc/ap/ArmorPlus.java b/src/main/java/gg/steve/mc/ap/ArmorPlus.java index 7bbd2d5..6a8db6a 100644 --- a/src/main/java/gg/steve/mc/ap/ArmorPlus.java +++ b/src/main/java/gg/steve/mc/ap/ArmorPlus.java @@ -1,12 +1,14 @@ package gg.steve.mc.ap; -import gg.steve.mc.ap.armor.SetManager; +import com.google.inject.Guice; +import com.google.inject.Injector; +import gg.steve.mc.ap.armor.ArmorSetCatalog; import gg.steve.mc.ap.gui.ApGui; import gg.steve.mc.ap.managers.ConfigManager; import gg.steve.mc.ap.managers.FileManager; import gg.steve.mc.ap.managers.SetupManager; import gg.steve.mc.ap.papi.ArmorPlusExpansion; -import gg.steve.mc.ap.player.SetPlayerManager; +import gg.steve.mc.ap.player.PlayerArmorSetService; import gg.steve.mc.ap.utils.LogUtil; import net.milkbowl.vault.economy.Economy; import org.bstats.bukkit.Metrics; @@ -24,6 +26,12 @@ public final class ArmorPlus extends JavaPlugin { private static ApGui apGui; private static DecimalFormat numberFormat = new DecimalFormat("#,###.##"); + /** + * The injected armor-set catalog, kept on the plugin so the still-static GUI-bootstrap + * facade below can hand it to {@link ApGui} without reaching for a static set map. + */ + private ArmorSetCatalog catalog; + @Override public void onEnable() { // Plugin startup logic @@ -31,10 +39,15 @@ public void onEnable() { // reset apgui to null for reloading apGui = null; SetupManager.setupFiles(new FileManager(instance)); - SetupManager.registerCommands(instance); - SetupManager.registerEvents(instance); - SetManager.loadSets(); - SetPlayerManager.init(); + // Composition root: build the injector once, then resolve the shared collaborators + // and hand them to the wiring below as instances instead of reaching for statics. + Injector injector = Guice.createInjector(new ArmorPlusModule(this)); + this.catalog = injector.getInstance(ArmorSetCatalog.class); + PlayerArmorSetService playerArmorSetService = injector.getInstance(PlayerArmorSetService.class); + SetupManager.registerCommands(instance, catalog); + SetupManager.registerEvents(instance, catalog, playerArmorSetService); + catalog.loadSets(); + playerArmorSetService.init(); // verify that the server is running vault so that eco features can be used if (Bukkit.getPluginManager().getPlugin("Vault") != null) { LogUtil.info("Vault found, hooking into it now..."); @@ -50,7 +63,7 @@ public void onEnable() { // placeholder hook if (Bukkit.getPluginManager().getPlugin("PlaceholderAPI") != null) { LogUtil.info("PlaceholderAPI found, registering expansion with it now..."); - new ArmorPlusExpansion(instance).register(); + new ArmorPlusExpansion(instance, catalog, playerArmorSetService).register(); } // metrics, just doing it here for now Metrics metrics = new Metrics(this, 7719); @@ -80,7 +93,7 @@ public static Economy eco() { public static ApGui getApGui() { if (apGui == null) { - apGui = new ApGui(ConfigManager.CONFIG.get().getConfigurationSection("gui")); + apGui = new ApGui(ConfigManager.CONFIG.get().getConfigurationSection("gui"), instance.catalog); } else { apGui.refresh(); } diff --git a/src/main/java/gg/steve/mc/ap/ArmorPlusModule.java b/src/main/java/gg/steve/mc/ap/ArmorPlusModule.java new file mode 100644 index 0000000..c183074 --- /dev/null +++ b/src/main/java/gg/steve/mc/ap/ArmorPlusModule.java @@ -0,0 +1,36 @@ +package gg.steve.mc.ap; + +import com.google.inject.AbstractModule; +import com.google.inject.Singleton; +import com.google.inject.TypeLiteral; +import gg.steve.mc.ap.armor.ArmorSetCatalog; +import gg.steve.mc.ap.armor.Set; +import gg.steve.mc.ap.model.player.PlayerArmorWearerRegistry; +import gg.steve.mc.ap.model.set.ArmorSetRegistry; +import gg.steve.mc.ap.player.PlayerArmorSetService; + +/** + * Guice bindings for the plugin's shared collaborators. + *

+ * The pure {@code model} registries carry no framework annotations, so they are bound here + * (Guice instantiates them via their no-arg constructors) rather than annotated in place - + * this keeps the domain layer free of any dependency-injection imports. The adapter-layer + * catalog and player service are constructor-injected singletons; a single instance of each + * is shared across every listener, command, and expansion the injector wires. + */ +public class ArmorPlusModule extends AbstractModule { + private final ArmorPlus plugin; + + public ArmorPlusModule(ArmorPlus plugin) { + this.plugin = plugin; + } + + @Override + protected void configure() { + bind(ArmorPlus.class).toInstance(plugin); + bind(new TypeLiteral>() {}).in(Singleton.class); + bind(PlayerArmorWearerRegistry.class).in(Singleton.class); + bind(ArmorSetCatalog.class).in(Singleton.class); + bind(PlayerArmorSetService.class).in(Singleton.class); + } +} diff --git a/src/main/java/gg/steve/mc/ap/armor/ArmorSetCatalog.java b/src/main/java/gg/steve/mc/ap/armor/ArmorSetCatalog.java new file mode 100644 index 0000000..77bf611 --- /dev/null +++ b/src/main/java/gg/steve/mc/ap/armor/ArmorSetCatalog.java @@ -0,0 +1,52 @@ +package gg.steve.mc.ap.armor; + +import com.google.inject.Inject; +import gg.steve.mc.ap.ArmorPlus; +import gg.steve.mc.ap.managers.ConfigManager; +import gg.steve.mc.ap.model.id.ArmorSetId; +import gg.steve.mc.ap.model.set.ArmorSetRegistry; +import gg.steve.mc.ap.utils.YamlFileUtil; + +import java.io.File; +import java.util.Map; + +/** + * The plugin's live catalog of armor sets, parsed from configuration and keyed by {@link ArmorSetId}. + *

+ * State lives in an injected {@link ArmorSetRegistry}; this class is the adapter that populates the + * registry from Bukkit-backed config files and reads it back for the platform layer. It is a shared + * singleton wired by Guice and passed to its callers as an injected instance, so there is no static + * map or global entry point to reset between reloads. + */ +public class ArmorSetCatalog { + private final ArmorSetRegistry registry; + private final ArmorPlus plugin; + + @Inject + public ArmorSetCatalog(ArmorSetRegistry registry, ArmorPlus plugin) { + this.registry = registry; + this.plugin = plugin; + } + + /** The registry this catalog owns, for callers that read the raw id-to-set view. */ + public ArmorSetRegistry getRegistry() { + return registry; + } + + public void loadSets() { + registry.clear(); + for (String set : ConfigManager.CONFIG.get().getStringList("loaded-sets")) { + YamlFileUtil fileUtil = new YamlFileUtil("sets" + File.separator + set + ".yml", plugin); + registry.register(ArmorSetId.of(set), new Set(set, fileUtil)); + } + } + + public Map getSets() { + return registry.getAll(); + } + + public Set getSet(String name) { + if (name == null || name.isEmpty()) return null; + return registry.get(ArmorSetId.of(name)).orElse(null); + } +} diff --git a/src/main/java/gg/steve/mc/ap/armor/SetManager.java b/src/main/java/gg/steve/mc/ap/armor/SetManager.java deleted file mode 100644 index 9d997e4..0000000 --- a/src/main/java/gg/steve/mc/ap/armor/SetManager.java +++ /dev/null @@ -1,44 +0,0 @@ -package gg.steve.mc.ap.armor; - -import gg.steve.mc.ap.ArmorPlus; -import gg.steve.mc.ap.managers.ConfigManager; -import gg.steve.mc.ap.model.id.ArmorSetId; -import gg.steve.mc.ap.model.set.ArmorSetRegistry; -import gg.steve.mc.ap.utils.YamlFileUtil; - -import java.io.File; -import java.util.Map; - -public class SetManager { - private static final SetManager INSTANCE = new SetManager(); - - private final ArmorSetRegistry registry = new ArmorSetRegistry<>(); - - /** The shared instance the static entry points below delegate to, so the registry state is held by an instance rather than a static map. */ - public static SetManager get() { - return INSTANCE; - } - - /** The registry this instance owns, for callers that can take the instance directly. */ - public ArmorSetRegistry getRegistry() { - return registry; - } - - public static void loadSets() { - ArmorSetRegistry registry = INSTANCE.registry; - registry.clear(); - for (String set : ConfigManager.CONFIG.get().getStringList("loaded-sets")) { - YamlFileUtil fileUtil = new YamlFileUtil("sets" + File.separator + set + ".yml", ArmorPlus.get()); - registry.register(ArmorSetId.of(set), new Set(set, fileUtil)); - } - } - - public static Map getSets() { - return INSTANCE.registry.getAll(); - } - - public static Set getSet(String name) { - if (name == null || name.isEmpty()) return null; - return INSTANCE.registry.get(ArmorSetId.of(name)).orElse(null); - } -} diff --git a/src/main/java/gg/steve/mc/ap/cmd/ApCmd.java b/src/main/java/gg/steve/mc/ap/cmd/ApCmd.java index cf8ff0e..e0d07b7 100644 --- a/src/main/java/gg/steve/mc/ap/cmd/ApCmd.java +++ b/src/main/java/gg/steve/mc/ap/cmd/ApCmd.java @@ -1,5 +1,6 @@ package gg.steve.mc.ap.cmd; +import gg.steve.mc.ap.armor.ArmorSetCatalog; import gg.steve.mc.ap.cmd.sub.GiveCmd; import gg.steve.mc.ap.cmd.sub.GuiCmd; import gg.steve.mc.ap.cmd.sub.HelpCmd; @@ -14,6 +15,11 @@ import java.util.List; public class ApCmd implements TabExecutor { + private final ArmorSetCatalog catalog; + + public ApCmd(ArmorSetCatalog catalog) { + this.catalog = catalog; + } @Override public boolean onCommand(CommandSender sender, Command command, String label, String[] args) { @@ -39,7 +45,7 @@ public boolean onCommand(CommandSender sender, Command command, String label, St break; case "g": case "give": - GiveCmd.give(sender, args); + GiveCmd.give(sender, args, catalog); break; default: CommandDebug.INVALID_COMMAND.message(sender); diff --git a/src/main/java/gg/steve/mc/ap/cmd/sub/GiveCmd.java b/src/main/java/gg/steve/mc/ap/cmd/sub/GiveCmd.java index 20c0fac..cf90d22 100644 --- a/src/main/java/gg/steve/mc/ap/cmd/sub/GiveCmd.java +++ b/src/main/java/gg/steve/mc/ap/cmd/sub/GiveCmd.java @@ -1,8 +1,8 @@ package gg.steve.mc.ap.cmd.sub; +import gg.steve.mc.ap.armor.ArmorSetCatalog; import gg.steve.mc.ap.armor.Piece; import gg.steve.mc.ap.armor.Set; -import gg.steve.mc.ap.armor.SetManager; import gg.steve.mc.ap.message.CommandDebug; import gg.steve.mc.ap.message.MessageType; import gg.steve.mc.ap.permission.PermissionNode; @@ -13,7 +13,7 @@ public class GiveCmd { - public static void give(CommandSender sender, String[] args) { + public static void give(CommandSender sender, String[] args, ArmorSetCatalog catalog) { // /ap g phantom nbdsteve helmet 1 if (args.length < 4) { CommandDebug.INVALID_NUMBER_OF_ARGUMENTS.message(sender); @@ -29,11 +29,11 @@ public static void give(CommandSender sender, String[] args) { return; } Set set; - if (SetManager.getSet(args[1]) == null) { + if (catalog.getSet(args[1]) == null) { CommandDebug.INVALID_SET.message(sender); return; } else { - set = SetManager.getSet(args[1]); + set = catalog.getSet(args[1]); } Piece piece = null; boolean all = false; diff --git a/src/main/java/gg/steve/mc/ap/gui/ApGui.java b/src/main/java/gg/steve/mc/ap/gui/ApGui.java index 5e20bf9..9099398 100644 --- a/src/main/java/gg/steve/mc/ap/gui/ApGui.java +++ b/src/main/java/gg/steve/mc/ap/gui/ApGui.java @@ -1,20 +1,23 @@ package gg.steve.mc.ap.gui; -import gg.steve.mc.ap.armor.SetManager; +import gg.steve.mc.ap.armor.ArmorSetCatalog; import gg.steve.mc.ap.utils.GuiItemUtil; import org.bukkit.configuration.ConfigurationSection; public class ApGui extends AbstractGui { private ConfigurationSection section; + private final ArmorSetCatalog catalog; /** * Constructor the create a new Gui * * @param section + * @param catalog the armor-set catalog used to resolve the set a slot action opens */ - public ApGui(ConfigurationSection section) { + public ApGui(ConfigurationSection section, ArmorSetCatalog catalog) { super(section, section.getString("type"), section.getInt("size")); this.section = section; + this.catalog = catalog; refresh(); } @@ -33,7 +36,7 @@ public void refresh() { player.closeInventory(); break; default: - SetManager.getSet(section.getString(entry + ".action")).openGui(player); + catalog.getSet(section.getString(entry + ".action")).openGui(player); break; } }); diff --git a/src/main/java/gg/steve/mc/ap/listener/ArmorBuffListener.java b/src/main/java/gg/steve/mc/ap/listener/ArmorBuffListener.java index 9ed3b31..fcd15b1 100644 --- a/src/main/java/gg/steve/mc/ap/listener/ArmorBuffListener.java +++ b/src/main/java/gg/steve/mc/ap/listener/ArmorBuffListener.java @@ -1,10 +1,10 @@ package gg.steve.mc.ap.listener; +import gg.steve.mc.ap.armor.ArmorSetCatalog; import gg.steve.mc.ap.armor.Set; -import gg.steve.mc.ap.armor.SetManager; import gg.steve.mc.ap.nbt.NBTItem; +import gg.steve.mc.ap.player.PlayerArmorSetService; import gg.steve.mc.ap.player.SetPlayer; -import gg.steve.mc.ap.player.SetPlayerManager; import org.bukkit.Material; import org.bukkit.entity.Arrow; import org.bukkit.entity.Player; @@ -16,6 +16,13 @@ import org.bukkit.event.entity.FoodLevelChangeEvent; public class ArmorBuffListener implements Listener { + private final ArmorSetCatalog catalog; + private final PlayerArmorSetService playerArmorSetService; + + public ArmorBuffListener(ArmorSetCatalog catalog, PlayerArmorSetService playerArmorSetService) { + this.catalog = catalog; + this.playerArmorSetService = playerArmorSetService; + } @EventHandler public void onHit(EntityDamageByEntityEvent event) { @@ -31,16 +38,16 @@ public void onHit(EntityDamageByEntityEvent event) { if (damager == null) { damager = (Player) event.getDamager(); } - if (!SetPlayerManager.isWearingSet(damager)) { + if (!playerArmorSetService.isWearingSet(damager)) { if (damager.getItemInHand() == null || damager.getItemInHand().getType().equals(Material.AIR)) return; NBTItem nbtItem = new NBTItem(damager.getItemInHand()); if (nbtItem.getString("armor+.set").equalsIgnoreCase("")) return; - Set set = SetManager.getSet(nbtItem.getString("armor+.set")); + Set set = catalog.getSet(nbtItem.getString("armor+.set")); set.getHandData().hitWithoutSetBuff(event, damager); return; } - SetPlayer player = SetPlayerManager.getSetPlayer(damager); + SetPlayer player = playerArmorSetService.getSetPlayer(damager); player.getSet().onHit(event, damager); } @@ -48,8 +55,8 @@ public void onHit(EntityDamageByEntityEvent event) { public void onDamage(EntityDamageByEntityEvent event) { if (event.isCancelled()) return; if (!(event.getEntity() instanceof Player)) return; - if (!SetPlayerManager.isWearingSet((Player) event.getEntity())) return; - SetPlayer player = SetPlayerManager.getSetPlayer((Player) event.getEntity()); + if (!playerArmorSetService.isWearingSet((Player) event.getEntity())) return; + SetPlayer player = playerArmorSetService.getSetPlayer((Player) event.getEntity()); player.getSet().onDamage(event); } @@ -57,9 +64,9 @@ public void onDamage(EntityDamageByEntityEvent event) { public void fall(EntityDamageEvent event) { if (event.isCancelled()) return; if (!(event.getEntity() instanceof Player)) return; - if (!SetPlayerManager.isWearingSet((Player) event.getEntity())) return; + if (!playerArmorSetService.isWearingSet((Player) event.getEntity())) return; if (!event.getCause().equals(EntityDamageEvent.DamageCause.FALL)) return; - SetPlayer player = SetPlayerManager.getSetPlayer((Player) event.getEntity()); + SetPlayer player = playerArmorSetService.getSetPlayer((Player) event.getEntity()); player.getSet().onFall(event); } @@ -67,8 +74,8 @@ public void fall(EntityDamageEvent event) { public void hunger(FoodLevelChangeEvent event) { if (event.isCancelled()) return; if (!(event.getEntity() instanceof Player)) return; - if (!SetPlayerManager.isWearingSet((Player) event.getEntity())) return; - SetPlayer player = SetPlayerManager.getSetPlayer((Player) event.getEntity()); + if (!playerArmorSetService.isWearingSet((Player) event.getEntity())) return; + SetPlayer player = playerArmorSetService.getSetPlayer((Player) event.getEntity()); player.getSet().onHungerDeplete(event); } @@ -76,8 +83,8 @@ public void hunger(FoodLevelChangeEvent event) { public void death(EntityDeathEvent event) { if (event.getEntity().getKiller() == null) return; Player killer = event.getEntity().getKiller(); - if (!SetPlayerManager.isWearingSet(killer)) return; - SetPlayer player = SetPlayerManager.getSetPlayer(killer); + if (!playerArmorSetService.isWearingSet(killer)) return; + SetPlayer player = playerArmorSetService.getSetPlayer(killer); player.getSet().onTargetDeath(event, killer); } } diff --git a/src/main/java/gg/steve/mc/ap/listener/PlayerCommandListener.java b/src/main/java/gg/steve/mc/ap/listener/PlayerCommandListener.java index 8f7dd0b..d16816b 100644 --- a/src/main/java/gg/steve/mc/ap/listener/PlayerCommandListener.java +++ b/src/main/java/gg/steve/mc/ap/listener/PlayerCommandListener.java @@ -1,8 +1,8 @@ package gg.steve.mc.ap.listener; +import gg.steve.mc.ap.armor.ArmorSetCatalog; import gg.steve.mc.ap.armor.Piece; import gg.steve.mc.ap.armor.Set; -import gg.steve.mc.ap.armor.SetManager; import gg.steve.mc.ap.message.CommandDebug; import gg.steve.mc.ap.message.MessageType; import gg.steve.mc.ap.model.id.ArmorSetId; @@ -15,19 +15,24 @@ import org.bukkit.inventory.ItemStack; public class PlayerCommandListener implements Listener { + private final ArmorSetCatalog catalog; + + public PlayerCommandListener(ArmorSetCatalog catalog) { + this.catalog = catalog; + } @EventHandler public void onCmd(PlayerCommandPreprocessEvent event) { String[] args = event.getMessage().split(" "); String setName = null; - for (ArmorSetId key : SetManager.getSets().keySet()) { + for (ArmorSetId key : catalog.getSets().keySet()) { if (args[0].equalsIgnoreCase("/" + key)) { setName = key.toString(); event.setCancelled(true); } } if (setName == null) return; - Set set = SetManager.getSet(setName); + Set set = catalog.getSet(setName); if (args.length == 1) { if (!PermissionNode.GUI.hasPermission(event.getPlayer())) { CommandDebug.INSUFFICIENT_PERMISSION.message(event.getPlayer(), PermissionNode.GUI.get()); diff --git a/src/main/java/gg/steve/mc/ap/listener/PlayerEquipListener.java b/src/main/java/gg/steve/mc/ap/listener/PlayerEquipListener.java index 679299b..8449d80 100644 --- a/src/main/java/gg/steve/mc/ap/listener/PlayerEquipListener.java +++ b/src/main/java/gg/steve/mc/ap/listener/PlayerEquipListener.java @@ -1,12 +1,12 @@ package gg.steve.mc.ap.listener; +import gg.steve.mc.ap.armor.ArmorSetCatalog; import gg.steve.mc.ap.armor.Piece; import gg.steve.mc.ap.armor.Set; -import gg.steve.mc.ap.armor.SetManager; import gg.steve.mc.ap.armorequipevent.ArmorEquipEvent; import gg.steve.mc.ap.managers.ConfigManager; import gg.steve.mc.ap.nbt.NBTItem; -import gg.steve.mc.ap.player.SetPlayerManager; +import gg.steve.mc.ap.player.PlayerArmorSetService; import gg.steve.mc.ap.utils.CommandUtil; import org.bukkit.Material; import org.bukkit.event.EventHandler; @@ -15,6 +15,13 @@ import org.bukkit.inventory.ItemStack; public class PlayerEquipListener implements Listener { + private final ArmorSetCatalog catalog; + private final PlayerArmorSetService playerArmorSetService; + + public PlayerEquipListener(ArmorSetCatalog catalog, PlayerArmorSetService playerArmorSetService) { + this.catalog = catalog; + this.playerArmorSetService = playerArmorSetService; + } @EventHandler public void equip(ArmorEquipEvent event) { @@ -43,7 +50,7 @@ public void equip(ArmorEquipEvent event) { } } String setName = nbtItem.getString("armor+.set"); - Set set = SetManager.getSet(setName); + Set set = catalog.getSet(setName); Piece piece = null; for (Piece pieces : set.getSetPieces().keySet()) { if (event.getNewArmorPiece().getType().equals(set.getPiece(pieces).getType())) piece = pieces; @@ -52,16 +59,16 @@ public void equip(ArmorEquipEvent event) { CommandUtil.execute(set.getConfig().getStringList("set-pieces." + piece.name().toLowerCase() + ".commands.apply"), event.getPlayer()); } if (set.isWearingSet(event.getPlayer(), event.getType(), event.getNewArmorPiece())) { - SetPlayerManager.addSetPlayer(event.getPlayer(), setName); + playerArmorSetService.addSetPlayer(event.getPlayer(), setName); set.apply(event.getPlayer()); } } @EventHandler public void join(PlayerJoinEvent event) { - for (Set set : SetManager.getSets().values()) { + for (Set set : catalog.getSets().values()) { if (!set.isWearingSet(event.getPlayer(), null, null)) continue; - SetPlayerManager.addSetPlayer(event.getPlayer(), set.getName()); + playerArmorSetService.addSetPlayer(event.getPlayer(), set.getName()); set.apply(event.getPlayer()); return; } diff --git a/src/main/java/gg/steve/mc/ap/listener/PlayerUnequipListener.java b/src/main/java/gg/steve/mc/ap/listener/PlayerUnequipListener.java index b8eb134..f1e757c 100644 --- a/src/main/java/gg/steve/mc/ap/listener/PlayerUnequipListener.java +++ b/src/main/java/gg/steve/mc/ap/listener/PlayerUnequipListener.java @@ -1,12 +1,12 @@ package gg.steve.mc.ap.listener; +import gg.steve.mc.ap.armor.ArmorSetCatalog; import gg.steve.mc.ap.armor.Piece; import gg.steve.mc.ap.armor.Set; -import gg.steve.mc.ap.armor.SetManager; import gg.steve.mc.ap.armorequipevent.ArmorEquipEvent; import gg.steve.mc.ap.managers.ConfigManager; import gg.steve.mc.ap.nbt.NBTItem; -import gg.steve.mc.ap.player.SetPlayerManager; +import gg.steve.mc.ap.player.PlayerArmorSetService; import gg.steve.mc.ap.utils.CommandUtil; import org.bukkit.Material; import org.bukkit.event.EventHandler; @@ -14,6 +14,13 @@ import org.bukkit.event.player.PlayerQuitEvent; public class PlayerUnequipListener implements Listener { + private final ArmorSetCatalog catalog; + private final PlayerArmorSetService playerArmorSetService; + + public PlayerUnequipListener(ArmorSetCatalog catalog, PlayerArmorSetService playerArmorSetService) { + this.catalog = catalog; + this.playerArmorSetService = playerArmorSetService; + } @EventHandler public void unEquip(ArmorEquipEvent event) { @@ -26,7 +33,7 @@ public void unEquip(ArmorEquipEvent event) { event.setCancelled(true); } String name = nbtItem.getString("armor+.set"); - Set set = SetManager.getSet(name); + Set set = catalog.getSet(name); Piece piece = null; for (Piece pieces : set.getSetPieces().keySet()) { if (event.getOldArmorPiece().getType().equals(set.getPiece(pieces).getType())) piece = pieces; @@ -35,16 +42,16 @@ public void unEquip(ArmorEquipEvent event) { CommandUtil.execute(set.getConfig().getStringList("set-pieces." + piece.name().toLowerCase() + ".commands.remove"), event.getPlayer()); } if (set.isWearingSet(event.getPlayer(), event.getType(), event.getOldArmorPiece())) { - SetPlayerManager.removeSetPlayer(event.getPlayer()); + playerArmorSetService.removeSetPlayer(event.getPlayer()); set.remove(event.getPlayer()); } } @EventHandler public void quit(PlayerQuitEvent event) { - for (Set set : SetManager.getSets().values()) { + for (Set set : catalog.getSets().values()) { if (!set.isWearingSet(event.getPlayer(), null, null)) continue; - SetPlayerManager.removeSetPlayer(event.getPlayer()); + playerArmorSetService.removeSetPlayer(event.getPlayer()); set.remove(event.getPlayer()); return; } diff --git a/src/main/java/gg/steve/mc/ap/managers/SetupManager.java b/src/main/java/gg/steve/mc/ap/managers/SetupManager.java index b771073..664db4c 100644 --- a/src/main/java/gg/steve/mc/ap/managers/SetupManager.java +++ b/src/main/java/gg/steve/mc/ap/managers/SetupManager.java @@ -1,12 +1,14 @@ package gg.steve.mc.ap.managers; import gg.steve.mc.ap.ArmorPlus; +import gg.steve.mc.ap.armor.ArmorSetCatalog; import gg.steve.mc.ap.armorequipevent.ArmorListener; import gg.steve.mc.ap.cmd.ApCmd; import gg.steve.mc.ap.data.utils.EngineerAttackUtil; import gg.steve.mc.ap.data.utils.TravellerAttackUtil; import gg.steve.mc.ap.gui.GuiClickListener; import gg.steve.mc.ap.listener.*; +import gg.steve.mc.ap.player.PlayerArmorSetService; import org.bukkit.plugin.Plugin; import org.bukkit.plugin.PluginManager; @@ -31,23 +33,25 @@ public static void setupFiles(FileManager fileManager) { } } - public static void registerCommands(ArmorPlus instance) { - instance.getCommand("ap").setExecutor(new ApCmd()); + public static void registerCommands(ArmorPlus instance, ArmorSetCatalog catalog) { + instance.getCommand("ap").setExecutor(new ApCmd(catalog)); } /** - * Register all of the events for the plugin + * Register all of the events for the plugin. * - * @param instance Plugin, the main plugin instance + * @param instance Plugin, the main plugin instance + * @param catalog the shared armor-set catalog the listeners resolve sets from + * @param playerArmorSetService the shared worn-set tracker the listeners record wearers in */ - public static void registerEvents(Plugin instance) { + public static void registerEvents(Plugin instance, ArmorSetCatalog catalog, PlayerArmorSetService playerArmorSetService) { PluginManager pm = instance.getServer().getPluginManager(); pm.registerEvents(new ArmorListener(ConfigManager.BLOCKED.get().getStringList("blocked")), instance); // pm.registerEvents(new DispenserArmorListener(), instance); - pm.registerEvents(new PlayerEquipListener(), instance); - pm.registerEvents(new PlayerUnequipListener(), instance); - pm.registerEvents(new PlayerCommandListener(), instance); - pm.registerEvents(new ArmorBuffListener(), instance); + pm.registerEvents(new PlayerEquipListener(catalog, playerArmorSetService), instance); + pm.registerEvents(new PlayerUnequipListener(catalog, playerArmorSetService), instance); + pm.registerEvents(new PlayerCommandListener(catalog), instance); + pm.registerEvents(new ArmorBuffListener(catalog, playerArmorSetService), instance); pm.registerEvents(new GuiClickListener(), instance); pm.registerEvents(new ArmorSwitchListener(), instance); pm.registerEvents(new TravellerAttackUtil(), instance); diff --git a/src/main/java/gg/steve/mc/ap/papi/ArmorPlusExpansion.java b/src/main/java/gg/steve/mc/ap/papi/ArmorPlusExpansion.java index bc1a4f1..c2ea10f 100644 --- a/src/main/java/gg/steve/mc/ap/papi/ArmorPlusExpansion.java +++ b/src/main/java/gg/steve/mc/ap/papi/ArmorPlusExpansion.java @@ -1,10 +1,10 @@ package gg.steve.mc.ap.papi; import gg.steve.mc.ap.ArmorPlus; +import gg.steve.mc.ap.armor.ArmorSetCatalog; import gg.steve.mc.ap.armor.Set; -import gg.steve.mc.ap.armor.SetManager; import gg.steve.mc.ap.managers.ConfigManager; -import gg.steve.mc.ap.player.SetPlayerManager; +import gg.steve.mc.ap.player.PlayerArmorSetService; import me.clip.placeholderapi.expansion.PlaceholderExpansion; import org.bukkit.ChatColor; import org.bukkit.entity.Player; @@ -13,9 +13,13 @@ public class ArmorPlusExpansion extends PlaceholderExpansion { private ArmorPlus instance; + private final ArmorSetCatalog catalog; + private final PlayerArmorSetService playerArmorSetService; - public ArmorPlusExpansion(ArmorPlus instance) { + public ArmorPlusExpansion(ArmorPlus instance, ArmorSetCatalog catalog, PlayerArmorSetService playerArmorSetService) { this.instance = instance; + this.catalog = catalog; + this.playerArmorSetService = playerArmorSetService; } @Override @@ -48,25 +52,25 @@ public String onPlaceholderRequest(Player player, String identifier) { if (player == null) return ""; String fallback = ConfigManager.CONFIG.get().getString("fallback-placeholder"); if (identifier.equalsIgnoreCase("current_name")) { - if (SetPlayerManager.isWearingSet(player)) { - return SetPlayerManager.getSetPlayer(player).getSet().getName(); + if (playerArmorSetService.isWearingSet(player)) { + return playerArmorSetService.getSetPlayer(player).getSet().getName(); } else { return fallback; } } if (identifier.contains("current_increase")) { double increase = 0; - if (SetPlayerManager.getSetFromHand(player) != null) { - Set set = SetPlayerManager.getSetFromHand(player); + if (playerArmorSetService.getSetFromHand(player) != null) { + Set set = playerArmorSetService.getSetFromHand(player); if (set.getHandData() != null) { if (identifier.endsWith("raw")) { - if (set.getHandData().requiresFullSet() && SetPlayerManager.isWearingSet(player) && SetPlayerManager.getSetPlayer(player).getSet().equals(set)) { + if (set.getHandData().requiresFullSet() && playerArmorSetService.isWearingSet(player) && playerArmorSetService.getSetPlayer(player).getSet().equals(set)) { increase += set.getHandData().getIncrease() - 1; } else if (!set.getHandData().requiresFullSet()) { increase += set.getHandData().getIncrease() - 1; } } else if (identifier.endsWith("percentage")) { - if (set.getHandData().requiresFullSet() && SetPlayerManager.isWearingSet(player) && SetPlayerManager.getSetPlayer(player).getSet().equals(set)) { + if (set.getHandData().requiresFullSet() && playerArmorSetService.isWearingSet(player) && playerArmorSetService.getSetPlayer(player).getSet().equals(set)) { increase += (set.getHandData().getIncrease() - 1) * 100; } else if (!set.getHandData().requiresFullSet()) { increase += (set.getHandData().getIncrease() - 1) * 100; @@ -74,8 +78,8 @@ public String onPlaceholderRequest(Player player, String identifier) { } } } - if (SetPlayerManager.isWearingSet(player)) { - Set set = SetPlayerManager.getSetPlayer(player).getSet(); + if (playerArmorSetService.isWearingSet(player)) { + Set set = playerArmorSetService.getSetPlayer(player).getSet(); if (set.getBasicData().getIncrease() == -1 && increase == 0) { return "Default"; } @@ -85,7 +89,7 @@ public String onPlaceholderRequest(Player player, String identifier) { if (set.getBasicData().getIncrease() != -1) increase += (set.getBasicData().getIncrease() - 1) * 100; } } - if (!SetPlayerManager.isWearingSet(player) && increase == 0) { + if (!playerArmorSetService.isWearingSet(player) && increase == 0) { return fallback; } if (identifier.endsWith("raw")) { @@ -97,46 +101,46 @@ public String onPlaceholderRequest(Player player, String identifier) { } } if (identifier.contains("current_reduction")) { - if (SetPlayerManager.isWearingSet(player)) { - if (SetPlayerManager.getSetPlayer(player).getSet().getBasicData().getReduction() == -1) { + if (playerArmorSetService.isWearingSet(player)) { + if (playerArmorSetService.getSetPlayer(player).getSet().getBasicData().getReduction() == -1) { return "Default"; } if (identifier.endsWith("raw")) { - return String.valueOf(SetPlayerManager.getSetPlayer(player).getSet().getBasicData().getReduction()); + return String.valueOf(playerArmorSetService.getSetPlayer(player).getSet().getBasicData().getReduction()); } else if (identifier.endsWith("formatted")) { - return ArmorPlus.formatNumber(SetPlayerManager.getSetPlayer(player).getSet().getBasicData().getReduction()); + return ArmorPlus.formatNumber(playerArmorSetService.getSetPlayer(player).getSet().getBasicData().getReduction()); } else if (identifier.endsWith("percentage")) { - return ArmorPlus.formatNumber(Math.abs((SetPlayerManager.getSetPlayer(player).getSet().getBasicData().getReduction() - 1) * 100)) + "%"; + return ArmorPlus.formatNumber(Math.abs((playerArmorSetService.getSetPlayer(player).getSet().getBasicData().getReduction() - 1) * 100)) + "%"; } } else { return fallback; } } if (identifier.contains("current_kb")) { - if (SetPlayerManager.isWearingSet(player)) { - if (SetPlayerManager.getSetPlayer(player).getSet().getBasicData().getKb() == -1) { + if (playerArmorSetService.isWearingSet(player)) { + if (playerArmorSetService.getSetPlayer(player).getSet().getBasicData().getKb() == -1) { return "Default"; } if (identifier.endsWith("raw")) { - return String.valueOf(SetPlayerManager.getSetPlayer(player).getSet().getBasicData().getKb()); + return String.valueOf(playerArmorSetService.getSetPlayer(player).getSet().getBasicData().getKb()); } else if (identifier.endsWith("formatted")) { - return ArmorPlus.formatNumber(SetPlayerManager.getSetPlayer(player).getSet().getBasicData().getKb()); + return ArmorPlus.formatNumber(playerArmorSetService.getSetPlayer(player).getSet().getBasicData().getKb()); } } else { return fallback; } } if (identifier.contains("current_health")) { - if (SetPlayerManager.isWearingSet(player)) { - if (SetPlayerManager.getSetPlayer(player).getSet().getBasicData().getHealth() == -1) { + if (playerArmorSetService.isWearingSet(player)) { + if (playerArmorSetService.getSetPlayer(player).getSet().getBasicData().getHealth() == -1) { return "Default"; } if (identifier.endsWith("raw")) { - return String.valueOf(SetPlayerManager.getSetPlayer(player).getSet().getBasicData().getHealth()); + return String.valueOf(playerArmorSetService.getSetPlayer(player).getSet().getBasicData().getHealth()); } else if (identifier.endsWith("formatted")) { - return ArmorPlus.formatNumber(SetPlayerManager.getSetPlayer(player).getSet().getBasicData().getHealth()); + return ArmorPlus.formatNumber(playerArmorSetService.getSetPlayer(player).getSet().getBasicData().getHealth()); } else if (identifier.endsWith("hearts")) { - return ArmorPlus.formatNumber(SetPlayerManager.getSetPlayer(player).getSet().getBasicData().getHealth() / 2) + ChatColor.RED + "❤"; + return ArmorPlus.formatNumber(playerArmorSetService.getSetPlayer(player).getSet().getBasicData().getHealth() / 2) + ChatColor.RED + "❤"; } } else { return fallback; @@ -145,15 +149,15 @@ public String onPlaceholderRequest(Player player, String identifier) { if (identifier.endsWith("pieces_wearing")) { String[] parts = identifier.split("_"); Set set; - if ((set = SetManager.getSet(parts[0])) == null) { + if ((set = catalog.getSet(parts[0])) == null) { return "Invalid set"; } - return String.valueOf(SetPlayerManager.getPiecesWearing(set, player)); + return String.valueOf(playerArmorSetService.getPiecesWearing(set, player)); } if (identifier.endsWith("pieces_total")) { String[] parts = identifier.split("_"); Set set; - if ((set = SetManager.getSet(parts[0])) == null) { + if ((set = catalog.getSet(parts[0])) == null) { return "Invalid set"; } return String.valueOf(set.getSetPieces().size()); diff --git a/src/main/java/gg/steve/mc/ap/player/SetPlayerManager.java b/src/main/java/gg/steve/mc/ap/player/PlayerArmorSetService.java similarity index 57% rename from src/main/java/gg/steve/mc/ap/player/SetPlayerManager.java rename to src/main/java/gg/steve/mc/ap/player/PlayerArmorSetService.java index 0544907..b8aeb62 100644 --- a/src/main/java/gg/steve/mc/ap/player/SetPlayerManager.java +++ b/src/main/java/gg/steve/mc/ap/player/PlayerArmorSetService.java @@ -1,8 +1,9 @@ package gg.steve.mc.ap.player; +import com.google.inject.Inject; +import gg.steve.mc.ap.armor.ArmorSetCatalog; import gg.steve.mc.ap.armor.Piece; import gg.steve.mc.ap.armor.Set; -import gg.steve.mc.ap.armor.SetManager; import gg.steve.mc.ap.model.id.ArmorSetId; import gg.steve.mc.ap.model.id.PlayerId; import gg.steve.mc.ap.model.player.PlayerArmorWearerRegistry; @@ -11,40 +12,57 @@ import org.bukkit.Material; import org.bukkit.entity.Player; -public class SetPlayerManager { - private static final PlayerArmorWearerRegistry registry = new PlayerArmorWearerRegistry(); +/** + * Tracks which online players are currently wearing which armor set and answers the + * platform's questions about worn sets. + *

+ * Wearer state lives in the injected pure {@link PlayerArmorWearerRegistry}; the injected + * {@link ArmorSetCatalog} resolves set names to {@link Set} instances. This is a shared + * singleton passed to its callers (listeners and the placeholder expansion) as an injected + * instance, so tests construct it with their own registry and catalog rather than resetting + * global state. + */ +public class PlayerArmorSetService { + private final PlayerArmorWearerRegistry registry; + private final ArmorSetCatalog catalog; - public static void init() { + @Inject + public PlayerArmorSetService(PlayerArmorWearerRegistry registry, ArmorSetCatalog catalog) { + this.registry = registry; + this.catalog = catalog; + } + + public void init() { registry.clear(); for (Player player : Bukkit.getOnlinePlayers()) { - for (Set set : SetManager.getSets().values()) { + for (Set set : catalog.getSets().values()) { if (!set.isWearingSet(player, null, null)) continue; - SetPlayerManager.addSetPlayer(player, set.getName()); + addSetPlayer(player, set.getName()); set.apply(player); return; } } } - public static void addSetPlayer(Player player, String setName) { + public void addSetPlayer(Player player, String setName) { registry.add(PlayerId.of(player.getUniqueId()), ArmorSetId.of(setName)); } - public static void removeSetPlayer(Player player) { + public void removeSetPlayer(Player player) { registry.remove(PlayerId.of(player.getUniqueId())); } - public static boolean isWearingSet(Player player) { + public boolean isWearingSet(Player player) { return registry.isWearing(PlayerId.of(player.getUniqueId())); } - public static SetPlayer getSetPlayer(Player player) { + public SetPlayer getSetPlayer(Player player) { return registry.get(PlayerId.of(player.getUniqueId())) - .map(wearer -> new SetPlayer(player, wearer.getSetId().toString())) + .map(wearer -> new SetPlayer(player, catalog.getSet(wearer.getSetId().toString()))) .orElse(null); } - public static int getPiecesWearing(Set set, Player player) { + public int getPiecesWearing(Set set, Player player) { int wearing = 0; for (Piece piece : set.getSetPieces().keySet()) { switch (piece) { @@ -68,10 +86,10 @@ public static int getPiecesWearing(Set set, Player player) { return wearing; } - public static Set getSetFromHand(Player player) { + public Set getSetFromHand(Player player) { if (player.getItemInHand() == null || player.getItemInHand().getType().equals(Material.AIR)) return null; NBTItem hand = new NBTItem(player.getItemInHand()); if (hand.getString("armor+.set").equalsIgnoreCase("")) return null; - return SetManager.getSet(hand.getString("armor+.set")); + return catalog.getSet(hand.getString("armor+.set")); } } diff --git a/src/main/java/gg/steve/mc/ap/player/SetPlayer.java b/src/main/java/gg/steve/mc/ap/player/SetPlayer.java index 034d48a..1f23978 100644 --- a/src/main/java/gg/steve/mc/ap/player/SetPlayer.java +++ b/src/main/java/gg/steve/mc/ap/player/SetPlayer.java @@ -1,16 +1,15 @@ package gg.steve.mc.ap.player; import gg.steve.mc.ap.armor.Set; -import gg.steve.mc.ap.armor.SetManager; import org.bukkit.entity.Player; public class SetPlayer { private Player player; private Set set; - public SetPlayer(Player player, String setName) { + public SetPlayer(Player player, Set set) { this.player = player; - this.set = SetManager.getSet(setName); + this.set = set; } public Player getPlayer() { diff --git a/src/test/java/gg/steve/mc/ap/armor/ArmorSetCatalogTest.java b/src/test/java/gg/steve/mc/ap/armor/ArmorSetCatalogTest.java new file mode 100644 index 0000000..3cc1699 --- /dev/null +++ b/src/test/java/gg/steve/mc/ap/armor/ArmorSetCatalogTest.java @@ -0,0 +1,50 @@ +package gg.steve.mc.ap.armor; + +import gg.steve.mc.ap.ArmorPlus; +import gg.steve.mc.ap.model.set.ArmorSetRegistry; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Characterization tests for {@link ArmorSetCatalog#getSet(String)} pinning the exact + * null contract that the registry extraction had to preserve. + *

+ * Real callers (GiveCmd, PlayerEquipListener, ArmorPlusExpansion) treat a null return as + * "no such set" and must never see an exception for a missing/blank name. Because the + * {@code ArmorSetRegistry} keys on {@code ArmorSetId} - which rejects null/empty via + * commons-lang3 {@code Validate} - {@code getSet} guards those inputs before constructing the id. + * These tests prove that guard preserves the original {@code map.get()} behavior on a catalog + * built from an empty registry. + */ +@ExtendWith(MockitoExtension.class) +class ArmorSetCatalogTest { + + @Mock private ArmorPlus plugin; + + private ArmorSetCatalog catalog; + + @BeforeEach + void setUp() { + catalog = new ArmorSetCatalog(new ArmorSetRegistry<>(), plugin); + } + + @Test + void getSet_nullName_returnsNullNotThrows() { + assertNull(catalog.getSet(null)); + } + + @Test + void getSet_emptyName_returnsNullNotThrows() { + assertNull(catalog.getSet("")); + } + + @Test + void getSet_unknownName_returnsNull() { + assertNull(catalog.getSet("no-such-set")); + } +} diff --git a/src/test/java/gg/steve/mc/ap/armor/SetManagerTest.java b/src/test/java/gg/steve/mc/ap/armor/SetManagerTest.java deleted file mode 100644 index 69c06b4..0000000 --- a/src/test/java/gg/steve/mc/ap/armor/SetManagerTest.java +++ /dev/null @@ -1,33 +0,0 @@ -package gg.steve.mc.ap.armor; - -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.*; - -/** - * Characterization tests for {@link SetManager#getSet(String)} pinning the exact - * null contract that the 4E registry extraction had to preserve. - *

- * Real callers (GiveCmd, PlayerEquipListener, ArmorPlusExpansion) treat a null return as - * "no such set" and must never see an exception for a missing/blank name. Because the new - * {@code ArmorSetRegistry} keys on {@code ArmorSetId} - which rejects null/empty via - * commons-lang3 {@code Validate} - {@code getSet} guards those inputs before constructing the id. - * These tests prove that guard preserves the original {@code map.get()} behavior. - */ -class SetManagerTest { - - @Test - void getSet_nullName_returnsNullNotThrows() { - assertNull(SetManager.getSet(null)); - } - - @Test - void getSet_emptyName_returnsNullNotThrows() { - assertNull(SetManager.getSet("")); - } - - @Test - void getSet_unknownName_returnsNull() { - assertNull(SetManager.getSet("no-such-set")); - } -} diff --git a/src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java b/src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java index 3aba995..3d2b426 100644 --- a/src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java +++ b/src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java @@ -1,13 +1,14 @@ package gg.steve.mc.ap.listener; +import gg.steve.mc.ap.ArmorPlus; +import gg.steve.mc.ap.armor.ArmorSetCatalog; import gg.steve.mc.ap.armor.Set; -import gg.steve.mc.ap.armor.SetManager; import gg.steve.mc.ap.managers.FileManager; import gg.steve.mc.ap.model.id.ArmorSetId; +import gg.steve.mc.ap.model.set.ArmorSetRegistry; import org.bukkit.configuration.file.YamlConfiguration; import org.bukkit.entity.Player; import org.bukkit.event.player.PlayerCommandPreprocessEvent; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -29,17 +30,15 @@ /** * End-to-end style test of {@link PlayerCommandListener}, the only player-facing surface the - * 4E registry extraction changed. Before the change, {@code SetManager.getSets()} returned a - * {@code Map} and the listener iterated raw {@code String} keys; now it returns the - * registry's {@code Map} view and the listener iterates {@code ArmorSetId} keys, - * using {@code key.toString()} to recover the raw set-name string. + * registry extraction changed. The listener iterates the catalog's {@code Map} + * view and uses {@code key.toString()} to recover the raw set-name string. *

* These tests fire real {@link PlayerCommandPreprocessEvent}s (exactly what Bukkit delivers when a - * player types a command in chat) at the real listener, wired to the real shared {@link SetManager} - * registry, and assert the interception behaves identically: the set name is matched - * case-insensitively, the event is cancelled, the matching set's GUI is opened, and unrelated - * commands pass straight through. A transcript of each simulated keystroke and its outcome is - * written to the evidence directory so a reviewer can see the player experience directly. + * player types a command in chat) at the real listener, wired to an {@link ArmorSetCatalog} the + * test constructs and populates directly, and assert the interception behaves identically: the set + * name is matched case-insensitively, the event is cancelled, the matching set's GUI is opened, and + * unrelated commands pass straight through. A transcript of each simulated keystroke and its outcome + * is written to the evidence directory so a reviewer can see the player experience directly. */ @ExtendWith(MockitoExtension.class) class PlayerCommandListenerTest { @@ -53,21 +52,19 @@ class PlayerCommandListenerTest { @Mock private Set dragonSet; @Mock private Set knightSet; @Mock private YamlConfiguration permissionsConfig; + @Mock private ArmorPlus plugin; - private final PlayerCommandListener listener = new PlayerCommandListener(); + private ArmorSetCatalog catalog; + private PlayerCommandListener listener; private final List transcript = new ArrayList<>(); @BeforeEach void setUp() { - // Register two sets into the real shared registry, exactly as loadSets() would. - SetManager.get().getRegistry().clear(); - SetManager.get().getRegistry().register(ArmorSetId.of("dragon"), dragonSet); - SetManager.get().getRegistry().register(ArmorSetId.of("knight"), knightSet); - } - - @AfterEach - void tearDown() { - SetManager.get().getRegistry().clear(); + // Register two sets into the catalog's registry, exactly as loadSets() would. + catalog = new ArmorSetCatalog(new ArmorSetRegistry<>(), plugin); + catalog.getRegistry().register(ArmorSetId.of("dragon"), dragonSet); + catalog.getRegistry().register(ArmorSetId.of("knight"), knightSet); + listener = new PlayerCommandListener(catalog); } /** Fire a command at the real listener and record the observable player-facing outcome. */ @@ -84,7 +81,7 @@ private PlayerCommandPreprocessEvent type(String typed) { private void writeTranscript(String title) { List lines = new ArrayList<>(); lines.add("=== " + title + " ==="); - lines.add("Registry keys (ArmorSetId): " + new ArrayList<>(SetManager.getSets().keySet())); + lines.add("Registry keys (ArmorSetId): " + new ArrayList<>(catalog.getSets().keySet())); lines.addAll(transcript); try { Files.createDirectories(EVIDENCE.getParent()); diff --git a/src/test/java/gg/steve/mc/ap/player/SetPlayerManagerTest.java b/src/test/java/gg/steve/mc/ap/player/PlayerArmorSetServiceTest.java similarity index 68% rename from src/test/java/gg/steve/mc/ap/player/SetPlayerManagerTest.java rename to src/test/java/gg/steve/mc/ap/player/PlayerArmorSetServiceTest.java index fa467e9..099f930 100644 --- a/src/test/java/gg/steve/mc/ap/player/SetPlayerManagerTest.java +++ b/src/test/java/gg/steve/mc/ap/player/PlayerArmorSetServiceTest.java @@ -1,17 +1,16 @@ package gg.steve.mc.ap.player; +import gg.steve.mc.ap.armor.ArmorSetCatalog; import gg.steve.mc.ap.armor.Piece; import gg.steve.mc.ap.armor.Set; -import gg.steve.mc.ap.armor.SetManager; +import gg.steve.mc.ap.model.player.PlayerArmorWearerRegistry; import org.bukkit.entity.Player; import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.PlayerInventory; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; -import org.mockito.MockedStatic; import org.mockito.junit.jupiter.MockitoExtension; import java.util.LinkedHashMap; @@ -22,86 +21,81 @@ import static org.mockito.Mockito.*; /** - * Characterization tests for SetPlayerManager state transitions. - * Pins add/remove/isWearing/getPiecesWearing before 4E extracts WearerRegistry. + * Characterization tests for PlayerArmorSetService state transitions. + * Pins add/remove/isWearing/getPiecesWearing behavior. Formerly exercised the static + * SetPlayerManager with a {@code MockedStatic}; now the service holds an injected + * wearer registry and catalog, so each test constructs a fresh instance with no static reset. */ @ExtendWith(MockitoExtension.class) -class SetPlayerManagerTest { +class PlayerArmorSetServiceTest { @Mock private Player player1; @Mock private Player player2; @Mock private Set mockSet; + @Mock private ArmorSetCatalog catalog; private final UUID uuid1 = UUID.randomUUID(); private final UUID uuid2 = UUID.randomUUID(); - private MockedStatic setManagerMock; + private PlayerArmorSetService service; @BeforeEach void setUp() { - when(player1.getUniqueId()).thenReturn(uuid1); + lenient().when(player1.getUniqueId()).thenReturn(uuid1); lenient().when(player2.getUniqueId()).thenReturn(uuid2); - // SetPlayer constructor calls SetManager.getSet(name) - mock it - setManagerMock = mockStatic(SetManager.class); - setManagerMock.when(() -> SetManager.getSet(anyString())).thenReturn(mockSet); + // getSetPlayer resolves the worn set name through the catalog. + lenient().when(catalog.getSet(anyString())).thenReturn(mockSet); - // Reset the static state by removing any players - SetPlayerManager.removeSetPlayer(player1); - SetPlayerManager.removeSetPlayer(player2); - } - - @AfterEach - void tearDown() { - setManagerMock.close(); + service = new PlayerArmorSetService(new PlayerArmorWearerRegistry(), catalog); } @Test void addSetPlayer_playerNotWearing_addsSuccessfully() { - SetPlayerManager.addSetPlayer(player1, "warrior"); + service.addSetPlayer(player1, "warrior"); - assertTrue(SetPlayerManager.isWearingSet(player1)); + assertTrue(service.isWearingSet(player1)); } @Test void addSetPlayer_playerAlreadyWearing_replacesExisting() { - SetPlayerManager.addSetPlayer(player1, "warrior"); - SetPlayerManager.addSetPlayer(player1, "mage"); + service.addSetPlayer(player1, "warrior"); + service.addSetPlayer(player1, "mage"); - assertTrue(SetPlayerManager.isWearingSet(player1)); - SetPlayer sp = SetPlayerManager.getSetPlayer(player1); + assertTrue(service.isWearingSet(player1)); + SetPlayer sp = service.getSetPlayer(player1); assertNotNull(sp); } @Test void removeSetPlayer_playerWearing_removesSuccessfully() { - SetPlayerManager.addSetPlayer(player1, "warrior"); - SetPlayerManager.removeSetPlayer(player1); + service.addSetPlayer(player1, "warrior"); + service.removeSetPlayer(player1); - assertFalse(SetPlayerManager.isWearingSet(player1)); + assertFalse(service.isWearingSet(player1)); } @Test void removeSetPlayer_playerNotWearing_noError() { - assertDoesNotThrow(() -> SetPlayerManager.removeSetPlayer(player1)); + assertDoesNotThrow(() -> service.removeSetPlayer(player1)); } @Test void isWearingSet_uninitializedMap_returnsFalse() { - assertFalse(SetPlayerManager.isWearingSet(player2)); + assertFalse(service.isWearingSet(player2)); } @Test void multiplePlayers_independentTracking() { - SetPlayerManager.addSetPlayer(player1, "warrior"); - SetPlayerManager.addSetPlayer(player2, "mage"); + service.addSetPlayer(player1, "warrior"); + service.addSetPlayer(player2, "mage"); - assertTrue(SetPlayerManager.isWearingSet(player1)); - assertTrue(SetPlayerManager.isWearingSet(player2)); + assertTrue(service.isWearingSet(player1)); + assertTrue(service.isWearingSet(player2)); - SetPlayerManager.removeSetPlayer(player1); - assertFalse(SetPlayerManager.isWearingSet(player1)); - assertTrue(SetPlayerManager.isWearingSet(player2)); + service.removeSetPlayer(player1); + assertFalse(service.isWearingSet(player1)); + assertTrue(service.isWearingSet(player2)); } // --- getPiecesWearing characterization --- @@ -132,7 +126,7 @@ void getPiecesWearing_allPiecesVerified_countsAll( when(set.verifyPiece(boots)).thenReturn(true); when(set.verifyPiece(hand)).thenReturn(true); - int result = SetPlayerManager.getPiecesWearing(set, player1); + int result = service.getPiecesWearing(set, player1); assertEquals(5, result); } @@ -156,7 +150,7 @@ void getPiecesWearing_noPiecesVerified_returnsZero( when(set.verifyPiece(any(ItemStack.class))).thenReturn(false); - int result = SetPlayerManager.getPiecesWearing(set, player1); + int result = service.getPiecesWearing(set, player1); assertEquals(0, result); } @@ -183,7 +177,7 @@ void getPiecesWearing_partialSet_countsOnlyVerified( when(set.verifyPiece(legs)).thenReturn(true); when(set.verifyPiece(boots)).thenReturn(false); - int result = SetPlayerManager.getPiecesWearing(set, player1); + int result = service.getPiecesWearing(set, player1); assertEquals(2, result); } } From 9e089f2f0e52b9eea7d51b59a07c7f48bb60443c Mon Sep 17 00:00:00 2001 From: Stephen Goodhill Date: Thu, 23 Jul 2026 21:07:36 +1000 Subject: [PATCH 7/9] no-mistakes(document): docs already in sync with Guice DI and registry rename --- .../gg/steve/mc/ap/ArmorPlusModuleTest.java | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 src/test/java/gg/steve/mc/ap/ArmorPlusModuleTest.java diff --git a/src/test/java/gg/steve/mc/ap/ArmorPlusModuleTest.java b/src/test/java/gg/steve/mc/ap/ArmorPlusModuleTest.java new file mode 100644 index 0000000..279b74b --- /dev/null +++ b/src/test/java/gg/steve/mc/ap/ArmorPlusModuleTest.java @@ -0,0 +1,125 @@ +package gg.steve.mc.ap; + +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.Key; +import com.google.inject.TypeLiteral; +import gg.steve.mc.ap.armor.ArmorSetCatalog; +import gg.steve.mc.ap.armor.Set; +import gg.steve.mc.ap.model.id.ArmorSetId; +import gg.steve.mc.ap.model.player.PlayerArmorWearerRegistry; +import gg.steve.mc.ap.model.set.ArmorSetRegistry; +import gg.steve.mc.ap.player.PlayerArmorSetService; +import gg.steve.mc.ap.player.SetPlayer; +import org.bukkit.entity.Player; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Exercises the real Guice composition root that {@link ArmorPlus#onEnable()} builds. + *

+ * This is the wiring the whole change turns on: the static {@code SetManager}/{@code SetPlayerManager} + * singletons were deleted and replaced by injected {@link ArmorSetCatalog} / {@link PlayerArmorSetService} + * instances that share the pure {@link ArmorSetRegistry} / {@link PlayerArmorWearerRegistry}. Every other + * test constructs those collaborators by hand with {@code new}; this one drives the actual + * {@link ArmorPlusModule} through a live {@link Injector} - the same objects {@code onEnable} resolves + * and threads to the listeners, commands and expansion - so the DI contract itself is verified, not + * just the classes in isolation. + *

+ * The end-to-end check proves the property the old statics used to give for free: state written through + * one injector-resolved collaborator (register a set on the catalog, mark a player wearing it on the + * service) is observable through another, because Guice hands out one shared instance of each. A + * transcript of what the injector produced is written to the evidence directory. + */ +@ExtendWith(MockitoExtension.class) +class ArmorPlusModuleTest { + + private static final Path EVIDENCE = Path.of( + System.getProperty("java.io.tmpdir"), + "no-mistakes-evidence", "01KY78FKDPAVYJW7ME7YQYCTNA", + "guice-composition-root-transcript.txt"); + + private static final Key> SET_REGISTRY_KEY = + Key.get(new TypeLiteral>() {}); + + @Mock private ArmorPlus plugin; + @Mock private Set dragonSet; + @Mock private Player player; + + @Test + void injectorWiresSharedSingletonsAndTheyRoundTripState() { + List transcript = new ArrayList<>(); + + // Composition root, exactly as ArmorPlus.onEnable builds it. + Injector injector = Guice.createInjector(new ArmorPlusModule(plugin)); + + // 1. Both adapters resolve as singletons: repeat lookups hand back the same object. + ArmorSetCatalog catalog = injector.getInstance(ArmorSetCatalog.class); + PlayerArmorSetService service = injector.getInstance(PlayerArmorSetService.class); + assertSame(catalog, injector.getInstance(ArmorSetCatalog.class), + "catalog must be a shared singleton (replacing the old static SetManager)"); + assertSame(service, injector.getInstance(PlayerArmorSetService.class), + "player service must be a shared singleton (replacing the old static SetPlayerManager)"); + transcript.add("injector.getInstance(ArmorSetCatalog) -> singleton " + (catalog != null)); + transcript.add("injector.getInstance(PlayerArmorSetService) -> singleton " + (service != null)); + + // 2. The pure registries are the same instances the adapters were constructor-injected with: + // the catalog holds the injector's ArmorSetRegistry singleton, proving the graph is threaded. + ArmorSetRegistry sharedRegistry = injector.getInstance(SET_REGISTRY_KEY); + assertSame(sharedRegistry, catalog.getRegistry(), + "the catalog must own the singleton ArmorSetRegistry the injector binds"); + assertSame(injector.getInstance(PlayerArmorWearerRegistry.class), + injector.getInstance(PlayerArmorWearerRegistry.class), + "wearer registry must be a shared singleton too"); + transcript.add("catalog.getRegistry() == injected ArmorSetRegistry singleton -> " + + (sharedRegistry == catalog.getRegistry())); + + // 3. End-to-end: write through the catalog, read back through the service. + // getSetPlayer resolves the worn set name back through the SAME catalog the service was + // injected with, so this only works if Guice threaded one shared catalog into the service. + UUID id = UUID.fromString("00000000-0000-0000-0000-0000000000aa"); + org.mockito.Mockito.when(player.getUniqueId()).thenReturn(id); + + catalog.getRegistry().register(ArmorSetId.of("dragon"), dragonSet); + service.addSetPlayer(player, "dragon"); + + assertTrue(service.isWearingSet(player), "service must see the player it just recorded"); + SetPlayer resolved = service.getSetPlayer(player); + assertNotNull(resolved, "getSetPlayer must reconstruct the wearer"); + assertSame(dragonSet, resolved.getSet(), + "the set resolved through the service must be the one registered on the shared catalog"); + transcript.add("catalog.register(dragon); service.addSetPlayer(player, \"dragon\")"); + transcript.add("service.isWearingSet(player) -> " + service.isWearingSet(player)); + transcript.add("service.getSetPlayer(player).getSet() -> same instance registered on catalog: " + + (resolved.getSet() == dragonSet)); + + writeTranscript(transcript); + } + + private void writeTranscript(List body) { + List lines = new ArrayList<>(); + lines.add("=== Guice composition root wires shared singletons over the pure registries ==="); + lines.add("(built from the real ArmorPlusModule via a live Injector - the same wiring ArmorPlus.onEnable uses)"); + lines.addAll(body); + try { + Files.createDirectories(EVIDENCE.getParent()); + Files.write(EVIDENCE, (String.join(System.lineSeparator(), lines) + System.lineSeparator()) + .getBytes(StandardCharsets.UTF_8)); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } +} From cdb0dcd17fa93cd142e0ea03bb01f970a605e271 Mon Sep 17 00:00:00 2001 From: Stephen Goodhill Date: Thu, 23 Jul 2026 21:17:34 +1000 Subject: [PATCH 8/9] no-mistakes: apply CI fixes --- src/main/java/gg/steve/mc/ap/ArmorPlus.java | 5 ++++- src/main/java/gg/steve/mc/ap/armor/ArmorSetCatalog.java | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/gg/steve/mc/ap/ArmorPlus.java b/src/main/java/gg/steve/mc/ap/ArmorPlus.java index 6a8db6a..4220278 100644 --- a/src/main/java/gg/steve/mc/ap/ArmorPlus.java +++ b/src/main/java/gg/steve/mc/ap/ArmorPlus.java @@ -13,12 +13,14 @@ import net.milkbowl.vault.economy.Economy; import org.bstats.bukkit.Metrics; import org.bukkit.Bukkit; +import org.bukkit.configuration.file.YamlConfiguration; import org.bukkit.plugin.RegisteredServiceProvider; import org.bukkit.plugin.java.JavaPlugin; import java.text.DecimalFormat; import java.util.HashMap; import java.util.Map; +import java.util.Objects; public final class ArmorPlus extends JavaPlugin { private static ArmorPlus instance; @@ -93,7 +95,8 @@ public static Economy eco() { public static ApGui getApGui() { if (apGui == null) { - apGui = new ApGui(ConfigManager.CONFIG.get().getConfigurationSection("gui"), instance.catalog); + YamlConfiguration config = Objects.requireNonNull(ConfigManager.CONFIG.get(), "armor+.yml config is not loaded"); + apGui = new ApGui(config.getConfigurationSection("gui"), instance.catalog); } else { apGui.refresh(); } diff --git a/src/main/java/gg/steve/mc/ap/armor/ArmorSetCatalog.java b/src/main/java/gg/steve/mc/ap/armor/ArmorSetCatalog.java index 77bf611..4701e99 100644 --- a/src/main/java/gg/steve/mc/ap/armor/ArmorSetCatalog.java +++ b/src/main/java/gg/steve/mc/ap/armor/ArmorSetCatalog.java @@ -6,9 +6,11 @@ import gg.steve.mc.ap.model.id.ArmorSetId; import gg.steve.mc.ap.model.set.ArmorSetRegistry; import gg.steve.mc.ap.utils.YamlFileUtil; +import org.bukkit.configuration.file.YamlConfiguration; import java.io.File; import java.util.Map; +import java.util.Objects; /** * The plugin's live catalog of armor sets, parsed from configuration and keyed by {@link ArmorSetId}. @@ -35,7 +37,8 @@ public ArmorSetRegistry getRegistry() { public void loadSets() { registry.clear(); - for (String set : ConfigManager.CONFIG.get().getStringList("loaded-sets")) { + YamlConfiguration config = Objects.requireNonNull(ConfigManager.CONFIG.get(), "armor+.yml config is not loaded"); + for (String set : config.getStringList("loaded-sets")) { YamlFileUtil fileUtil = new YamlFileUtil("sets" + File.separator + set + ".yml", plugin); registry.register(ArmorSetId.of(set), new Set(set, fileUtil)); } From 91f3a7df517e57685566a6d5aa7f65a9886c2fb3 Mon Sep 17 00:00:00 2001 From: Stephen Goodhill Date: Fri, 24 Jul 2026 09:04:50 +1000 Subject: [PATCH 9/9] refactor: trim comments to terse WHY-only across the DI change Purge bloated javadoc and comments that merely restate the code (one-line method narration, class summaries derivable from the name). Keep only the few genuinely non-obvious WHYs: why the model registries are bound rather than annotated, why ArmorSetRegistry is generic, and the DI-wiring invariant the Guice composition-root test turns on. No behavior change; characterization suite untouched. --- src/main/java/gg/steve/mc/ap/ArmorPlus.java | 7 +----- .../java/gg/steve/mc/ap/ArmorPlusModule.java | 10 +------- .../gg/steve/mc/ap/armor/ArmorSetCatalog.java | 9 ------- src/main/java/gg/steve/mc/ap/gui/ApGui.java | 6 ----- .../gg/steve/mc/ap/managers/SetupManager.java | 7 ------ .../player/PlayerArmorWearerRegistry.java | 13 ---------- .../mc/ap/model/set/ArmorSetRegistry.java | 16 +----------- .../mc/ap/player/PlayerArmorSetService.java | 10 -------- .../gg/steve/mc/ap/ArmorPlusModuleTest.java | 25 ++----------------- .../mc/ap/armor/ArmorSetCatalogTest.java | 12 +-------- .../listener/PlayerCommandListenerTest.java | 17 +------------ .../ap/player/PlayerArmorSetServiceTest.java | 10 -------- 12 files changed, 7 insertions(+), 135 deletions(-) diff --git a/src/main/java/gg/steve/mc/ap/ArmorPlus.java b/src/main/java/gg/steve/mc/ap/ArmorPlus.java index 4220278..91ea324 100644 --- a/src/main/java/gg/steve/mc/ap/ArmorPlus.java +++ b/src/main/java/gg/steve/mc/ap/ArmorPlus.java @@ -28,10 +28,7 @@ public final class ArmorPlus extends JavaPlugin { private static ApGui apGui; private static DecimalFormat numberFormat = new DecimalFormat("#,###.##"); - /** - * The injected armor-set catalog, kept on the plugin so the still-static GUI-bootstrap - * facade below can hand it to {@link ApGui} without reaching for a static set map. - */ + // kept on the plugin so the static getApGui() facade can reach it private ArmorSetCatalog catalog; @Override @@ -41,8 +38,6 @@ public void onEnable() { // reset apgui to null for reloading apGui = null; SetupManager.setupFiles(new FileManager(instance)); - // Composition root: build the injector once, then resolve the shared collaborators - // and hand them to the wiring below as instances instead of reaching for statics. Injector injector = Guice.createInjector(new ArmorPlusModule(this)); this.catalog = injector.getInstance(ArmorSetCatalog.class); PlayerArmorSetService playerArmorSetService = injector.getInstance(PlayerArmorSetService.class); diff --git a/src/main/java/gg/steve/mc/ap/ArmorPlusModule.java b/src/main/java/gg/steve/mc/ap/ArmorPlusModule.java index c183074..b0966d2 100644 --- a/src/main/java/gg/steve/mc/ap/ArmorPlusModule.java +++ b/src/main/java/gg/steve/mc/ap/ArmorPlusModule.java @@ -9,15 +9,6 @@ import gg.steve.mc.ap.model.set.ArmorSetRegistry; import gg.steve.mc.ap.player.PlayerArmorSetService; -/** - * Guice bindings for the plugin's shared collaborators. - *

- * The pure {@code model} registries carry no framework annotations, so they are bound here - * (Guice instantiates them via their no-arg constructors) rather than annotated in place - - * this keeps the domain layer free of any dependency-injection imports. The adapter-layer - * catalog and player service are constructor-injected singletons; a single instance of each - * is shared across every listener, command, and expansion the injector wires. - */ public class ArmorPlusModule extends AbstractModule { private final ArmorPlus plugin; @@ -28,6 +19,7 @@ public ArmorPlusModule(ArmorPlus plugin) { @Override protected void configure() { bind(ArmorPlus.class).toInstance(plugin); + // Bind the pure model registries here rather than annotating them, so the domain layer stays DI-free. bind(new TypeLiteral>() {}).in(Singleton.class); bind(PlayerArmorWearerRegistry.class).in(Singleton.class); bind(ArmorSetCatalog.class).in(Singleton.class); diff --git a/src/main/java/gg/steve/mc/ap/armor/ArmorSetCatalog.java b/src/main/java/gg/steve/mc/ap/armor/ArmorSetCatalog.java index 4701e99..f88e008 100644 --- a/src/main/java/gg/steve/mc/ap/armor/ArmorSetCatalog.java +++ b/src/main/java/gg/steve/mc/ap/armor/ArmorSetCatalog.java @@ -12,14 +12,6 @@ import java.util.Map; import java.util.Objects; -/** - * The plugin's live catalog of armor sets, parsed from configuration and keyed by {@link ArmorSetId}. - *

- * State lives in an injected {@link ArmorSetRegistry}; this class is the adapter that populates the - * registry from Bukkit-backed config files and reads it back for the platform layer. It is a shared - * singleton wired by Guice and passed to its callers as an injected instance, so there is no static - * map or global entry point to reset between reloads. - */ public class ArmorSetCatalog { private final ArmorSetRegistry registry; private final ArmorPlus plugin; @@ -30,7 +22,6 @@ public ArmorSetCatalog(ArmorSetRegistry registry, ArmorPlus plugin) { this.plugin = plugin; } - /** The registry this catalog owns, for callers that read the raw id-to-set view. */ public ArmorSetRegistry getRegistry() { return registry; } diff --git a/src/main/java/gg/steve/mc/ap/gui/ApGui.java b/src/main/java/gg/steve/mc/ap/gui/ApGui.java index 9099398..b250b58 100644 --- a/src/main/java/gg/steve/mc/ap/gui/ApGui.java +++ b/src/main/java/gg/steve/mc/ap/gui/ApGui.java @@ -8,12 +8,6 @@ public class ApGui extends AbstractGui { private ConfigurationSection section; private final ArmorSetCatalog catalog; - /** - * Constructor the create a new Gui - * - * @param section - * @param catalog the armor-set catalog used to resolve the set a slot action opens - */ public ApGui(ConfigurationSection section, ArmorSetCatalog catalog) { super(section, section.getString("type"), section.getInt("size")); this.section = section; diff --git a/src/main/java/gg/steve/mc/ap/managers/SetupManager.java b/src/main/java/gg/steve/mc/ap/managers/SetupManager.java index 664db4c..4e5961e 100644 --- a/src/main/java/gg/steve/mc/ap/managers/SetupManager.java +++ b/src/main/java/gg/steve/mc/ap/managers/SetupManager.java @@ -37,13 +37,6 @@ public static void registerCommands(ArmorPlus instance, ArmorSetCatalog catalog) instance.getCommand("ap").setExecutor(new ApCmd(catalog)); } - /** - * Register all of the events for the plugin. - * - * @param instance Plugin, the main plugin instance - * @param catalog the shared armor-set catalog the listeners resolve sets from - * @param playerArmorSetService the shared worn-set tracker the listeners record wearers in - */ public static void registerEvents(Plugin instance, ArmorSetCatalog catalog, PlayerArmorSetService playerArmorSetService) { PluginManager pm = instance.getServer().getPluginManager(); pm.registerEvents(new ArmorListener(ConfigManager.BLOCKED.get().getStringList("blocked")), instance); diff --git a/src/main/java/gg/steve/mc/ap/model/player/PlayerArmorWearerRegistry.java b/src/main/java/gg/steve/mc/ap/model/player/PlayerArmorWearerRegistry.java index 741a081..139a237 100644 --- a/src/main/java/gg/steve/mc/ap/model/player/PlayerArmorWearerRegistry.java +++ b/src/main/java/gg/steve/mc/ap/model/player/PlayerArmorWearerRegistry.java @@ -7,46 +7,33 @@ import java.util.Map; import java.util.Optional; -/** - * Pure in-memory record of which players are currently wearing which armor set. - * Holds the {@link PlayerId}-to-{@link ArmorSetWearer} state, with no reference to - * Bukkit types. Instances are mutable but self-contained, so they can be unit-tested - * without any server or static-state gymnastics. - */ public final class PlayerArmorWearerRegistry { private final Map wearers = new HashMap<>(); - /** Record that a player is wearing a set, replacing any existing entry for that player. */ public void add(ArmorSetWearer wearer) { wearers.put(wearer.getPlayerId(), wearer); } - /** Convenience overload building the {@link ArmorSetWearer} from its identity parts. */ public void add(PlayerId playerId, ArmorSetId setId) { add(new ArmorSetWearer(playerId, setId)); } - /** Forget any set the player was recorded as wearing. */ public void remove(PlayerId playerId) { wearers.remove(playerId); } - /** Whether the player is currently recorded as wearing a set. */ public boolean isWearing(PlayerId playerId) { return wearers.containsKey(playerId); } - /** The wearer record for a player, if one exists. */ public Optional get(PlayerId playerId) { return Optional.ofNullable(wearers.get(playerId)); } - /** Number of players currently recorded as wearing a set. */ public int count() { return wearers.size(); } - /** Drop all wearer records. */ public void clear() { wearers.clear(); } diff --git a/src/main/java/gg/steve/mc/ap/model/set/ArmorSetRegistry.java b/src/main/java/gg/steve/mc/ap/model/set/ArmorSetRegistry.java index fb48862..148031d 100644 --- a/src/main/java/gg/steve/mc/ap/model/set/ArmorSetRegistry.java +++ b/src/main/java/gg/steve/mc/ap/model/set/ArmorSetRegistry.java @@ -7,40 +7,26 @@ import java.util.Map; import java.util.Optional; -/** - * Pure registry of the armor sets known to the plugin, keyed by {@link ArmorSetId}. - * Preserves registration order so callers that iterate see a deterministic sequence. - *

- * Generic over the stored set type: the pure domain does not yet own an {@code ArmorSet} - * aggregate, so this registry stays value-type-agnostic and Bukkit-free while the - * platform layer parameterizes it over its own set representation. - * - * @param the stored representation of an armor set - */ +// Generic over the set type: the pure domain has no ArmorSet aggregate yet, so the platform layer supplies its own. public final class ArmorSetRegistry { private final Map sets = new LinkedHashMap<>(); - /** Register (or replace) the set stored under the given id. */ public void register(ArmorSetId id, T set) { sets.put(id, set); } - /** The set stored under the given id, if one exists. */ public Optional get(ArmorSetId id) { return Optional.ofNullable(sets.get(id)); } - /** An unmodifiable view of every registered set, in registration order. */ public Map getAll() { return Collections.unmodifiableMap(sets); } - /** Number of registered sets. */ public int count() { return sets.size(); } - /** Drop all registered sets. */ public void clear() { sets.clear(); } diff --git a/src/main/java/gg/steve/mc/ap/player/PlayerArmorSetService.java b/src/main/java/gg/steve/mc/ap/player/PlayerArmorSetService.java index b8aeb62..c33dabc 100644 --- a/src/main/java/gg/steve/mc/ap/player/PlayerArmorSetService.java +++ b/src/main/java/gg/steve/mc/ap/player/PlayerArmorSetService.java @@ -12,16 +12,6 @@ import org.bukkit.Material; import org.bukkit.entity.Player; -/** - * Tracks which online players are currently wearing which armor set and answers the - * platform's questions about worn sets. - *

- * Wearer state lives in the injected pure {@link PlayerArmorWearerRegistry}; the injected - * {@link ArmorSetCatalog} resolves set names to {@link Set} instances. This is a shared - * singleton passed to its callers (listeners and the placeholder expansion) as an injected - * instance, so tests construct it with their own registry and catalog rather than resetting - * global state. - */ public class PlayerArmorSetService { private final PlayerArmorWearerRegistry registry; private final ArmorSetCatalog catalog; diff --git a/src/test/java/gg/steve/mc/ap/ArmorPlusModuleTest.java b/src/test/java/gg/steve/mc/ap/ArmorPlusModuleTest.java index 279b74b..2cf373d 100644 --- a/src/test/java/gg/steve/mc/ap/ArmorPlusModuleTest.java +++ b/src/test/java/gg/steve/mc/ap/ArmorPlusModuleTest.java @@ -28,22 +28,7 @@ import static org.junit.jupiter.api.Assertions.*; -/** - * Exercises the real Guice composition root that {@link ArmorPlus#onEnable()} builds. - *

- * This is the wiring the whole change turns on: the static {@code SetManager}/{@code SetPlayerManager} - * singletons were deleted and replaced by injected {@link ArmorSetCatalog} / {@link PlayerArmorSetService} - * instances that share the pure {@link ArmorSetRegistry} / {@link PlayerArmorWearerRegistry}. Every other - * test constructs those collaborators by hand with {@code new}; this one drives the actual - * {@link ArmorPlusModule} through a live {@link Injector} - the same objects {@code onEnable} resolves - * and threads to the listeners, commands and expansion - so the DI contract itself is verified, not - * just the classes in isolation. - *

- * The end-to-end check proves the property the old statics used to give for free: state written through - * one injector-resolved collaborator (register a set on the catalog, mark a player wearing it on the - * service) is observable through another, because Guice hands out one shared instance of each. A - * transcript of what the injector produced is written to the evidence directory. - */ +/** Verifies the real Guice composition root wires shared singletons over the pure registries. */ @ExtendWith(MockitoExtension.class) class ArmorPlusModuleTest { @@ -63,10 +48,8 @@ class ArmorPlusModuleTest { void injectorWiresSharedSingletonsAndTheyRoundTripState() { List transcript = new ArrayList<>(); - // Composition root, exactly as ArmorPlus.onEnable builds it. Injector injector = Guice.createInjector(new ArmorPlusModule(plugin)); - // 1. Both adapters resolve as singletons: repeat lookups hand back the same object. ArmorSetCatalog catalog = injector.getInstance(ArmorSetCatalog.class); PlayerArmorSetService service = injector.getInstance(PlayerArmorSetService.class); assertSame(catalog, injector.getInstance(ArmorSetCatalog.class), @@ -76,8 +59,6 @@ void injectorWiresSharedSingletonsAndTheyRoundTripState() { transcript.add("injector.getInstance(ArmorSetCatalog) -> singleton " + (catalog != null)); transcript.add("injector.getInstance(PlayerArmorSetService) -> singleton " + (service != null)); - // 2. The pure registries are the same instances the adapters were constructor-injected with: - // the catalog holds the injector's ArmorSetRegistry singleton, proving the graph is threaded. ArmorSetRegistry sharedRegistry = injector.getInstance(SET_REGISTRY_KEY); assertSame(sharedRegistry, catalog.getRegistry(), "the catalog must own the singleton ArmorSetRegistry the injector binds"); @@ -87,9 +68,7 @@ void injectorWiresSharedSingletonsAndTheyRoundTripState() { transcript.add("catalog.getRegistry() == injected ArmorSetRegistry singleton -> " + (sharedRegistry == catalog.getRegistry())); - // 3. End-to-end: write through the catalog, read back through the service. - // getSetPlayer resolves the worn set name back through the SAME catalog the service was - // injected with, so this only works if Guice threaded one shared catalog into the service. + // round-trips only if Guice threaded one shared catalog into the service UUID id = UUID.fromString("00000000-0000-0000-0000-0000000000aa"); org.mockito.Mockito.when(player.getUniqueId()).thenReturn(id); diff --git a/src/test/java/gg/steve/mc/ap/armor/ArmorSetCatalogTest.java b/src/test/java/gg/steve/mc/ap/armor/ArmorSetCatalogTest.java index 3cc1699..b11a247 100644 --- a/src/test/java/gg/steve/mc/ap/armor/ArmorSetCatalogTest.java +++ b/src/test/java/gg/steve/mc/ap/armor/ArmorSetCatalogTest.java @@ -10,17 +10,7 @@ import static org.junit.jupiter.api.Assertions.*; -/** - * Characterization tests for {@link ArmorSetCatalog#getSet(String)} pinning the exact - * null contract that the registry extraction had to preserve. - *

- * Real callers (GiveCmd, PlayerEquipListener, ArmorPlusExpansion) treat a null return as - * "no such set" and must never see an exception for a missing/blank name. Because the - * {@code ArmorSetRegistry} keys on {@code ArmorSetId} - which rejects null/empty via - * commons-lang3 {@code Validate} - {@code getSet} guards those inputs before constructing the id. - * These tests prove that guard preserves the original {@code map.get()} behavior on a catalog - * built from an empty registry. - */ +// getSet must return null for null/blank names: callers rely on it, but ArmorSetId.of rejects them. @ExtendWith(MockitoExtension.class) class ArmorSetCatalogTest { diff --git a/src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java b/src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java index 3d2b426..f656ac6 100644 --- a/src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java +++ b/src/test/java/gg/steve/mc/ap/listener/PlayerCommandListenerTest.java @@ -28,18 +28,6 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; -/** - * End-to-end style test of {@link PlayerCommandListener}, the only player-facing surface the - * registry extraction changed. The listener iterates the catalog's {@code Map} - * view and uses {@code key.toString()} to recover the raw set-name string. - *

- * These tests fire real {@link PlayerCommandPreprocessEvent}s (exactly what Bukkit delivers when a - * player types a command in chat) at the real listener, wired to an {@link ArmorSetCatalog} the - * test constructs and populates directly, and assert the interception behaves identically: the set - * name is matched case-insensitively, the event is cancelled, the matching set's GUI is opened, and - * unrelated commands pass straight through. A transcript of each simulated keystroke and its outcome - * is written to the evidence directory so a reviewer can see the player experience directly. - */ @ExtendWith(MockitoExtension.class) class PlayerCommandListenerTest { @@ -60,17 +48,14 @@ class PlayerCommandListenerTest { @BeforeEach void setUp() { - // Register two sets into the catalog's registry, exactly as loadSets() would. catalog = new ArmorSetCatalog(new ArmorSetRegistry<>(), plugin); catalog.getRegistry().register(ArmorSetId.of("dragon"), dragonSet); catalog.getRegistry().register(ArmorSetId.of("knight"), knightSet); listener = new PlayerCommandListener(catalog); } - /** Fire a command at the real listener and record the observable player-facing outcome. */ private PlayerCommandPreprocessEvent type(String typed) { - // Three-arg constructor takes an explicit recipient set, avoiding the internal - // getServer().getOnlinePlayers() call the two-arg form makes. + // explicit recipient set avoids the internal getServer().getOnlinePlayers() the two-arg form calls PlayerCommandPreprocessEvent event = new PlayerCommandPreprocessEvent(player, typed, Collections.emptySet()); listener.onCmd(event); diff --git a/src/test/java/gg/steve/mc/ap/player/PlayerArmorSetServiceTest.java b/src/test/java/gg/steve/mc/ap/player/PlayerArmorSetServiceTest.java index 099f930..f6189ba 100644 --- a/src/test/java/gg/steve/mc/ap/player/PlayerArmorSetServiceTest.java +++ b/src/test/java/gg/steve/mc/ap/player/PlayerArmorSetServiceTest.java @@ -20,12 +20,6 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; -/** - * Characterization tests for PlayerArmorSetService state transitions. - * Pins add/remove/isWearing/getPiecesWearing behavior. Formerly exercised the static - * SetPlayerManager with a {@code MockedStatic}; now the service holds an injected - * wearer registry and catalog, so each test constructs a fresh instance with no static reset. - */ @ExtendWith(MockitoExtension.class) class PlayerArmorSetServiceTest { @@ -43,8 +37,6 @@ class PlayerArmorSetServiceTest { void setUp() { lenient().when(player1.getUniqueId()).thenReturn(uuid1); lenient().when(player2.getUniqueId()).thenReturn(uuid2); - - // getSetPlayer resolves the worn set name through the catalog. lenient().when(catalog.getSet(anyString())).thenReturn(mockSet); service = new PlayerArmorSetService(new PlayerArmorWearerRegistry(), catalog); @@ -98,8 +90,6 @@ void multiplePlayers_independentTracking() { assertTrue(service.isWearingSet(player2)); } - // --- getPiecesWearing characterization --- - @Test void getPiecesWearing_allPiecesVerified_countsAll( @Mock Set set, @Mock PlayerInventory inv,