Skip to content

Fix build, shutdown cleanup and player data lifecycle - #2

Merged
Rethinger merged 2 commits into
mainfrom
claude/project-improvement-recommendations-ooz4vx
Jul 26, 2026
Merged

Fix build, shutdown cleanup and player data lifecycle#2
Rethinger merged 2 commits into
mainfrom
claude/project-improvement-recommendations-ooz4vx

Conversation

@Rethinger

Copy link
Copy Markdown
Owner

The project did not build: de.eisi05:NpcApi-Paper is published on JitPack but
no such repository was declared, so dependency resolution failed outright. The
CI workflow was an unmodified GitHub template that would have failed anyway --
JDK 11 against a Java 21 source level, and an mvn deploy step with no
distributionManagement in the POM.

Build:

  • Declare the JitPack repository so NpcApi-Paper resolves.
  • CI: JDK 21, build on push and pull_request (not only on release), upload the
    jar as an artifact, and attach it to releases instead of deploying nowhere.
  • Pin maven-surefire-plugin 3.2.5. Maven < 3.9 defaults to surefire 2.12.4,
    which cannot run JUnit 5 and reports "no tests to run" instead of failing.
  • Relocate the shaded NpcApi classes so the plugin cannot clash with another
    plugin bundling the same library.
  • Drop dev.folia:folia-api (zero usages, and pinned to 1.20.1 in a 1.21.1
    project) and the test-scope spigot-api 1.21.3 that conflicted with
    paper-api 1.21.1; paper-api already covers the Bukkit API the tests use.
  • Restrict resource filtering to plugin.yml, the only resource using ${}.
  • Restore .gitignore, deleted in 5088e94.

Shutdown:

  • Add the Cleanable interface and collect registered components into a list
    that onDisable() walks. Bukkit cancels scheduler tasks by itself, but boss
    bars stayed pinned to the screens of players who survived a /reload while
    the summoned boss stayed in the world untracked.
  • Stop localising the disable log line: messageManager is null when startup
    failed early, and the NPE masked the original error.

Player data:

  • Evict a player's cached profile on quit. DataManager loaded profiles lazily
    and never removed them, so the cache grew for every player who ever joined.
  • Back the cache with a ConcurrentHashMap; it is read from scheduler tasks.
  • Add saveAsync() for the five-minute autosave: configurations are serialised
    on the main thread, only the disk writes move off it. The old path wrote
    every cached profile plus a backup copy inline on the server tick.

Cleanup:

  • Remove src/main/NpcApi-Paper-master/, a vendored copy of the upstream Gradle
    project (47 files, incl. a gradle-wrapper.jar). Maven only compiles
    src/main/java, so it never took part in the build.
  • Remove bosses/backup/OriginalBoss2Backup.java; git history covers this.
  • Register TextureTestCommand, declared in plugin.yml but never bound.
  • Gate Act4Listener behind act4.autoSpawnArtifacts instead of a commented-out
    registration line.
  • Fail loudly when a command is missing from plugin.yml rather than NPE.
  • Drop the duplicate getNpcManager() getter; keep getNPCManager().
  • Replace printStackTrace() with logger calls that keep the stack trace.
  • Correct README claims that did not match the code: storage is YAML, not
    JSON, and saving was neither async nor cleaning tasks up until now.

Add PlayerSettingsTest covering the dialog speed cycle and fromString
fallback.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_017441xEm5EainbMSewf8u6e

claude added 2 commits July 26, 2026 14:31
The project did not build: de.eisi05:NpcApi-Paper is published on JitPack but
no such repository was declared, so dependency resolution failed outright. The
CI workflow was an unmodified GitHub template that would have failed anyway --
JDK 11 against a Java 21 source level, and an `mvn deploy` step with no
distributionManagement in the POM.

Build:
- Declare the JitPack repository so NpcApi-Paper resolves.
- CI: JDK 21, build on push and pull_request (not only on release), upload the
  jar as an artifact, and attach it to releases instead of deploying nowhere.
- Pin maven-surefire-plugin 3.2.5. Maven < 3.9 defaults to surefire 2.12.4,
  which cannot run JUnit 5 and reports "no tests to run" instead of failing.
- Relocate the shaded NpcApi classes so the plugin cannot clash with another
  plugin bundling the same library.
- Drop dev.folia:folia-api (zero usages, and pinned to 1.20.1 in a 1.21.1
  project) and the test-scope spigot-api 1.21.3 that conflicted with
  paper-api 1.21.1; paper-api already covers the Bukkit API the tests use.
- Restrict resource filtering to plugin.yml, the only resource using ${}.
- Restore .gitignore, deleted in 5088e94.

Shutdown:
- Add the Cleanable interface and collect registered components into a list
  that onDisable() walks. Bukkit cancels scheduler tasks by itself, but boss
  bars stayed pinned to the screens of players who survived a /reload while
  the summoned boss stayed in the world untracked.
- Stop localising the disable log line: messageManager is null when startup
  failed early, and the NPE masked the original error.

Player data:
- Evict a player's cached profile on quit. DataManager loaded profiles lazily
  and never removed them, so the cache grew for every player who ever joined.
- Back the cache with a ConcurrentHashMap; it is read from scheduler tasks.
- Add saveAsync() for the five-minute autosave: configurations are serialised
  on the main thread, only the disk writes move off it. The old path wrote
  every cached profile plus a backup copy inline on the server tick.

Cleanup:
- Remove src/main/NpcApi-Paper-master/, a vendored copy of the upstream Gradle
  project (47 files, incl. a gradle-wrapper.jar). Maven only compiles
  src/main/java, so it never took part in the build.
- Remove bosses/backup/OriginalBoss2Backup.java; git history covers this.
- Register TextureTestCommand, declared in plugin.yml but never bound.
- Gate Act4Listener behind act4.autoSpawnArtifacts instead of a commented-out
  registration line.
- Fail loudly when a command is missing from plugin.yml rather than NPE.
- Drop the duplicate getNpcManager() getter; keep getNPCManager().
- Replace printStackTrace() with logger calls that keep the stack trace.
- Correct README claims that did not match the code: storage is YAML, not
  JSON, and saving was neither async nor cleaning tasks up until now.

Add PlayerSettingsTest covering the dialog speed cycle and fromString
fallback.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017441xEm5EainbMSewf8u6e
messages_en.yml defined three keys twice. YAML keeps only the last definition,
so each duplicate silently discarded the block above it:

- chest.items (lines 194 and 253) dropped six story items - stabilization_core,
  act1_skeleton_key, boss1_material, boss1_catalyst, boss1_summon_key and
  boss2_structure_key - leaving English players with the Russian fallback for
  every one of their names and lore lines.
- act5 (lines 71 and 363) dropped exit_blocked, too_far, artifacts_count and
  returned_overworld.
- npc (lines 6 and 420) dropped direction_marker.

The entity names had also drifted apart structurally: Russian nests them under
npc.entities, English had them at the top level as entities. Act3Listener:165
read entities.end_guardian, a path that only ever existed in the English file,
so Russian players saw the boss named "entities.end_guardian" - the raw key
that MessageManager returns when a lookup misses. The other two call sites
already used npc.entities, so English now follows that shape and the listener
was corrected.

With the duplicates merged and the remaining gaps translated in both
directions, the two files expose an identical set of 322 keys.

Two regression tests cover this: one asserts the locale key sets match, the
other rejects duplicate keys. Both were checked against the pre-fix file to
confirm they fail on the bugs described above.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017441xEm5EainbMSewf8u6e
@Rethinger
Rethinger merged commit 65520db into main Jul 26, 2026
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants