Skip to content

feat(physics): add boat and horse prediction with minecart riding - #9

Merged
zardoy merged 19 commits into
zardoy:gen-the-masterfrom
sandexzx:feat/boat-physics-1.17.1
Aug 2, 2026
Merged

zardoy merged 19 commits into
zardoy:gen-the-masterfrom
sandexzx:feat/boat-physics-1.17.1

Conversation

@sandexzx

@sandexzx sandexzx commented Jul 12, 2026 •

Copy link
Copy Markdown

Add locally predicted boat and horse control, plus server-authoritative minecart riding

Context

Mineflayer previously treated mounted entities as server-driven objects. That is
insufficient for responsive vehicle control: a controlling passenger of a boat or
a horse must simulate movement locally, send input/paddle state, and reconcile
authoritative server corrections. Minecarts, by contrast, are driven entirely by
the server and only need the rider to follow along accurately.

Root causes addressed

  • Passenger lifecycle relied on local assumptions instead of authoritative
    set_passengers packets.
  • Duplicate entity objects could prevent controller detection when passenger
    identity was compared by object reference.
  • Routine entity broadcasts could overwrite locally predicted vehicle movement
    and cause visible jitter.
  • Player and vehicle state could diverge during acceleration.
  • Correction packets needed explicit acknowledgement and simulation rebasing.
  • Remote (non-controlled) vehicles and their passengers were not observed
    correctly (attach events and rider look/rotation).

Implementation

Shared

  • Make set_passengers authoritative for mount and dismount state.
  • Detect the controlling passenger by entity ID.
  • Keep the player synchronized with its current vehicle.
  • Ignore routine relative movement, teleport, and velocity broadcasts for
    locally controlled vehicles.
  • Expose bot.vehicle, bot.moveVehicle(left, forward, jump), and a
    vehicleCorrection event; expose boat/horse physics status for rendering
    integration.

Boats (locally predicted, 1.17.1)

  • Create and clean up a BoatPhysics context for controlled boats.
  • Map Mineflayer controls to boat steering and paddle state.
  • Simulate controlled boats each physics tick and send finite vehicle_move
    updates using protocol-degree yaw/pitch.
  • Accept authoritative vehicle_move corrections, rebase simulation, and
    acknowledge them immediately.
  • Disable local boat prediction after repeated rapid corrections.

Horses (locally predicted, 1.17.1)

  • Create and clean up a HorsePhysics context for saddled, ridden horses.
  • Gate control on saddle flag and controlling-passenger ID.
  • Simulate rider input (movement + jump charge) each tick, send updates, and
    reconcile vehicle_move corrections the same way as boats, including the
    correction-limit safeguard and attribute updates.

Minecarts (server-authoritative, any version)

  • Follow the server-driven minecart position via entityMoved and keep the
    rider in sync; no local simulation is performed.

Observation

  • Emit attach events for remote vehicle passengers and handle rider look /
    rotation so other players' vehicles are observed correctly.

  • Preserve existing behavior for unsupported and legacy protocol versions.

Testing

  • npx mocha test/vehiclePhysicsTest.js — 19 passing.
  • Regression suites added:
    • vehiclePhysicsTest.js / vehicleLifecycleTest.js — boat movement, paddle
      packets, corrections, mount/dismount, duplicate passengers, first- vs
      second-passenger control, cleanup on dismount/removal/respawn, fallback to
      normal player physics, legacy behavior.
    • horsePhysicsTest.js / horseLifecycleTest.js / entityPhysicsHorseTest.js
      — horse rider input, jump charge, corrections, lifecycle, and horse entity
      physics (including non-1.17.1 versions).
    • minecartLifecycleTest.js — server-authoritative minecart mount, sync, and
      dismount.
    • entityLookTest.js — remote passenger look / rotation observation.

Related PRs

Scope

Local prediction is enabled only for the explicitly supported Minecraft 1.17.1
boat and horse paths; other protocol versions retain their previous behavior for
these. Minecart riding is server-authoritative and version-neutral.

Why 1.17.1 only (for now): the limit comes from the boat path, not from a
general design constraint:

  • Boat protocol/input handling — paddle state is sent with steer_boat,
    while mounted movement input uses steer_vehicle on older versions and the
    unified player_input on 1.21.3+. Although vehicle_move remains available,
    supporting additional versions still requires version-aware input handling
    and validation.
  • Boat physics model — the simulation reproduces 1.17.1 water/buoyancy
    behavior, which vanilla changed across versions, so it would drift elsewhere.
  • Horse — the horse simulation itself matches current (1.21.x) vanilla and is
    not inherently tied to 1.17.1; it is gated to 1.17.1 only because it shares the
    same version-locked correction plumbing as the boat and was validated there.

Broadening version support is a follow-up: it requires version-aware input
handling, plus per-version validation of boat physics and correction behavior.
Minecart riding already works across versions because it is
server-authoritative.

Summary by CodeRabbit

  • New Features

    • Added local physics support for controlling boats and saddled horses on Minecraft 1.17.1.
    • Improved vehicle movement, steering, jumping, mounting, and dismounting behavior.
    • Added synchronization for passengers transferring between vehicles.
    • Added vehicle state access through the bot API and optional jump control for moveVehicle.
  • Bug Fixes

    • Prevented conflicting server updates from overriding locally controlled vehicles.
    • Improved dismount positioning and minecart lifecycle handling.
    • Preserved correct entity rotation updates when packet values are zero or unavailable.

sandexzx added 6 commits July 12, 2026 19:39
Treat set_passengers as authoritative for mount state and keep dismount
requests active until the server confirms them.
Add local boat prediction, movement packets, server correction rebasing,
and lifecycle cleanup for the controlling passenger.
Compare passengers by entity ID and preserve the canonical bot entity when
world switches leave a stale passenger reference.
Keep local simulation authoritative for routine entity broadcasts while preserving exact vehicle_move acknowledgements and rebases. Add regression coverage for correction and passenger-control paths.
Allow rendering clients to derive water-mask visibility from the authoritative local boat simulation state. Add vehicle physics coverage for the status accessor.
Preserve the upstream physics pause for ordinary vehicles while allowing the locally predicted 1.17.1 boat path to keep ticking. Populate boat fixtures for the new generic entity-physics plugin introduced by the updated base.
@coderabbitai

coderabbitai Bot commented Jul 12, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds version-specific controlled boat and horse synchronization and local physics for 1.17.1, updates passenger and dismount handling, exposes vehicle and rideable-physics state, and adds lifecycle, entity-look, minecart, boat-physics, and horse-physics coverage.

Changes

Controlled Rideable Support

Layer / File(s) Summary
Entity and passenger synchronization
lib/plugins/entities.js
Filters controlled vehicle broadcasts, processes vehicle_move corrections, reconciles passengers incrementally, and adds timed dismount steering state.
Boat and horse physics integration
lib/plugins/physics.js
Adds controlled boat and horse simulation, correction handling, packet emission, lifecycle cleanup, dismount offsets, and runtime accessors.
Entity physics eligibility rules
lib/plugins/entity_physics.js
Excludes rideable horses and controlled vehicles from applicable entity simulation and exports horse classification.
Vehicle state and control declarations
index.d.ts
Adds Bot.vehicle, optional jump input for moveVehicle, and boat and horse physics hooks.
Vehicle lifecycle validation
test/vehicleLifecycleTest.js, test/minecartLifecycleTest.js
Tests passenger synchronization, lifecycle events, steering masks, dismount positioning, minecart behavior, and packet selection.
Boat physics validation
test/vehiclePhysicsTest.js, test/entityPhysicsVehicleTest.js
Tests 1.17.1 boat simulation, correction handling, broadcast filtering, context lifecycle, generic-physics exclusion, and legacy 1.18.2 behavior.
Horse physics and lifecycle validation
test/entityPhysicsHorseTest.js, test/horseLifecycleTest.js, test/horsePhysicsTest.js
Tests horse simulation eligibility, mounting, packet output, jumps, corrections, seat anchoring, and lifecycle cleanup.
Entity-look regression coverage
test/entityLookTest.js
Tests entity-look conversion, null handling, and entityMoved emission.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Server
  participant EntityHandlers
  participant PhysicsPlugin
  participant BoatPhysics
  participant HorsePhysics
  Server->>EntityHandlers: Send vehicle_move or entity update
  EntityHandlers->>PhysicsPlugin: Emit vehicleCorrection
  PhysicsPlugin->>BoatPhysics: Rebase or simulate controlled boat
  PhysicsPlugin->>HorsePhysics: Rebase or simulate controlled horse
  BoatPhysics->>PhysicsPlugin: Return boat state
  HorsePhysics->>PhysicsPlugin: Return horse state
  PhysicsPlugin->>Server: Send controlled vehicle movement packet
