Single source of truth: combat logic, storage parity, slot ordering #3

Merged
robert merged 32 commits from single-source into main 2026-07-05 00:07:57 -04:00
32 Commits
Author SHA1 Message Date
david raistrick 9f8b09c7d9 feat(ui): first-class undo/redo buttons in combat controls
Undo/redo pills in InitiativeControls, always visible when encounter open.

- Undo = latest non-undone log for this encounter, applies undo.updates.
- Redo = latest undone log, applies undo.redo (forward patch).
- encounterPath added to all 14 logAction contexts (filter key).
- redo:patch added to undoData so redo replays real forward state.
- Disabled when stack empty. Tooltip shows target action message.
- Uses current 2-write undo (non-tx). Race safety deferred to FEAT-LOG.

TODO updated. 84 FE tests green.
2026-07-04 22:48:58 -04:00
david raistrick ed1783e419 docs: reorg TODO — merge FEAT-2+M6 into FEAT-LOG, add undo/redo UI + Is NPC clarify 2026-07-04 22:36:15 -04:00
david raistrick 6fe2b762ab docs: update TODO — toast/modal + dropdown filter done 2026-07-04 22:30:33 -04:00
david raistrick 9ef3e3bd9e feat(ui): filter already-added chars from participant dropdown
Character dropdown in add-participant picker excluded characters already
in the encounter. Prevents dup-add at the source — user can only select
chars not yet participating.
2026-07-04 22:28:41 -04:00
david raistrick 3423319b9b feat(ui): replace all alert() with toast + info modal
Native alert() vanished instantly (browser focus loss when player display
window or devtools steals focus). Replaced all 23 alert() calls with two
in-app feedback paths via React context:

- ToastStack: bottom-right stack, 6s auto-dismiss + manual X. For transient
  failures (storage throws, catch blocks).
- InfoModal: yellow notice modal with OK button, persistent until dismissed.
  For validations (dup-add, empty/invalid fields).

UIFeedbackProvider wraps all 3 App branches (admin/player/logs).
useUIFeedback() hook in 6 components (CharacterManager, ParticipantManager,
InitiativeControls, EncounterManager, LogsView, AdminView).

244 tests green (App 84, shared 133, server 27).
2026-07-04 22:27:32 -04:00
david raistrick 7bcf01dcf9 fix: custom conditions apply on add; correct server WS env docs
Custom conditions (per-campaign freeform) previously only persisted to the
campaign palette — Add button did NOT apply them to the targeted participant,
and badges rendered via CONDITIONS (missing custom ids) so applied custom
conditions did not display. Now:

- addCustomCondition(label, participantId) persists to palette AND toggles
  onto the participant in one step. Enter + Add button both pass p.id.
- Admin badge lookup uses allConditions (built-ins + custom), not CONDITIONS.

Shared contract: toggleCondition accepts ANY string. Prove it in tests:
- turn.combat.test.js adds CUSTOM_CONDITIONS ('hexed','rager',
  'marked_for_death','🛡️blessed') to the seeded queue + random pool, asserts
  condition-not-string invariant, and a dedicated add/remove round-trip test.
- replay-combat.js mirrors the same CUSTOM_CONDITIONS through the live backend.

Server-mode docs/env: REACT_APP_BACKEND_WS (stale pre-rename) -> correct
REACT_APP_BACKEND_REALTIME_URL=ws://.../ws in README, env.example,
docker/Dockerfile (STORAGE=ws -> server). DEVELOPMENT.md + scripts/README.md
now point to ./scripts/dev-start.sh as the primary dev entrypoint.

133 shared tests green.
2026-07-04 20:41:34 -04:00
david raistrick 1ad4b660a4 feat: custom conditions per campaign
DM can add freeform custom conditions alongside built-ins. Persists to
campaign doc, survives across encounters, shared with display view.

Implementation:
- ParticipantManager subscribes campaign doc via useFirestoreDocument
- customConditions[] merged with built-in CONDITIONS at render
- Input field + Add button in conditions picker (Enter or click)
- Dedup case-insensitive vs built-ins + existing custom
- maxLength 40
- addCustomCondition writes to campaigns/{id}.customConditions
- Badge render: custom conditions show raw label (no emoji)
- DisplayView: same fallback render for custom ids
- toggleCondition unchanged (already accepts any string id)
- campaignId prop passed from EncounterManager -> ParticipantManager

243 tests green. Build clean.
2026-07-04 18:20:32 -04:00
david raistrick 9c90c1500c feat(turn): D&D 5e death saves — success/fail tracking
Old model broken: single deathSaves counter, 3=revive. No fails tracked,
successes conflated with fails. DM couldn't model real death save outcomes.

New model (D&D 5e):
- deathSave(enc, id, type, n) — type='success'|'fail', n=1|2|3
- Participant fields: deathSaves (successes), deathFails, isStable, isDying
- 3 successes = stable (0hp unconscious, safe until healed)
- 3 failures = dead (isDying, caller animates removal)
- Toggling same pip = undo that pip
- Returns { patch, log, status } — status: 'stable'|'dead'|'pending'
  isDying back-compat flag kept for App.js removal animation

App.js:
- UI split into green ✓ saves row + red ✕ fails row
- Status badges: Stabilized (emerald) / Dying (red pulse)
- handleDeathSaveChange(participantId, type, saveNumber) — new sig
- makeParticipant init: deathFails + isStable defaults

Data loss: old single-counter participants lose saves (feature didn't work,
no real state to preserve). User confirmed acceptable.

Tests:
- turn.deathsave.test.js: RED-then-GREEN, 3-success stable, 3-fail dead,
  independent tracking, new signature
- Updated 6 callers across characterization/combat/dead-skip/logging/scenario
- Logs UI tests: new title selectors (Success N / Fail N)

243 tests green.
2026-07-04 17:56:14 -04:00
david raistrick f2797595f4 test: static guard — every mutation logged
Per-op logging test encoded gaps as expected (asserted log:null = pass).
Missed 4 real bugs (BUG-7 + deathSave + addParticipants + updateParticipant).
Bunk. Static scan catches regressions regardless of test enumeration.

static.no-unlogged.test.js: scans turn.js source, extracts function bodies
via brace counting, finds any return with patch!=null AND log:null.
Verified: injected fake unlogged mutation in endEncounter → guard fails
with offender name + return expr. Restored → 128 green.

Logging contract now enforced structurally, not by enumeration.
2026-07-04 17:26:59 -04:00
david raistrick 32b8dfd8a1 fix(turn): log addParticipants + updateParticipant (logging gaps)
addParticipants + updateParticipant returned log:null → invisible in
combat log. Contract test caught them. Now both return
log:{ message, undo:{ participants }}.

App.js handlers wired (handleAddAllCharacters, handleUpdateParticipant,
handleInlineInitiative) — call logAction on logged mutations.

Logging contract now complete: every mutating combat op produces a log
entry. No gaps. turn.logging.test.js proves it.

238 tests green.
2026-07-04 17:25:22 -04:00
david raistrick ab9f8fa2e7 fix(turn): log reorder + deathSave (BUG-7)
reorderParticipants returned log:null → handler skipped logAction → drag
invisible in combat log, no undo payload. Now returns
log:{ message, undo:{ participants, turnOrderIds, currentTurnParticipantId }}.
App.js handleDrop calls logAction on logged reorders.

Found second gap: deathSave also returned null log (both branches).
Fixed — message + undo (participants snapshot).

Added logging contract test (turn.logging.test.js):
- all mutating ops logged (start/next/pause/add/remove/toggle/hp/deathsave/
  condition/reorder/end)
- no-ops return null log (same-id, cross-init, cross-pointer blocks)
- undo payloads valid + restore prior state
- documents addParticipants + updateParticipant gaps (null log, intentional)

235 tests green.
2026-07-04 17:22:52 -04:00
david raistrick 5a09f71d4c refactor(turn): remove dead computeTurnOrderAfterAddition
Zero call sites. Old turn-array addition logic, superseded by 1-list
slotIndexForInit model. Removed func, export, stale comment, and 3
characterization tests that only locked the dead func's behavior.
2026-07-04 17:18:29 -04:00
david raistrick 942545c085 fix(turn): block cross-pointer reorder (BUG-13)
reorderParticipants allowed dragging across current pointer mid-round.
nextTurn walks turnOrderIds forward from pointer, so cross-pointer drag
= ambiguous who-acts-next: upcoming dragged ahead of pointer = skipped
(walk wraps past), acted dragged behind pointer = double-act.

Full fix needs actedThisRound tracking. Pragmatic now: block both
directions during active encounter. Pre-combat: free reorder (no pointer).

reorderParticipants: added pointer check (currentTurn index). If drag
crosses pointer, return null patch (no-op). Same-side reorder allowed.
syncTurnOrder now always mirrors (was started-only — pre-combat reorder
left turnOrderIds stale).

Tests:
- turn.bug13.test.js: cross-pointer blocked (both dirs), same-side allowed,
  pre-combat free.
- turn.reorder.test.js: 3 tests updated to pre-combat (cross-pointer now
  blocked post-start).
- turn.invariant.test.js: reorder test uses same-side upcoming pair.

214 tests green.
2026-07-04 17:12:13 -04:00
david raistrick 0ff50561c7 test: BUG-10 deact+reactivate double-act (already fixed by 1-list model)
1-list model keeps slot position on toggle. Reactivate does not re-insert
by initiative, so no second turn. computeTurnOrderAfterAddition is dead code
(no call sites). Tests prove no double-act in both scenarios.
2026-07-04 17:06:07 -04:00
david raistrick 8ad750c60a test: add 10s timeout guard across all suites
Hangs/regressions fail fast instead of stalling CI.

- shared/jest.config.js: testTimeout 10000
- src/setupTests.js: jest.setTimeout(10000) (CRA blocks testTimeout in
  package.json — not in allowlist)
- server/jest.config.js: already had 10000

Verified: injected hanging test -> 'Exceeded timeout of 10000 ms'.
2026-07-04 17:03:34 -04:00
david raistrick 81a1629118 docs: rewrite TODO clean — closed items to history, open items current
Cross-referenced every item vs actual code state. 11 items closed (moved to
history): BUG-4/5/6/8/11/12/14/15/16/17/18, FEAT-1, add-all-characters,
warning=failure, scenario-100-rounds.

Open (7): BUG-7 (reorder undo), BUG-10 (deact/reactivate double-act),
BUG-13 (cross-pointer reorder), FEAT-2 (structured logs), FEAT-3 (initiative
field), FEAT-M6 (transactional undo), death-saves-fails, custom-conditions,
test-timeouts.
2026-07-04 16:59:52 -04:00
david raistrick 4da5ed9bcc Merge remote-tracking branch 'upstream/main' into single-source
# Conflicts:
#	src/App.js
2026-07-04 16:43:14 -04:00
david raistrick c556860e49 refactor(App): remove dead SDK imports (BUG-17)
App.js imported 9 Firestore SDK functions it never called standalone:
doc, setDoc, addDoc, collection, onSnapshot, updateDoc, deleteDoc, query,
writeBatch. All writes go through storage adapter (storage.*). Auth pieces
(initializeApp, getAuth, signInAnonymously, onAuthStateChanged,
signInWithCustomToken, getFirestore) stay — real SDK init in firebase mode.

orderBy/limit stay — now neutral builders from index.js (prev commit).
2026-07-04 16:07:21 -04:00
david raistrick 27b2272ced fix(storage): neutral queryConstraints honored by both adapters
Bug: server adapter accepted queryConstraints (orderBy/limit) but ignored them.
Combat log [orderBy('timestamp','desc'), limit(500)] returned ALL logs unordered
in server mode — unbounded growth. No test caught it: shared contract tested
subscribeCollection with 0 constraints; the firebase-only queryConstraint test
never touched the server adapter. Parity gap.

Fix (Plan A — neutral shape):
- index.js: orderBy()/limit() now NEUTRAL builders returning {__type}. Removed
  SDK orderBy/limit re-export.
- firebase.js: subscribeCollection translates neutral -> SDK query constraints.
- server.js: applyConstraints() helper sorts/limits client-side (backend returns
  all). Constraints stored per collection, applied on initial fetch + WS change.
- contract.js: added 3 queryConstraint tests (orderBy desc, limit, no-constraints)
  to SHARED contract — both adapters now run identical assertions.
- firebase.contract.test.js: removed now-redundant firebase-only constraint
  block (covered by shared contract).

Data impact: ZERO. Constraints are query-time only, never stored. Combat log
docs keep timestamp field. No Firebase data migration needed.

208 tests green.
2026-07-04 16:06:28 -04:00
david raistrick e94e1959ff refactor: rename ws adapter to server across codebase
'ws' conflated storage mode with transport protocol (WebSocket). Renamed for
clarity: the adapter talks the self-hosted server (src/storage/server.js), which
happens to use WebSocket for realtime — but the name should describe what it
connects to, not how.

Renames:
- src/storage/ws.js -> server.js
- createWsStorage() -> createServerStorage()
- storage mode 'ws' -> 'server'
- REACT_APP_BACKEND_WS -> REACT_APP_BACKEND_REALTIME_URL
- factory param wsUrl -> realtimeUrl
- server/tests/ws-*.test.js -> server-*.test.js

Kept literal: 'ws' npm package, ws:// protocol URLs, /ws WebSocket endpoint.
Transport is accurate there; only storage-naming changed.

Updated all docs (DEVELOPMENT, TESTING, REWORK_PLAN, GLOSSARY, TODO), scripts
(dev-start.sh, replay-combat.js), factory + ESM tests. 204 tests green.
2026-07-04 15:47:13 -04:00
david raistrick cb41d9ec8f fix(turn): slot-not-sort in add/update; static guard against stray .sort()
addParticipant + updateParticipant used sortParticipantsByInitiative (stable
sort, tie-break = original array index). That destroyed manually-dragged tie
order when a drag moved a same-init pair — violated the slot-not-sort design
(docs/INITIATIVE_ORDERING.md: 'Re-slotting on add/edit must preserve
drag-established tie order').

Fix: new slotIndexForInit(list, init) returns splice index into the CURRENT
list (initiative-descending). addParticipant inserts there; updateParticipant
re-inserts only when initiative changed (unrelated edits keep slot — avoids
mid-round rotation dupes surfaced by the 100-round combat test).

Tests:
- turn.slot-not-sort.test.js: drag tie order survives add/edit (4 cases).
- static.no-sort.test.js: errs if .sort( added outside allowlist
  (sortParticipantsByInitiative only). Verified by injecting a stray sort —
  guard caught it.
2026-07-04 15:40:39 -04:00
david raistrick 6baecd374e ws adapter: match main truth (id injection + setDoc merge)
- getDoc/getCollection/subscribe inject {id,...data} (main firebase shape)
- getCollection normalizes backend {id,data}[] OR bare data[] → {id,...data}
- setDoc(opts.merge) → PATCH (create-on-miss); else PUT (replace)
- ws-reconnect test asserts id now present

24/24 server green. All suites green: App 83, shared 91, server 24.
2026-07-04 12:35:28 -04:00
david raistrick 79af61cb8b match main firebase behavior; fix contract + mismatches
Contract rewritten to match main prod truth:
- require id injection in all doc/collection results
- honor setDoc({merge:true}) (main L1624 + 4 activeDisplay sites)
- drop invented bare<->prefixed cross-lookup tests (main never does this)
- add setDoc{merge}, batch set-only/update-only contract coverage

Fix mismatches vs main:
- subscribeCollection hook now forwards queryConstraints (was dropped;
  LOG_QUERY sort+limit honored again). Mock onSnapshot honors orderBy+limit.
  2 new contract tests prove the chain.
- subscribeDoc/subscribeCollection adapters now forward errCb. App hooks +
  DisplayView propagate subscribe errors to UI (match main onSnapshot 3rd arg).
- ws adapter signatures accept queryConstraints + errCb (interface match).

activeDisplay writes match main:
- 5 unguarded sites -> setDoc({merge:true}) (create-if-missing, not updateDoc)
- 3 guarded sites stay updateDoc

Delete memory adapter: third storage system added complexity, zero value.
firebase-mock covers fast tests, ws covers server path. Factory throws on
unknown mode now.

Delete phantom 'Phase A/B' comment in firebase.js header.

Tests updated to assert setDoc{merge} (not updateDoc) for activeDisplay writes.
83/83 green.
2026-07-04 12:22:19 -04:00
david raistrick b12ac904a6 test: add storage coverage (firebase contract, factory), silence scenario log
- firebase.contract.test.js: run storage contract against createFirebaseStorage
  via SDK mock. Currently 12 fail (firebase diverges from contract: no norm(),
  injects id). Failures real — firebase copied from main, memory/ws invented
  new requirements. Contract under review vs main prod truth.
- storage.factory.test.js: getStorage() routing per REACT_APP_STORAGE env,
  singleton cache, getStorageMode() reporting.
- Combat.scenario.test.js: remove console.log (test noise).
- package.json: test:all now runs App + shared + server suites (was missing App).
2026-07-03 18:53:58 -04:00
david raistrick 7f60ec140e fix(scenario): run 100 rounds not turns, exercise all combat paths
Previous test lied: called nextTurn 100x = ~100 turns = ~12 rounds (see
docs/GLOSSARY.md turn vs round). Now loops by actual round-wrap count.

100 rounds = 975 turns verified via console log + expect(roundsDone).toBe(100).

Exercises ALL shared combat paths deterministically (vs replay random):
startEncounter, nextTurn, togglePause, addParticipant, addParticipants,
updateParticipant, removeParticipant, toggleParticipantActive, applyHpChange
(damage+heal), deathSave (1/2/3-success → isDying path), toggleCondition,
reorderParticipants (drag same-init tie), endEncounter, activateDisplay,
clearDisplay, toggleHidePlayerHp.

Rotation integrity checked every 10 rounds: no dup turnOrderIds, currentTurn
in order, turnOrderIds === participants.map(id).

Run time: 19ms (was 72s React, lieing).
2026-07-03 18:29:35 -04:00
david raistrick e650cd2710 refactor: single-source display lifecycle + scenario test no React
Move activeDisplay lifecycle to shared (one path, not duplicated in App):
- activateDisplay({campaignId, encounterId})
- clearDisplay()
- toggleHidePlayerHp(current)
All 6 App sites wired (start/end/2×delete-encounter/delete-campaign/hp-toggle).
Existing {patch,log} return shape preserved. No interface changes.

Combat.scenario.test.js rewritten to call shared directly, no React:
- Same actions as old UI version (roster chars, monsters, addAll, hp-toggle×2,
  start, 100 rounds of damage/heal/conditions/toggle/edit/deathsave/pause/
  resume/add/remove, end). Same funcs, same order, same data.
- Models two firestore docs (encounter + activeDisplay) via shared patches.
- Run time: 72s -> 4ms.

Both suites green: shared 91/91, App 77/77.
2026-07-03 18:26:02 -04:00
david raistrick d83d1383c0 refactor: single-source combat logic (App handlers call shared, slot model)
App.js no longer inlines combat logic. All handlers delegate to @ttrpg/shared:
startEncounter, nextTurn, togglePause, endEncounter, addParticipant(s),
updateParticipant, removeParticipant, toggleParticipantActive, applyHpChange,
deathSave, toggleCondition, reorderParticipants. ONE code path for UI.

shared/turn.js aligned to slot model (docs/INITIATIVE_ORDERING.md):
- addParticipant: splice into slot by initiative (no sort)
- updateParticipant: re-slot on initiative change (no sort)
- reorderParticipants: same-init drag only (cross-init blocked)
- applyHpChange: death flips isActive=false (matches App), revive reactivates

No renames, no API changes. Shared func signatures unchanged.

Tests updated to match unified behavior:
- dead-skip: death deactivates (was FEAT-1 stays active)
- characterization: death deactivates + revive reactivates
- reorder/invariant: cross-init drag blocked, same-init allowed

Both suites green: shared 91/91, App 77/77, 0 warnings.
2026-07-03 17:59:41 -04:00
david raistrick c3d89554e8 docs: initiative ordering design (slot, never sort)
Single list = display = turn order. Slot by initiative, tie-break = add
order. Drag = same-init only (DM tie override). No re-sort on mutation.
Round wrap no rebuild. No cross-init drag.
2026-07-03 17:18:00 -04:00
david raistrick d1cc3a8b9a test: warning=failure guard + fix ambiguous switch selector
- setupTests.js: console.error/warn now throw. Warning = failure.
- Combat.characterization: two role=switch (player HP + NPC HP), scope
  to player-HP label. getByRole('switch') was ambiguous.

Full suite: 77/77 pass, 0 warnings.
2026-07-03 16:05:04 -04:00
david raistrick 31d19c5912 test: kill act env warnings + remove debug console noise
Root cause: mock _notify cb fires during waitFor poll, but testing-library
asyncWrapper flips IS_REACT_ACT_ENVIRONMENT=false. Raw react act() then
warns 'not configured to support act' (146 warnings).

Fix: _notify sets globalThis.IS_REACT_ACT_ENVIRONMENT=true around act(),
restores after. setupTests sets flag globally. Both together = 0 warnings.

Also:
- Remove 4 debug console.log from App.js (encounter start/end, drag, add chars)
- Remove always-fires scenario summary console.log
- Add fastWaitFor helper (5ms poll) to testHelpers
- Swap scenario waitFor -> fastWaitFor

Tests: 77 pass / 0 fail / 0 warnings / 0 console noise.
2026-07-03 15:59:14 -04:00
david raistrick 530aaadf90 test: kill act env warnings (globalThis flag + _notify act wrapper)
- setupTests.js: set globalThis.IS_REACT_ACT_ENVIRONMENT = true (RTL reads
  getGlobalThis, not global). Kills 'environment not configured for act'.
- _mock-db.js _notify: set flag true around subscriber cb, restore prev
  after. RTL waitFor flips flag false mid-poll -> act() warned. Now clean.
- 146 act warnings -> 0.
2026-07-03 15:58:50 -04:00
david raistrick 1a744e511a test: upgrade @testing-library/react v14, kill act warnings
@testing-library/react v13 -> v14. v13 used deprecated
ReactDOMTestUtils.act on every render -> 708 deprecation warnings.
v14 imports act from react directly. Drops to 0.

Mock _notify (src/__mocks__/firebase/_mock-db.js): wrap subscriber
callbacks in act(() => cb()). Sync setState from subscribe callbacks
fired outside test act boundary -> 14 warnings. Now 0. try/catch
fallback if react/act unavailable (defensive).

Full suite: 166 pass / 1 fail (Combat.scenario BUG-11 pre-existing
crash). Warnings: 1416 -> 0.

Note: NOT complete kill-shared. App.js still inlines 8 logic fns
(startEncounter, nextTurn, togglePause, endEncounter, addParticipant,
updateParticipant, reorder/drag). turn.js versions dead in app, used
by replay + turn tests only. Next: make handlers call turn.js.
2026-07-03 15:46:27 -04:00