Complete roadmap foundations and optimize chunk streaming

[MichaelFisher1997]

Summary

• add asynchronous loading presentation, first-run hints, motion/camera settings, falling-block physics, migration coverage, asset auditing, and dependency-free unit tests
• prioritize generation and meshing nearest the player, reduce population/mesher CPU work, preserve streaming while menus pause simulation, and debounce render-distance changes
• add default-off 2x2 compact-LOD render batching plus deterministic CPU/GPU gameplay profiling; no horizons or Full Detail quality reductions
• update ROADMAP and architecture/performance documentation with measured results, limitations, and review boundaries
• protect cross blocks from silent gravity replacement and prevent unsupported future session metadata from being downgraded by autosave

Verification

• redot --editor --headless --path . --quit
• complete local equivalents of all four .github/workflows/tests.yml shards
• redot --headless --path . --script res://tools/stream_full_verify.gd -- --async-start (441/441 chunks; 7.99 s settled; 346.5 ms safe ring)
• rendered Forward+ gameplay profiler at RD10/RD16 on RX 5700 XT
• git diff --check

Notes

• Experimental LOD batching is disabled by default because draw-call reductions did not consistently improve frame time.
• Render profiling identifies sun shadows as the largest measured steady-state GPU cost but does not lower shadow quality or disable effects.
• Horizon impostors remain unimplemented and open on the roadmap.

[github-actions]

📋 Summary

No linked issues ("Fixes #N") are referenced in the PR body; the work is tracked against ROADMAP.md items instead (Loading progress, First-run hints, Motion/camera options, Block physics, Unit tests, Migration tests, Asset/license check, Chunk merging — all updated in this PR, with honest "experimental/off by default" caveats for LOD batching and a remaining "branch protection pending" note for unit tests).

This PR lands eight roadmap foundations (async world-entry presentation, first-run hints, per-axis camera settings + reduced motion, persisted falling-block physics for sand/red sand/gravel, storage-migration fixtures, headless asset/license audit, dependency-free unit tests) plus a substantial chunk-streaming optimization pass (min-heap nearest-first scheduling, population pruning, mesher micro-optimizations, pause-resilient streaming, render-distance debounce) and a default-off experimental 2x2 compact-LOD render batcher. The implementation is disciplined about the repo's hardest invariants — worker-thread immutability, edit-version/config-revision commit checks, LOD refusing edits, determinism via global anchors and pre-optimization golden hashes — and ships a matching verifier for nearly every new behavior (12 new/extended tools, all wired into the CI shards).

📌 Review Metadata

• Reviewed Commit SHA: 5e088adde94f031f7d64068bcf83fe8d3441179d
• Reviewed PR: Complete roadmap foundations and optimize chunk streaming #25

Invariants I specifically verified in the full files (not just the diff):

• _schedule_jobs keeps the bounded overflow contract: the hard pending > _max_active_jobs break at world/voxel_world.gd:663 still caps total jobs at _max_active_jobs + 1; urgent dispatch beyond the soft cap can only use that one slot.
• The heap rewrite (_work_push/_work_pop/_work_sift_down/_next_work_is_mesh) maintains the queue↔membership-map invariant across _rebuild_desired (which clears and requeues all dirty chunks), and priority keys depend only on _stream_center + the edited flag.
• Mesher optimizations are exact-equivalence rewrites: max(a,b) < L → a < L && b < L (world/worldgen/voxel_populator.gd:559), per-y limit tables bounded to the carve loop's y ≤ SEA_LEVEL+52 (tunnel_limits size 101, max index 100), and the removed light-volume x/z bounds check in _append_face is safe because AO offsets are ±1 per axis from a centered 16-wide tile inside the 48-wide volume (confirmed volume.w = volume.d = CHUNK_SIZE*3, LIGHT_PAD = 16). Population pruning predicates are position-global and order-independent, and pre-pruning golden hashes in tools/population_benchmark.gd / mesh_parity_verify.gd pin identical output.
• Gravity: _flush_gravity_changes runs once per tick (matching the water-tick pattern), every move goes through _record_edit() on both cells, LOD/unloaded chunks are refused in _queue_gravity/_update_gravity_cell, cross blocks are support (_is_gravity_passable), and reseed ordering is sorted/deterministic.
• LOD batching: aggregates are read-only presentation (no physics/edits), invalidation reveals sources before hiding, _build_lod_batch reads ARRAY_CUSTOM0/1 which build_lod does emit (verified _append_lod_block_face writes result.light/result.layers), and arrays_to_mesh keeps the custom-format flags.
• Async entry: initial_stream_ready gates on real collision readiness of the 3x3 ring, player is locked via loading_locked (input + _physics_process), and streaming continues while paused because VoxelWorld is PROCESS_MODE_ALWAYS while water/gravity/fire accumulators stay frozen behind get_tree().paused — asserted by stream_transition_verify.gd.
• Future-version session state is protected: _migrate_session_state sets _session_state_read_only, and _flush_world_save skips the metadata flush and the active_world_metadata refresh (region edits still persist, which is additive and version-safe).

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified. ✅ No previous automated reviews existed, so there are no prior issues to re-check; nothing new rises to crash/data-loss severity, and no worker-thread or determinism hazard was found in the new code paths.

⚠️ High Priority Issues (Should Fix)

None identified.

💡 Medium Priority Issues (Nice to Fix)

[MEDIUM] world/voxel_world.gd:1466 - _seed_gravity_edits re-runs on every full-detail commit, rescanning and re-sorting the entire edit bucket
Confidence: Medium
Description: _commit_chunk calls _seed_gravity_edits(pos) on every full-detail mesh commit, and the function sorts the chunk's whole _edits_by_chunk bucket (world/voxel_world.gd:2406-2419) and issues two _queue_gravity checks per edit. A nuke crater (radius 18 ≈ 12k removed cells, several thousand edits per chunk bucket via carve_sphere → _record_edit_in_chunk) triggers many commits over the following seconds (gravity settles → _flush_gravity_changes requeues the chunk plus its light ring each tick), so each commit re-pays an O(edits·log edits) sort with a lambda comparator plus O(edits) dictionary/queue probes on the main thread. Settled sand is also re-queued for no-op gravity passes after every remesh (bounded at 512/tick, but repeated).
Impact: Multi-millisecond main-thread hitches recurring for seconds after large explosions in editable terrain; wasted budget inside the 2 ms commit window. Worst case is proportional to edit-bucket size, so heavily played chunks degrade over time.
Suggested Fix: Reseed once per chunk load rather than per commit — live edits already seed through _seed_gravity(), so the bucket pass only needs to run when edits are (re)hydrated:

var _gravity_seeded_chunks := {}

func _seed_gravity_edits(pos: Vector2i) -> void:
	if _gravity_seeded_chunks.has(pos):
		return
	_gravity_seeded_chunks[pos] = true
	# ... existing sort-and-queue body ...

Clear the entry in _free_chunk/_discard_unloaded_chunk_state and in set_edit_store so unload/reload still resumes interrupted falls (the persisted-resume behavior pinned by tools/block_physics_verify.gd must keep passing).

ℹ️ Low Priority Suggestions (Optional)

[LOW] ui/loading_overlay.gd:140 - Direct GameConfig autoload reference executed from a --script harness
Confidence: Medium
Description: LoadingOverlay.complete() calls GameConfig.is_reduced_motion(). tools/loading_progress_verify.gd:40 invokes complete() from a --script SceneTree harness, where autoload identifiers are unavailable (AGENTS.md documents this; it's why motion_camera_verify and block_physics_verify are .tscn). The identifier resolves to null at runtime, printing an "invalid call" error line; execution continues with false, so the check still passes and run_checks.sh (exit-code based) stays green — but it adds error noise to CI logs and deviates from the lookup pattern this same PR added in ui/motion.gd:5091 (tree.root.get_node_or_null("GameConfig")).
Impact: Misleading runtime errors in the fast-checks shard log; no functional failure.
Suggested Fix: Reuse the root-lookup pattern inside complete():

var config := get_tree().root.get_node_or_null("GameConfig")
if config != null and config.has_method("is_reduced_motion") and bool(config.is_reduced_motion()):
	queue_free()
	return

[LOW] AGENTS.md - Source-of-truth docs not updated for the new verification surface and streaming behaviors
Confidence: High
Description: ROADMAP.md and the worldgen/ui READMEs were updated, but AGENTS.md still enumerates only the pre-PR verifier list, omitting unit_tests.gd, chunk_priority_verify.gd, mesh_parity_verify.gd, population_pruning_verify.gd, population_benchmark.gd, stream_transition_verify.gd, lod_batch_verify.gd, block_physics_verify.tscn, loading_progress_verify.gd, first_run_hints_verify.gd, motion_camera_verify.tscn, and asset_license_verify.gd. It also doesn't mention the persisted gravity queue, the begin_initial_stream()/initial_stream_ready entry contract that Main now depends on, or that VoxelWorld now runs PROCESS_MODE_ALWAYS (streaming/commits continue while the tree is paused — an architectural behavior change reviewers need to know).
Impact: Future sessions will under-verify and miss the new contracts; AGENTS.md is declared the source of truth.
Suggested Fix: Add the new verifiers to the Verification section and one sentence each for the gravity system, async entry contract, and pause-resilient streaming.

[LOW] world/voxel_world.gd:548-559 - initial_stream_ready has no timeout fallback if a spawn-ring chunk can never acquire collision
Confidence: Low
Description: _update_initial_stream_state requires all 3x3 spawn chunks to be non-LOD with a committed collision shape. _ensure_near_collision requeues a full chunk forever if res.collision is empty (the mesh commits without a shape and the loop requeues next tick). Generation always produces ground in every world type, so this cannot happen today — but the new player-lock dependency converts a previously benign requeue churn into a potential permanent entry deadlock (the loading overlay consumes Esc, and Player input is locked).
Impact: Theoretical only; harsh failure mode if a future world type or edit pattern ever yields an all-air full chunk in the spawn ring.
Suggested Fix: After N seconds of active initial streaming without readiness, fall back to setup_player(player, true) (synchronous ring) or unlock on data-committed-only chunks, and surface a status message.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	7	Entry presentation, hints, and profiling are delegated to dedicated nodes/classes; VoxelWorld keeps growing (3,034 lines, now telemetry + batching + gravity), though each subsystem stays cohesive and documented
Open/Closed	8	Provider callables (FirstRunHints, PhotoMode), settings-driven toggles, and the IsolatedProfileMain subclass extend behavior without touching production paths
Liskov Substitution	8	The profiling subclass overrides only persistence seams and preserves Main's contract; LodRenderBatch/heap helpers honor their documented invariants
Interface Segregation	8	Player↔Main remains signal-only; hints/overlay consume narrow dictionaries instead of world internals
Dependency Inversion	7	New code injects callables and reads main-thread snapshots; direct GameConfig autoload references persist in LoadingOverlay (flagged above)
Average	7.6

🎯 Final Assessment

Overall Confidence Score: 82%

The riskiest changes (scheduler heap rewrite, async entry, gravity persistence, mesher strides, population pruning) each carry a dedicated verifier including pre-change golden hashes, and my line-level audit of the surrounding invariants (commit versioning, LOD refusal, worker immutability, determinism) found no violations. The engine binary is unavailable on this runner, so none of the verifiers were re-executed here — the author's listed local runs (all four shards, stream_full_verify --async-start 441/441) are the executed evidence, which is residual but disclosed risk.

Confidence Breakdown:

• Code Quality: 88% (careful comments, exact-equivalence rewrites, typed GDScript, consistent patterns)
• Completeness: 90% (all eight claimed roadmap items implemented with docs and tests; honest about experimental batching)
• Risk Level: 72% (large streaming rewrite + new persisted simulation; well-guarded but only locally verified; one medium perf concern in edited terrain)
• Verification: 78% (12 new/extended verifiers, golden hashes, CI wiring; not runnable in this review environment)

Merge Readiness:

• All critical issues resolved
• SOLID average score >= 6.0
• Overall confidence >= 60%
• No security concerns
• No unresolved worker-thread, determinism, or data-loss risk

Verdict:

MERGE WITH FIXES — Mergeable once the per-commit _seed_gravity_edits rescan is gated (the only Medium finding); the two Low items are polish that can follow.

Machine Readable Verdict

{
  "reviewed_sha": "5e088adde94f031f7d64068bcf83fe8d3441179d",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 1,
  "overall_confidence_score": 82,
  "recommendation": "MERGE WITH FIXES"
}

opencode session  |  github run

[github-actions]

All verification complete. set_edit_store runs in Main._ready() (line 97) before streaming begins (line 119), so the seeded-flag clearing can never miss a hydration. I have everything needed for the final review.

📋 Summary

No linked issues ("Fixes #N") are referenced in the PR body; work is tracked against ROADMAP.md items (Loading progress, First-run hints, Motion/camera options, Block physics, Unit tests, Migration tests, Asset/license check, Chunk merging), all updated in this PR.

This PR lands eight roadmap foundations (async world-entry presentation, first-run hints, per-axis camera settings + reduced motion, persisted falling-block physics, storage-migration fixtures, headless asset/license audit, dependency-free unit tests) plus a chunk-streaming optimization pass (min-heap nearest-first scheduling, population pruning, pause-resilient streaming) and a default-off 2x2 compact-LOD render batcher. Since the previous review, commit c2de5dff ("Avoid repeated gravity edit scans") was pushed, surgically addressing the one Medium and two of the three Low findings; my independent re-audit of the heap invariants, commit versioning, gravity seeding coverage, LOD batching, and mesher equivalence rewrites confirms the previous review's conclusions.

📌 Review Metadata

• Reviewed Commit SHA: c2de5dff7699cff3addcef158543d6d17c10fc1d
• Reviewed PR: Complete roadmap foundations and optimize chunk streaming #25

🔴 Critical Issues (Must Fix - Blocks Merge)

✅ All previously reported critical issues have been resolved (none were ever reported; nothing new rises to this severity).

None identified.

⚠️ High Priority Issues (Should Fix)

✅ All previously reported high priority issues have been resolved (none were ever reported).

None identified.

💡 Medium Priority Issues (Nice to Fix)

✅ [FIXED] Previous issue: _seed_gravity_edits re-ran on every full-detail commit, rescanning/re-sorting the entire edit bucket.
The fix adds _gravity_seeded_chunks gating (world/voxel_world.gd:2413-2416) with correct lifecycle coverage on every state transition I could find:

• _free_chunk erases on unload (world/voxel_world.gd:1613), so reload reseeds and interrupted falls still resume;
• LOD commits erase the entry (world/voxel_world.gd:1469-1472), so full→LOD→full transitions retry candidates that were unavailable while compact;
• set_edit_store clears all (world/voxel_world.gd:375), and in production it only runs in Main._ready() (game/main.gd:97) before streaming starts (game/main.gd:119), so it can never skip a pending hydration.
  The gating is sound because edits are always hydrated before the first full-detail commit (_chunk_edits_for → _hydrate_chunk_edits at both dispatch sites, world/voxel_world.gd:608 and :723), and every live mutation path seeds gravity independently (11 _seed_gravity call sites covering break/place/door/carve_sphere/fire-extinguish/water at lines 1802-2532). The new _check_edit_seeding_once_per_residency in tools/block_physics_verify.gd:161-177 pins the behavior, and its size() == 1 assertion is consistent with _queue_gravity filtering non-gravity cells (world/voxel_world.gd:2441).

None new identified.

ℹ️ Low Priority Suggestions (Optional)

✅ [FIXED] Previous issue: direct GameConfig autoload reference in ui/loading_overlay.gd:140. complete() now uses the root-lookup pattern with a has_method guard (ui/loading_overlay.gd:140-144), matching ui/motion.gd, so the --script harness in tools/loading_progress_verify.gd:40 no longer emits autoload resolution errors.

✅ [FIXED] Previous issue: AGENTS.md not updated for the new verification surface and streaming behaviors. All requested content is present: the verifier list now includes unit_tests.gd, chunk_priority_verify.gd, mesh_parity_verify.gd, population_pruning_verify.gd, stream_transition_verify.gd, lod_batch_verify.gd, asset_license_verify.gd, loading_progress_verify.gd, first_run_hints_verify.gd; the scene-based section covers block_physics_verify.tscn and motion_camera_verify.tscn; the benchmarks line covers population_benchmark.gd and both rendered profiles; the Gravity paragraph, the begin_initial_stream()/initial_stream_ready contract, and PROCESS_MODE_ALWAYS are all documented.

[LOW] world/voxel_world.gd:550-561 - initial_stream_ready still has no timeout fallback if a spawn-ring chunk can never acquire collision (previously reported, unchanged)
Confidence: Low
Description: _update_initial_stream_state still returns early if any 3x3 spawn-ring chunk is missing, LOD, or lacks a committed collision shape, and _ensure_near_collision (world/voxel_world.gd:567-586) requeues such chunks indefinitely. Generation always produces ground in every current world type, so this cannot happen today — but with player input locked and the overlay consuming Esc, a future all-air spawn-ring chunk would deadlock world entry.
Impact: Theoretical only; harsh failure mode contingent on a future world type or edit pattern.
Suggested Fix: After N seconds of active initial streaming without readiness, fall back to setup_player(player, true) (synchronous ring) or unlock on data-committed-only chunks, and surface a status message.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	7	Entry presentation, hints, gravity, and batching each stay in dedicated seams with clear ownership comments; VoxelWorld remains a 3,044-line hub but each subsystem is cohesive
Open/Closed	8	Provider callables (FirstRunHints, PhotoMode camera settings), settings-driven toggles, and the IsolatedProfileMain subclass extend behavior without touching production paths
Liskov Substitution	8	The profiling subclass overrides only persistence seams; LodRenderBatch and heap helpers honor their documented invariants (verified: queue↔membership-map consistency across _rebuild_desired, promote-on-edited sift-up)
Interface Segregation	8	Player↔Main remains signal-only; the overlay consumes narrow progress dictionaries; hint nodes take injected providers instead of world internals
Dependency Inversion	8	The LoadingOverlay autoload fix removes the last flagged direct dependency; providers are injected throughout the new UI/game nodes
Average	7.8

🎯 Final Assessment

Overall Confidence Score: 88%

The only Medium finding was fixed correctly with full lifecycle coverage and a dedicated verifier assertion; my independent audit of the riskiest areas (heap rewrite, commit version checks at world/voxel_world.gd:822, gravity live-seeding coverage, LOD-batch main-thread-only mesh reads with sources-revealed-before-hidden invalidation, and the mesher's sorted sparse-emitter rewrite which provably preserves the original BFS seeding order) found no violations or new issues in c2de5dff. The engine binary is unavailable on this runner, so block_physics_verify.tscn (now including _check_edit_seeding_once_per_residency), loading_progress_verify.gd, and the parse check were not re-executed here — residual, disclosed risk.

Confidence Breakdown:

• Code Quality: 89% (surgical fix with lifecycle comments, typed GDScript, exact-equivalence rewrites documented inline)
• Completeness: 92% (all roadmap items plus fixes for every actionable prior finding; one theoretical Low deferred)
• Risk Level: 78% (large streaming rewrite + persisted simulation, well-guarded with verifiers; experimental batching default-off)
• Verification: 84% (new verifier coverage pins the fixed behavior; not executable in this environment)

Merge Readiness:

• All critical issues resolved
• SOLID average score >= 6.0
• Overall confidence >= 60%
• No security concerns
• No unresolved worker-thread, determinism, or data-loss risk

Verdict:

MERGE — The single Medium finding is correctly fixed with verifier coverage, both actionable Low findings are addressed, and the remaining Low (theoretical initial-stream timeout) is non-blocking.

Machine Readable Verdict

{
  "reviewed_sha": "c2de5dff7699cff3addcef158543d6d17c10fc1d",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 0,
  "overall_confidence_score": 88,
  "recommendation": "MERGE"
}

opencode session  |  github run