Loading

Suggested reviewers: generelschwerz

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change set: boat and horse prediction plus minecart riding support.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
lib/plugins/entities.js (1)

424-436: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Remove the unreachable emitBoatVehicleCorrection calls

entity_velocity, rel_entity_move, entity_move_look, and entity_teleport all return early for isControlledBoat(entity), while emitBoatVehicleCorrection() only emits when isControlledBoat(entity) is true. These calls can never fire, so only vehicle_move can reach vehicleCorrection.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/plugins/entities.js` around lines 424 - 436, Remove the unreachable
emitBoatVehicleCorrection call from the entity_velocity handler and also remove
equivalent calls from the rel_entity_move, entity_move_look, and entity_teleport
handlers. Preserve vehicle correction emission only through the vehicle_move
path, which can reach controlled boats.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/plugins/physics.js`:
- Around line 637-695: Emit entityPhysicsTick from the mounted vehicle path as
well as the existing player path, after vehicle physics synchronization
completes. Update the vehicle branch around syncEntityWithVehicle and
clampPassengerYawRelativeToBoat so tracked non-player entities continue
advancing while mounted, without removing the existing emission for unmounted
physics.

---

Nitpick comments:
In `@lib/plugins/entities.js`:
- Around line 424-436: Remove the unreachable emitBoatVehicleCorrection call
from the entity_velocity handler and also remove equivalent calls from the
rel_entity_move, entity_move_look, and entity_teleport handlers. Preserve
vehicle correction emission only through the vehicle_move path, which can reach
controlled boats.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: be2c1512-4869-480e-9cf8-346802295193

📥 Commits

Reviewing files that changed from the base of the PR and between 5f9c940 and 6a18d80.

📒 Files selected for processing (4)
  • lib/plugins/entities.js
  • lib/plugins/physics.js
  • test/vehicleLifecycleTest.js
  • test/vehiclePhysicsTest.js

Comment thread lib/plugins/physics.js
@zardoy

zardoy commented Jul 12, 2026

Copy link
Copy Markdown
Owner

@sandexzx
Does it mean that minecart and horse riding should now work without any problems as well? It sounds like it should. Though iirc minecarts have their own sim logic in vanilla.

sandexzx added 5 commits July 12, 2026 22:36
Keep remote passenger vehicle state in sync when handling
set_passengers packets and emit entityAttach/entityDetach events
after the corresponding state changes.
Track minecart movement from server entity updates and keep the mounted
player position synchronized without sending client-authoritative
vehicle movement packets.

Preserve steering and dismount input, synchronize camera movement without
a physics-tick delay, and handle zero-valued entity look angles correctly.
Remove vestigial correction calls from entity movement handlers that
already ignore the locally controlled boat.

Keep vehicle_move as the authoritative server correction path without
changing minecart or remote entity handling.
Add local horse physics lifecycle, rider control synchronization,
jump charging, vehicle corrections, and vanilla passenger positioning

Send horse movement using the vanilla packet order and protocol-degree
rotation while preserving boat and minecart authority behavior.
Keep remote horses server-authoritative and preserve safe boat dismount offsets across repeated server position corrections.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/plugins/physics.js (1)

1114-1146: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Convert legacy vehicle_move angles to notchian degrees
bot.vehicle.yaw / bot.vehicle.pitch are radians here, while vehicle_move packets are handled as notchian degrees elsewhere, so this fallback should match the boat/horse senders and convert both angles before writing.

🔧 Suggested fix
   function sendPacketVehicleMove () {
     sendPacketSteerVehicle()

     const vehicle = bot.vehicle
     const x = vehicle.position.x
     const y = vehicle.position.y
     const z = vehicle.position.z
-    const yaw = vehicle.yaw
-    const pitch = vehicle.pitch
+    const yaw = Math.fround(conv.toNotchianYaw(vehicle.yaw))
+    const pitch = Math.fround(conv.toNotchianPitch(vehicle.pitch))
     if (![x, y, z, yaw, pitch].every(Number.isFinite)) return

     bot._client.write('vehicle_move', { x, y, z, yaw, pitch })
   }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/plugins/physics.js` around lines 1114 - 1146, Update
sendPacketVehicleMove to convert vehicle.yaw and vehicle.pitch from radians to
notchian degrees before the finite-value check and vehicle_move packet write,
matching the angle conversion used by the boat and horse senders while leaving
position handling unchanged.
🧹 Nitpick comments (5)
test/entityPhysicsHorseTest.js (1)

10-120: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test-infra helpers duplicated across three new horse test files.

createBlockWorldStub, stubLoadedWorld, loginBot, and the teardown logic are copy-pasted nearly verbatim in test/horseLifecycleTest.js and test/horsePhysicsTest.js. See consolidated comment.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/entityPhysicsHorseTest.js` around lines 10 - 120, Extract the duplicated
helpers createBlockWorldStub, stubLoadedWorld, loginBot, setupBotTest,
withLogin, and teardownBotTest from the horse test files into a shared test
utility module, then update entityPhysicsHorseTest.js, horseLifecycleTest.js,
and horsePhysicsTest.js to import and reuse them while preserving their existing
behavior.
test/horsePhysicsTest.js (1)

10-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test-infra helpers duplicated across three new horse test files.

Same createBlockWorldStub/stubLoadedWorld/loginBot/teardown boilerplate as in test/entityPhysicsHorseTest.js and test/horseLifecycleTest.js. See consolidated comment.

Also applies to: 123-142

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/horsePhysicsTest.js` around lines 10 - 91, Extract the duplicated
helpers createBlockWorldStub, stubLoadedWorld, loginBot, captureWrites,
setupHorse, and withLogin into the existing shared horse-test utility used by
entityPhysicsHorseTest.js and horseLifecycleTest.js. Update this test file to
import and reuse those shared symbols, removing its local copies while
preserving their current behavior.
test/horseLifecycleTest.js (1)

9-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test-infra helpers duplicated across three new horse test files.

Same createBlockWorldStub/stubLoadedWorld/loginBot/teardown boilerplate as in test/entityPhysicsHorseTest.js and test/horsePhysicsTest.js. See consolidated comment.

Also applies to: 106-125

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/horseLifecycleTest.js` around lines 9 - 45, Consolidate the duplicated
createBlockWorldStub, stubLoadedWorld, and loginBot helpers from
test/horseLifecycleTest.js with the shared test helpers already used by
test/entityPhysicsHorseTest.js and test/horsePhysicsTest.js. Update this test to
import and reuse the common helpers, removing its local definitions while
preserving existing behavior.
lib/plugins/entity_physics.js (1)

8-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate RIDEABLE_HORSE_ENTITY_NAMES definitions in lib/plugins/entity_physics.js and lib/plugins/physics.js. Both files independently declare the same horse-name set backing equivalent predicates; a future horse variant (or a typo fix) would need to be updated in both places to stay consistent.

  • lib/plugins/entity_physics.js#L8-L18: keep as the canonical definition (it already exports isRideableHorseEntityName).
  • lib/plugins/physics.js#L53-L59: drop the local RIDEABLE_HORSE_ENTITY_NAMES/isRideableHorseEntity and import isRideableHorseEntityName from entity_physics.js, adapting call sites (isRideableHorseEntity(vehicle) → isRideableHorseEntityName(vehicle?.name)).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/plugins/entity_physics.js` around lines 8 - 18, The canonical
RIDEABLE_HORSE_ENTITY_NAMES and isRideableHorseEntityName in
lib/plugins/entity_physics.js require no direct change. In
lib/plugins/physics.js lines 53-59, remove the duplicate set and
isRideableHorseEntity definition, import isRideableHorseEntityName from
entity_physics.js, and update every call site to pass vehicle?.name instead of
the vehicle object.
lib/plugins/physics.js (1)

538-628: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Horse context lifecycle closely mirrors the boat context lifecycle (lines 727-758) with near-identical create/ensure/destroy/disable/correction logic duplicated across the two vehicle types.

