From 48c8fb7dea9420f5c17988fbb3b5d4d0a3fa838f Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Sun, 19 Apr 2026 14:21:03 +0200 Subject: [PATCH] =?UTF-8?q?fix(ui):=20PR=20#131=20review=20round=202=20?= =?UTF-8?q?=E2=80=94=20manifest=20cleanup,=20system=20index=20helper,=20st?= =?UTF-8?q?derr=20help?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses 6 mechanical issues from PR review: - ImplantAppManifest: drop dead display_name and icon_path fields. default_key carries a TODO noting its future migration to a keybinds manifest (input concern in app manifest is a layering violation, tracked explicitly). - default_mode wire format is now a String ("fullscreen" / "insert") for mod-author discovery. ImplantRegistry parses via _MODE_MAP, caches the resolved int in _resolved_modes, and exposes get_resolved_mode(app_path). main.gd reads the resolved int directly instead of re-parsing. - Extract client/ui/implant/widgets/system_index.gd (class_name SystemIndex, static get_sorted_systems). Removes duplicated star_map_data.json loader + sort lambda from atlas_app and economics overview_screen. - ImplantApp.on_insert_deactivated() default auto-closes only when the app is active in INSERT mode. FULLSCREEN apps no longer spuriously close on insert state changes. - tooling/db/sqlite-query and sqlite-exec: --help output goes to stderr (exit 0). Keeps stdout reserved for JSON payloads so JSON-parsing callers can't get silently corrupted. Co-Authored-By: Claude Opus 4.6 --- client/scripts/main.gd | 3 +- client/ui/implant/apps/atlas/app.tres | 4 +-- client/ui/implant/apps/atlas/atlas_app.gd | 26 ++-------------- client/ui/implant/apps/economics/app.tres | 4 +-- .../apps/economics/screens/overview_screen.gd | 22 +------------ client/ui/implant/implant_app.gd | 4 ++- client/ui/implant/implant_app_manifest.gd | 8 ++--- client/ui/implant/implant_registry.gd | 20 +++++++++--- client/ui/implant/widgets/system_index.gd | 31 +++++++++++++++++++ tooling/db/sqlite-exec | 16 +++++----- tooling/db/sqlite-query | 16 +++++----- 11 files changed, 77 insertions(+), 77 deletions(-) create mode 100644 client/ui/implant/widgets/system_index.gd diff --git a/client/scripts/main.gd b/client/scripts/main.gd index ec26cd941..7ea1ec96f 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -181,7 +181,8 @@ func _unhandled_key_input(event: InputEvent) -> void: for manifest: Variant in ImplantRegistry.get_manifests(): if manifest.get("default_key") == key_event.keycode: HudGroups.toggle_app( - manifest.get("app_path"), manifest.get("default_mode", HudGroups.Mode.FULLSCREEN) + manifest.get("app_path"), + ImplantRegistry.get_resolved_mode(manifest.get("app_path", "")) ) return # [ / ] — cycle economics system selector (app-specific, not generic enough for manifest). diff --git a/client/ui/implant/apps/atlas/app.tres b/client/ui/implant/apps/atlas/app.tres index fd28cd764..b3b56ab7b 100644 --- a/client/ui/implant/apps/atlas/app.tres +++ b/client/ui/implant/apps/atlas/app.tres @@ -5,9 +5,7 @@ [resource] script = ExtResource("1") app_path = "implant/map" -display_name = "ATLAS" -icon_path = "" scene_path = "res://ui/implant/apps/atlas/atlas_app.tscn" -default_mode = 2 +default_mode = "fullscreen" default_key = 77 preserves_state = true diff --git a/client/ui/implant/apps/atlas/atlas_app.gd b/client/ui/implant/apps/atlas/atlas_app.gd index 480dce586..cb65eec87 100644 --- a/client/ui/implant/apps/atlas/atlas_app.gd +++ b/client/ui/implant/apps/atlas/atlas_app.gd @@ -6,8 +6,6 @@ extends ImplantApp signal economics_link_requested(system_id: String) -const STAR_MAP_DATA := "res://data/star_map_data.json" - var _systems: Array = [] var _system_lookup: Dictionary = {} # system_id → system dict @@ -193,24 +191,6 @@ func _system_idx_by_id(system_id: String) -> int: func _load_system_data() -> void: - if not FileAccess.file_exists(STAR_MAP_DATA): - push_warning("AtlasApp: %s not found" % STAR_MAP_DATA) - return - var file := FileAccess.open(STAR_MAP_DATA, FileAccess.READ) - if file == null: - return - var parsed: Variant = JSON.parse_string(file.get_as_text()) - file.close() - if not (parsed is Dictionary): - return - for node: Dictionary in parsed.get("nodes", []): - var sid: String = node.get("system_id", "") - if not sid.is_empty(): - _systems.append(node) - _system_lookup[sid] = node - _systems.sort_custom( - func(a: Dictionary, b: Dictionary) -> bool: - var na: String = a.get("proper_name", a.get("system_id", "")) - var nb: String = b.get("proper_name", b.get("system_id", "")) - return na < nb - ) + _systems = SystemIndex.get_sorted_systems() + for node: Dictionary in _systems: + _system_lookup[node.get("system_id", "")] = node diff --git a/client/ui/implant/apps/economics/app.tres b/client/ui/implant/apps/economics/app.tres index 242ac6f8b..0b7b0962f 100644 --- a/client/ui/implant/apps/economics/app.tres +++ b/client/ui/implant/apps/economics/app.tres @@ -5,9 +5,7 @@ [resource] script = ExtResource("1") app_path = "implant/economics" -display_name = "ECONOMICS MONITOR" -icon_path = "" scene_path = "res://ui/implant/apps/economics/economics_app.tscn" -default_mode = 1 +default_mode = "insert" default_key = 78 preserves_state = true diff --git a/client/ui/implant/apps/economics/screens/overview_screen.gd b/client/ui/implant/apps/economics/screens/overview_screen.gd index ed51abce1..26aae710a 100644 --- a/client/ui/implant/apps/economics/screens/overview_screen.gd +++ b/client/ui/implant/apps/economics/screens/overview_screen.gd @@ -10,7 +10,6 @@ extends Control signal economy_data_updated(system_id: String, data: Dictionary) const RING_BUFFER_SIZE: int = 20 -const STAR_MAP_DATA := "res://data/star_map_data.json" const PANEL_WIDTH: float = 340.0 const PANEL_MARGIN: float = 16.0 @@ -112,26 +111,7 @@ func navigate(delta: int) -> void: func _load_system_list() -> void: - if not FileAccess.file_exists(STAR_MAP_DATA): - push_warning("OverviewScreen: %s not found" % STAR_MAP_DATA) - return - var file := FileAccess.open(STAR_MAP_DATA, FileAccess.READ) - if file == null: - return - var parsed: Variant = JSON.parse_string(file.get_as_text()) - file.close() - if not (parsed is Dictionary): - return - for node: Dictionary in parsed.get("nodes", []): - var sid: String = node.get("system_id", "") - if not sid.is_empty(): - _systems.append(node) - _systems.sort_custom( - func(a: Dictionary, b: Dictionary) -> bool: - var na: String = a.get("proper_name", a.get("system_id", "")) - var nb: String = b.get("proper_name", b.get("system_id", "")) - return na < nb - ) + _systems = SystemIndex.get_sorted_systems() if not _systems.is_empty(): selected_system = _systems[0].get("system_id", "") diff --git a/client/ui/implant/implant_app.gd b/client/ui/implant/implant_app.gd index c3dac6114..702969a87 100644 --- a/client/ui/implant/implant_app.gd +++ b/client/ui/implant/implant_app.gd @@ -76,8 +76,10 @@ func on_close() -> void: func on_insert_deactivated() -> void: + # Default: close only if active in INSERT mode. FULLSCREEN apps override to customize. if manifest and HudGroups.is_app_active(manifest.app_path): - HudGroups.close_app() + if HudGroups.get_active_mode() == HudGroups.Mode.INSERT: + HudGroups.close_app() func _on_screen_changed(_screen_id: String) -> void: diff --git a/client/ui/implant/implant_app_manifest.gd b/client/ui/implant/implant_app_manifest.gd index 4812e7e51..d989315d8 100644 --- a/client/ui/implant/implant_app_manifest.gd +++ b/client/ui/implant/implant_app_manifest.gd @@ -4,9 +4,9 @@ extends Resource ## that declares identity and capabilities (#844, D-191). @export var app_path: String = "" -@export var display_name: String = "" -@export var icon_path: String = "" @export var scene_path: String = "" -@export var default_mode: int = 2 # HudGroups.Mode.FULLSCREEN -@export var default_key: int = -1 # KEY_M = 77, KEY_N = 78; -1 = no binding +@export var default_mode: String = "fullscreen" +# TODO: extract to a keybinds manifest when settings-UI remapping lands. +# KEY_M = 77, KEY_N = 78; -1 = no binding. +@export var default_key: int = -1 @export var preserves_state: bool = true diff --git a/client/ui/implant/implant_registry.gd b/client/ui/implant/implant_registry.gd index 00771997d..590d554cd 100644 --- a/client/ui/implant/implant_registry.gd +++ b/client/ui/implant/implant_registry.gd @@ -4,7 +4,10 @@ extends Node ## Does NOT reference ImplantAppManifest class_name at load time (autoload ## parse-order rule; see CLAUDE.md). +const _MODE_MAP: Dictionary = {"gameplay": 0, "insert": 1, "fullscreen": 2} + var _manifests: Array = [] # Array[ImplantAppManifest] +var _resolved_modes: Dictionary = {} # app_path -> HudGroups.Mode int var _scanned: bool = false @@ -14,11 +17,17 @@ func get_manifests() -> Array: return _manifests +func get_resolved_mode(app_path: String) -> int: + if not _scanned: + _scan() + return _resolved_modes.get(app_path, HudGroups.Mode.FULLSCREEN) + + func _scan() -> void: _scanned = true _manifests.clear() + _resolved_modes.clear() var key_owners: Dictionary = {} # default_key -> app_path (collision detection) - var valid_modes: Array = HudGroups.Mode.values() var dir := DirAccess.open("res://ui/implant/apps") if dir == null: push_warning("ImplantRegistry: could not open res://ui/implant/apps") @@ -34,12 +43,12 @@ func _scan() -> void: push_warning("ImplantRegistry: invalid or missing app_path in %s" % tres_path) else: var app_path: String = m.get("app_path") - var default_mode: int = m.get("default_mode", 2) + var mode_str: String = m.get("default_mode", "fullscreen") var default_key: int = m.get("default_key", -1) - if not valid_modes.has(default_mode): + if not _MODE_MAP.has(mode_str): push_warning( - "ImplantRegistry: invalid default_mode %d in %s — skipping" % [ - default_mode, tres_path + "ImplantRegistry: invalid default_mode \"%s\" in %s — skipping" % [ + mode_str, tres_path ] ) elif default_key >= 0 and key_owners.has(default_key): @@ -51,6 +60,7 @@ func _scan() -> void: else: if default_key >= 0: key_owners[default_key] = app_path + _resolved_modes[app_path] = _MODE_MAP[mode_str] _manifests.append(m) entry = dir.get_next() dir.list_dir_end() diff --git a/client/ui/implant/widgets/system_index.gd b/client/ui/implant/widgets/system_index.gd new file mode 100644 index 000000000..6025e2088 --- /dev/null +++ b/client/ui/implant/widgets/system_index.gd @@ -0,0 +1,31 @@ +class_name SystemIndex +## Shared system data loader for implant apps (#844). +## Static helper — call as SystemIndex.get_sorted_systems(). + +const DATA_PATH := "res://data/star_map_data.json" + + +static func get_sorted_systems() -> Array: + if not FileAccess.file_exists(DATA_PATH): + push_warning("SystemIndex: %s not found" % DATA_PATH) + return [] + var file := FileAccess.open(DATA_PATH, FileAccess.READ) + if file == null: + push_warning("SystemIndex: could not open %s" % DATA_PATH) + return [] + var parsed: Variant = JSON.parse_string(file.get_as_text()) + file.close() + if not (parsed is Dictionary): + return [] + var nodes: Array = [] + for node: Dictionary in parsed.get("nodes", []): + var sid: String = node.get("system_id", "") + if not sid.is_empty(): + nodes.append(node) + nodes.sort_custom( + func(a: Dictionary, b: Dictionary) -> bool: + var na: String = a.get("proper_name", a.get("system_id", "")) + var nb: String = b.get("proper_name", b.get("system_id", "")) + return na < nb + ) + return nodes diff --git a/tooling/db/sqlite-exec b/tooling/db/sqlite-exec index ba3174b7d..15c687b13 100755 --- a/tooling/db/sqlite-exec +++ b/tooling/db/sqlite-exec @@ -2,14 +2,14 @@ # Run an INSERT/UPDATE/DELETE on the ticketing database. Whitelistable command. # Usage: sqlite-exec "UPDATE tickets SET status='done' WHERE id=1" if [ "$1" = "--help" ] || [ "$1" = "-h" ]; then - echo "Usage: sqlite-exec " - echo "" - echo "Run an INSERT, UPDATE, or DELETE on the ticketing database." - echo "Output is JSON: {\"ok\": true, \"affected_rows\": N}" - echo "" - echo "Examples:" - echo " sqlite-exec \"UPDATE tickets SET status='done' WHERE id=42\"" - echo " sqlite-exec \"UPDATE tickets SET assigned_to='stig' WHERE id=844\"" + echo "Usage: sqlite-exec " >&2 + echo "" >&2 + echo "Run an INSERT, UPDATE, or DELETE on the ticketing database." >&2 + echo "Output is JSON: {\"ok\": true, \"affected_rows\": N}" >&2 + echo "" >&2 + echo "Examples:" >&2 + echo " sqlite-exec \"UPDATE tickets SET status='done' WHERE id=42\"" >&2 + echo " sqlite-exec \"UPDATE tickets SET assigned_to='stig' WHERE id=844\"" >&2 exit 0 fi exec python3 "$(dirname "$0")/sqlite_connector.py" execute "$@" diff --git a/tooling/db/sqlite-query b/tooling/db/sqlite-query index 460fe0e39..71293c37b 100755 --- a/tooling/db/sqlite-query +++ b/tooling/db/sqlite-query @@ -2,14 +2,14 @@ # Run a SELECT query on the ticketing database. Whitelistable command. # Usage: sqlite-query "SELECT * FROM tickets WHERE status='in_progress'" if [ "$1" = "--help" ] || [ "$1" = "-h" ]; then - echo "Usage: sqlite-query " - echo "" - echo "Run a SELECT query on the ticketing database." - echo "Output is JSON: {\"ok\": true, \"count\": N, \"rows\": [...]}" - echo "" - echo "Examples:" - echo " sqlite-query \"SELECT id, title, status FROM tickets WHERE sprint_id=36\"" - echo " sqlite-query \"SELECT * FROM tickets WHERE assigned_to='stig'\"" + echo "Usage: sqlite-query " >&2 + echo "" >&2 + echo "Run a SELECT query on the ticketing database." >&2 + echo "Output is JSON: {\"ok\": true, \"count\": N, \"rows\": [...]}" >&2 + echo "" >&2 + echo "Examples:" >&2 + echo " sqlite-query \"SELECT id, title, status FROM tickets WHERE sprint_id=36\"" >&2 + echo " sqlite-query \"SELECT * FROM tickets WHERE assigned_to='stig'\"" >&2 exit 0 fi exec python3 "$(dirname "$0")/sqlite_connector.py" query "$@"