Keep faction vehicle records when a vehicle's chunk unloads

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();

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;
+    }
+}

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,24 +72,12 @@ 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) {
             store.forgetVehicle(event.getVehicle().getUUID());
             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;
-    }
 }

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));
+    }
+}

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"));
+    }
+}