This isn't incorrect, but the copy-paste pattern is exactly what produced the sign divergence flagged below in offsetPlayerFromHorse vs offsetPlayerFromBoat. Consider extracting a shared "controlled-vehicle-context" helper (context create/ensure/destroy, correction-count tracking with disable-after-N-corrections, disable-with-log) parameterized by the physics/state classes, and have both boat and horse call into it, keeping only the genuinely-divergent bits (paddle vs jump-charge, saddle check, etc.) specialized.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/plugins/physics.js` around lines 538 - 628, Extract the duplicated
controlled-vehicle lifecycle logic from the horse functions createHorseContext,
ensureHorseContext, destroyHorseContext, resetHorseCorrectionTracking,
disableHorsePhysicsWithLog, and applyPendingHorseCorrection into a shared
parameterized helper reusable by the boat context. Pass vehicle validation,
physics/state construction, entity setup, and vehicle-specific correction
behavior as parameters, while keeping saddle and jump-charge handling in
horse-specific code and paddle handling in boat-specific code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/plugins/physics.js`:
- Around line 1803-1809: Update the bot.on('mount') handler so unsupported
mounts do not disable legacy vehicle packet processing: preserve
shouldUsePhysics for boat/horse mounts that are not new controller cases, while
retaining the existing enablement for isBoatController(), isHorseController(),
and isServerAuthoritativeMinecart(). Ensure updatePosition() and physicsTick()
continue running so their legacy steer_boat and vehicle_move paths remain
active.

In `@test/horseLifecycleTest.js`:
- Around line 47-52: Update withLogin to wait for the initial forcedMove event
after the bot’s login before invoking runTest. Mirror the sequencing used by
sibling horse tests, preserving the existing promise completion and error
handling through done.

In `@test/horsePhysicsTest.js`:
- Around line 156-164: Update the “does not control horse without saddle” test
to await or trigger a physicsTick after setupHorse before asserting
_horsePhysics.getCtx() is null, matching the sibling saddled-passenger test.
Keep the unsaddled setup and null-context assertion so the test exercises the
saddle-gating behavior rather than only the pre-tick state.

---

Outside diff comments:
In `@lib/plugins/physics.js`:
- Around line 1114-1146: Update sendPacketVehicleMove to convert vehicle.yaw and
vehicle.pitch from radians to notchian degrees before the finite-value check and
vehicle_move packet write, matching the angle conversion used by the boat and
horse senders while leaving position handling unchanged.

---

Nitpick comments:
In `@lib/plugins/entity_physics.js`:
- Around line 8-18: The canonical RIDEABLE_HORSE_ENTITY_NAMES and
isRideableHorseEntityName in lib/plugins/entity_physics.js require no direct
change. In lib/plugins/physics.js lines 53-59, remove the duplicate set and
isRideableHorseEntity definition, import isRideableHorseEntityName from
entity_physics.js, and update every call site to pass vehicle?.name instead of
the vehicle object.

In `@lib/plugins/physics.js`:
- Around line 538-628: Extract the duplicated controlled-vehicle lifecycle logic
from the horse functions createHorseContext, ensureHorseContext,
destroyHorseContext, resetHorseCorrectionTracking, disableHorsePhysicsWithLog,
and applyPendingHorseCorrection into a shared parameterized helper reusable by
the boat context. Pass vehicle validation, physics/state construction, entity
setup, and vehicle-specific correction behavior as parameters, while keeping
saddle and jump-charge handling in horse-specific code and paddle handling in
boat-specific code.

In `@test/entityPhysicsHorseTest.js`:
- Around line 10-120: Extract the duplicated helpers createBlockWorldStub,
stubLoadedWorld, loginBot, setupBotTest, withLogin, and teardownBotTest from the
horse test files into a shared test utility module, then update
entityPhysicsHorseTest.js, horseLifecycleTest.js, and horsePhysicsTest.js to
import and reuse them while preserving their existing behavior.

In `@test/horseLifecycleTest.js`:
- Around line 9-45: Consolidate the duplicated createBlockWorldStub,
stubLoadedWorld, and loginBot helpers from test/horseLifecycleTest.js with the
shared test helpers already used by test/entityPhysicsHorseTest.js and
test/horsePhysicsTest.js. Update this test to import and reuse the common
helpers, removing its local definitions while preserving existing behavior.

In `@test/horsePhysicsTest.js`:
- Around line 10-91: Extract the duplicated helpers createBlockWorldStub,
stubLoadedWorld, loginBot, captureWrites, setupHorse, and withLogin into the
existing shared horse-test utility used by entityPhysicsHorseTest.js and
horseLifecycleTest.js. Update this test file to import and reuse those shared
symbols, removing its local copies while preserving their current behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f0f975fd-2fdf-4ce8-8a14-2ab6ef49a35f

📥 Commits

Reviewing files that changed from the base of the PR and between e7549bd and d539646.

📒 Files selected for processing (8)
  • index.d.ts
  • lib/plugins/entities.js
  • lib/plugins/entity_physics.js
  • lib/plugins/physics.js
  • test/entityPhysicsHorseTest.js
  • test/horseLifecycleTest.js
  • test/horsePhysicsTest.js
  • test/vehicleLifecycleTest.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/plugins/entities.js

Comment thread lib/plugins/physics.js
Comment thread test/horseLifecycleTest.js
Comment thread test/horsePhysicsTest.js
@sandexzx sandexzx changed the title feat(physics): add locally predicted boat control for 1.17.1 feat(physics): add boat and horse prediction with minecart riding Jul 15, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
test/entityPhysicsVehicleTest.js (3)

134-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Await physicsTick to guarantee post-simulation state.

While awaiting physicsTickBegin works here due to Node's microtask queue (the synchronous physics tick completes before the await continuation resumes), it is semantically incorrect and fragile.

Await the physicsTick event instead to explicitly express the intent of checking state after the physics simulation boundary has completed, aligning with upstream testing patterns.

♻️ Proposed refactor
-  await once(bot, 'physicsTickBegin')
+  await once(bot, 'physicsTick')
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/entityPhysicsVehicleTest.js` at line 134, Replace the await on the
physicsTickBegin event in the vehicle physics test with an await on physicsTick,
ensuring state assertions occur after the simulation completes and matching the
established post-simulation event pattern.

101-110: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Clear timeout to prevent leaks and ensure the server closes on timeout.

The current teardown logic has two issues:

  1. The 3000ms timeout is not cleared when the test finishes early, leaving a lingering timer in the event loop.
  2. If the test hangs and the timeout fires, finish() is called, completing the test without closing the server, which leaks the server port for subsequent tests.

Please clear the timeout in finish() and ensure the server is forcefully closed if the timeout triggers.

