From 522116fb121fbff79b047e85fa983fe6cb7ba995 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Sun, 19 Apr 2026 15:26:31 +0200 Subject: [PATCH] =?UTF-8?q?docs:=20PR=20#131=20review=20=E2=80=94=20lifecy?= =?UTF-8?q?cle=20ordering=20contract=20(item=209)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documents the ImplantApp lifecycle ordering and the nav-stack state guarantee at each hook: - Class-level docstring on implant_app.gd describes on_install, on_open, on_close, and on_insert_deactivated: when each fires, what nav state subclasses can rely on, and what is safe to do (construct + register_screen in on_install; data refresh + read nav.current() in on_open; pause timers in on_close; no close_app manual call in on_insert_deactivated — call super or replicate the guard). - Arch doc gains a "Lifecycle hooks" subsection under ImplantApp base class with a four-row contract table plus explanatory notes on two load-bearing invariants: why on_install sees an empty stack (bottom-up _ready order, no open signal yet); why on_close must not push/pop (would destroy preserved position on reopen). Co-Authored-By: Claude Opus 4.6 --- client/ui/implant/implant_app.gd | 29 +++++++++++++++++++++++- docs/architecture/implant-app-pattern.md | 15 ++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/client/ui/implant/implant_app.gd b/client/ui/implant/implant_app.gd index b406e981c..dbe737dbe 100644 --- a/client/ui/implant/implant_app.gd +++ b/client/ui/implant/implant_app.gd @@ -3,7 +3,34 @@ extends Control ## Base class for all implant apps. Absorbs HudGroups boilerplate; subclasses ## override lifecycle hooks only (#844, D-191). ## -## Subclass _ready() pattern: +## === Lifecycle ordering === +## +## on_install() — called once from _ready(), after nav is created but BEFORE any +## HudGroups open event fires. The nav stack is EMPTY at this point. Use this +## hook to: construct screens and call register_screen(id, screen), set +## nav.set_default("..."), wire intra-screen signals. Do NOT rely on +## current_screen_id() here — no screen has been pushed yet. +## +## on_open(mode) — called every time HudGroups activates this app (FULLSCREEN or +## INSERT). By the time on_open fires, the base has ensured the nav stack is +## non-empty: if preserves_state=false, nav.reset_to_default() was called; if +## preserves_state=true and the stack was empty, nav.push_default() was called. +## nav.current() returns the visible screen id. Safe to read navigation state +## and trigger data refreshes here. +## +## on_close() — called every time HudGroups deactivates this app (GAMEPLAY mode +## or another app taking focus). Nav stack state is preserved here — do not +## push or pop screens in on_close. Use this hook for: pausing timers, stopping +## animations, unsubscribing from high-frequency feeds. The stack survives +## intact for the next on_open (if preserves_state=true). +## +## on_insert_deactivated() — called by SnapshotConsumers when the server drops +## insert state. The base implementation closes the app only if it is currently +## active in INSERT mode. FULLSCREEN apps inherit a no-op; override to add +## custom handling (e.g. save draft, emit warning). Do not call close_app() +## manually — call super() or replicate the guard condition. +## +## === Subclass _ready() pattern === ## func _ready() -> void: ## manifest = load("res://ui/implant/apps/my_app/app.tres") ## super._ready() diff --git a/docs/architecture/implant-app-pattern.md b/docs/architecture/implant-app-pattern.md index 661d29d0a..5ec3cce7a 100644 --- a/docs/architecture/implant-app-pattern.md +++ b/docs/architecture/implant-app-pattern.md @@ -184,6 +184,21 @@ func handle_intent(_action: String, _params: Dictionary) -> void: pass # future - `_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. +### Lifecycle hooks + +The four hooks fire in a defined order with a defined nav-stack guarantee at each point. Subclasses can rely on this contract without inspecting `HudGroups` state directly. + +| Hook | When it fires | Nav stack state when it fires | Intended use | +|------|--------------|-------------------------------|--------------| +| `on_install()` | Once, from `_ready()`, after nav is created | **Empty.** No screens have been pushed yet. | Construct screens; call `register_screen(id, screen)`; call `nav.set_default("...")`. Wiring only — do not read `nav.current()`. | +| `on_open(mode)` | Every activation (FULLSCREEN or INSERT) | **Non-empty.** Base ensures: `preserves_state=false` → `reset_to_default()` called; `preserves_state=true` and stack was empty → `push_default()` called; `preserves_state=true` and stack non-empty → untouched. | Data refresh; announce current screen; start animations. `nav.current()` is safe here. | +| `on_close()` | Every deactivation (GAMEPLAY or different app takes focus) | Stack preserved — base does not mutate it. | Pause timers; stop high-frequency feeds; save scroll position. Do **not** push or pop — the stack survives for the next `on_open`. | +| `on_insert_deactivated()` | SnapshotConsumers calls this when server drops insert state | Whatever `on_close()` left it (if the app was already closed) or the live state (if the app is still open) | Base default: close if active in INSERT mode, no-op otherwise. FULLSCREEN apps override. Do not call `close_app()` manually — call `super()` or replicate the guard. | + +**Why `on_install` sees an empty stack:** `_ready()` fires bottom-up — children before parents. The base's `_ready()` creates `nav` and then calls `on_install()` synchronously. No `HudGroups.app_changed` signal has fired yet (that comes from `open_app()`, which requires the game to be running). Subclasses that call `nav.current()` in `on_install` always see `""` — which is always wrong. The right pattern is to call `nav.set_default("my_first_screen")` in `on_install` and let the base push it on the first `on_open`. + +**Why `on_close` must not push/pop:** The nav stack is the in-flight navigation position that survives across close/reopen cycles (when `preserves_state=true`). Mutating it in `on_close` destroys the user's position. If an app needs to reset navigation on close, set `preserves_state=false` in the manifest instead — the base handles the reset at the top of `on_open`, which is the right moment. + ## `ImplantNavStack` — intra-app navigation Apps push and pop screens. The stack is owned by `ImplantApp` (one per app instance — no global nav state).