Merge remote-tracking branch 'origin/sprint-37/server'
This commit is contained in:
@@ -213,15 +213,12 @@ Note: `ConfirmBookmark` is the **trigger** for transitioning from the character-
|
||||
|
||||
```rust
|
||||
/// The confirmed bookmark selection for the current session.
|
||||
/// Populated when `ConfirmBookmark` is processed. `None` during the
|
||||
/// character-creation phase (before confirm) and always `None` in a
|
||||
/// fresh session.
|
||||
/// Populated when `ConfirmBookmark` is processed. `None` fields during the
|
||||
/// character-creation phase (before confirm) and in a fresh session.
|
||||
///
|
||||
/// **v0.2 scope: transient only.** Not serialized — save/load of
|
||||
/// `SelectedBookmark` is deferred to Sprint 37 (follow-up ticket
|
||||
/// filed alongside #614). Add `Serialize`/`Deserialize` derives and
|
||||
/// wire into `SaveState` when that ticket is claimed.
|
||||
#[derive(Resource, Debug, Clone, Default)]
|
||||
/// Serialized into `SaveStateV1.selected_bookmark` (#863) so that a loaded
|
||||
/// game remembers which bookmark and starting location were chosen.
|
||||
#[derive(Resource, Debug, Clone, Default, Serialize, Deserialize)]
|
||||
pub struct SelectedBookmark {
|
||||
pub bookmark_id: Option<String>,
|
||||
pub starting_location_id: Option<String>,
|
||||
@@ -231,15 +228,11 @@ pub struct SelectedBookmark {
|
||||
Downstream systems (apartment generator, skill seeder) read from this
|
||||
resource.
|
||||
|
||||
**Save/load scope (v0.2 deferred):** `SelectedBookmark` is transient for
|
||||
v0.2 — it lives in-memory from `ConfirmBookmark` through session end and
|
||||
is not persisted. A reload after quit returns the player to the
|
||||
character-creation screen. Promotion to persistent state (adding
|
||||
`Serialize`/`Deserialize` and threading into `SaveState` / #553) is
|
||||
tracked in a follow-up ticket for Sprint 37. `SelectedBookmark` must
|
||||
carry an inline `// TODO(sprint-37): serialize — see #<follow-up ticket>`
|
||||
comment in `server/src/bookmark/mod.rs` pointing at the follow-up so the
|
||||
omission is greppable.
|
||||
**Save/load scope (Sprint 37, #863):** `SelectedBookmark` is persisted into
|
||||
`SaveStateV1.selected_bookmark`. After `load_from_file` completes, the resource
|
||||
reflects the bookmark confirmed at session-start. Saves created before Sprint 37
|
||||
will deserialize the field as `SelectedBookmark::default()` (both fields `None`)
|
||||
via `#[serde(default)]` on the `SaveStateV1` field.
|
||||
|
||||
## 5. Content source — how bookmarks get into the registry
|
||||
|
||||
|
||||
@@ -0,0 +1,211 @@
|
||||
# Sprint 37 #878 — CharacterArchetype audit (D-032 obsolete claim)
|
||||
|
||||
**Author:** Tyre (architecture)
|
||||
**Date:** 2026-04-21 (audit); 2026-04-22 (lead override amendment)
|
||||
**Ticket:** #878 — Audit and purge dead smuggler/detective character enum (D-032 obsolete)
|
||||
**Status:** CLOSED — lead override 2026-04-21: strip stays. See override section below.
|
||||
|
||||
---
|
||||
|
||||
## Lead override (2026-04-21)
|
||||
|
||||
**Decision:** STRIP the full `CharacterArchetype` trace from the server. The
|
||||
original audit (below) recommended Option A (no-op, docs-only) on the basis
|
||||
that grep identified five load-bearing consumers. The lead reframed the
|
||||
live-vs-filler determination:
|
||||
|
||||
> "It is live because we have not scrapped the system loading it in the
|
||||
> client. This is not live gameplay. Only the character creation elements
|
||||
> and the insert screens are actual production code. The rest is uncleaned
|
||||
> filler — per the cascade, we don't deal with character and NPC
|
||||
> instructions." — Jeroen, 2026-04-21
|
||||
|
||||
**Rationale — the cascade framing.** Per `CLAUDE.md`, development follows a
|
||||
strict six-phase cascade. Current focus is Phase 1 (wiki content). Character
|
||||
model, NPC differentiation, verb-label relabeling, and character-keyed
|
||||
monologue pools all belong to Phase 6 (detail coloring). Code that executes
|
||||
at runtime is **not** automatically production — if the topic belongs to a
|
||||
later cascade phase, it is pre-cascade filler regardless of how deeply it is
|
||||
wired in.
|
||||
|
||||
The audit's identification of `archetype_verb_label` as "live feature code"
|
||||
was mechanically correct: the code runs, produces output, and is observed on
|
||||
the wire. But the **feature itself is premature**. Container verb
|
||||
differentiation (Smuggler Move/Stash vs Detective Scan/Flag) and
|
||||
archetype-keyed monologue pools are Phase 6 detail, not Phase 1–3
|
||||
scaffolding. Uniform verb labels and a single monologue pool are the
|
||||
intended end state until a Phase 6 character system is designed — not a
|
||||
regression.
|
||||
|
||||
**Audit correction.** The audit's TL;DR conclusion ("v0.2 dropped reverses
|
||||
D-117 which reverses D-032-obsolete, therefore keep the enum") was the
|
||||
wrong frame. The supersession chain collapsed — yes — but the correct
|
||||
reading is that **all three decisions sit below the current cascade
|
||||
floor**, so the enum is cruft on cascade grounds independent of the D-117
|
||||
revocation. The audit should have consulted the cascade phase before
|
||||
grep-counting consumers; that is the non-obvious precedent captured in
|
||||
memory (`feedback_running_code_not_production.md`).
|
||||
|
||||
**Scope of the strip (Sprint 37, #878, merged via PR #137). Commit `cae3d3ab`
|
||||
— "refactor(simulation): strip archetype trace + HeritageRoot per cascade
|
||||
(#877, #878)" — is the authoritative file list; the summary below is the
|
||||
high-signal view:**
|
||||
|
||||
- `server/src/bridge/types.rs` — `CharacterArchetype` enum +
|
||||
`StartupMessage.character_archetype` field removed. Protocol break rides
|
||||
the `PROTOCOL_VERSION` drop in #874 (D-192), co-shipped in the same PR.
|
||||
- `server/src/perception/observer/mod.rs` — `apply_phase2_verb_filter`
|
||||
loses its archetype parameter; `archetype_verb_label` helper + the
|
||||
container-relabeling block deleted.
|
||||
- `server/src/simulation/monologue.rs` — `MonologueState.character`
|
||||
field + `Default` value removed. Pool selection is now uniform.
|
||||
- `server/src/simulation/examine.rs` — `generate_examine_text` collapses
|
||||
two archetype-specific branches into a single detective-style frame.
|
||||
- `server/src/test_world/mod.rs` + `server/src/main.rs` — `setup_gauntlet`
|
||||
and `setup_proof_room` drop the archetype parameter; all internal
|
||||
callsites + the production main loop updated.
|
||||
- `server/tests/archetype_monologue.rs` — deleted (regression guard for
|
||||
#587's archetype→monologue wiring; wiring itself deleted).
|
||||
- `server/tests/v01_integration_playthrough.rs` — deleted (5
|
||||
archetype-using integration tests; superseded by Gauntlet coverage).
|
||||
- `server/content/schemas/drama_module.schema.yaml` — deleted.
|
||||
- `server/content/modules/tier1/smuggling_ring_v0_1.yaml` — deleted.
|
||||
|
||||
**Client follow-up: ticket #882** — *"Strip archetype-driven client code
|
||||
(follow-up to #878 server)"*. The server strip leaves client-side code
|
||||
referencing the removed `character_archetype` wire field and archetype-
|
||||
keyed palette/monologue branches. Client cleanup preserves the character-
|
||||
creation UI and insert screens (production per the lead call) and strips
|
||||
`lattice_profile` branching, `character.txt` session I/O, and protocol
|
||||
decoding of the removed field. Blocked by this PR; cross-referenced in
|
||||
the server task description.
|
||||
|
||||
**Decision record amendments (2026-04-22):**
|
||||
|
||||
- `decisions/content.md` D-032 `[SUPERSEDED]` header rewritten to cite the
|
||||
cascade instead of the dropped D-117.
|
||||
- `decisions/content.md` D-035 monologue `character` enum note updated
|
||||
(was `[Obsolete post-D-117]`, now cites the cascade strip).
|
||||
- `decisions/perception.md` D-057 marked `[PARTIALLY SUPERSEDED]` with
|
||||
the archetype verb relabeling portion crossed out; vertical-list +
|
||||
Phase 1/2 split preserved.
|
||||
|
||||
**Regression guards for the new uniform behavior** are being added under
|
||||
separate tasks (Hoshe, Sprint 37) — positive assertions that container
|
||||
verb labels and monologue pool selection are archetype-independent, to
|
||||
prevent silent reintroduction.
|
||||
|
||||
**DECISION: STRIP.**
|
||||
|
||||
The original audit body below is retained as a historical record of the
|
||||
pre-override analysis. Do not take its recommendation as current.
|
||||
|
||||
---
|
||||
|
||||
## TL;DR (original audit, SUPERSEDED by lead override above)
|
||||
|
||||
The ticket's premise — that `CharacterArchetype` (Smuggler/Detective) is dead
|
||||
code per D-032's "[Obsolete post-D-117]" footnote — is **stale**. The
|
||||
footnote relied on D-117 (tycoon is the v0.2 bookmark), but `CLAUDE.md`
|
||||
now declares **"v0.2 target is dropped. No scoping negotiations."** That
|
||||
revocation is the most recent architectural directive, and it rolls back
|
||||
the premise that justified marking the enum obsolete.
|
||||
|
||||
Recommendation: **do not delete `CharacterArchetype`**. Instead, update
|
||||
the decision record to clear the stale obsolete footnote, close the
|
||||
ticket as "no-op — premise superseded," and (optionally) claim a small
|
||||
D-record documenting the reversal chain.
|
||||
|
||||
## Audit
|
||||
|
||||
Grep was run against `server/`, `tooling/`, and `tests/`. `CharacterArchetype`
|
||||
has **five load-bearing consumers** plus content-schema users:
|
||||
|
||||
### 1. IPC protocol surface
|
||||
- `server/src/bridge/types.rs:51-54` — `StartupMessage.character_archetype: CharacterArchetype`.
|
||||
Field is serialized into the session handshake. Removing it is a protocol break.
|
||||
- `server/src/bridge/types.rs:500-518` — enum + `as_monologue_key()` helper + `Default = Detective`.
|
||||
- Roundtrip tests at lines 1133, 1146, 1157, 1166, 1173, 1184 exercise the field.
|
||||
|
||||
### 2. Observer pipeline (D-057, #422) — real runtime behavior
|
||||
- `server/src/perception/observer/mod.rs:81,117,138,200` — archetype flows through
|
||||
`apply_phase2_verb_filter`.
|
||||
- `server/src/perception/observer/mod.rs:657-760` — `archetype_verb_label()` swaps
|
||||
container verb labels based on archetype (Smuggler sees `Move`/`Stash`, Detective
|
||||
sees `Scan`/`Flag`). This is live feature code, not scaffolding.
|
||||
|
||||
### 3. Monologue pool selection (D-032, #587, #595)
|
||||
- `server/src/simulation/monologue.rs:134-152` — `MonologueState.character: String` is
|
||||
initialized from `CharacterArchetype.as_monologue_key()` at session start.
|
||||
Pool partition is by string key, but the string is *derived* from the enum.
|
||||
|
||||
### 4. Gauntlet test-world plumbing
|
||||
- `server/src/test_world/mod.rs:98` — `pub fn setup_gauntlet(app: &mut App, archetype: CharacterArchetype)`.
|
||||
- 5 internal callsites (lines 702, 725, 742, 761, 778) plus the external
|
||||
`archetype_monologue.rs` integration suite.
|
||||
|
||||
### 5. Regression test suite
|
||||
- `server/tests/archetype_monologue.rs` — entire file is a regression guard
|
||||
against #587 (archetype→monologue character wiring). Seven tests, four
|
||||
explicitly assert Smuggler vs Detective behavior. Deleting the enum requires
|
||||
deleting this guard, which is the thing that catches the bug it was built for.
|
||||
|
||||
### 6. Content schemas (authoring)
|
||||
- `server/content/schemas/drama_module.schema.yaml:231,233,234,280,558` —
|
||||
schema enumerates `smuggler | detective | any` for dialogue/monologue
|
||||
partitioning in drama modules.
|
||||
|
||||
## Decision chain (why the ticket premise is stale)
|
||||
|
||||
```
|
||||
D-027 (v0.1 vertical slice = smuggler + detective)
|
||||
└─ superseded by D-117 (2026-03-05: tycoon is the v0.2 bookmark)
|
||||
└─ superseded by "v0.2 target is dropped" (CLAUDE.md, current)
|
||||
```
|
||||
|
||||
The obsolete footnote in D-035 line 83 and the `[SUPERSEDED]` header on
|
||||
D-032 both point at D-117 as the supersession. With v0.2 dropped, we are
|
||||
back to the v0.1 smuggler/detective frame as the implemented base until
|
||||
the 6-phase cascade reaches Phase 4 (Player control) — and even then,
|
||||
the cascade describes a 2-floor test map + character rendering, not a
|
||||
wholesale character-model replacement.
|
||||
|
||||
## Proposed alternative scope for #878
|
||||
|
||||
Three options, cheapest first:
|
||||
|
||||
### A. Close as no-op + documentation cleanup (recommended)
|
||||
- Strip the `[Obsolete post-D-117]` footnote from `decisions/content.md:83`
|
||||
(D-035 tag taxonomy).
|
||||
- Remove the `[SUPERSEDED]` marker from `decisions/content.md:47` (D-032
|
||||
header) or add a "supersession reversed" note.
|
||||
- Optionally claim a new D-record in `decisions/scope.md` documenting
|
||||
that the v0.2-dropped directive implicitly reverses D-117's character
|
||||
frame revocation.
|
||||
- Zero code changes. Build stays green. 30 minutes.
|
||||
|
||||
### B. Narrow the ticket to the authoring-side leftovers
|
||||
- If there are *authoring* artifacts (half-written tycoon monologue
|
||||
partitioning, stale schema fields) that were added in anticipation of
|
||||
D-117 and never used, those can be purged. But a quick scan of
|
||||
`drama_module.schema.yaml` shows the schema is consistent with v0.1 usage.
|
||||
- Requires a content team review — server-team scope alone cannot
|
||||
confirm what is live in authoring.
|
||||
|
||||
### C. Rename without removing (if lead wants distance from v0.1 framing)
|
||||
- Rename `CharacterArchetype` → `PlayerCharacterRole` (or similar) and
|
||||
its variants to preserve behavior while shedding the "smuggler/detective
|
||||
investigation framing" language. Higher risk, touches ~45 files, and
|
||||
doesn't actually change runtime. **Not recommended** unless the lead
|
||||
specifically wants the naming to match post-cascade vocabulary.
|
||||
|
||||
## Recommendation (SUPERSEDED — see "Lead override" at top)
|
||||
|
||||
~~Go with **Option A**. The enum is architecturally sound, the ticket is
|
||||
a casualty of the v0.2→cascade pivot, and the cleanup is documentation-
|
||||
only.~~
|
||||
|
||||
**Actual decision:** strip (Option D, not enumerated above — full trace
|
||||
purge driven by cascade framing, not by the D-117/v0.2 supersession
|
||||
chain). The audit's grep-count-first methodology was the wrong starting
|
||||
heuristic; cascade phase comes first. See top of document.
|
||||
Reference in New Issue
Block a user