Skip to content

Add functional blocks and sleeping - #24

Merged
MichaelFisher1997 merged 4 commits into
mainfrom
functional-blocks-sleeping-spawn
Sep 18, 2026
Merged

MichaelFisher1997 merged 4 commits into
mainfrom
functional-blocks-sleeping-spawn

Conversation

@MichaelFisher1997

Copy link
Copy Markdown
Contributor

Summary

  • add byte-safe rotated stairs, slabs, paired doors, wall ladders, signs, and beds with shape-specific meshes and collision
  • add door interaction, ladder climbing, canonical inventory/drop handling, crafting recipes, and dedicated utility/functional block textures
  • add night sleeping with rain/threat/safe-spawn guards, sunrise skipping, lunar progression, and persisted respawn points
  • update the roadmap and add focused functional-block verification

Verification

  • redot --editor --headless --path . --quit
  • redot --headless --path . --script res://tools/functional_blocks_verify.gd
  • redot --headless --path . --script res://tools/inventory_crafting_verify.gd
  • redot --headless --path . --script res://tools/inventory_ui_verify.gd
  • redot --headless --path . --script res://tools/audio_verify.gd
  • redot --headless --path . --script res://tools/weather_verify.gd
  • redot --headless --path . --script res://tools/gameplay_integration_verify.gd
  • redot --headless --path . res://tools/player_target_verify.tscn
  • redot --headless --path . res://tools/explosives_verify.tscn
  • redot --headless --path . res://tools/fire_verify.tscn
  • redot --headless --path . --script res://tools/world_storage_verify.gd
  • redot --headless --path . --script res://tools/chunk_data_compression_verify.gd
  • live game startup smoke check with no runtime errors

@github-actions

Copy link
Copy Markdown

I have now examined the full diff and read the surrounding code in every touched file. No previous automated reviews exist for this PR, so there are no earlier issues to re-check or fixes to acknowledge.

📋 Summary

No linked issues ("Fixes #X"/"Closes #X") appear in the PR description. The PR implements two ROADMAP items (Functional blocks, Sleeping and spawn points) and marks them [x]: byte-safe state IDs 75–108 for stairs/slabs/doors/ladders/signs/beds with shape-specific mesh/collision via a new mesher path, canonical-id inventory/drop handling, door toggle/ladder climbing/sleeping with guards, plus generated textures, recipes, audio mappings, and a new focused verifier.

Overall this is a well-executed, convention-respecting change: the state-ID encoding is correct (door bit layout verified: facing/open/upper decode consistently), all edit paths route through _record_edit(), batch flows bump chunk edit versions, worker-thread rules are respected (_shape is built once in _init and only read afterwards), and IDs stay under 256. One real correctness bug in the sleep/respawn feature and a few minor gaps remain.

📌 Review Metadata

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

⚠️ High Priority Issues (Should Fix)

[HIGH] game/main.gd:950-955 + world/voxel_world.gd:2190-2217 - Indoor beds set the respawn point on the roof
Confidence: High
Description: _attempt_sleep() reuses VoxelWorld.find_safe_spawn(), which scans each column top-down from desired.y + 20 for the topmost opaque block with two air cells above (_find_column_spawn(), voxel_world.gd:2205-2217). In a roofed bedroom — the normal case — the ceiling is the topmost opaque block of every in-room column, has two air cells above it (above the roof), and qualifies. The returned spawn (ceiling_y + 1.5) differs from requested_spawn, so the is_equal_approx "no safe spawn" check passes and the respawn point is stored on the roof of the house. Outdoor beds work correctly (neighbor floor column wins), so the feature's persisted respawn is wrong exactly in the common indoor case. On death, Player.respawn() (player.gd:211) re-runs the same search, so it consistently puts the player on the roof/outside the walls.
Impact: Sleeping in any roofed building stores a respawn point outside the building (on the roof), potentially on a tall structure, stranding the player outside a sealed base once mobs exist, or forcing a fall on respawn.
Suggested Fix: Add a dedicated bed-level spawn search instead of reusing the surface-oriented column scanner — e.g. a find_bed_spawn(bed_position) on VoxelWorld that iterates a small ring of cells at the bed's level and accepts a cell whose block below is solid (opaque or any is_breakable solid) and whose two cells above are air, falling back to the bed-adjacent cell:

