Skip to content

fix: preserve claim guards across vehicle event API versions - #166

Merged
ryanbarlow97 merged 1 commit into
mainfrom
stage7/account-api
Oct 14, 2026
Merged

ryanbarlow97 merged 1 commit into
mainfrom
stage7/account-api

Conversation

@ryanbarlow97

@ryanbarlow97 ryanbarlow97 commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Keep vehicle claims protected when VehicleFramework removes VehicleOwnerClaimedEvent. The fallback guard now cancels modern VehicleOwnershipChangeEvent CLAIM actions for non-staff; a separate legacy listener loads only after the modern event probe fails and the legacy event class exists. Staff policy, other ownership reasons and full character-mode integration keep their existing checks. An unknown provider without either event is rejected explicitly.

The legacy event type appears only in the isolated bridge and test fixtures. The selector and normal plugin startup have no hard linkage to it. Update a generic cancellable fixture to VehiclePreInteractEvent and remove an obsolete DE mock verification so SF compiles against the contracted economy API. Optional throwing Bank mutators stay for this bounded change; they grant no financial authority.

Validation:

  • Java 21 tfmc-mvn -o -B clean verify: 8,043 tests, no failures/errors/skips; existing coverage checks pass.
  • Unit guard matrix covers player/staff/absent actors, CLAIM vs other reasons, modern preference, legacy fallback, class linkage failure and unsupported providers. Existing lifecycle coverage exercises full integration and NAMES/probe-failure fallback.
  • Actual VF2.10.0 and published2.13.0 JARs plus the new VF candidate select/link the appropriate handler. Modern providers never load the obsolete bridge or event.
  • Full production/test compilation against the DE candidate passes; all production class bytes match the full-gate build. All 305 focused runtime tests against the new DE candidate also pass.
  • Refreshed41-repository source and89-distinct-JAR consumer census identifies this guard as the sole external consumer of the removed VF event.

Coordinated source: TF-Minecraft/DenarEconomy#40 and TF-Minecraft/VehicleFramework#72
Docs: TF-Minecraft/Docs#240

Install new SF before the contracted VF if the cohort ever spans restarts; otherwise use one reviewed stopped Dev cohort after verified6c activation. Preserve data shapes/readers and the complete6c rollback set. No Main deployment.

@github-actions github-actions Bot added the size:M 30-99 effective changed lines (tests excluded in mixed PRs). label Oct 14, 2026
@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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: 6aaaed92-5d29-4cf3-9124-2413c898c9d9
📥 Commits

Reviewing files that changed from the base of the PR and between 59bd617 and 8f3f979.

📒 Files selected for processing: 8
  • src/main/java/net/tfminecraft/simplefactions/SimpleFactions.java
  • src/main/java/net/tfminecraft/simplefactions/vehicles/owners/LegacyVehicleClaimsOff.java
  • src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimGuards.java
  • src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimsOff.java
  • src/test/java/net/tfminecraft/simplefactions/managers/PlayerManagerCoverageTest.java
  • src/test/java/net/tfminecraft/simplefactions/vehicles/berth/VehicleInteractionsCoverageTest.java
  • src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimsOffTest.java
  • src/test/java/net/tfminecraft/simplefactions/vehicles/registry/VehicleRegistryClaimListenerTest.java
💤 Files with no reviewable changes: 2
  • src/test/java/net/tfminecraft/simplefactions/managers/PlayerManagerCoverageTest.java
  • src/test/java/net/tfminecraft/simplefactions/vehicles/berth/VehicleInteractionsCoverageTest.java

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

📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Vehicle claim restrictions now apply when full vehicle ownership integration is unavailable, using either supported claim-event format.
    • Ordinary players’ claims are refused with a message; staff claims and takeovers remain allowed.
    • Claim handling is more consistent across modern and legacy ownership integrations.

The fallback vehicle integration now selects a listener for the available ownership event API. Both listeners use shared rules to refuse claims from non-admin players.

Changes: Vehicle claim compatibility

Layer / File(s) Summary
Provider selection and claim refusal
src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimGuards.java, src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimsOff.java, src/main/java/net/tfminecraft/simplefactions/vehicles/owners/LegacyVehicleClaimsOff.java, src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimsOffTest.java
VehicleClaimGuards checks for the modern event class first, then the legacy event class. The modern listener handles CLAIM ownership changes, and the legacy listener handles owner-claimed events. Both use shared refusal rules. Tests cover listener selection and claim handling.
Fallback registration and test updates
src/main/java/net/tfminecraft/simplefactions/SimpleFactions.java, src/test/java/net/tfminecraft/simplefactions/vehicles/registry/VehicleRegistryClaimListenerTest.java, src/test/java/net/tfminecraft/simplefactions/vehicles/berth/VehicleInteractionsCoverageTest.java
The non-full integration branch registers the listener returned by VehicleClaimGuards.create(). Registry tests now construct VehiclePreInteractEvent; the vehicle interactions test removes an unused import.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant SimpleFactions
  participant VehicleClaimGuards
  participant VehicleClaimsOff
  participant LegacyVehicleClaimsOff
  participant Player
  SimpleFactions->>VehicleClaimGuards: create()
  VehicleClaimGuards->>VehicleClaimGuards: Check modern event class
  alt Modern event class is available
    VehicleClaimGuards-->>SimpleFactions: VehicleClaimsOff
    VehicleClaimsOff->>VehicleClaimGuards: refuse(player, event) for CLAIM
  else Modern event class is unavailable
    VehicleClaimGuards->>VehicleClaimGuards: Check legacy event class
    VehicleClaimGuards-->>SimpleFactions: LegacyVehicleClaimsOff
    LegacyVehicleClaimsOff->>VehicleClaimGuards: refuse(player, event)
  end
  alt Player is null or an admin
    VehicleClaimGuards-->>VehicleClaimsOff: Leave event unchanged
  else Player is not an admin
    VehicleClaimGuards->>Player: Send MESSAGE
    VehicleClaimGuards-->>VehicleClaimsOff: Cancel event
  end
Loading
Merge Risk: ⚪ Minimal · up to 8f3f9

No actionable issue is established for the vehicle-claim compatibility change; it is ready for normal merge checks.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@ryanbarlow97
ryanbarlow97 merged commit b3372b1 into main Oct 14, 2026
3 checks passed
@ryanbarlow97
ryanbarlow97 deleted the stage7/account-api branch October 14, 2026 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 effective changed lines (tests excluded in mixed PRs).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant