Voxel lighting + day/night cycle (sun, moon, two-channel shading)

[MichaelFisher1997]

Adds a full Minetest/Luanti-style voxel lighting system and a day/night cycle with a visual sun and moon. Built on the Babylon.js version of the game; does not touch inventory/survival/creative.

What's included

Voxel light engine (src/game/lighting/)

• Two per-voxel channels (sun + block light), 0–15, stored per chunk in typed arrays.
• SunLightPropagator: top-down sky exposure + BFS flood fill. Sunlight streams straight down through air/glass with no decay and attenuates sideways/up; water and leaves break the column so depth and canopies dim naturally. Cross-chunk boundary inflow keeps chunk seams correct. The topmost light-passing-but-not-sun-passing cell (water/leaves at the world ceiling) is seeded with one decay step so it isn't pitch-black.
• BlockLightPropagator: emitter BFS decaying 1/step through light-passing cells (opaque = wall). Glowstone (id 28) emits 15.
• VoxelLightEngine recomputes a chunk from scratch with a border-change diff that queues neighbours; reusable scratch buffers (zero alloc). Relighting is budgeted (3 chunks/frame) and only runs when something changes.

Block registry lighting fields — BlockLightDefinition (lightPassesThrough, sunlightPassesThrough, lightEmission, lightAbsorption, casts/receivesShadows) with a cached resolveLight(). Data-driven, never hardcoded in the mesher.

Day/night cycle

• DayNightCycle: timeOfDay (0=midnight, 0.25=sunrise, 0.5=noon, 0.75=sunset), configurable day length + timeScale, pause/resume, preset helpers. Sun orbits east→west with a slight tilt; moon is the antipode. dayFactor/moonFactor and a multi-stop sky gradient are recomputed each frame and pushed to the Babylon lights, sky dome, and fog (using reused scratch vectors — no per-frame allocation).
• CelestialSystem: camera-anchored sun disc + additive halo + cratered moon billboards that fade at the horizon and warm at golden hour. Stay in rendering group 0 (set once) so terrain occludes them; never cast/receive shadows.
• VoxelTerrainMaterial: custom two-channel ShaderMaterial. Sun and block light are baked into vertex-colour .r/.g; the shader combines max(sun*dayFactor, sun*moonFloor, block). So night dims outdoor terrain without touching torch/glowstone (block) light and without rebuilding any chunk mesh — only uniforms update per frame. Linear fog is manual.

Shadows — ShadowManager retained but disabled by default (terrain uses voxel sunlight), which permanently retires the earlier rectangular-frustum artifact. Its render-list diff is allocation-free.

Debug tooling — overlay (L), raw sun/block/combined view (K, now a uniform — no remesh), pause (T), shadow toggle (H), time presets ([ / ]), speed (O / I), midnight/midday (; / '). __voxl.lighting() dumps live state + the shadow render list.

Fixes along the way

• Shadow-frustum rectangle artifact (frustumEdgeFalloff + caster margin + centred frustum), then disabled terrain shadow receiving in favour of voxel sunlight.
• Custom terrain shader compile error (uTexture was listed as both uniform and sampler; switched to inline vertexSource/fragmentSource).
• Sun/moon showing through blocks (they were in a higher rendering group whose depth buffer was cleared).
• World-top skylight for water/leaves at the ceiling (Phase A now seeds the surface cell).

Verification

• bun run typecheck and bun run build (game) and bun run typecheck (website, astro check) all pass.
• bun run test:light — 7 propagation scenarios / 15 assertions pass (open shaft, enclosed pocket, glowstone decay, cross-chunk bleed, canopy attenuation, water depth, world-top water boundary).

Known limitations / TODOs

• Terrain shadowing is top-down voxel sunlight, not sun-angle-angled shadow maps.
• Water still uses StandardMaterial (dims via Babylon lights).
• No stars / moon phases (texture-only cratered moon).

Manual test checklist

1. Noon: open terrain bright, sun overhead, blue sky.
2. Sunrise/Sunset: warm horizon, sun low east/west, purple zenith.
3. Midnight: dark, moon visible, dim moonlit ground.
4. Cave at day/night: walls dark; glowstone radius stays bright at night (block channel unaffected).
5. Walk chunk boundaries at dusk — no lighting seams.
6. Fast-forward (O) a full day — no hitching (uniforms only).
7. L overlay shows time/sun dir; K raw channels.

[github-actions]

opencode session  |  github run

[github-actions]

📋 Summary

No linked issues are referenced in the PR (closingIssuesReferences is empty).

PR #1 adds a Minetest/Luanti-style two-channel voxel light engine (sun + block light), a day/night cycle with celestial billboards, a custom terrain shader for cheap time-of-day dimming, and debug tooling. The game package typechecks and builds cleanly, and the website package builds successfully. However, the website typecheck fails due to a pre-existing missing optional playwright dependency, and the propagation tests mentioned in the PR description are not present in the repo.

📌 Review Metadata

• Reviewed Commit SHA: 93934776ca89aa8a4c01992e5d488e2354a3172d
• Reviewed PR: Voxel lighting + day/night cycle (sun, moon, two-channel shading) #1
• Linked Issues: None

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

⚠️ High Priority Issues (Should Fix)

[HIGH] src/game/lighting/SunLightPropagator.ts:49-80 - World-top sunlight is lost for non-sun-passing light-passing blocks
Confidence: High
Description: Phase A seeds open-sky columns, but when the topmost cell is lightPassesThrough but not sunlightPassesThrough (e.g., water or leaves at y = CHUNK_HEIGHT - 1), it closes the column without seeding that cell. There is no vertical top-edge inflow in Phase A2, so the cell stays dark. The PR's own comment at pullInflow claims a top-edge straight-down rule exists, but the code only handles horizontal boundaries.
Impact: Blocks at the world ceiling (e.g., a water column or leaf canopy built at the top) are incorrectly pitch-black on their sides/bottoms; caves/openings that reach the ceiling receive no skylight.
Suggested Fix: In Phase A, when skyExposed is true and the current block is non-sunlightPassesThrough but lightPassesThrough, seed it with LIGHT_MAX - 1 - absorption and enqueue it before closing the column. This matches the downward spread rule and correctly lights the world-top boundary.

💡 Medium Priority Issues (Nice to Fix)

[MEDIUM] website/scripts/screenshot.ts:23 - Website typecheck fails because playwright is not installed
Confidence: High
Description: bun run typecheck in website/ exits with Cannot find module 'playwright'. This is pre-existing and unrelated to the lighting changes (screenshot.ts is not touched), but it is a documented validation command and currently fails.
Impact: Local/CI validation of the website package fails; contributors may think the PR broke it.
Suggested Fix: Either install the optional dependency (bun add -D playwright and run bunx playwright install chromium) or exclude scripts/screenshot.ts from astro check via website/tsconfig.json.

[MEDIUM] src/game/lighting/LightingSystem.ts:161-164 - LightingSystem peeks a private World property via as unknown
Confidence: High
Description: lightDirtyCount() casts this.world to an ad-hoc type to read the private lightDirty Set size.
Impact: Brittle coupling; a rename or refactor of World.lightDirty will silently break the debug overlay's queue count.
Suggested Fix: Add a public getter to World:

get lightDirtyCount(): number { return this.lightDirty.size; }

Then use this.world.lightDirtyCount in LightingSystem.

[MEDIUM] PR description claims 14 propagation tests pass, but no tests are committed
Confidence: High
Description: The PR body says "14 standalone propagation tests pass", but the repository contains no test files or test runner script.
Impact: The verification is not reproducible and there is no regression coverage for the new light engine.
Suggested Fix: Commit the propagation tests or add a minimal bun test suite covering open shaft, enclosed pocket, glowstone decay, cross-chunk bleed, canopy attenuation, and the world-top boundary case.

[MEDIUM] src/game/lighting/VoxelLightEngine.ts:138-140 - Empty early-exit branch wastes a full chunk scan
Confidence: Medium
Description: After changed && borderChanged are both true, the diff scan continues over the remaining voxels.
Impact: Minor but unnecessary CPU work every relight (24k voxels × budget).
Suggested Fix: Break out of the loops once both flags are true, or remove the dead if block.

[MEDIUM] src/game/lighting/SunLightPropagator.ts:183,190 - Unused sunPass variable and misleading comment
Confidence: Medium
Description: pullInflow computes sunPass and then immediately void sunPass;. The function comment also describes a top-edge straight-down rule that is not implemented.
Impact: Dead code and documentation drift; the reader expects vertical boundary handling that does not exist.
Suggested Fix: Remove the variable and comment, or implement top-edge inflow (see the High issue above).

[MEDIUM] src/game/lighting/CelestialSystem.ts:121-123 - renderingGroupId is set every frame; halo render-order intent is unimplemented
Confidence: Low
Description: update() assigns renderingGroupId = 0 every frame although it never changes. The large comment says the halo must render before the disc, but no renderingOrder is set.
Impact: Wasted per-frame work; possible halo-over-disc z-fighting if both use depth-write-off.
Suggested Fix: Set renderingGroupId = 0 in makeDisc, and set sunHalo.renderingOrder = -1 / sunDisc.renderingOrder = 1 to realize the documented ordering.

ℹ️ Low Priority Suggestions (Optional)

[LOW] src/game/lighting/LightMap.ts:55-59 - markValid() is unused and dirty is never reset
Description: markValid is not called anywhere, and the dirty flag it sets has no consumer.
Suggested Fix: Remove the method and field, or wire them into VoxelLightEngine to skip already-fresh maps.

[LOW] src/game/lighting/VoxelLightEngine.ts:36-49 - No explicit disposal for light maps
Description: VoxelLightEngine clears maps only when chunks unload; World.dispose() does not clear the engine's internal Map.
Suggested Fix: Add dispose(): void { this.maps.clear(); } and call it from World.dispose().

[LOW] src/game/lighting/DayNightCycle.ts:170-201 - Many temporary Color3/Color4 allocations per frame
Description: apply() creates new colors every frame.
Suggested Fix: Reuse a few scratch Color3/Color4 instances to reduce GC pressure, though the current rate is not a bottleneck.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	7	LightingSystem is a clean facade, but DayNightCycle mixes clock, sky colors, and light intensities.
Open/Closed	8	New blocks append IDs; lighting behavior is data-driven via BlockLightDefinition.
Liskov Substitution	8	No inheritance hierarchy to violate.
Interface Segregation	8	LightAccess, BrightnessSampler, and ShadowConfig are small and focused.
Dependency Inversion	8	Propagators depend on LightAccess, not World; engine uses a block-lookup callback.
Average	7.8

🎯 Final Assessment

Overall Confidence Score: 72%

Confidence Breakdown:

• Code Quality: 85%
• Completeness: 70%
• Risk Level: 75%
• Test Coverage: 30%

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

Game logic is well-structured and the build/typecheck pass, but fix the world-top skylight seed, commit the promised propagation tests, and tidy the encapsulation/dead-code nits before merging.

opencode session  |  github run

[github-actions]

📋 Summary

No linked issues are referenced in the PR body.

PR #1 adds a Minetest/Luanti-style two-channel voxel light engine (sun + block light), a day/night cycle with camera-anchored sun/moon billboards, a custom two-channel terrain shader for time-of-day dimming without remeshing, and debug tooling. The author has addressed almost every issue raised in the previous automated review: world-top skylight seeding is fixed, LightingSystem no longer peeks World's private state, propagation tests are committed, per-frame allocations are removed, and dead code is cleaned up. Game typecheck/build pass, bun run test:light passes, and the website build passes. The only remaining validation failure is the pre-existing website typecheck error caused by the optional playwright dependency.

📌 Review Metadata

• Reviewed Commit SHA: 85c2ff36dc493cc44316a453ade6ff5d35344f27
• Reviewed PR: Voxel lighting + day/night cycle (sun, moon, two-channel shading) #1
• Linked Issues: None

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

✅ All previously reported critical issues have been resolved.

⚠️ High Priority Issues (Should Fix)

None identified.

💡 Medium Priority Issues (Nice to Fix)

[MEDIUM] website/scripts/screenshot.ts:23 - Website typecheck still fails because playwright is not installed
Confidence: High
Description: bun run typecheck in website/ exits with Cannot find module 'playwright'. This is pre-existing and unrelated to the lighting changes, but it is a documented validation command and currently fails. The latest commit message explicitly leaves it untouched as out-of-scope.
Impact: Local/CI validation of the website package fails; contributors may think the PR broke it.
Suggested Fix: Either install the optional dependency (bun add -D playwright and run bunx playwright install chromium) or exclude scripts/screenshot.ts from astro check via website/tsconfig.json.

[MEDIUM] src/game/lighting/LightingSystem.ts:142-143 - LightingSystem bypasses DayNightCycle private fields to read light intensities
Confidence: High
Description: buildDebugInfo() reads this.dayNight["sun"].intensity and this.dayNight["ambient"].intensity, accessing private fields of DayNightCycle via bracket notation. This is the same brittle coupling pattern previously flagged for World.lightDirty (which was fixed in this PR).
Impact: A rename or refactor of DayNightCycle's private light references will silently break the debug overlay's intensity readouts.
Suggested Fix: Add public getters to DayNightCycle:

get sunIntensity(): number { return this.sun.intensity; }
get ambientIntensity(): number { return this.ambient.intensity; }

Then use this.dayNight.sunIntensity and this.dayNight.ambientIntensity in LightingSystem.

[MEDIUM] PR description still claims "14 standalone propagation tests pass" but scripts/lighttest.ts contains 7 scenarios / 15 assertions
Confidence: High
Description: The PR body says "14 standalone propagation tests pass", but the committed test file has 7 top-level scenarios with 15 individual check() assertions. The count mismatch remains even though tests are now committed and passing.
Impact: Verification claim is inaccurate, which erodes confidence in the manual checklist.
Suggested Fix: Update the PR description to match the committed test file (e.g., "7 propagation scenarios / 15 assertions") or add additional scenarios to reach 14.

ℹ️ Low Priority Suggestions (Optional)

[LOW] src/engine/Sky.ts:115-118 - setDomeColours allocates two Vector3 every frame
Description: Called once per frame from LightingSystem.update(), it creates new Vector3 instances for the dome shader uniforms.
Suggested Fix: Keep two scratch Vector3 fields on Sky and mutate them in setDomeColours to avoid per-frame GC pressure.

[LOW] src/game/lighting/ShadowManager.ts:178 - sameSet allocates a Set every frame when shadows are enabled
Description: update() calls sameSet(next, this.casters) each frame; the function builds new Set(b) from the current caster list even when nothing changed.
Suggested Fix: Compare lengths first (already done), then compare element-by-element without constructing a new Set to avoid per-frame allocation while shadows are active.

✅ Previously Reported Issues — Status

Previous Issue	Status
SunLightPropagator world-top sunlight lost for non-sun-passing light-passing blocks	✅ [FIXED] Seeded at LIGHT_MAX - 1 - absorption in Phase A; world-top water test passes.
LightingSystem peeked World.lightDirty via as unknown	✅ [FIXED] World now exposes public lightDirtyCount getter.
No propagation tests committed	✅ [FIXED] scripts/lighttest.ts added with bun run test:light; all pass.
VoxelLightEngine wasted chunk scan with empty early-exit	✅ [FIXED] Now breaks the y-loop once both flags are set.
SunLightPropagator unused sunPass / misleading comment	✅ [FIXED] sunPass is used in spread; comment clarified.
CelestialSystem renderingGroupId set every frame	✅ [FIXED] Set once in makeDisc.
LightMap.markValid() unused / dirty never reset	✅ [FIXED] Removed.
VoxelLightEngine no explicit disposal	✅ [FIXED] dispose() added and called from World.dispose().
DayNightCycle per-frame Color3/Color4 allocations	✅ [FIXED] Scratch instances reused.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	8	LightingSystem is a clean facade; DayNightCycle still mixes clock, colors, and light intensities but is better isolated.
Open/Closed	8	New blocks append IDs; lighting behavior is data-driven via BlockLightDefinition.
Liskov Substitution	9	No inheritance hierarchy to violate.
Interface Segregation	8	LightAccess, BrightnessSampler, and ShadowConfig are small and focused.
Dependency Inversion	8	Propagators depend on LightAccess, not World; engine uses a block-lookup callback.
Average	8.2

🎯 Final Assessment

Overall Confidence Score: 82%

Confidence Breakdown:

• Code Quality: 88% (well-structured, previous review nits mostly addressed)
• Completeness: 85% (feature-complete per PR scope, debug overlay + tests present)
• Risk Level: 75% (large new subsystem touching meshing, shader, and world streaming)
• Test Coverage: 65% (standalone propagation tests exist; no integration/visual tests)

Merge Readiness:

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

Verdict:

MERGE WITH FIXES

The lighting system is in good shape and the previous review feedback has been thoroughly addressed. Before merging, fix the DayNightCycle private-field access in LightingSystem and update the PR description/test count claim; the website typecheck failure is pre-existing and can be handled separately.

opencode session  |  github run

[github-actions]

📋 Summary

No linked issues are referenced in the PR body (closingIssuesReferences is empty).

PR #1 adds a Minetest/Luanti-style two-channel voxel light engine (sun + block light), a day/night cycle with camera-anchored sun/moon billboards, a custom two-channel terrain shader for time-of-day dimming without remeshing, and debug tooling. The author has addressed every substantive issue raised in the previous automated reviews: world-top skylight seeding is fixed, encapsulation violations are resolved via public getters, propagation tests are committed and passing, per-frame allocations are removed, dead code is cleaned up, and the website typecheck failure caused by the optional playwright dependency is resolved by excluding website/scripts. Game typecheck/build pass, bun run test:light passes, and the website typecheck/build both pass.

📌 Review Metadata

• Reviewed Commit SHA: 8efb89540f7f2e3309c8624fee9a73e56c9f66f3
• Reviewed PR: Voxel lighting + day/night cycle (sun, moon, two-channel shading) #1
• Linked Issues: None

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

✅ All previously reported critical issues have been resolved.

⚠️ High Priority Issues (Should Fix)

None identified.

💡 Medium Priority Issues (Nice to Fix)

[MEDIUM] PR description still claims "14 standalone propagation tests pass" but scripts/lighttest.ts contains 7 scenarios / 15 assertions
Confidence: High
Description: The PR body says "14 standalone propagation tests pass", but the committed test file has 7 top-level scenarios with 15 individual check() assertions. This mismatch remains even though the tests are now committed and passing.
Impact: Verification claim is inaccurate, which erodes confidence in the manual checklist.
Suggested Fix: Update the PR description to match the committed test file (e.g., "7 propagation scenarios / 15 assertions") or add additional scenarios to reach 14.

ℹ️ Low Priority Suggestions (Optional)

[LOW] src/game/lighting/CelestialSystem.ts:102-120 - Per-frame Vector3 and Color3 allocations
Confidence: Medium
Description: update() creates new Vector3s via cameraPosition.add(...) / .scale(...) and new Color3s via Color3.Lerp every frame.
Impact: Minor GC pressure in a per-frame hot path.
Suggested Fix: Add scratch Vector3s to the class and use scaleToRef/addToRef, and reuse two scratch Color3s for the sun/halo tints.

[LOW] src/game/lighting/LightMap.ts:31-33 - Stale JSDoc references removed dirty field
Confidence: High
Description: The class comment describes a dirty flag that no longer exists on LightMap (it was removed in a previous cleanup pass).
Impact: Documentation drift; confusing to readers.
Suggested Fix: Remove the sentence about dirty from the JSDoc.

[LOW] src/game/ChunkMesher.ts:209-246 - Unused base parameter in pushCross
Confidence: High
Description: pushCross captures const base = b.vertexCount but never uses it, then void base; at the end.
Impact: Dead code / lint noise.
Suggested Fix: Remove the base variable and the void base; line.

[LOW] src/game/World.ts:358-386 - applyMesh ignores its _existing parameter
Confidence: High
Description: rebuildMesh passes existing into applyMesh, but the function re-fetches the entry from this.meshes and never reads _existing.
Impact: Misleading API; dead parameter.
Suggested Fix: Drop the _existing parameter and have rebuildMesh call applyMesh(k, slot, result[slot], material).

✅ Previously Reported Issues — Status

Previous Issue	Status
SunLightPropagator world-top sunlight lost for non-sun-passing light-passing blocks	✅ [FIXED] Seeded at LIGHT_MAX - 1 - absorption in Phase A; world-top water test passes.
LightingSystem peeked World.lightDirty via as unknown	✅ [FIXED] World now exposes public lightDirtyCount getter.
LightingSystem bypassed DayNightCycle private light fields	✅ [FIXED] DayNightCycle now exposes public sunIntensity/ambientIntensity getters.
Website typecheck failed because playwright was not installed	✅ [FIXED] website/tsconfig.json now excludes scripts/.
No propagation tests committed	✅ [FIXED] scripts/lighttest.ts added with bun run test:light; all pass.
VoxelLightEngine wasted chunk scan with empty early-exit	✅ [FIXED] Now breaks the y-loop once both flags are set.
SunLightPropagator unused sunPass / misleading comment	✅ [FIXED] sunPass is used in spread; comment clarified.
CelestialSystem renderingGroupId set every frame	✅ [FIXED] Set once in makeDisc.
LightMap.markValid() unused / dirty never reset	✅ [FIXED] Removed.
VoxelLightEngine no explicit disposal	✅ [FIXED] dispose() added and called from World.dispose().
DayNightCycle per-frame Color3/Color4 allocations	✅ [FIXED] Scratch instances reused.
Sky.setDomeColours allocated Vector3 every frame	✅ [FIXED] Reuses two scratch Vector3s.
ShadowManager.sameSet allocated a Set every frame	✅ [FIXED] Allocation-free O(n²) membership check.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	8	LightingSystem is a clean facade; DayNightCycle still mixes clock, colors, and light intensities but is better isolated.
Open/Closed	8	New blocks append IDs; lighting behavior is data-driven via BlockLightDefinition.
Liskov Substitution	9	No inheritance hierarchy to violate.
Interface Segregation	8	LightAccess, BrightnessSampler, and ShadowConfig are small and focused.
Dependency Inversion	8	Propagators depend on LightAccess, not World; engine uses a block-lookup callback.
Average	8.2

🎯 Final Assessment

Overall Confidence Score: 85%

Confidence Breakdown:

• Code Quality: 88% (well-structured, previous review nits mostly addressed)
• Completeness: 85% (feature-complete per PR scope, debug overlay + tests present)
• Risk Level: 75% (large new subsystem touching meshing, shader, and world streaming)
• Test Coverage: 65% (standalone propagation tests exist; no integration/visual tests)

Merge Readiness:

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

Verdict:

MERGE WITH FIXES

The lighting system is in good shape and all substantive review feedback has been addressed. Before merging, update the PR description/test count claim; the remaining low-priority nits are optional cleanups.

opencode session  |  github run

[github-actions]

📋 Summary

No linked issues are referenced in the PR body (closingIssuesReferences is empty).

PR #1 adds a Minetest/Luanti-style two-channel voxel light engine (sun + block light), a day/night cycle with camera-anchored sun/moon billboards, a custom two-channel terrain shader for time-of-day dimming without remeshing, and debug tooling. The author has addressed every substantive issue raised in the previous automated reviews: world-top skylight seeding is fixed, encapsulation violations are resolved via public getters, propagation tests are committed and passing, per-frame allocations are removed, dead code is cleaned up, and the website typecheck failure caused by the optional playwright dependency is resolved by excluding website/scripts. Game typecheck/build pass, bun run test:light passes, website typecheck passes, and the website build passes. The only remaining items are four low-priority cleanups carried over from the previous review.

📌 Review Metadata

• Reviewed Commit SHA: decf91936322040dd0ffb1201171e3cbdec0b80c
• Reviewed PR: Voxel lighting + day/night cycle (sun, moon, two-channel shading) #1
• Linked Issues: None

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

✅ All previously reported critical issues have been resolved.

⚠️ High Priority Issues (Should Fix)

None identified.

💡 Medium Priority Issues (Nice to Fix)

None identified.

✅ [FIXED] Previous issue: Website typecheck failed because playwright was not installed — website/tsconfig.json now excludes scripts/.

✅ [FIXED] Previous issue: LightingSystem bypassed DayNightCycle private light fields — DayNightCycle now exposes public sunIntensity/ambientIntensity getters.

✅ [FIXED] Previous issue: PR description claimed "14 standalone propagation tests pass" — the PR body now correctly states "7 propagation scenarios / 15 assertions pass".

ℹ️ Low Priority Suggestions (Optional)

[LOW] src/game/lighting/CelestialSystem.ts:105,109,118,120 - Per-frame Vector3 and Color3 allocations
Confidence: Medium
Description: update() creates new Vector3s via cameraPosition.add(...) / .scale(...) and new Color3s via Color3.Lerp every frame.
Impact: Minor GC pressure in a per-frame hot path.
Suggested Fix: Add scratch Vector3s to the class and use scaleToRef/addToRef, and reuse two scratch Color3s for the sun/halo tints.

[LOW] src/game/lighting/LightMap.ts:31-33 - Stale JSDoc references removed dirty field
Confidence: High
Description: The class comment describes a dirty flag that no longer exists on LightMap (it was removed in a previous cleanup pass).
Impact: Documentation drift; confusing to readers.
Suggested Fix: Remove the sentence about dirty from the JSDoc.

[LOW] src/game/ChunkMesher.ts:213,245 - Unused base parameter in pushCross
Confidence: High
Description: pushCross captures const base = b.vertexCount but never uses it, then void base; at the end.
Impact: Dead code / lint noise.
Suggested Fix: Remove the base variable and the void base; line.

[LOW] src/game/World.ts:363 - applyMesh ignores its _existing parameter
Confidence: High
Description: rebuildMesh passes existing into applyMesh, but the function re-fetches the entry from this.meshes and never reads _existing.
Impact: Misleading API; dead parameter.
Suggested Fix: Drop the _existing parameter and have rebuildMesh call applyMesh(k, slot, result[slot], material).

✅ Previously Reported Issues — Status

Previous Issue	Status
SunLightPropagator world-top sunlight lost for non-sun-passing light-passing blocks	✅ [FIXED] Seeded at LIGHT_MAX - 1 - absorption in Phase A; world-top water test passes.
LightingSystem peeked World.lightDirty via as unknown	✅ [FIXED] World now exposes public lightDirtyCount getter.
LightingSystem bypassed DayNightCycle private light fields	✅ [FIXED] DayNightCycle now exposes public sunIntensity/ambientIntensity getters.
Website typecheck failed because playwright was not installed	✅ [FIXED] website/tsconfig.json now excludes scripts/.
PR description test count mismatch	✅ [FIXED] PR body now reads "7 propagation scenarios / 15 assertions pass".
No propagation tests committed	✅ [FIXED] scripts/lighttest.ts added with bun run test:light; all pass.
VoxelLightEngine wasted chunk scan with empty early-exit	✅ [FIXED] Now breaks the y-loop once both flags are set.
SunLightPropagator unused sunPass / misleading comment	✅ [FIXED] sunPass is used in spread; comment clarified.
CelestialSystem renderingGroupId set every frame	✅ [FIXED] Set once in makeDisc.
LightMap.markValid() unused / dirty never reset	✅ [FIXED] Removed.
VoxelLightEngine no explicit disposal	✅ [FIXED] dispose() added and called from World.dispose().
DayNightCycle per-frame Color3/Color4 allocations	✅ [FIXED] Scratch instances reused.
Sky.setDomeColours allocated Vector3 every frame	✅ [FIXED] Reuses two scratch Vector3s.
ShadowManager.sameSet allocated a Set every frame	✅ [FIXED] Allocation-free O(n²) membership check.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	8	LightingSystem is a clean facade; DayNightCycle still mixes clock, colors, and light intensities but is better isolated.
Open/Closed	8	New blocks append IDs; lighting behavior is data-driven via BlockLightDefinition.
Liskov Substitution	9	No inheritance hierarchy to violate.
Interface Segregation	8	LightAccess, BrightnessSampler, and ShadowConfig are small and focused.
Dependency Inversion	8	Propagators depend on LightAccess, not World; engine uses a block-lookup callback.
Average	8.2

🎯 Final Assessment

Overall Confidence Score: 85%

Confidence Breakdown:

• Code Quality: 88% (well-structured, previous review nits mostly addressed)
• Completeness: 90% (feature-complete per PR scope, debug overlay + tests present, PR description now accurate)
• Risk Level: 75% (large new subsystem touching meshing, shader, and world streaming)
• Test Coverage: 65% (standalone propagation tests exist; no integration/visual tests)

Merge Readiness:

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

Verdict:

MERGE WITH FIXES

The lighting system is in good shape and all substantive review feedback has been thoroughly addressed. The remaining four low-priority cleanups are optional but worth tidying before merge.

opencode session  |  github run