From 48445d45f808bf8ca1fab5d5cd9c57d8138328c6 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Sun, 19 Apr 2026 15:18:15 +0200 Subject: [PATCH] docs(architecture): update implant-app-pattern for PR #131 review outcomes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reflects the final shape of the ImplantApp pattern after PR #131 review rounds: - Manifest: schema_version field, default_mode as String, drop display_name / icon_path (no callers). Documents the tiered schema_version behavior and the default_mode string-to-enum resolution. - ImplantApp base class: add register_screen, current_screen_id, default _on_screen_changed with has_method tolerance + same-screen- replace detection. Drop the unused insert_deactivated signal; on_insert_deactivated() default closes if active in INSERT mode. - Screens paragraph: rewrite to describe subclass-constructs-then- registers flow; base owns add_child, visibility, enter/leave dispatch. - Phasing: promote "Lands this PR" to "Landed — Sprint 36 #844 and review rounds" with concrete surface of the full shipped API (instantiate_all, get_app_instance, get_resolved_mode, schema_version). - Review checklist: rewrite as a per-app PR checklist for future apps entering the pattern (manifest shape, on_install contract, no direct instantiation in hud.tscn, no KEY_* literals in main.gd). Draft by Tyre; committed by team lead per the team-lead-commits rule. Co-Authored-By: Claude Opus 4.6 --- docs/architecture/implant-app-pattern.md | 107 ++++++++++++++++------- 1 file changed, 73 insertions(+), 34 deletions(-) diff --git a/docs/architecture/implant-app-pattern.md b/docs/architecture/implant-app-pattern.md index 549663b7f..661d29d0a 100644 --- a/docs/architecture/implant-app-pattern.md +++ b/docs/architecture/implant-app-pattern.md @@ -36,15 +36,25 @@ Each app directory contains an `app.tres` (Godot `Resource`) declaring the app's extends Resource class_name ImplantAppManifest +@export var schema_version: int = 1 # bump on breaking manifest changes @export var app_path: String = "" # unique HudGroups id, e.g. "implant/map" -@export var display_name: String = "" # human-facing, e.g. "Atlas" -@export var icon_path: String = "" # res:// path to icon texture -@export var scene_path: String = "" # res:// path to the app's root scene -@export var default_mode: int = 2 # HudGroups.Mode.FULLSCREEN -@export var default_key: int = -1 # e.g. KEY_M; -1 = no default binding +@export var scene_path: String = "" # res:// path to the app's root scene (empty = metadata-only) +@export var default_mode: String = "fullscreen" # "gameplay" | "insert" | "fullscreen" +@export var default_key: int = -1 # e.g. KEY_M = 77; -1 = no default binding @export var preserves_state: bool = true # keep nav stack across close/open ``` +**`schema_version`** lets the registry differentiate mods built against older or newer manifest shapes. `ImplantRegistry.CURRENT_SCHEMA_VERSION` names the version the current build understands. On scan: +- `schema_version == CURRENT`: silent, proceed normally. +- `schema_version < CURRENT`: `print_verbose("backfilled from v{n}")`, proceed with field defaults. +- `schema_version > CURRENT`: `push_warning("proceeding best-effort")`, proceed. + +Reject-on-unknown is deliberately avoided — forward tolerance matters more than strict validation in a modded context. An app that relies on a field the build does not understand may malfunction at runtime; that failure is traceable via the warning. + +**`default_mode`** is a string (`"gameplay"` / `"insert"` / `"fullscreen"`) rather than the raw `HudGroups.Mode` int, because editing enum ints in `.tres` is fragile. The registry resolves the string to `HudGroups.Mode` via `ImplantRegistry.get_resolved_mode(app_path)`; invalid strings skip the manifest with a warning. + +`display_name` and `icon_path` are intentionally omitted at this stage — they add surface without callers. Add them only when a launcher UI needs them. + Keeping the manifest declarative — a `Resource`, not a script — means the installed-app list is data. Mod authors don't need to touch GDScript to register. The scan output can be cached. Validation happens in one place. ## Generic key routing in `main.gd` @@ -83,9 +93,11 @@ extends Control var manifest: ImplantAppManifest = null var nav: ImplantNavStack = null +var _screens: Dictionary = {} # screen_id → Control +var _current_screen_id: String = "" + signal app_opened(mode: int) signal app_closed -signal insert_deactivated func _ready() -> void: set_anchors_preset(Control.PRESET_FULL_RECT) @@ -130,26 +142,47 @@ func _internal_app_changed(app_path: String, mode: int) -> void: on_close() app_closed.emit() +# --- Screen management (base-owned) --- + +## 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: + # Warns + returns on duplicate id; base-parents if unparented; forces visible = false. + ... + +func current_screen_id() -> String: + return _current_screen_id + +## Default screen-swap. Base handles leave/hide/enter/show with has_method tolerance +## and skips leave/hide when replacing the same screen id (payload-only update). +## Subclasses rarely override. +func _on_screen_changed(new_id: String) -> void: + # See implant_app.gd for the full body. + ... + # --- Lifecycle hooks — subclasses override --- func on_install() -> void: pass # once, after _ready func on_open(_mode: int) -> void: pass # every transition to visible func on_close() -> void: pass # every transition to hidden -func on_insert_deactivated() -> void: pass # SnapshotConsumers calls this -func _on_screen_changed(_screen_id: String) -> void: pass # override to swap visible screen +func on_insert_deactivated() -> void: # SnapshotConsumers calls this + # Default: closes the app if it is active in INSERT mode. FULLSCREEN apps inherit no-op. + pass func handle_intent(_action: String, _params: Dictionary) -> void: pass # future — Intents ``` **What the subclass is responsible for:** -- Building its root scene (via `scene_path` in the manifest). -- Registering screens with the nav stack. -- Overriding `_on_screen_changed` to swap the visible screen. -- Emitting app-specific signals (e.g. `economics_link_requested` today, `intent_requested` later). +- Loading its own manifest in `_ready()` before calling `super._ready()`. +- In `on_install`: constructing screens, calling `setup`/signal wiring on them, then `register_screen(id, screen)`. Setting `nav.set_default(id)` once. +- Reading state via `current_screen_id()`, emitting app-specific signals, handling app-specific input. **What the subclass is *not* responsible for:** - `HudGroups` wiring. Ever. -- `visible` management. Ever. +- `visible` management (app-level or screen-level). Ever. - Anchor / mouse-filter boilerplate. Ever. +- `add_child` on registered screens. `register_screen` handles parenting. +- `_current_screen_id` bookkeeping. The base owns it. +- Enter/leave dispatch. The default `_on_screen_changed` drives it; screens that need enter/leave implement those methods, the base invokes them via `has_method` tolerance. ## `ImplantNavStack` — intra-app navigation @@ -179,14 +212,16 @@ func current_payload() -> Dictionary **Mutation is synchronous.** `push()` updates `_stack`, emits `screen_changed` before returning. The shell's handler synchronously calls `enter(payload)` on the new screen and `leave()` on the previous one. No `call_deferred` races; a frame always ends with a coherent nav state. -**Screens are plain `Control` nodes.** They expose: +**Screens are plain `Control` nodes, registered via `ImplantApp.register_screen(id, screen)`.** Subclass `on_install` constructs each screen, wires its signals, and calls `register_screen` — base handles `add_child` and `visible = false`. Screens never touch `HudGroups`, `APP_PATH`, `app_changed`, or their own visibility. They only emit nav signals back to the shell (e.g. `select_system(system_id)`), and the shell translates to `nav.push("system", {...})`. + +Screens optionally expose: ```gdscript -func enter(payload: Dictionary) -> void # called when pushed / re-surfaced -func leave() -> void # called when a new screen pushes on top / popped +func enter(payload: Dictionary) -> void # called when pushed or re-surfaced +func leave() -> void # called when popped or superseded ``` -Screens never touch `HudGroups`, `APP_PATH`, or `app_changed`. They only emit nav signals back to the shell — e.g. `select_system(system_id)` — and the shell translates to `nav.push("system", {...})`. +The base's default `_on_screen_changed` invokes these via `has_method` — screens that need neither don't ship stubs. The base also detects same-screen-replace (`nav.replace("system", new_payload)` while already on `"system"`) and re-enters with the fresh payload without calling `leave` first. Atlas exercises this on the reach → system-picker → system-orbital transition. ### Nav-stack edge cases @@ -280,16 +315,20 @@ Autoload order: `implant_registry` must scan before `main.gd` queries manifests ## Phasing -### Lands this PR (#844) +### Landed — Sprint 36 #844 and review rounds - `implant_app.gd`, `implant_nav_stack.gd`, `implant_app_manifest.gd` — new classes -- `implant_registry.gd` — new autoload; scans `apps/*/app.tres`, exposes `get_manifests()` +- `implant_registry.gd` — autoload; lazy-scans `apps/*/app.tres` on first `get_manifests()` call, exposes `get_manifests()`, `get_resolved_mode()`, `instantiate_all(parent)`, `get_app_instance(app_path)` +- Manifest `schema_version` field with `ImplantRegistry.CURRENT_SCHEMA_VERSION = 1`; tiered warn/backfill/proceed behavior +- `scene_path` consumed: `ImplantRegistry.instantiate_all($AppsContainer)` called from `hud.gd._ready()`. `hud.tscn` no longer direct-instances apps — holds only the `AppsContainer` placement node. `main.gd` looks up app instances via `ImplantRegistry.get_app_instance(app_path)`. - `main.gd` key routing replaced with registry-driven loop; `KEY_M` / `KEY_N` literals removed -- Atlas: split into `apps/atlas/` with `atlas_app.gd` + per-screen `Control`s. `Level` enum removed in favor of nav-stack screen ids -- Economics: relocated to `apps/economics/economics_app.gd`, refactored to `extends ImplantApp` -- Both apps ship their `app.tres` manifests -- `SnapshotConsumers` updated to call `on_insert_deactivated()` uniformly instead of per-panel hacks +- `ImplantApp` base owns `_screens` / `_current_screen_id`; `register_screen(id, screen)` + `current_screen_id()` + default `_on_screen_changed` with `has_method` tolerance and same-screen-replace detection +- Atlas: split into `apps/atlas/` with `atlas_app.gd` + per-screen `Control`s. `Level` enum replaced by nav-stack screen ids. `on_install` is construct → setup → wire → `register_screen` per screen. +- Economics: relocated to `apps/economics/economics_app.gd`, refactored to `extends ImplantApp`; `on_install` reduced to three lines (construct overview_screen, register_screen, nav.set_default) +- Both apps ship `schema_version = 1` manifests +- `SnapshotConsumers` calls `on_insert_deactivated()` uniformly; the base's default closes the app when active in INSERT mode - Legacy `implant/map/starchart` HudGroups path deleted +- `hud.gd`: single `_ready()` call to `ImplantRegistry.instantiate_all($AppsContainer)` seeds all registered apps; Godot's bottom-up `_ready` ordering guarantees `main.gd` finds them when it looks up by `app_path` ### Follow-up (not this sprint, not blockers) @@ -305,18 +344,18 @@ Autoload order: `implant_registry` must scan before `main.gd` queries manifests - Scripting API surface — what mods can safely call into core - Mod registry UI — enable/disable, conflict resolution, version compatibility -## Review checklist for #844 +## Review checklist — for future PRs adding a new implant app -Use while implementing; check during review. - -- [ ] Every cargo-culted snippet absorbed into `ImplantApp`: anchoring, `visible = false`, `MOUSE_FILTER_STOP`, `HudGroups.register`, `app_changed` filtering, `set_insert_active`, `toggle_visible`. No app shell re-implements any of these. -- [ ] Atlas shell uses `ImplantNavStack` — no resurrected `Level` enum by another name. -- [ ] Screens are plain `Control`s. They do not touch `HudGroups`, `APP_PATH`, or `app_changed`. They only emit nav signals to the shell. -- [ ] `SnapshotConsumers` calls `on_insert_deactivated()` uniformly — no per-panel type checks. -- [ ] Economics refactor is behaviorally identical — same `economics_link_requested` signal path, intra-app selection state preserved across close/reopen. -- [ ] `main.gd` contains no `KEY_M` / `KEY_N` literals tied to specific apps — all routing via `ImplantRegistry.get_manifests()`. -- [ ] Both apps ship a valid `app.tres` with all required fields. -- [ ] No app reaches into another app's internals — cross-app communication stays on the `economics_link_requested` seam (until Intents lands). +- [ ] App directory lives under `client/ui/implant/apps/{name}/` with `app.tres`, `{name}_app.gd`, `{name}_app.tscn` +- [ ] Manifest includes `schema_version: int = 1` (or higher if manifest schema has bumped) +- [ ] Manifest `default_mode` is one of `"gameplay"`, `"insert"`, `"fullscreen"` +- [ ] Manifest `app_path` is unique; `default_key` either -1 or non-colliding +- [ ] App script `extends ImplantApp`; its `_ready()` assigns `manifest = load("res://ui/implant/apps/{name}/app.tres")` then calls `super._ready()` +- [ ] `on_install` constructs and `register_screen`s each screen; sets `nav.set_default("...")` once. No direct `add_child` on screens; no `.visible =` on screens; no `_current_screen_id` field. +- [ ] Screens are plain `Control`s. They optionally define `enter(payload)` / `leave()`. They do not touch `HudGroups`, `APP_PATH`, or `app_changed`. +- [ ] Nothing in `hud.tscn` direct-instances the new app. Discovery happens via the registry. +- [ ] No `KEY_*` literals in `main.gd` tied to the new app — rely on `default_key` in the manifest. +- [ ] Cross-app wiring (if any) stays on a named signal today; migrate to `handle_intent` when Intents dispatcher lands. ## Risks / open items