refactor: replace static manager singletons with Guice-injected services

[nbdSteve]

Intent

Behavior-preserving dependency-injection refactor of the ArmorPlus Spigot plugin, plus a follow-up comment-style cleanup.

Core change: replace the two static god-object singletons (SetManager, SetPlayerManager) with Guice-injected instances. ArmorSetCatalog (adapter over the pure model.set.ArmorSetRegistry) replaces SetManager; PlayerArmorSetService (adapter over the pure model.player.PlayerArmorWearerRegistry, resolving set names through the injected catalog) replaces SetPlayerManager. A new ArmorPlusModule Guice module is the composition root, built once in ArmorPlus.onEnable and used to resolve the shared singletons that are then threaded to listeners, commands, and the PAPI expansion as constructor-injected instances instead of reached-for statics. Google Guice 5.1.0 (JSR-330) was chosen over Dagger to avoid a second annotation processor alongside Lombok; it is shaded/relocated under gg.steve.mc.ap.lib.*. The pure model registries are deliberately bound by type in the module (not annotated) so the domain layer stays DI-framework-free. ArmorSetCatalog.getSet keeps its original null/empty guard so callers never see an exception for a blank/unknown set name (ArmorSetId.of rejects null/empty). Two pre-existing latent NPEs on ConfigManager.CONFIG.get() (in ArmorPlus.getApGui and ArmorSetCatalog.loadSets) were hardened with Objects.requireNonNull during earlier CI fix rounds; happy-path behavior unchanged.

Tests: the static-manager tests were converted to construct the new injected instances directly (SetManagerTest->ArmorSetCatalogTest, SetPlayerManagerTest->PlayerArmorSetServiceTest), keeping all behavioral assertions byte-identical; only the static->instance plumbing changed. A new ArmorPlusModuleTest drives the real Guice injector end-to-end to prove the shared-singleton wiring. The sacrosanct characterization suite (damage calc, warp, XP, potion, set-detection, messaging) is untouched and byte-identical.

This latest commit is a standing-convention comment cleanup requested by the user: comments must be extremely terse, documenting only non-obvious WHY. It purges the bloated javadoc and restating comments added during the DI work (one-line method narration, class summaries derivable from the name), keeping only a few genuine WHY notes. It is comment-only: zero non-comment code lines changed, all 235 tests still pass, and the 100% core-coverage gate still holds. Do NOT change package structure - a separate decision on that is pending.

What Changed

• Replaced the SetManager and SetPlayerManager static singletons with constructor-injected ArmorSetCatalog and PlayerArmorSetService, both adapters over new Bukkit-free domain registries (model.set.ArmorSetRegistry, model.player.PlayerArmorWearerRegistry); PlayerArmorSetService.getSetPlayer now reconstructs the SetPlayer on demand from the wearer record, and set iteration follows deterministic loaded-config order.
• Added an ArmorPlusModule Guice composition root, built once in ArmorPlus.onEnable, to resolve the shared singletons and thread them through listeners, commands, the PAPI expansion, and the GUI in place of static .get() access; pulled in Guice 5.1.0 (JSR-330) in pom.xml, shaded and relocated under gg.steve.mc.ap.lib.* with annotation-only transitives excluded.
• Migrated the former manager tests to construct the injected instances directly (ArmorSetCatalogTest, PlayerArmorSetServiceTest) with assertions byte-identical, added ArmorPlusModuleTest driving the real injector end-to-end plus registry and PlayerCommandListenerTest coverage, and trimmed the DI-era comments to terse WHY-only.

Risk Assessment

✅ Low: Behavior-preserving DI refactor with logic-identical registries, complete Guice shade relocation, comprehensive new/converted tests, and only benign, mostly-intentional semantic side-effects - no reachable correctness regressions found.

Testing

