Water overhaul: liquid simulation, flow physics, rendering, interaction

[MichaelFisher1997]

Summary

A full voxel-water overhaul turning water from a visual surface into a real, Minetest/Luanti-inspired liquid system — plus a clean, responsive rendering and interaction pass.

Liquid system (src/game/liquid/)

• Source + flowing water block pair (id 7 source / id 29 flowing) with liquidType + shared LiquidDef (range, viscosity, renewable, swimmable, fog).
• Per-voxel level storage (Chunk.levels), mirrors the lighting LightMap.
• Bounded update queue with priority + normal lanes: player edits jump ahead of the chunk-gen ocean-seeding backlog, and an immediate priority burst on every block edit makes water react within a frame (fixed a multi-second flow delay caused by the FIFO backlog).
• Flow rules: down-first pour, horizontal decay, waterfall columns (no sideways plume), conservative renewable sources. Proven convergent (8/8 unit tests — no endless loops, stable oceans drain to queue 0).

Rendering (ChunkMesher, WaterMaterial)

• World-space UVs + a subtle animated procedural surface texture → continuous body, no per-block grid.
• Depth-write OFF (transparent best practice) so terrain behind water always renders cleanly (no black gaps); no bottom/internal faces emitted so the stacked-glass grid doesn't return.
• Partial-height flowing water with stepped shorelines; shared material; Low/Medium/High tiers.

Player & underwater (Player, UnderwaterRenderer)

• Swim physics (buoyancy/drag/swim-up), in-water + eye-submerged detection.
• Lerped underwater fog pull-in + DOM tint for stable surface crossings.

Interaction (BlockRaycaster, Player, Game)

• Raycast passes through water by default (Luanti liquids=false) so you can mine/build underwater; F4 toggles liquid-targeting to select/remove water itself. Break/place through water wired into the liquid queue.

Debug

• Perf overlay liquid + target lines (queue, priority queue, budget, ms-since-tick, writes, targeting mode).
• Console toggles: waterOpaque, waterDepth, waterSimple, waterSides, waterAnim, targetLiquids, plus placeWater/removeWater/liquidBudget/liquid.

Verification

• tsc --noEmit clean; vite build succeeds.
• Unit tests: liquid flow (8/8), raycast-through-water (5/5), responsiveness/priority (proves same-frame edit response vs backlog).

Notes

• No persistence of block/water edits yet (world regenerates from seed; deterministic).
• The in-place dist/ couldn't be rebuilt here due to root-owned stale Docker artifacts (EACCES); code builds cleanly to a fresh outDir. Unrelated to this change.

[github-actions]

📋 Summary

No linked issues (e.g., "Fixes #123") are mentioned in the PR description.

This PR overhauls water from a static transparent block into a Minetest/Luanti-style liquid system: source/flowing block pair, per-voxel levels, a bounded priority+normal update queue, partial-height water meshing with world-space UVs, swim physics, an underwater fog/tint renderer, raycast-through-water targeting, and a suite of debug toggles/overlays. The implementation is large but well-organized, and both the game and website build and typecheck cleanly.

📌 Review Metadata

• Reviewed Commit SHA: e449ecdcf166c653980567ea19306e58eae76e65
• Reviewed PR: Water overhaul: liquid simulation, flow physics, rendering, interaction #4

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified. bun run typecheck, bun run build, website/bun run typecheck, and website/bun run build all succeed.

⚠️ High Priority Issues (Should Fix)

[HIGH] src/game/World.ts:629-635 — Chunk unload only marks neighbours dirty for remesh, but does not wake the liquid simulator
Confidence: High
Description: When a chunk unloads, the code comments say it will "Re-seed liquid along the new world edge so neighbours of the unloaded chunk recompute their border flow." In practice it only calls markDirty on the four neighbours. markDirty flags a chunk for remesh, but it does not enqueue any cells in this.liquid. Boundary liquid cells in the neighbour chunks therefore never recompute their state; water that was flowing into the now-unloaded region stays as stale liquid blocks rather than drying up.
Impact: Visual and simulation inconsistency at chunk borders after streaming/unloading; flowing water can appear to hang in mid-air at world edges.
Suggested Fix: Wake the liquid simulator for border cells of the neighbouring chunks, e.g.:

this.liquid.enqueueAround((chunk.cx - 1) * CHUNK_SIZE + CHUNK_SIZE - 1, y, (chunk.cz + 0) * CHUNK_SIZE + z);
// ... for each of the four neighbours / relevant y layers

(or enqueue a representative ring of cells at the new border).

[HIGH] src/game/World.ts:247-266 — setLiquid does not mark neighbour chunks dirty for mesh updates
Confidence: Medium
Description: setLiquid marks the edited chunk dirty via chunk.setLocalWithLevel, but when the edit is on a chunk border it only queues neighbours for lighting. Cross-chunk liquid level changes can alter the water side/step faces visible in a neighbour chunk, yet the neighbour is not marked dirty for remesh unless its lighting also happens to change. setBlock has the same pattern, but liquids are transparent and light-conducting, so lighting changes are much less guaranteed.
Impact: Stale water step faces / missing side faces at chunk borders after flow reaches an equilibrium.
Suggested Fix: In setLiquid, mirror setBlock's border handling explicitly for mesh dirtiness:

if (lx === 0) this.markDirty(cx - 1, cz);
if (lx === CHUNK_SIZE - 1) this.markDirty(cx + 1, cz);
if (lz === 0) this.markDirty(cx, cz - 1);
if (lz === CHUNK_SIZE - 1) this.markDirty(cx, cz + 1);

💡 Medium Priority Issues (Nice to Fix)

[MEDIUM] src/game/liquid/LiquidSimulator.ts:335-336 and src/game/liquid/LiquidSimulator.ts:423-424 — Renewable source rule is more permissive than documented
Confidence: Medium
Description: The JSDoc and code comments state that renewable source creation requires "solid/source support below", but the actual check is const supported = !belowFloodable || below === WATER_BLOCK;. !belowFloodable is true for any liquid below, not just source water. This allows a flowing cell suspended over flowing water to convert to a source if it has two horizontal source neighbours, creating sources in waterfalls/plumes rather than only in genuine pools.
Impact: Possible unintended infinite-source creation in flowing columns.
Suggested Fix: Tighten the support rule to match the documented intent:

const supported = getBlock(below).solid || below === WATER_BLOCK;

[MEDIUM] src/game/Player.ts:186-187 — Water drag is frame-rate dependent
Confidence: High
Description: Horizontal velocity is multiplied by 0.86 every frame while in water, regardless of dt. At 30 FPS the player loses horizontal speed much faster than at 144 FPS.
Impact: Inconsistent swim physics across refresh rates.
Suggested Fix: Use a dt-aware exponential decay, e.g.:

const drag = Math.pow(0.86, dt * 60);
this.velocity.x *= drag;
this.velocity.z *= drag;

