From 461c9d98ab34f2b7636bf1c2d93cc4edf8e77f43 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Sat, 26 Sep 2026 00:19:39 +0000 Subject: [PATCH 1/4] fix: keep train direction when joining track, guard dropped turnouts Trains face the +s direction of their spline, so any edit that reverses a spline turns the loco round and puts its cars on the other side. - Closing a loop keeps the track's own direction and adds the closing curve backwards instead. - Joining two tracks builds the result in whichever direction reverses neither, or else only one without trains that is not a branch. Joining two occupied tracks that would need reversing is refused. - The remover's train check also covers branch turnouts a dig would drop because their frog ends up on a piece shorter than the minimum lay. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../vehicleframework/VehicleFramework.java | 1 + .../tracks/TrackCommands.java | 11 +- .../tracks/TrackRegistry.java | 116 +++++++++++++-- .../vehicles/handlers/TrainHandler.java | 14 ++ .../tracks/TrackRegistryDirectionTest.java | 134 ++++++++++++++++++ .../handlers/TrainReversePlacementTest.java | 25 +++- 6 files changed, 283 insertions(+), 18 deletions(-) create mode 100644 src/test/java/net/tfminecraft/vehicleframework/tracks/TrackRegistryDirectionTest.java diff --git a/src/main/java/net/tfminecraft/vehicleframework/VehicleFramework.java b/src/main/java/net/tfminecraft/vehicleframework/VehicleFramework.java index 045ae39..8049f18 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/VehicleFramework.java +++ b/src/main/java/net/tfminecraft/vehicleframework/VehicleFramework.java @@ -65,6 +65,7 @@ public void onEnable() { plugin = this; trackRegistry = new TrackRegistry(getDataFolder()); trackRegistry.onRebuilt(TrainHandler::retrackTrains); + trackRegistry.occupiedBy(TrainHandler::anyTrainOn); log = new LogWriter(getDataFolder()); VFLogger.info("Running checks..."); createFolders(); diff --git a/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackCommands.java b/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackCommands.java index 718d1b0..e52b59d 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackCommands.java +++ b/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackCommands.java @@ -363,11 +363,14 @@ private static boolean trainOn(TrackRegistry.DigTarget target) { if (vehicles == null) { return false; } - UUID trackId = target.spline().getId(); for (ActiveVehicle vehicle : vehicles.get().values()) { - if (vehicle.isTrain() && !vehicle.hasParent() - && vehicle.getTrainHandler().occupies(trackId, target.centreS(), target.halfSpan())) { - return true; + if (!vehicle.isTrain() || vehicle.hasParent()) { + continue; + } + for (TrackRegistry.Span span : target.spans()) { + if (vehicle.getTrainHandler().occupies(span.trackId(), span.centreS(), span.halfSpan())) { + return true; + } } } return false; diff --git a/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackRegistry.java b/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackRegistry.java index 052f57c..0bdbd67 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackRegistry.java +++ b/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackRegistry.java @@ -12,6 +12,7 @@ import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.function.BiConsumer; +import java.util.function.Predicate; import org.bukkit.World; @@ -26,6 +27,7 @@ public final class TrackRegistry { private final Map junctions = new ConcurrentHashMap<>(); private BiConsumer> rebuilt = (old, next) -> { }; + private Predicate occupied = id -> false; public TrackRegistry(File dataFolder) { this.store = new TrackStore(dataFolder); @@ -41,6 +43,11 @@ public void onRebuilt(BiConsumer> listener) { } : listener; } + /** Tells the registry which splines have a train bound to them. */ + public void occupiedBy(Predicate test) { + occupied = test == null ? id -> false : test; + } + public void loadFromDisk() { splines.clear(); junctions.clear(); @@ -161,10 +168,14 @@ public Optional findEnd(String world, double x, double y, double z) { } /** - * The sample a dig at this point would remove, and the arc span of track - * it takes with it: {@code centreS} plus or minus {@code halfSpan}. + * The sample a dig at this point would remove, and every stretch of track + * the dig may take with it, including turnouts it would drop. */ - public record DigTarget(TrackSpline spline, int index, double centreS, double halfSpan) { + public record DigTarget(TrackSpline spline, int index, List spans) { + } + + /** Track from {@code centreS - halfSpan} to {@code centreS + halfSpan} on {@code trackId}. */ + public record Span(UUID trackId, double centreS, double halfSpan) { } public Optional digTarget(String world, double x, double y, double z) { @@ -190,9 +201,7 @@ public Optional digTarget(String world, double x, double y, double z) Optional turnout = turnoutDug(spline, digS); if (turnout.isPresent()) { // Digging a turnout removes the branch all the way back to the stem. - double turnoutEnd = turnout.get().turnoutEndS; - return Optional.of(new DigTarget(spline, index, turnoutEnd / 2, - turnoutEnd / 2 + TrackGenerate.STEP)); + return Optional.of(new DigTarget(spline, index, List.of(turnoutSpan(turnout.get(), spline)))); } double before = index > 0 ? digS - samples.get(index - 1).s : 0; double after = index < samples.size() - 1 ? samples.get(index + 1).s - digS : 0; @@ -201,7 +210,49 @@ public Optional digTarget(String world, double x, double y, double z) before = index == 0 ? seam : before; after = index == 0 ? after : seam; } - return Optional.of(new DigTarget(spline, index, digS, Math.max(before, after))); + List spans = new ArrayList<>(); + spans.add(new Span(spline.getId(), digS, Math.max(before, after))); + // A junction whose frog ends up on a piece too short for it loses its + // turnout when rehomed (see rehomeJunctions), so that turnout goes too. + for (TrackJunction junction : junctionsOn(spline.getId())) { + TrackSpline branch = junction.branchSplineId == null ? null : splines.get(junction.branchSplineId); + if (branch != null && frogPieceLength(spline, index, junction.s) < Cache.trackMinLayDistance - 1e-9) { + spans.add(turnoutSpan(junction, branch)); + } + } + return Optional.of(new DigTarget(spline, index, spans)); + } + + private static Span turnoutSpan(TrackJunction junction, TrackSpline branch) { + double cutoff = turnoutCutoff(junction, branch); + return new Span(branch.getId(), cutoff / 2, cutoff / 2 + TrackGenerate.STEP); + } + + /** + * Length of the piece a frog at {@code frogS} lands on after digging + * {@code index}. A frog inside the dug gap may land on either piece. + */ + private static double frogPieceLength(TrackSpline spline, int index, double frogS) { + List samples = spline.getSamples(); + int n = samples.size(); + if (n <= 2) { + return 0; + } + if (index == 0) { + return samples.get(n - 1).s - samples.get(1).s; + } + if (index == n - 1) { + return samples.get(n - 2).s; + } + double head = samples.get(index - 1).s; + double tail = samples.get(n - 1).s - samples.get(index + 1).s; + if (frogS <= head + 1e-9) { + return head; + } + if (frogS >= samples.get(index + 1).s - 1e-9 && frogS <= samples.get(n - 1).s + 1e-9) { + return tail; + } + return Math.min(head, tail); } public DigResult digAt(TrackSpline spline, int index) { @@ -322,10 +373,18 @@ private StrokeLay closeLoop(TrackEnd from, TrackEnd to, World bukkitWorld) throw Cache.trackMinLayDistance, Cache.trackMaxTurnDegrees, Cache.trackDesiredGradeDegrees, Cache.trackMaxGradeDegrees, TrackGenerate.STEP); TrackClearance.check(bukkitWorld, extra, this, Set.of(spline.getId())); - List merged = orientedToJoin(spline, from.prepend); + // Close the loop in the track's own direction. Reversing it would turn + // round any train on it and leave its junctions facing the wrong way. + List merged = spline.xyz(); int last = extra.size() - 1; - for (int i = 1; i < last; i++) { - merged.add(extra.get(i)); + if (from.prepend) { + for (int i = last - 1; i >= 1; i--) { + merged.add(extra.get(i)); + } + } else { + for (int i = 1; i < last; i++) { + merged.add(extra.get(i)); + } } TrackDisplayManager displays = VehicleFramework.getTrackDisplayManager(); if (displays != null) { @@ -336,6 +395,21 @@ private StrokeLay closeLoop(TrackEnd from, TrackEnd to, World bukkitWorld) throw } private StrokeLay connect(TrackEnd from, TrackEnd to, World bukkitWorld) throws TrackLayException { + // Laid as-is, joining from a start reverses `from` and joining to an end + // reverses `to`. Building the joined track the other way round flips + // both. Trains face +s, so never reverse a track with a train on it. + boolean keepReversed = from.prepend; + boolean dropReversed = !to.prepend; + boolean flip = reversalCost(from.spline, !keepReversed) + reversalCost(to.spline, !dropReversed) + < reversalCost(from.spline, keepReversed) + reversalCost(to.spline, dropReversed); + if (flip) { + keepReversed = !keepReversed; + dropReversed = !dropReversed; + } + if ((keepReversed && occupied.test(from.spline.getId())) + || (dropReversed && occupied.test(to.spline.getId()))) { + throw new TrackLayException("A train is on this track. Move it before joining here."); + } TrackSample originA = from.prepend ? from.spline.first() : from.spline.last(); float yaw = originA.yaw; if (from.prepend) { @@ -356,6 +430,9 @@ private StrokeLay connect(TrackEnd from, TrackEnd to, World bukkitWorld) throws for (int i = 1; i < rest.size(); i++) { merged.add(rest.get(i)); } + if (flip) { + Collections.reverse(merged); + } UUID drop = to.spline.getId(); UUID keep = from.spline.getId(); List keepSaved = List.copyOf(junctionsOn(keep)); @@ -375,8 +452,8 @@ private StrokeLay connect(TrackEnd from, TrackEnd to, World bukkitWorld) throws TrackSpline.shouldLoop(merged, Cache.trackJoinDistance), merged); TrackSpline stored = replaceQuietly(next); - rehomeJunctions(keepSaved, oldKeep, from.prepend, stored); - rehomeJunctions(dropSaved, oldDrop, !to.prepend, stored); + rehomeJunctions(keepSaved, oldKeep, keepReversed, stored); + rehomeJunctions(dropSaved, oldDrop, dropReversed, stored); // Retrack only once junctions sit on the joined spline, so train routes survive. rebuilt.accept(oldKeep, List.of(stored)); rebuilt.accept(oldDrop, List.of(stored)); @@ -386,6 +463,21 @@ private StrokeLay connect(TrackEnd from, TrackEnd to, World bukkitWorld) throws private record StrokeLay(TrackSpline spline, List stroke, int previousCount) { } + // Branches must start at their frog, and trains would turn round. + private int reversalCost(TrackSpline spline, boolean reversed) { + if (!reversed) { + return 0; + } + int cost = 1; + if (occupied.test(spline.getId())) { + cost += 2; + } + if (junctionByBranch(spline.getId()).isPresent()) { + cost += 4; + } + return cost; + } + private static List orientedToJoin(TrackSpline spline, boolean joinAtFirst) { List xyz = spline.xyz(); if (joinAtFirst) { diff --git a/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java b/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java index d54142d..ba90295 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java +++ b/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java @@ -788,6 +788,20 @@ public static void retrackTrains(TrackSpline old, List rebuilt) { } } + /** Whether any train car is bound to the given track. */ + public static boolean anyTrainOn(UUID splineId) { + VehicleManager vehicles = VehicleFramework.getVehicleManager(); + if (vehicles == null || splineId == null) { + return false; + } + for (ActiveVehicle vehicle : vehicles.get().values()) { + if (vehicle.isTrain() && splineId.equals(vehicle.getTrainHandler().getSplineId())) { + return true; + } + } + return false; + } + /** * Keeps this car where it physically was after its track is rebuilt. * Digging splits or trims a spline, which re-ids the far piece and shifts diff --git a/src/test/java/net/tfminecraft/vehicleframework/tracks/TrackRegistryDirectionTest.java b/src/test/java/net/tfminecraft/vehicleframework/tracks/TrackRegistryDirectionTest.java new file mode 100644 index 0000000..a1655fc --- /dev/null +++ b/src/test/java/net/tfminecraft/vehicleframework/tracks/TrackRegistryDirectionTest.java @@ -0,0 +1,134 @@ +package net.tfminecraft.vehicleframework.tracks; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.UUID; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class TrackRegistryDirectionTest { + + @Test + void connectFromStartToEndKeepsBothTracksDirection(@TempDir Path dir) throws Exception { + TrackRegistry registry = new TrackRegistry(dir.toFile()); + TrackSpline a = registry.lay("world", 0, 64, 30, 0, 64, 40).spline(); + registry.lay("world", 0, 64, 0, 0, 64, 10); + TrackSpline joined = registry.lay("world", 0, 64, 30, 0, 64, 10).spline(); + assertEquals(a.getId(), joined.getId()); + assertEquals(0, joined.first().z, 0.5); + assertEquals(40, joined.last().z, 0.5); + } + + @Test + void connectingTwoStartsReversesTheTrackWithoutTrains(@TempDir Path dir) throws Exception { + TrackRegistry registry = new TrackRegistry(dir.toFile()); + TrackSpline a = registry.lay("world", 0, 64, 30, 0, 64, 40).spline(); + registry.lay("world", 0, 64, 10, 0, 64, 0); + registry.occupiedBy(a.getId()::equals); + TrackSpline joined = registry.lay("world", 0, 64, 30, 0, 64, 10).spline(); + assertEquals(0, joined.first().z, 0.5); + assertEquals(40, joined.last().z, 0.5); + } + + @Test + void connectingTwoStartsKeepsOccupiedDropTrackDirection(@TempDir Path dir) throws Exception { + TrackRegistry registry = new TrackRegistry(dir.toFile()); + registry.lay("world", 0, 64, 30, 0, 64, 40); + TrackSpline b = registry.lay("world", 0, 64, 10, 0, 64, 0).spline(); + registry.occupiedBy(b.getId()::equals); + TrackSpline joined = registry.lay("world", 0, 64, 30, 0, 64, 10).spline(); + assertEquals(40, joined.first().z, 0.5); + assertEquals(0, joined.last().z, 0.5); + } + + @Test + void connectingTwoOccupiedStartsIsRefused(@TempDir Path dir) throws Exception { + TrackRegistry registry = new TrackRegistry(dir.toFile()); + TrackSpline a = registry.lay("world", 0, 64, 30, 0, 64, 40).spline(); + TrackSpline b = registry.lay("world", 0, 64, 10, 0, 64, 0).spline(); + registry.occupiedBy(Set.of(a.getId(), b.getId())::contains); + assertThrows(TrackLayException.class, () -> registry.lay("world", 0, 64, 30, 0, 64, 10)); + assertEquals(2, registry.inWorld("world").size()); + assertEquals(30, registry.get(a.getId()).orElseThrow().first().z, 0.5); + } + + @Test + void closingLoopFromStartKeepsTrackDirection(@TempDir Path dir) throws Exception { + TrackRegistry registry = new TrackRegistry(dir.toFile()); + TrackSpline arc = registry.replace(arc()); + assertTrue(!arc.isLoop(), "Setup: the track starts open"); + TrackSample first = arc.first(); + TrackSample second = arc.getSamples().get(1); + TrackSample last = arc.last(); + TrackSpline loop = registry.lay("world", first.x, first.y, first.z, last.x, last.y, last.z).spline(); + assertTrue(loop.isLoop()); + assertEquals(arc.getId(), loop.getId()); + assertEquals(first.x, loop.first().x, 1e-9); + assertEquals(first.z, loop.first().z, 1e-9); + assertEquals(second.x, loop.getSamples().get(1).x, 1e-9); + assertEquals(second.z, loop.getSamples().get(1).z, 1e-9); + } + + @Test + void digTargetIncludesTurnoutDroppedWithShortStemPiece(@TempDir Path dir) throws Exception { + TrackRegistry registry = new TrackRegistry(dir.toFile()); + TrackSpline stem = registry.lay("world", 0, 64, 0, 0, 64, 40).spline(); + TrackJunction placed = registry.putJunction(new TrackJunction( + UUID.randomUUID(), stem.getId(), 34, 1, TrackJunction.Side.RIGHT, null)); + TrackSpline branch = registry.layBranch(placed.id, "world", null, 2, 64, 52); + stem = registry.get(stem.getId()).orElseThrow(); + + TrackSample far = stem.getSamples().get(10); + assertEquals(1, registry.digTarget("world", far.x, far.y, far.z).orElseThrow().spans().size()); + + // Cutting at s=32 leaves the frog at s=34 on a 7-block piece, under the 8 minimum. + int near = indexAt(stem, 32); + TrackSample cut = stem.getSamples().get(near); + TrackRegistry.DigTarget target = registry.digTarget("world", cut.x, cut.y, cut.z).orElseThrow(); + assertEquals(2, target.spans().size()); + assertEquals(branch.getId(), target.spans().get(1).trackId()); + + registry.digAt(stem, near); + assertTrue(registry.getJunction(placed.id).isEmpty(), "The dig must really drop that turnout"); + } + + private static int indexAt(TrackSpline spline, double s) { + List samples = spline.getSamples(); + for (int i = 0; i < samples.size(); i++) { + if (Math.abs(samples.get(i).s - s) < 1e-6) { + return i; + } + } + throw new AssertionError("No sample at s=" + s); + } + + // A racetrack open along one straight, so the closing piece runs straight. + private static TrackSpline arc() { + List points = new ArrayList<>(); + for (int z = 10; z <= 40; z++) { + points.add(new double[] {0, 64, z}); + } + addSemicircle(points, 20, 40, Math.PI); + for (int z = 39; z >= 0; z--) { + points.add(new double[] {40, 64, z}); + } + addSemicircle(points, 20, 0, 0); + return TrackSpline.fromPoints(UUID.randomUUID(), "world", false, points); + } + + private static void addSemicircle(List points, double cx, double cz, double from) { + int steps = 63; + for (int i = 1; i < steps; i++) { + double angle = from - Math.PI * i / steps; + points.add(new double[] {cx + 20 * Math.cos(angle), 64, cz + 20 * Math.sin(angle)}); + } + } + +} diff --git a/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java b/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java index 413f0cd..661a913 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java @@ -486,6 +486,26 @@ void splittingStemKeepsRouteOfTrainLeavingBranch() { assertEquals(before.getZ(), first.v.getEntity().getLocation().getZ(), 1e-8); } + @Test + void joiningAnotherTrackToTrainTracksStartKeepsTrainFacingTheSameWay() throws Exception { + TrackSpline track = denseTrack(); + TrainHandler loco = consist(track, 60); + TrainHandler first = loco.getChild().getTrainHandler(); + TrainHandler last = first.getChild().getTrainHandler(); + registry.occupiedBy(id -> List.of(loco, first, last).stream() + .anyMatch(car -> id.equals(car.getSplineId()))); + registry.lay("world", 0, 64, -20, 0, 64, -30); + // Start to start: one of the two tracks has to be reversed. + registry.lay("world", 0, 64, 0, 0, 64, -20); + loco.splineTick(); + assertEquals(60, loco.v.getEntity().getLocation().getZ(), 1e-6); + assertEquals(50, first.v.getEntity().getLocation().getZ(), 1e-6); + assertEquals(40, last.v.getEntity().getLocation().getZ(), 1e-6); + loco.v.getAccessPanel().setSpeed(0.2); + loco.splineTick(); + assertEquals(60.2, loco.v.getEntity().getLocation().getZ(), 1e-6); + } + @Test void deletingTrackDoesNotMoveTrainOntoCrossingTrack() { TrackSpline track = denseTrack(); @@ -519,8 +539,9 @@ void digTargetCoversTheEdgesItRemoves() { TrackSpline track = denseTrack(); TrackRegistry.DigTarget target = registry.digTarget("world", 0, 64, 20.2).orElseThrow(); assertEquals(20, target.index()); - assertEquals(20, target.centreS(), 1e-8); - assertEquals(1, target.halfSpan(), 1e-8); + assertEquals(1, target.spans().size()); + assertEquals(20, target.spans().get(0).centreS(), 1e-8); + assertEquals(1, target.spans().get(0).halfSpan(), 1e-8); assertEquals(track.getId(), target.spline().getId()); } From 46ecceea605fa772e4c53f37a76dbb73385e9fff Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Sat, 26 Sep 2026 00:29:10 +0000 Subject: [PATCH 2/4] fix: treat trains as hard limits on joins, repair unloaded trains - Joining picks a direction that never reverses an occupied track, and only penalises reversing the kept track when it is a branch. It looks each track up once. - Occupancy for joins also counts trains saved in unloaded chunks, since a long track can reach them. - A loaded train checks its saved (spline, s) against where its entity respawned, and re-finds the track under it if the track was edited while it was unloaded, or unbinds if the track is gone. - The turnout check models one-sample stubs and whole-track deletes, which keep the turnout, and errs on the safe side of the length limit. - The remover plans each consist once for all spans. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../tracks/TrackCommands.java | 10 +- .../tracks/TrackRegistry.java | 125 +++++++------- .../vehicles/handlers/TrainHandler.java | 154 +++++++++++++----- .../tracks/TrackRegistryDirectionTest.java | 16 ++ .../handlers/TrainReversePlacementTest.java | 60 ++++++- 5 files changed, 255 insertions(+), 110 deletions(-) diff --git a/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackCommands.java b/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackCommands.java index e52b59d..0fdefae 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackCommands.java +++ b/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackCommands.java @@ -364,13 +364,9 @@ private static boolean trainOn(TrackRegistry.DigTarget target) { return false; } for (ActiveVehicle vehicle : vehicles.get().values()) { - if (!vehicle.isTrain() || vehicle.hasParent()) { - continue; - } - for (TrackRegistry.Span span : target.spans()) { - if (vehicle.getTrainHandler().occupies(span.trackId(), span.centreS(), span.halfSpan())) { - return true; - } + if (vehicle.isTrain() && !vehicle.hasParent() + && vehicle.getTrainHandler().occupies(target.spans())) { + return true; } } return false; diff --git a/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackRegistry.java b/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackRegistry.java index 0bdbd67..7ed3cec 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackRegistry.java +++ b/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackRegistry.java @@ -21,6 +21,8 @@ public final class TrackRegistry { private static final double PRUNE_NESTED_MAX_LENGTH = 16.0; + private static final double REHOME_REACH = 2.5; + private static final double DROP_MARGIN = 0.5; private final TrackStore store; private final Map splines = new ConcurrentHashMap<>(); @@ -216,7 +218,9 @@ public Optional digTarget(String world, double x, double y, double z) // turnout when rehomed (see rehomeJunctions), so that turnout goes too. for (TrackJunction junction : junctionsOn(spline.getId())) { TrackSpline branch = junction.branchSplineId == null ? null : splines.get(junction.branchSplineId); - if (branch != null && frogPieceLength(spline, index, junction.s) < Cache.trackMinLayDistance - 1e-9) { + // Resettling can change piece length slightly, so err towards protecting the turnout. + if (branch != null + && frogPieceLength(spline, index, junction.s) < Cache.trackMinLayDistance + DROP_MARGIN) { spans.add(turnoutSpan(junction, branch)); } } @@ -229,30 +233,40 @@ private static Span turnoutSpan(TrackJunction junction, TrackSpline branch) { } /** - * Length of the piece a frog at {@code frogS} lands on after digging - * {@code index}. A frog inside the dug gap may land on either piece. + * Length of the piece a frog at {@code frogS} is rehomed onto after digging + * {@code index}, following digAt and rehomeJunctions. Infinite when the + * turnout survives: the whole track is deleted (only the junction record + * goes), or no piece is near enough and just the junction is deleted. */ private static double frogPieceLength(TrackSpline spline, int index, double frogS) { List samples = spline.getSamples(); int n = samples.size(); if (n <= 2) { - return 0; - } - if (index == 0) { - return samples.get(n - 1).s - samples.get(1).s; - } - if (index == n - 1) { - return samples.get(n - 2).s; + return Double.POSITIVE_INFINITY; } - double head = samples.get(index - 1).s; - double tail = samples.get(n - 1).s - samples.get(index + 1).s; - if (frogS <= head + 1e-9) { - return head; + List pieces = new ArrayList<>(); + if (index == 0 || index == n - 1) { + int first = index == 0 ? 1 : 0; + int last = index == 0 ? n - 1 : n - 2; + pieces.add(new double[] {samples.get(first).s, samples.get(last).s}); + } else { + // A piece needs two samples; a one-sample stub is dropped. + if (index >= 2) { + pieces.add(new double[] {0, samples.get(index - 1).s}); + } + if (index <= n - 3) { + pieces.add(new double[] {samples.get(index + 1).s, samples.get(n - 1).s}); + } } - if (frogS >= samples.get(index + 1).s - 1e-9 && frogS <= samples.get(n - 1).s + 1e-9) { - return tail; + double length = Double.POSITIVE_INFINITY; + for (double[] piece : pieces) { + // rehomeJunctions matches within 2.5 blocks, so a frog in the gap + // can land on the end of either piece. + if (frogS >= piece[0] - REHOME_REACH && frogS <= piece[1] + REHOME_REACH) { + length = Math.min(length, piece[1] - piece[0]); + } } - return Math.min(head, tail); + return length; } public DigResult digAt(TrackSpline spline, int index) { @@ -395,21 +409,26 @@ private StrokeLay closeLoop(TrackEnd from, TrackEnd to, World bukkitWorld) throw } private StrokeLay connect(TrackEnd from, TrackEnd to, World bukkitWorld) throws TrackLayException { - // Laid as-is, joining from a start reverses `from` and joining to an end - // reverses `to`. Building the joined track the other way round flips - // both. Trains face +s, so never reverse a track with a train on it. - boolean keepReversed = from.prepend; - boolean dropReversed = !to.prepend; - boolean flip = reversalCost(from.spline, !keepReversed) + reversalCost(to.spline, !dropReversed) - < reversalCost(from.spline, keepReversed) + reversalCost(to.spline, dropReversed); - if (flip) { - keepReversed = !keepReversed; - dropReversed = !dropReversed; - } - if ((keepReversed && occupied.test(from.spline.getId())) - || (dropReversed && occupied.test(to.spline.getId()))) { + // Laid from `from` to `to`, joining from a start reverses `from` and + // joining to an end reverses `to`. Building the joined track the other + // way round flips both. Trains face +s, so a track with a train on it + // must keep its direction; a kept branch must still start at its frog. + boolean fromReversedAsLaid = from.prepend; + boolean toReversedAsLaid = !to.prepend; + boolean fromOccupied = occupied.test(from.spline.getId()); + boolean toOccupied = occupied.test(to.spline.getId()); + boolean asLaidOk = !(fromReversedAsLaid && fromOccupied) && !(toReversedAsLaid && toOccupied); + boolean flippedOk = !(!fromReversedAsLaid && fromOccupied) && !(!toReversedAsLaid && toOccupied); + if (!asLaidOk && !flippedOk) { throw new TrackLayException("A train is on this track. Move it before joining here."); } + // The dropped track's junction refs are cleared anyway, so only the kept track's branch matters. + int fromBranchCost = junctionByBranch(from.spline.getId()).isPresent() ? 5 : 1; + int asLaidCost = (fromReversedAsLaid ? fromBranchCost : 0) + (toReversedAsLaid ? 1 : 0); + int flippedCost = (fromReversedAsLaid ? 0 : fromBranchCost) + (toReversedAsLaid ? 0 : 1); + boolean flip = !asLaidOk || (flippedOk && flippedCost < asLaidCost); + boolean keepReversed = flip != fromReversedAsLaid; + boolean dropReversed = flip != toReversedAsLaid; TrackSample originA = from.prepend ? from.spline.first() : from.spline.last(); float yaw = originA.yaw; if (from.prepend) { @@ -422,17 +441,16 @@ private StrokeLay connect(TrackEnd from, TrackEnd to, World bukkitWorld) throws Cache.trackDesiredGradeDegrees, Cache.trackMaxGradeDegrees, TrackGenerate.STEP); TrackClearance.check( bukkitWorld, extra, this, Set.of(from.spline.getId(), to.spline.getId())); - List merged = orientedToJoin(from.spline, from.prepend); - for (int i = 1; i < extra.size(); i++) { - merged.add(extra.get(i)); - } - List rest = orientedFromJoin(to.spline, to.prepend); - for (int i = 1; i < rest.size(); i++) { - merged.add(rest.get(i)); - } + List fromPart = points(from.spline, keepReversed); + List toPart = points(to.spline, dropReversed); + List stroke = new ArrayList<>(extra); if (flip) { - Collections.reverse(merged); + Collections.reverse(stroke); } + List merged = new ArrayList<>(flip ? toPart : fromPart); + merged.addAll(stroke.subList(1, stroke.size())); + List tail = flip ? fromPart : toPart; + merged.addAll(tail.subList(1, tail.size())); UUID drop = to.spline.getId(); UUID keep = from.spline.getId(); List keepSaved = List.copyOf(junctionsOn(keep)); @@ -463,32 +481,9 @@ private StrokeLay connect(TrackEnd from, TrackEnd to, World bukkitWorld) throws private record StrokeLay(TrackSpline spline, List stroke, int previousCount) { } - // Branches must start at their frog, and trains would turn round. - private int reversalCost(TrackSpline spline, boolean reversed) { - if (!reversed) { - return 0; - } - int cost = 1; - if (occupied.test(spline.getId())) { - cost += 2; - } - if (junctionByBranch(spline.getId()).isPresent()) { - cost += 4; - } - return cost; - } - - private static List orientedToJoin(TrackSpline spline, boolean joinAtFirst) { - List xyz = spline.xyz(); - if (joinAtFirst) { - Collections.reverse(xyz); - } - return xyz; - } - - private static List orientedFromJoin(TrackSpline spline, boolean joinAtFirst) { + private static List points(TrackSpline spline, boolean reversed) { List xyz = spline.xyz(); - if (!joinAtFirst) { + if (reversed) { Collections.reverse(xyz); } return xyz; @@ -727,7 +722,7 @@ private void rehomeJunctions( for (TrackJunction junction : saved) { TrackPose pose = from.sampleAt(junction.s); TrackSpline best = null; - double bestD = 2.5; + double bestD = REHOME_REACH; double bestS = 0; for (TrackSpline target : onto) { if (target == null) { diff --git a/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java b/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java index ba90295..a036700 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java +++ b/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java @@ -1,6 +1,7 @@ package net.tfminecraft.vehicleframework.vehicles.handlers; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.UUID; @@ -12,6 +13,8 @@ import org.bukkit.entity.Player; import org.bukkit.inventory.ItemStack; import org.bukkit.util.Vector; +import org.json.simple.JSONObject; +import org.json.simple.parser.JSONParser; import com.ticxo.modelengine.api.model.ActiveModel; @@ -20,6 +23,8 @@ import net.tfminecraft.vehicleframework.cache.Cache; import net.tfminecraft.vehicleframework.database.ConsistData; import net.tfminecraft.vehicleframework.database.PersistenceLog; +import net.tfminecraft.vehicleframework.database.VehicleRepository; +import net.tfminecraft.vehicleframework.database.VehicleSnapshot; import net.tfminecraft.vehicleframework.enums.Direction; import net.tfminecraft.vehicleframework.managers.VehicleManager; import net.tfminecraft.vehicleframework.tracks.ThrottleTape; @@ -54,6 +59,8 @@ public class TrainHandler { private String pendingChild; private UUID splineId; private double s; + // Track may have been edited while this train was unloaded. + private boolean checkLoadedPosition; private int travelSign = 1; private UUID armedJunctionId; private TrackJunction.Side armedSide; @@ -216,6 +223,7 @@ public void applyConsist(ConsistData consist) { splineId = null; } s = consist.getS() == null ? 0 : consist.getS(); + checkLoadedPosition = splineId != null; travelSign = consist.getTravelSign(); routeJunctionId = null; takeBranch = consist.isDiverge(); @@ -674,9 +682,39 @@ public void splineTick() { public void placeLoadedCars() { PersistenceLog.placeCars(v); + if (checkLoadedPosition && !v.hasParent()) { + checkLoadedPosition = false; + followTrackUnderEntity(); + } applyPlacements(planCars()); } + /** + * Saved {@code (spline, s)} is only valid if the track was not edited while + * the train was unloaded. The entity respawns where it was saved, so check + * that point and re-find the track under it if they disagree. + */ + private void followTrackUnderEntity() { + TrackRegistry registry = VehicleFramework.getTrackRegistry(); + if (registry == null || splineId == null || v == null || v.getEntity() == null + || v.getEntity().getWorld() == null) { + return; + } + Location loc = v.getEntity().getLocation(); + TrackPose at = new TrackPose(loc.getX(), loc.getY() - Cache.trackVehicleYOffset, loc.getZ(), 0, 0); + TrackSpline current = boundSpline(); + if (current != null && onTrack(current.sampleAt(s), at)) { + return; + } + TrackMatch match = nearestTrack(registry.inWorld(v.getEntity().getWorld().getName()), at); + if (match == null) { + PersistenceLog.append("RETRACK_LOAD none " + PersistenceLog.vehicle(v)); + unbind(); + return; + } + moveTo(registry, match); + } + private record CarPlacement(ActiveVehicle vehicle, TrackSpline spline, double s, int sign, double missingSpacing) { TrackPose pose() { return spline.sampleAt(s); @@ -788,15 +826,41 @@ public static void retrackTrains(TrackSpline old, List rebuilt) { } } - /** Whether any train car is bound to the given track. */ + /** + * Whether any train car is bound to the given track, loaded or not. A + * long track can reach chunks with trains parked in them. This reads every + * saved vehicle, so keep it to rare edits such as joins. + */ public static boolean anyTrainOn(UUID splineId) { + if (splineId == null) { + return false; + } VehicleManager vehicles = VehicleFramework.getVehicleManager(); - if (vehicles == null || splineId == null) { + if (vehicles != null) { + for (ActiveVehicle vehicle : vehicles.get().values()) { + if (vehicle.isTrain() && splineId.equals(vehicle.getTrainHandler().getSplineId())) { + return true; + } + } + } + VehicleRepository repository = VehicleFramework.getVehicleRepository(); + if (repository == null) { return false; } - for (ActiveVehicle vehicle : vehicles.get().values()) { - if (vehicle.isTrain() && splineId.equals(vehicle.getTrainHandler().getSplineId())) { - return true; + String id = splineId.toString(); + JSONParser parser = new JSONParser(); + for (VehicleSnapshot snapshot : repository.listAllLive()) { + String payload = snapshot.getPayloadJson(); + if (payload == null || !payload.contains(id)) { + continue; + } + try { + if (parser.parse(payload) instanceof JSONObject json + && id.equals(ConsistData.fromJson(json).getSplineId())) { + return true; + } + } catch (Exception ignored) { + // An unreadable row cannot be loaded either, so it holds no train. } } return false; @@ -813,37 +877,50 @@ public void retrack(TrackSpline old, List rebuilt) { if (splineId == null || old == null || rebuilt == null || !splineId.equals(old.getId())) { return; } - TrackPose was = old.sampleAt(s); - TrackSpline best = null; - double bestS = 0; + TrackMatch match = nearestTrack(rebuilt, old.sampleAt(s)); + if (match != null) { + moveTo(VehicleFramework.getTrackRegistry(), match); + } + } + + private record TrackMatch(TrackSpline spline, double s) { + } + + /** The closest point on these tracks that counts as the same place, preferring the current track. */ + private TrackMatch nearestTrack(Collection candidates, TrackPose was) { + TrackMatch best = null; double bestD = Double.POSITIVE_INFINITY; - for (TrackSpline candidate : rebuilt) { + for (TrackSpline candidate : candidates) { double candidateS = candidate.nearestS(was.x, was.y, was.z); TrackPose at = candidate.sampleAt(candidateS); - double horiz = Math.hypot(at.x - was.x, at.z - was.z); - double vert = Math.abs(at.y - was.y); - if (horiz > TrackClearance.OVERLAP_HORIZ || vert > TrackClearance.OVERLAP_VERT) { + if (!onTrack(at, was)) { continue; } - double d = horiz * horiz + vert * vert; - if (d < bestD) { - best = candidate; - bestS = candidateS; + double d = Math.pow(at.x - was.x, 2) + Math.pow(at.y - was.y, 2) + Math.pow(at.z - was.z, 2); + boolean tie = Math.abs(d - bestD) <= 1e-9; + if (d < bestD - 1e-9 || (tie && candidate.getId().equals(splineId))) { + best = new TrackMatch(candidate, candidateS); bestD = d; } } - if (best == null) { - return; - } - TrackRegistry registry = VehicleFramework.getTrackRegistry(); - if (!best.getId().equals(splineId) && registry != null && !routeTouches(registry, best.getId())) { + return best; + } + + private static boolean onTrack(TrackPose at, TrackPose was) { + return Math.hypot(at.x - was.x, at.z - was.z) <= TrackClearance.OVERLAP_HORIZ + && Math.abs(at.y - was.y) <= TrackClearance.OVERLAP_VERT; + } + + private void moveTo(TrackRegistry registry, TrackMatch match) { + UUID target = match.spline().getId(); + if (!target.equals(splineId) && registry != null && !routeTouches(registry, target)) { routeJunctionId = null; takeBranch = false; } PersistenceLog.append("RETRACK " + PersistenceLog.vehicle(v) - + " from=" + splineId + "@" + s + " to=" + best.getId() + "@" + bestS); - splineId = best.getId(); - s = bestS; + + " from=" + splineId + "@" + s + " to=" + target + "@" + match.s()); + splineId = target; + s = match.s(); } private boolean routeTouches(TrackRegistry registry, UUID trackId) { @@ -856,23 +933,26 @@ private boolean routeTouches(TrackRegistry registry, UUID trackId) { } /** - * Whether any car of this consist sits within {@code halfSpan} of arc - * length {@code at} on the given track, counting each car out to its couplers. + * Whether any car of this consist sits on any of these spans, counting + * each car out to its couplers. */ - public boolean occupies(UUID trackId, double at, double halfSpan) { - if (trackId == null || v == null || v.hasParent() || boundSpline() == null) { + public boolean occupies(List spans) { + if (spans == null || spans.isEmpty() || v == null || v.hasParent() || boundSpline() == null) { return false; } for (CarPlacement car : planCars()) { - if (!car.spline.getId().equals(trackId)) { - continue; - } - double d = Math.abs(car.s - at); - if (car.spline.isLoop()) { - d = Math.min(d, car.spline.length() - d); - } - if (d <= reach(car.vehicle.getTrainHandler()) + halfSpan) { - return true; + double reach = reach(car.vehicle.getTrainHandler()); + for (TrackRegistry.Span span : spans) { + if (!car.spline.getId().equals(span.trackId())) { + continue; + } + double d = Math.abs(car.s - span.centreS()); + if (car.spline.isLoop()) { + d = Math.min(d, car.spline.length() - d); + } + if (d <= reach + span.halfSpan()) { + return true; + } } } return false; diff --git a/src/test/java/net/tfminecraft/vehicleframework/tracks/TrackRegistryDirectionTest.java b/src/test/java/net/tfminecraft/vehicleframework/tracks/TrackRegistryDirectionTest.java index a1655fc..06053b3 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/tracks/TrackRegistryDirectionTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/tracks/TrackRegistryDirectionTest.java @@ -99,6 +99,22 @@ void digTargetIncludesTurnoutDroppedWithShortStemPiece(@TempDir Path dir) throws assertTrue(registry.getJunction(placed.id).isEmpty(), "The dig must really drop that turnout"); } + @Test + void digTargetSkipsTurnoutWhenFrogStaysOnLongPiece(@TempDir Path dir) throws Exception { + TrackRegistry registry = new TrackRegistry(dir.toFile()); + TrackSpline stem = registry.lay("world", 0, 64, 0, 0, 64, 40).spline(); + TrackJunction placed = registry.putJunction(new TrackJunction( + UUID.randomUUID(), stem.getId(), 34, 1, TrackJunction.Side.RIGHT, null)); + registry.layBranch(placed.id, "world", null, 2, 64, 52); + stem = registry.get(stem.getId()).orElseThrow(); + // Digging next to the end leaves a one-sample stub; the frog stays on the 38-block piece. + int nearEnd = stem.getSamples().size() - 2; + TrackSample cut = stem.getSamples().get(nearEnd); + assertEquals(1, registry.digTarget("world", cut.x, cut.y, cut.z).orElseThrow().spans().size()); + registry.digAt(stem, nearEnd); + assertTrue(registry.getJunction(placed.id).isPresent()); + } + private static int indexAt(TrackSpline spline, double s) { List samples = spline.getSamples(); for (int i = 0; i < samples.size(); i++) { diff --git a/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java b/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java index 661a913..5721f1c 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java @@ -34,6 +34,8 @@ import net.tfminecraft.vehicleframework.VehicleFramework; import net.tfminecraft.vehicleframework.database.ConsistData; +import net.tfminecraft.vehicleframework.database.VehicleRepository; +import net.tfminecraft.vehicleframework.database.VehicleSnapshot; import net.tfminecraft.vehicleframework.tracks.TrackJunction; import net.tfminecraft.vehicleframework.tracks.TrackRegistry; import net.tfminecraft.vehicleframework.tracks.TrackSpline; @@ -506,6 +508,56 @@ void joiningAnotherTrackToTrainTracksStartKeepsTrainFacingTheSameWay() throws Ex assertEquals(60.2, loco.v.getEntity().getLocation().getZ(), 1e-6); } + @Test + void loadingTrainAfterTrackWasSplitWhileUnloadedKeepsItsPosition() { + TrackSpline track = denseTrack(); + TrainHandler loco = consist(track, 60); + inWorld(loco, "world"); + registry.onRebuilt(null); + registry.digAt(track, 20); + // Loading restores the (spline, s) saved before the edit. + loco.applyConsist(new ConsistData(null, null, track.getId().toString(), 60d, 1)); + loco.placeLoadedCars(); + assertNotEquals(track.getId(), loco.getSplineId()); + assertEquals(39, loco.getS(), 1e-8); + assertEquals(60, loco.v.getEntity().getLocation().getZ(), 1e-8); + assertEquals(40, loco.getChild().getTrainHandler().getChild().getTrainHandler() + .v.getEntity().getLocation().getZ(), 1e-8); + } + + @Test + void loadingTrainWhoseTrackWasRemovedWhileUnloadedUnbindsIt() { + TrackSpline track = denseTrack(); + TrainHandler loco = consist(track, 60); + inWorld(loco, "world"); + registry.onRebuilt(null); + registry.delete(track.getId()); + loco.applyConsist(new ConsistData(null, null, track.getId().toString(), 60d, 1)); + loco.placeLoadedCars(); + assertFalse(loco.isBound()); + } + + @Test + void savedTrainOnTrackCountsAsOccupyingItWhileUnloaded() throws Exception { + TrackSpline track = denseTrack(); + Field repositoryField = VehicleFramework.class.getDeclaredField("vehicleRepository"); + repositoryField.setAccessible(true); + Object previous = repositoryField.get(null); + VehicleRepository repository = VehicleRepository.open(directory.resolve("vehicles.db").toFile()); + try { + repositoryField.set(null, repository); + assertFalse(TrainHandler.anyTrainOn(track.getId())); + String payload = "{\"splineId\":\"" + track.getId() + "\",\"s\":60.0,\"travelSign\":1}"; + repository.upsert(new VehicleSnapshot(UUID.randomUUID().toString(), "loco", "world", + 0, 64.5, 60, 0f, 0, 3, payload, VehicleRepository.SCHEMA_VERSION, 1, false, 1L)); + assertTrue(TrainHandler.anyTrainOn(track.getId())); + assertFalse(TrainHandler.anyTrainOn(UUID.randomUUID())); + } finally { + repositoryField.set(null, previous); + repository.close(); + } + } + @Test void deletingTrackDoesNotMoveTrainOntoCrossingTrack() { TrackSpline track = denseTrack(); @@ -531,7 +583,7 @@ void breakingTrackPieceElsewhereKeepsTrainPosition() { void consistOccupiesTrackOutToItsCouplers(double at, double halfSpan, boolean occupied) { TrackSpline track = denseTrack(); TrainHandler loco = consist(track, 60); - assertEquals(occupied, loco.occupies(track.getId(), at, halfSpan)); + assertEquals(occupied, loco.occupies(List.of(new TrackRegistry.Span(track.getId(), at, halfSpan)))); } @Test @@ -664,6 +716,12 @@ private void wall(TrainHandler loco, BoundingBox obstacle) { } } + private static void inWorld(TrainHandler loco, String name) { + World world = stub(World.class); + when(world.getName()).thenReturn(name); + when(loco.v.getEntity().getWorld()).thenReturn(world); + } + private static T stub(Class type) { return mock(type); } From eb9b5c483f678c551e10d5cb25f5935be461c889 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Sat, 26 Sep 2026 00:42:35 +0000 Subject: [PATCH 3/4] fix: only rehome a loaded train onto track running its way The load repair matched track by position alone, so a train could come back on reversed or crossing track and face the wrong way. It now reads the model's saved heading (entity yaw minus bone yaw, the inverse of applyPose) and only accepts track within 60 degrees of it, unbinding otherwise. Without a rotator the check is skipped. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../vehicles/handlers/TrainHandler.java | 37 +++++++++++++++--- .../handlers/TrainReversePlacementTest.java | 39 +++++++++++++++++++ 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java b/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java index a036700..70116ff 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java +++ b/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java @@ -13,6 +13,7 @@ import org.bukkit.entity.Player; import org.bukkit.inventory.ItemStack; import org.bukkit.util.Vector; +import org.joml.Quaternionf; import org.json.simple.JSONObject; import org.json.simple.parser.JSONParser; @@ -706,7 +707,7 @@ private void followTrackUnderEntity() { if (current != null && onTrack(current.sampleAt(s), at)) { return; } - TrackMatch match = nearestTrack(registry.inWorld(v.getEntity().getWorld().getName()), at); + TrackMatch match = nearestTrack(registry.inWorld(v.getEntity().getWorld().getName()), at, savedModelYaw()); if (match == null) { PersistenceLog.append("RETRACK_LOAD none " + PersistenceLog.vehicle(v)); unbind(); @@ -877,7 +878,7 @@ public void retrack(TrackSpline old, List rebuilt) { if (splineId == null || old == null || rebuilt == null || !splineId.equals(old.getId())) { return; } - TrackMatch match = nearestTrack(rebuilt, old.sampleAt(s)); + TrackMatch match = nearestTrack(rebuilt, old.sampleAt(s), null); if (match != null) { moveTo(VehicleFramework.getTrackRegistry(), match); } @@ -886,14 +887,18 @@ public void retrack(TrackSpline old, List rebuilt) { private record TrackMatch(TrackSpline spline, double s) { } - /** The closest point on these tracks that counts as the same place, preferring the current track. */ - private TrackMatch nearestTrack(Collection candidates, TrackPose was) { + /** + * The closest point on these tracks that counts as the same place, + * preferring the current track. With {@code facing}, only track whose +s + * runs the way the model faces qualifies; the consist cannot face -s. + */ + private TrackMatch nearestTrack(Collection candidates, TrackPose was, Float facing) { TrackMatch best = null; double bestD = Double.POSITIVE_INFINITY; for (TrackSpline candidate : candidates) { double candidateS = candidate.nearestS(was.x, was.y, was.z); TrackPose at = candidate.sampleAt(candidateS); - if (!onTrack(at, was)) { + if (!onTrack(at, was) || (facing != null && !facesAlong(facing, at))) { continue; } double d = Math.pow(at.x - was.x, 2) + Math.pow(at.y - was.y, 2) + Math.pow(at.z - was.z, 2); @@ -906,6 +911,28 @@ private TrackMatch nearestTrack(Collection candidates, TrackPose wa return best; } + /** + * World yaw the model faced when saved. applyPose turns the bone to the + * track's +s heading relative to the entity, so undo that. Null without a rotator. + */ + private Float savedModelYaw() { + if (v == null || v.getEntity() == null || v.getBehaviourHandler() == null) { + return null; + } + BoneRotator rotator = v.getBehaviourHandler().getRotator(); + if (rotator == null || rotator.getAnimator() == null || rotator.getAnimator().getRotation() == null) { + return null; + } + float boneYaw = new ConvertedAngle(new Quaternionf(rotator.getAnimator().getRotation())).getYaw(); + return ConvertedAngle.wrapDegrees(v.getEntity().getLocation().getYaw() - boneYaw); + } + + // Loose enough for curves and turnouts, tight enough to reject crossings and reversed track. + static boolean facesAlong(float modelYaw, TrackPose pose) { + float trackYaw = TrackSplineMotion.worldHeading(null, pose, 1).getYaw(); + return Math.abs(ConvertedAngle.wrapDegrees(trackYaw - modelYaw)) <= 60f; + } + private static boolean onTrack(TrackPose at, TrackPose was) { return Math.hypot(at.x - was.x, at.z - was.z) <= TrackClearance.OVERLAP_HORIZ && Math.abs(at.y - was.y) <= TrackClearance.OVERLAP_VERT; diff --git a/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java b/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java index 5721f1c..0c753da 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java @@ -13,6 +13,7 @@ import java.lang.reflect.Field; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.UUID; @@ -24,6 +25,7 @@ import org.bukkit.util.Vector; import org.bukkit.util.BoundingBox; import org.bukkit.util.VoxelShape; +import org.joml.Quaternionf; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -32,11 +34,15 @@ import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; +import com.ticxo.modelengine.api.model.bone.SimpleManualAnimator; + import net.tfminecraft.vehicleframework.VehicleFramework; +import net.tfminecraft.vehicleframework.bones.BoneRotator; import net.tfminecraft.vehicleframework.database.ConsistData; import net.tfminecraft.vehicleframework.database.VehicleRepository; import net.tfminecraft.vehicleframework.database.VehicleSnapshot; import net.tfminecraft.vehicleframework.tracks.TrackJunction; +import net.tfminecraft.vehicleframework.tracks.TrackPose; import net.tfminecraft.vehicleframework.tracks.TrackRegistry; import net.tfminecraft.vehicleframework.tracks.TrackSpline; import net.tfminecraft.vehicleframework.tracks.TrackStore; @@ -513,6 +519,7 @@ void loadingTrainAfterTrackWasSplitWhileUnloadedKeepsItsPosition() { TrackSpline track = denseTrack(); TrainHandler loco = consist(track, 60); inWorld(loco, "world"); + savedBoneYaw(loco, 0); registry.onRebuilt(null); registry.digAt(track, 20); // Loading restores the (spline, s) saved before the edit. @@ -525,6 +532,28 @@ void loadingTrainAfterTrackWasSplitWhileUnloadedKeepsItsPosition() { .v.getEntity().getLocation().getZ(), 1e-8); } + @Test + void loadingTrainOntoTrackReversedWhileUnloadedUnbindsIt() { + TrackSpline track = denseTrack(); + TrainHandler loco = consist(track, 60); + inWorld(loco, "world"); + // Entity yaw 0 and bone yaw 0: the model faces +z, the track's +s. + savedBoneYaw(loco, 0); + registry.onRebuilt(null); + List reversed = new ArrayList<>(track.xyz()); + Collections.reverse(reversed); + registry.replace(TrackSpline.fromPoints(track.getId(), "world", false, reversed)); + loco.applyConsist(new ConsistData(null, null, track.getId().toString(), 60d, 1)); + loco.placeLoadedCars(); + assertFalse(loco.isBound(), "A train must not come back facing the other way"); + } + + @ParameterizedTest + @CsvSource({"0, true", "45, true", "-45, true", "90, false", "180, false", "-120, false"}) + void modelMustFaceAlongTrackToRebind(float modelYaw, boolean along) { + assertEquals(along, TrainHandler.facesAlong(modelYaw, new TrackPose(0, 64, 0, 0f, 0f))); + } + @Test void loadingTrainWhoseTrackWasRemovedWhileUnloadedUnbindsIt() { TrackSpline track = denseTrack(); @@ -716,6 +745,16 @@ private void wall(TrainHandler loco, BoundingBox obstacle) { } } + private static void savedBoneYaw(TrainHandler loco, float yaw) { + SimpleManualAnimator animator = stub(SimpleManualAnimator.class); + when(animator.getRotation()).thenReturn(new Quaternionf().rotateYXZ((float) Math.toRadians(yaw), 0, 0)); + BoneRotator rotator = stub(BoneRotator.class); + when(rotator.getAnimator()).thenReturn(animator); + BehaviourHandler behaviour = stub(BehaviourHandler.class); + when(behaviour.getRotator()).thenReturn(rotator); + when(loco.v.getBehaviourHandler()).thenReturn(behaviour); + } + private static void inWorld(TrainHandler loco, String name) { World world = stub(World.class); when(world.getName()).thenReturn(name); From 66fab5cea7bba032aeaa65d5bebe0408fe9491ce Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Sat, 26 Sep 2026 00:49:07 +0000 Subject: [PATCH 4/4] fix: check heading even when a loaded train's saved spot still matches A track reversed in place could still put the saved s on the entity, so the fast path kept a binding facing the wrong way. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../vehicles/handlers/TrainHandler.java | 10 +++++++--- .../handlers/TrainReversePlacementTest.java | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java b/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java index 70116ff..a1fca23 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java +++ b/src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java @@ -704,10 +704,14 @@ private void followTrackUnderEntity() { Location loc = v.getEntity().getLocation(); TrackPose at = new TrackPose(loc.getX(), loc.getY() - Cache.trackVehicleYOffset, loc.getZ(), 0, 0); TrackSpline current = boundSpline(); - if (current != null && onTrack(current.sampleAt(s), at)) { - return; + Float facing = savedModelYaw(); + if (current != null) { + TrackPose saved = current.sampleAt(s); + if (onTrack(saved, at) && (facing == null || facesAlong(facing, saved))) { + return; + } } - TrackMatch match = nearestTrack(registry.inWorld(v.getEntity().getWorld().getName()), at, savedModelYaw()); + TrackMatch match = nearestTrack(registry.inWorld(v.getEntity().getWorld().getName()), at, facing); if (match == null) { PersistenceLog.append("RETRACK_LOAD none " + PersistenceLog.vehicle(v)); unbind(); diff --git a/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java b/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java index 0c753da..057561e 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainReversePlacementTest.java @@ -548,6 +548,22 @@ void loadingTrainOntoTrackReversedWhileUnloadedUnbindsIt() { assertFalse(loco.isBound(), "A train must not come back facing the other way"); } + @Test + void loadingTrainAtMiddleOfTrackReversedWhileUnloadedUnbindsIt() { + TrackSpline track = denseTrack(); + TrainHandler loco = consist(track, 50); + inWorld(loco, "world"); + savedBoneYaw(loco, 0); + registry.onRebuilt(null); + List reversed = new ArrayList<>(track.xyz()); + Collections.reverse(reversed); + // s=50 still lands on the same spot, but the track now runs the other way. + registry.replace(TrackSpline.fromPoints(track.getId(), "world", false, reversed)); + loco.applyConsist(new ConsistData(null, null, track.getId().toString(), 50d, 1)); + loco.placeLoadedCars(); + assertFalse(loco.isBound()); + } + @ParameterizedTest @CsvSource({"0, true", "45, true", "-45, true", "90, false", "180, false", "-120, false"}) void modelMustFaceAlongTrackToRebind(float modelYaw, boolean along) {