Add survival modes, inventory, and crafting progression

[MichaelFisher1997]

Summary

• Add creation-only Creative and Survival modes, persisted per world; Survival starts empty and legacy worlds default to Creative.
• Implement survival vitals and respawn, tiered tools and timed mining feedback, physical drops, crouching, third-person view, and block picking.
• Add stack inventory, crafting tables, chests, furnaces, and versioned save migration.
• Complete all-species log-to-plank progression through stone and iron tools, charcoal, lighting, and prepared foods; add a searchable recipe book with safe batch crafting.
• Preserve older world generation while enabling ores independently of caves in version 14; fix flat-world flooding and warn about Flat Survival resource limits.

Verification

• Installed Redot 26.2 headless editor parse check and 17 focused/regression verifiers passed locally.
• End-to-end progression verified for all six log species through real mining, physical pickup, stations, smelting, and save restoration.
• Rendered isolated smoke tests covered both modes, crafting UI, station transfers, death/respawn, and scaled layouts.
• New regression verifiers are included in CI.

Compatibility and limitations

• Existing IDs remain stable; old inventories migrate and saved modes persist through loading, duplication, and backups.
• Planks currently use a procedural placeholder texture, not newly imported resource-pack artwork.
• Farming, animal-derived Survival meat, and slash commands remain unimplemented. There is no in-game mode switch.
• Detailed progression is documented in CRAFTING.md and ROADMAP.md.

[github-actions]

All verification complete. Here is the review.

📋 Summary

No linked issues ("Fixes #N"/"Closes #N") are referenced in the PR body.

This PR lands the survival layer roadmap items: creation-only Creative/Survival modes persisted in world metadata (session schema v2 with legacy migration), timed tiered mining with tool durability, a 40-slot stack inventory with drag/drop/keyboard transfers, chests/furnaces/crafting tables, physical item drops, survival vitals with death/respawn, crouch/third-person/block-pick verbs, and worldgen v14 (cave-independent ores) plus the flat-world flooding fix. Implementation quality is high: model/UI separation is clean, crafting is atomic and capacity-safe, the seven new verifiers plus updated regression verifiers are wired into CI, and AGENTS/ROADMAP/CRAFTING.md are updated consistently. I found no worker-thread, determinism, duplication, or data-loss defects; the issues below are a convention violation and small gameplay/cosmetic edges.

📌 Review Metadata

• Reviewed Commit SHA: fc3d24375ef933088227e2d944e6a592244eef01
• Reviewed PR: Add survival modes, inventory, and crafting progression #23

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified. Worker safety (all new gameplay code is main-thread; BlockRegistry.break_seconds/can_harvest are static over const data), determinism (v14 gates only stage eligibility with the ore catalog capped at LEGACY_VERSION_MAX, and the v13 byte-identical fixture is pinned), item conservation (atomic InventoryTransfer, single-ownership BlockContainers.remove, overflow-preserving drops), and save migration (v1→v2 inventory with overflow as physical drops, mode persisted in metadata not session) all check out.

⚠️ High Priority Issues (Should Fix)

None identified.

💡 Medium Priority Issues (Nice to Fix)

[MEDIUM] player/player.gd:667, game/main.gd:943, game/main.gd:761 - New code reaches into other nodes' private state and methods
Confidence: High
Description: _update_survival reads world._burning.has(ground_cell) (VoxelWorld's private fire dictionary), _check_removed_containers calls the private world._loaded_chunk_for(), and Main calls the private player._cancel_mining() in _on_inventory_opened (main.gd:761) and select_slot (main.gd:1007). AGENTS.md states "player/player.gd talks to Main only through signals ... Keep that pattern instead of reaching into nodes" — these accesses cut across that boundary in both directions and couple Player/Main to VoxelWorld internals that the fire/streaming systems may refactor.
Impact: Silent breakage if _burning's structure changes (e.g., the 4 Hz fire tick reworking), and erosion of the signal contract that future features (command console, mobs) will rely on.
Suggested Fix: Add small public accessors instead:

# world/voxel_world.gd
func is_burning_at(cell: Vector3i) -> bool:
	return _burning.has(cell)

func is_chunk_resident_at(cell: Vector3i) -> bool:
	var chunk := _loaded_chunk_for(cell)
	return chunk != null and not chunk.lod

and either make cancel_mining() public on Player or have Player observe modal state via an existing/new signal.

ℹ️ Low Priority Suggestions (Optional)

[LOW] world/block_registry.gd:164 + game/item_registry.gd:101 - Fire blocks are harvestable and collectible in Survival
Confidence: High
Description: BLOCK_FIRE has hardness 0.0 / required tier 0, so can_harvest returns true and harvest_drop passes the id through: LMB on a lit flame instantly breaks it and spawns a placeable BLOCK_FIRE stack. Fire is deliberately excluded from the Creative catalog, and this gives free infinite fire placement without flint-and-steel (which has 64-use durability in Survival).
Impact: Minor gameplay inconsistency/exploit; placed fire still decays when unsupported, so impact is contained.
Suggested Fix: Special-case fire in harvest_drop (return {}) or give BLOCK_FIRE hardness -1.0 and cancel mining in _tick_mining (already handled for negative seconds).

[LOW] player/player.gd:550 + game/main.gd:1015 - Empty-hand right-click shows "No AIR left"
Confidence: High
Description: With an empty hotbar slot selected (the default Survival start), RMB on a block fails can_place_selected() and emits "No %s left" with _selected_name() → get_block_name(0) = "AIR".
Impact: Cosmetic; a confusing first toast for new Survival players.
Suggested Fix: In _place_target, return early without a status when selected_block == BlockRegistry.BLOCK_AIR.

[LOW] ui/inventory_overlay.gd:317 - Recipe group lookup keys are capitalize()d but stored raw
Confidence: Medium
Description: _recipe_groups[category] is populated with raw RECIPE_CATEGORIES strings, while lookups use str(recipe["category"]).capitalize(). This is identity for all current single-word categories, but a future multi-word category ("Storage Boxes") would make the lookup return null and crash on add_child.
Impact: Latent crash only when a new category is added; no current bug.
Suggested Fix: Use the raw string on both sides (or add an explicit category_id field).

[LOW] game/survival_ui.gd:87-93 - Death modal swallows all unhandled input, not just ui_cancel
Confidence: High
Description: _unhandled_input calls set_input_as_handled() for every event while the death modal is visible, blocking e.g. F2 screenshots while dead.
Impact: Trivial UX limitation during death only.
Suggested Fix: Only mark handled for events the modal actually consumes (ui_cancel/ui_confirm/navigation keys).

[LOW] world/worldgen/voxel_populator.gd:441-443 - Flat-world dry fix applies to all worldgen versions
Confidence: High
Description: _column_water_y returns -1 for flat worlds unconditionally, so existing v13-and-older flat saves regenerate their previously generated below-sea water as air (player-placed water survives as edits). This is a deliberate bug fix documented in the PR body and ROADMAP, but it silently mutates old saves' generated terrain on load.
Impact: Expected and verified by worldgen_water_verify.gd; informational only.

[LOW] player/player.gd:607-613 - Held-tool visual allocates a new BoxMesh + StandardMaterial3D on every selection change
Confidence: High
Description: _update_held_block constructs fresh resources each time a tool is selected rather than caching one tool mesh/material.
Impact: Negligible garbage; not a per-frame path.
Suggested Fix: Cache the tool mesh/material in member variables on first use.

✅ Verification Note

The engine binary is not available on this runner, so nothing was executed here. The changes are covered by the parse check plus player_survival_verify.gd, inventory_crafting_verify.gd, inventory_ui_verify.gd, item_drops_verify.gd, gameplay_integration_verify.gd, survival_progression_verify.gd, flat_survival_warning_verify.gd, and the updated worldgen_ore_verify.gd, worldgen_water_verify.gd, world_storage_verify.gd, ui_flow_verify.gd, player_target_verify.tscn, explosives_verify.tscn, fire_verify.tscn — all present in .github/workflows/tests.yml. That they were not run in this review is residual risk, not a blocker.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	8	Player owns vitals/mining, Main owns inventory/drops/stations, SurvivalUI presentation, ItemInventory/CraftingRecipes/InventoryTransfer/BlockContainers are focused models
Open/Closed	7	Data-driven recipes, tool tiers, and appended BLOCK_DEFS columns; recipe table lookup has one fragile keying (capitalize)
Liskov Substitution	7	No meaningful inheritance; test doubles subclass VoxelWorld/Player without contract breaks
Interface Segregation	7	Narrow callable contracts (can_place_check, interact_check, _accepts) and small slot/drag APIs
Dependency Inversion	6	Good injected callables, but Player/Main depend on VoxelWorld/Player concrete internals (_burning, _loaded_chunk_for, _cancel_mining)
Average	7.0

🎯 Final Assessment

Overall Confidence Score: 82%

Confidence Breakdown:

• Code Quality: 85% (clean, typed, well-commented; a few cross-boundary private accesses)
• Completeness: 88% (implements seven ROADMAP items with honest documented limitations; no mode switching as specified)
• Risk Level: 78% (large surface area touching save migration and player loop, but conservation/determinism invariants hold and coverage is strong)
• Verification: 75% (excellent verifier suite included and CI-wired, but not executed 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 WITH FIXES

One medium convention violation (cross-module private access) plus a small fire-block harvest leak should be addressed, but the survival layer is functionally sound, well-verified, and ready once those are cleaned up.

Machine Readable Verdict

{
  "reviewed_sha": "fc3d24375ef933088227e2d944e6a592244eef01",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 1,
  "overall_confidence_score": 82,
  "recommendation": "MERGE_WITH_FIXES"
}

opencode session  |  github run

[MichaelFisher1997]

Addressed the actionable review findings in 73c97e9: public fire/residency queries and mining cancellation replace cross-module private access; extinguished fire produces no collectible drop; empty-hand right-click is silent while station interaction still works. Added regressions for each.

The CI LOD failure is also fixed: v14 compact columns now sample the same ordered ore stamps at exposed top/sub cells without allocating full voxel data. The flat occlusion fixture now isolates lighting from biome tint and asserts level, dry terrain; existing shading thresholds remain unchanged. Local LOD, ore fingerprints, worldgen, survival progression, gameplay integration, fire, inventory, targeting and editor checks passed.

Optional review dispositions: current recipe categories are single-word matching keys (no current mismatch); the death modal intentionally blocks unhandled gameplay input; held-tool allocation is selection-only and left for later optimization. The all-version dry-flat fix is intentional and documented, while v13-and-older ore eligibility remains preserved. No third-party texture assets were imported.

[github-actions]

All verification complete. Here is the review.

📋 Summary

No linked issues ("Fixes #N"/"Closes #N") are referenced in the PR body.

This commit (73c97e9) is a focused follow-up to the survival-layer PR: it resolves every actionable finding from the previous review (public accessors replace cross-module private access, fire no longer drops a collectible item, empty-hand right-click is silent) and fixes the CI LOD failure by sampling the shared ordered ore stamps at compact top/sub cells in v14 LOD chunks, plus hardening the flat LOD shading fixture. Every fix ships with a new regression assertion in the headless verifiers, all of which remain CI-wired in .github/workflows/tests.yml.

📌 Review Metadata

• Reviewed Commit SHA: 73c97e9bdc78790bda0c509e81ef70121cac3f36
• Reviewed PR: Add survival modes, inventory, and crafting progression #23

🔴 Critical Issues (Must Fix - Blocks Merge)

✅ All previously reported critical issues have been resolved (none were reported).

None identified. The new compact ore stamping is deterministic (same hash_3d(seed+1009) cell enumeration and order as the full volume), stone-only replacement preserves cross-segment ordering parity, and its bounds math exactly mirrors the full path (top_y >= 1 and top_y < WORLD_HEIGHT; top_y > 1 and top_y <= WORLD_HEIGHT ⇔ top_y-1 in [1, WORLD_HEIGHT) for ints). Stage ordering also matches full detail (surface → floor patches → ores in both populate() at voxel_populator.gd:141-156 and populate_lod() at voxel_populator.gd:210-213), and canopies/scrub only raise columns above terrain afterward, so LOD/full parity holds.

⚠️ High Priority Issues (Should Fix)

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

None identified.

💡 Medium Priority Issues (Nice to Fix)

✅ [FIXED] Previous issue: cross-module private access (world._burning, world._loaded_chunk_for, player._cancel_mining)
The fix matches the suggested shape: is_full_chunk_resident_at() (world/voxel_world.gd:1219) and is_burning_at() (world/voxel_world.gd:1224) wrap the private state, _cancel_mining() was renamed to public cancel_mining() (player/player.gd:502) with all call sites updated, and a repo-wide grep confirms zero world._/player._ private references remain in game/, player/, or ui/ gameplay code. Regressions added in player_survival_verify.gd:37-40,158-168, gameplay_integration_verify.gd:118-124, and fire_verify.gd.

None identified.

ℹ️ Low Priority Suggestions (Optional)

✅ [FIXED] Previous issue: fire blocks harvestable/collectible in Survival — harvest_drop now returns {} for BLOCK_FIRE (game/item_registry.gd:102-103) and _on_mined_block skips empty drops (game/main.gd:893-895); the only gameplay consumer is guarded, and regressions added at gameplay_integration_verify.gd:77-79 and player_survival_verify.gd:174-175.

✅ [FIXED] Previous issue: empty-hand right-click shows "No AIR left" — _place_target returns early on selected_block == BLOCK_AIR (player/player.gd:545-546), correctly placed after interact_check so empty-hand station interaction still works, with a regression at player_survival_verify.gd:158-165.

ℹ️ Previously reported LOW items declined by the author with reasoned dispositions — noted for the record, no action required: recipe category capitalize() keying (ui/inventory_overlay.gd:317 — identity for all current single-word categories), death modal consuming all unhandled input (game/survival_ui.gd:90-94 — intentional), held-tool mesh/material allocated per selection change (player/player.gd:610-613 — selection-only path), and the all-version flat-world dry fix (documented, covered by worldgen_water_verify.gd).

[LOW] world/worldgen/voxel_populator.gd:211-213 - Legacy worlds with caves still show an ore pop at the LOD→full boundary
Confidence: High
Description: The compact ore stamp gates on worldgen_version >= CAVE_INDEPENDENT_ORES_VERSION, while the full-detail path stamps ores for any version when cave_density > 0 (voxel_populator.gd:150). So a legacy (v13 or older) world with caves enabled shows exposed ore in full chunks but plain stone in LOD chunks.
Impact: Visual pop when distant chunks promote to full detail on legacy worlds; pre-existing behavior (LOD previously skipped ores at every version), not a regression from this PR.
Suggested Fix: If desired, drop the version gate and keep only the non-flat check, since the stamp itself is version-independent; otherwise leave as documented LOD simplification.

✅ Verification Note

The engine binary is not available on this runner, so nothing was executed here. The fix commit is covered by the parse check plus the updated worldgen_ore_verify.gd (new _verify_compact_stamps sweeps every exposed height 1..191 × four origins including negatives × mixed grass/dirt surfaces against the frozen full-volume stage — an exhaustive parity check), worldgen_lod_verify.gd (level dry-ground precondition + tint neutralization), player_survival_verify.gd, gameplay_integration_verify.gd, and fire_verify.gd, all present in .github/workflows/tests.yml. The author reports local passes; that they were not run in this review is residual risk, not a blocker.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	8	Unchanged; compact stamping lives inside the existing ore stage rather than a parallel path
Open/Closed	8	Ore stamps extended via optional compact-array parameters without touching the full-volume contract
Liskov Substitution	7	No meaningful inheritance; verifier doubles unchanged
Interface Segregation	8	New public surface is two narrow queries (is_burning_at, is_full_chunk_resident_at) plus one verb (cancel_mining)
Dependency Inversion	7	Improved: Player/Main now depend on public VoxelWorld/Player contracts instead of private internals
Average	7.6

🎯 Final Assessment

Overall Confidence Score: 88%

Confidence Breakdown:

• Code Quality: 88% (surgical fixes matching the suggested shapes; exhaustive new parity verifier)
• Completeness: 92% (all actionable findings addressed with regressions; dispositions documented for the rest)
• Risk Level: 85% (compact ore path is deterministic and bounds-exact; small well-contained diff)
• Verification: 82% (strong verifier suite, CI-wired and locally green per the author, but not executed 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

All previously reported actionable issues are fixed with regression coverage, and the new compact ore stamping is deterministic, order-consistent, and parity-exact against the full volume.

Machine Readable Verdict

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

opencode session  |  github run