- test_character_creation_sprint28: before_each now seeds
_selected_bookmark_id and _selected_location_id so the new disabled-
guard in _on_start() (round 2) doesn't silently block 5 existing
tests that call _on_start()/KEY_ENTER without setting up a valid
bookmark selection. Restores the 2 tests Hoshe flagged as R2-H1 plus
3 siblings that would have degraded the same way under the guard.
- tests/run-godot: LOG_FILE now includes $$ (PID) so concurrent runs
across worktrees don't clobber each other's logs. Path is echoed
back via the stdout JSON "log" field and the stderr hint line, so
callers never need to predict it (R2-H2).
Addresses Tyre's 9 architecture items from the sprint-36 client review.
- decisions/architecture.md (Tyre #1): D-192 now says "deprecate; removal
tracked in #868" instead of "remove". The branch does not remove the
version field or guard — that belongs in the coordinated server+client
PR. The decision text now matches the code on this branch.
- meta_stack.gd (#2): handle_escape() on a screen with
closable_by_escape=false now consumes the event unconditionally. Was
returning whatever on_escape() returned, which default-returned false
and leaked ESC into main.gd's implant/settings chain — opening the
settings dialog behind the loading screen.
- debug_console.gd (#4): drop the direct KEY_ESCAPE branch in
_unhandled_input. ESC now falls through to main.gd → MetaStack, which
finds the console on top of the stack and closes it via the normal
path. Other keys are still consumed so movement/action can't leak.
- main.gd (#6, #10): extract the ESC priority chain into
_handle_menu_key() so "MetaStack → implant → settings" is a named
thing. Add a comment near connect_to_sim explaining that
GameState.bookmark_catalog survives the Option A scene transition via
the autoload.
- main_menu.gd (#7): header comment documenting the double LoadingScreen
lifecycle — safe today because main_menu.tscn and main.tscn never
co-exist, noted for future promotion to autoload if that changes.
- meta_screen.gd (#8): apply captures_input symmetrically in open()/
close() — was set in open() only, so a screen changing the flag
between open+close kept the opened value forever.
- meta_screen.gd (#9): on_escape() docstring clarifies the tri-state
(consume-and-hold / consume-and-close / ignore) — and that
closable_by_escape=false is the screen-wide way to say
"consume-and-hold".
- bug_report_dialog.gd (#11): capture_cancelled now emits from
on_close() (covers any close path — ESC, MetaStack pop, programmatic
close) rather than only on_escape(). A new _completed flag
distinguishes completion from cancel so the two signals stay
mutually exclusive.
Addresses Hoshe's 3 code-quality items from the sprint-36 client review.
- character_creation: drop CARDINAL_NAMES (was [south, east, north,
west]) and use CARDINAL_DIRS ([south, west, north, east]) for both
facing and screenshot filename label. The two arrays indexed by the
same _screenshot_cardinal_idx produced swapped labels at indices 1
and 3 — screenshots at those positions had filenames that did not
match the character's actual facing.
- character_creation: Enter/KP_ENTER now honors _footer_start.disabled.
Without a bookmark selected the Start button disables, but the
keyboard path called _on_start() unconditionally — a player could
confirm creation with empty bookmark/location strings. Guard at the
top of _on_start.
- protocol.gd: raw_bm.get("career", "tycoon") hardcoded a content
default in the wire decoder — a missing server field silently became
"tycoon". Empty string is the correct protocol default;
_make_bookmark_card already skips the career label when empty.
test_character_visual_sprint28: after_test() was freeing every node
returned by get_children(), including GdUnit4's own internal infrastructure
attached to the suite. That destroyed the runner mid-suite, hanging
make test-client indefinitely on the second compositor test. Now tracks
the nodes _make_compositor() spawned and frees only those. Suite goes
from "hangs forever" to 52/52 pass in 39s.
test_protocol_bridge, test_signal_sprint24: delete the
test_protocol_version_is_NN assertions. They asserted a constant equals
its own literal, failed mechanically on every protocol bump, and never
caught a real bug. Field-presence and roundtrip behavior is covered by
the surrounding tests; the runtime mismatch guard is exercised by
test_rejects_version_6. Surfaced D-192 (drop the version handshake
entirely) — see ticket #868.
- loading_screen: opaque BG (was 0.75 alpha) + mouse_filter STOP so the
loading state genuinely occludes the underlying screen
- main_menu: poll SimBridge.poll_snapshot in _process while waiting for
the bookmark catalog so the new-game flow doesn't stall on the
catalog round-trip introduced in Workstream 3 (#680)
W5 restructured character_creation's TabContainer to 4 top-level tabs
(Bookmark, Appearance, Skills, Debug) from the old 5-tab flat layout.
Three assertions in test_character_creation_sprint28.gd still referred
to the old shape; they didn't fail because the suite runs vacuously
in headless (the 3D SubViewport scene can't instantiate without a
render context), but the assertions were stale and would fire wrong
once the suite eventually runs non-headless.
Fixed:
- test_tab_container_has_five_tabs → renamed test_tab_container_
has_four_tabs, expected count 5 → 4.
- test_tab_names: expected ["Body","Head","Hair","Clothing","Debug"]
→ ["Bookmark","Appearance","Skills","Debug"]
- test_tab_navigation_wraps: current_tab = 4 (invalid on a 4-tab
container) → 3.
Header note added documenting the vacuous-headless behavior so the
suite reads correctly.
88/88 pass — unchanged — but the assertions are now correct for
non-headless invocation.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replaces the W5 placeholder inside the Skills tab with a properly
framed stub per Araminta's spec. Final workstream of the #618 + #680
+ MetaScreen implementation.
Content:
- MarginContainer (8 px sides, 4 px top — matches existing tab
padding)
- Single centered Label: "Skills allocation — coming soon."
- DIM_TEXT color, font_body size (11 px), horizontally and vertically
centered inside the tab content area
No inputs, no interactivity — real skill allocation lands in a future
sprint when the skills system exists server-side. Players selecting
a bookmark still proceed to Start regardless of what they see on this
tab.
Closes the implementation half of #618 (CK3-style character creation
screen) and #680 (location picker) — Hoshe's revised test plans for
MetaScreen pattern, #618, and #680 can now run end-to-end.
Verification:
- gdlint clean
- Headless parse + widened scanner check (per hardened pr-push skill)
clean; pre-existing autoload class_name noise filtered.
- test_character_creation_sprint28 88/88, test_protocol 62/62,
test_client_p3 24/24, test_ui_framework_sprint15 54/54
Next: Hoshe runs her full revised test gauntlet against the shipped
shape; if green, the PR pushes via /pr-push.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds the starting-location picker as a sub-component of the Bookmark
tab detail view, per Araminta's spec and D-128's "culture implicit in
location" constraint. Fills the space W6 reserved below the CAREER /
CAPITAL data rows.
Picker structure:
- "STARTING LOCATION" section label (DIM_TEXT, 10px, all-caps).
- ScrollContainer min_height=80 → VBoxContainer of selectable items.
- Each item: Button with child VBox carrying the location name Label
(PRIMARY_TEXT, 12px) and an optional culture tag Label (DIM_TEXT,
10px, mouse_filter IGNORE per D-128). Culture label is NOT rendered
when `allowed_locations_cultures[i]` is empty — no "Unknown"
placeholder, the row just shrinks.
Behavior:
- Clicking a bookmark card auto-selects its default_location in the
picker (handled via _selected_location_id + _update_location_selection).
- Clicking a location item updates _selected_location_id and re-gates
the Start button (already checked in W6).
- Switching bookmarks rebuilds the picker list for the new
allowed_locations; prior selection cleared.
- Parallel-array length mismatch defended: reads
`cultures[i] if i < cultures.size() else ""` so a short cultures
array won't crash rendering.
D-128 compliance:
- No culture dropdown or filter anywhere.
- Culture tag Label is strictly display: MOUSE_FILTER_IGNORE, no
signal handlers.
- CharacterProfile carries only start_location_id; no culture_id.
Other: removes W6's placeholder "Starting Location: X" ImplantDataRow
since the picker supersedes it; separator before the picker preserved.
Verification:
- gdlint clean
- Headless parse + scanner check (widened per hardened pr-push skill):
no new errors. Pre-existing autoload class_name noise filtered per
CLAUDE.md.
- test_character_creation_sprint28 88/88, test_protocol 62/62,
test_implant_* all green, test_client_p3 24/24,
test_ui_framework_sprint15 54/54.
Workstream 8 (Skills tab stub content) lands next — #618 closes then.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fills in the Bookmark tab stubbed in W5 with the full spec from
Araminta: card list + detail view + start-button gating. Also changes
the creation_confirmed signal to carry a CharacterProfile instead of
bare CharacterVisualDescriptor, consolidating bookmark + location
selection into one payload object.
Bookmark tab (left pane, 35%):
- ScrollContainer over VBoxContainer of card Buttons, one per entry in
GameState.bookmark_catalog. Each card: title (PRIMARY_TEXT,
font_header 15px) / subtitle (DIM_TEXT, font_small 10px, clipped) /
career badge (ACCENT_ACTIVE, all-caps). Selected state uses existing
ITEM_SELECTED_BG + ITEM_SELECTED_BORDER. custom_minimum_size
Vector2(180, 64).
Detail view (right pane, 65%):
- ImplantPanel composed via add_component:
- ImplantHeader (bookmark.title, bookmark.subtitle)
- ImplantSeparator
- ImplantTextBlock (flavor, autowrap, PRIMARY_TEXT)
- ImplantSeparator
- ImplantDataRow CAREER (accent_active) / CAPITAL (accent_positive,
format "%d Tractus") / STARTING LOCATION
- ImplantSeparator
- [location picker space reserved — W7 fills it]
Selection:
- Card click stores _selected_bookmark_id, auto-assigns
_selected_location_id from bookmark.default_location, rebuilds
detail view.
- Start button (footer) gated on both _selected_bookmark_id and
_selected_location_id non-empty.
- Randomize while Bookmark tab is active picks a random bookmark +
one of its allowed_locations and skips appearance randomization.
Signal contract change:
- creation_confirmed(profile: CharacterProfile) replaces
creation_confirmed(descriptor: CharacterVisualDescriptor).
- CharacterProfile now extends RefCounted (was Resource) with
non-exported fields — it's a one-shot signal payload, never
persisted. This also sidesteps the scanner error that the prior
@export var descriptor: CharacterVisualDescriptor on a Resource
caused (RefCounted types cannot be @export-ed).
- _on_start emits a CharacterProfile built from _descriptor +
_selected_bookmark_id + _selected_location_id, then sends
ConfirmBookmark via SimBridge.send_named_action before scene
transition.
Test updates:
- test_character_creation_sprint28.gd signal receivers switched to
untyped to accept CharacterProfile without hitting class_name
parse-order at test-suite scan time. 88/88 pass.
Verification:
- gdlint clean
- godot --headless --path client --quit — no SCRIPT ERROR (prior
character_profile.gd scanner noise now gone after the RefCounted
conversion)
- test_character_creation_sprint28 88/88, test_protocol 62/62,
test_implant_nav_stack 52/52
Workstream 7 (location picker as sub-component of the Bookmark detail
view) follows. W8 fills the Skills tab.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Clean the regression signal for the remaining MetaScreen workstreams
by either fixing or surgically skipping tests that had been failing
for design reasons or against stale APIs.
test_sprint2_proof.gd — all 3 tests prefixed skip_test_. Root cause:
hardcoded Sprint 2 room coordinates + protocol v1 assumptions; not
adaptable to current protocol v23 or Gauntlet layout. Suite now reports
0 tests rather than 14 failures / 3 errors.
test_dialogue_sprint18.gd — 40 tests pass (was 48 errors / 3 failures).
Root cause of the errors: GameState.has() calls hitting Node.has()
which does not exist. Fixed by removing guards and accessing
GameState.current_examine_result directly (present since v14 / #174).
Two real bugs surfaced after the error noise cleared; skipped with
ticket references:
- #866 (high): dialogue_box._escape_bbcode chains .replace('[','[lb]')
.replace(']','[rb]') which turns [lb] into [lb[rb]. BBCode injection
guard broken.
- #867: confrontation_monologue signal doesn't fire in headless; the
create_tween call in _start_confrontation_beat likely aborts before
the emit.
test_client_p2.gd — 26 tests pass (was 2 failures). Three #117-fallout
camera-smoothing tests skipped (main.gd disables
position_smoothing_enabled permanently by design since #117 manual
lerp). One MonologueDisplay API test skipped pending #864 (asserts
mono.is_visible, but the display was refactored to _visible:
Array[Dictionary]).
No production code changes. Every skipped test carries a skip_test_
prefix + inline TODO pointing at the owning ticket. Bug tickets #864,
#866, #867 filed to the backlog.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Consolidates the character creation TabContainer from 8 flat tabs
(Body / Head / Hair / Clothing / Accessories / Debug plus the two
being-added Skills / Bookmark) into 4 top-level tabs per Araminta's
revised spec: Bookmark, Appearance, Skills, Debug.
The existing five appearance sub-tabs (Body, Head, Hair, Clothing,
Accessories) now live inside the Appearance tab as a horizontal
segmented sub-navigation using the existing `_make_slot_btn()` pattern
— consistent with the Clothing/Accessories slot row vocabulary.
Selected sub-section uses existing ITEM_SELECTED_BG / BORDER styling.
Structural changes:
- New APPEARANCE_SUB_NAMES const lists the five sub-sections.
- Renamed _tab_search → _appearance_search, _tab_grids →
_appearance_grids. Scope changed from "top-level tabs" to
"Appearance sub-sections" but index 0..4 semantics preserved.
- Added _appearance_active_idx, _appearance_sub_btns,
_appearance_sub_sections state.
- _ready() builds exactly 4 top-level tabs; tab builders invoked
explicitly per index.
- New _build_bookmark_tab / _build_skills_tab render TEXT_DIM
placeholder labels ("Bookmark content lands in Workstream 6", etc.)
— actual content in W6/W8.
- _build_appearance_tab constructs the sub-nav strip and stacks all
5 sub-sections up front with visibility-toggle swap
(_on_appearance_sub_selected). Comment explains the up-front build
choice and the free-and-rebuild fallback if performance regresses.
- Existing _build_body_tab / _head / _hair / _clothing / _accessories /
_debug remain unchanged — they now receive Appearance sub-section
Controls as their tab argument instead of top-level tabs. _make_tab_vbox
anchors full-rect in both parent contexts, so layout is preserved.
Verification:
- gdlint clean
- godot --headless --path client --quit — no SCRIPT ERROR
- test_protocol 62/62, test_client_p3 24/24, test_ui_framework_sprint15
54/54, test_implant_nav_stack 52/52, test_implant_registry 42/42,
test_implant_app_lifecycle 36/36
Workstream 6 (Bookmark tab content: card list + detail view + location
picker per Araminta's spec) lands next. W7 (location picker as a
sub-component of Bookmark tab) follows. W8 (Skills stub content) is
last. Hoshe's parallel Task #21 (test hygiene triage) commits
separately.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
main_menu now connects SimBridge before character_creation opens, gating
the transition on first ObserverSnapshot carrying a bookmark_catalog.
Loading screen is shown during the connect; on cancel the SimBridge
subprocess is torn down and the player returns to main_menu. Catalog is
read straight from GameState.bookmark_catalog in W6.
Flow (Option A):
1. New Game → SessionManager.new_game() creates save dir
2. main_menu pushes loading_screen via MetaStack with "Connecting to
simulation..." message
3. SimBridge.connect_to_sim() spawned; main_menu listens on
connection_state_changed, then on snapshot_received for the catalog
4. On catalog arrival: loading_screen closed, scene-transition to
character_creation
5. character_creation Cancel → SimBridge.disconnect_from_sim() + scene
transition back to main_menu (Tyre's recommendation: clean state per
session over warm-start savings)
6. character_creation Start → ConfirmBookmark sent (stubbed for W4 with
first catalog entry; real bookmark + location from W6's UI)
ESC priority chain in main.gd OPEN_MENU handler:
- MetaStack.handle_escape() first — closes the topmost meta overlay
- HudGroups.is_implant_active() / close_app() — closes active implant
- Fallback: toggle settings_dialog (existing W2 behavior)
Files:
- sim_bridge.gd: send_named_action(action_name, action_data) helper.
Bridges named tag-enum PlayerActions (RequestBookmarkCatalog,
ConfirmBookmark) into the existing outbound buffer, parallel to
send_input's InputMapper.Action handling.
- loading_screen.gd: set_message(text) for the connecting/loading label.
- main_menu.gd: full Option A flow rewrite. Tracks _waiting_for_catalog
so re-clicking New Game during connect is a no-op.
- character_creation.gd: _on_back disconnect path + _on_start
ConfirmBookmark stub. MAIN_MENU_SCENE / GAME_SCENE constants.
- main.gd: connect_to_sim guard (don't reconnect when Option A leaves
it CONNECTED). ESC chain wiring.
Verification:
- gdlint clean
- godot --headless --path client --quit — no SCRIPT ERROR
- test_protocol 62/62, test_implant_nav_stack 52/52, test_client_p3
24/24, test_ui_framework_sprint15 54/54
Pre-existing failing suites unchanged: test_sprint2_proof,
test_dialogue_sprint18, test_client_p2 (camera-smoothing assertions
that pre-date W4 — main.gd has disabled position_smoothing_enabled
since #117 / #501 / #117 manual-lerp; tests were stale).
Workstream 5 (3-tab restructure of character_creation: Bookmark /
Appearance with sub-nav / Skills / Debug) lands next.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds client-side wire support for the bookmark catalog (#614) and the
two associated player actions. PROTOCOL_VERSION bumps from 21 to 23:
- v22 (server): RequestBookmarkCatalog + ConfirmBookmark player actions
- v23 (server): bookmark_catalog field on ObserverSnapshot
Decode:
- protocol.gd decode_snapshot extracts optional bookmark_catalog.
Defensive parse of BookmarkWire fields (id, title, subtitle, flavor,
default_location, allowed_locations, allowed_locations_cultures,
career, starting_capital_tractus). Missing or malformed → null.
- snapshot_handler.gd caches the catalog into GameState.bookmark_catalog
on each snapshot (server pushes on tick 0; re-fetchable via
RequestBookmarkCatalog).
- GameState gains bookmark_catalog: Array = [] (untyped per autoload
parse-order discipline; default empty so callers can iterate without
null checks).
Encode:
- encode_request_bookmark_catalog() — unit variant, sent to trigger a
re-push if the cached catalog is missing.
- encode_confirm_bookmark(bookmark_id, starting_location_id) — struct
variant matching server rmp_serde shape. Called from character
creation on Start (lands in Workstream 6).
Tests:
- 5 new cases in test_protocol.gd: hand-built bookmark_catalog decode
(all 9 fields asserted), fixture-based decode round-trip, missing-
field null behavior, RequestBookmarkCatalog encode roundtrip,
ConfirmBookmark encode roundtrip.
- All 12 existing snapshot fixtures regenerated from server via
`cargo test --test gen_fixtures -- --ignored`. The new
snapshot_with_bookmark_catalog.msgpack fixture was generated by the
same pass.
Verification:
- gdlint clean
- godot --headless --path client --quit — no SCRIPT ERROR
- test_protocol 62/62, test_client_p3 24/24, test_implant_nav_stack
52/52, test_implant_registry 42/42, test_implant_app_lifecycle 36/36
Workstream 4 (Option A sequencing via loading_screen + SimBridge
connect) lands next.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Relocates main_menu, character_creation, settings_dialog, debug_console,
bug_report_dialog, loading_screen from flat client/ui/ into structured
client/ui/meta/screens/<name>/. All six now extend MetaScreen instead
of Control; the base handles open/close lifecycle, visibility,
captures_input, and — for overlays — the sim-pause contract.
Screen policies set per Tyre's proposal:
- settings_dialog: pauses_sim=false, PUSHES onto MetaStack
- debug_console: pauses_sim=true, PUSHES (D-088 routing via base)
- bug_report_dialog: pauses_sim=true, PUSHES
- loading_screen: closable_by_escape=false, PUSHES
- main_menu, character_creation: scene-roots, extend MetaScreen for
the lifecycle contract only, do NOT push onto the stack
character_creation stays at its current surface (tabs, descriptor,
creation_confirmed signal unchanged). Tab consolidation and
CharacterProfile migration happen in Workstreams 5 and 6.
Knock-on changes:
- main.tscn ModalLayer CanvasLayer renamed to MetaLayer; main.gd
@onready refs updated; constants.gd comment updated; test_client_p3
and test_ui_framework_sprint15 assertions updated; test_monologue_display
and .tscn header comments updated.
- OPEN_MENU handler now pushes settings_dialog onto MetaStack before
calling open(). Full ESC priority chain lands in Workstream 4.
- atlas_app.gd: _unhandled_key_input signature widened from
InputEventKey to InputEvent with an is-check, per Godot 4 API. Pre-
existing narrowing was silently tolerated until main.tscn started
fully instantiating under the new pattern.
- test_client_p3: entity_renderer type annotations corrected from
ColorRect to Sprite2D (stale since a prior refactor); facing
indicator rotation assertion switched to angle_difference() for
modular-safe comparison.
Verification:
- gdlint client/scripts/ client/ui/ — zero problems
- godot --headless --path client --quit — no SCRIPT ERROR
- test_client_p3: 24/24 pass
- test_ui_framework_sprint15: 54/54 pass
- test_implant_nav_stack: 52/52 pass
- test_implant_registry: 42/42 pass
- test_implant_app_lifecycle: 36/36 pass
Workstream 1 foundation (84105916) remains unchanged. Workstreams 3-8
follow: protocol layer, Option A sequencing, 3-tab restructure,
Bookmark tab, location picker, Skills stub.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Establishes the faux-game-menu base pattern for non-diegetic UI,
analogous to ImplantApp but for pre-gameplay and meta-overlay
screens (main menu, character creation, settings, debug console,
bug report, loading screen). Workstream 1 of the MetaScreen
refactor — foundation only, no screen migrations yet.
- client/ui/meta/meta_screen.gd: base class (Control) with
HIDDEN/OPENING/OPEN/CLOSING phase tracking, three orthogonal
policy booleans (pauses_sim, closable_by_escape, captures_input),
open/close lifecycle, on_escape contract, closed + escape_pressed
signals, subclass hooks (on_open, on_close).
- client/ui/meta/meta_stack.gd: autoload coordinator. Overlay stack
with push/pop/top/is_active; handle_escape chain; sim-pause
coordination via SimBridge when pauses_sim=true; meta_active_changed
signal. All class references kept inside method bodies — no
top-level class_name refs, matching HudGroups / GameState
autoload parse-order discipline.
- client/scripts/character_profile.gd: Resource wrapping the
visual descriptor with bookmark_id and start_location_id.
Target of the creation_confirmed signal once the character
creation flow migrates.
- client/project.godot: MetaStack registered as autoload after
HudGroups, before ImplantRegistry.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Three test suites covering ImplantNavStack, ImplantRegistry, and
ImplantApp lifecycle:
- test_implant_nav_stack.gd: 26 tests — push/pop/replace/reset,
push_default, is_empty, current/current_payload, signal emission,
re-entrancy guard, stack-floor-to-default-on-last-pop.
- test_implant_registry.gd: 21 tests — _is_valid_manifest validation,
lazy scan flag, get_manifests / get_resolved_mode both trigger scan,
cache on second call, real scan finds atlas and economics, no
duplicate keys, _MODE_MAP coverage, get_app_instance null before
instantiate_all, CURRENT_SCHEMA_VERSION = 1.
- test_implant_app_lifecycle.gd: 18 tests — nav created in _ready,
starts hidden, open FULLSCREEN/INSERT makes visible, GAMEPLAY/wrong
path doesn't open, nav non-empty on open, close hides, app-switching
closes active, preserves_state true/false, on_insert_deactivated
gated on INSERT (closes) vs FULLSCREEN (no-op), register_screen
adds hidden child, duplicate id does not overwrite (first-wins).
Two team-lead fix-ups before commit (Stig caught the class_name
parse-order issue but used the wrong gdUnit4 hook names):
- before_each/after_each renamed to before_test/after_test per
gdUnit4 API. test_game_state.gd's use of before_each appears to
work by coincidence (that test resets autoload state rather than
constructing objects, so the never-called hook didn't matter);
tests that rely on hook-driven setup need the correct names.
- test_register_screen_duplicate_id_does_not_overwrite rewritten to
assert the actual contract (first-wins on _screens dict +
duplicate screen is not reparented) instead of Control.visible
default, which defaults to true regardless of registration.
Also removed a stray client/ui/implant/apps/collision_test/app.tres
fixture left over from Hoshe's earlier manual collision-warning
verification. It was untracked and would have blocked atlas from
registering at runtime (KEY_M collision, collision_test won the scan
order). Not committing it.
All three suites exit 0, totals 26/26, 42/42, 36/36 passed.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Seven nit-level fixes from PR review:
- Remove dead signal insert_deactivated from ImplantApp; the hook method
on_insert_deactivated() is the actual contract.
- Rewrite _on_atlas_economics_link comment in main.gd to reflect the
actual flow (AtlasApp closes as a consequence of HudGroups single-
active-app, not before emitting anything).
- Document the "pop never empties below default" invariant on
ImplantNavStack.pop() with a pointer to reset_to_default.
- Add _mutating re-entrancy guard on ImplantNavStack mutation methods.
push_error + early return if called during a screen_changed emission.
- main.gd registry loop now uses typed ImplantAppManifest property
access (manifest.app_path, manifest.default_key) instead of
dictionary-style .get() calls. Empty app_path triggers push_warning.
- Document the economics [/] hotkey exception in main.gd and reference
the planned handle_global_key lifecycle hook. Arch doc Follow-up
section gains a bullet for the new hook.
- Comment the independent-version-read rationale above client_ver and
proto_ver in loading_screen.gd.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Addresses blocker comments from PR review:
- Delete star_map.gd and star_map.tscn — dead implant/map/starchart
HudGroups registration that should have landed with the atlas
unification (D-191 criterion 1)
- Remove test_star_map_is_accessible_from_insert_ui and
test_star_map_scene_exists from test_sprint30.gd — D-191 supersedes
the insert-UI accessibility pattern
- ImplantRegistry._scan() now detects default_key collisions
(first-wins with push_warning) and validates default_mode against
HudGroups.Mode enum (skip + warn on invalid)
star_map_data.json remains — still used by atlas_app and
economics_app for system index lookups.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Loading screen reads the client version from project.yaml (root version
field) and displays it alongside Protocol.PROTOCOL_VERSION at the bottom
of the overlay: "v0.1.35 · protocol 21".
Falls back to "?.?.?" if project.yaml is missing or unreadable (e.g.
when run from an exported pck where the relative path is unavailable).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Per D-191, the atlas is the star map extended downward — not a separate
app. AtlasPanel now owns the full Reach → system → planet → heightmap
zoom hierarchy as a single HudGroups app (implant/map).
- Add REACH_MAP as Level 0 of AtlasPanel's zoom hierarchy; renumber the
enum so higher index = deeper zoom
- Port hop-ring rendering (pan/zoom, system markers, hover/info, sector
layout) from star_map.gd into AtlasPanel methods
- Change AtlasPanel.APP_PATH from "implant/map/atlas" to "implant/map"
- main.gd: KEY_M toggles unified atlas; KEY_A binding removed
- Symmetric nav: ORBITAL_DIAGRAM back goes to REACH_MAP (not
SYSTEM_PICKER), matching the forward skip
- hud_groups.gd docstring documents the unified path and flags the
legacy starchart path as kept-for-compat (retirement tracked in #852)
star_map.gd's HudGroups registration stays live but inert — no key
binding reaches it. Full retirement follows in #852 after a sprint of
soak on the unified panel.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
10. Dropped the "no body data" fallback string in the picker panel — the
generator always writes entry["bodies"], so the fallback was dead. Use
the "—" convention the rest of the panel already follows.
12. Removed the autoload parse-order comment from AtlasPanel (it is
scene-instanced, not an autoload, so the rule does not apply), and
collapsed _build_heightmap_viewer to a direct AtlasViewer.new() —
mirroring the rest of the file rather than dancing around a risk that
is not real for this class.
13. AtlasMarkerOverlay._draw_cities now compares hover/selection by a
stable _city_key() (name → city_id → pos → hash) instead of
Dictionary.==, which was O(fields) per city per redraw. Preps the
renderer for much larger city counts without a rewrite.
14. Orbital click/draw handlers guard against missing body_id /
station_id by reading through str(dict.get(..., "")) and skipping
empty ids. Matches the defensive style already used for parent_body_id
and keeps a NULL id from crashing _draw_bodies / _handle_orbital_click.
4. AtlasPanel and AtlasViewer now compose their title + hint from an
ImplantHeader child rather than hand-rolling them via draw_string, so the
D-169 "theme swap changes the implant hardware appearance" invariant
holds end-to-end. _refresh_screen_header() drives content per level and
on system navigation.
5. AtlasViewer exposes city_canvas_pos(), get_hovered_city(),
get_selected_city(), and get_overlay_defs() as public API — the marker
overlay no longer reaches into underscore-prefixed state, which is
especially important because viewer is an untyped var in the overlay.
6. KEY_N now consumes unconditionally while the viewer is visible, and
main.gd's global economics-monitor toggle is gated on
!HudGroups.is_app_active("implant/map/atlas"). Previously pressing N
without a selected city fell through and closed the fullscreen atlas as
a side effect.
7. OVERLAY_DEFS lives in AtlasViewer as the single source of truth.
AtlasOverlayBar reads the list via viewer.get_overlay_defs(), and
AtlasViewer derives _overlay_visibility / _overlay_locked from the same
table at _ready() — no more hand-maintained parallel lists, so the bar
and the guard in set_overlay_visible can't drift.
8. AtlasOverlayBar drops `class_name`: it now loads via
load("res://ui/implant/atlas_overlay_bar.gd") from AtlasViewer, the same
pattern AtlasPanel uses for AtlasViewer. _init(viewer_ref = null) keeps
the required-arg footgun off the editor's introspection path.
9. `star-map-data` make target added to regenerate
client/data/star_map_data.json from systems.db + wiki, and
`check-star-map` wired into pre-pr-validate + pre-pr-client so any
commit that touches the generator (or any downstream systems.db change
like server #839) fails pre-pr until the JSON is regenerated. The
terrain_reference data-availability dependency is no longer tribal
knowledge.
Also addresses review #15 (push_warning on unknown overlay id in
set_overlay_visible) and #16 (disabled always-on buttons drop handler
churn) as part of the same refactor.
1. Moon placement on the orbital diagram divided by a hard-coded 4 — a gas
giant with five or more moons would overlap satellites and make them
unclickable. Count moons per parent and distribute them evenly.
2. AtlasViewer._load_markers seeded _grid_w/_grid_h from _tex_w/_tex_h
before the terrain_reference null check, so opening a body with no
heightmap after one that had a heightmap left the grid dimensions
pointing at the previous texture and misplaced markers. Reset texture +
grid dims to sentinel defaults at the top of _load_heightmap.
3. AtlasMarkerOverlay's three toggleable overlays (production_zones,
shadow_economy, corp_presence) read markers.production_zones /
markers.shadow_zones / markers.corp_presence — keys that don't exist in
D-191 §8's markers schema, so toggling was a silent no-op. Derive them
from cities[] instead: primary_function ∈ PRODUCTION_FUNCTIONS for
production, absent Commission presence for shadow bands, Commission
presence for corp dots. Follow-up ticket will formalise per-overlay
arrays once the server schema lands.
Addresses PR #128 review blockers 1-3.
AtlasOverlayBar is an HBoxContainer docked top-right of the heightmap viewer
with 11 short-label buttons mapping to D-191 §7 / D-181 signal visibility:
always-on (5) TER INF NAM GAT POL — terrain, infrastructure, named
features, gate markers, political zones. Pinned on; clicks
are swallowed so the layers can't accidentally be disabled.
toggleable (4) POP PRD SHD CRP — population density, production zones,
shadow economy, corporate presence. Reflect and mutate
viewer overlay state.
locked (2) STK BSL — stockpile_weeks, production_vs_baseline. Disabled
and greyed out with unlock-requirement tooltips, per D-181
semi-private/private tiers; kept in the bar so players see
that deeper data exists and is gated.
Each button writes through AtlasViewer.set_overlay_visible(), which is the
single entry point into the viewer's _overlay_visibility dict consumed by
AtlasMarkerOverlay._draw(). Locked overlays short-circuit in that setter.
Per D-191 criterion 6.
Adds Level.HEIGHTMAP_VIEWER to AtlasPanel. The viewer loads a body's
terrain_reference heightmap PNG, pairs it with markers.json (roads, rail, POIs,
cities, rivers/oceans/mountains), and renders markers in texture-space via an
AtlasMarkerOverlay Node2D child of a transformed canvas — pan = offset,
zoom = scale.
Pan/zoom is cursor-centred (wheel zooms under the mouse, drag pans), with a
fit-to-view reset on R. Empty markers.json state renders a bare heightmap;
missing terrain_reference shows a themed "terrain data pending (#839)" notice
instead of crashing.
City data sidebar rebuilds from the selected city: name, pop tier, function,
currency zone, Commission presence, shadow zone, gate distance. Pressing N on
a selected city emits economics_link_requested(system_id) — main.gd bridges
this to EconomicsPanel.select_system() + HudGroups.open_app("implant/economics")
as an insert overlay, satisfying the D-191 Phase 2/3 cross-panel integration.
The overlay renders all nine D-191 overlay layers off of per-overlay visibility
flags in AtlasViewer. Overlay toggling for the regional view (#836) plugs into
set_overlay_visible(); the five always-on layers (terrain, infrastructure,
named features, gate markers, political zones) draw by default, the four
toggleable layers draw from placeholder data, and the two locked layers
(stockpile_weeks, production_vs_baseline) remain off until unlocked.
Per D-191 criteria 1, 4, 5.
3-level atlas navigation as an implant/map/atlas FULLSCREEN app (D-170 z=20):
system picker (◄ ► cycle, Enter opens orbital) → orbital diagram (star centre,
bodies grouped by orbit_index with moons sub-orbiting parents, stations as
markers, click-to-open body or mini station panel) → body entry (data sheet,
Enter stub for #835 heightmap viewer, Esc back). Composed from the ImplantPanel
component library (D-169).
Wires the panel into hud.tscn, adds an A-key toggle in main.gd, propagates
insert_state via snapshot_consumers. Extends generate-star-map-data.py to emit
per-system orbit_bodies + stations arrays (with currency_zone, atmosphere,
population, terrain_reference, etc.) from systems.db; star_map_data.json
regenerated deterministically.
Per D-191 §6, Phase 3.
CursorRenderer now toggles Input.MOUSE_MODE_VISIBLE when gameplay is
occluded, restores MOUSE_MODE_HIDDEN when gameplay resumes. Without
this, fullscreen apps (star map, future atlas) had no cursor at all —
the custom diegetic cursor hid correctly but the system cursor was
never restored.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
was_occluded was computed AFTER _active_mode and _active_app were
updated to the new values, so it always matched now_occluded on the
first toggle (both TRUE). The signal condition (was != now) never
triggered. Moved the check before the state mutation.
This bug affected every GameplayRenderer (world, entities, fog,
cursor) and the stance indicator — none of them hid on first
fullscreen app open.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- world_radial.gd: use set_deferred("size", ...) to avoid anchor conflict warning
- storyteller: remove noisy "no Simmering triangles" warn (normal state, not exceptional)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Bump client protocol version 20 → 21 to match server (#822)
- Fix render_priority parameter name (was _render_priority, unused prefix)
- Fix debug console type inference (var sub := → var sub: String =)
- Economics panel: add population row, improve key hint text
- Stance indicator: hide on gameplay_occluded (D-170 fullscreen apps)
- Remove 5 broken clothing items from manifest and delete their GLBs
(boots_work, coveralls_basic, jacket_utility, pants_cargo, shirt_henley)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
E is claimed by InputMap "interact" action — InputMapper consumes
it before _unhandled_key_input. N is free, adjacent to M (star map),
reads as "Numbers" for the economics monitor.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Economics panel: replace dead _gui_input LEFT/RIGHT with public
navigate() method, wire [ ] keys in main.gd (avoids movement
key conflict, fixes focus_mode=NONE issue)
- Debug console: add explicit effect guard in econ inject no-commodity
branch so invalid effects don't fall to commodity-form error
- Snapshot consumer: null-clear GameState.economy_snapshot after
consuming (matches one-shot consumer invariant)
- Star map: remove duplicate doc comment above set_insert_active()
- Generate script: remove stale comment, dead _WORKTREE_PARENT var,
dead field extraction in parse_wiki_index, add try/except around
DB queries
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Three new econ subcommands in the debug console: inject (supply
shocks/boosts), param (α/β/friction mutation), inspect (all 7
D-181 signals). Command parsing and validation complete; dispatch
wired through existing DebugCommand IPC flow. Server handler
ships with #823.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
New implant panel at implant/economics: system selector, 6-commodity
price table with trend indicators, GDP strip. Composed from D-169
component library. Ring buffer caches last 20 ticks per system.
Snapshot routing wired through snapshot_handler → GameState →
snapshot_consumers → economics_panel. Placeholder prices shown
until server ships EconomySnapshot (#822).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Star map popup now shows POPULATION and GDP rows when data is present.
Generation script updated to compute GDP from population × tier-based
per-capita schedule. 275/301 systems have GDP data (26 uninhabited
correctly omitted).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Startup crash — preload loop referenced STANCE_ICONS but the constant
was accidentally dropped when rewriting the icon cache logic.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Preload stance icons at _ready() into cache Dictionary
- Null-guard shader + theme loads with push_error() + early return
- Guard _icon_mat.set_shader_parameter inside if _icon_mat:
- Icon size 18→20px per D-086, container height 44→46px to fit
- Add _apply_stance test coverage (known + unknown stance)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace text-only stance labels with SVG icons + tint shader per D-086.
Alpha-mask shader swaps color at runtime via ShaderMaterial parameter.
Icons: walk, crouch, careful, sprint at 18px inside ImplantPanel row.
Also fix stale UILayer/HUD test path in test_ui_framework_sprint15.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
PO-built prototype (FBM terrain, Whittaker biomes, procedural globe)
with test body definitions for all planet types. Replaces pyplatec
approach. Spike validates the pipeline architecture for batch #817.
Includes handover doc, 9 test body definitions, and updated spike
pipeline documentation. Stale pyplatec outputs removed.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
