[XxFran10xX]

Problem

A player on Main (carpiie, 2026-10-06) picked Ranger, reopened /class and saw Archer unselected. Picking a subclass replaces the base class in MMOCore, and the class window only marks the exact current class. So they picked Archer again, which removed Ranger, then picked Ranger again, and went round in circles.

They also had ARROW_VOLLEY bound from before the class rework, when base Archer still had it. MMOCore gives every class every MythicLib skill (the ones the class file doesn't list are locked) and keeps saved bindings. So the old binding survived the rework and stayed on the skill bar as Archer, even though /skills said the class had no skills. Ranger is passives-only, so as Ranger F had nothing to cast, and it looked as if the subclass was broken.

Changes

• In the class window, the base class of the player's current subclass now shows "Your class, via " with a glint. It also warns: "Picking it removes your subclass." It can still be picked; the price and the click-to-confirm step are unchanged.
• ClassPickService.subclassOf(character, base) returns the character's subclass when it belongs to that base.
• Whenever a class is applied (a pick, a character switch or a join), ClassService unbinds every bound skill the class has not unlocked. A skill counts as unlocked if the class file lists it (unlocked by default) or the player unlocked it. It uses PlayerData.hasUnlocked, not the level check, so a level that is briefly low on join can't drop real bindings.

No class on Main lists a skill as locked by default (unlocked-by-default: false only appears on skill slots), and no TFMC plugin binds skills itself. So the only bindings removed are leftovers like this one.

Tests

• ClassServiceTest.boundSkillsTheClassHasNotUnlockedAreUnboundWhenItIsApplied: re-applying the current class keeps the unlocked binding and drops a locked one and one the class no longer has. Switching class drops a binding the new class lacks.
• ClassPickTest.baseClassOfTheCurrentSubclassIsMarkedAndWarnsThatPickingItRemovesTheSubclass
• Local mvn verify (Windows): 1661 tests pass, and the changed classes have full line coverage. I excluded the tests that only fail on Windows (POSIX permissions, file locks, locale); Linux CI runs them.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 6f72b4ee-f3f3-4f98-887e-11d9dda1b871

📥 Commits

Reviewing files that changed from the base of the PR and between 8551bf8 and 979d990.

📒 Files selected for processing (5)

• src/main/java/net/tfminecraft/rpcharacters/classpick/ClassPickGui.java
• src/main/java/net/tfminecraft/rpcharacters/classpick/ClassPickService.java
• src/main/java/net/tfminecraft/rpcharacters/mmocore/ClassService.java
• src/test/java/net/tfminecraft/rpcharacters/classpick/ClassPickTest.java
• src/test/java/net/tfminecraft/rpcharacters/mmocore/ClassServiceTest.java

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

---

📝 Summary

Summary by CodeRabbit

• New Features
  • The class picker now identifies the subclass associated with a base-class option and warns that selecting the base class will remove that subclass.
• Bug Fixes
  • Skill bindings that are unavailable or not yet unlocked for the active class are now removed when a class is applied. Valid, unlocked bindings are retained.

Walkthrough

The class picker now identifies when choosing a base class removes the current subclass. Applying a class now removes bound skills that the class lacks or has not unlocked.

Changes

Class management

Layer / File(s)	Summary
Subclass removal information src/main/java/net/tfminecraft/rpcharacters/classpick/ClassPickService.java, src/main/java/net/tfminecraft/rpcharacters/classpick/ClassPickGui.java, src/test/java/net/tfminecraft/rpcharacters/classpick/ClassPickTest.java	The picker identifies the current subclass associated with a base-class option. It displays a removal warning and glint for that option. The test covers subclass lookup and picker display.
Bound skill cleanup src/main/java/net/tfminecraft/rpcharacters/mmocore/ClassService.java, src/test/java/net/tfminecraft/rpcharacters/mmocore/ClassServiceTest.java	Class application removes bindings for skills that the class lacks or has not unlocked. The test checks that invalid bindings are removed and an unlocked skill binding is retained.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: ryanbarlow97

Merge Risk: ⚪ Minimal · up to 979d9

The picker now shows which class is the player's via a subclass and warns that picking the base class removes it. Class application also drops saved skill bindings the class lacks or has not unlocked. No concrete defect was established. The remaining uncertainty is the ordering of binding cleanup against missed-reward claims, which depends on MMOCore behaviour that could not be confirmed.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 979d9

The class-selection changes preserve existing eligibility, payment and confirmation checks. The main risk is recovery: a failed class change can remove skill bindings without restoring them. The effect appears limited to the affected player, but the production unlock rules still need confirmation.

Retained concerns

• Low · reliability · inferred: New binding deletions are outside the picker’s recovery boundary. If a progression operation fails after cleanup, the picker restores the previous stored class and refunds payment, but does not restore removed MMOCore bindings. This extends the pre-existing nontransactional class-loading behavior with an additional destructive mutation. The inconsistent recovery path is supported by source; a production failure after cleanup was not demonstrated.

Security review details

Security Blast Radius

• inferred — Automatic cleanup can affect any player whose class is applied on join, activation or selection, but each inspected invocation mutates that player’s MMOCore bindings. The changed paths do not establish new authority over another player or a broader service or environment.

Trust Boundaries and Controls

• observed — The GUI renders the player’s active character and retains its two-click confirmation. The authoritative selection service still checks an active character, creation state, class validity, subclass eligibility, level and payment before application. Subclass highlighting does not replace these controls.

Resilience and Maintainability Implications

• observed — Delayed join recovery checks that the player remains online and that the player-data object is still current, then selects the currently active character. Failed applications receive bounded retries. These safeguards limit stale lifecycle work, but do not restore bindings already removed during an unsuccessful application.

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}