feat(simulation): add hardened liquid movement - #2
Merged
Conversation
Audits the liquid-simulation branch against oomph-ac/oomph PR #145 at its head commit 0bcbb8be25593f836a66ee6a4e302d4fb81fd2bb and closes the gaps found, then covers the whole feature with tests. Implementation fixes: - Add the swimming hitbox. While Swimming, BoundingBox and ClientBoundingBox collapse the height to the entity width, matching the client's swim pose. This was missing entirely and affects collisions, liquid detection and the liquid exit probe, not just liquid travel. - Restore edge-triggered jumping. state.Jumping was widened to StartJumping || Jumping, which re-armed a ground jump on every tick the jump key was held. Upstream is StartJumping only; the held-key and automatic ascent cases are already carried by EffectiveJumping. This was a regression in non-liquid movement. - Add DolphinBoostTicks and count it down in tickState, resetting SwimSpeedMultiplier to its default on expiry, matching upstream. Callers previously had to manage boost expiry themselves. - Only clear Gliding and GlideBoostTicks when actually gliding, so a pending glide boost survives water contact as it does upstream. - Revert the sneak impulse clamp to its v0.1.3 form. The WantDown/WantDownSlow exception had no basis in the source and changed non-liquid movement. - Route liquid flow's collision lookups through a nil-safe helper so an absent world reads as empty space rather than panicking. - Fix the climb debug label to report effectiveJumping, matching the branch condition. Documents in liquid.go the deliberate divergences from the source: float64 arithmetic, LiquidType matching, the LiquidProvider layer-1 adaptation, zero-means-default speed fields, nil effects tolerance, and why the impulse clamps are retained. Adds liquid_test.go: 107 tests covering water and lava detection across both layers, the swim hitbox, effective jump/ascend/descend inputs, acceleration and drag, gravity, levitation, depth strider, dolphin boost, flow including falling liquid and solid faces, collision and exit probing, gliding, ladder climbing, reliability, determinism, and the provider fallback paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses an independent review of the liquid port. The physics remain a 1:1 port of oomph-ac/oomph PR #145; these changes cover the gap between a proxy-embedded simulator that can trust its anticheat and a standalone authoritative library that cannot. Critical: bound water travel on server evidence. Upstream gates water travel on `touchingWater || Swimming`, and sizes the hitbox off the same flag. Since Swimming comes straight from the client, a latched flag granted indefinite zero-gravity hovering in open air with no correction raised, and shrank the server-side hitbox from 1.8 to 0.6 so the player fit through gaps a standing player cannot. Verified against the previous commit: 60 ticks of spoofed swimming in a dry world ended with velocity 0 and zero altitude lost. Both the water-travel branch and the swim pose now require recent server-observed water contact, bounded by SwimWaterGraceTicks (default 10). The budget refills only on real contact, is clamped to its bound before anything reads it, and is cleared on every frame that was not simulated: unreliable, unloaded chunk, immobile, and teleport. Lava the player is actually standing in takes priority over a retained water grace. The budget is constant across a tick, so collision, liquid detection and exit probing always agree on one hitbox; the cost is that entering water adopts the swim pose one tick later than upstream, erring toward the larger box. Important: make liquid-layer support explicit and checkable. Layer-1 liquids were discovered only by type-asserting World, so a missing or mistyped implementation silently mis-simulated waterlogged blocks as dry. Adds an explicit Simulator.Liquids field, HasLiquidLayer, and an opt-in RequireLiquidLayer that fails closed with SimulationOutcomeUnreliable. The World type assertion still works as a fallback for existing integrations. Important: replace tests that did not distinguish behavior. - TestFlyingSkipsLiquidPhysics was vacuous, since simulationIsReliable exits before physics. Split into a test for that contract and one exercising the liquid gate directly. - TestLiquidSimulationIsDeterministic compared the implementation to itself. Replaced by a pinned golden, plus a repeat-run check for nondeterminism. - TestLiquidExitProbeBlockedByWall had liquid in the probe box, so it could not attribute the result. Rebuilt to isolate the collision term, verifying the probe box is liquid-free. Adds coverage for stairs solid-face flow blocking, the nil-world guard, the +8 drop weight and -6 falling magnitude, swim-hitbox collision interaction, and the DefaultSwimWaterGraceTicks literal. Each new security branch was confirmed by mutation to fail a test when removed. Also: surfaces the sneak/consuming impulse-clamp divergence in the README and adds SimulationOptions.UpstreamImpulseClamping to opt into upstream's behavior without a breaking change; captures EffectiveJumping before updateSwimTravel to match upstream ordering; replaces the stringly liquid type with an internal liquidKind; documents the defensive depth-strider and nil-world adaptations; deprecates the unused MaxNormalizedImpulse; and drops a nil check on a type assertion that could never fire. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe simulator now supports water and lava movement physics, layered liquid providers, swimming state and pose handling, Depth Strider and dolphin effects, water-contact grace, liquid flow, and configurable reliability and impulse-clamping behavior. Tests cover physics, transitions, hardening cases, determinism, and golden outcomes. ChangesLiquid movement simulation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Simulator
participant LiquidProvider
participant MovementState
Client->>Simulator: submit movement and swimming inputs
Simulator->>LiquidProvider: query liquid contacts and second-layer liquids
LiquidProvider-->>Simulator: return water or lava data
Simulator->>MovementState: update grace, velocity, flow, and pose state
Simulator-->>Client: return simulated movement outcome
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
27 tasks
HashimTheArab
marked this pull request as ready for review
July 20, 2026 14:40
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@liquid_test.go`:
- Around line 106-112: Update layeredLiquidWorld.Liquid to call the promoted
Block method directly through w instead of explicitly selecting the embedded
liquidWorld field, preserving the existing type assertion and return 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 Plus
Run ID: bddbe93b-1468-4637-83b0-0b0a5af3bb4b
📒 Files selected for processing (11)
README.mdbbox.goconstants.goinput.gointerfaces.goliquid.goliquid_hardening_test.goliquid_test.gomovement.gosimulation.gosimulator.go
This was referenced Jul 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
0bcbb8be25593f836a66ee6a4e302d4fb81fd2bb.Why
Cinnabar and other bedsim consumers need a reusable, deterministic liquid oracle instead of maintaining client-specific liquid physics in each caller. The security hardening deliberately narrows the upstream behavior where an untrusted swimming flag could otherwise create indefinite air hovering.
Validation
go test ./... -count=3— 145 tests pass.go vet ./...— clean.git diff --check— clean.main..0202be6range; the final verdict wasAPPROVEwith no Critical or Important findings.Remaining risk
This is structurally aligned to the pinned Oomph PR, but bedsim uses
float64while Oomph usesfloat32, so it is not bit-identical. Native/consumer integration evidence remains separate from this library PR.Summary by CodeRabbit
New Features
RequireLiquidLayer,SwimWaterGraceTicks, andUpstreamImpulseClamping, including a dedicated liquids provider.Bug Fixes
Documentation
Tests