Ran the full JDK 25 Maven build (mvn -B clean verify): all 235 tests pass and the 100% core-coverage gate holds. Beyond green tests, I confirmed the intent at the product level — inspected the shaded JAR to prove Guice, javax.inject, guava, and aopalliance are relocated under gg/steve/mc/ap/lib/* (with annotation-only transitives excluded), which is the real classpath-collision guarantee for a shared Spigot server; captured two end-to-end evidence transcripts showing the actual ArmorPlusModule injector wiring shared singletons that round-trip wearer state and routing player set-commands to their GUIs; verified the head commit changes zero non-comment code lines; and confirmed the sacrosanct characterization suite is byte-identical to base. No product UI surface exists (headless plugin JAR), so CLI/transcript and jar-structure artifacts are the appropriate reviewer-visible evidence. Cleaned the build output afterward, leaving a clean worktree.

Evidence: Guice composition-root wiring transcript (live injector, shared singletons round-trip state)

=== Guice composition root wires shared singletons over the pure registries === (built from the real ArmorPlusModule via a live Injector - the same wiring ArmorPlus.onEnable uses) injector.getInstance(ArmorSetCatalog) -> singleton true injector.getInstance(PlayerArmorSetService) -> singleton true catalog.getRegistry() == injected ArmorSetRegistry singleton -> true catalog.register(dragon); service.addSetPlayer(player, "dragon") service.isWearingSet(player) -> true service.getSetPlayer(player).getSet() -> same instance registered on catalog: true

=== Guice composition root wires shared singletons over the pure registries ===
(built from the real ArmorPlusModule via a live Injector - the same wiring ArmorPlus.onEnable uses)
injector.getInstance(ArmorSetCatalog)        -> singleton true
injector.getInstance(PlayerArmorSetService)  -> singleton true
catalog.getRegistry() == injected ArmorSetRegistry singleton -> true
catalog.register(dragon); service.addSetPlayer(player, "dragon")
service.isWearingSet(player)                 -> true
service.getSetPlayer(player).getSet()        -> same instance registered on catalog: true

Evidence: Player set-command routing transcript (injected catalog)

=== Each set command routes to its own set; unknown command passes through === Registry keys (ArmorSetId): [dragon, knight] player types "/dragon" -> intercepted=true player types "/knight" -> intercepted=true player types "/spawn" -> intercepted=false

=== Each set command routes to its own set; unknown command passes through ===
Registry keys (ArmorSetId): [dragon, knight]
player types "/dragon"              -> intercepted=true
player types "/knight"              -> intercepted=true
player types "/spawn"               -> intercepted=false

Evidence: Shaded-jar DI library relocation verification

Relocated lib packages in target/ArmorPlus-v2.3.6.jar:
gg/steve/mc/ap/lib/aopalliance/
gg/steve/mc/ap/lib/commons/
gg/steve/mc/ap/lib/guava/
gg/steve/mc/ap/lib/inject/ (guice)
gg/steve/mc/ap/lib/javax/ (javax.inject)
Unrelocated com/google/inject at root: 0
Unrelocated root javax/inject: 0
Excluded transitives (checkerframework/errorprone/j2objc/findbugs): 0 each

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 2 infos

• ℹ️ src/main/java/gg/steve/mc/ap/model/set/ArmorSetRegistry.java:11 - ArmorSetRegistry backs its map with a LinkedHashMap and getAll() returns an unmodifiable view, whereas the old SetManager.getSets() returned a live, hash-ordered HashMap. Consumers that pick the 'first' set a player is wearing (PlayerEquipListener.join, PlayerUnequipListener.quit, PlayerArmorSetService.init) now iterate in deterministic config (loaded-sets) order instead of arbitrary hash order. This only changes the outcome in the rare case where a player's armor simultaneously matches two sets; the new deterministic behavior is an improvement, and all call sites are read-only so the unmodifiable view breaks nothing. Noted so the semantic change is on the record.
• ℹ️ src/main/java/gg/steve/mc/ap/player/PlayerArmorSetService.java:49 - PlayerArmorSetService.getSetPlayer now reconstructs a fresh SetPlayer (allocating PlayerId + ArmorSetId and doing two map lookups plus a catalog re-resolve) on every call, where the old SetPlayerManager returned a single cached SetPlayer instance stored at add time. In ArmorPlusExpansion this runs on the PAPI placeholder path, which can be invoked frequently (e.g. per-tick scoreboards) and calls getSetPlayer multiple times per request. The extra allocation is negligible for modern JVM GC and the on-demand reconstruction is explicitly documented as intended design in AGENTS.md, so no action needed - flagged only as an efficiency-vs-old-behavior note.

✅ **Test** - passed

✅ No issues found.

• JAVA_HOME=<corretto-25> mvn -B clean verify — BUILD SUCCESS, 235 tests pass, JaCoCo check-core gate met
• Shaded-jar relocation check: unzip -l target/ArmorPlus-v2.3.6.jar confirms guice/javax.inject/guava/aopalliance relocated under gg/steve/mc/ap/lib/*, zero unrelocated com/google/inject or root javax/inject, checker-qual/error_prone/j2objc/jsr305 excluded
• ArmorPlusModuleTest.injectorWiresSharedSingletonsAndTheyRoundTripState — real Guice injector wires shared singletons that round-trip register→isWearingSet→getSetPlayer state (evidence transcript)
• PlayerCommandListenerTest — injected catalog routes /dragon and /knight to their GUIs, /spawn passes through (evidence transcript)
• Head commit 91f3a7d comment-only check: comment/blank-stripped before/after of all 12 files is byte-identical
• Characterization suite (git diff base HEAD over data/message/warp/potion tests) — byte-identical to base

✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

[nbdSteve]

Rather than having everything static can be pass around instances? We shouldnt need to rely on static maps, pass around the registry class instance - might need to use DI

[nbdSteve]

We should just return the registry object

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud