From 96b01a4fb5a40ab7cbf362d7952ae1e90cfb4b54 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Wed, 11 Nov 2026 18:51:07 +0100 Subject: [PATCH 1/2] feat: idle vehicle cleanup for worlds like the Tutorial New config section `cleanup:` (worlds, keep-owners, idle-minutes). Every minute, a loaded vehicle on a cleanup world that has had no passenger and has not moved for idle-minutes is removed like an admin kill (removed and tombstoned). Owners listed in keep-owners (stored owner values, e.g. char_ for a staff demo row) are never removed. Moving counts as use, so train cars behind an occupied locomotive, horse-drawn carts and towed vehicles stay. No worlds (the default) means no cleanup. Co-Authored-By: Claude Opus 5.5 --- .../vehicleframework/cache/Cache.java | 6 + .../loaders/ConfigLoader.java | 8 ++ .../managers/VehicleCleanup.java | 84 ++++++++++++ .../managers/VehicleManager.java | 28 ++++ src/main/resources/config.yml | 9 ++ .../ConfigurationLoadersCoverageTest.java | 7 + .../managers/VehicleCleanupTest.java | 125 ++++++++++++++++++ .../managers/VehicleManagerCoverageTest.java | 33 ++++- 8 files changed, 299 insertions(+), 1 deletion(-) create mode 100644 src/main/java/net/tfminecraft/vehicleframework/managers/VehicleCleanup.java create mode 100644 src/test/java/net/tfminecraft/vehicleframework/managers/VehicleCleanupTest.java diff --git a/src/main/java/net/tfminecraft/vehicleframework/cache/Cache.java b/src/main/java/net/tfminecraft/vehicleframework/cache/Cache.java index ef26c42..8291256 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/cache/Cache.java +++ b/src/main/java/net/tfminecraft/vehicleframework/cache/Cache.java @@ -22,6 +22,12 @@ public class Cache { // Keep the same defaults as an empty config if the first YAML load fails. public static int despawnDistance = 64 * 64; + /** cleanup.worlds (lower case): idle vehicles there are removed; empty means no cleanup. */ + public static Set cleanupWorlds = new HashSet<>(); + /** cleanup.keep-owners: stored owner values whose vehicles the cleanup never removes. */ + public static Set cleanupKeepOwners = new HashSet<>(); + /** cleanup.idle-minutes, in milliseconds: how long a vehicle may stand empty. */ + public static long cleanupIdleMillis = 30 * 60_000L; public static boolean blockDamage = true; diff --git a/src/main/java/net/tfminecraft/vehicleframework/loaders/ConfigLoader.java b/src/main/java/net/tfminecraft/vehicleframework/loaders/ConfigLoader.java index 515c203..6c4c518 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/loaders/ConfigLoader.java +++ b/src/main/java/net/tfminecraft/vehicleframework/loaders/ConfigLoader.java @@ -2,6 +2,7 @@ import java.io.File; import java.io.IOException; +import java.util.HashSet; import java.util.Locale; import org.bukkit.Material; @@ -90,6 +91,13 @@ public void load(File configFile) { Cache.despawnDistance = (int) Math.round(Math.pow(config.getInt("despawn-distance", 64), 2)); + Cache.cleanupWorlds = new HashSet<>(); + for (String world : config.getStringList("cleanup.worlds")) { + Cache.cleanupWorlds.add(world.toLowerCase(Locale.ROOT)); + } + Cache.cleanupKeepOwners = new HashSet<>(config.getStringList("cleanup.keep-owners")); + Cache.cleanupIdleMillis = Math.max(1, config.getInt("cleanup.idle-minutes", 30)) * 60_000L; + Cache.skinItem = config.getString("skin-item", "v.bucket"); Cache.repairItem = config.getString("repair-item", "v.iron_shovel"); Cache.destroyItem = config.getString("destroy-item", "v.stone_axe"); diff --git a/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleCleanup.java b/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleCleanup.java new file mode 100644 index 0000000..1cb46c0 --- /dev/null +++ b/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleCleanup.java @@ -0,0 +1,84 @@ +package net.tfminecraft.vehicleframework.managers; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; + +import org.bukkit.Location; +import org.bukkit.World; +import org.bukkit.entity.Entity; + +import net.tfminecraft.vehicleframework.cache.Cache; +import net.tfminecraft.vehicleframework.data.OwnerData; +import net.tfminecraft.vehicleframework.vehicles.ActiveVehicle; + +/** + * Finds idle vehicles to remove on the worlds in {@code cleanup.worlds} (config.yml), such as a + * tutorial server's world. A vehicle is due once it has had no passenger and has not moved for + * {@code cleanup.idle-minutes}, counted from when the cleanup first saw it loaded or last saw it in + * use. Moving counts as use, so a train car behind an occupied locomotive, a horse-drawn cart or a + * towed vehicle is not idle. Owners in {@code cleanup.keep-owners} (stored owner values, e.g. + * {@code char_}) are never removed. With no worlds configured nothing is ever due. + */ +public final class VehicleCleanup { + + private final Map lastActive = new HashMap<>(); + private final Map lastPosition = new HashMap<>(); + + /** The loaded vehicles to remove now; also forgets vehicles that are no longer loaded. */ + public List due(Collection vehicles, long now) { + List due = new ArrayList<>(); + if (Cache.cleanupWorlds.isEmpty()) { + lastActive.clear(); + lastPosition.clear(); + return due; + } + Set seen = new HashSet<>(); + for (ActiveVehicle v : vehicles) { + String id = v.getUUID(); + // One vehicle can be registered under several entities: judge it once. + if (id == null || v.isDestroyed() || !seen.add(id) || !onCleanupWorld(v) || isKept(v)) { + continue; + } + boolean moved = moved(id, v.getEntity().getLocation()); + if (moved || !v.getSeatHandler().getPassengers().isEmpty()) { + lastActive.put(id, now); + continue; + } + Long since = lastActive.putIfAbsent(id, now); + if (since != null && now - since >= Cache.cleanupIdleMillis) { + due.add(v); + } + } + lastActive.keySet().retainAll(seen); + lastPosition.keySet().retainAll(seen); + return due; + } + + /** Whether the vehicle is more than a block from where the last check saw it. */ + private boolean moved(String id, Location at) { + if (at == null) { + return false; + } + double[] now = {at.getX(), at.getY(), at.getZ()}; + double[] before = lastPosition.put(id, now); + return before != null + && Math.abs(now[0] - before[0]) + Math.abs(now[1] - before[1]) + Math.abs(now[2] - before[2]) > 1.0; + } + + private static boolean onCleanupWorld(ActiveVehicle v) { + Entity entity = v.getEntity(); + World world = entity == null ? null : entity.getWorld(); + return world != null && Cache.cleanupWorlds.contains(world.getName().toLowerCase(Locale.ROOT)); + } + + private static boolean isKept(ActiveVehicle v) { + OwnerData owner = v.getOwnerData(); + return owner != null && owner.getOwner() != null && Cache.cleanupKeepOwners.contains(owner.getOwner()); + } +} diff --git a/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java b/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java index e12b6c4..4df885f 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java +++ b/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java @@ -132,6 +132,7 @@ public class VehicleManager implements Listener{ private HashMap> ejectCooldown = new HashMap<>(); private final OwnershipManager ownership = new OwnershipManager(this); + private final VehicleCleanup cleanup = new VehicleCleanup(); private HashMap tow = new HashMap<>(); @@ -296,6 +297,33 @@ public void start() { vehicleFastTickCycle(); vehicleSlowTickCycle(); snapshotCycle(); + cleanupCycle(); + } + + /** Every minute: remove the idle vehicles on the cleanup worlds (cleanup: in config.yml). */ + private void cleanupCycle() { + new BukkitRunnable() { + @Override + public void run() { + runCleanup(System.currentTimeMillis()); + } + }.runTaskTimer(VehicleFramework.plugin, 1200L, 1200L); + } + + /** Removes the vehicles the cleanup finds idle, like an admin kill; returns how many. */ + int runCleanup(long now) { + int removed = 0; + for (ActiveVehicle v : cleanup.due(new ArrayList<>(vehicles.values()), now)) { + String described = describeVehicle(v) + " at " + describeLocation(v); + v.remove(VehicleRemoveReason.ADMIN_KILL); + if (!persistDestroy(v)) { + VFLogger.log("Failed to tombstone " + described + " after cleanup"); + } + VFLogger.info("Cleanup removed " + described + " (empty for " + + Cache.cleanupIdleMillis / 60_000L + " minutes)"); + removed++; + } + return removed; } public void reload() { diff --git a/src/main/resources/config.yml b/src/main/resources/config.yml index efed6fb..748fa5d 100644 --- a/src/main/resources/config.yml +++ b/src/main/resources/config.yml @@ -101,6 +101,15 @@ enable-logging: false ##How far away does the player need to be for the vehicle to despawn? despawn-distance: 160 +##Idle vehicle cleanup, for a world where vehicles are only for trying out (e.g. a tutorial server). +##Every minute, a vehicle on one of these worlds that has had no passenger for idle-minutes is removed +##like an admin kill. keep-owners lists stored owner values (e.g. char_) that are never removed. +##No worlds (the default) means no cleanup. +cleanup: + worlds: [] + keep-owners: [] + idle-minutes: 30 + ##Explosion Damage block-damage: true diff --git a/src/test/java/net/tfminecraft/vehicleframework/loaders/ConfigurationLoadersCoverageTest.java b/src/test/java/net/tfminecraft/vehicleframework/loaders/ConfigurationLoadersCoverageTest.java index dcbfd27..f9ee617 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/loaders/ConfigurationLoadersCoverageTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/loaders/ConfigurationLoadersCoverageTest.java @@ -131,6 +131,10 @@ void configLoadsValidMaterialsAndAllRuntimeOptions() throws Exception { enable-logging: true block-damage: false despawn-distance: 17 + cleanup: + worlds: [TFMC_S2] + keep-owners: [char_demo] + idle-minutes: 0 skin-item: custom.skin repair-item: custom.repair destroy-item: custom.destroy @@ -153,6 +157,9 @@ void configLoadsValidMaterialsAndAllRuntimeOptions() throws Exception { assertTrue(Cache.enableLogging); assertFalse(Cache.blockDamage); assertEquals(289, Cache.despawnDistance); + assertEquals(Set.of("tfmc_s2"), Cache.cleanupWorlds); + assertEquals(Set.of("char_demo"), Cache.cleanupKeepOwners); + assertEquals(60_000L, Cache.cleanupIdleMillis); // at least a minute assertEquals("custom.skin", Cache.skinItem); assertEquals("custom.repair", Cache.repairItem); assertEquals("custom.destroy", Cache.destroyItem); diff --git a/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleCleanupTest.java b/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleCleanupTest.java new file mode 100644 index 0000000..94ec6c5 --- /dev/null +++ b/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleCleanupTest.java @@ -0,0 +1,125 @@ +package net.tfminecraft.vehicleframework.managers; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import java.util.*; + +import org.bukkit.Location; +import org.bukkit.World; +import org.bukkit.entity.Entity; +import org.bukkit.entity.Player; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import net.tfminecraft.vehicleframework.cache.Cache; +import net.tfminecraft.vehicleframework.data.OwnerData; +import net.tfminecraft.vehicleframework.vehicles.ActiveVehicle; + +class VehicleCleanupTest { + private static final long MIN = 60_000L; + private Set worlds, keep; + private long idle; + private final World tutorial = mock(World.class), main = mock(World.class); + private final VehicleCleanup cleanup = new VehicleCleanup(); + + @BeforeEach + void configure() { + worlds = Cache.cleanupWorlds; keep = Cache.cleanupKeepOwners; idle = Cache.cleanupIdleMillis; + when(tutorial.getName()).thenReturn("TFMC_S2"); + when(main.getName()).thenReturn("TFMC_Map"); + Cache.cleanupWorlds = new HashSet<>(Set.of("tfmc_s2")); + Cache.cleanupKeepOwners = new HashSet<>(Set.of("char_demo")); + Cache.cleanupIdleMillis = 30 * MIN; + } + + @AfterEach + void restore() { + Cache.cleanupWorlds = worlds; Cache.cleanupKeepOwners = keep; Cache.cleanupIdleMillis = idle; + } + + private ActiveVehicle vehicle(String id, World world, String owner) { + ActiveVehicle v = mock(ActiveVehicle.class, RETURNS_DEEP_STUBS); + when(v.getUUID()).thenReturn(id); + when(v.getEntity().getWorld()).thenReturn(world); + OwnerData data = new OwnerData(); + data.setOwner(owner); + when(v.getOwnerData()).thenReturn(data); + when(v.getSeatHandler().getPassengers()).thenReturn(new ArrayList<>()); + when(v.getEntity().getLocation()).thenReturn(new Location(world, 0, 64, 0)); + return v; + } + + @Test + void idleVehiclesOnCleanupWorldsAreDueAfterTheIdleTime() { + ActiveVehicle built = vehicle("built", tutorial, "player_Saint"); + ActiveVehicle unowned = vehicle("unowned", tutorial, "none"); + ActiveVehicle demo = vehicle("demo", tutorial, "char_demo"); + ActiveVehicle elsewhere = vehicle("elsewhere", main, "player_Saint"); + List all = List.of(built, unowned, demo, elsewhere, built); // built twice: two entities + + // First seen: the idle clock starts, nothing is due yet. + assertTrue(cleanup.due(all, 0).isEmpty()); + assertTrue(cleanup.due(all, 29 * MIN).isEmpty()); + // Kept owners and other worlds never come due; a vehicle registered twice is listed once. + assertEquals(List.of(built, unowned), cleanup.due(all, 30 * MIN)); + } + + @Test + void aPassengerRestartsTheIdleClock() { + ActiveVehicle car = vehicle("car", tutorial, "player_Saint"); + List all = List.of(car); + cleanup.due(all, 0); + when(car.getSeatHandler().getPassengers()).thenReturn(new ArrayList<>(List.of(mock(Player.class)))); + assertTrue(cleanup.due(all, 25 * MIN).isEmpty()); // occupied: clock now at 25 + when(car.getSeatHandler().getPassengers()).thenReturn(new ArrayList<>()); + assertTrue(cleanup.due(all, 50 * MIN).isEmpty()); + assertEquals(List.of(car), cleanup.due(all, 55 * MIN)); + } + + @Test + void aMovingVehicleWithoutPassengersIsNotIdle() { + // a train car behind an occupied locomotive, or a horse-drawn cart: no seat taken, but moving + ActiveVehicle car = vehicle("car", tutorial, "player_Saint"); + List all = List.of(car); + cleanup.due(all, 0); + when(car.getEntity().getLocation()).thenReturn(new Location(tutorial, 40, 64, 0)); + assertTrue(cleanup.due(all, 20 * MIN).isEmpty()); // moved: clock now at 20 + when(car.getEntity().getLocation()).thenReturn(new Location(tutorial, 40.5, 64, 0.4)); + assertTrue(cleanup.due(all, 45 * MIN).isEmpty()); // drifted under a block: not use + assertEquals(List.of(car), cleanup.due(all, 50 * MIN)); + when(car.getEntity().getLocation()).thenReturn(null); + assertEquals(List.of(car), cleanup.due(all, 51 * MIN)); // no location: judged by passengers only + } + + @Test + void unloadedVehiclesAreForgottenAndNoWorldsMeansNoCleanup() { + ActiveVehicle car = vehicle("car", tutorial, "player_Saint"); + cleanup.due(List.of(car), 0); + cleanup.due(List.of(), 10 * MIN); // unloaded: forgotten + assertTrue(cleanup.due(List.of(car), 40 * MIN).isEmpty()); // seen again: a fresh clock + assertEquals(List.of(car), cleanup.due(List.of(car), 70 * MIN)); + + Cache.cleanupWorlds = new HashSet<>(); + assertTrue(cleanup.due(List.of(car), 200 * MIN).isEmpty()); + Cache.cleanupWorlds = new HashSet<>(Set.of("tfmc_s2")); + assertTrue(cleanup.due(List.of(car), 201 * MIN).isEmpty()); // the clock was cleared too + } + + @Test + void vehiclesWithoutAnIdWorldOrOwnerAreHandledSafely() { + ActiveVehicle noId = vehicle(null, tutorial, "player_A"); + ActiveVehicle destroyed = vehicle("destroyed", tutorial, "player_A"); + when(destroyed.isDestroyed()).thenReturn(true); + ActiveVehicle noEntity = vehicle("noEntity", tutorial, "player_A"); + when(noEntity.getEntity()).thenReturn(null); + ActiveVehicle noWorld = vehicle("noWorld", null, "player_A"); + ActiveVehicle noOwnerData = vehicle("noOwnerData", tutorial, "x"); + when(noOwnerData.getOwnerData()).thenReturn(null); + ActiveVehicle nullOwner = vehicle("nullOwner", tutorial, null); + List all = List.of(noId, destroyed, noEntity, noWorld, noOwnerData, nullOwner); + cleanup.due(all, 0); + assertEquals(List.of(noOwnerData, nullOwner), cleanup.due(all, 30 * MIN)); + } +} diff --git a/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleManagerCoverageTest.java b/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleManagerCoverageTest.java index 187e870..ff2b28a 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleManagerCoverageTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleManagerCoverageTest.java @@ -456,10 +456,41 @@ void diagnosticDescriptionsHandleMissingAndInvalidEntities() { when(v.getEntity().isDead()).thenReturn(true); assertEquals("unknown location", VehicleManager.describeLocation(v)); } + @Test + void cleanupRemovesIdleVehiclesOnCleanupWorldsLikeAnAdminKill() throws Exception { + Set worlds = Cache.cleanupWorlds; long idle = Cache.cleanupIdleMillis; + try { + Cache.cleanupWorlds = new HashSet<>(Set.of("world")); Cache.cleanupIdleMillis = 60_000L; + ActiveVehicle v = vehicle("v"); register(v); + when(v.getSeatHandler().getPassengers()).thenReturn(new ArrayList<>()); + VehiclePersistence persistence = persistence(); + when(persistence.tombstone("v")).thenReturn(false); + manager.start(); + timers.get(3).run(); // first sight: the idle clock starts + verify(v, never()).remove(any(VehicleRemoveReason.class)); + assertEquals(1, manager.runCleanup(System.currentTimeMillis() + 120_000L)); + verify(v).remove(VehicleRemoveReason.ADMIN_KILL); + verify(persistence).tombstone("v"); + String described = VehicleManager.describeVehicle(v) + " at " + VehicleManager.describeLocation(v); + logger.verify(() -> VFLogger.log("Failed to tombstone " + described + " after cleanup")); + logger.verify(() -> VFLogger.info("Cleanup removed " + described + " (empty for 1 minutes)")); + // a later vehicle on its own clock; the tombstone succeeds this time (no failure line) + when(persistence.tombstone("w")).thenReturn(true); + manager.get().remove(v.getEntity()); + ActiveVehicle w = vehicle("w"); register(w); + when(w.getSeatHandler().getPassengers()).thenReturn(new ArrayList<>()); + assertEquals(0, manager.runCleanup(0)); + assertEquals(1, manager.runCleanup(120_000L)); + verify(w).remove(VehicleRemoveReason.ADMIN_KILL); + } finally { + Cache.cleanupWorlds = worlds; Cache.cleanupIdleMillis = idle; + } + } + @Test void timersRunIndependentTicksAndSkipUnavailablePersistence() throws Exception { ActiveVehicle v = vehicle("v"); register(v); Player player = player("Ryan"); - manager.start(); assertEquals(3, timers.size()); verify(manager.getSpawnManager()).start(); + manager.start(); assertEquals(4, timers.size()); verify(manager.getSpawnManager()).start(); timers.get(0).run(); timers.get(1).run(); timers.get(2).run(); verify(v).tick(); verify(v).slowTick(); doThrow(new IllegalStateException("broken tick")).when(v).tick(); From 00b3cf803965426ff0f9d56b9aa6333015fd66a4 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Wed, 11 Nov 2026 19:21:49 +0100 Subject: [PATCH 2/2] Cleanup: a reloaded vehicle starts a fresh clock; mounting or dismounting counts as use CodeRabbit on #93: track the loaded instance, not only its id, so a vehicle that unloads and loads again between checks is not removed on its first check back; mount and dismount mark the vehicle used immediately, so a short use between two checks restarts its idle clock. Co-Authored-By: Claude Opus 5.5 --- .../managers/VehicleCleanup.java | 60 +++++++++++++------ .../managers/VehicleManager.java | 3 +- .../managers/VehicleCleanupTest.java | 18 ++++++ 3 files changed, 62 insertions(+), 19 deletions(-) diff --git a/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleCleanup.java b/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleCleanup.java index 1cb46c0..e221eca 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleCleanup.java +++ b/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleCleanup.java @@ -20,22 +20,42 @@ /** * Finds idle vehicles to remove on the worlds in {@code cleanup.worlds} (config.yml), such as a * tutorial server's world. A vehicle is due once it has had no passenger and has not moved for - * {@code cleanup.idle-minutes}, counted from when the cleanup first saw it loaded or last saw it in - * use. Moving counts as use, so a train car behind an occupied locomotive, a horse-drawn cart or a - * towed vehicle is not idle. Owners in {@code cleanup.keep-owners} (stored owner values, e.g. - * {@code char_}) are never removed. With no worlds configured nothing is ever due. + * {@code cleanup.idle-minutes}, counted from when the cleanup first saw it loaded or it was last in + * use. Use is a passenger at a check, a mount or dismount at any time ({@link #markUsed}), or moving + * more than a block between checks, so a train car behind an occupied locomotive, a horse-drawn cart + * or a towed vehicle is not idle. A vehicle that unloads and loads again starts a fresh clock. + * Owners in {@code cleanup.keep-owners} (stored owner values, e.g. {@code char_}) are never + * removed. With no worlds configured nothing is ever due. */ public final class VehicleCleanup { - private final Map lastActive = new HashMap<>(); - private final Map lastPosition = new HashMap<>(); + /** One loaded vehicle: the instance, when it was last in use and where the last check saw it. */ + private static final class Track { + final ActiveVehicle vehicle; + long since; + double[] position; + + Track(ActiveVehicle vehicle, long since) { + this.vehicle = vehicle; + this.since = since; + } + } + + private final Map tracks = new HashMap<>(); + + /** Someone got in or out: the vehicle's idle clock starts again now. */ + public void markUsed(ActiveVehicle v, long now) { + Track track = v == null || v.getUUID() == null ? null : tracks.get(v.getUUID()); + if (track != null && track.vehicle == v) { + track.since = now; + } + } /** The loaded vehicles to remove now; also forgets vehicles that are no longer loaded. */ public List due(Collection vehicles, long now) { List due = new ArrayList<>(); if (Cache.cleanupWorlds.isEmpty()) { - lastActive.clear(); - lastPosition.clear(); + tracks.clear(); return due; } Set seen = new HashSet<>(); @@ -45,28 +65,32 @@ public List due(Collection vehicles, long now) { if (id == null || v.isDestroyed() || !seen.add(id) || !onCleanupWorld(v) || isKept(v)) { continue; } - boolean moved = moved(id, v.getEntity().getLocation()); - if (moved || !v.getSeatHandler().getPassengers().isEmpty()) { - lastActive.put(id, now); + Track track = tracks.get(id); + if (track == null || track.vehicle != v) { + // first sight, or loaded again since the last check: a fresh clock + track = new Track(v, now); + tracks.put(id, track); + moved(track, v.getEntity().getLocation()); continue; } - Long since = lastActive.putIfAbsent(id, now); - if (since != null && now - since >= Cache.cleanupIdleMillis) { + if (moved(track, v.getEntity().getLocation()) || !v.getSeatHandler().getPassengers().isEmpty()) { + track.since = now; + } else if (now - track.since >= Cache.cleanupIdleMillis) { due.add(v); } } - lastActive.keySet().retainAll(seen); - lastPosition.keySet().retainAll(seen); + tracks.keySet().retainAll(seen); return due; } - /** Whether the vehicle is more than a block from where the last check saw it. */ - private boolean moved(String id, Location at) { + /** Whether the vehicle is more than a block from where the last check saw it; remembers where it is. */ + private static boolean moved(Track track, Location at) { if (at == null) { return false; } double[] now = {at.getX(), at.getY(), at.getZ()}; - double[] before = lastPosition.put(id, now); + double[] before = track.position; + track.position = now; return before != null && Math.abs(now[0] - before[0]) + Math.abs(now[1] - before[1]) + Math.abs(now[2] - before[2]) > 1.0; } diff --git a/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java b/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java index 4df885f..11fb432 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java +++ b/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java @@ -437,7 +437,7 @@ public boolean isPassenger(Entity e) { } public void dismount(Player p) { - if(activeVehicle.containsKey(p)) activeVehicle.remove(p); + cleanup.markUsed(activeVehicle.remove(p), System.currentTimeMillis()); if (p != null) { packetSneak.remove(p.getUniqueId()); mountedLeftClickAt.remove(p.getUniqueId()); @@ -1027,6 +1027,7 @@ private boolean takeSeat(Player p, ActiveVehicle v, Seat seat) { } if (tempVehicle.containsKey(p)) tempVehicle.remove(p); if (!activeVehicle.containsKey(p)) activeVehicle.put(p, v); + cleanup.markUsed(v, System.currentTimeMillis()); return true; } diff --git a/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleCleanupTest.java b/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleCleanupTest.java index 94ec6c5..06a0a7e 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleCleanupTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleCleanupTest.java @@ -93,6 +93,24 @@ void aMovingVehicleWithoutPassengersIsNotIdle() { assertEquals(List.of(car), cleanup.due(all, 51 * MIN)); // no location: judged by passengers only } + @Test + void aReloadedVehicleStartsAFreshClockAndGettingInOrOutCountsAsUse() { + ActiveVehicle car = vehicle("car", tutorial, "player_Saint"); + cleanup.due(List.of(car), 0); + // unloaded and loaded again between two checks: a new instance with the same id + ActiveVehicle reloaded = vehicle("car", tutorial, "player_Saint"); + assertTrue(cleanup.due(List.of(reloaded), 29 * MIN).isEmpty()); + assertTrue(cleanup.due(List.of(reloaded), 58 * MIN).isEmpty()); + cleanup.markUsed(car, 58 * MIN); // the old instance: ignored + // someone got in and out between checks, at minute 58 + cleanup.markUsed(reloaded, 58 * MIN); + assertTrue(cleanup.due(List.of(reloaded), 60 * MIN).isEmpty()); + assertEquals(List.of(reloaded), cleanup.due(List.of(reloaded), 88 * MIN)); + cleanup.markUsed(null, 0); // a player who was in no vehicle + cleanup.markUsed(vehicle(null, tutorial, "x"), 0); + cleanup.markUsed(vehicle("never-seen", tutorial, "x"), 0); + } + @Test void unloadedVehiclesAreForgottenAndNoWorldsMeansNoCleanup() { ActiveVehicle car = vehicle("car", tutorial, "player_Saint");