func find_bed_spawn(bed_position: Vector3i) -> Vector3i:
	var feet_y := bed_position.y + 1
	for radius in range(0, 3):
		for dx in range(-radius, radius + 1):
			for dz in range(-radius, radius + 1):
				if maxi(absi(dx), absi(dz)) != radius:
					continue
				var cell := bed_position + Vector3i(dx, 1, dz)
				if get_block_world(cell) != BlockRegistry.BLOCK_AIR \
						or get_block_world(cell + Vector3i.UP) != BlockRegistry.BLOCK_AIR:
					continue
				var below := get_block_world(cell + Vector3i.DOWN)
				if below != BlockRegistry.BLOCK_AIR and not _blocks.is_water_id(below) \
						and not _blocks.has_flag(below, BlockRegistry.FLAG_CROSS):
					return cell
	return Vector3i.ZERO  # caller refuses sleep

Then use it in _attempt_sleep() and keep the guards unchanged.

💡 Medium Priority Issues (Nice to Fix)

[MEDIUM] tools/functional_blocks_verify.gd:40-113 - World-level door/ladder/sleep lifecycle is unverified
Confidence: High
Description: The new verifier covers pure units (registry state mapping, single-block mesh emission, recipes, clock math, the threat callable), but none of the riskiest new logic runs against a live VoxelWorld: _place_door() occupancy/chunk checks, toggle_door() half-pairing and persistence via _record_edit()/_edits_by_chunk, break_block() door canonical return, ladder support rejection (voxel_world.gd:1350-1356), _destroy_burnt_block()/carve_sphere() door counterpart removal, and _attempt_sleep()'s guard ordering. The other listed verifiers (explosives/fire) predate doors and don't place them, so a regression in the door edit flows would pass the whole suite. The engine is not available on this runner, so these paths are also unverified here.
Impact: Silent regressions in door placement/toggle/burn/persist behavior would not be caught by any headless check.
Suggested Fix: Extend functional_blocks_verify.gd (or add a scene-based .tscn verifier) that instantiates a VoxelWorld the way explosives_verify.gd/fire_verify.gd do, places a door via place_block(), toggles it, asserts both voxel IDs and _edits_by_chunk contents, breaks a half, and exercises the ladder-support rejection and the sleep guard order.

ℹ️ Low Priority Suggestions (Optional)

[LOW] world/block_registry.gd:396-397 - is_inventory_block hard-codes the last state constant
Confidence: Medium
Description: block_id <= BLOCK_WOOD_BED_LAST is correct today (the verifier asserts BLOCK_DEFS.size() == BLOCK_WOOD_BED_LAST + 1), but the next block appended to BLOCK_DEFS (id 109+) will silently fail is_inventory_block(), making it unusable in inventory/hotbar/catalog via ItemRegistry.is_valid() until someone remembers this constant.
Suggested Fix: Use the table size instead of the state constant:

static func is_inventory_block(block_id: int) -> bool:
	return block_id > BLOCK_AIR and block_id < BLOCK_DEFS.size() and canonical_id(block_id) == block_id

[LOW] world/block_registry.gd:200-233 vs ui/block_icon.gd:57 - Shape blocks render as generic cubes in hotbar/inventory
Confidence: High
Description: Stairs/slabs/doors/ladders/beds get accurate 3D previews via ChunkMesher.make_block_mesh() (player.gd:656 uses it for the held block), but BlockIcon.make_icon() still draws an isometric full cube from the top/side textures, so hotbar and catalog icons don't reflect the shapes.
Suggested Fix: Route non-cross, non-cube shapes through a make_block_mesh()-based icon render in BlockIcon (or reuse the held-block mesh render at icon size).

[LOW] world/voxel_world.gd:1342-1372 - Support is placement-only; breaking the support leaves floating blocks
Confidence: High
Description: Ladders validate wall support only at placement time; breaking the supporting block leaves a climbable floating ladder. Doors/beds/signs place with no support check at all, so they can float mid-air. This matches the existing permissive behavior for torches/cross plants, so it's a parity nit rather than a regression.
Suggested Fix: If desired, add a neighbor-break hook that pops unsupported ladders (like BLOCK_FIRE decay) and a ground check in _place_door()/bed placement; otherwise document the behavior in BLOCK_DEFS comments.

[LOW] world/block_registry.gd:200-233 - New shapes do not attenuate light
Confidence: Medium
Description: None of the new blocks carry FLAG_OPAQUE, so light_attenuation() returns 0: sky/block light passes through a closed door, a stack of slabs, or a bed exactly as through air. This is a reasonable simplification (and keeps face culling simple), but sealed rooms built from these blocks stay fully sky-lit, which can look odd.
Suggested Fix: If it matters later, give full-box-ish shapes (bed base, stacked-slab usage) a small attenuation or document the decision beside _opacity in chunk_mesher.gd.

📊 SOLID Principles Score

Principle Score Notes
Single Responsibility 8 State encoding/decoding is centralized in BlockRegistry, shape emission in ChunkMesher, sleep flow in Main; _attempt_sleep mixes guard order with spawn policy slightly
Open/Closed 6 Adding a shape family touches several match sites (defs, shape_type, canonical_id, placement_variant, mesher _append_custom_block) — explicit but not extensible without editing each
Liskov Substitution 8 No inheritance introduced; the threat_check callable and place_block default args preserve existing caller contracts (old 2-arg call sites still work)
Interface Segregation 8 PlayerMain still signal-only; interact_check closure extended without new coupling; world exposes narrow helpers (is_threatened, toggle_door)
Dependency Inversion 7 threat_check: Callable is a clean inversion for the future mob system; mesher/registry remain constructor-injected and immutable for workers
Average 7.4

🎯 Final Assessment

Overall Confidence Score: 72%

Confidence Breakdown:

  • Code Quality: 85% (clean, commented, convention-following; state math verified correct)
  • Completeness: 80% (both roadmap items delivered with recipes/textures/verification; respawn placement is wrong for indoor beds)
  • Risk Level: 75% (no worker-thread/determinism/byte-safety regressions found; main risk is the respawn bug and unverified world-level door lifecycle)
  • Verification: 65% (broad verifier list in the PR, but the new world-level paths lack coverage and nothing could be re-run on this runner)

Merge Readiness:

  • All critical issues resolved (none reported)
  • SOLID average score >= 6.0
  • Overall confidence >= 60%
  • No security concerns
  • No unresolved worker-thread, determinism, or data-loss risk (the HIGH issue is gameplay-correctness, not data loss)

Verdict:

MERGE WITH FIXES

The bed-respawn roof bug should be fixed (and ideally covered by a world-level verifier) before merge; everything else is solid, thread-safe, and convention-compliant.

Machine Readable Verdict

{
  "reviewed_sha": "baf102619f07d929a2d675b3aaf83079fea5f670",
  "critical_issues": 0,
  "high_priority_issues": 1,
  "medium_priority_issues": 1,
  "overall_confidence_score": 72,
  "recommendation": "MERGE WITH FIXES"
}

New%20session%20-%202026-09-18T20%3A13%3A48.196Z
opencode session  |  github run

@github-actions

Copy link
Copy Markdown

All verification complete. The fix commit addressed storage but not the effective respawn placement. Compiling the final review.

📋 Summary

No linked issues ("Fixes #X"/"Closes #X") appear in the PR description. The PR implements two ROADMAP items (Functional blocks, Sleeping and spawn points) and marks them [x] with accurate descriptions.

The PR adds byte-safe state IDs 75–108 for stairs/slabs/doors/ladders/signs/beds with shape-specific mesh/collision, canonical-id inventory/drop handling, door toggling, ladder climbing, night sleeping with guards, generated textures, recipes, and a focused verifier. Commit 2 ("Fix indoor bed respawn placement") responds to the previous review: it adds VoxelWorld.find_bed_spawn() (a same-floor ring search), uses it in _attempt_sleep(), extends the verifier with a live-world indoor-bed fixture, and fixes the is_inventory_block hard-coded bound. The storage side of the respawn bug is genuinely fixed; however, Player.respawn() still re-runs the surface-oriented find_safe_spawn() over the stored point, so the effective respawn after death still lands on the roof for indoor beds — the user-visible half of the bug remains.

📌 Review Metadata

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

⚠️ High Priority Issues (Should Fix)

[HIGH] player/player.gd:211 + world/voxel_world.gd:2190-2202 - Respawn still re-runs the surface scanner, landing indoor-bed respawns on the roof
Confidence: High
Description:[FIXED] Previous issue (partial): the stored/persisted respawn point is now correct — _attempt_sleep() (game/main.gd:952-956) stores the bed-level point from the new find_bed_spawn() (world/voxel_world.gd:2208-2224), whose same-floor ring search I verified cell-by-cell (ring 0 is the occupied bed cell, correctly skipped; feet+head air, solid non-cross ground required).

However, Player.respawn() still overwrites the corrected value: line 211 runs global_position = world.find_safe_spawn(spawn_position), and find_safe_spawn scans each column from desired.y + 20 downward for the topmost opaque block with two air cells above (_find_column_spawn, voxel_world.gd:2227-2239). For a bed-level stored spawn under any roof, ring 0 (the bed-adjacent column) resolves to the roof slab, so the player is physically placed on the roof at the moment of death — exactly the behavior the fix commit's message claims to fix. This line is pre-existing on main (harmless when spawns were only outdoor world spawns), but this PR introduces indoor spawn_position values, making it wrong for the first time. The persisted metadata is now bed-level while the actual respawn discards it.
Impact: Dying after sleeping in any roofed building still respawns the player on the roof (or, for underground bedrooms, on the surface above the base) — the previous review's user-visible bug persists despite the corrected stored point.
Suggested Fix: Trust the validated bed spawn and only fall back to the surface scan when its cells are no longer safe. Add a small predicate mirroring find_bed_spawn's cell check:

func is_standable_spawn(world_position: Vector3) -> bool:
	var cell := Vector3i(floori(world_position.x), floori(world_position.y), floori(world_position.z))
	if get_block_world(cell) != BlockRegistry.BLOCK_AIR \
			or get_block_world(cell + Vector3i.UP) != BlockRegistry.BLOCK_AIR:
		return false
	var below := get_block_world(cell + Vector3i.DOWN)
	return below != BlockRegistry.BLOCK_AIR and not _blocks.is_water_id(below) \
			and not _blocks.has_flag(below, BlockRegistry.FLAG_CROSS)

Then in respawn():

world.setup_player(self, true)
if not world.is_standable_spawn(spawn_position):
	global_position = world.find_safe_spawn(spawn_position)
world.setup_player(self, true)

The indoor-bed verifier fixture (tools/functional_blocks_verify.gd:116-136) can additionally assert is_standable_spawn on the stored point, pinning the effective respawn rather than only the search.

💡 Medium Priority Issues (Nice to Fix)

[MEDIUM] tools/functional_blocks_verify.gd:116-141 - World-level door/ladder lifecycle still unverified (bed-spawn search now covered)
Confidence: High
Description:[FIXED] Previous issue (partial): the new _verify_indoor_bed_spawn() builds a live VoxelWorld fixture (chunk-in-dict, matching index math, floor/bed/roof) and covers find_bed_spawn() — the fixture is sound (_loaded_chunk_for only checks chunk presence and y bounds, _restore_chunk_data early-returns on non-empty data).

Still uncovered: the riskiest new world-mutating paths — _place_door() occupancy/chunk checks (voxel_world.gd:1375-1388), toggle_door() half-pairing and persistence through _record_edit()/_edits_by_chunk (1420+), break_block() door canonical return and counterpart removal (1330-1334), ladder support rejection (1350-1356), _destroy_burnt_block()/carve_sphere() door handling, and _attempt_sleep() guard ordering. The explosives/fire verifiers predate doors, so a regression in these flows passes the whole suite.
Impact: Silent regressions in door placement/toggle/persist behavior would not be caught by any headless check.
Suggested Fix: Extend _verify_indoor_bed_spawn-style fixtures to doors: place via place_block(), assert both half IDs, toggle and assert open state plus _edits_by_chunk contents, break one half and assert both clear, and exercise the ladder-support rejection.

ℹ️ Low Priority Suggestions (Optional)

[FIXED] Previous issue: is_inventory_block hard-coded BLOCK_WOOD_BED_LAST — now block_id < BLOCK_DEFS.size() (world/block_registry.gd:397), so future appended blocks stay usable automatically.

[LOW] ui/block_icon.gd - Shape blocks still render as generic cubes in hotbar/inventory
Confidence: High
Description: Unchanged from previous review: BlockIcon.make_icon() still draws isometric full cubes while ChunkMesher.make_block_mesh() produces accurate shape previews for the held block. ui/block_icon.gd is untouched by this PR.
Suggested Fix: Route shape blocks through a make_block_mesh()-based icon render in BlockIcon.

