diff --git a/assets/sprites/aux_terrain_tileset.bmp b/assets/sprites/aux_terrain_tileset.bmp deleted file mode 100644 index cc1a456..0000000 Binary files a/assets/sprites/aux_terrain_tileset.bmp and /dev/null differ diff --git a/assets/sprites/aux_terrain_tileset.bmp.import b/assets/sprites/aux_terrain_tileset.bmp.import deleted file mode 100644 index 950e4cb..0000000 --- a/assets/sprites/aux_terrain_tileset.bmp.import +++ /dev/null @@ -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 diff --git a/docs/review/2026-04-07-architecture-review.md b/docs/review/2026-04-07-architecture-review.md index 62a2726..6ae49ee 100644 --- a/docs/review/2026-04-07-architecture-review.md +++ b/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` @@ -151,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` @@ -179,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` @@ -219,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` @@ -247,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~~ | **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** | diff --git a/nodes/combat_map.gd b/nodes/combat_map.gd index 8fdaa87..e516b50 100644 --- a/nodes/combat_map.gd +++ b/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: diff --git a/nodes/combat_system.gd b/nodes/combat_system.gd index 72911df..1e05a1a 100644 --- a/nodes/combat_system.gd +++ b/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) diff --git a/prefabs/unit.tscn b/prefabs/unit.tscn index 47e7d7b..0d7551b 100644 --- a/prefabs/unit.tscn +++ b/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"] diff --git a/project.godot b/project.godot index 9a63225..a49fdad 100644 --- a/project.godot +++ b/project.godot @@ -30,7 +30,7 @@ debug_toggle={ [physics] -3d/physics_engine="Jolt Physics" +3d/physics_engine="Dummy" [rendering]