Graphics & performance pass: presets, perf overlay, water, debug tooling

[MichaelFisher1997]

Summary

A browser-friendly graphics/performance foundation for the Babylon.js voxel game. Presets, a performance overlay, scalable settings, water/cloud/shadow quality tiers, and developer debug tooling. This branch also fixes a couple of regressions introduced and resolved during the pass (missing-terrain, invisible water, chunk seams).

What's included

Graphics settings & presets

• Low / Medium / High presets + individual toggles: render scale, shadows, water, foliage, clouds, FXAA
• Persisted to localStorage; low-end device autodetect picks conservative defaults
• Reactive apply — no page reload needed

Performance overlay (F3)

• FPS, frame ms, ~draws, triangles, active/total meshes
• loaded / meshed / visible / culled chunks, mesh+light queue depth
• shadow casters, water meshes
• ambient/sun/dayFactor, fog start/end, water alpha/quality, render scale, DPR, GPU renderer, JS heap

Rendering

• Render scale via hardware scaling + optional pure-FXAA (image-processing disabled to avoid the High-preset washout)
• Adaptive chunk gen/mesh/light budgets driven by frame time (catch up when smooth, trickle when stuttering)
• Frozen world matrices on static chunk meshes; foliage render-distance cull

Water

• StandardMaterial blue tint with an emissive floor + specular glint, lit by scene lights and fogged by scene fog
• Vertex colours stripped from the water mesh so lakes render continuously across chunks (no chunk-aligned seams)
• Quality tiers vary alpha (all clearly readable); debug opaque/off toggles

Clouds & shadows

• Cloud tiers (off/simple/fancy) with day/night dimming (simple skips top faces)
• Shadow tiers (off/low/med/high) with runtime reconfigure

Debug tooling

• F3 perf overlay · N wireframe · B chunk borders
• Console: __voxl.water(), __voxl.waterOpaque(), __voxl.shadows(), __voxl.fog(), __voxl.post(), __voxl.dumpMaterials(), __voxl.waterStats(), __voxl.safeMode(), __voxl.terrainCulling(), __voxl.renderAllChunks()
• Lifecycle logging via localStorage["voxl.debug"]="1"

Safeguards

• WebGL context-loss handling, render-distance cap (2–12), clean disposal of pipeline/materials/world

Notes / known limitations

• Back-face culling on opaque terrain was attempted for fragment savings but reverted (face winding didn't match Babylon's front-face convention → holes). A __voxl.terrainCulling() toggle is provided to re-test safely. Real-time shadows currently affect nothing visible (terrain has receiveShadows=false, water/clouds/sky are custom/unlit) — they're GPU cost only; this is intentional pending a receiver pass.
• Mesh update-in-place (vs dispose+recreate) deferred; greedy meshing intentionally not implemented.

Verification

• tsc --noEmit clean
• vite build succeeds
• Manual test checklist from the task spec recommended (presets, render distance, shadows on/off, clouds, foliage, frame pacing, memory after travel)

Changed files

16 modified + 4 new (GraphicsController, GraphicsSettings, WaterMaterial, PerfOverlay, ChunkBorderOverlay).

[github-actions]

📋 Summary

No linked issues found in the PR description.

This PR introduces a graphics/performance foundation for the Babylon.js voxel game: three graphics presets (Low/Medium/High), a runtime graphics controller, an F3 performance overlay, scalable water/cloud/shadow/foliage tiers, debug console helpers, and adaptive chunk streaming budgets. It also fixes water visibility and chunk-seam issues by switching water to a uniform-tint StandardMaterial and stripping per-chunk vertex colours.

All verification commands pass: bun run typecheck (game), bun run build (game), bun run typecheck (website), and bun run build (website).

📌 Review Metadata

• Reviewed Commit SHA: a836a927722e6750d698c2bcd145e91eb7f0c995
• Reviewed PR: Graphics & performance pass: presets, perf overlay, water, debug tooling #3
• Linked Issues: None identified.

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

⚠️ High Priority Issues (Should Fix)

None identified.

💡 Medium Priority Issues (Nice to Fix)

[MEDIUM] src/game/Game.ts:276-279 — Graphics presets do not adjust render distance despite helper and documentation
Confidence: High
Description: applyGraphicsPreset() only applies graphicsFromPreset() and never calls presetRenderDistance() (exported in src/game/graphics/GraphicsController.ts:179-186 but unused). The method's own JSDoc says it should "nudge the render distance to the preset's recommended value," so Low/Medium/High presets currently leave the user's existing view distance unchanged.
Impact: Preset behaviour is incomplete and contradicts the in-code documentation; a "Low" preset may keep a high view distance, undermining the performance goal.
Suggested Fix:

import { presetRenderDistance } from "./graphics/GraphicsController";

applyGraphicsPreset(preset: GraphicsPreset): void {
  const graphics = graphicsFromPreset(preset);
  this.applySettings({ graphics, viewDistance: presetRenderDistance(preset) });
}

[MEDIUM] src/game/World.ts:660-670 — Water audit hardcodes block id 7
Confidence: High
Description: waterStats() uses const WATER = 7; instead of importing WATER_BLOCK from Blocks.ts. Block ids are documented as stable, but the codebase already exports a named constant (WATER_BLOCK) and uses it elsewhere (Player.ts, Items.ts, TerrainGenerator.ts).
Impact: If the water block id ever changes, the debug audit silently reports zero water blocks, making it misleading.
Suggested Fix:

import { WATER_BLOCK } from "./Blocks";
// ...
if (b[i] === WATER_BLOCK) local++;

[MEDIUM] src/ui/ChunkBorderOverlay.ts:73-99 — Chunk-border overlay allocates heavily and ignores updatable buffer
Confidence: Medium
Description: Every time the player crosses a chunk boundary, update() allocates a fresh Vector3 for every corner of every loaded chunk (e.g., ~3,000 Vector3 objects at max view distance) and disposes/recreates the LinesMesh instead of updating the existing updatable buffer declared with updatable: true.
Impact: A noticeable GC spike each time the debug overlay rebuilds, which works against the PR's frame-pacing goals.
Suggested Fix: Preallocate a Float32Array/Vector3[] pool and call this.lines.updateVerticesData("position", positions) instead of disposing and recreating the mesh.

ℹ️ Low Priority Suggestions (Optional)

[LOW] src/game/World.ts:46-53 and src/game/lighting/VoxelTerrainMaterial.ts:76-83 — Stale comments about opaque back-face culling
Confidence: High
Description: The World JSDoc says terrainOpaque "culls back faces" and the material option comment says the opaque pass "should cull back faces — default false (culling ON)." In practice World constructs both materials with doubleSided: true, so both are double-sided (culling OFF). The revert is intentional per the PR notes, but the comments now describe the opposite behaviour.
Impact: Confusing for future maintainers; risk of someone re-enabling culling based on the comment.
Suggested Fix: Update both comments to state that culling is currently disabled because the mesher winding does not match Babylon's front-face convention.

[LOW] src/game/ChunkMesher.ts:179-181 and src/game/ChunkMesher.ts:309 — Comment says water uses scalar brightness, code passes two-channel
Confidence: High
Description: The pushFace call for liquids now passes true for twoChannel, yet the comment still says "Water pass keeps a single scalar brightness." The colours are stripped later for the transparent mesh, so this is functionally harmless, but the comment is stale.
Suggested Fix: Update the comment to reflect that all faces currently bake the same four-channel colour data and the water pass discards it via result.transparent.colors = null.

[LOW] src/game/lighting/LightingSystem.ts:74-79 and src/game/lighting/WaterMaterial.ts:85-99 — Water uniform comments contradict no-op implementation
Confidence: High
Description: LightingSystem.update() calls waterShader.setDayNight(), setFog(), and setTime() with a comment saying "Water shader uniforms mirror the terrain." In WaterMaterial these methods are explicitly no-ops because day/night/fog are handled by Babylon's StandardMaterial using scene lights/fog.
Impact: Misleading documentation; future readers may expect custom water uniforms that do not exist.
Suggested Fix: Update LightingSystem's comment to note that water uses scene lights/fog and the calls are retained only for API compatibility.

[LOW] src/game/World.ts:545,562 — Redundant receiveShadows = false for cutout meshes
Confidence: High
Description: applyMesh() sets mesh.receiveShadows = false for all slots, then sets it again when slot === "cutout".
Suggested Fix: Remove the cutout-specific reassignment.

[LOW] src/engine/Clouds.ts:216-221 — addQuad parameters shadow outer cz
Confidence: Medium
Description: The addQuad closure parameter list includes c and cz, where cz shadows rebuild()'s cz. This is legal but makes the function harder to follow.
Suggested Fix: Rename the corner-coordinate parameters (e.g., cx2, cy2, cz2) to avoid shadowing.

