jpmschweitzer/settled-reach
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(client): address PR #36 review — 10 items from Hoshe + Tyre

Browse Source 
Critical:
- deactivate_insert() now called when selecting non-Insert spoke,
  cancelling with no selection, or pressing Escape while insert is
  active. Fixes simulation staying paused permanently after Insert.

Warnings:
- Checklist conditions with empty id excluded from get_results() and
  get_total_count() — prevents impossible-to-complete checklists.
  Warns at load time when empty-id conditions are found.
- _content_base now checks res://content/ first (exported builds),
  falls back to ../content for editor/dev mode.
- 26 new tests for D-054 functions: _angle_to_octant (8 octants),
  _snap_to_octant_dir (9 cases incl. zero/tiny), _wasd_to_world_dir
  (8 facing/movement combos). New test file: test_input_mapper_facing.gd.

Suggestions:
- Cached get_theme_default_font() in checklist overlay _ready().
- Documented InputMapper → GameState coupling as intentional.
- Documented YAML parser # truncation limitation.
- _insert_active reset on Escape dismiss (Tyre #3).
- SimBridge test mode SetFacing reads action_data.facing instead of
  InputMapper global (Tyre #4).
- Removed dead _facing_to_rotation() from entity_renderer.gd (Tyre #5).
- Fixed 2 failing facing indicator tests to use InputMapper.facing_angle
  instead of GameState.player_facing.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
jpmschweitzer
2026-02-18 13:09:38 +01:00
co-authored by Claude Opus 4.6
parent ab85fbd652
commit 919ef39cbd
9 changed files with 216 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
client/scripts/autoloads/input_mapper.gd
+2
View File
@@ -122,6 +122,8 @@ func flush_queue() -> Array[Dictionary]:
# D-054: Compute facing angle from mouse position relative to player screen position.
# Uses viewport canvas transform to convert world coords to screen coords.
# Intentional coupling: reads GameState.player_position directly — InputMapper is an
# autoload that runs before game loop rendering, so position is always current-tick.
func _update_facing_from_mouse() -> void:
var vp := get_viewport()
if vp == null:
client/scripts/autoloads/sim_bridge.gd
+10 -8
View File
@@ -177,7 +177,16 @@ func send_input(player_input: Dictionary) -> Error:
var action: int = player_input.get("action", -1)
var wire_name: String = _action_enum_to_wire(action)
if not wire_name.is_empty():
_test_input_queue.append(wire_name)
if wire_name == "SetFacing":
# D-054: Use action_data.facing from the input dict, not InputMapper global
var facing: String = ""
var action_data: Variant = player_input.get("action_data")
if action_data is Dictionary:
facing = str(action_data.get("facing", ""))
if not facing.is_empty():
_test_facing = facing
else:
_test_input_queue.append(wire_name)
return OK
var action_name := _action_enum_to_wire(player_input.get("action", -1))
if action_name.is_empty():
@@ -282,18 +291,11 @@ func _test_snapshot() -> Dictionary:
if dist <= 2 and _test_has_los(_test_player_pos, npc_pos):
_test_in_dialogue = true
continue
if action_name == "SetFacing":
# D-054: facing update handled separately — octant comes from InputMapper
_test_facing = InputMapper.facing_octant
continue
var delta := _action_to_delta(action_name)
var new_pos := _test_player_pos + delta
if _test_is_walkable(new_pos):
_test_player_pos = new_pos
if delta != Vector2i.ZERO:
# D-054: Facing is now mouse-driven, not movement-driven.
# Use InputMapper's octant instead of deriving from movement delta.
_test_facing = InputMapper.facing_octant
# Walk-away dismisses dialogue (D-064)
if _test_in_dialogue:
_test_in_dialogue = false
client/scripts/checklist/checklist_evaluator.gd
+29 -4
View File
@@ -24,8 +24,15 @@ var _loaded: bool = false
func _init() -> void:
var project_path := ProjectSettings.globalize_path("res://")
_content_base = project_path.path_join("../content")
# Content directory lives at repo root (content/), one level above the Godot
# project (client/). In editor/dev mode we resolve via the project path.
# In exported builds, content is expected at res://content/ (copied by export
# preset) — the globalize fallback won't exist, so check res:// first.
if DirAccess.dir_exists_absolute("res://content"):
_content_base = ProjectSettings.globalize_path("res://content")
else:
var project_path := ProjectSettings.globalize_path("res://")
_content_base = project_path.path_join("../content")
## Load checklist for a room. Clears per-room latches; cross-room latches persist.
@@ -52,6 +59,7 @@ func load_room(room_id: String) -> void:
var room_data := _load_checklist_file(room_path)
if room_data.has("conditions"):
_room_conditions = room_data["conditions"]
_warn_empty_ids(_room_conditions, room_path)
# Load cross-room checks (only on first load)
if _cross_conditions.is_empty():
@@ -59,6 +67,7 @@ func load_room(room_id: String) -> void:
var cross_data := _load_checklist_file(cross_path)
if cross_data.has("conditions"):
_cross_conditions = cross_data["conditions"]
_warn_empty_ids(_cross_conditions, cross_path)
_loaded = true
@@ -74,10 +83,13 @@ func evaluate() -> void:
## Returns array of {id, description, met} for all loaded conditions.
## Conditions with empty id are excluded (invalid, cannot be latched).
func get_results() -> Array:
var results: Array = []
for cond in _room_conditions + _cross_conditions:
var cid: String = cond.get("id", "")
if cid.is_empty():
continue
results.append({
"id": cid,
"description": cond.get("description", ""),
@@ -87,9 +99,13 @@ func get_results() -> Array:
return results
## Total number of loaded conditions.
## Total number of loaded conditions (excludes conditions with empty id).
func get_total_count() -> int:
return _room_conditions.size() + _cross_conditions.size()
var count: int = 0
for cond in _room_conditions + _cross_conditions:
if not cond.get("id", "").is_empty():
count += 1
return count
## Number of latched (met) conditions.
@@ -116,6 +132,12 @@ func reset() -> void:
_loaded = false
static func _warn_empty_ids(conditions: Array, path: String) -> void:
for i in conditions.size():
if conditions[i].get("id", "").is_empty():
push_warning("ChecklistEvaluator: condition at index %d in %s has empty id — will be excluded from results" % [i, path])
# -- Condition evaluation ------------------------------------------------------
func _evaluate_condition(cond: Dictionary) -> bool:
@@ -211,6 +233,9 @@ func _find_entity(entity_id: int) -> bool:
# -- YAML parsing (checklist-specific) -----------------------------------------
# Handles the constrained checklist YAML format: top-level key:value pairs,
# a conditions array of flat dictionaries. No nested arrays or anchors.
#
# Limitation: unquoted values containing " #" are truncated at the comment marker.
# Use quoted strings ("value # with hash") if values must contain literal hashes.
func _load_checklist_file(path: String) -> Dictionary:
if not FileAccess.file_exists(path):
client/scripts/rendering/entity_renderer.gd
-13
View File
@@ -158,16 +158,3 @@ func _add_facing_indicator(parent_node: Control) -> void:
# Position at center of parent ColorRect — rotation around this point
indicator.position = Vector2(ENTITY_SIZE / 2.0, ENTITY_SIZE / 2.0)
parent_node.add_child(indicator)
# Convert facing direction string to rotation in radians (0 = North/up)
static func _facing_to_rotation(facing: String) -> float:
match facing:
"North": return 0.0
"Northeast": return PI / 4.0
"East": return PI / 2.0
"Southeast": return 3.0 * PI / 4.0
"South": return PI
"Southwest": return 5.0 * PI / 4.0
"West": return 3.0 * PI / 2.0
"Northwest": return 7.0 * PI / 4.0
_: return 0.0