From e83aa6644f0f2600d50d7169dcd730d95ed3355c Mon Sep 17 00:00:00 2001 From: Ryan <7389646+ryanbarlow97@users.noreply.github.com> Date: Sun, 27 Sep 2026 13:45:23 +0000 Subject: [PATCH] Keep faction vehicle records when a vehicle's chunk unloads 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) --- .../vehicles/VehicleIntegrationListener.java | 14 ++++++- .../vehicles/VehicleRemovals.java | 21 ++++++++++ .../fees/VehicleReclaimFeeListener.java | 17 +------- .../vehicles/VehicleRemovalsTest.java | 26 ++++++++++++ ...VehicleRemoveKeepsFactionVehiclesTest.java | 42 +++++++++++++++++++ .../fees/VehicleReclaimFeeListenerTest.java | 24 ----------- 6 files changed, 104 insertions(+), 40 deletions(-) create mode 100644 src/main/java/net/tfminecraft/simplefactions/vehicles/VehicleRemovals.java create mode 100644 src/test/java/net/tfminecraft/simplefactions/vehicles/VehicleRemovalsTest.java create mode 100644 src/test/java/net/tfminecraft/simplefactions/vehicles/VehicleRemoveKeepsFactionVehiclesTest.java delete mode 100644 src/test/java/net/tfminecraft/simplefactions/vehicles/fees/VehicleReclaimFeeListenerTest.java diff --git a/src/main/java/net/tfminecraft/simplefactions/vehicles/VehicleIntegrationListener.java b/src/main/java/net/tfminecraft/simplefactions/vehicles/VehicleIntegrationListener.java index 6ffd6b0..39154ff 100644 --- a/src/main/java/net/tfminecraft/simplefactions/vehicles/VehicleIntegrationListener.java +++ b/src/main/java/net/tfminecraft/simplefactions/vehicles/VehicleIntegrationListener.java @@ -14,6 +14,7 @@ import org.bukkit.event.Listener; import net.tfminecraft.simplefactions.SimpleFactions; +import net.tfminecraft.simplefactions.vehicles.registry.PlayerVehicleRegistry; import net.tfminecraft.vfbuilders.core.Blueprint; import net.tfminecraft.vfbuilders.events.BeginVehicleConstructionEvent; import net.tfminecraft.vfbuilders.events.VehicleConstructEvent; @@ -69,7 +70,7 @@ public void onVehicleRemove(VehicleRemoveEvent event) { return; } VehicleRemovePayload payload = event.getPayload(); - if (SimpleFactions.getVehicleRegistry().unregister(vehicle.getUUID())) { + if (dropRecord(SimpleFactions.getVehicleRegistry(), vehicle.getUUID(), payload)) { SimpleFactions.getInstance().saveVehicleRegistry(); if (payload != null && payload.isDeath()) { payload.getDeathCause().ifPresent(cause -> @@ -84,6 +85,17 @@ public void onVehicleRemove(VehicleRemoveEvent event) { } } + /** + * Drops a removed vehicle's faction record. A berthed or pool vehicle whose chunk unloads is + * still the faction's, so it only loses its record when it is destroyed. + */ + static boolean dropRecord(PlayerVehicleRegistry registry, String vehicleUuid, VehicleRemovePayload payload) { + if (!VehicleRemovals.isGoneForGood(payload)) { + return false; + } + return registry.unregister(vehicleUuid); + } + private static String resolveOwnerEntry(UUID constructorUuid, Player onlineConstructor) { if (onlineConstructor != null) { return "player_" + onlineConstructor.getName(); diff --git a/src/main/java/net/tfminecraft/simplefactions/vehicles/VehicleRemovals.java b/src/main/java/net/tfminecraft/simplefactions/vehicles/VehicleRemovals.java new file mode 100644 index 0000000..465e23c --- /dev/null +++ b/src/main/java/net/tfminecraft/simplefactions/vehicles/VehicleRemovals.java @@ -0,0 +1,21 @@ +package net.tfminecraft.simplefactions.vehicles; + +import net.tfminecraft.vehicleframework.data.VehicleRemovePayload; +import net.tfminecraft.vehicleframework.enums.VehicleRemoveReason; + +/** + * VehicleFramework fires VehicleRemoveEvent when a vehicle's chunk unloads as well as when + * it is destroyed. An unloaded vehicle comes back when its chunk loads, so state kept about + * it must survive the unload. + */ +public final class VehicleRemovals { + private VehicleRemovals() {} + + /** False only for an unload; a missing payload counts as gone, as before payloads existed. */ + public static boolean isGoneForGood(VehicleRemovePayload payload) { + if (payload == null || payload.isDeath()) { + return true; + } + return payload.getRemoveReason().orElse(null) != VehicleRemoveReason.UNLOAD; + } +} diff --git a/src/main/java/net/tfminecraft/simplefactions/vehicles/fees/VehicleReclaimFeeListener.java b/src/main/java/net/tfminecraft/simplefactions/vehicles/fees/VehicleReclaimFeeListener.java index 7184ff5..820031e 100644 --- a/src/main/java/net/tfminecraft/simplefactions/vehicles/fees/VehicleReclaimFeeListener.java +++ b/src/main/java/net/tfminecraft/simplefactions/vehicles/fees/VehicleReclaimFeeListener.java @@ -7,9 +7,8 @@ import net.tfminecraft.simplefactions.government.proposal.FeeKind; import net.tfminecraft.simplefactions.utils.Permissions; +import net.tfminecraft.simplefactions.vehicles.VehicleRemovals; import net.tfminecraft.simplefactions.vehicles.fees.VehicleFeeService.Quote; -import net.tfminecraft.vehicleframework.data.VehicleRemovePayload; -import net.tfminecraft.vehicleframework.enums.VehicleRemoveReason; import net.tfminecraft.vehicleframework.events.VehicleOwnerClaimedEvent; import net.tfminecraft.vehicleframework.events.VehicleRemoveEvent; import net.tfminecraft.vehicleframework.vehicles.ActiveVehicle; @@ -73,7 +72,7 @@ public void onClaimed(VehicleOwnerClaimedEvent event) { @EventHandler(priority = EventPriority.MONITOR) public void onVehicleRemove(VehicleRemoveEvent event) { - if (event.getVehicle() == null || !isDestroyed(event.getPayload())) { + if (event.getVehicle() == null || !VehicleRemovals.isGoneForGood(event.getPayload())) { return; } if (store.getLastOwner(event.getVehicle().getUUID()) != null) { @@ -81,16 +80,4 @@ public void onVehicleRemove(VehicleRemoveEvent event) { saver.run(); } } - - /** Chunk unloads also fire VehicleRemoveEvent; only a vehicle that is gone for good is forgotten. */ - static boolean isDestroyed(VehicleRemovePayload payload) { - if (payload == null) { - return false; - } - if (payload.isDeath()) { - return true; - } - VehicleRemoveReason reason = payload.getRemoveReason().orElse(null); - return reason == VehicleRemoveReason.PLAYER_DESTROY || reason == VehicleRemoveReason.ADMIN_KILL; - } } diff --git a/src/test/java/net/tfminecraft/simplefactions/vehicles/VehicleRemovalsTest.java b/src/test/java/net/tfminecraft/simplefactions/vehicles/VehicleRemovalsTest.java new file mode 100644 index 0000000..93f5b08 --- /dev/null +++ b/src/test/java/net/tfminecraft/simplefactions/vehicles/VehicleRemovalsTest.java @@ -0,0 +1,26 @@ +package net.tfminecraft.simplefactions.vehicles; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +import net.tfminecraft.vehicleframework.data.VehicleRemovePayload; +import net.tfminecraft.vehicleframework.enums.VehicleDeath; +import net.tfminecraft.vehicleframework.enums.VehicleRemoveReason; + +class VehicleRemovalsTest { + @Test + void chunkUnloadIsNotGone() { + assertFalse(VehicleRemovals.isGoneForGood(VehicleRemovePayload.remove(VehicleRemoveReason.UNLOAD))); + } + + @Test + void destructionIsGone() { + assertTrue(VehicleRemovals.isGoneForGood(VehicleRemovePayload.death(VehicleDeath.DIE))); + assertTrue(VehicleRemovals.isGoneForGood(VehicleRemovePayload.remove(VehicleRemoveReason.PLAYER_DESTROY))); + assertTrue(VehicleRemovals.isGoneForGood(VehicleRemovePayload.remove(VehicleRemoveReason.ADMIN_KILL))); + assertTrue(VehicleRemovals.isGoneForGood(VehicleRemovePayload.remove(VehicleRemoveReason.GENERIC))); + assertTrue(VehicleRemovals.isGoneForGood(null)); + } +} diff --git a/src/test/java/net/tfminecraft/simplefactions/vehicles/VehicleRemoveKeepsFactionVehiclesTest.java b/src/test/java/net/tfminecraft/simplefactions/vehicles/VehicleRemoveKeepsFactionVehiclesTest.java new file mode 100644 index 0000000..9d22177 --- /dev/null +++ b/src/test/java/net/tfminecraft/simplefactions/vehicles/VehicleRemoveKeepsFactionVehiclesTest.java @@ -0,0 +1,42 @@ +package net.tfminecraft.simplefactions.vehicles; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.UUID; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import net.tfminecraft.simplefactions.vehicles.registry.OwnershipMode; +import net.tfminecraft.simplefactions.vehicles.registry.PlayerVehicleRecord; +import net.tfminecraft.simplefactions.vehicles.registry.PlayerVehicleRegistry; +import net.tfminecraft.vehicleframework.data.VehicleRemovePayload; +import net.tfminecraft.vehicleframework.enums.VehicleDeath; +import net.tfminecraft.vehicleframework.enums.VehicleRemoveReason; + +class VehicleRemoveKeepsFactionVehiclesTest { + private PlayerVehicleRegistry registry; + + @BeforeEach + void setUp() { + registry = new PlayerVehicleRegistry(); + registry.register(new PlayerVehicleRecord(UUID.randomUUID(), "v1", "sloop", OwnershipMode.POOL, null, "rome")); + } + + @Test + void chunkUnloadKeepsThePoolRecord() { + assertFalse(VehicleIntegrationListener.dropRecord( + registry, "v1", VehicleRemovePayload.remove(VehicleRemoveReason.UNLOAD))); + + assertTrue(registry.isFactionOwned("v1")); + } + + @Test + void destroyedVehicleLosesItsRecord() { + assertTrue(VehicleIntegrationListener.dropRecord( + registry, "v1", VehicleRemovePayload.death(VehicleDeath.DIE))); + + assertFalse(registry.isFactionOwned("v1")); + } +} diff --git a/src/test/java/net/tfminecraft/simplefactions/vehicles/fees/VehicleReclaimFeeListenerTest.java b/src/test/java/net/tfminecraft/simplefactions/vehicles/fees/VehicleReclaimFeeListenerTest.java deleted file mode 100644 index fa18655..0000000 --- a/src/test/java/net/tfminecraft/simplefactions/vehicles/fees/VehicleReclaimFeeListenerTest.java +++ /dev/null @@ -1,24 +0,0 @@ -package net.tfminecraft.simplefactions.vehicles.fees; - -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import org.junit.jupiter.api.Test; - -import net.tfminecraft.vehicleframework.data.VehicleRemovePayload; -import net.tfminecraft.vehicleframework.enums.VehicleRemoveReason; - -class VehicleReclaimFeeListenerTest { - @Test - void chunkUnloadKeepsTheLastOwner() { - assertFalse(VehicleReclaimFeeListener.isDestroyed(VehicleRemovePayload.remove(VehicleRemoveReason.UNLOAD))); - assertFalse(VehicleReclaimFeeListener.isDestroyed(VehicleRemovePayload.remove(VehicleRemoveReason.GENERIC))); - assertFalse(VehicleReclaimFeeListener.isDestroyed(null)); - } - - @Test - void destroyedVehiclesAreForgotten() { - assertTrue(VehicleReclaimFeeListener.isDestroyed(VehicleRemovePayload.remove(VehicleRemoveReason.PLAYER_DESTROY))); - assertTrue(VehicleReclaimFeeListener.isDestroyed(VehicleRemovePayload.remove(VehicleRemoveReason.ADMIN_KILL))); - } -}