[R2-6][P2] Replace production catch unreachable in worldgen lighting with try

[MichaelFisher1997]

Part of #839 — [Audit][Round 2] A++ umbrella.
Phase: P2 · Finding: R2-6

Problem

Two registered production generators use catch unreachable on lighting computation. Any allocation failure inside LightingComputer will @panic the game for players using the flat or shadow-test world types. This is the riskiest error-handling pattern in the codebase.

Evidence

modules/worldgen-flat/src/root.zig:62 (verified at HEAD):

LightingComputer.computeSkylight(chunk, self.allocator) catch unreachable;

modules/worldgen-test/src/root.zig:59-60 (per Round 2 audit; verify exact lines during fix):

LightingComputer.computeSkylight(chunk, self.allocator) catch unreachable;
LightingComputer.computeBlockLight(chunk, self.allocator) catch unreachable;

Both FlatWorldGenerator and ShadowTestWorldGenerator are registered in world-worldgen/src/registry.zig:21 and selectable by players.

For context: the rest of the codebase has 11 production catch unreachable total, 8 of which are defensible (bounds-checked indexing after a length check, font glyph indices clamped to a known range). These lighting ones are not — LightingComputer.computeSkylight allocates working buffers for BFS flood fill and can return error.OutOfMemory.

Fix

Three steps:

1. Make generator interface error-aware. The generator generate signature already returns error{OutOfMemory} (or similar) in some paths — confirm and propagate. Change catch unreachable to try:

   try LightingComputer.computeSkylight(chunk, self.allocator);
   try LightingComputer.computeBlockLight(chunk, self.allocator);

2. Define a WorldgenError set in worldgen-api (or extend existing RegistryError):

   pub const WorldgenError = error{
       OutOfMemory,
       ChunkAllocationFailed,
       LightingComputationFailed,
   };

   Map from LightingComputer's native error set in one place.

3. Caller-side recovery. WorldStreamer and chunk_queue_coordinator already handle error.OutOfMemory by requeuing the chunk (chunk_queue_coordinator.zig:326-353). Verify the new propagated error reaches that path and is handled gracefully — the chunk stays in pending state and is retried next frame, instead of crashing the game.

Verification

• nix develop --command zig build test
• nix develop --command zig build test -- --test-filter "LightingComputer"
• nix develop --command zig build test -- --test-filter "FlatWorld" and --test-filter "ShadowTest"
• Add a property test: inject a failing allocator (use std.testing.FailingAllocator) into FlatWorldGenerator.generate and assert the error propagates cleanly (no panic).
• Manually launch the flat world type via -Dauto-world=flat -Dskip-present to confirm it still generates.

Constraints

• Behavior change is limited to: lighting allocation failure → recoverable error instead of panic. No visual change in the success path.
• Small reviewable PR targeting dev; conventional commit (fix(worldgen): propagate lighting allocation failure instead of panic).
• Do not change LightingComputer's algorithm — only error propagation.

Notes

• Other concerning silent-failure patterns to address in the same PR (or follow-up):
  • game-ui/screens/world_list.zig:364,384,413 — silent catch {} on world delete/clear/rename (covered in R2-8)
  • game-core/src/player.zig:386,405 — silent catch {} on block mutation (covered in R2-8)
• Out of scope: the other 8 defensible catch unreachable (font/bounds-checked indexing). Leave them.

Tracking: #839

[github-actions]

Comment posted on the issue at #1 (comment)

Summary of analysis:

• Classified: Bug (production catch unreachable that can panic a player session on memory pressure)
• Validated: All three evidence citations verified at HEAD — exact code present at the cited file paths and lines.
• Refined the proposed fix: The audit's Step 1 (changing the Generator.VTable.generate signature to return an error) and Step 2 (introducing a new WorldgenError set) are unnecessary. The codebase already has two working precedents (overworld_generator.zig:230-239 and overworld-v2/root.zig:159) that handle the same condition with catch return / catch |err| { log; return; } — and the caller-side recovery (chunk_queue_coordinator.zig:288-290, world_streamer.zig:216-219) already detects chunk.generated == false and requeues the chunk. A 10-line, 2-file change matching that idiom is reviewable and avoids a wide VTable ripple.
• Concrete next steps: Provided exact try-replacement code for both generators, plus the recommended std.testing.FailingAllocator regression test that proves no panic on allocation failure.
• Scope: Confirmed the issue is appropriately scoped as a single small PR; flagged that R2-8 (world_list.zig/player.zig silent catch {}) and the defensible catch unreachable cases should stay out of this PR.

opencode session  |  github run