Repository navigation
perf: skip debug log building and re-placing standing trains every tick - #85
Conversation
… tick The recorder and persistence logs now return before building a line, key or map lookup when their log is off. Lines written with the log on are unchanged. A train standing on its track is posed every tick. A car that is already still at its pose (same entity as last placed, within a millionth of a block, zero velocity, not a Mob) is no longer teleported and stopped again. Gravity, deck placement and the body turn still run every tick. A moved or pushed car fails the check and snaps back as before. The first pose after loading or binding is always applied. BoneRotator.rotateToTarget skips a call that repeats the last call that left the bone unchanged, from that same rotation. The turn depends only on its arguments and the rotation, so the skipped call is a no-op. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Settled turn cache and validation src/main/java/net/tfminecraft/vehicleframework/bones/BoneRotator.java, src/test/java/net/tfminecraft/vehicleframework/bones/BoneRotatorSettleTest.java |
rotateToTarget returns a stored result when the call inputs and rotation state match a settled call. Tests cover matching calls, changed inputs, external rotation and drive yaw. |
Disabled logging
| Layer / File(s) | Summary |
|---|---|
Persistence log guards and tests src/main/java/net/tfminecraft/vehicleframework/database/PersistenceLog.java, src/test/java/net/tfminecraft/vehicleframework/database/PersistenceLogQuietTest.java |
Persistence logging methods return early when logging is disabled or the log file is unset. Tests cover disabled logging, spawn-cycle updates and selected enabled log output. |
Recorder log guards and tests src/main/java/net/tfminecraft/vehicleframework/tracks/RecorderLog.java, src/test/java/net/tfminecraft/vehicleframework/tracks/RecorderLogQuietTest.java |
Recorder logging methods return early when logging is disabled or the log file is unset. Tests cover disabled and enabled logging. |
Standing train placement
| Layer / File(s) | Summary |
|---|---|
Pose tracking and conditional placement src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java, src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainStandingPoseTest.java |
applyPose skips teleporting and clearing velocity when the same entity remains at the target pose and is still. Consist changes and unbinding clear the cached entity. Tests cover movement, displacement, entity changes and mob cars. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Refactor
Possibly related PRs
- TF-Minecraft/VehicleFramework#81: Changes
BoneRotatorheading state by recalculatingdriveYaw, which is part of the settled-call cache check in this change.
Suggested reviewers: ryanbarlow97
Merge Risk: 
Minimal · up to 5cc37
-
The reviewed changes are mergeable after normal checks; no actionable risk remains identified.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Dev test (TFMCDev01, 10-29 13:26 UTC, this PR's jar; Two parked consists near 5945,239,5229 (3 train bases tracked from a bot's view):
So standing cars stay put, no longer send a zero-velocity packet every tick, and still snap back at once after an outside move. |
|
@coderabbitai review |
✅ Action performed
|
Why
VehicleManager's fast tick runs every loaded vehicle on every tick. Two things did work there that changed nothing:RecorderLog.posebuilt a"pose:"+uuidkey and did a map lookup for each car on each tick. Up to 4 times a second it also did about 9String.formatcalls plustrain(v).PersistenceLog.applyPosebuilt a long line fromxyz()of 3 locations (each one callingloc.getChunk()twice) andvehicle(). Both lines were then thrown away.placeCars,particleOffset,playbackand the rest followed the same pattern.placeLoadedCars→applyPlacements→applyPoseteleported each car, zeroed its velocity and ran the quaternion turn every tick, even when nothing had moved.Changes
RecorderLog/PersistenceLog: a privateoff()(!enabled || logFile == null).pose,sample,playback,interact,arm,junction,placeCars,applyPose,particleOffset,spawn*,save*,loadJson,tryLink,mount,remountanddismountnow return before building anything. Two calls keep their side effects:spawnManagerStartstill advances the spawn cycle, andunloadstill clears the throttle maps. The formattersxyz/vehicle/trainare unchanged.TrainHandler.applyPose: the teleport andsetVelocity(0)are skipped only when all of these hold:Mob.setGravity(false), deck placement, the body turn, the logs and the track crumbs still run every tick.posedEntityis cleared inapplyConsistandunbind, so the first pose after a load or a bind is always applied in full.BoneRotator.rotateToTarget: the old body moved into a privateturnToTarget. The wrapper remembers the last call that left the rotation bit-for-bit unchanged. When the same call comes again (same arguments, same rotation, anddriveYawalready at the target yaw), it returns the remembered result without doing the turn.Why behaviour is identical
Logs: the code after each guard is unchanged. A golden test checks exact line text for
POSE,SAMPLE,PLAY,INTERACT,ARM,JUNCTION,SPAWN_MANAGER_START,PLACE_CARS,APPLY_POSE,SMOKE_OFFSET,SPAWN_LOADandUNLOAD, and it passes against bothorigin/mainand this branch.n=counters start from 0 instead of counting the ticks when it was off.Location.getChunk(), which could read a chunk that is not loaded. See Notes.Teleport skip: the target comes from
(spline, s), not from the car's current position. A standing car's target is therefore bit-identical every tick, and the most a car can differ from its target is 1e-6 blocks; there is no drift that builds up over time. The car is still put back when:teleport(next)keeps the entity's own yaw and pitch, so teleporting a still car that is already atnextchanges nothing on an ArmorStand base, the default. Mob bases are always teleported, because there the teleport also brings the body back in line with the head.Rotation skip:
turnToTargetdepends only on its arguments, the animator's rotation anddriveYaw. If one call left the rotation unchanged, the same call from the same state leaves it unchanged again. A test checks that a fresh rotator, run in full from the settled rotation, gives the same result.Testing
TrainStandingPoseTest: standing cars are placed once and then left alone; the first placement applies even when a car is already at its pose; a pushed car snaps back, even after a 1e-4 push; a car set moving is stopped; a respawned entity, anapplyConsistand anunbind+bind each trigger a full placement; Mob cars are always teleported; the deck is placed every tick; a moving train is teleported every tick.BoneRotatorSettleTest: settles across yaws including ±180; the skipped turn equals the full turn; an outside turn is put back; a re-readdriveYawis restored; other turns are not skipped.RecorderLogQuietTest/PersistenceLogQuietTest: with the log off,verifyNoInteractionson the vehicle and locations; with the log on, the golden lines above.origin/main, as expected.mvn -B -o clean verify: 1815 tests, 0 failures, 9 skipped. The coverage gate misses the same 10 lines as theorigin/mainbaseline (VehicleRepository, TrackStore and template loaders, all covered by tests that are skipped on Windows). Linux CI is authoritative.Notes
Location.getChunk(). Onorigin/mainthey did this on every tick, including for the pose target, which can lie in the next chunk ahead of a moving train. No gameplay effect is expected, but trains running at the edge of loaded chunks are worth watching on Dev.🤖 Generated with Claude Code