🛠️ Proposed refactor
 function teardownBotTest (bot, server, done) {
   let finished = false
+  let timer
   const finish = () => {
     if (finished) return
     finished = true
+    clearTimeout(timer)
     done()
   }
-  setTimeout(finish, 3000)
+  timer = setTimeout(() => {
+    if (server) try { server.close() } catch (e) {}
+    finish()
+  }, 3000)
   if (bot && !bot._client.ended) {
     bot.once('end', () => server.close(() => finish()))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/entityPhysicsVehicleTest.js` around lines 101 - 110, Update
teardownBotTest so the timeout handle is cleared inside finish() before calling
done(), and make the timeout path forcefully close the server before completing.
Preserve the existing bot end-event shutdown path and ensure finish() remains
idempotent while avoiding double-closing the server.

93-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid mutating the shared registry packet.

Modifying registry.loginPacket.entityId directly mutates the cached object in the prismarine-registry instance. This can cause cross-test contamination if subsequent tests expect the original defaults. Use object spread to create a new packet instead.

♻️ Proposed refactor
   bot.test.generateLoginPacket = () => {
-    const loginPacket = registry.loginPacket
-    loginPacket.entityId = 0
-    return loginPacket
+    return { ...registry.loginPacket, entityId: 0 }
   }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/entityPhysicsVehicleTest.js` around lines 93 - 97, Update
bot.test.generateLoginPacket to avoid mutating the shared registry.loginPacket
object; create and return a new packet via object spread with entityId set to 0,
while preserving the existing packet defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/entityPhysicsVehicleTest.js`:
- Line 134: Replace the await on the physicsTickBegin event in the vehicle
physics test with an await on physicsTick, ensuring state assertions occur after
the simulation completes and matching the established post-simulation event
pattern.
- Around line 101-110: Update teardownBotTest so the timeout handle is cleared
inside finish() before calling done(), and make the timeout path forcefully
close the server before completing. Preserve the existing bot end-event shutdown
path and ensure finish() remains idempotent while avoiding double-closing the
server.
- Around line 93-97: Update bot.test.generateLoginPacket to avoid mutating the
shared registry.loginPacket object; create and return a new packet via object
spread with entityId set to 0, while preserving the existing packet defaults.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8f11c3e7-5422-41d4-99f6-77f2134cc918

📥 Commits

Reviewing files that changed from the base of the PR and between d539646 and 971222e.

📒 Files selected for processing (6)
  • lib/plugins/entity_physics.js
  • lib/plugins/physics.js
  • test/entityPhysicsVehicleTest.js
  • test/horseLifecycleTest.js
  • test/horsePhysicsTest.js
  • test/vehiclePhysicsTest.js
🚧 Files skipped from review as they are similar to previous changes (4)
  • test/horseLifecycleTest.js
  • test/vehiclePhysicsTest.js
  • lib/plugins/physics.js
  • test/horsePhysicsTest.js

sandexzx added 7 commits July 16, 2026 19:02
Add getPaddleState() to the boat physics API so consumers can reuse
the same paddle state that is sent in steer_boat packets.

Return null when no local boat physics context is active and cover
the vanilla forward, backward, left, and right control mappings.
Mineflayer must start when the physics util lacks horse exports (boats-only fork).
Detect transport capabilities at inject time and keep legacy boat behavior when local
physics is unavailable.
Run all export scenarios in one child process, add stub horse exports for
full-transport checks, verify warn-once across mount/dismount/remount, and
use fast bot/server teardown to avoid 20s hangs per scenario.
Split Minecraft version checks from transport capability checks so
server-authoritative unsaddled horse sync still runs on 1.17.1 when
HorsePhysics/HorseState are absent. Local context and corrections
continue to require the full export set.
Ignore entity_head_rotation for locally controlled vehicles so server
echoes do not steer the horse head sideways during fast turns.
@zardoy
zardoy merged commit 1aec6ff into zardoy:gen-the-master Aug 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants