Good place

assets/sprites/aux_terrain_tileset.bmp.import

@@ -1,40 +0,0 @@
-[remap]
-
-importer="texture"
-type="CompressedTexture2D"
-uid="uid://dbcuqclg3xhrk"
-path="res://.godot/imported/aux_terrain_tileset.bmp-cfff3a0c5b37bcf01b239cc3e0e49ae5.ctex"
-metadata={
-"vram_texture": false
-}
-
-[deps]
-
-source_file="res://assets/sprites/aux_terrain_tileset.bmp"
-dest_files=["res://.godot/imported/aux_terrain_tileset.bmp-cfff3a0c5b37bcf01b239cc3e0e49ae5.ctex"]
-
-[params]
-
-compress/mode=0
-compress/high_quality=false
-compress/lossy_quality=0.7
-compress/uastc_level=0
-compress/rdo_quality_loss=0.0
-compress/hdr_compression=1
-compress/normal_map=0
-compress/channel_pack=0
-mipmaps/generate=false
-mipmaps/limit=-1
-roughness/mode=0
-roughness/src_normal=""
-process/channel_remap/red=0
-process/channel_remap/green=1
-process/channel_remap/blue=2
-process/channel_remap/alpha=3
-process/fix_alpha_border=true
-process/premult_alpha=false
-process/normal_map_invert_y=false
-process/hdr_as_srgb=false
-process/hdr_clamp_exposure=false
-process/size_limit=0
-detect_3d/compress_to=1

docs/review/2026-04-07-architecture-review.md

@@ -33,7 +33,9 @@ This pass re-checks the 15 issues from the prior review and surfaces new finding
 
 ## High Priority
 
-### A. `apply_proposal` does not validate unit references
+### A. `apply_proposal` does not validate unit references — **Resolved 2026-04-07**
+
+`apply_proposal` now early-returns if either unit reference is invalid, and re-checks both before the counterattack branch.
 
 **File:** `nodes/combat_system.gd:129-148`
 
@@ -109,7 +111,9 @@ Both systems independently iterate `get_nodes_in_group("units")`, hook `node_add
 
 ---
 
-### H. Two combat entry points; `process_combat` is dead
+### H. Two combat entry points; `process_combat` is dead — **Resolved 2026-04-07**
+
+`process_combat()` deleted from `combat_system.gd`. Only `create_proposal` → `apply_proposal` remains.
 
 **File:** `nodes/combat_system.gd:114-127`
 
@@ -129,7 +133,9 @@ Both systems independently iterate `get_nodes_in_group("units")`, hook `node_add
 
 ---
 
-### J. Sprite spam in `FogRenderer` and `WallRenderer`
+### J. Sprite spam in `FogRenderer` and `WallRenderer` — **Resolved 2026-04-07**
+
+Both renderers now override `_draw()` and use `draw_texture_rect_region()` directly. Zero child sprites; layout changes call `queue_redraw()` instead of thrashing the scene tree. Findings **O** (redundant `match` arms) and **P** (undocumented `FOG_RECT`) were folded into the same change.
 
 **Files:** `nodes/fog_renderer.gd:13-43`, `nodes/wall_renderer.gd:44-176`
 
@@ -149,7 +155,9 @@ Both renderers create one `Sprite2D` per fog tile / per wall half-segment / per
 
 ---
 
-### L. `target_tile` highlight updates while combat UI is open
+### L. `target_tile` highlight updates while combat UI is open — **Not a bug (2026-04-07)**
+
+On re-reading `player_controller.gd:44-46`, `_process` already early-returns when `input_disabled` is true. Original finding was based on a misread; no code change needed.
 
 **File:** `nodes/player_controller.gd:44-52`
 
@@ -177,7 +185,9 @@ The function is called but no-ops. The TODO is fine for now, but the matching at
 
 ---
 
-### O. `WallRenderer._draw_tile_walls` match arms are identical
+### O. `WallRenderer._draw_tile_walls` match arms are identical — **Resolved 2026-04-07**
+
+Collapsed to a single `for edge in edges: _build_edge_segments(...)` loop as part of finding J's refactor.
 
 **File:** `nodes/wall_renderer.gd:103-112`
 
@@ -217,7 +227,9 @@ Rename to `display_name` or `unit_name` to disambiguate from `Node.name` / `Obje
 
 ---
 
-### S. `CombatMap.is_wall` and `is_tile_valid` have legacy fallbacks
+### S. `CombatMap.is_wall` and `is_tile_valid` have legacy fallbacks — **Resolved 2026-04-07**
+
+Fallback branches removed from `is_tile_passable` and `is_tile_valid`; both now `assert(map_layout != null)`. The now-dead `is_wall()` helper was deleted.
 
 **File:** `nodes/combat_map.gd:97-108`
 
@@ -245,22 +257,22 @@ Findings 2 and 9 from the prior review are the largest gameplay gap. Until that
 
 | # | Finding | Priority | Effort |
 |---|---------|----------|--------|
-| A | `apply_proposal` no `is_instance_valid` | High | Small |
+| A | `apply_proposal` no `is_instance_valid` | ~~High~~ | **Done** |
 | B | No allegiance check | High | Small |
 | C | `queue_free` dangling refs | High | Small |
 | D | `atk_range` not enforced pre-request | High | Small |
 | E | Greedy axis movement, no pathfinding | Medium | Medium |
 | F | Duplicated unit-tracking | Medium | Medium |
 | G | `remove_unit` leaks signal connections | Medium | Small |
-| H | Dead `process_combat` | Medium | Trivial |
+| H | Dead `process_combat` | ~~Medium~~ | **Done** |
 | I | O(n) unit lookup | Medium | Small |
-| J | Sprite-per-tile renderers | Medium | Medium |
+| J | Sprite-per-tile renderers | ~~Medium~~ | **Done** |
 | K | `MapLayout.initialize` not guarded | Medium | Small |
-| L | Highlight updates while modal open | Medium | Trivial |
+| L | Highlight updates while modal open | ~~Medium~~ | **Not a bug** |
 | M | Inline scripts in `unit.tscn` | Medium | Small |
 | N | `_draw_outer_corners` stub | Low | Medium |
-| O | Redundant `match` in wall renderer | Low | Trivial |
+| O | Redundant `match` in wall renderer | ~~Low~~ | **Done** |
 | P | Magic `FOG_RECT` constant | Low | Trivial |
 | Q | Jolt 3D in 2D project | Low | Trivial |
 | R | `UnitInfo.name` shadows | Low | Trivial |
-| S | Dead fallback paths in `CombatMap` | Low | Trivial |
+| S | Dead fallback paths in `CombatMap` | ~~Low~~ | **Done** |

nodes/combat_map.gd

@@ -74,9 +74,6 @@ func _apply_deploy(unit: Unit, coords: Vector2i) -> void:
 	add_child(unit)
 
 
-func is_wall(coords: Vector2i) -> bool:
-	return tile_map.get_cell_atlas_coords(coords) == tile_set.wall_tile_coords
-	
 func remove_unit(unit: Unit) -> void:
 	if unit.get_parent() == self:
 		remove_child(unit)
@@ -95,17 +92,13 @@ func apply_layout(layout: MapLayout) -> void:
 
 
 func is_tile_passable(from: Vector2i, to: Vector2i) -> bool:
-	if map_layout:
-		return map_layout.is_passable(from, to)
-	# Fallback: no room system, use legacy wall check
-	return not is_wall(to)
+	assert(map_layout != null, "CombatMap.is_tile_passable called before map_layout was set")
+	return map_layout.is_passable(from, to)
 
 
 func is_tile_valid(coords: Vector2i) -> bool:
-	if map_layout:
-		return map_layout.is_tile_valid(coords)
-	# Fallback: no room system, any non-wall tile is valid
-	return not is_wall(coords)
+	assert(map_layout != null, "CombatMap.is_tile_valid called before map_layout was set")
+	return map_layout.is_tile_valid(coords)
 
 
 func draw_room_walls() -> void:

nodes/combat_system.gd

@@ -111,27 +111,15 @@ func select_ai_tactic(unit: Unit, opponent: Unit, available_tactics: Array[Comba
 	return best_tactic
 
 
-func process_combat(attacker: Unit, defender: Unit, distance: int) -> void:
-	if not attacker.is_alive() or not defender.is_alive():
-		return
-	var proposal := create_proposal(attacker, defender, distance)
-	var atk_name := attacker.current_info.name
-	var def_name := defender.current_info.name
-	print("=== Combat: %s vs %s ===" % [atk_name, def_name])
-	print("  %s — HP:%d ATK:%d DEF:%d HIT:%d" % [atk_name, proposal.attacker.hp, proposal.attacker.atk, proposal.attacker.def, proposal.attacker.hit])
-	print("  %s — HP:%d ATK:%d DEF:%d HIT:%d" % [def_name, proposal.defender.hp, proposal.defender.atk, proposal.defender.def, proposal.defender.hit])
-	apply_proposal(proposal)
-	var atk_hp := attacker.current_stats.current_hp if is_instance_valid(attacker) else 0
-	var def_hp := defender.current_stats.current_hp if is_instance_valid(defender) else 0
-	print("  Result: %s HP=%d, %s HP=%d" % [atk_name, atk_hp, def_name, def_hp])
-
-
 func apply_proposal(proposal: CombatProposal) -> void:
 	var atk_stats := proposal.attacker
 	var def_stats := proposal.defender
 	var atk_unit := atk_stats.unit
 	var def_unit := def_stats.unit
 
+	if not is_instance_valid(atk_unit) or not is_instance_valid(def_unit):
+		return
+
 	# Attacker strikes (if their tactic deals damage)
 	if atk_stats.selected_tactic and atk_stats.selected_tactic.deals_damage():
 		var atk_roll := randi_range(1, 100)
@@ -140,7 +128,9 @@ func apply_proposal(proposal: CombatProposal) -> void:
 			def_unit.take_damage(damage)
 
 	# Counterattack if defender survives and their tactic deals damage
-	if def_unit.is_alive() and def_stats.selected_tactic and def_stats.selected_tactic.deals_damage():
+	if is_instance_valid(def_unit) and def_unit.is_alive() \
+			and is_instance_valid(atk_unit) \
+			and def_stats.selected_tactic and def_stats.selected_tactic.deals_damage():
 		var def_roll := randi_range(1, 100)
 		if def_roll <= def_stats.hit:
 			var damage := maxi(def_stats.atk - atk_stats.def, 0)

nodes/fog_renderer.gd

@@ -5,39 +5,35 @@ extends Node2D
 ## that is not part of any room. Future: drive visibility from map state.
 
 const TILE_SIZE := 100.0
+## Fog tile region in aux_terrain.BMP
 const FOG_RECT := Rect2(53, 53, 100, 100)
 
 @export var atlas_texture: Texture2D
 
+var _fog_tiles: PackedVector2Array = []
+
 
 func draw_fog_for_layout(map_layout: MapLayout) -> void:
-	_clear()
+	_fog_tiles.clear()
 	if not map_layout or not atlas_texture:
+		queue_redraw()
 		return
 	for y in map_layout.size.y:
 		for x in map_layout.size.x:
 			var tile := Vector2i(x, y)
 			if map_layout.is_tile_valid(tile):
 				continue
-			_create_fog_sprite(Vector2(tile) * TILE_SIZE)
+			_fog_tiles.append(Vector2(tile) * TILE_SIZE)
+	queue_redraw()
 
 
-func _clear() -> void:
-	for child in get_children():
-		child.queue_free()
-
-
-func _create_fog_sprite(pos: Vector2) -> void:
-	var atlas := AtlasTexture.new()
-	atlas.atlas = atlas_texture
-	atlas.region = FOG_RECT
-
-	var sprite := Sprite2D.new()
-	sprite.texture = atlas
-	sprite.centered = false
-	sprite.position = pos
-	sprite.scale = Vector2(
-		TILE_SIZE / FOG_RECT.size.x,
-		TILE_SIZE / FOG_RECT.size.y,
-	)
-	add_child(sprite)
+func _draw() -> void:
+	if not atlas_texture:
+		return
+	var dest_size := Vector2(TILE_SIZE, TILE_SIZE)
+	for pos in _fog_tiles:
+		draw_texture_rect_region(
+			atlas_texture,
+			Rect2(pos, dest_size),
+			FOG_RECT,
+		)

nodes/wall_renderer.gd

@@ -40,21 +40,28 @@ const HALF_EDGE := TILE_SIZE / 2.0
 
 @export var atlas_texture: Texture2D
 
+# Each entry is [dest_rect: Rect2, source_rect: Rect2]
+var _segments: Array = []
+
 
 func draw_walls_for_layout(map_layout: MapLayout) -> void:
-	_clear_walls()
+	_segments.clear()
 	if not map_layout or not atlas_texture:
+		queue_redraw()
 		return
 
 	var tile_edges := _collect_tile_edges(map_layout)
 	for tile in tile_edges:
 		var edges: Array = tile_edges[tile]
-		_draw_tile_walls(tile, edges)
+		_build_tile_walls(tile, edges)
+	queue_redraw()
 
 
-func _clear_walls() -> void:
-	for child in get_children():
-		child.queue_free()
+func _draw() -> void:
+	if not atlas_texture:
+		return
+	for seg in _segments:
+		draw_texture_rect_region(atlas_texture, seg[0], seg[1])
 
 
 func _collect_tile_edges(map_layout: MapLayout) -> Dictionary:
@@ -97,28 +104,20 @@ func _direction_to_edge(dir: Vector2i) -> StringName:
 	return &""
 
 
-func _draw_tile_walls(tile: Vector2i, edges: Array) -> void:
+func _build_tile_walls(tile: Vector2i, edges: Array) -> void:
 	var tile_origin := Vector2(tile) * TILE_SIZE
 
 	for edge in edges:
-		match edge:
-			&"left":
-				_draw_edge_segments(tile_origin, edge)
-			&"right":
-				_draw_edge_segments(tile_origin, edge)
-			&"top":
-				_draw_edge_segments(tile_origin, edge)
-			&"bottom":
-				_draw_edge_segments(tile_origin, edge)
+		_build_edge_segments(tile_origin, edge)
 
 	# TODO: Outer corner segments
-	_draw_outer_corners(tile, tile_origin, edges)
+	_build_outer_corners(tile, tile_origin, edges)
 
-	# TODO: Inner corner segments (for non-rectangular room support)
-	_draw_inner_corners(tile, tile_origin, edges)
+	# Inner corner segments (for non-rectangular room support)
+	_build_inner_corners(tile, tile_origin, edges)
 
 
-func _draw_edge_segments(tile_origin: Vector2, edge: StringName) -> void:
+func _build_edge_segments(tile_origin: Vector2, edge: StringName) -> void:
 	var seg_a_rect: Rect2
 	var seg_b_rect: Rect2
 	var seg_a_offset: Vector2
@@ -156,34 +155,22 @@ func _draw_edge_segments(tile_origin: Vector2, edge: StringName) -> void:
 			seg_a_offset = Vector2(0, TILE_SIZE - WALL_THICKNESS)
 			seg_b_offset = Vector2(HALF_EDGE, TILE_SIZE - WALL_THICKNESS)
 
-	_create_segment_sprite(tile_origin + seg_a_offset, seg_a_size, seg_a_rect)
-	_create_segment_sprite(tile_origin + seg_b_offset, seg_b_size, seg_b_rect)
+	_queue_segment(tile_origin + seg_a_offset, seg_a_size, seg_a_rect)
+	_queue_segment(tile_origin + seg_b_offset, seg_b_size, seg_b_rect)
 
 
-func _create_segment_sprite(pos: Vector2, target_size: Vector2, source_rect: Rect2) -> void:
-	var atlas := AtlasTexture.new()
-	atlas.atlas = atlas_texture
-	atlas.region = source_rect
-
-	var sprite := Sprite2D.new()
-	sprite.texture = atlas
-	sprite.centered = false
-	sprite.position = pos
-	sprite.scale = Vector2(
-		target_size.x / source_rect.size.x,
-		target_size.y / source_rect.size.y
-	)
-	add_child(sprite)
+func _queue_segment(pos: Vector2, target_size: Vector2, source_rect: Rect2) -> void:
+	_segments.append([Rect2(pos, target_size), source_rect])
 
 
-func _draw_outer_corners(_tile: Vector2i, _tile_origin: Vector2, _edges: Array) -> void:
+func _build_outer_corners(_tile: Vector2i, _tile_origin: Vector2, _edges: Array) -> void:
 	# TODO: Implement outer corner segments
 	# Check pairs of adjacent edges (e.g., "top" + "left" → top-left outer corner)
 	# and draw the corner piece from the atlas.
 	pass
 
 
-func _draw_inner_corners(_tile: Vector2i, tile_origin: Vector2, edges: Array) -> void:
+func _build_inner_corners(_tile: Vector2i, tile_origin: Vector2, edges: Array) -> void:
 	## Draws decorative corner pieces wherever two perpendicular wall edges meet
 	## on the same tile (e.g., a top wall + left wall produces an upper-left corner).
 	var has_top := edges.has(&"top")
@@ -193,25 +180,25 @@ func _draw_inner_corners(_tile: Vector2i, tile_origin: Vector2, edges: Array) ->
 	var corner_size := Vector2(CORNER_SIZE, CORNER_SIZE)
 
 	if has_top and has_left:
-		_create_segment_sprite(
+		_queue_segment(
 			tile_origin + Vector2(0, 0),
 			corner_size,
 			INNER_CORNER_UPPER_LEFT_RECT
 		)
 	if has_top and has_right:
-		_create_segment_sprite(
+		_queue_segment(
 			tile_origin + Vector2(TILE_SIZE - CORNER_SIZE, 0),
 			corner_size,
 			INNER_CORNER_UPPER_RIGHT_RECT
 		)
 	if has_bottom and has_left:
-		_create_segment_sprite(
+		_queue_segment(
 			tile_origin + Vector2(0, TILE_SIZE - CORNER_SIZE),
 			corner_size,
 			INNER_CORNER_LOWER_LEFT_RECT
 		)
 	if has_bottom and has_right:
-		_create_segment_sprite(
+		_queue_segment(
 			tile_origin + Vector2(TILE_SIZE - CORNER_SIZE, TILE_SIZE - CORNER_SIZE),
 			corner_size,
 			INNER_CORNER_LOWER_RIGHT_RECT

prefabs/unit.tscn

@@ -5,6 +5,14 @@
 [ext_resource type="Shader" uid="uid://bd8ki8xwym5nc" path="res://shaders/chroma_key.gdshader" id="3_fhs1y"]
 [ext_resource type="Texture2D" uid="uid://dyutp4m5d53gd" path="res://assets/sprites/CP002AA.BMP" id="3_on614"]
 
+[sub_resource type="GDScript" id="GDScript_on614"]
+resource_name = "UnitSelectorHandler"
+script/source = "extends ColorRect
+
+func _unit_selected_changed(_unit: Unit, selected: bool) -> void:
+	visible = selected
+"
+
 [sub_resource type="GDScript" id="GDScript_fhs1y"]
 resource_name = "AllegianceIndicatorManager"
 script/source = "extends Sprite2D
@@ -55,18 +63,17 @@ animations = [{
 "speed": 5.0
 }]
 
-[sub_resource type="GDScript" id="GDScript_on614"]
-resource_name = "UnitSelectorHandler"
-script/source = "extends ColorRect
-
-func _unit_selected_changed(_unit: Unit, selected: bool) -> void:
-	visible = selected
-"
-
 [node name="Unit" type="Node2D" unique_id=1893234933 groups=["units"]]
 script = ExtResource("1_cq4v0")
 metadata/_custom_type_script = "uid://c016mxgatcpse"
 
+[node name="SelectionIndicator" type="ColorRect" parent="." unique_id=1313394023]
+visible = false
+offset_right = 100.0
+offset_bottom = 100.0
+color = Color(1, 1, 0.3019608, 0.36078432)
+script = SubResource("GDScript_on614")
+
 [node name="AllegianceIndicator" type="Sprite2D" parent="." unique_id=1567439632]
 z_index = 2
 texture = ExtResource("2_fhs1y")
@@ -80,12 +87,5 @@ sprite_frames = SubResource("SpriteFrames_7jqdg")
 animation = &"idle"
 autoplay = "idle"
 
-[node name="ColorRect" type="ColorRect" parent="." unique_id=1313394023]
-visible = false
-offset_right = 100.0
-offset_bottom = 100.0
-color = Color(1, 1, 0.3019608, 0.36078432)
-script = SubResource("GDScript_on614")
-
 [connection signal="unit_allegiance_changed" from="." to="AllegianceIndicator" method="_on_unit_unit_allegiance_changed"]
-[connection signal="unit_selected_changed" from="." to="ColorRect" method="_unit_selected_changed"]
+[connection signal="unit_selected_changed" from="." to="SelectionIndicator" method="_unit_selected_changed"]

project.godot

@@ -30,7 +30,7 @@ debug_toggle={
 
 [physics]
 
-3d/physics_engine="Jolt Physics"
+3d/physics_engine="Dummy"
 
 [rendering]