[MEDIUM] PR description claims unit tests that are not present in the repository
Confidence: High
Description: The PR body lists "Unit tests: liquid flow (8/8), raycast-through-water (5/5), responsiveness/priority (proves same-frame edit response vs backlog)." No test files appear in the diff or in the repository (**/*.test.ts, test/, etc. all return empty).
Impact: Claims cannot be verified; future regressions in the liquid system are unprotected.
Suggested Fix: Add the described test files (even a lightweight Bun test runner setup) or remove the claim from the PR description.

[MEDIUM] src/game/liquid/LiquidSimulator.ts:331-362 — bestFeeder has two overlapping JSDoc blocks
Confidence: High
Description: Two consecutive JSDoc comments precede bestFeeder. The first is stale/redundant and the second is the one that matches the implementation.
Impact: Documentation confusion for maintainers.
Suggested Fix: Delete the first JSDoc block (lines ~331-347) and keep the second.

ℹ️ Low Priority Suggestions (Optional)

[LOW] src/game/ChunkMesher.ts:334 — Dead variable below
Confidence: High
Description: const below = getBlockWorld(x, y - 1, z); is declared and then explicitly voided with void below;.
Suggested Fix: Remove the variable.

[LOW] src/game/lighting/WaterMaterial.ts:1 — Unused Vector3 import
Confidence: High
Description: Vector3 is imported from @babylonjs/core but never used.
Suggested Fix: Remove Vector3 from the import.

[LOW] src/game/liquid/LiquidTypes.ts:50 — LiquidDebugMode type is defined but unused
Confidence: High
Description: The type is exported but never referenced elsewhere.
Suggested Fix: Remove it or wire it to a future overlay mode.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	8/10	Liquid flow, rendering, and player physics are cleanly separated; World gains a few new responsibilities but keeps them well-scoped.
Open/Closed	7/10	New liquids require editing BLOCKS directly; meshing is reasonably open via liquidType/LiquidDef.
Liskov Substitution	9/10	getBlock(id) fallback to AIR keeps all consumers safe; new flowing block integrates without breaking existing lookups.
Interface Segregation	8/10	LiquidAccess is a narrow, testable surface; PerfSnapshot grows but remains a single snapshot object.
Dependency Inversion	8/10	Simulator depends on LiquidAccess, not World; Game still reaches into World internals for debug overlays.
Average	8.0

🎯 Final Assessment

Overall Confidence Score: 75%

Confidence Breakdown:

• Code Quality: 80% (clean, well-commented, but some dead code/doc drift)
• Completeness: 70% (feature-complete, but claimed tests are missing and chunk-border edge cases need handling)
• Risk Level: 65% (large change touching meshing, physics, and simulation; border cases are the main risk)
• Test Coverage: 40% (no tests present despite PR claims)

Merge Readiness:

• All critical issues resolved
• SOLID average score >= 6.0
• Overall confidence >= 60%
• No security concerns
• Tests present and passing (if applicable)

Verdict:

MERGE WITH FIXES — The core implementation is solid, builds are clean, and the feature scope is impressive. Address the chunk-border liquid remesh/re-seed issues and either add the claimed tests or update the PR description before merging.

opencode session  |  github run

[github-actions]

📋 Summary

No linked issues are mentioned in the PR description.

The previous review's feedback has been fully addressed in commit 501c28f: chunk-unload now wakes the liquid simulator, setLiquid marks neighbour chunks dirty, renewable source support is tightened to solid/source below, water drag is dt-aware, the duplicate JSDoc/dead-code clean-ups are applied, and the claimed test suites (tests/liquid.test.ts, tests/raycast.test.ts, tests/responsiveness.test.ts) have been added and wired to bun run test. Builds and tests are green.

📌 Review Metadata

• Reviewed Commit SHA: 501c28f572fced9b7ba0a88e69817ba81410de94
• Reviewed PR: Water overhaul: liquid simulation, flow physics, rendering, interaction #4

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

✅ All previously reported critical issues have been resolved.

⚠️ High Priority Issues (Should Fix)

None identified.

✅ [FIXED] Previous issue: World.ts:629-635 — chunk unload now wakes the liquid simulator via wakeBorderLiquid so border water recomputes instead of hanging in air.

✅ [FIXED] Previous issue: World.ts:247-266 — setLiquid now explicitly calls markDirty on neighbour chunks for border edits.

💡 Medium Priority Issues (Nice to Fix)

None identified.

✅ [FIXED] Previous issue: LiquidSimulator.ts:331-362 — renewable source support tightened to getBlock(below).solid || below === WATER_BLOCK, no longer accepts any non-floodable below.

✅ [FIXED] Previous issue: Player.ts:186-187 — horizontal water drag is now dt-aware (Math.pow(0.86, dt * 60)).

✅ [FIXED] Previous issue: missing unit tests — tests/liquid.test.ts (8 tests), tests/raycast.test.ts (5 tests), and tests/responsiveness.test.ts (2 tests) added and passing.

✅ [FIXED] Previous issue: duplicate bestFeeder JSDoc block removed.

ℹ️ Low Priority Suggestions (Optional)

✅ [FIXED] Previous issue: ChunkMesher.ts:334 — dead below variable removed.

✅ [FIXED] Previous issue: WaterMaterial.ts:1 — Vector3 is now used in setFog(_cameraPosition: Vector3, ...).

✅ [FIXED] Previous issue: LiquidTypes.ts:50 — unused LiquidDebugMode type removed.

[LOW] src/game/World.ts:100 — liquidTickBudget field is assigned but never read
Confidence: High
Description: The field is initialized at line 100 and updated at line 637, but liquidDebug() returns this.liquid.debug whose budget comes from the simulator directly. The field is dead code.
Impact: None functional; minor maintenance clutter.
Suggested Fix: Either expose it in the debug snapshot or remove the field and the assignment at line 637.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	8/10	Liquid flow, rendering, and player physics remain cleanly separated; World coordinates them without absorbing simulator internals.
Open/Closed	7/10	New liquids still require editing BLOCKS, but the LiquidDef/liquidType hooks and LiquidAccess interface make extension straightforward.
Liskov Substitution	9/10	getBlock(id) fallback to AIR keeps all consumers safe; new flowing block integrates without breaking existing lookups.
Interface Segregation	8/10	LiquidAccess is a narrow, testable surface; PerfSnapshot grows but remains a single snapshot object.
Dependency Inversion	8/10	Simulator depends on LiquidAccess, not World; debug wiring in Game still reaches into World internals but is isolated to dev surface.
Average	8.0

🎯 Final Assessment

Overall Confidence Score: 88%

Confidence Breakdown:

• Code Quality: 85% (clean, well-commented, one dead field remaining)
• Completeness: 90% (feature-complete, previous gaps addressed, tests present)
• Risk Level: 80% (large change, but simulation is bounded, convergent, and tested)
• Test Coverage: 85% (three focused suites covering flow rules, raycast behavior, and priority responsiveness)

Merge Readiness:

• All critical issues resolved
• SOLID average score >= 6.0
• Overall confidence >= 60%
• No security concerns
• Tests present and passing

Verdict:

MERGE — The previous review feedback has been comprehensively addressed, all builds and tests pass, and the water system is now well-tested and ready for integration.

opencode session  |  github run