[LOW] world/voxel_world.gd:1350-1356 - Support remains placement-only; breaking the support leaves floating blocks
Confidence: High
Description: Unchanged from previous review: ladders validate wall support only at placement; doors/beds/signs place with no support check (_place_door() has no ground check). Parity with torches/plants, so a nit.
Suggested Fix: Add a neighbor-break hook that pops unsupported ladders (like BLOCK_FIRE decay) or document the permissive behavior.

[LOW] world/block_registry.gd:200-233 - New shapes still do not attenuate light
Confidence: Medium
Description: Unchanged from previous review: rows 75–108 carry only FLAG_FLAMMABLE (no FLAG_OPAQUE), so light_attenuation() is 0 and sealed rooms of these blocks stay sky-lit.
Suggested Fix: If desired later, give full-box shapes small attenuation or document the decision.

📊 SOLID Principles Score

Principle Score Notes
Single Responsibility 8 The fix improves on the previous state: spawn policy now lives in a dedicated find_bed_spawn instead of reusing the surface scanner; respawn() fallback policy remains in pre-existing Player code
Open/Closed 6 Adding a shape family still touches several match sites (defs, shape_type, canonical_id, placement_variant, mesher)
Liskov Substitution 8 place_block default args preserve old call sites; find_bed_spawn's Dictionary return is a new narrow contract
Interface Segregation 8 PlayerMain still signal-only; world exposes narrow helpers (find_bed_spawn, toggle_door, is_threatened)
Dependency Inversion 7 threat_check: Callable remains a clean inversion; mesher/registry stay immutable for workers
Average 7.4

🎯 Final Assessment

Overall Confidence Score: 75%

Confidence Breakdown:

  • Code Quality: 85% (fix is clean, cell predicate correct, verifier fixture math verified against _data_index and chunk lookup paths)
  • Completeness: 82% (stored respawn and its verification fixed; effective respawn placement still wrong for indoor beds)
  • Risk Level: 78% (no worker-thread, determinism, byte-safety, or data-loss risk found; remaining bug is gameplay-correctness)
  • Verification: 68% (new indoor-bed verifier added but cannot be run on this runner; door lifecycle still uncovered — functional_blocks_verify.gd should be run before merge)

Merge Readiness:

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

Verdict:

MERGE WITH FIXES

The fix commit corrects the stored respawn point and adds real coverage for the bed search, but Player.respawn()'s re-scan still discards it and places indoor sleepers on the roof — gate the surface scan behind a cell-validation fallback and this is ready.

Machine Readable Verdict

{
  "reviewed_sha": "533122dc4ea78df1f04ed2bc9dccb2e6643ee862",
  "critical_issues": 0,
  "high_priority_issues": 1,
  "medium_priority_issues": 1,
  "overall_confidence_score": 75,
  "recommendation": "MERGE WITH FIXES"
}

New%20session%20-%202026-09-18T20%3A25%3A46.209Z
opencode session  |  github run

@github-actions

Copy link
Copy Markdown

All checks complete. Commit 3 fixes the HIGH exactly as suggested, with a shared predicate and a verifier assertion; the MEDIUM coverage gap and three LOWs remain. Compiling the final review.

📋 Summary

No linked issues ("Fixes #X"/"Closes #X") appear in the PR description. The PR implements two ROADMAP items (Functional blocks, Sleeping and spawn points) and marks them [x]: byte-safe state IDs 75–108 for stairs/slabs/doors/ladders/signs/beds with shape-specific mesh/collision, canonical-id inventory/drop handling, door toggling, ladder climbing, night sleeping with guards, generated textures, recipes, and a focused verifier.

Commit 3 ("Preserve validated bed respawns") closes out the previous HIGH issue: Player.respawn() now keeps the validated bed spawn and only falls back to the surface scanner when its cells are no longer standable, find_bed_spawn() and the new is_standable_spawn() share one _is_standable_cell() predicate, and the verifier pins the stored point as directly usable. The remaining gap is verifier coverage for the world-level door/ladder lifecycle, plus the previously noted polish nits.

📌 Review Metadata

🔴 Critical Issues (Must Fix - Blocks Merge)

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

⚠️ High Priority Issues (Should Fix)

