docs(architecture): update implant-app-pattern for PR #131 review outcomes

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 <noreply@anthropic.com>
This commit is contained in:
2026-04-19 15:18:15 +02:00
co-authored by Claude Opus 4.6
parent 284ac44412
commit 48445d45f8
+73 -34
View File
@@ -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