Skip to content

Remove nameplate functionality from rpk-characters-bukkit - #6

Merged
dmccoystephenson merged 2 commits into
mainfrom
feature/remove-nameplate-functionality
Aug 10, 2026
Merged

Remove nameplate functionality from rpk-characters-bukkit#6
dmccoystephenson merged 2 commits into
mainfrom
feature/remove-nameplate-functionality

Conversation

@dmccoystephenson

Copy link
Copy Markdown
Member

Summary

Ports upstream PR RP-Kit#669 into this fork, resolving the last compile error blocking the build.

This is not my design decision — it is the upstream maintainer's. On RP-Kit#666, renbinden (project owner) ruled:

I think we just go 2.5 and gut the nameplate functionality, it only causes problems and is doable with placeholders I think

There is a feature to update player nameplates to character names. It only worked on the specific combination of ProtocolLib and Minecraft it was designed for each time it was updated, and each update was a huge pain

Upstream PR RP-Kit#669 (dmccoystephenson, 2024-07-10, still open) implements that. Cherry-picked here as commit 73fa53a9, preserving authorship.

The alternative — disambiguating the PlayerInfoData constructor call to keep the feature — was deliberately not taken: it contradicts the maintainer's direction, and choosing between the two 7-argument overloads has runtime implications on a code path that cannot be exercised without a live Minecraft server.

What this removes

Protocol.kt in its entirety (122 lines) plus its call sites — 208 deletions across 8 files, exactly matching upstream RP-Kit#669:

  • protocol/Protocol.kt — deleted
  • RPKCharactersBukkit.kt, RPKCharacterServiceImpl.kt, CharacterSetNameCommand.kt, PlayerJoinListener.kt, CharacterHandler.kt — call sites removed
  • build.gradle — ProtocolLib dependency dropped
  • config.yml — the associated key removed

One deliberate divergence from upstream RP-Kit#669

plugin.yml: dropped the now-dead ProtocolLib softdepend (+1 file, 1 deletion beyond the cherry-pick).

Upstream RP-Kit#669 removes every use of ProtocolLib but leaves softdepend: [..., ProtocolLib] in place, so the module would still advertise an optional dependency on a library it no longer touches. Harmless at runtime, but inaccurate as a declaration and exactly the kind of doc drift worth not importing. Called out here rather than folded in silently.

Conflict resolution

The cherry-pick conflicted in rpk-characters-bukkit/build.gradle: this fork had just bumped PlaceholderAPI to 2.11.7 (#3), while upstream RP-Kit#669 predates that and still carries 2.11.2. Resolved by keeping 2.11.7 (the version that actually resolves) and taking RP-Kit#669's removal of the ProtocolLib line. Both intents preserved.

Anchor verdict

UNVERIFIED locally — no JDK 17 on the development device, so ./gradlew cannot produce a signal there. CI is the anchor; its run on this PR is the real verdict, and this PR exists specifically to turn it green.

Test plan

  • Cherry-pick matches upstream Removed nameplate functionality from rpk-characters-bukkit RP-Kit/RPKit#669 exactly — 8 files, 0 additions, 208 deletions
  • Protocol.kt deleted; no remaining reference to bukkit.protocol, ProtocolLibrary, or protocolManager in the module (the sole hit is a pre-existing commented-out line at RPKCharactersBukkit.kt:213, left untouched as out of scope)
  • plugin.yml parses as valid YAML once the @version@ Gradle token is substituted; softdepend is now ['PlaceholderAPI']
  • Confirmed the nameplate hits in rpk-monsters-bukkit are an unrelated monster-nameplate feature in a different module, untouched
  • repo.properties untouched
  • CI compiles rpk-characters-bukkit — the module that has been failing

Do-not-auto-merge

Modifies **/plugin.yml and **/build.gradle, both on the do-not-auto-merge list, and deletes well over 50 lines from a single file. Left for human review.

Closes #5

dmccoystephenson and others added 2 commits August 10, 2026 00:07
Removing the nameplate functionality removed the module's only use of
ProtocolLib, but plugin.yml still advertised it as an optional
dependency. Harmless at runtime, inaccurate as a declaration.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmccoystephenson

Copy link
Copy Markdown
Member Author

Self-review rubric:

  • CI: PASS — green, and verified rather than taken from the summary line: gh run view --json jobs shows both Compile and Test with conclusion: success, 73 :test tasks executed, and two BUILD SUCCESSFUL markers (one per Gradle invocation). This is the first green build on this fork.
  • Scope: PASS — 9 files. 8 are the cherry-pick of upstream Removed nameplate functionality from rpk-characters-bukkit RP-Kit/RPKit#669 verbatim; the 9th (plugin.yml) is the declared divergence, called out in the PR body rather than folded in silently.
  • Issue resolution: PASS — every acceptance criterion on Fix "Overload resolution ambiguity" compiling rpk-characters-bukkit #5 is met: rpk-characters-bukkit compiles in CI, the approach follows the upstream maintainer's stated direction (removal, not a patch to the ambiguous call), and no behaviour changed beyond what removing the feature implies.
  • Docs: PASS — both documentation rows in the Phase 7 table were checked against the implementation, not assumed. config.yml: the cherry-pick removed set-player-nameplate: true, and grep confirms no Kotlin remains that reads it, so there is no dangling config read at runtime. plugin.yml: parses as valid YAML once the @version@ Gradle token is substituted, softdepend is now ['PlaceholderAPI'], and the character/species command declarations are untouched.
  • Correct module of the lib/impl pair: PASS — all changes are in rpk-characters-bukkit (the implementation). rpk-character-lib-bukkit is untouched, which is right: Protocol.kt was implementation-only and exposed no API surface.
  • Fidelity to upstream: PASS — the cherry-pick is byte-identical to upstream Removed nameplate functionality from rpk-characters-bukkit RP-Kit/RPKit#669's commit 73fa53a9 (8 files, 0 additions, 208 deletions) with authorship preserved. The single addition beyond it is declared in the PR body. No acceptance criterion was invented: removal is what the maintainer asked for on Fix "Overload resolution ambiguity" error occurring upon compiling rpk-characters-bukkit module RP-Kit/RPKit#666, quoted verbatim in Fix "Overload resolution ambiguity" compiling rpk-characters-bukkit #5.
  • No credential churn: PASSrepo.properties is not in the diff.
  • Tests-new: not applicable — this PR is a pure deletion; it adds no public methods to test.
  • Tests-fix: not applicable, and worth being precise about why — the Phase 8 regression gate covers behavioural bug fixes. This was a compile-time defect: the code did not build at all, so there was no runtime behaviour to characterise and no stash-and-run experiment to run. The anchor going from red to green on the exact module that was failing is the empirical evidence, and it is direct rather than inferred.
  • License header / Permission declared / Result type / Service resolution / Main-thread I/O / Messages externalized: not applicable — no files added, no commands or permissions changed (plugin.yml's command block is untouched), and no code paths added.

One thing I want a reviewer's eye on rather than asserting myself: this removes a user-facing feature. Any server currently running this fork with set-player-nameplate: true loses character-name nameplates on upgrade. That is the maintainer's intended outcome and renbinden noted the capability is reachable via placeholders, but I have not verified that placeholder route works, and it is not something CI can tell us. Worth a line in release notes whenever this fork is next deployed.

Recommendation: merge. Green anchor, scope matches the issue, and the divergence from upstream is one line and declared.

@dmccoystephenson
dmccoystephenson merged commit 1b0e790 into main Aug 10, 2026
1 check 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.

Fix "Overload resolution ambiguity" compiling rpk-characters-bukkit

1 participant