[LOW] src/engine/Sky.ts:125-127 — setCloudsEnabled is dead code
Confidence: High
Description: The old setCloudsEnabled() method is no longer called; all cloud state changes go through setClouds().
Suggested Fix: Remove the unused method.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	7	GraphicsController cleanly separates settings data from engine application; Game.ts accumulates many debug hooks but they are isolated behind _ methods.
Open/Closed	6	New quality tiers are added via switch statements; extending them requires editing multiple files.
Liskov Substitution	8	No inheritance hierarchies introduced; new materials conform to existing usage patterns.
Interface Segregation	7	WaterMaterial exposes unused setDayNight/setFog/setTime no-ops purely for compatibility.
Dependency Inversion	7	Settings are plain data; controller depends on Babylon types, but world/scene coupling is already established.
Average	7.0

🎯 Final Assessment

Overall Confidence Score: 78%

Confidence Breakdown:

• Code Quality: 80% (clean, well-commented, typechecks and builds; some stale comments and unused helpers)
• Completeness: 75% (presets are missing the advertised render-distance nudge; otherwise feature-complete per PR description)
• Risk Level: 70% (low-risk additive changes, but debug-only water audit uses a magic number)
• Test Coverage: 40% (manual-test checklist recommended; no automated tests added)

Merge Readiness:

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

Verdict:

MERGE WITH FIXES — Presets should wire up presetRenderDistance and WATER_BLOCK should replace the magic number before merging; remaining items are documentation/cleanup.

opencode session  |  github run

[github-actions]

📋 Summary

No linked issues found in the PR description.

This PR adds a browser-friendly graphics/performance layer: three scalable presets, an F3 performance overlay, adaptive chunk streaming budgets, a new StandardMaterial-based water layer to fix chunk seams, runtime shadow/water/cloud/foliage tiers, and a suite of console debug helpers. It also migrates legacy boolean clouds settings. All verification commands pass.

📌 Review Metadata

• Reviewed Commit SHA: 4e6f2c50c6ba94c35c1ff6bdc30c81db464dfddd
• Reviewed PR: Graphics & performance pass: presets, perf overlay, water, debug tooling #3
• Linked Issues: None identified.

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

⚠️ High Priority Issues (Should Fix)

None identified.

💡 Medium Priority Issues (Nice to Fix)

[MEDIUM] src/ui/ChunkBorderOverlay.ts:17 — Chunk-border vertex pool is too small for max view distance
Confidence: High
Description: MAX_CHUNKS is computed from MAX_RENDER_DISTANCE (12), but World.update() at src/game/World.ts:454 unloads chunks at viewDistance + 2. At the max view distance the world can therefore hold (2*14+1)^2 = 841 loaded chunks while the overlay only allocates 657. The pool guard silently stops drawing borders for outer chunks.
Impact: The chunk-border debug overlay is incomplete when the view distance slider is at 12.
Suggested Fix: Size the pool for the unload radius:

const MAX_CHUNKS = (2 * (MAX_RENDER_DISTANCE + 2) + 1) * (2 * (MAX_RENDER_DISTANCE + 2) + 1) + 32;

[MEDIUM] src/game/Game.ts:1131-1133 — __voxl.shadows() bypasses settings persistence and UI sync
Confidence: High
Description: _setShadows() calls this.lighting?.shadows.setEnabled(on) directly. It does not update this.settings.graphics.shadows, persist to localStorage, or refresh the settings menu. In contrast, _setFog() and _setPost() route through applySettings() and stay in sync.
Impact: The shadow dropdown becomes stale; a subsequent settings change can revert the console toggle unexpectedly.
Suggested Fix: Route through settings so the UI and persistence stay consistent:

_setShadows(on: boolean): void {
  const current = this.settings.graphics.shadows;
  const shadows = on ? (current !== "off" ? current : "medium") : "off";
  this.applySettings({ graphics: { ...this.settings.graphics, shadows } });
  this.hud.showToast(on ? "Shadows: on" : "Shadows: off");
}

[MEDIUM] src/game/lighting/ShadowManager.ts:124-156 — configure() rebuilds the shadow generator on every settings change
Confidence: Medium
Description: When shadows are enabled and configure() is called, the method always disposes an existing generator and recreates it (if (hasGen) this.dispose(); this.setup();). GraphicsController.applyShadows() invokes this whenever any graphics field changes (e.g. render scale or water quality), even if the shadow config is identical.
Impact: Unnecessarily expensive generator rebuilds each time the user tweaks unrelated graphics settings.
Suggested Fix: Compare the resolved config against the current config and skip the teardown/rebuild when nothing changed.

ℹ️ Low Priority Suggestions (Optional)

[LOW] src/game/lighting/LightingSystem.ts:35 and src/game/lighting/WaterMaterial.ts:88-90 — elapsed time is accumulated but never used
Confidence: High
Description: LightingSystem increments this.elapsed each frame and passes it to waterShader.setTime(), but WaterMaterial.setTime() is explicitly a no-op. The comment says it drives water animation, yet no animation exists.
Impact: Misleading comment and a few wasted cycles per frame.
Suggested Fix: Remove this.elapsed, the setTime() call, and the no-op setTime() method until water animation is actually implemented.

[LOW] src/game/Game.ts:251 — applySettings mutates the incoming patch
Confidence: Medium
Description: applySettings clamps patch.viewDistance in place. Callers do not reuse the object today, but mutating a parameter is a latent foot-gun.
Suggested Fix: Copy the patch before clamping:

patch = { ...patch, viewDistance: Math.max(MIN_RENDER_DISTANCE, Math.min(MAX_RENDER_DISTANCE, Math.round(patch.viewDistance))) };

✅ Fixed Issues From Previous Review

• ✅ applyGraphicsPreset() now calls presetRenderDistance() (src/game/Game.ts:280).
• ✅ World.waterStats() now imports and uses WATER_BLOCK (src/game/World.ts:28,667).
• ✅ ChunkBorderOverlay now uses a preallocated Float32Array pool and updateVerticesData() instead of recreating the mesh.
• ✅ Stale comments about opaque back-face culling and water scalar brightness have been updated.
• ✅ Sky.setCloudsEnabled() dead code removed and Clouds.addQuad parameter shadowing fixed.
• ✅ Redundant receiveShadows = false assignment in World.applyMesh() removed.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	7	GraphicsController cleanly separates settings data from engine application; Game.ts accumulates debug hooks but they are isolated behind _ methods.
Open/Closed	6	New quality tiers are added via switch statements; extending them requires editing multiple files.
Liskov Substitution	8	No inheritance hierarchies introduced; new materials conform to existing usage patterns.
Interface Segregation	7	WaterMaterial exposes unused setDayNight/setFog/setTime no-ops purely for compatibility.
Dependency Inversion	7	Settings are plain data; controller depends on Babylon types, but world/scene coupling is already established.
Average	7.0

🎯 Final Assessment

Overall Confidence Score: 78%

Confidence Breakdown:

• Code Quality: 85% (clean, well-commented, typechecks and builds; a few debug-helper inconsistencies and an undersized debug buffer)
• Completeness: 80% (feature-complete per PR description; debug shadow toggle and chunk-border pool need finishing)
• Risk Level: 75% (large additive PR with low game-logic risk; shadow generator rebuild and debug toggles are the main operational concerns)
• Test Coverage: 40% (manual-test checklist recommended; no automated tests added)

Merge Readiness:

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

Verdict:

MERGE WITH FIXES — Increase the chunk-border vertex pool to account for the viewDistance + 2 unload margin, route __voxl.shadows() through applySettings, and make ShadowManager.configure() skip rebuilding when the config is unchanged.

opencode session  |  github run

[github-actions]

📋 Summary

No linked issues found in the PR description.

This PR adds a browser-friendly graphics/performance layer to the Babylon.js voxel game: three scalable presets, an F3 performance overlay, adaptive chunk streaming budgets, a StandardMaterial-based water layer to fix chunk seams, runtime shadow/water/cloud/foliage tiers, and a suite of console debug helpers. All verification commands pass (bun run typecheck and bun run build for both root and website/).

📌 Review Metadata

• Reviewed Commit SHA: 40aacd8de9e1d996922517d4382265f9ecbf7ca6
• Reviewed PR: Graphics & performance pass: presets, perf overlay, water, debug tooling #3
• Linked Issues: None identified.

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

⚠️ High Priority Issues (Should Fix)

None identified.

💡 Medium Priority Issues (Nice to Fix)

[MEDIUM] src/game/World.ts:473-482 — Foliage distance-cull parses chunk keys every frame
Confidence: Medium
Description: When foliage density is Low or Medium, World.update() iterates every meshed chunk and extracts cx/cz by calling mk.indexOf(",") and parseInt on the map key for each chunk that has a cutout mesh. At max view distance this is hundreds of string operations per frame inside the main streaming hot path.
Impact: Unnecessary CPU overhead that runs counter to the PR's performance goals.
Suggested Fix: Store chunk coordinates in the ChunkMeshes entry so the loop reads numeric coords directly:

interface ChunkMeshes {
  opaque?: Mesh;
  cutout?: Mesh;
  transparent?: Mesh;
  cx: number;
  cz: number;
}

Then pass chunk.cx/chunk.cz into applyMesh and rewrite the cull loop without string parsing:

for (const entry of this.meshes.values()) {
  const m = entry.cutout;
  if (!m) continue;
  const dx = entry.cx * 16 + 8 - playerX;
  const dz = entry.cz * 16 + 8 - playerZ;
  m.setEnabled(dx * dx + dz * dz <= cutSq);
}

ℹ️ Low Priority Suggestions (Optional)

[LOW] src/engine/Clouds.ts:285 — bindFog allocates a new Vector2 each frame
Confidence: High
Description: Clouds.bindFog() calls this.material.setVector2("fogRange", new Vector2(start, end)); on every frame, creating a per-frame Babylon vector allocation in the render loop.
Impact: Minor GC pressure.
Suggested Fix: Use a scratch vector:

private readonly _fogRange = new Vector2(0, 0);
bindFog(color: Color3, start: number, end: number, cameraPos: Vector3): void {
  this.material.setColor3("fogColor", color);
  this._fogRange.set(start, end);
  this.material.setVector2("fogRange", this._fogRange);
  this.material.setVector3("cameraPos", cameraPos);
}

[LOW] src/game/graphics/GraphicsSettings.ts:36 and :131 — dprCap documentation and clamp disagree
Confidence: High
Description: The dprCap JSDoc says the range is 1.5..2, but migrateGraphics() clamps the loaded value to [1, 3].
Impact: Saved values outside the documented range are accepted silently, which is confusing.
Suggested Fix: Align the two; either clamp to [1.5, 2] or update the JSDoc to reflect [1, 3].

[LOW] src/game/World.ts:572-580 — forEachOpaqueMesh repeats the same string-key parsing
Confidence: Low
Description: The shadow manager and lighting-debug paths parse the meshes map key into cx/cz for every opaque mesh. The same fix as the foliage-cull issue above removes this parsing.
Impact: Only matters when shadows are enabled or the lighting debug overlay is open.
Suggested Fix: Reuse the cx/cz stored on ChunkMeshes once the medium-priority foliage fix is applied.

✅ Fixed Issues From Previous Review

• ✅ applyGraphicsPreset() now calls presetRenderDistance() (src/game/Game.ts:282-285).
• ✅ World.waterStats() now imports and uses WATER_BLOCK (src/game/World.ts:28,667).
• ✅ ChunkBorderOverlay pool is now sized for the viewDistance + 2 unload margin (src/ui/ChunkBorderOverlay.ts:24-25).
• ✅ Stale comments about opaque back-face culling and water brightness have been updated.
• ✅ Sky.setCloudsEnabled() dead code removed and Clouds.addQuad parameter shadowing fixed.
• ✅ Redundant receiveShadows = false assignment in World.applyMesh() removed.
• ✅ __voxl.shadows() now routes through applySettings() so persistence and UI stay in sync (src/game/Game.ts:1137-1142).
• ✅ ShadowManager.configure() now skips teardown/rebuild when the resolved config is unchanged (src/game/lighting/ShadowManager.ts:140-158).

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	7	GraphicsController cleanly separates settings data from engine application; Game.ts accumulates debug hooks but they are isolated behind _ methods.
Open/Closed	6	New quality tiers are added via switch statements; extending them requires editing multiple files.
Liskov Substitution	8	No inheritance hierarchies introduced; new materials conform to existing usage patterns.
Interface Segregation	7	WaterMaterial exposes unused setDayNight/setFog no-ops purely for API compatibility.
Dependency Inversion	7	Settings are plain data; controller depends on Babylon types, but world/scene coupling is already established.
Average	7.0