[FIXED] Previous issue: respawn re-ran the surface scanner and landed indoor-bed respawns on the roof. Verified across the full chain:

  • player/player.gd:208-213respawn() now runs world.setup_player(self, true), then only repositions via find_safe_spawn() when world.is_standable_spawn(spawn_position) is false, then re-runs setup_player — exactly the gating the previous review proposed.
  • world/voxel_world.gd:2224-2237is_standable_spawn() (with an is_finite guard) delegates to _is_standable_cell(), which is now the single source of truth also used by find_bed_spawn() (line 2216), so the stored point and the respawn check cannot drift apart.
  • Encoding is self-consistent: find_bed_spawn returns feet_cell + (0.5, 0.5, 0.5) and find_safe_spawn returns ground_y + 1.5; floori() recovers the correct feet cell for both, so is_standable_spawn validates either encoding.
  • Unloaded/LOD chunks make get_block_world() return BLOCK_AIR (voxel_world.gd:1236-1241), so the check degrades to the old surface-scan fallback rather than crashing or trusting stale data.
  • game/main.gd:950-957 stores the bed-level point into player.spawn_position and _flush_world_save() persists it immediately; player.gd:125/163 round-trips it through save metadata.
  • tools/functional_blocks_verify.gd:137 now asserts world.is_standable_spawn(position) on the stored indoor-bed respawn — pinning the exact call respawn() makes, not just the search.

No new high-priority issues found in the new commit.

💡 Medium Priority Issues (Nice to Fix)

[MEDIUM] tools/functional_blocks_verify.gd:116-141 - World-level door/ladder lifecycle still unverified
Confidence: High
Description: Unchanged by commit 3 (it added only the one-line is_standable_spawn assertion). The verifier covers pure units (state encoding, single-block mesh emission, recipes, clock math, threat hook) and now the bed-spawn search against a live VoxelWorld fixture, but the riskiest world-mutating paths have no live-world coverage: _place_door() occupancy/chunk checks (voxel_world.gd:1375-1388), toggle_door() half-pairing and persistence through _record_edit()/_edits_by_chunk (1420-1433), break_block() door canonical return and counterpart removal (1330-1334), ladder support rejection (1350-1356), and burn/explosion paths that must remove door halves. I grepped explosives_verify.gd and fire_verify.gd — neither places doors, so a regression in these flows passes the entire suite.
Impact: Silent regressions in door placement/toggle/persist behavior would not be caught by any headless check.
Suggested Fix: Extend the _verify_indoor_bed_spawn() fixture pattern to doors: seed a loaded chunk, place a door via place_block(), assert both half IDs and _edits_by_chunk entries, toggle and assert the open state on both halves, break one half and assert both clear plus the canonical BLOCK_WOOD_DOOR return, and exercise the ladder support rejection (normal with missing/soft support must return false).

ℹ️ Low Priority Suggestions (Optional)

[FIXED] Previous issue: is_inventory_block hard-coded BLOCK_WOOD_BED_LAST — remains block_id < BLOCK_DEFS.size() in the current code, so future appended blocks stay usable automatically.

[LOW] ui/block_icon.gd:16 - Shape blocks still render as generic cubes in hotbar/inventory
Confidence: High
Description: Unchanged from previous reviews: BlockIcon.make_icon() still draws isometric full cubes from the top/side textures while ChunkMesher.make_block_mesh() (used for the held block, player.gd:657) produces accurate shape previews. The file is untouched by this PR.
Suggested Fix: Route shape blocks through a make_block_mesh()-based icon render in BlockIcon.

[LOW] world/voxel_world.gd:1350-1356, 1375-1388 - Support remains placement-only; breaking the support leaves floating blocks
Confidence: High
Description: Unchanged: ladders validate wall support only at placement; doors/beds/signs place with no ground check. Parity with torches/plants, so a nit rather than a regression.
Suggested Fix: Add a neighbor-break hook that pops unsupported ladders (like BLOCK_FIRE decay) or document the permissive behavior.

[LOW] world/block_registry.gd:200-233 - New shapes still do not attenuate light
Confidence: Medium
Description: Unchanged: rows 75–108 carry only FLAG_FLAMMABLE (no FLAG_OPAQUE), so light_attenuation() is 0 and sealed rooms built from these blocks stay sky-lit.
Suggested Fix: If desired later, give full-box shapes small attenuation or document the decision beside _opacity in chunk_mesher.gd.

📊 SOLID Principles Score

