From 284ac44412217e7edfab812188f575a6920a7a4f Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Sun, 19 Apr 2026 15:11:12 +0200 Subject: [PATCH] =?UTF-8?q?feat(ui):=20PR=20#131=20review=20round=204=20?= =?UTF-8?q?=E2=80=94=20scene=5Fpath,=20schema=5Fversion,=20screens=20absor?= =?UTF-8?q?ption?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three architectural review comments implemented per Tyre's proposals: #3 — scene_path consumption (full, Option A) - ImplantRegistry.instantiate_all(parent) loads and instantiates all registered apps with a declared scene_path. Manifests with empty scene_path are treated as metadata-only (silently skipped). - ImplantRegistry.get_app_instance(app_path) returns the live instance. - hud.tscn no longer direct-instances AtlasApp or EconomicsApp — an AppsContainer Control holds the registry-managed children. - hud.gd._ready() calls ImplantRegistry.instantiate_all($AppsContainer). - main.gd drops @onready vars for atlas_app/economics_app; looks up both from the registry at the top of _ready(). #6c — schema_version on manifest - ImplantAppManifest: @export var schema_version: int = 1 (first field). - ImplantRegistry: const CURRENT_SCHEMA_VERSION := 1; tiered check in _scan() — older-than-current emits print_verbose, newer-than-current emits push_warning, both proceed best-effort. - apps/atlas/app.tres + apps/economics/app.tres: schema_version = 1. #10 — absorb _on_screen_changed boilerplate into ImplantApp - Base class gains _screens: Dictionary, _current_screen_id: String, register_screen(id, screen), current_screen_id(), and a real default _on_screen_changed implementation that handles leave+hide+enter+show with has_method guards and same-screen-replace detection. - atlas_app: deletes _current_screen_id, _get_screen(), _on_screen_changed() override; on_install() collapses to construct → setup → wire → register_screen(id, screen) per screen. Preserves direct screen refs for atlas-specific signal wiring and method calls. - economics_app: deletes same scaffolding; on_install() reduces to three lines (construct overview_screen, register_screen, nav.set_default). Also: replaced Resource.get(name, default) dict-style calls with direct property access on typed ImplantAppManifest reads (2-arg get() is Dictionary-only; causes "Too many arguments" parse errors on Resources). _is_valid_manifest gained a Resource type guard and a property-exists check. Co-Authored-By: Claude Opus 4.6 --- client/scripts/main.gd | 9 ++- client/ui/hud.gd | 2 + client/ui/hud.tscn | 21 +++---- client/ui/implant/apps/atlas/app.tres | 1 + client/ui/implant/apps/atlas/atlas_app.gd | 59 ++++-------------- client/ui/implant/apps/economics/app.tres | 1 + .../implant/apps/economics/economics_app.gd | 26 +------- client/ui/implant/implant_app.gd | 37 +++++++++++- client/ui/implant/implant_app_manifest.gd | 1 + client/ui/implant/implant_registry.gd | 60 ++++++++++++++++--- 10 files changed, 120 insertions(+), 97 deletions(-) diff --git a/client/scripts/main.gd b/client/scripts/main.gd index 734a109e7..488763b15 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -2,6 +2,9 @@ extends Node2D const TELEPORT_DISTANCE_THRESHOLD: float = 5.0 +var economics_app = null # EconomicsApp — populated in _ready() via ImplantRegistry +var atlas_app = null # AtlasApp — populated in _ready() via ImplantRegistry + var _camera_anchored: bool = false var _flash_rect: ColorRect = null # #502/#501: ephemeral screen flash overlay var _teleport_in_progress: bool = false # #501/#117: forces camera snap on next frame @@ -32,13 +35,15 @@ var _dialogue: DialogueCoordinator # #775: dialogue consumers + signal handlers @onready var loading_screen = $ModalLayer/LoadingScreen # #257: blocking overlay during load @onready var debug_console = $ModalLayer/DebugConsole # #581: tilde debug console @onready var news_ticker = $UILayer/NewsTicker # #592: scrolling headline bar (D-049 z-7) -@onready var economics_app = $InsertOverlay/HUD/EconomicsApp # #824: economics monitor (D-170) -@onready var atlas_app = $InsertOverlay/HUD/AtlasApp # #844: atlas implant — reach → system → planet → regional (D-191) func _ready() -> void: print("The Settled Reach — client initialized") + # #844 D-191: Populate app refs from registry (hud._ready already called instantiate_all). + atlas_app = ImplantRegistry.get_app_instance("implant/map") + economics_app = ImplantRegistry.get_app_instance("implant/economics") + # #117: Manual lerp approach — disable Godot's built-in Camera2D smoothing. camera.position_smoothing_enabled = false diff --git a/client/ui/hud.gd b/client/ui/hud.gd index 076be411f..4018e4eba 100644 --- a/client/ui/hud.gd +++ b/client/ui/hud.gd @@ -10,6 +10,8 @@ var _perception_row: ImplantDataRow func _ready() -> void: + ImplantRegistry.instantiate_all($AppsContainer) + # Remove the old raw MarginContainer/labels if present var old := get_node_or_null("MarginContainer") if old: diff --git a/client/ui/hud.tscn b/client/ui/hud.tscn index 01c648f7d..834cef69e 100644 --- a/client/ui/hud.tscn +++ b/client/ui/hud.tscn @@ -1,8 +1,6 @@ -[gd_scene load_steps=3 format=3 uid="uid://cq1y5w3hmxr8b"] +[gd_scene load_steps=2 format=3 uid="uid://cq1y5w3hmxr8b"] [ext_resource type="Script" path="res://ui/hud.gd" id="1_hud"] -[ext_resource type="PackedScene" path="res://ui/implant/apps/economics/economics_app.tscn" id="2_econ"] -[ext_resource type="PackedScene" path="res://ui/implant/apps/atlas/atlas_app.tscn" id="3_atlas"] [node name="HUD" type="Control"] layout_mode = 3 @@ -14,12 +12,11 @@ grow_vertical = 2 mouse_filter = 2 script = ExtResource("1_hud") -; #824: Economics Monitor — implant/economics INSERT panel. Toggled via N key (manifest.default_key). -; Composes ImplantPanel from the D-169 component library. Placeholder data until #822 ships. -[node name="EconomicsApp" parent="." instance=ExtResource("2_econ")] -visible = false - -; #844: Atlas implant app — FULLSCREEN app at implant/map per D-170. -; Reach map → system orbital → planet entry → regional heightmap viewer. -[node name="AtlasApp" parent="." instance=ExtResource("3_atlas")] -visible = false +[node name="AppsContainer" type="Control" parent="."] +layout_mode = 1 +anchors_preset = 15 +anchor_right = 1.0 +anchor_bottom = 1.0 +grow_horizontal = 2 +grow_vertical = 2 +mouse_filter = 2 diff --git a/client/ui/implant/apps/atlas/app.tres b/client/ui/implant/apps/atlas/app.tres index b3b56ab7b..d8decc82c 100644 --- a/client/ui/implant/apps/atlas/app.tres +++ b/client/ui/implant/apps/atlas/app.tres @@ -4,6 +4,7 @@ [resource] script = ExtResource("1") +schema_version = 1 app_path = "implant/map" scene_path = "res://ui/implant/apps/atlas/atlas_app.tscn" default_mode = "fullscreen" diff --git a/client/ui/implant/apps/atlas/atlas_app.gd b/client/ui/implant/apps/atlas/atlas_app.gd index cb65eec87..aaf8ff8d2 100644 --- a/client/ui/implant/apps/atlas/atlas_app.gd +++ b/client/ui/implant/apps/atlas/atlas_app.gd @@ -13,7 +13,6 @@ var _reach_screen = null # ReachScreen var _system_screen = null # SystemScreen var _planet_screen = null # PlanetScreen var _regional_screen = null # RegionalScreen -var _current_screen_id: String = "" func _ready() -> void: @@ -26,65 +25,41 @@ func on_install() -> void: var implant_theme = load("res://ui/implant/default_implant.tres") _reach_screen = ReachScreen.new() - _reach_screen.name = "ReachScreen" - _reach_screen.visible = false - add_child(_reach_screen) _reach_screen.setup(implant_theme) _reach_screen.set_systems(_systems, _system_lookup) _reach_screen.system_selected.connect(_on_system_selected) + register_screen("reach", _reach_screen) _system_screen = SystemScreen.new() - _system_screen.name = "SystemScreen" - _system_screen.visible = false - add_child(_system_screen) _system_screen.setup(implant_theme) _system_screen.set_systems(_systems) _system_screen.body_selected.connect(_on_body_selected) + register_screen("system", _system_screen) _planet_screen = PlanetScreen.new() - _planet_screen.name = "PlanetScreen" - _planet_screen.visible = false - add_child(_planet_screen) _planet_screen.setup(implant_theme) + register_screen("planet", _planet_screen) _regional_screen = RegionalScreen.new() - _regional_screen.name = "RegionalScreen" - _regional_screen.visible = false - add_child(_regional_screen) _regional_screen.back_requested.connect(_on_regional_back) _regional_screen.economics_link_requested.connect(_forward_economics_link) + register_screen("regional", _regional_screen) nav.set_default("reach") func on_open(_mode: int) -> void: # Base class handles nav.push_default() on first open. - if _current_screen_id == "reach" and _reach_screen: + if current_screen_id() == "reach" and _reach_screen: _reach_screen.refresh_info_panel_visibility() -func _on_screen_changed(new_id: String) -> void: - # Skip leave/hide when replacing the same screen (e.g. picker→orbital transition). - if _current_screen_id != new_id: - var old := _get_screen(_current_screen_id) - if old: - old.leave() - old.visible = false - - _current_screen_id = new_id - - var s := _get_screen(new_id) - if s: - s.visible = true - s.enter(nav.current_payload()) - - func _unhandled_key_input(event: InputEventKey) -> void: if manifest == null or not HudGroups.is_app_active(manifest.app_path): return if not event.is_pressed() or event.is_echo(): return - if _current_screen_id == "regional": + if current_screen_id() == "regional": return # AtlasViewer handles its own keyboard input _handle_key(event) get_viewport().set_input_as_handled() @@ -93,22 +68,22 @@ func _unhandled_key_input(event: InputEventKey) -> void: func _handle_key(event: InputEventKey) -> void: match event.keycode: KEY_ESCAPE: - if _current_screen_id == "reach": + if current_screen_id() == "reach": HudGroups.close_app() else: nav.pop() KEY_ENTER, KEY_KP_ENTER: _handle_enter() KEY_BRACKETLEFT: - if _current_screen_id == "system" and _system_screen: + if current_screen_id() == "system" and _system_screen: _system_screen.navigate_system(-1) KEY_BRACKETRIGHT: - if _current_screen_id == "system" and _system_screen: + if current_screen_id() == "system" and _system_screen: _system_screen.navigate_system(1) func _handle_enter() -> void: - match _current_screen_id: + match current_screen_id(): "reach": if _reach_screen and _reach_screen.has_selection(): _reach_screen.trigger_enter() @@ -169,20 +144,6 @@ func _forward_economics_link(system_id: String) -> void: # ============================================================================= -func _get_screen(screen_id: String) -> Control: - match screen_id: - "reach": - return _reach_screen as Control - "system": - return _system_screen as Control - "planet": - return _planet_screen as Control - "regional": - return _regional_screen as Control - _: - return null - - func _system_idx_by_id(system_id: String) -> int: for i: int in range(_systems.size()): if _systems[i].get("system_id", "") == system_id: diff --git a/client/ui/implant/apps/economics/app.tres b/client/ui/implant/apps/economics/app.tres index 0b7b0962f..0be7f728e 100644 --- a/client/ui/implant/apps/economics/app.tres +++ b/client/ui/implant/apps/economics/app.tres @@ -4,6 +4,7 @@ [resource] script = ExtResource("1") +schema_version = 1 app_path = "implant/economics" scene_path = "res://ui/implant/apps/economics/economics_app.tscn" default_mode = "insert" diff --git a/client/ui/implant/apps/economics/economics_app.gd b/client/ui/implant/apps/economics/economics_app.gd index c15921191..9ede29730 100644 --- a/client/ui/implant/apps/economics/economics_app.gd +++ b/client/ui/implant/apps/economics/economics_app.gd @@ -5,7 +5,6 @@ extends ImplantApp ## Delegates all data/rendering to OverviewScreen. var _overview_screen = null # OverviewScreen -var _current_screen_id: String = "" func _ready() -> void: @@ -15,33 +14,10 @@ func _ready() -> void: func on_install() -> void: _overview_screen = OverviewScreen.new() - _overview_screen.name = "OverviewScreen" - _overview_screen.visible = false - add_child(_overview_screen) + register_screen("overview", _overview_screen) nav.set_default("overview") -func _on_screen_changed(new_id: String) -> void: - if _current_screen_id != new_id: - var old := _get_screen(_current_screen_id) - if old: - old.leave() - old.visible = false - - _current_screen_id = new_id - - var s := _get_screen(new_id) - if s: - s.visible = true - s.enter(nav.current_payload()) - - -func _get_screen(screen_id: String) -> Control: - if screen_id == "overview": - return _overview_screen as Control - return null - - # ============================================================================= # Public API — delegates to OverviewScreen # ============================================================================= diff --git a/client/ui/implant/implant_app.gd b/client/ui/implant/implant_app.gd index 1be95d6a6..b406e981c 100644 --- a/client/ui/implant/implant_app.gd +++ b/client/ui/implant/implant_app.gd @@ -15,6 +15,9 @@ signal app_closed var manifest: ImplantAppManifest = null var nav: ImplantNavStack = null +var _screens: Dictionary = {} # screen_id → Control +var _current_screen_id: String = "" + func _ready() -> void: set_anchors_preset(Control.PRESET_FULL_RECT) @@ -81,8 +84,38 @@ func on_insert_deactivated() -> void: HudGroups.close_app() -func _on_screen_changed(_screen_id: String) -> void: - pass +# --- Screen management --- + + +## Register a screen under an id. Base adds it as a child and starts it hidden. +## Call from on_install(). Wire signals before calling register_screen. +func register_screen(id: String, screen: Control) -> void: + if _screens.has(id): + push_warning("ImplantApp: screen id '%s' already registered" % id) + return + _screens[id] = screen + screen.visible = false + if screen.get_parent() == null: + add_child(screen) + + +func current_screen_id() -> String: + return _current_screen_id + + +func _on_screen_changed(new_id: String) -> void: + if _current_screen_id != new_id: + var old: Control = _screens.get(_current_screen_id, null) + if old: + if old.has_method("leave"): + old.leave() + old.visible = false + _current_screen_id = new_id + var s: Control = _screens.get(new_id, null) + if s: + s.visible = true + if s.has_method("enter"): + s.enter(nav.current_payload()) func handle_intent(_action: String, _params: Dictionary) -> void: diff --git a/client/ui/implant/implant_app_manifest.gd b/client/ui/implant/implant_app_manifest.gd index d989315d8..2b8e931a7 100644 --- a/client/ui/implant/implant_app_manifest.gd +++ b/client/ui/implant/implant_app_manifest.gd @@ -3,6 +3,7 @@ extends Resource ## Manifest resource for an implant app. Each app directory ships one app.tres ## that declares identity and capabilities (#844, D-191). +@export var schema_version: int = 1 @export var app_path: String = "" @export var scene_path: String = "" @export var default_mode: String = "fullscreen" diff --git a/client/ui/implant/implant_registry.gd b/client/ui/implant/implant_registry.gd index 590d554cd..24d646f9a 100644 --- a/client/ui/implant/implant_registry.gd +++ b/client/ui/implant/implant_registry.gd @@ -1,13 +1,15 @@ extends Node ## Lazy registry of installed implant apps (#844, D-191). ## Autoload — scans apps/*/app.tres on first get_manifests() call. -## Does NOT reference ImplantAppManifest class_name at load time (autoload -## parse-order rule; see CLAUDE.md). +## Does NOT reference ImplantAppManifest or ImplantApp class_names at load time +## (autoload parse-order rule; see CLAUDE.md). +const CURRENT_SCHEMA_VERSION := 1 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 _instances: Dictionary = {} # app_path -> ImplantApp var _scanned: bool = false @@ -23,6 +25,33 @@ func get_resolved_mode(app_path: String) -> int: return _resolved_modes.get(app_path, HudGroups.Mode.FULLSCREEN) +## Instantiate all registered apps whose manifest declares a scene_path and add +## them as children of parent. Manifests without scene_path are metadata-only +## and are silently skipped. Called from hud.gd._ready() — not from _ready() here. +func instantiate_all(parent: Node) -> void: + for m in get_manifests(): + var scene_path: String = m.scene_path + if scene_path.is_empty(): + continue + if not ResourceLoader.exists(scene_path): + push_warning( + "ImplantRegistry: scene_path not found for %s: %s" % [m.app_path, scene_path] + ) + continue + var packed = load(scene_path) + if not (packed is PackedScene): + push_warning("ImplantRegistry: %s is not a PackedScene" % scene_path) + continue + var instance = packed.instantiate() + _instances[m.app_path] = instance + parent.add_child(instance) + + +## Return the live ImplantApp instance for app_path, or null if not instantiated. +func get_app_instance(app_path: String): # returns ImplantApp + return _instances.get(app_path, null) + + func _scan() -> void: _scanned = true _manifests.clear() @@ -42,9 +71,23 @@ func _scan() -> void: if not _is_valid_manifest(m): push_warning("ImplantRegistry: invalid or missing app_path in %s" % tres_path) else: - var app_path: String = m.get("app_path") - var mode_str: String = m.get("default_mode", "fullscreen") - var default_key: int = m.get("default_key", -1) + var app_path: String = m.app_path + var mode_str: String = m.default_mode + var default_key: int = m.default_key + var schema_ver: int = m.schema_version + # Schema version check — best-effort in both directions + if schema_ver < CURRENT_SCHEMA_VERSION: + print_verbose( + "ImplantRegistry: backfilled manifest from v%d at %s" % [ + schema_ver, tres_path + ] + ) + elif schema_ver > CURRENT_SCHEMA_VERSION: + push_warning( + "ImplantRegistry: manifest at %s declares schema_version %d; this build supports v%d; proceeding best-effort" % [ + tres_path, schema_ver, CURRENT_SCHEMA_VERSION + ] + ) if not _MODE_MAP.has(mode_str): push_warning( "ImplantRegistry: invalid default_mode \"%s\" in %s — skipping" % [ @@ -69,5 +112,8 @@ func _scan() -> void: func _is_valid_manifest(m: Variant) -> bool: if m == null: return false - var app_path = m.get("app_path") - return app_path != null and not (app_path as String).is_empty() + if not (m is Resource): + return false + if not "app_path" in m: + return false + return not (m.app_path as String).is_empty()