From d409553000f2be7b47b945c9276118f48f7d563d Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Fri, 13 Nov 2026 14:06:01 +0100 Subject: [PATCH 1/4] perf: let an idle terrain-following vehicle stop reading the ground every tick A vehicle whose ground state follows the terrain (the Small Car) is never parked: its step traces a ray under every wheel and teleports the vehicle onto the result, every tick, also when it stands empty on the spot the step keeps returning. On Main one Small Car cost about 0.1 ms per tick. TerrainRest counts the steps that leave an idle vehicle where it was with the same tilt. After 20 in a row the step is left out until the vehicle is found anywhere else, is no longer idle, or 20 ticks have passed, so a block changed under a vehicle nothing moves is seen within a second. The left-out tick still zeroes the velocity and stops the drive animations, as the step did. Co-Authored-By: Claude Fable 5.2 --- .../vehicles/controller/TerrainRest.java | 95 ++++++++++++++ .../controller/VehicleMovementController.java | 48 ++++++- .../vehicles/controller/TerrainRestTest.java | 124 ++++++++++++++++++ ...VehicleMovementControllerCoverageTest.java | 124 ++++++++++++++++++ 4 files changed, 385 insertions(+), 6 deletions(-) create mode 100644 src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/TerrainRest.java create mode 100644 src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/TerrainRestTest.java diff --git a/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/TerrainRest.java b/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/TerrainRest.java new file mode 100644 index 0000000..e6ea3bd --- /dev/null +++ b/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/TerrainRest.java @@ -0,0 +1,95 @@ +package net.tfminecraft.vehicleframework.vehicles.controller; + +/** + * Lets a terrain-following vehicle that stands idle stop re-reading the ground every tick. + * The ground step traces rays under every wheel and teleports the vehicle onto the result, even + * when the result is the spot it already stands on. Once {@link #SETTLE_STEPS} steps in a row + * leave the vehicle exactly where it was, with the same tilt, the step is left out until the + * vehicle is found anywhere else (a push, a fall, a piston) or is no longer idle. A full step + * still runs every {@link #RECHECK_TICKS} ticks, so a block placed or removed under a vehicle + * that nothing moves is seen within that time. + */ +final class TerrainRest { + /** Still steps before the vehicle counts as settled; also lets the tilt smoothing finish. */ + static final int SETTLE_STEPS = 20; + /** While settled, one tick in this many runs the full ground step. */ + static final int RECHECK_TICKS = 20; + private static final double POSITION_EPSILON = 1e-4; + private static final double TILT_EPSILON = 1e-3; + + private int still; + private int skipped; + private double x; + private double y; + private double z; + private boolean tilted; + private float pitch; + private float roll; + + /** Whether this tick's ground step can be left out for a vehicle now standing at this position. */ + boolean skip(double currentX, double currentY, double currentZ) { + if (still < SETTLE_STEPS) { + return false; + } + if (!at(currentX, currentY, currentZ)) { + reset(); + return false; + } + if (skipped >= RECHECK_TICKS - 1) { + skipped = 0; + return false; + } + skipped++; + return true; + } + + /** + * Records one full ground step: where the vehicle stood before it, where the step put it, whether + * the step gave it any velocity, and the tilt it asked for ({@code hasTilt} false for none). + */ + void observe( + double beforeX, double beforeY, double beforeZ, + double destX, double destY, double destZ, + boolean moving, boolean hasTilt, float pitchDeg, float rollDeg) { + boolean stayed = !moving + && near(beforeX, destX) && near(beforeY, destY) && near(beforeZ, destZ); + boolean sameAsLast = still == 0 + || (at(destX, destY, destZ) && tilted == hasTilt + && (!hasTilt || (Math.abs(pitch - pitchDeg) <= TILT_EPSILON + && Math.abs(roll - rollDeg) <= TILT_EPSILON))); + if (!stayed) { + still = 0; + } else if (sameAsLast) { + still = Math.min(SETTLE_STEPS, still + 1); + } else { + still = 1; + } + if (still < SETTLE_STEPS) { + skipped = 0; + } + x = destX; + y = destY; + z = destZ; + tilted = hasTilt; + pitch = pitchDeg; + roll = rollDeg; + } + + /** Forgets the settled spot: the next {@link #SETTLE_STEPS} steps all run. */ + void reset() { + still = 0; + skipped = 0; + } + + boolean settled() { + return still >= SETTLE_STEPS; + } + + private boolean at(double px, double py, double pz) { + return near(px, x) && near(py, y) && near(pz, z); + } + + private static boolean near(double a, double b) { + return Math.abs(a - b) <= POSITION_EPSILON; + } +} diff --git a/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementController.java b/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementController.java index 9656132..98351a8 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementController.java +++ b/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementController.java @@ -1,5 +1,6 @@ package net.tfminecraft.vehicleframework.vehicles.controller; +import org.bukkit.Location; import org.bukkit.entity.Entity; import org.bukkit.entity.LivingEntity; import org.bukkit.entity.Player; @@ -8,6 +9,7 @@ import net.tfminecraft.vehicleframework.VehicleFramework; import net.tfminecraft.vehicleframework.bones.BoneRotator; import net.tfminecraft.vehicleframework.bones.VectorBone; +import net.tfminecraft.vehicleframework.cache.Cache; import net.tfminecraft.vehicleframework.enums.Animation; import net.tfminecraft.vehicleframework.enums.Component; import net.tfminecraft.vehicleframework.enums.Direction; @@ -45,6 +47,8 @@ public class VehicleMovementController implements MovementInterface{ protected FloatController floatController = new FloatController(); protected ThrottleController throttleController = new ThrottleController(); protected RotateController rotateController = new RotateController(); + /** When this state's ground step may be left out for a vehicle standing idle. */ + private final TerrainRest terrainRest = new TerrainRest(); @@ -256,8 +260,24 @@ private void move() { * The result of its MOVE step then depends on neither the heading bone nor the blocks around * it, so {@link #moveParked} can apply that result directly. Checked every tick: a driver, * throttle, a state change (water, open air) or a tow brings back the full step that tick. + * A state that follows the terrain is never parked, because its step does read the blocks + * under it; {@link TerrainRest} thins that step out instead once the vehicle has settled. */ boolean isParked() { + return standsIdle() && !followsTerrain(); + } + + /** Whether this controller's state places the vehicle on the ground it reads every step. */ + private boolean followsTerrain() { + if (state.isDefault()) { + return false; + } + TerrainFollowConfig follow = state.getTerrainFollow(); + return follow != null && follow.isEnabled(); + } + + /** {@link #isParked} without the terrain rule: nothing aboard, driving, towing or burning. */ + private boolean standsIdle() { // The full step reads the vehicle's current state, which is this controller's. if (v.getCurrentState() != state || state.getType() != State.GROUND) { return false; @@ -265,12 +285,6 @@ boolean isParked() { if (v.isTrain() || v.isDestroyed() || v.shouldFloat()) { return false; } - if (!state.isDefault()) { - TerrainFollowConfig follow = state.getTerrainFollow(); - if (follow != null && follow.isEnabled()) { - return false; - } - } if (v.hasComponent(Component.HARNESS) || !(v.hasComponent(Component.ENGINE) || v.hasComponent(Component.GEARED_ENGINE))) { return false; @@ -346,10 +360,32 @@ private boolean applyTerrainFollow(Direction dir) { if (v.isTrain()) { return false; } + // The debug view draws the probes from inside the step, so it keeps every step. + boolean idle = !Cache.terrainFollowDebug && standsIdle(); + Location before = idle ? e.getLocation() : null; + if (idle && terrainRest.skip(before.getX(), before.getY(), before.getZ())) { + // Settled and found where the last step left it: that step would put it here again. + Vector none = new Vector(); + e.setVelocity(none); + propagate(none, dir); + animateMove(dir); + return true; + } TerrainFollowEngine.Result result = TerrainFollowEngine.step(v, vector, dir, baseController, follow); if (result == null) { + terrainRest.reset(); return true; } + if (idle) { + TerrainFollowMath.Tilt tilt = result.tilt; + terrainRest.observe( + before.getX(), before.getY(), before.getZ(), + result.location.getX(), result.location.getY(), result.location.getZ(), + result.velocity.lengthSquared() > 0, + tilt != null, tilt == null ? 0 : tilt.pitchDeg, tilt == null ? 0 : tilt.rollDeg); + } else { + terrainRest.reset(); + } e.teleport(result.location); e.setVelocity(result.velocity); if (v.hasComponent(Component.HARNESS)) { diff --git a/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/TerrainRestTest.java b/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/TerrainRestTest.java new file mode 100644 index 0000000..6e28301 --- /dev/null +++ b/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/TerrainRestTest.java @@ -0,0 +1,124 @@ +package net.tfminecraft.vehicleframework.vehicles.controller; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +class TerrainRestTest { + private final TerrainRest rest = new TerrainRest(); + + /** One full step that leaves the vehicle where it stood, level. */ + private void stillStep(double x, double y, double z) { + rest.observe(x, y, z, x, y, z, false, true, 0f, 0f); + } + + private void settleAt(double x, double y, double z) { + for (int i = 0; i < TerrainRest.SETTLE_STEPS; i++) { + assertFalse(rest.skip(x, y, z), "step " + i + " must still run"); + stillStep(x, y, z); + } + assertTrue(rest.settled()); + } + + @Test + void everyStepRunsUntilEnoughOfThemInARowChangeNothing() { + for (int i = 0; i < TerrainRest.SETTLE_STEPS - 1; i++) { + stillStep(8, 64, 8); + assertFalse(rest.settled()); + assertFalse(rest.skip(8, 64, 8)); + } + stillStep(8, 64, 8); + assertTrue(rest.settled()); + assertTrue(rest.skip(8, 64, 8)); + } + + @Test + void aSettledVehicleRunsOneFullStepInEveryRecheckWindow() { + settleAt(8, 64, 8); + for (int window = 0; window < 3; window++) { + int skipped = 0; + while (rest.skip(8, 64, 8)) { + skipped++; + } + assertEquals(TerrainRest.RECHECK_TICKS - 1, skipped); + stillStep(8, 64, 8); + assertTrue(rest.settled(), "an unchanged recheck keeps the vehicle settled"); + } + } + + @Test + void aVehicleFoundAnywhereElseRunsEveryStepAgain() { + settleAt(8, 64, 8); + assertTrue(rest.skip(8, 64, 8)); + assertFalse(rest.skip(8, 63.9, 8), "it fell"); + assertFalse(rest.settled()); + assertFalse(rest.skip(8, 64, 8), "back on the old spot is not settled either"); + } + + @Test + void aStepThatMovesOrPushesTheVehicleStartsTheCountAgain() { + settleAt(8, 64, 8); + rest.observe(8, 64, 8, 8, 63.5, 8, false, true, 0f, 0f); + assertFalse(rest.settled(), "the step dropped it half a block"); + settleAt(8, 63.5, 8); + rest.observe(8, 63.5, 8, 8, 63.5, 8, true, true, 0f, 0f); + assertFalse(rest.settled(), "the step gave it velocity"); + } + + @Test + void aChangingTiltOrALostTiltKeepsTheStepRunningUntilItHolds() { + for (int i = 0; i < TerrainRest.SETTLE_STEPS * 2; i++) { + rest.observe(8, 64, 8, 8, 64, 8, false, true, i * .5f, 0f); + assertFalse(rest.settled(), "the tilt is still turning"); + } + settleAt(8, 64, 8); + rest.observe(8, 64, 8, 8, 64, 8, false, false, 0f, 0f); + assertFalse(rest.settled(), "a wheel left the ground, so the step gave no tilt"); + for (int i = 0; i < TerrainRest.SETTLE_STEPS - 1; i++) { + rest.observe(8, 64, 8, 8, 64, 8, false, false, 0f, 0f); + } + assertTrue(rest.settled(), "no tilt at all also settles once it holds"); + } + + @Test + void aStillStepSomewhereNewCountsAsTheFirstOfItsRun() { + settleAt(8, 64, 8); + stillStep(20, 70, 20); + assertFalse(rest.settled()); + for (int i = 0; i < TerrainRest.SETTLE_STEPS - 2; i++) { + stillStep(20, 70, 20); + assertFalse(rest.settled()); + } + stillStep(20, 70, 20); + assertTrue(rest.settled()); + } + + @Test + void everyAxisAndBothTiltAnglesCount() { + double[][] elsewhere = {{8.5, 64, 8}, {8, 64.5, 8}, {8, 64, 8.5}}; + for (double[] spot : elsewhere) { + settleAt(8, 64, 8); + assertFalse(rest.skip(spot[0], spot[1], spot[2]), "found off its spot on one axis"); + settleAt(8, 64, 8); + rest.observe(8, 64, 8, spot[0], spot[1], spot[2], false, true, 0f, 0f); + assertFalse(rest.settled(), "the step moved it along one axis"); + rest.reset(); + } + settleAt(8, 64, 8); + rest.observe(8, 64, 8, 8, 64, 8, false, true, 0f, 2f); + assertFalse(rest.settled(), "the roll changed"); + settleAt(8, 64, 8); + rest.observe(8, 64, 8, 8, 64, 8, false, true, 2f, 0f); + assertFalse(rest.settled(), "the pitch changed"); + } + + @Test + void resetForgetsTheSpot() { + settleAt(8, 64, 8); + rest.reset(); + assertFalse(rest.settled()); + assertFalse(rest.skip(8, 64, 8)); + } +} diff --git a/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java b/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java index 698c750..fad42de 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java @@ -19,6 +19,7 @@ import org.mockito.*; import net.tfminecraft.vehicleframework.bones.BoneRotator; import net.tfminecraft.vehicleframework.bones.VectorBone; +import net.tfminecraft.vehicleframework.cache.Cache; import net.tfminecraft.vehicleframework.enums.Animation; import net.tfminecraft.vehicleframework.enums.Component; import net.tfminecraft.vehicleframework.enums.Direction; @@ -570,6 +571,129 @@ void invalidTerrainEntityProducesNoFallbackVelocity() { verify(vehicle, never()).animate(any()); } + /** An empty engine vehicle with no throttle whose ground step keeps answering "stay here, level". */ + private void idleOnTerrain() { + component(Component.ENGINE, Engine.class); + park(); + follow(); + terrainResult(new TerrainFollowEngine.Result(location.clone(), new Vector(), new TerrainFollowMath.Tilt(0, 0))); + } + + private void moveTicks(int ticks) { + for (int i = 0; i < ticks; i++) controller.input(player, Input.MOVE); + } + + private void verifySteps(int count) { + terrain.verify(() -> TerrainFollowEngine.step(eq(vehicle), any(VectorBone.class), any(Direction.class), + any(BaseController.class), any(TerrainFollowConfig.class)), times(count)); + } + + @Test + void anIdleTerrainVehicleStopsReadingTheGroundOnceSettledAndRechecksOnceAWindow() { + idleOnTerrain(); + assertFalse(controller.isParked(), "a terrain state is never parked"); + moveTicks(TerrainRest.SETTLE_STEPS); + verifySteps(TerrainRest.SETTLE_STEPS); + verify(entity, times(TerrainRest.SETTLE_STEPS)).teleport(any(Location.class)); + clearInvocations(entity, vehicle, rotator); + velocity = new Vector(.2, -.3, .1); + + moveTicks(TerrainRest.RECHECK_TICKS - 1); + verifySteps(TerrainRest.SETTLE_STEPS); + verify(entity, never()).teleport(any(Location.class)); + verify(rotator, never()).rotateToTarget(anyFloat(), anyFloat(), anyFloat(), anyFloat(), anyBoolean(), anyBoolean(), anyBoolean()); + // What the left-out step would still have done: no velocity, drive animations stopped. + assertEquals(new Vector(), velocity); + verify(entity, times(TerrainRest.RECHECK_TICKS - 1)).setVelocity(new Vector()); + verify(vehicle, times(TerrainRest.RECHECK_TICKS - 1)).stopAnimation(Animation.FORWARD); + verify(vehicle, times(TerrainRest.RECHECK_TICKS - 1)).stopAnimation(Animation.BACKWARD); + + moveTicks(1); + verifySteps(TerrainRest.SETTLE_STEPS + 1); + verify(entity).teleport(any(Location.class)); + moveTicks(TerrainRest.RECHECK_TICKS - 1); + verifySteps(TerrainRest.SETTLE_STEPS + 1); + } + + @Test + void anIdleTerrainVehicleSettlesWithoutATiltButNotWhileTheStepGivesItVelocity() { + component(Component.ENGINE, Engine.class); + park(); + follow(); + terrainResult(new TerrainFollowEngine.Result(location.clone(), new Vector(), null)); + moveTicks(TerrainRest.SETTLE_STEPS + 5); + verifySteps(TerrainRest.SETTLE_STEPS); + // Same spot, but the step still pushes it: it has not come to rest. + location = new Location(world, 30, 64, 30); + terrainResult(new TerrainFollowEngine.Result(location.clone(), new Vector(.1, 0, 0), null)); + moveTicks(TerrainRest.SETTLE_STEPS * 2); + verifySteps(TerrainRest.SETTLE_STEPS * 3); + } + + @Test + void aSettledTerrainVehicleFoundSomewhereElseReadsTheGroundAgainAtOnce() { + idleOnTerrain(); + moveTicks(TerrainRest.SETTLE_STEPS + 3); + verifySteps(TerrainRest.SETTLE_STEPS); + // Something other than the ground step moved it: it fell two blocks. + location = new Location(world, 8, 62, 8); + terrainResult(new TerrainFollowEngine.Result(location.clone(), new Vector(), new TerrainFollowMath.Tilt(0, 0))); + moveTicks(TerrainRest.SETTLE_STEPS); + verifySteps(TerrainRest.SETTLE_STEPS * 2); + moveTicks(1); + verifySteps(TerrainRest.SETTLE_STEPS * 2); + } + + @Test + void anyoneAboardThrottleFireOrTheDebugViewKeepsEveryGroundStep() { + idleOnTerrain(); + moveTicks(TerrainRest.SETTLE_STEPS + 1); + verifySteps(TerrainRest.SETTLE_STEPS); + int steps = TerrainRest.SETTLE_STEPS; + + when(vehicle.getSeatHandler().hasPassengers()).thenReturn(true); + moveTicks(5); + verifySteps(steps += 5); + when(vehicle.getSeatHandler().hasPassengers()).thenReturn(false); + // Leaving again does not bring the old rest back: it settles from the start. + moveTicks(TerrainRest.SETTLE_STEPS); + verifySteps(steps += TerrainRest.SETTLE_STEPS); + moveTicks(1); + verifySteps(steps); + + when(vehicle.isOnFire()).thenReturn(true); + moveTicks(3); + verifySteps(steps += 3); + when(vehicle.isOnFire()).thenReturn(false); + moveTicks(TerrainRest.SETTLE_STEPS + 1); + verifySteps(steps += TerrainRest.SETTLE_STEPS); + + Cache.terrainFollowDebug = true; + try { + moveTicks(4); + verifySteps(steps += 4); + } finally { + Cache.terrainFollowDebug = false; + } + } + + @Test + void aGroundStepThatStillMovesOrTurnsTheVehicleNeverLetsItRest() { + component(Component.ENGINE, Engine.class); + park(); + follow(); + // Sliding: each step answers one spot further along. + terrain.when(() -> TerrainFollowEngine.step(eq(vehicle), any(VectorBone.class), any(Direction.class), + any(BaseController.class), any(TerrainFollowConfig.class))).thenAnswer(call -> + new TerrainFollowEngine.Result(location.clone().add(0, -.05, 0), new Vector(), new TerrainFollowMath.Tilt(0, 0))); + moveTicks(TerrainRest.SETTLE_STEPS * 2); + verifySteps(TerrainRest.SETTLE_STEPS * 2); + // An invalid entity gives no result and forgets any rest. + terrainResult(null); + moveTicks(2); + verifySteps(TerrainRest.SETTLE_STEPS * 2 + 2); + } + @Test void balloonClimbAndDescentPreserveHorizontalVelocityAndSetDelta() { Balloon balloon = component(Component.BALLOON, Balloon.class); From 2190fec99d0a70417662049d929b91bd6267a8f9 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Fri, 13 Nov 2026 14:42:13 +0100 Subject: [PATCH 2/4] test: cover throttle, speed and an occupied tow waking a resting terrain vehicle Review of #94: the test named throttle but only exercised passengers, fire and the debug view. Co-Authored-By: Claude Fable 5.2 --- ...VehicleMovementControllerCoverageTest.java | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java b/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java index fad42de..a298dd9 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java @@ -645,7 +645,7 @@ void aSettledTerrainVehicleFoundSomewhereElseReadsTheGroundAgainAtOnce() { } @Test - void anyoneAboardThrottleFireOrTheDebugViewKeepsEveryGroundStep() { + void anyoneAboardThrottleSpeedATowFireOrTheDebugViewKeepsEveryGroundStep() { idleOnTerrain(); moveTicks(TerrainRest.SETTLE_STEPS + 1); verifySteps(TerrainRest.SETTLE_STEPS); @@ -668,6 +668,31 @@ void anyoneAboardThrottleFireOrTheDebugViewKeepsEveryGroundStep() { moveTicks(TerrainRest.SETTLE_STEPS + 1); verifySteps(steps += TerrainRest.SETTLE_STEPS); + when(vehicle.getThrottle().getCurrent()).thenReturn(5); + moveTicks(3); + verifySteps(steps += 3); + when(vehicle.getThrottle().getCurrent()).thenReturn(0); + moveTicks(TerrainRest.SETTLE_STEPS + 1); + verifySteps(steps += TerrainRest.SETTLE_STEPS); + + when(vehicle.getAccessPanel().getSpeed()).thenReturn(.05); + moveTicks(3); + verifySteps(steps += 3); + when(vehicle.getAccessPanel().getSpeed()).thenReturn(0d); + moveTicks(TerrainRest.SETTLE_STEPS + 1); + verifySteps(steps += TerrainRest.SETTLE_STEPS); + + when(vehicle.hasTowHandler()).thenReturn(true); + moveTicks(2); + verifySteps(steps); + // An empty hitch changes nothing; something on it does. + when(vehicle.getTowHandler().isOccupied()).thenReturn(true); + moveTicks(3); + verifySteps(steps += 3); + when(vehicle.getTowHandler().isOccupied()).thenReturn(false); + moveTicks(TerrainRest.SETTLE_STEPS + 1); + verifySteps(steps += TerrainRest.SETTLE_STEPS); + Cache.terrainFollowDebug = true; try { moveTicks(4); From e75fcecd5f13c231f05543c1e5251780022139f4 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Fri, 13 Nov 2026 17:07:46 +0100 Subject: [PATCH 3/4] fix: forget a terrain rest when the vehicle's state swaps or a tow resets its bone CodeRabbit on #94: TowHandler.attach resets the towed vehicle's rotator to level. A vehicle resting on a slope only runs one ground step a second, so its tilt would come back one smoothing step a second. The same reset answers the earlier review note about a rest surviving a state swap: StateHandler.swapState now forgets every state's rest. Co-Authored-By: Claude Fable 5.2 --- .../controller/VehicleMovementController.java | 16 +++++++++++++ .../vehicles/handlers/StateHandler.java | 9 +++++++ .../vehicles/handlers/TowHandler.java | 3 +++ .../VehicleControllersCoverageTest.java | 24 +++++++++++++++++++ ...VehicleMovementControllerCoverageTest.java | 14 +++++++++++ .../handlers/VehicleHandlersCoverageTest.java | 2 +- 6 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementController.java b/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementController.java index 98351a8..42a1692 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementController.java +++ b/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementController.java @@ -267,6 +267,22 @@ boolean isParked() { return standsIdle() && !followsTerrain(); } + /** + * Forgets a settled terrain rest, so the ground step runs every tick again until the vehicle + * has settled anew. For whatever changes the vehicle without going through that step: its + * state swapped (the rest belongs to the ground it stood on in this state), or its bone reset + * (the step's tilt smoothing has to bring it back, and a resting vehicle only steps once a + * second). + */ + public void resetTerrainRest() { + terrainRest.reset(); + } + + /** For tests: this controller's rest. */ + TerrainRest terrainRest() { + return terrainRest; + } + /** Whether this controller's state places the vehicle on the ground it reads every step. */ private boolean followsTerrain() { if (state.isDefault()) { diff --git a/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/StateHandler.java b/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/StateHandler.java index ba052f7..8aa4b54 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/StateHandler.java +++ b/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/StateHandler.java @@ -185,8 +185,17 @@ private void swapState(State s, String reason) { to, reason, next != null && next.isDefault())); + // A rest settled in the state it leaves, or in the one it returns to, is about other ground. + resetTerrainRest(); setState(s); } + + /** Every state's ground step runs each tick again; see {@link VehicleMovementController#resetTerrainRest}. */ + public void resetTerrainRest() { + for (VehicleState each : states.values()) { + each.getMoveControls().resetTerrainRest(); + } + } //Currentstate getters and methods diff --git a/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TowHandler.java b/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TowHandler.java index 7aa44b5..50bdb14 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TowHandler.java +++ b/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TowHandler.java @@ -71,6 +71,9 @@ public void attach(ActiveVehicle target) { if(towPoint.isOccupied()) return; towPoint.mount(target, v); target.getBehaviourHandler().getRotator().reset(); + // Its bone is level again: a vehicle resting on a slope must go back to stepping every tick, + // or the tilt would only come back one smoothing step a second. + target.getStateHandler().resetTerrainRest(); } public void unattach() { diff --git a/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleControllersCoverageTest.java b/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleControllersCoverageTest.java index 8220c92..69f0934 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleControllersCoverageTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleControllersCoverageTest.java @@ -252,6 +252,30 @@ class VehicleControllersCoverageTest { handler.tick(); assertEquals(State.GROUND, handler.getCurrentState().getType()); } + @Test void aStateSwapForgetsEveryStatesTerrainRest() { + Water f = new Water(); YamlConfiguration config = yaml(); config.createSection("floating"); + StateHandler handler = new StateHandler(f.vehicle, new StateHandler(config)); + f.grid.fill = p -> p.y < 64 ? Material.STONE : Material.AIR; + handler.tick(); assertEquals(State.GROUND, handler.getCurrentState().getType()); + TerrainRest ground = handler.getVehicleState(State.GROUND).getMoveControls().terrainRest(); + TerrainRest floating = handler.getVehicleState(State.FLOATING).getMoveControls().terrainRest(); + for (TerrainRest rest : List.of(ground, floating)) { + for (int i = 0; i < TerrainRest.SETTLE_STEPS; i++) rest.observe(8, 64, 8, 8, 64, 8, false, false, 0f, 0f); + assertTrue(rest.settled()); + } + // Staying in the same state leaves a rest alone. + handler.tick(); assertTrue(ground.settled()); assertTrue(floating.settled()); + // Water rises over it: whatever either state had settled on is no longer the ground it stands on. + f.grid.fill = p -> p.y <= 66 ? Material.WATER : Material.AIR; + handler.tick(); assertEquals(State.FLOATING, handler.getCurrentState().getType()); + assertFalse(ground.settled()); assertFalse(floating.settled()); + // The same when it comes back to the very spot it rested on. + for (int i = 0; i < TerrainRest.SETTLE_STEPS; i++) ground.observe(8, 64, 8, 8, 64, 8, false, false, 0f, 0f); + f.grid.fill = p -> p.y < 64 ? Material.STONE : Material.AIR; + handler.tick(); assertEquals(State.GROUND, handler.getCurrentState().getType()); + assertFalse(ground.settled()); assertFalse(ground.skip(8, 64, 8)); + } + @Test void stateTicksOnlyReadTheBlocksTheirConfiguredStatesNeed() { Water f = new Water(); f.grid.fill = p -> p.y <= 66 ? Material.WATER : Material.AIR; diff --git a/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java b/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java index a298dd9..5eb8661 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java @@ -630,6 +630,20 @@ void anIdleTerrainVehicleSettlesWithoutATiltButNotWhileTheStepGivesItVelocity() verifySteps(TerrainRest.SETTLE_STEPS * 3); } + @Test + void forgettingTheRestBringsEveryGroundStepBackUntilItSettlesAgain() { + idleOnTerrain(); + moveTicks(TerrainRest.SETTLE_STEPS + 2); + verifySteps(TerrainRest.SETTLE_STEPS); + // Its bone was reset, or its state swapped: nothing about the vehicle's place has changed. + controller.resetTerrainRest(); + moveTicks(TerrainRest.SETTLE_STEPS); + verifySteps(TerrainRest.SETTLE_STEPS * 2); + verify(rotator, times(TerrainRest.SETTLE_STEPS * 2)).rotateToTarget(anyFloat(), anyFloat(), anyFloat(), anyFloat(), anyBoolean(), anyBoolean(), anyBoolean()); + moveTicks(1); + verifySteps(TerrainRest.SETTLE_STEPS * 2); + } + @Test void aSettledTerrainVehicleFoundSomewhereElseReadsTheGroundAgainAtOnce() { idleOnTerrain(); diff --git a/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/VehicleHandlersCoverageTest.java b/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/VehicleHandlersCoverageTest.java index 6adcdc8..0b41565 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/VehicleHandlersCoverageTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/VehicleHandlersCoverageTest.java @@ -302,7 +302,7 @@ void towingMaintainsAttachmentPoseReversalAndVehicleControls(boolean reversed) { YamlConfiguration config = yaml(); config.set("bone", "tow"); config.set("reversed", reversed); TowHandler handler = new TowHandler(vehicle, new TowHandler(config)); assertFalse(handler.isOccupied()); handler.tick(); handler.animate(Direction.FORWARD); handler.attach(trailer); handler.attach(vehicle); assertTrue(handler.isOccupied()); assertSame(trailer, handler.getTowPoint().getMountedVehicle()); - verify(trailer).setParent(vehicle); verify(trailer.getBehaviourHandler().getRotator()).reset(); handler.tick(); + verify(trailer).setParent(vehicle); verify(trailer.getBehaviourHandler().getRotator()).reset(); verify(trailer.getStateHandler()).resetTerrainRest(); handler.tick(); ArgumentCaptor target = ArgumentCaptor.forClass(Location.class); verify(trailer.getEntity()).teleport(target.capture()); assertEquals(reversed ? 90 : -90, target.getValue().getYaw()); assertEquals(handler.getTowLocation().toVector(), target.getValue().toVector()); handler.animate(Direction.FORWARD); verify(trailer.getMoveControls()).animateMove(reversed ? Direction.BACKWARD : Direction.FORWARD); From 47ee5620c76e96f7f40edb829720422a98fe4f31 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Fri, 13 Nov 2026 17:41:12 +0100 Subject: [PATCH 4/4] chore: answer the dual review's nits on the terrain rest - isParked() checks the terrain rule first, so a terrain vehicle's idle check runs once a tick (same result). - Comments: what the ground engine log shows for a resting vehicle, and what the settle count does for the tilt. - Test: a recheck that finds the ground changed brings every step back. Co-Authored-By: Claude Fable 5.2 --- .../vehicles/controller/TerrainRest.java | 2 +- .../controller/VehicleMovementController.java | 7 +++++-- ...VehicleMovementControllerCoverageTest.java | 21 +++++++++++++++++++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/TerrainRest.java b/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/TerrainRest.java index e6ea3bd..48b3ea7 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/TerrainRest.java +++ b/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/TerrainRest.java @@ -10,7 +10,7 @@ * that nothing moves is seen within that time. */ final class TerrainRest { - /** Still steps before the vehicle counts as settled; also lets the tilt smoothing finish. */ + /** Still steps before the vehicle counts as settled; the tilt smoothing has all but finished by then, and the rechecks complete it. */ static final int SETTLE_STEPS = 20; /** While settled, one tick in this many runs the full ground step. */ static final int RECHECK_TICKS = 20; diff --git a/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementController.java b/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementController.java index 42a1692..8f5cd52 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementController.java +++ b/src/main/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementController.java @@ -264,7 +264,9 @@ private void move() { * under it; {@link TerrainRest} thins that step out instead once the vehicle has settled. */ boolean isParked() { - return standsIdle() && !followsTerrain(); + // The terrain rule first: a terrain vehicle is never parked, and its idle check runs once, + // in the ground step. + return !followsTerrain() && standsIdle(); } /** @@ -376,7 +378,8 @@ private boolean applyTerrainFollow(Direction dir) { if (v.isTrain()) { return false; } - // The debug view draws the probes from inside the step, so it keeps every step. + // The debug view draws the probes from inside the step, so it keeps every step. The ground + // engine log does not: with it on, a resting vehicle logs one step a second, not twenty. boolean idle = !Cache.terrainFollowDebug && standsIdle(); Location before = idle ? e.getLocation() : null; if (idle && terrainRest.skip(before.getX(), before.getY(), before.getZ())) { diff --git a/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java b/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java index 5eb8661..bf6218f 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/vehicles/controller/VehicleMovementControllerCoverageTest.java @@ -630,6 +630,27 @@ void anIdleTerrainVehicleSettlesWithoutATiltButNotWhileTheStepGivesItVelocity() verifySteps(TerrainRest.SETTLE_STEPS * 3); } + @Test + void aRecheckThatFindsTheGroundChangedBringsEveryGroundStepBack() { + idleOnTerrain(); + moveTicks(TerrainRest.SETTLE_STEPS); + verifySteps(TerrainRest.SETTLE_STEPS); + // A block went from under it. Nothing moved the vehicle, so only the recheck can notice. + Location lower = new Location(world, 8, 63, 8); + terrainResult(new TerrainFollowEngine.Result(lower, new Vector(), new TerrainFollowMath.Tilt(0, 0))); + moveTicks(TerrainRest.RECHECK_TICKS - 1); + verifySteps(TerrainRest.SETTLE_STEPS); + assertEquals(new Location(world, 8, 64, 8), location); + moveTicks(1); + verifySteps(TerrainRest.SETTLE_STEPS + 1); + assertEquals(lower, location); + // From there every tick steps again until it has settled on the new ground. + moveTicks(TerrainRest.SETTLE_STEPS); + verifySteps(TerrainRest.SETTLE_STEPS * 2 + 1); + moveTicks(1); + verifySteps(TerrainRest.SETTLE_STEPS * 2 + 1); + } + @Test void forgettingTheRestBringsEveryGroundStepBackUntilItSettlesAgain() { idleOnTerrain();