Principle Score Notes
Single Responsibility 9 Improved: _is_standable_cell() is now the one shared predicate for bed search, standability, and (via composition) the respawn fallback policy
Open/Closed 6 Adding a shape family still touches several match sites (defs, shape_type, canonical_id, placement_variant, mesher)
Liskov Substitution 8 place_block default args preserve old call sites; find_bed_spawn's Dictionary return remains a narrow new contract
Interface Segregation 8 PlayerMain still signal-only; world exposes narrow helpers (is_standable_spawn, find_bed_spawn, toggle_door, is_threatened)
Dependency Inversion 7 threat_check: Callable remains a clean inversion; mesher/registry stay immutable for workers
Average 7.6

🎯 Final Assessment

Overall Confidence Score: 80%

Confidence Breakdown:

  • Code Quality: 87% (fix is minimal, correctly gated, and single-sources the cell predicate; no dead code or contract drift)
  • Completeness: 88% (both roadmap items delivered; both halves of the respawn bug — stored and effective — are now fixed and pinned by the verifier)
  • Risk Level: 82% (no worker-thread, determinism, byte-safety, or data-loss risk found; unload/LOD fallback path verified safe)
  • Verification: 68% (verifier extended, but nothing could be re-run on this runner — functional_blocks_verify.gd should be run before merge; door lifecycle still uncovered)

Merge Readiness:

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

Verdict:

MERGE WITH FIXES

The respawn bug is fully resolved and verified by inspection; extend the verifier to the world-level door/ladder lifecycle (or accept that gap consciously) and this is ready.

Machine Readable Verdict

{
  "reviewed_sha": "a269bcfa81f14c427faf3913cdb98772f558363d",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 1,
  "overall_confidence_score": 80,
  "recommendation": "MERGE WITH FIXES"
}

New%20session%20-%202026-09-18T20%3A32%3A40.290Z
opencode session  |  github run

@github-actions

Copy link
Copy Markdown

All verification complete. Commit 4 addresses the remaining MEDIUM exactly as previously suggested, and I verified every new assertion against the real voxel_world.gd code paths. Compiling the final review.

📋 Summary

No linked issues ("Fixes #X"/"Closes #X") appear in the PR description. The PR implements two ROADMAP items (Functional blocks, Sleeping and spawn points) and marks them [x] with accurate descriptions.

The PR adds byte-safe state IDs 75–108 for stairs/slabs/doors/ladders/signs/beds with shape-specific meshes and collision, canonical-id inventory/drop handling, door toggling, ladder climbing, night sleeping with guards, generated textures, recipes, and a focused verifier. Commit 4 ("Cover functional block lifecycles") closes the last open issue: it refactors the verifier's world fixture into a shared _world_fixture() and adds _verify_world_block_lifecycle(), which drives a live VoxelWorld through door place/toggle/break with _edits_by_chunk persistence assertions, blocked-upper-cell rejection, ladder support rejection/acceptance, carve_sphere and _destroy_burnt_block door-half removal. I traced each new assertion against the implementation (place_block/_place_door voxel_world.gd:1342-1388, toggle_door 1420-1433, break_block 1330-1334, carve_sphere 1472-1543, _destroy_burnt_block 1823-1851) and the door bit layout in block_registry.gd:402-451; all expectations match, and every exercised path is headless-safe (queue-only rebuilds via _queue_rebuild, _edit_store == null guards, _restore_chunk_data early-returns on the fixture's non-empty data). The three previously noted LOW polish nits remain and are optional.

📌 Review Metadata

🔴 Critical Issues (Must Fix - Blocks Merge)

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

⚠️ High Priority Issues (Should Fix)

[FIXED] Previous issue: indoor-bed respawns landing on the roof (stored and effective). Re-verified intact at the current head: player/player.gd:204-213 keeps the validated bed spawn and only falls back to find_safe_spawn() when is_standable_spawn() fails; world/voxel_world.gd:2208-2237 shares the single _is_standable_cell() predicate between find_bed_spawn() and is_standable_spawn(); game/main.gd:950-957 stores the bed-level point and flushes the save; the verifier pins the stored point as directly usable (tools/functional_blocks_verify.gd:136).

No new high-priority issues found in commit 4.

💡 Medium Priority Issues (Nice to Fix)

[FIXED] Previous issue: world-level door/ladder lifecycle unverified. Commit 4's _verify_world_block_lifecycle() (tools/functional_blocks_verify.gd:139-202) covers every path the previous review listed: _place_door() occupancy rejection (blocked upper cell, line 176-179) and oriented halves (145-154), _edits_by_chunk persistence for place/toggle/break (155-157, 163-165, 171-174), toggle_door() from the upper half opening both halves (158-162), break_block() canonical BLOCK_WOOD_DOOR return with both halves cleared (166-170), ladder support rejection then acceptance (180-185), carve_sphere() removing both door halves (187-193), and _destroy_burnt_block() removing the whole door while marking its chunk changed (194-202). I verified the fixture is sound (coordinates don't collide across sub-fixtures; the radius-1 blast at (10,5,10) cannot reach the ladder support at (7,5,8)) and that all assertions match the implementation. No new medium-priority issues found.

ℹ️ Low Priority Suggestions (Optional)

[FIXED] Previous issue: is_inventory_block hard-coded BLOCK_WOOD_BED_LAST — remains block_id < BLOCK_DEFS.size() (world/block_registry.gd:397), so future appended blocks stay usable automatically.

[LOW] ui/block_icon.gd:16-28 - Shape blocks still render as generic cubes in hotbar/inventory
Confidence: High
Description: Unchanged from previous reviews: BlockIcon.make_icon() still branches only cross-vs-cube, so stairs/slabs/doors/ladders/beds draw as isometric full cubes while ChunkMesher.make_block_mesh() produces accurate shape previews for the held block. The file is untouched by this PR.
Suggested Fix: Route shape blocks through a make_block_mesh()-based icon render in BlockIcon.

[LOW] world/voxel_world.gd:1350-1356 - Support remains placement-only; breaking the support leaves floating blocks
Confidence: High
Description: Unchanged: ladders validate wall support only at placement; doors/beds/signs place with no ground check. Parity with torches/plants, so a nit rather than a regression.
Suggested Fix: Add a neighbor-break hook that pops unsupported ladders (like BLOCK_FIRE decay) or document the permissive behavior.

[LOW] world/block_registry.gd:200-233 - New shapes still do not attenuate light
Confidence: Medium
Description: Unchanged: rows 75–108 carry only FLAG_FLAMMABLE (no FLAG_OPAQUE), so light_attenuation() is 0 and sealed rooms built from these blocks stay sky-lit.
Suggested Fix: If desired later, give full-box shapes small attenuation or document the decision beside _opacity in chunk_mesher.gd.

📊 SOLID Principles Score

Principle Score Notes
Single Responsibility 9 _is_standable_cell() remains the single shared spawn predicate; _world_fixture() deduplicates verifier setup instead of duplicating fixture wiring
Open/Closed 6 Adding a shape family still touches several match sites (defs, shape_type, canonical_id, placement_variant, mesher)
Liskov Substitution 8 place_block default args preserve old call sites; find_bed_spawn's Dictionary return remains a narrow new contract
Interface Segregation 8 PlayerMain still signal-only; world exposes narrow helpers (is_standable_spawn, find_bed_spawn, toggle_door, is_threatened)
Dependency Inversion 7 threat_check: Callable remains a clean inversion; mesher/registry stay immutable for workers
Average 7.6

🎯 Final Assessment

Overall Confidence Score: 85%

Confidence Breakdown:

  • Code Quality: 88% (commit 4 is test-only, assertion-rich, and matches the implementation exactly; production code unchanged since the last reviewed fix)
  • Completeness: 92% (both roadmap items delivered; both halves of the respawn bug fixed and pinned; the lifecycle coverage gap is closed)
  • Risk Level: 84% (no worker-thread, determinism, byte-safety, or data-loss risk; new verifier paths are all headless-safe by construction)
  • Verification: 78% (the verifier is now comprehensive across registry, meshes, recipes, sleep, bed spawn, and world lifecycle, but nothing could be executed on this runner — run redot --headless --path . --script res://tools/functional_blocks_verify.gd and the parse check before merge as routine confirmation)

Merge Readiness:

  • All critical issues resolved (none reported)
  • 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 blocking and should-fix issues are resolved and verified by inspection; the only remaining items are optional polish nits, and the sole residual risk is that the (now comprehensive) verifier has not been executed on this runner.

Machine Readable Verdict

{
  "reviewed_sha": "dfc367108f4070fa8e9bfce84d0e8c7280ba7b99",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 0,
  "overall_confidence_score": 85,
  "recommendation": "MERGE"
}

New%20session%20-%202026-09-18T20%3A43%3A05.022Z
opencode session  |  github run

@MichaelFisher1997
MichaelFisher1997 merged commit 1af34a2 into main Sep 18, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant