From f51366b3f5abf9b595817e34638d7e9591fcaebe Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Fri, 13 Mar 2026 11:39:47 +0100 Subject: [PATCH] =?UTF-8?q?fix(ui):=20address=20PR=20#90=20review=20?= =?UTF-8?q?=E2=80=94=208=20items=20(W1=E2=80=93W4,=20S5=E2=80=93S6,=20A3?= =?UTF-8?q?=E2=80=93A4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit W1/A2: add SimBridge.CONNECTED guard before send_input in toggle handler W2/A1: assert PlatformInfo != null in HardwareDetector._ready(); add ordering comment in project.godot [autoload] section W3: replace FileAccess.open() with get_file_as_string() in read_benchmark_cache() — auto-closes, no leak W4/A3: add _extract_bool_setting() helper in game_state.gd that handles both {"Bool": true} and plain bool; push_warning on type mismatch S5: sync _pre_battery_pref from GameState after load_ai_pref() in HardwareDetector._ready() — closes race if system starts on battery S6: separate "red" TPT status into its own arm with STATUS_RED dot and label colour — three-colour mapping: green/yellow/red now distinct A4: replace DebugConsole.PREFS_PATH compile-time dependency in _save_ai_pref() with local SETTINGS_CFG_PATH constant S7 (battery CI guard) already applied by Hoshe in test file. Co-Authored-By: Claude Sonnet 4.6 --- client/project.godot | 2 ++ client/scripts/autoloads/game_state.gd | 17 +++++++++++++++-- client/ui/hardware_detector.gd | 8 +++++--- client/ui/settings_dialog.gd | 26 +++++++++++++++----------- 4 files changed, 37 insertions(+), 16 deletions(-) diff --git a/client/project.godot b/client/project.godot index 234ff6d10..4d65afa64 100644 --- a/client/project.godot +++ b/client/project.godot @@ -17,6 +17,8 @@ config/icon="res://icon.svg" [autoload] +; PlatformInfo MUST remain first — HardwareDetector (and other autoloads) depend on it +; being initialised before their own _ready() runs. Do not reorder. PlatformInfo="*res://scripts/autoloads/platform_info.gd" Protocol="*res://scripts/protocol/protocol.gd" SimBridge="*res://scripts/autoloads/sim_bridge.gd" diff --git a/client/scripts/autoloads/game_state.gd b/client/scripts/autoloads/game_state.gd index 9114716bd..d5a918df6 100644 --- a/client/scripts/autoloads/game_state.gd +++ b/client/scripts/autoloads/game_state.gd @@ -352,8 +352,8 @@ func apply_snapshot(snapshot: Dictionary) -> void: continue if entry.get("key") == "ai_dialogue.enabled": var val: Variant = entry.get("value") - if val is Dictionary and val.has("Bool"): - ai_enhanced_dialogue_enabled = bool(val["Bool"]) + if val != null: + ai_enhanced_dialogue_enabled = _extract_bool_setting("ai_dialogue.enabled", val) else: settings_response = null @@ -403,3 +403,16 @@ func apply_snapshot(snapshot: Dictionary) -> void: boundary_positions[pos] = true elif not has_explicit_positions: visible_positions[pos] = true + + +# -- Helpers ------------------------------------------------------------------ + +## Extract a bool from a tagged-union {"Bool": true} or plain bool value. +## Handles both serde encoding styles; emits push_warning on unrecognised format. +static func _extract_bool_setting(key: String, val: Variant) -> bool: + if val is bool: + return val + if val is Dictionary and val.has("Bool"): + return bool(val["Bool"]) + push_warning("GameState: unexpected type for setting '%s': %s" % [key, str(val)]) + return false diff --git a/client/ui/hardware_detector.gd b/client/ui/hardware_detector.gd index 9409d479a..48f09cedf 100644 --- a/client/ui/hardware_detector.gd +++ b/client/ui/hardware_detector.gd @@ -32,7 +32,9 @@ var _pre_battery_pref: bool = true # -- Lifecycle ---------------------------------------------------------------- func _ready() -> void: + assert(PlatformInfo != null, "PlatformInfo must load before HardwareDetector") load_ai_pref() + _pre_battery_pref = GameState.ai_enhanced_dialogue_enabled PlatformInfo.power_profile_changed.connect(_on_power_profile_changed) @@ -75,11 +77,11 @@ func read_benchmark_cache() -> Variant: var cache_path: String = PlatformInfo.benchmark_cache_path if not FileAccess.file_exists(cache_path): return null - var f := FileAccess.open(cache_path, FileAccess.READ) - if f == null: + var text: String = FileAccess.get_file_as_string(cache_path) + if text.is_empty(): return null var json := JSON.new() - if json.parse(f.get_as_text()) != OK: + if json.parse(text) != OK: return null var data: Variant = json.get_data() if not data is Dictionary or not data.has("tps"): diff --git a/client/ui/settings_dialog.gd b/client/ui/settings_dialog.gd index b86889f4d..f6cbdaeff 100644 --- a/client/ui/settings_dialog.gd +++ b/client/ui/settings_dialog.gd @@ -16,6 +16,9 @@ const STATUS_RED := Color("#c84040") const FONT_SIZE := 14 const FONT_SIZE_SMALL := 11 +## Shared client preferences file — avoids compile-time dependency on DebugConsole for path. +const SETTINGS_CFG_PATH := "user://settings.cfg" + const BOX_WIDTH := 460 const BOX_HEIGHT := 500 # +36 debug console, +88 AI dialogue section const PADDING := 20 @@ -203,9 +206,9 @@ func _build_ui() -> void: match _ai_hw_status: "pass", "green": status_dot.color = STATUS_GREEN - "marginal", "yellow", "red": + "marginal", "yellow": status_dot.color = STATUS_YELLOW - _: # "fail" + _: # "red", "fail" status_dot.color = STATUS_RED ai_hbox.add_child(status_dot) @@ -227,9 +230,9 @@ func _build_ui() -> void: ai_status_label.autowrap_mode = TextServer.AUTOWRAP_WORD_SMART ai_status_label.custom_minimum_size = Vector2(BOX_WIDTH - PADDING * 2, 0) match _ai_hw_status: - "marginal", "yellow", "red": + "marginal", "yellow": ai_status_label.add_theme_color_override("font_color", STATUS_YELLOW) - "fail": + "red", "fail": ai_status_label.add_theme_color_override("font_color", STATUS_RED) _: ai_status_label.add_theme_color_override("font_color", TEXT_COLOR) @@ -247,11 +250,12 @@ func _build_ui() -> void: _ai_check_node.toggled.connect(func(enabled: bool) -> void: GameState.ai_enhanced_dialogue_enabled = enabled _save_ai_pref(enabled) - SimBridge.send_input({ - "action": InputMapper.Action.CHANGE_SETTINGS, - "action_data": {"ai_enhanced_dialogue": enabled}, - "timestamp_msec": Time.get_ticks_msec(), - }) + if SimBridge.state == SimBridge.ConnectionState.CONNECTED: + SimBridge.send_input({ + "action": InputMapper.Action.CHANGE_SETTINGS, + "action_data": {"ai_enhanced_dialogue": enabled}, + "timestamp_msec": Time.get_ticks_msec(), + }) ai_dialogue_toggled.emit(enabled) ) @@ -335,9 +339,9 @@ func set_ai_inference_suspended(suspended: bool) -> void: ## Uses the same settings.cfg as DebugConsole — different section ("ai_dialogue"). func _save_ai_pref(enabled: bool) -> void: var cfg := ConfigFile.new() - cfg.load(DebugConsole.PREFS_PATH) + cfg.load(SETTINGS_CFG_PATH) cfg.set_value("ai_dialogue", "enabled", enabled) - cfg.save(DebugConsole.PREFS_PATH) + cfg.save(SETTINGS_CFG_PATH) func _draw() -> void: