From 1de00e3d8ee6c6d32d7c15e9aa4586a033cc9af9 Mon Sep 17 00:00:00 2001 From: Lukas Date: Sat, 28 Mar 2026 11:12:41 +0100 Subject: [PATCH] Move entity rehydration to domain layer, fix tempHp gap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rehydration functions (reconstructing typed domain objects from untyped JSON) lived in persistence adapters, duplicating domain validation. Adding a field required updating both the domain type and a separate adapter function — the adapter was missed for `level`, silently dropping it on reload. Now adding a field only requires updating the domain type and its co-located rehydration function. - Add `rehydratePlayerCharacter` and `rehydrateCombatant` to domain - Persistence adapters delegate to domain instead of reimplementing - Add `tempHp` validation (was silently dropped during rehydration) - Tighten initiative validation to integer-only - Exhaustive domain tests (53 cases); adapter tests slimmed to round-trip - Remove stale `jsinspect-plus` Knip ignoreDependencies entry Co-Authored-By: Claude Opus 4.6 (1M context) --- .../__tests__/encounter-storage.test.ts | 141 +--------- .../player-character-storage.test.ts | 129 +--------- apps/web/src/persistence/encounter-storage.ts | 115 ++------- apps/web/src/persistence/export-import.ts | 4 +- .../persistence/player-character-storage.ts | 57 +---- .../research/2026-03-28-entity-rehydration.md | 238 +++++++++++++++++ knip.json | 1 - .../src/__tests__/rehydrate-combatant.test.ts | 241 ++++++++++++++++++ .../rehydrate-player-character.test.ts | 136 ++++++++++ packages/domain/src/index.ts | 2 + packages/domain/src/rehydrate-combatant.ts | 106 ++++++++ .../domain/src/rehydrate-player-character.ts | 55 ++++ 12 files changed, 799 insertions(+), 426 deletions(-) create mode 100644 docs/agents/research/2026-03-28-entity-rehydration.md create mode 100644 packages/domain/src/__tests__/rehydrate-combatant.test.ts create mode 100644 packages/domain/src/__tests__/rehydrate-player-character.test.ts create mode 100644 packages/domain/src/rehydrate-combatant.ts create mode 100644 packages/domain/src/rehydrate-player-character.ts diff --git a/apps/web/src/persistence/__tests__/encounter-storage.test.ts b/apps/web/src/persistence/__tests__/encounter-storage.test.ts index 66112b2..e2697a6 100644 --- a/apps/web/src/persistence/__tests__/encounter-storage.test.ts +++ b/apps/web/src/persistence/__tests__/encounter-storage.test.ts @@ -122,64 +122,7 @@ describe("loadEncounter", () => { expect(loadEncounter()).toBeNull(); }); - // US3: Corrupt data scenarios - it("returns null for non-object JSON (string)", () => { - localStorage.setItem(STORAGE_KEY, JSON.stringify("hello")); - expect(loadEncounter()).toBeNull(); - }); - - it("returns null for non-object JSON (number)", () => { - localStorage.setItem(STORAGE_KEY, JSON.stringify(42)); - expect(loadEncounter()).toBeNull(); - }); - - it("returns null for non-object JSON (array)", () => { - localStorage.setItem(STORAGE_KEY, JSON.stringify([1, 2, 3])); - expect(loadEncounter()).toBeNull(); - }); - - it("returns null for non-object JSON (null)", () => { - localStorage.setItem(STORAGE_KEY, "null"); - expect(loadEncounter()).toBeNull(); - }); - - it("returns null when combatants is a string instead of array", () => { - localStorage.setItem( - STORAGE_KEY, - JSON.stringify({ - combatants: "not-array", - activeIndex: 0, - roundNumber: 1, - }), - ); - expect(loadEncounter()).toBeNull(); - }); - - it("returns null when activeIndex is a string instead of number", () => { - localStorage.setItem( - STORAGE_KEY, - JSON.stringify({ - combatants: [{ id: "1", name: "Aria" }], - activeIndex: "zero", - roundNumber: 1, - }), - ); - expect(loadEncounter()).toBeNull(); - }); - - it("returns null when combatant entry is missing id", () => { - localStorage.setItem( - STORAGE_KEY, - JSON.stringify({ - combatants: [{ name: "Aria" }], - activeIndex: 0, - roundNumber: 1, - }), - ); - expect(loadEncounter()).toBeNull(); - }); - - it("returns null when combatant entry is missing name", () => { + it("returns null when combatant has invalid required fields", () => { localStorage.setItem( STORAGE_KEY, JSON.stringify({ @@ -191,88 +134,6 @@ describe("loadEncounter", () => { expect(loadEncounter()).toBeNull(); }); - it("returns null for negative roundNumber", () => { - localStorage.setItem( - STORAGE_KEY, - JSON.stringify({ - combatants: [{ id: "1", name: "Aria" }], - activeIndex: 0, - roundNumber: -1, - }), - ); - expect(loadEncounter()).toBeNull(); - }); - - it("returns empty encounter for zero combatants (cleared state)", () => { - localStorage.setItem( - STORAGE_KEY, - JSON.stringify({ combatants: [], activeIndex: 0, roundNumber: 1 }), - ); - const result = loadEncounter(); - expect(result).toEqual({ - combatants: [], - activeIndex: 0, - roundNumber: 1, - }); - }); - - it("round-trip preserves combatant AC value", () => { - const result = createEncounter( - [{ id: combatantId("1"), name: "Aria", ac: 18 }], - 0, - 1, - ); - if (isDomainError(result)) throw new Error("unreachable"); - saveEncounter(result); - const loaded = loadEncounter(); - expect(loaded?.combatants[0].ac).toBe(18); - }); - - it("round-trip preserves combatant without AC", () => { - const result = createEncounter( - [{ id: combatantId("1"), name: "Aria" }], - 0, - 1, - ); - if (isDomainError(result)) throw new Error("unreachable"); - saveEncounter(result); - const loaded = loadEncounter(); - expect(loaded?.combatants[0].ac).toBeUndefined(); - }); - - it("discards invalid AC values during rehydration", () => { - localStorage.setItem( - STORAGE_KEY, - JSON.stringify({ - combatants: [ - { id: "1", name: "Neg", ac: -1 }, - { id: "2", name: "Float", ac: 3.5 }, - { id: "3", name: "Str", ac: "high" }, - ], - activeIndex: 0, - roundNumber: 1, - }), - ); - const loaded = loadEncounter(); - expect(loaded).not.toBeNull(); - expect(loaded?.combatants[0].ac).toBeUndefined(); - expect(loaded?.combatants[1].ac).toBeUndefined(); - expect(loaded?.combatants[2].ac).toBeUndefined(); - }); - - it("preserves AC of 0 during rehydration", () => { - localStorage.setItem( - STORAGE_KEY, - JSON.stringify({ - combatants: [{ id: "1", name: "Aria", ac: 0 }], - activeIndex: 0, - roundNumber: 1, - }), - ); - const loaded = loadEncounter(); - expect(loaded?.combatants[0].ac).toBe(0); - }); - it("saving after modifications persists the latest state", () => { const encounter = makeEncounter(); saveEncounter(encounter); diff --git a/apps/web/src/persistence/__tests__/player-character-storage.test.ts b/apps/web/src/persistence/__tests__/player-character-storage.test.ts index 064a342..139769b 100644 --- a/apps/web/src/persistence/__tests__/player-character-storage.test.ts +++ b/apps/web/src/persistence/__tests__/player-character-storage.test.ts @@ -90,134 +90,7 @@ describe("player-character-storage", () => { }); }); - describe("per-character validation", () => { - it("discards character with missing name", () => { - mockStorage.setItem( - STORAGE_KEY, - JSON.stringify([ - { id: "pc-1", ac: 10, maxHp: 50, color: "blue", icon: "sword" }, - ]), - ); - expect(loadPlayerCharacters()).toEqual([]); - }); - - it("discards character with empty name", () => { - mockStorage.setItem( - STORAGE_KEY, - JSON.stringify([ - { - id: "pc-1", - name: "", - ac: 10, - maxHp: 50, - color: "blue", - icon: "sword", - }, - ]), - ); - expect(loadPlayerCharacters()).toEqual([]); - }); - - it("discards character with invalid color", () => { - mockStorage.setItem( - STORAGE_KEY, - JSON.stringify([ - { - id: "pc-1", - name: "Test", - ac: 10, - maxHp: 50, - color: "neon", - icon: "sword", - }, - ]), - ); - expect(loadPlayerCharacters()).toEqual([]); - }); - - it("discards character with invalid icon", () => { - mockStorage.setItem( - STORAGE_KEY, - JSON.stringify([ - { - id: "pc-1", - name: "Test", - ac: 10, - maxHp: 50, - color: "blue", - icon: "banana", - }, - ]), - ); - expect(loadPlayerCharacters()).toEqual([]); - }); - - it("discards character with negative AC", () => { - mockStorage.setItem( - STORAGE_KEY, - JSON.stringify([ - { - id: "pc-1", - name: "Test", - ac: -1, - maxHp: 50, - color: "blue", - icon: "sword", - }, - ]), - ); - expect(loadPlayerCharacters()).toEqual([]); - }); - - it("discards character with maxHp of 0", () => { - mockStorage.setItem( - STORAGE_KEY, - JSON.stringify([ - { - id: "pc-1", - name: "Test", - ac: 10, - maxHp: 0, - color: "blue", - icon: "sword", - }, - ]), - ); - expect(loadPlayerCharacters()).toEqual([]); - }); - - it("preserves level through save/load round-trip", () => { - const pc = makePC({ level: 5 }); - savePlayerCharacters([pc]); - const loaded = loadPlayerCharacters(); - expect(loaded[0].level).toBe(5); - }); - - it("preserves undefined level through save/load round-trip", () => { - const pc = makePC(); - savePlayerCharacters([pc]); - const loaded = loadPlayerCharacters(); - expect(loaded[0].level).toBeUndefined(); - }); - - it("discards character with invalid level", () => { - mockStorage.setItem( - STORAGE_KEY, - JSON.stringify([ - { - id: "pc-1", - name: "Test", - ac: 10, - maxHp: 50, - color: "blue", - icon: "sword", - level: 25, - }, - ]), - ); - expect(loadPlayerCharacters()).toEqual([]); - }); - + describe("delegation to domain rehydration", () => { it("keeps valid characters and discards invalid ones", () => { mockStorage.setItem( STORAGE_KEY, diff --git a/apps/web/src/persistence/encounter-storage.ts b/apps/web/src/persistence/encounter-storage.ts index 2b44547..1d4beb1 100644 --- a/apps/web/src/persistence/encounter-storage.ts +++ b/apps/web/src/persistence/encounter-storage.ts @@ -1,14 +1,9 @@ import { - type ConditionId, - combatantId, + type Combatant, createEncounter, - creatureId, type Encounter, isDomainError, - playerCharacterId, - VALID_CONDITION_IDS, - VALID_PLAYER_COLORS, - VALID_PLAYER_ICONS, + rehydrateCombatant, } from "@initiative/domain"; const STORAGE_KEY = "initiative:encounter"; @@ -21,93 +16,6 @@ export function saveEncounter(encounter: Encounter): void { } } -function validateAc(value: unknown): number | undefined { - return typeof value === "number" && Number.isInteger(value) && value >= 0 - ? value - : undefined; -} - -function validateConditions(value: unknown): ConditionId[] | undefined { - if (!Array.isArray(value)) return undefined; - const valid = value.filter( - (v): v is ConditionId => - typeof v === "string" && VALID_CONDITION_IDS.has(v), - ); - return valid.length > 0 ? valid : undefined; -} - -function validateCreatureId(value: unknown) { - return typeof value === "string" && value.length > 0 - ? creatureId(value) - : undefined; -} - -function validateHp( - rawMaxHp: unknown, - rawCurrentHp: unknown, -): { maxHp: number; currentHp: number } | undefined { - if ( - typeof rawMaxHp !== "number" || - !Number.isInteger(rawMaxHp) || - rawMaxHp < 1 - ) { - return undefined; - } - const validCurrentHp = - typeof rawCurrentHp === "number" && - Number.isInteger(rawCurrentHp) && - rawCurrentHp >= 0 && - rawCurrentHp <= rawMaxHp; - return { - maxHp: rawMaxHp, - currentHp: validCurrentHp ? rawCurrentHp : rawMaxHp, - }; -} - -function rehydrateCombatant(c: unknown) { - const entry = c as Record; - const base = { - id: combatantId(entry.id as string), - name: entry.name as string, - initiative: - typeof entry.initiative === "number" ? entry.initiative : undefined, - }; - - const color = - typeof entry.color === "string" && VALID_PLAYER_COLORS.has(entry.color) - ? entry.color - : undefined; - const icon = - typeof entry.icon === "string" && VALID_PLAYER_ICONS.has(entry.icon) - ? entry.icon - : undefined; - const pcId = - typeof entry.playerCharacterId === "string" && - entry.playerCharacterId.length > 0 - ? playerCharacterId(entry.playerCharacterId) - : undefined; - - const shared = { - ...base, - ac: validateAc(entry.ac), - conditions: validateConditions(entry.conditions), - isConcentrating: entry.isConcentrating === true ? true : undefined, - creatureId: validateCreatureId(entry.creatureId), - color, - icon, - playerCharacterId: pcId, - }; - - const hp = validateHp(entry.maxHp, entry.currentHp); - return hp ? { ...shared, ...hp } : shared; -} - -function isValidCombatantEntry(c: unknown): boolean { - if (typeof c !== "object" || c === null || Array.isArray(c)) return false; - const entry = c as Record; - return typeof entry.id === "string" && typeof entry.name === "string"; -} - export function rehydrateEncounter(parsed: unknown): Encounter | null { if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return null; @@ -129,14 +37,21 @@ export function rehydrateEncounter(parsed: unknown): Encounter | null { }; } - if (!combatants.every(isValidCombatantEntry)) return null; + const rehydrated: Combatant[] = []; + for (const c of combatants) { + const result = rehydrateCombatant(c); + if (result === null) return null; + rehydrated.push(result); + } - const rehydrated = combatants.map(rehydrateCombatant); + const encounter = createEncounter( + rehydrated, + obj.activeIndex, + obj.roundNumber, + ); + if (isDomainError(encounter)) return null; - const result = createEncounter(rehydrated, obj.activeIndex, obj.roundNumber); - if (isDomainError(result)) return null; - - return result; + return encounter; } export function loadEncounter(): Encounter | null { diff --git a/apps/web/src/persistence/export-import.ts b/apps/web/src/persistence/export-import.ts index f7af4f8..1c354ef 100644 --- a/apps/web/src/persistence/export-import.ts +++ b/apps/web/src/persistence/export-import.ts @@ -4,8 +4,8 @@ import type { PlayerCharacter, UndoRedoState, } from "@initiative/domain"; +import { rehydratePlayerCharacter } from "@initiative/domain"; import { rehydrateEncounter } from "./encounter-storage.js"; -import { rehydrateCharacter } from "./player-character-storage.js"; function rehydrateStack(raw: unknown[]): Encounter[] { const result: Encounter[] = []; @@ -21,7 +21,7 @@ function rehydrateStack(raw: unknown[]): Encounter[] { function rehydrateCharacters(raw: unknown[]): PlayerCharacter[] { const result: PlayerCharacter[] = []; for (const entry of raw) { - const rehydrated = rehydrateCharacter(entry); + const rehydrated = rehydratePlayerCharacter(entry); if (rehydrated !== null) { result.push(rehydrated); } diff --git a/apps/web/src/persistence/player-character-storage.ts b/apps/web/src/persistence/player-character-storage.ts index ccaea24..500ebf8 100644 --- a/apps/web/src/persistence/player-character-storage.ts +++ b/apps/web/src/persistence/player-character-storage.ts @@ -1,9 +1,5 @@ import type { PlayerCharacter } from "@initiative/domain"; -import { - playerCharacterId, - VALID_PLAYER_COLORS, - VALID_PLAYER_ICONS, -} from "@initiative/domain"; +import { rehydratePlayerCharacter } from "@initiative/domain"; const STORAGE_KEY = "initiative:player-characters"; @@ -15,55 +11,6 @@ export function savePlayerCharacters(characters: PlayerCharacter[]): void { } } -function isValidOptionalMember( - value: unknown, - valid: ReadonlySet, -): boolean { - return value === undefined || (typeof value === "string" && valid.has(value)); -} - -export function rehydrateCharacter(raw: unknown): PlayerCharacter | null { - if (typeof raw !== "object" || raw === null || Array.isArray(raw)) - return null; - const entry = raw as Record; - - if (typeof entry.id !== "string" || entry.id.length === 0) return null; - if (typeof entry.name !== "string" || entry.name.trim().length === 0) - return null; - if ( - typeof entry.ac !== "number" || - !Number.isInteger(entry.ac) || - entry.ac < 0 - ) - return null; - if ( - typeof entry.maxHp !== "number" || - !Number.isInteger(entry.maxHp) || - entry.maxHp < 1 - ) - return null; - if (!isValidOptionalMember(entry.color, VALID_PLAYER_COLORS)) return null; - if (!isValidOptionalMember(entry.icon, VALID_PLAYER_ICONS)) return null; - if ( - entry.level !== undefined && - (typeof entry.level !== "number" || - !Number.isInteger(entry.level) || - entry.level < 1 || - entry.level > 20) - ) - return null; - - return { - id: playerCharacterId(entry.id), - name: entry.name, - ac: entry.ac, - maxHp: entry.maxHp, - color: entry.color as PlayerCharacter["color"], - icon: entry.icon as PlayerCharacter["icon"], - level: entry.level, - }; -} - export function loadPlayerCharacters(): PlayerCharacter[] { try { const raw = localStorage.getItem(STORAGE_KEY); @@ -74,7 +21,7 @@ export function loadPlayerCharacters(): PlayerCharacter[] { const characters: PlayerCharacter[] = []; for (const item of parsed) { - const pc = rehydrateCharacter(item); + const pc = rehydratePlayerCharacter(item); if (pc !== null) { characters.push(pc); } diff --git a/docs/agents/research/2026-03-28-entity-rehydration.md b/docs/agents/research/2026-03-28-entity-rehydration.md new file mode 100644 index 0000000..ddc5f7e --- /dev/null +++ b/docs/agents/research/2026-03-28-entity-rehydration.md @@ -0,0 +1,238 @@ +--- +date: 2026-03-28T01:35:07.925247+00:00 +git_commit: f4fb69dbc763fefe4a73b3491c27093bbd06af0d +branch: main +topic: "Entity rehydration: current implementation and migration surface" +tags: [research, codebase, rehydration, persistence, domain, player-character, combatant] +status: complete +--- + +# Research: Entity Rehydration — Current Implementation and Migration Surface + +## Research Question + +Map all entity rehydration logic (reconstructing typed domain objects from untyped JSON) across the codebase. Document what validation each rehydration function performs, where it lives, how functions cross-reference each other, and what the domain layer already provides that could replace adapter-level validation. This research supports Issue #20: Move entity rehydration to domain layer. + +## Summary + +Entity rehydration currently lives entirely in `apps/web/src/persistence/`. Two primary rehydration functions exist: + +1. **`rehydrateCharacter`** in `player-character-storage.ts` — validates and reconstructs `PlayerCharacter` from unknown JSON +2. **`rehydrateCombatant`** (private) + **`rehydrateEncounter`** (exported) in `encounter-storage.ts` — validates and reconstructs `Combatant`/`Encounter` from unknown JSON + +These are consumed by three call sites: localStorage loading, undo/redo stack loading, and JSON import validation. The domain layer already contains parallel validation logic in `createPlayerCharacter`, `addCombatant`/`validateInit`, and `createEncounter`, but the rehydration functions duplicate this validation with subtly different rules (rehydration is lenient/recovering; creation is strict/rejecting). + +## Detailed Findings + +### 1. PlayerCharacter Rehydration + +**File**: `apps/web/src/persistence/player-character-storage.ts:25-65` + +`rehydrateCharacter(raw: unknown): PlayerCharacter | null` performs: + +| Field | Validation | Behavior on invalid | +|-------|-----------|-------------------| +| `id` | `typeof string`, non-empty | Return `null` (reject entire PC) | +| `name` | `typeof string`, non-empty after trim | Return `null` | +| `ac` | `typeof number`, integer, `>= 0` | Return `null` | +| `maxHp` | `typeof number`, integer, `>= 1` | Return `null` | +| `color` | Optional; if present, must be in `VALID_PLAYER_COLORS` | Return `null` | +| `icon` | Optional; if present, must be in `VALID_PLAYER_ICONS` | Return `null` | +| `level` | Optional; if present, must be integer 1-20 | Return `null` | + +Constructs result via branded `playerCharacterId()` and type assertions for color/icon. + +**Helper**: `isValidOptionalMember(value, validSet)` — shared check for optional set-membership fields (lines 18-23). + +**Callers**: +- `loadPlayerCharacters()` (same file, line 67) — loads from localStorage +- `rehydrateCharacters()` in `export-import.ts:21-30` — filters PCs during import validation + +### 2. Combatant Rehydration + +**File**: `apps/web/src/persistence/encounter-storage.ts:67-103` + +`rehydrateCombatant(c: unknown)` (private, no return type annotation) performs: + +| Field | Validation | Behavior on invalid | +|-------|-----------|-------------------| +| `id` | Cast directly (`entry.id as string`) | No validation (relies on `isValidCombatantEntry` pre-check) | +| `name` | Cast directly (`entry.name as string`) | No validation (relies on pre-check) | +| `initiative` | `typeof number` or `undefined` | Defaults to `undefined` | +| `ac` | Via `validateAc`: integer `>= 0` | Defaults to `undefined` | +| `conditions` | Via `validateConditions`: array, each in `VALID_CONDITION_IDS` | Defaults to `undefined` | +| `isConcentrating` | Strictly `=== true` | Defaults to `undefined` | +| `creatureId` | Via `validateCreatureId`: non-empty string | Defaults to `undefined` | +| `color` | String in `VALID_PLAYER_COLORS` | Defaults to `undefined` | +| `icon` | String in `VALID_PLAYER_ICONS` | Defaults to `undefined` | +| `playerCharacterId` | Non-empty string | Defaults to `undefined` | +| `maxHp` / `currentHp` | Via `validateHp`: maxHp integer >= 1, currentHp integer 0..maxHp | Defaults to `undefined`; invalid currentHp falls back to maxHp | + +**Key difference from PC rehydration**: Combatant rehydration is *lenient* — invalid optional fields are silently dropped rather than rejecting the entire entity. Only `id` and `name` are required (checked by `isValidCombatantEntry` at line 105-109 before `rehydrateCombatant` is called). + +**Helper functions** (all private): +- `validateAc(value)` — lines 24-28 +- `validateConditions(value)` — lines 30-37 +- `validateCreatureId(value)` — lines 39-43 +- `validateHp(rawMaxHp, rawCurrentHp)` — lines 45-65 + +### 3. Encounter Rehydration + +**File**: `apps/web/src/persistence/encounter-storage.ts:111-140` + +`rehydrateEncounter(parsed: unknown): Encounter | null` validates the encounter envelope: +- Must be a non-null, non-array object +- `combatants` must be an array +- `activeIndex` must be a number +- `roundNumber` must be a number +- Empty combatant array → returns hardcoded `{ combatants: [], activeIndex: 0, roundNumber: 1 }` +- All entries must pass `isValidCombatantEntry` (id + name check) +- Maps entries through `rehydrateCombatant`, then passes to domain's `createEncounter` for invariant enforcement + +**Callers**: +- `loadEncounter()` (same file, line 142) — localStorage +- `loadStack()` in `undo-redo-storage.ts:17-36` — undo/redo stacks from localStorage +- `rehydrateStack()` in `export-import.ts:10-19` — import validation +- `validateImportBundle()` in `export-import.ts:32-65` — import validation (direct call for the main encounter) + +### 4. Import Bundle Validation + +**File**: `apps/web/src/persistence/export-import.ts:32-65` + +`validateImportBundle(data: unknown): ExportBundle | string` validates the bundle envelope: +- Version must be `1` +- `exportedAt` must be a string +- `undoStack` and `redoStack` must be arrays +- `playerCharacters` must be an array +- Delegates to `rehydrateEncounter` for the encounter +- Delegates to `rehydrateStack` (which calls `rehydrateEncounter`) for undo/redo +- Delegates to `rehydrateCharacters` (which calls `rehydrateCharacter`) for PCs + +This function validates the *envelope* structure. Entity-level validation is fully delegated. + +### 5. Domain Layer Validation (Existing) + +The domain already contains validation for the same fields, but in *creation* context (typed inputs, DomainError returns): + +**`createPlayerCharacter`** (`packages/domain/src/create-player-character.ts:17-100`): +- Same field rules as `rehydrateCharacter`: name non-empty, ac >= 0 integer, maxHp >= 1 integer, color/icon in valid sets, level 1-20 +- Returns `DomainError` on invalid input (not `null`) + +**`validateInit`** in `addCombatant` (`packages/domain/src/add-combatant.ts:27-53`): +- Validates maxHp (positive integer), ac (non-negative integer), initiative (integer) +- Does NOT validate conditions, color, icon, playerCharacterId, creatureId, isConcentrating + +**`createEncounter`** (`packages/domain/src/types.ts:50-71`): +- Validates activeIndex bounds and roundNumber (positive integer) +- Already used by `rehydrateEncounter` as the final step + +**`editPlayerCharacter`** (`packages/domain/src/edit-player-character.ts`): +- `validateFields` validates the same PC fields for edits + +### 6. Validation Overlap and Gaps + +| Field | Rehydration validates | Domain validates | +|-------|----------------------|-----------------| +| PC.id | Non-empty string | N/A (caller provides) | +| PC.name | Non-empty string | Non-empty (trimmed) | +| PC.ac | Integer >= 0 | Integer >= 0 | +| PC.maxHp | Integer >= 1 | Integer >= 1 | +| PC.color | In VALID_PLAYER_COLORS | In VALID_PLAYER_COLORS | +| PC.icon | In VALID_PLAYER_ICONS | In VALID_PLAYER_ICONS | +| PC.level | Integer 1-20 | Integer 1-20 | +| Combatant.id | Non-empty string (via pre-check) | N/A (caller provides) | +| Combatant.name | String type (via pre-check) | Non-empty (trimmed) | +| Combatant.initiative | `typeof number` | Integer | +| Combatant.ac | Integer >= 0 | Integer >= 0 | +| Combatant.maxHp | Integer >= 1 | Integer >= 1 | +| Combatant.currentHp | Integer 0..maxHp | N/A (set = maxHp on add) | +| Combatant.tempHp | **Not validated** | N/A | +| Combatant.conditions | Each in VALID_CONDITION_IDS | N/A (toggleCondition checks) | +| Combatant.isConcentrating | Strictly `true` or dropped | N/A (toggleConcentration) | +| Combatant.creatureId | Non-empty string | N/A (passed through) | +| Combatant.color | In VALID_PLAYER_COLORS | N/A (passed through) | +| Combatant.icon | In VALID_PLAYER_ICONS | N/A (passed through) | +| Combatant.playerCharacterId | Non-empty string | N/A (passed through) | + +Key observations: +- Rehydration validates `id` (required for identity); domain creation functions receive `id` as a typed parameter +- Combatant rehydration does NOT validate `tempHp` at all — it's silently passed through or ignored +- Combatant rehydration checks `initiative` as `typeof number` but domain checks `Number.isInteger` — slightly different strictness +- Domain validation for combatant optional fields is scattered across individual mutation functions, not centralized + +### 7. Test Coverage + +**Persistence tests** (adapter layer): +- `encounter-storage.test.ts` — ~27 tests covering round-trip, corrupt data, AC validation, edge cases +- `player-character-storage.test.ts` — ~17 tests covering round-trip, corrupt data, field validation, level + +**Import tests** (adapter layer): +- `validate-import-bundle.test.ts` — ~21 tests covering envelope validation, graceful recovery, PC filtering +- `export-import.test.ts` — ~15 tests covering bundle assembly, round-trip, filename resolution + +**Domain tests**: No rehydration tests exist in `packages/domain/` — all rehydration testing is in the adapter layer. + +### 8. Cross-Reference Map + +``` +loadPlayerCharacters() ──→ rehydrateCharacter() + ↑ +validateImportBundle() ──→ rehydrateCharacters() ──┘ + ├─→ rehydrateEncounter() ──→ isValidCombatantEntry() + │ ├─→ rehydrateCombatant() ──→ validateAc() + │ │ ├─→ validateConditions() + │ │ ├─→ validateCreatureId() + │ │ └─→ validateHp() + │ └─→ createEncounter() [domain] + └─→ rehydrateStack() ───→ rehydrateEncounter() [same as above] + +loadEncounter() ───────→ rehydrateEncounter() [same as above] + +loadUndoRedoStacks() ──→ loadStack() ──→ rehydrateEncounter() [same as above] +``` + +## Code References + +- `apps/web/src/persistence/player-character-storage.ts:25-65` — `rehydrateCharacter` (PC rehydration) +- `apps/web/src/persistence/player-character-storage.ts:18-23` — `isValidOptionalMember` helper +- `apps/web/src/persistence/encounter-storage.ts:24-28` — `validateAc` helper +- `apps/web/src/persistence/encounter-storage.ts:30-37` — `validateConditions` helper +- `apps/web/src/persistence/encounter-storage.ts:39-43` — `validateCreatureId` helper +- `apps/web/src/persistence/encounter-storage.ts:45-65` — `validateHp` helper +- `apps/web/src/persistence/encounter-storage.ts:67-103` — `rehydrateCombatant` (combatant rehydration) +- `apps/web/src/persistence/encounter-storage.ts:105-109` — `isValidCombatantEntry` (pre-check) +- `apps/web/src/persistence/encounter-storage.ts:111-140` — `rehydrateEncounter` (encounter envelope rehydration) +- `apps/web/src/persistence/export-import.ts:10-30` — `rehydrateStack` / `rehydrateCharacters` (collection wrappers) +- `apps/web/src/persistence/export-import.ts:32-65` — `validateImportBundle` (import envelope validation) +- `apps/web/src/persistence/undo-redo-storage.ts:17-36` — `loadStack` (undo/redo rehydration) +- `packages/domain/src/create-player-character.ts:17-100` — PC creation validation +- `packages/domain/src/add-combatant.ts:27-53` — `validateInit` (combatant creation validation) +- `packages/domain/src/types.ts:50-71` — `createEncounter` (encounter invariant enforcement) +- `packages/domain/src/types.ts:12-26` — `Combatant` type definition +- `packages/domain/src/player-character-types.ts:70-83` — `PlayerCharacter` type definition + +## Architecture Documentation + +### Current pattern +Rehydration is an adapter concern — persistence adapters validate raw JSON and construct typed domain objects. The domain provides creation functions that validate typed inputs for new entities, but no functions for reconstructing entities from untyped serialized data. + +### Rehydration vs. creation semantics +Rehydration and creation serve different purposes: +- **Creation** (domain): Validates business rules for *new* entities. Receives typed parameters. Returns `DomainError` on failure. +- **Rehydration** (adapter): Reconstructs *previously valid* entities from serialized JSON. Receives `unknown`. Returns `null` on failure. May be lenient (combatants drop invalid optional fields) or strict (PCs reject on any invalid field). + +### Delegation chain +`rehydrateEncounter` already delegates to `createEncounter` for encounter-level invariants. The entity-level rehydration functions (`rehydrateCharacter`, `rehydrateCombatant`) do NOT delegate to any domain function — they re-implement field validation inline. + +### tempHp gap +`Combatant.tempHp` is defined in the domain type but has no validation in the current rehydration code. It appears to be silently included or excluded depending on what `rehydrateCombatant` constructs (it's not in the explicit field list, so it would be dropped during rehydration). + +## Open Questions + +1. **Should `rehydrateCombatant` remain lenient (drop invalid optional fields) or become strict like `rehydrateCharacter` (reject on any invalid field)?** The current asymmetry is intentional: combatants can exist with minimal data (just id + name), while PCs always require ac/maxHp. + +2. **Should `tempHp` be validated during rehydration?** It's currently missing from combatant rehydration but is a valid field on the type. + +3. **Should `rehydrateEncounter` move to domain too, or only the entity-level functions?** The issue acceptance criteria says "validateImportBundle and rehydrateEncounter are unchanged" — but `rehydrateEncounter` currently lives alongside `rehydrateCombatant` and would need to import from domain instead of calling the local function. + +4. **Should `isValidCombatantEntry` (the pre-check) be part of the domain rehydration or remain in the adapter?** It's currently the gate that ensures `id` and `name` exist before `rehydrateCombatant` is called. diff --git a/knip.json b/knip.json index 625a2f2..3db2a0d 100644 --- a/knip.json +++ b/knip.json @@ -1,6 +1,5 @@ { "$schema": "https://unpkg.com/knip@5/schema.json", - "ignoreDependencies": ["jsinspect-plus"], "workspaces": { ".": { "entry": ["scripts/*.mjs"] diff --git a/packages/domain/src/__tests__/rehydrate-combatant.test.ts b/packages/domain/src/__tests__/rehydrate-combatant.test.ts new file mode 100644 index 0000000..fe37972 --- /dev/null +++ b/packages/domain/src/__tests__/rehydrate-combatant.test.ts @@ -0,0 +1,241 @@ +import { describe, expect, it } from "vitest"; +import { rehydrateCombatant } from "../rehydrate-combatant.js"; + +function validCombatant(overrides: Record = {}) { + return { + id: "c-1", + name: "Goblin", + initiative: 12, + ac: 15, + maxHp: 7, + currentHp: 5, + tempHp: 3, + conditions: ["poisoned"], + isConcentrating: true, + creatureId: "creature-goblin", + color: "red", + icon: "skull", + playerCharacterId: "pc-1", + ...overrides, + }; +} + +function minimalCombatant() { + return { id: "c-1", name: "Goblin" }; +} + +describe("rehydrateCombatant", () => { + describe("valid input", () => { + it("accepts a combatant with all fields", () => { + const result = rehydrateCombatant(validCombatant()); + expect(result).not.toBeNull(); + expect(result?.name).toBe("Goblin"); + expect(result?.initiative).toBe(12); + expect(result?.ac).toBe(15); + expect(result?.maxHp).toBe(7); + expect(result?.currentHp).toBe(5); + expect(result?.tempHp).toBe(3); + expect(result?.conditions).toEqual(["poisoned"]); + expect(result?.isConcentrating).toBe(true); + expect(result?.creatureId).toBe("creature-goblin"); + expect(result?.color).toBe("red"); + expect(result?.icon).toBe("skull"); + expect(result?.playerCharacterId).toBe("pc-1"); + }); + + it("accepts a minimal combatant (id + name only)", () => { + const result = rehydrateCombatant(minimalCombatant()); + expect(result).not.toBeNull(); + expect(result?.id).toBe("c-1"); + expect(result?.name).toBe("Goblin"); + expect(result?.initiative).toBeUndefined(); + expect(result?.ac).toBeUndefined(); + expect(result?.maxHp).toBeUndefined(); + }); + + it("preserves branded CombatantId", () => { + const result = rehydrateCombatant(minimalCombatant()); + expect(result?.id).toBe("c-1"); + }); + }); + + describe("required field rejection", () => { + it.each([ + null, + 42, + "string", + [1, 2], + undefined, + ])("rejects non-object input: %j", (input) => { + expect(rehydrateCombatant(input)).toBeNull(); + }); + + it("rejects missing id", () => { + const { id: _, ...rest } = minimalCombatant(); + expect(rehydrateCombatant(rest)).toBeNull(); + }); + + it("rejects empty id", () => { + expect(rehydrateCombatant({ ...minimalCombatant(), id: "" })).toBeNull(); + }); + + it("rejects missing name", () => { + const { name: _, ...rest } = minimalCombatant(); + expect(rehydrateCombatant(rest)).toBeNull(); + }); + + it("rejects non-string name", () => { + expect( + rehydrateCombatant({ ...minimalCombatant(), name: 42 }), + ).toBeNull(); + expect( + rehydrateCombatant({ ...minimalCombatant(), name: null }), + ).toBeNull(); + }); + }); + + describe("optional field leniency", () => { + it("drops invalid ac — keeps combatant", () => { + for (const ac of [-1, 1.5, "15"]) { + const result = rehydrateCombatant({ ...minimalCombatant(), ac }); + expect(result).not.toBeNull(); + expect(result?.ac).toBeUndefined(); + } + }); + + it("drops invalid maxHp — keeps combatant", () => { + for (const maxHp of [0, 1.5, "7"]) { + const result = rehydrateCombatant({ ...minimalCombatant(), maxHp }); + expect(result).not.toBeNull(); + expect(result?.maxHp).toBeUndefined(); + } + }); + + it("falls back currentHp to maxHp when currentHp invalid", () => { + const result = rehydrateCombatant({ + ...minimalCombatant(), + maxHp: 10, + currentHp: "bad", + }); + expect(result?.maxHp).toBe(10); + expect(result?.currentHp).toBe(10); + }); + + it("falls back currentHp to maxHp when currentHp > maxHp", () => { + const result = rehydrateCombatant({ + ...minimalCombatant(), + maxHp: 10, + currentHp: 15, + }); + expect(result?.maxHp).toBe(10); + expect(result?.currentHp).toBe(10); + }); + + it("drops invalid initiative — keeps combatant", () => { + for (const initiative of [1.5, "12"]) { + const result = rehydrateCombatant({ + ...minimalCombatant(), + initiative, + }); + expect(result).not.toBeNull(); + expect(result?.initiative).toBeUndefined(); + } + }); + + it("drops invalid conditions — keeps combatant", () => { + const result = rehydrateCombatant({ + ...minimalCombatant(), + conditions: "poisoned", + }); + expect(result).not.toBeNull(); + expect(result?.conditions).toBeUndefined(); + }); + + it("drops unknown condition IDs", () => { + const result = rehydrateCombatant({ + ...minimalCombatant(), + conditions: ["fake-condition"], + }); + expect(result).not.toBeNull(); + expect(result?.conditions).toBeUndefined(); + }); + + it("filters valid conditions from mixed array", () => { + const result = rehydrateCombatant({ + ...minimalCombatant(), + conditions: ["poisoned", "fake", "blinded"], + }); + expect(result?.conditions).toEqual(["poisoned", "blinded"]); + }); + + it("drops invalid color — keeps combatant", () => { + const result = rehydrateCombatant({ + ...minimalCombatant(), + color: "neon", + }); + expect(result).not.toBeNull(); + expect(result?.color).toBeUndefined(); + }); + + it("drops invalid icon — keeps combatant", () => { + const result = rehydrateCombatant({ + ...minimalCombatant(), + icon: "rocket", + }); + expect(result).not.toBeNull(); + expect(result?.icon).toBeUndefined(); + }); + + it("drops isConcentrating when not strictly true", () => { + for (const isConcentrating of [false, "true", 1]) { + const result = rehydrateCombatant({ + ...minimalCombatant(), + isConcentrating, + }); + expect(result).not.toBeNull(); + expect(result?.isConcentrating).toBeUndefined(); + } + }); + + it("drops invalid creatureId", () => { + for (const creatureId of ["", 42]) { + const result = rehydrateCombatant({ + ...minimalCombatant(), + creatureId, + }); + expect(result).not.toBeNull(); + expect(result?.creatureId).toBeUndefined(); + } + }); + + it("drops invalid playerCharacterId", () => { + for (const playerCharacterId of ["", 42]) { + const result = rehydrateCombatant({ + ...minimalCombatant(), + playerCharacterId, + }); + expect(result).not.toBeNull(); + expect(result?.playerCharacterId).toBeUndefined(); + } + }); + + it("drops invalid tempHp — keeps combatant", () => { + for (const tempHp of [-1, 1.5, "3"]) { + const result = rehydrateCombatant({ + ...minimalCombatant(), + tempHp, + }); + expect(result).not.toBeNull(); + expect(result?.tempHp).toBeUndefined(); + } + }); + + it("preserves valid tempHp of 0", () => { + const result = rehydrateCombatant({ + ...minimalCombatant(), + tempHp: 0, + }); + expect(result?.tempHp).toBe(0); + }); + }); +}); diff --git a/packages/domain/src/__tests__/rehydrate-player-character.test.ts b/packages/domain/src/__tests__/rehydrate-player-character.test.ts new file mode 100644 index 0000000..6d69bb2 --- /dev/null +++ b/packages/domain/src/__tests__/rehydrate-player-character.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it } from "vitest"; +import { rehydratePlayerCharacter } from "../rehydrate-player-character.js"; + +function validPc(overrides: Record = {}) { + return { + id: "pc-1", + name: "Aria", + ac: 16, + maxHp: 45, + color: "blue", + icon: "sword", + level: 5, + ...overrides, + }; +} + +describe("rehydratePlayerCharacter", () => { + describe("valid input", () => { + it("accepts a valid PC with all fields", () => { + const result = rehydratePlayerCharacter(validPc()); + expect(result).not.toBeNull(); + expect(result?.name).toBe("Aria"); + expect(result?.ac).toBe(16); + expect(result?.maxHp).toBe(45); + expect(result?.color).toBe("blue"); + expect(result?.icon).toBe("sword"); + expect(result?.level).toBe(5); + }); + + it("accepts a valid PC without optional color/icon/level", () => { + const result = rehydratePlayerCharacter( + validPc({ color: undefined, icon: undefined, level: undefined }), + ); + expect(result).not.toBeNull(); + expect(result?.color).toBeUndefined(); + expect(result?.icon).toBeUndefined(); + expect(result?.level).toBeUndefined(); + }); + + it("preserves branded PlayerCharacterId", () => { + const result = rehydratePlayerCharacter(validPc()); + expect(result?.id).toBe("pc-1"); + }); + }); + + describe("required field rejection", () => { + it.each([ + null, + 42, + "string", + [1, 2], + undefined, + ])("rejects non-object input: %j", (input) => { + expect(rehydratePlayerCharacter(input)).toBeNull(); + }); + + it("rejects missing id", () => { + const { id: _, ...rest } = validPc(); + expect(rehydratePlayerCharacter(rest)).toBeNull(); + }); + + it("rejects empty id", () => { + expect(rehydratePlayerCharacter(validPc({ id: "" }))).toBeNull(); + }); + + it("rejects missing name", () => { + const { name: _, ...rest } = validPc(); + expect(rehydratePlayerCharacter(rest)).toBeNull(); + }); + + it("rejects empty/whitespace name", () => { + expect(rehydratePlayerCharacter(validPc({ name: "" }))).toBeNull(); + expect(rehydratePlayerCharacter(validPc({ name: " " }))).toBeNull(); + }); + + it("rejects missing ac", () => { + const { ac: _, ...rest } = validPc(); + expect(rehydratePlayerCharacter(rest)).toBeNull(); + }); + + it("rejects negative ac", () => { + expect(rehydratePlayerCharacter(validPc({ ac: -1 }))).toBeNull(); + }); + + it("rejects float ac", () => { + expect(rehydratePlayerCharacter(validPc({ ac: 1.5 }))).toBeNull(); + }); + + it("rejects string ac", () => { + expect(rehydratePlayerCharacter(validPc({ ac: "16" }))).toBeNull(); + }); + + it("rejects missing maxHp", () => { + const { maxHp: _, ...rest } = validPc(); + expect(rehydratePlayerCharacter(rest)).toBeNull(); + }); + + it("rejects maxHp of 0", () => { + expect(rehydratePlayerCharacter(validPc({ maxHp: 0 }))).toBeNull(); + }); + + it("rejects float maxHp", () => { + expect(rehydratePlayerCharacter(validPc({ maxHp: 1.5 }))).toBeNull(); + }); + + it("rejects string maxHp", () => { + expect(rehydratePlayerCharacter(validPc({ maxHp: "45" }))).toBeNull(); + }); + }); + + describe("optional field rejection (strict)", () => { + it("rejects invalid color", () => { + expect(rehydratePlayerCharacter(validPc({ color: "neon" }))).toBeNull(); + }); + + it("rejects invalid icon", () => { + expect(rehydratePlayerCharacter(validPc({ icon: "rocket" }))).toBeNull(); + }); + + it("rejects level 0", () => { + expect(rehydratePlayerCharacter(validPc({ level: 0 }))).toBeNull(); + }); + + it("rejects level 21", () => { + expect(rehydratePlayerCharacter(validPc({ level: 21 }))).toBeNull(); + }); + + it("rejects float level", () => { + expect(rehydratePlayerCharacter(validPc({ level: 3.5 }))).toBeNull(); + }); + + it("rejects string level", () => { + expect(rehydratePlayerCharacter(validPc({ level: "5" }))).toBeNull(); + }); + }); +}); diff --git a/packages/domain/src/index.ts b/packages/domain/src/index.ts index b185d2c..9abdacc 100644 --- a/packages/domain/src/index.ts +++ b/packages/domain/src/index.ts @@ -94,6 +94,8 @@ export { VALID_PLAYER_COLORS, VALID_PLAYER_ICONS, } from "./player-character-types.js"; +export { rehydrateCombatant } from "./rehydrate-combatant.js"; +export { rehydratePlayerCharacter } from "./rehydrate-player-character.js"; export { type RemoveCombatantSuccess, removeCombatant, diff --git a/packages/domain/src/rehydrate-combatant.ts b/packages/domain/src/rehydrate-combatant.ts new file mode 100644 index 0000000..be04d15 --- /dev/null +++ b/packages/domain/src/rehydrate-combatant.ts @@ -0,0 +1,106 @@ +import type { ConditionId } from "./conditions.js"; +import { VALID_CONDITION_IDS } from "./conditions.js"; +import { creatureId } from "./creature-types.js"; +import { + playerCharacterId, + VALID_PLAYER_COLORS, + VALID_PLAYER_ICONS, +} from "./player-character-types.js"; +import type { Combatant } from "./types.js"; +import { combatantId } from "./types.js"; + +function validateAc(value: unknown): number | undefined { + return typeof value === "number" && Number.isInteger(value) && value >= 0 + ? value + : undefined; +} + +function validateConditions(value: unknown): ConditionId[] | undefined { + if (!Array.isArray(value)) return undefined; + const valid = value.filter( + (v): v is ConditionId => + typeof v === "string" && VALID_CONDITION_IDS.has(v), + ); + return valid.length > 0 ? valid : undefined; +} + +function validateHp( + rawMaxHp: unknown, + rawCurrentHp: unknown, +): { maxHp: number; currentHp: number } | undefined { + if ( + typeof rawMaxHp !== "number" || + !Number.isInteger(rawMaxHp) || + rawMaxHp < 1 + ) { + return undefined; + } + const validCurrentHp = + typeof rawCurrentHp === "number" && + Number.isInteger(rawCurrentHp) && + rawCurrentHp >= 0 && + rawCurrentHp <= rawMaxHp; + return { + maxHp: rawMaxHp, + currentHp: validCurrentHp ? rawCurrentHp : rawMaxHp, + }; +} + +function validateTempHp(value: unknown): number | undefined { + return typeof value === "number" && Number.isInteger(value) && value >= 0 + ? value + : undefined; +} + +function validateInteger(value: unknown): number | undefined { + return typeof value === "number" && Number.isInteger(value) + ? value + : undefined; +} + +function validateSetMember( + value: unknown, + valid: ReadonlySet, +): string | undefined { + return typeof value === "string" && valid.has(value) ? value : undefined; +} + +function validateNonEmptyString(value: unknown): string | undefined { + return typeof value === "string" && value.length > 0 ? value : undefined; +} + +function parseOptionalFields(entry: Record) { + return { + initiative: validateInteger(entry.initiative), + ac: validateAc(entry.ac), + conditions: validateConditions(entry.conditions), + isConcentrating: entry.isConcentrating === true ? true : undefined, + creatureId: validateNonEmptyString(entry.creatureId) + ? creatureId(entry.creatureId as string) + : undefined, + color: validateSetMember(entry.color, VALID_PLAYER_COLORS), + icon: validateSetMember(entry.icon, VALID_PLAYER_ICONS), + playerCharacterId: validateNonEmptyString(entry.playerCharacterId) + ? playerCharacterId(entry.playerCharacterId as string) + : undefined, + tempHp: validateTempHp(entry.tempHp), + }; +} + +export function rehydrateCombatant(raw: unknown): Combatant | null { + if (typeof raw !== "object" || raw === null || Array.isArray(raw)) + return null; + const entry = raw as Record; + + if (typeof entry.id !== "string" || entry.id.length === 0) return null; + if (typeof entry.name !== "string") return null; + + const shared: Combatant = { + id: combatantId(entry.id), + name: entry.name, + ...parseOptionalFields(entry), + }; + + const hp = validateHp(entry.maxHp, entry.currentHp); + return hp ? { ...shared, ...hp } : shared; +} diff --git a/packages/domain/src/rehydrate-player-character.ts b/packages/domain/src/rehydrate-player-character.ts new file mode 100644 index 0000000..2d15d7b --- /dev/null +++ b/packages/domain/src/rehydrate-player-character.ts @@ -0,0 +1,55 @@ +import type { PlayerCharacter } from "./player-character-types.js"; +import { + playerCharacterId, + VALID_PLAYER_COLORS, + VALID_PLAYER_ICONS, +} from "./player-character-types.js"; + +function isValidOptionalMember( + value: unknown, + valid: ReadonlySet, +): boolean { + return value === undefined || (typeof value === "string" && valid.has(value)); +} + +export function rehydratePlayerCharacter(raw: unknown): PlayerCharacter | null { + if (typeof raw !== "object" || raw === null || Array.isArray(raw)) + return null; + const entry = raw as Record; + + if (typeof entry.id !== "string" || entry.id.length === 0) return null; + if (typeof entry.name !== "string" || entry.name.trim().length === 0) + return null; + if ( + typeof entry.ac !== "number" || + !Number.isInteger(entry.ac) || + entry.ac < 0 + ) + return null; + if ( + typeof entry.maxHp !== "number" || + !Number.isInteger(entry.maxHp) || + entry.maxHp < 1 + ) + return null; + if (!isValidOptionalMember(entry.color, VALID_PLAYER_COLORS)) return null; + if (!isValidOptionalMember(entry.icon, VALID_PLAYER_ICONS)) return null; + if ( + entry.level !== undefined && + (typeof entry.level !== "number" || + !Number.isInteger(entry.level) || + entry.level < 1 || + entry.level > 20) + ) + return null; + + return { + id: playerCharacterId(entry.id), + name: entry.name, + ac: entry.ac, + maxHp: entry.maxHp, + color: entry.color as PlayerCharacter["color"], + icon: entry.icon as PlayerCharacter["icon"], + level: entry.level, + }; +}