Keep faction vehicle records when a vehicle's chunk unloads - #65
Conversation
VehicleFramework fires VehicleRemoveEvent when a chunk unloads, and the integration listener dropped the pool or installation record for any removal. A faction vehicle whose chunk unloaded lost its record, so it came back as the leader's personal vehicle. Records are now only dropped when the vehicle is gone for good, which the vehicle fee listener shares through VehicleRemovals. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughVehicle removal handling now preserves faction records when vehicles unload. A shared utility classifies permanent removals, and both faction-record and reclaim-fee listeners use that classification. ChangesVehicle removal handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to Vehicle records are retained on unload, and no confirmed issue blocks merging after normal checks. The effect of other removal reasons on reclaim-fee records remains unverified. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change preserves existing faction records during temporary unloads without adding a way to create or transfer ownership. No newly exposed access path was established. Its correctness still depends on the vehicle framework reporting removal reasons accurately. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
A rabbit watched the carts roll by, Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
2 participants
VehicleFramework fires
VehicleRemoveEventwhen a vehicle's chunk unloads (VehicleRemoveReason.UNLOAD), not only when it is destroyed.VehicleIntegrationListener.onVehicleRemovedropped the vehicle's registry record on any removal and saved the registry. So a pool or installation vehicle whose chunk unloaded lost its faction record and came back as the leader's personal vehicle: the faction stopped paying its upkeep, it counted against the leader's personal slots, and it was no longer berthed.Fix. A new
VehicleRemovals.isGoneForGood(payload)returns false only forUNLOAD. Deaths, player destroy, admin kill,GENERICand a missing payload still count as removal, as before. The listener only drops the record when the vehicle is gone for good. The vehicle fee last-owner listener (#64) now uses the same helper instead of its own copy. It also now forgets the last owner onGENERICremovals, which VehicleFramework only sends when a vehicle dies.The only VehicleFramework paths that remove without destroying are the chunk-unload persist and a failed spawn setup. Both use
UNLOAD.Impact on Main. Main's registry is currently empty and nobody has pooled or berthed a vehicle there yet, so nothing has been lost so far.
Tests.
VehicleRemovalsTestandVehicleRemoveKeepsFactionVehiclesTest(a pool record survives an unload and is dropped on death).mvn verifypasses locally (2271 tests).🤖 Generated with Claude Code
Summary by CodeRabbit