🎯 Final Assessment

Overall Confidence Score: 82%

Confidence Breakdown:

• Code Quality: 85% (clean, well-commented, typechecks and builds; minor hot-path inefficiencies)
• Completeness: 85% (feature-complete per PR description; presets, water fix, debug tooling all wired)
• Risk Level: 80% (large additive PR but low game-logic risk; main concern is a small hot-path perf regression)
• Test Coverage: 40% (manual-test checklist recommended; no automated tests added)

Merge Readiness:

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

Verdict:

MERGE WITH FIXES — Remove the per-frame string-key parsing from the foliage cull (and forEachOpaqueMesh) before merging; the remaining items are minor cleanups.

opencode session  |  github run

[github-actions]

📋 Summary

No linked issues found in the PR description.

This PR adds a browser-friendly graphics/performance layer to the Babylon.js voxel game: three scalable presets, an F3 performance overlay, adaptive chunk streaming budgets, a StandardMaterial-based water layer to fix chunk seams, runtime shadow/water/cloud/foliage tiers, and a suite of console debug helpers. Since the previous review, the round-3 fixes correctly cache numeric chunk coords, reuse a scratch Vector2 for cloud fog, and align dprCap migration with its documented range. All verification commands pass (bun run typecheck and bun run build for both root and website/), and the light tests pass.

📌 Review Metadata

• Reviewed Commit SHA: 843400c94201e77c58ee141c0250797ea48fc6bb
• Reviewed PR: Graphics & performance pass: presets, perf overlay, water, debug tooling #3
• Linked Issues: None identified.

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

⚠️ High Priority Issues (Should Fix)

None identified.

💡 Medium Priority Issues (Nice to Fix)

None identified.

ℹ️ Low Priority Suggestions (Optional)

[LOW] src/game/World.ts:136 — Opaque material getter comment still claims back-face culling
Confidence: High
Description: The opaqueMaterial JSDoc says "shared, back-face culled", but both terrain materials are constructed with doubleSided: true (culling OFF). The main class JSDoc at src/game/World.ts:51-61 correctly explains the winding issue, but the getter comment contradicts it.
Impact: Misleading for future maintainers; risk of someone re-enabling culling based on the comment.
Suggested Fix: Update the getter comment to match the actual double-sided behavior:

/** The opaque ShaderMaterial (shared, double-sided until winding is fixed). */

[LOW] src/game/World.ts:131-133 — World constructor comment still calls water an "animated water shader"
Confidence: High
Description: The comment says "Transparent pass: animated water shader... Day/night + fog are pushed each frame by LightingSystem." Water now uses a plain StandardMaterial, and the LightingSystem day/night/fog calls on waterShader are no-ops because scene lights/fog handle them.
Impact: Misleading; implies custom water animation/shader that does not exist.
Suggested Fix: Update the comment to describe the StandardMaterial-based water pass and note that day/night/fog come from scene state.

[LOW] src/game/graphics/GraphicsSettings.ts:140-143 — migrateGraphics can preserve a boolean graphics.clouds value
Confidence: Medium
Description: migrateGraphics assigns parsed.clouds directly to clouds. If a stale/legacy save contains graphics: { clouds: true } (boolean), the variable becomes true rather than undefined, so the boolean-to-tier conversion is skipped and an invalid value is returned.
Impact: Saved settings could end up with a boolean value for clouds, causing downstream type/runtime issues.
Suggested Fix: Only accept string values from parsed.clouds:

let clouds: CloudsQuality | undefined =
  typeof parsed.clouds === "string" ? parsed.clouds : undefined;
if (clouds === undefined && typeof legacy.clouds === "boolean") {
  clouds = legacy.clouds ? "fancy" : "off";
}

[LOW] src/game/graphics/GraphicsSettings.ts:116-127 and src/game/graphics/GraphicsController.ts:189 — Dead exports
Confidence: High
Description: graphicsEqual (in GraphicsSettings.ts) and isBuiltinPreset (in GraphicsController.ts) are exported but never imported or used anywhere in the codebase.
Impact: Minor dead code; adds noise to the public API.
Suggested Fix: Remove both unused exports.

✅ Fixed Issues From Previous Review

• ✅ applyGraphicsPreset() now calls presetRenderDistance() (src/game/Game.ts:282-285).
• ✅ World.waterStats() now imports and uses WATER_BLOCK (src/game/World.ts:28,667).
• ✅ ChunkBorderOverlay pool is sized for the viewDistance + 2 unload margin and uses a preallocated Float32Array with updateVerticesData() (src/ui/ChunkBorderOverlay.ts:19,111).
• ✅ Stale comments about opaque back-face culling (main World JSDoc) and water brightness have been updated.
• ✅ Sky.setCloudsEnabled() dead code removed and Clouds.addQuad parameter shadowing fixed.
• ✅ Redundant receiveShadows = false assignment in World.applyMesh() removed.
• ✅ __voxl.shadows() now routes through applySettings() so persistence and UI stay in sync (src/game/Game.ts:1137-1142).
• ✅ ShadowManager.configure() now skips teardown/rebuild when the resolved config is unchanged (src/game/lighting/ShadowManager.ts:140-158).
• ✅ Foliage cull and forEachOpaqueMesh now read cached cx/cz from ChunkMeshes instead of parsing map keys (src/game/World.ts:481-483,576-580).
• ✅ Clouds.bindFog() reuses a scratch Vector2 instead of allocating per frame (src/engine/Clouds.ts:115,287-290).
• ✅ GraphicsSettings.migrateGraphics() now clamps dprCap to the documented [1.5, 2] range (src/game/graphics/GraphicsSettings.ts:147).
• ✅ Game.applySettings() no longer mutates the incoming patch object (src/game/Game.ts:249-254).

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	7	GraphicsController cleanly separates settings data from engine application; Game.ts accumulates debug hooks but they are isolated behind _ methods.
Open/Closed	6	New quality tiers are added via switch statements; extending them requires editing multiple files.
Liskov Substitution	8	No inheritance hierarchies introduced; new materials conform to existing usage patterns.
Interface Segregation	7	WaterMaterial exposes unused setDayNight/setFog no-ops purely for API compatibility.
Dependency Inversion	7	Settings are plain data; controller depends on Babylon types, but world/scene coupling is already established.
Average	7.0

🎯 Final Assessment

Overall Confidence Score: 85%

Confidence Breakdown:

• Code Quality: 85% (clean, well-commented, typechecks and builds; a few stale comments and unused helpers)
• Completeness: 90% (feature-complete per PR description; presets, water fix, debug tooling all wired)
• Risk Level: 80% (large additive PR but low game-logic risk; debug-only helpers are the main operational concern)
• Test Coverage: 40% (manual-test checklist recommended; light tests pass; no automated tests added)

Merge Readiness:

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

Verdict:

MERGE WITH FIXES — Address the remaining stale comments, dead-code exports, and the defensive graphics.clouds migration edge case before merging; all substantive issues from previous rounds are resolved.

opencode session  |  github run

[github-actions]

📋 Summary

No linked issues found in the PR description.

This PR adds a browser-friendly graphics/performance foundation for the Babylon.js voxel game: three graphics presets (Low/Medium/High), a runtime GraphicsController, an F3 performance overlay, adaptive chunk streaming budgets, a StandardMaterial-based water layer to fix chunk seams, runtime shadow/water/cloud/foliage tiers, and a suite of console debug helpers. It also migrates legacy boolean clouds settings and fixes several regressions noted in the PR body.

All verification commands pass: bun run typecheck (game), bun run build (game), bun run typecheck (website), bun run build (website), and bun run test:light.

📌 Review Metadata

• Reviewed Commit SHA: 9094658023c99af8ff9ef1a0281aefd04afeee25
• Reviewed PR: Graphics & performance pass: presets, perf overlay, water, debug tooling #3
• Linked Issues: None identified.

This is the fifth review round. All substantive issues from prior rounds (preset render distance, magic-number water block id, chunk-border allocations, stale culling/water-brightness comments, shadow UI sync, shadow generator rebuild guard, foliage key parsing, cloud fog allocation, dprCap clamp, and applySettings patch mutation) have been addressed. The remaining findings are minor documentation inconsistencies.

🔴 Critical Issues (Must Fix - Blocks Merge)

✅ All previously reported critical issues have been resolved.

None identified.

⚠️ High Priority Issues (Should Fix)

✅ All previously reported high priority issues have been resolved.

None identified.

💡 Medium Priority Issues (Nice to Fix)

✅ All previously reported medium priority issues have been resolved.

None identified.

ℹ️ Low Priority Suggestions (Optional)

[LOW] src/game/World.ts:64 — Property comment still calls water an "animated water shader"
Confidence: High
Description: The waterShader property JSDoc says "Animated water shader (shared)", but WaterMaterial is a plain StandardMaterial and the PR notes explicitly state that a setTime/animation hook is omitted until water animation is implemented.
Impact: Misleading; implies custom animation that does not exist.
Suggested Fix: Update the comment to describe the StandardMaterial-based water pass:

/** Plain StandardMaterial water pass (shared). {@link waterMaterial} exposes the raw material. */

[LOW] src/game/World.ts:331-333 — applyWaterQuality comment references a non-existent animated shader
Confidence: High
Description: The JSDoc says "The full animated shader is layered on top in the water-rendering pass", but no animated water shader exists in the codebase.
Impact: Confusing for future maintainers; suggests a rendering layer that is not implemented.
Suggested Fix: Update the comment to match the actual implementation:

/**
 * Apply the water tier to the shared water material. Centralised so the water
 * quality change and a world recreate both call it. Only alpha changes between tiers.
 */

✅ Fixed Issues From Previous Review

• ✅ applyGraphicsPreset() now calls presetRenderDistance() (src/game/Game.ts:282-285).
• ✅ World.waterStats() now imports and uses WATER_BLOCK (src/game/World.ts:28,670).
• ✅ ChunkBorderOverlay pool is sized for the viewDistance + 2 unload margin and uses a preallocated Float32Array with updateVerticesData() (src/ui/ChunkBorderOverlay.ts:19,111).
• ✅ Stale comments about opaque back-face culling and water brightness have been updated.
• ✅ Sky.setCloudsEnabled() dead code removed and Clouds.addQuad parameter shadowing fixed.
• ✅ Redundant receiveShadows = false assignment in World.applyMesh() removed.
• ✅ __voxl.shadows() now routes through applySettings() so persistence and UI stay in sync (src/game/Game.ts:1137-1142).
• ✅ ShadowManager.configure() now skips teardown/rebuild when the resolved config is unchanged (src/game/lighting/ShadowManager.ts:140-158).
• ✅ Foliage cull and forEachOpaqueMesh now read cached cx/cz from ChunkMeshes instead of parsing map keys (src/game/World.ts:481-483,576-580).
• ✅ Clouds.bindFog() reuses a scratch Vector2 instead of allocating per frame (src/engine/Clouds.ts:115,287-290).
• ✅ GraphicsSettings.migrateGraphics() now clamps dprCap to the documented [1.5, 2] range (src/game/graphics/GraphicsSettings.ts:135).
• ✅ Game.applySettings() no longer mutates the incoming patch object (src/game/Game.ts:249-254).
• ✅ World.opaqueMaterial getter comment now correctly describes double-sided rendering (src/game/World.ts:138).
• ✅ World constructor comment now describes the plain StandardMaterial water pass (src/game/World.ts:131-135).
• ✅ migrateGraphics() now rejects a boolean parsed.clouds value and only accepts strings (src/game/graphics/GraphicsSettings.ts:127-131).
• ✅ Dead exports graphicsEqual and isBuiltinPreset have been removed.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	7	GraphicsController cleanly separates settings data from engine application; Game.ts accumulates many debug hooks but they are isolated behind _ methods.
Open/Closed	6	New quality tiers are added via switch statements; extending them requires editing multiple files.
Liskov Substitution	8	No inheritance hierarchies introduced; new materials conform to existing usage patterns.
Interface Segregation	7	WaterMaterial exposes unused setDayNight/setFog no-ops purely for API compatibility.
Dependency Inversion	7	Settings are plain data; controller depends on Babylon types, but world/scene coupling is already established.
Average	7.0

🎯 Final Assessment

Overall Confidence Score: 88%

Confidence Breakdown:

• Code Quality: 88% (clean, well-commented, typechecks and builds; only two stale doc comments remain)
• Completeness: 92% (feature-complete per PR description; presets, water fix, debug tooling all wired)
• Risk Level: 85% (large additive PR but low game-logic risk; debug-only helpers are the main operational concern)
• Test Coverage: 45% (manual-test checklist recommended; light tests pass; no automated tests added)

Merge Readiness:

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

Verdict:

MERGE WITH FIXES — Address the two remaining stale water-animation comments before merging; all substantive issues from previous rounds are resolved.

opencode session  |  github run