You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix: preserve claim guards across vehicle event API versions - #166
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.
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.
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.
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size:M30-99 effective changed lines (tests excluded in mixed PRs).
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
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.