Files

205 lines
11 KiB
Markdown
Raw Permalink Normal View History

# TODO
Backlog of bugs + long-term items, from user. Milestones live in
REWORK_PLAN.md.
## Feature backlog
### CRITICAL BUG - storage
- docker for sql is not using persistant storage...
### feat - campaign section rollup
### feat - add all characters to participants list
### FEAT-M6: Transactional undo (moved from REWORK_PLAN)
- Every mutating action writes event: `(type, payload, undo_payload, undone, ts)`.
- Undo = apply `undo_payload` in same SQLite tx, flip `undone`. Transactional,
no stale clobber.
- Replaces fragile `/logs` snapshot-write undo.
- Migration: keep old undo working for existing entries until cleared; new
format for new entries.
- Related: BUG-7 (reorder no undo).
## Architecture: 1-list turn order model (DONE)
- Single source: turnOrderIds === participants.map(id). No re-sort after
startEncounter. nextTurn skips inactive (predicate), inactive stay in slot.
- Drag (reorder) overrides initiative --- cross-init allowed, DM choice.
- startEncounter sorts ALL participants by init once, then frozen.
- addParticipant splices by init pos. remove/toggle/reorder sync list.
- Display renders participants[] directly (no sortParticipantsByInitiative).
- BUG-6 (reorder divergence) fixed structurally. BUG-5 (rotation) held
(500 rounds CLEAN).
### FEAT-3: initiative first-class entry (add + edit)
- Current: only initMod at char-build. No initiative field at add-participant
or edit. 3-step to set after other steps.
- Need: initiative field at add-char, add-monster, AND edit participant.
- Separate design + RED. Own work item.
- Related: tie-break = drag order (current, works). Expose clearly.
### FEAT-1: Dead participants stay in turn order --- DONE
- Fixed: `applyHpChange` no longer flips `isActive` or touches `turnOrderIds`
on death/revive. Dead stay in rotation, `nextTurn` visits them, PCs get
death-save turn. `isActive` = DM toggle only.
- Tests: `shared/tests/turn.dead-skip.test.js` (4 green). Char tests updated
to new behavior.
### FEAT-2: upgrade app internal logs to be parseable
- Goal: combat logs in Firestore store enough structured state to run
skip/rotation analysis on ANY historic round --- not just replay stdout.
- Current logs: `{timestamp, message, encounterName, undo}`. Parser must
guess roster from message strings. Brittle.
- Upgrade: add structured fields at turn-state mutation log sites in
App.js (startEncounter, toggleActive, addParticipant, removeParticipant,
applyHpChange death/revive, togglePause, nextTurn):
```
turnSnapshot: { round, currentTurnParticipantId, turnOrderIds, activeIds }
```
- Then `scripts/analyze-turns.js` ingests app logs directly (adapter fetch).
Works on real game sessions, any round, deterministic.
- Parser scaffold NOW ingests replay stdout only (stopgap until FEAT-2).
## Confirmed bugs (tests written, NOT fixed)
### BUG-1: addParticipant + pause/resume corrupts turn rotation
- **RESOLVED** as side effect of BUG-2 fix (dup-id rejection broke chain).
- Audit: 0 violations / 100 rounds after BUG-2 fix.
- Replay: 10 rounds clean, no skip/dupe.
- Audit: 128 violations / 100 rounds, 4 symptom faces.
- Symptom chain (one bug family):
1. pause blocks nextTurn advance → totalTurns stays frozen (e.g. 120)
2. addParticipant re-adds same `r${totalTurns}` id (BUG-2: no dedup)
3. togglePause resume rebuilds turnOrderIds → dup id appears x2
4. nextTurn gets stuck on dup id → rotation breaks
5. eventually nextTurn throws 'Encounter not running'
- Symptom counts (audit-state.js, 100 rounds):
62x turnOrder-no-dup, 52x rotation-dupes, 14x nextTurn-throws
- Repro in replay round 10+: current stuck on one participant forever,
nextTurn returns same id, round never advances.
- Clean minimal repro (shared/tests/turn.pause-add.test.js) PASSES = combo
needs more state than single add+pause. Audit authoritative repro.
- Clean subsystems (zero violations): HP bounds, isActive, deathSave
range, conditions, removeParticipant orphans.
- Real repro = `node scripts/audit-state.js` (or audit-rotation.js).
### BUG-2: addParticipant allows duplicate id
- **FIXED** (commit: addParticipant throws on dup id).
- Test: `shared/tests/turn.characterization.test.js` 'addParticipant rejects
2026-06-30 16:30:26 -04:00
duplicate id' --- GREEN.
### bug-3 was a halucination has been removed
### BUG-4: hide-player-HP breaks display view (preexisting) --- PROD FIXED, TEST RED (mock bug)
- **Broader than hide-HP**: ALL 5 `storage.setDoc(getPath.activeDisplay(), ...)` calls
use `{merge:true}` which is IGNORED (setDoc = replace per contract).
Each write clobbers other fields on activeDisplay/status doc.
- line 1619: hide-HP toggle → clobbers campaignId+encounterId (display paused)
- line 1648: start combat → clobbers hidePlayerHp
- line 1779: end combat → clobbers hidePlayerHp
- line 1997: deactivate → clobbers hidePlayerHp
- line 2002: activate → clobbers hidePlayerHp
- Toggle "hide player HP" in admin → display view flips to "Game Session Paused".
- Toggling back does NOT recover. Must re-activate encounter in encounters
panel to restore display.
- Expected: hide-HP toggle updates one field on activeDisplay/status doc,
display stays live on current encounter.
- Likely cause: toggle writes to wrong path, or clobbers activeCampaignId/
activeEncounterId with null (setDoc replace vs updateDoc patch).
- Fix: use updateDoc (patch) not setDoc (replace); or include all existing
fields when writing.
- Status update (2026-07): all 5 sites now use `{merge:true}`. Real firebase
adapter honors merge → production works. BUT jsdom test still RED because
`src/__mocks__/firebase/firestore.js` setDoc records call, IGNORES opts
(no actual merge). Mock must simulate firebase merge semantics for test
to pass. Fix = mock setDoc: if opts.merge, MOCK_DB.merge(path,data) else
replace. OR change App.js setDoc(merge) → updateDoc (cleaner, ws adapter
uses PATCH). Decide which.
- Test: render App + DisplayView, toggle hide-HP, assert display still shows
encounter (not paused).
### BUG-5: mid-round addParticipant/revive corrupts rotation --- FIXED
- Fixed (commit `494327f`). Slot-array turn order + DRY advance core
`nextActiveAfter`. Both nextTurn + computeTurnOrderAfterRemoval delegate.
- 500-round replay: 0 skips, 0 double-acts.
### BUG-6: reorderParticipants doesn't update turnOrderIds --- FIXED
- Fixed structurally by 1-list model (commit 5d3a060). turnOrderIds =
participants.map(id) always. reorder cross-init allowed (DM override).
Display === rotation by construction.
- Test: `shared/tests/turn.reorder.test.js` 'reorder updates turnOrderIds' (RED).
- `reorderParticipants(enc, draggedId, targetId)` swaps two same-initiative
participants in `participants[]` array but leaves `turnOrderIds` unchanged.
- nextTurn rotates via `turnOrderIds` only → reorder has NO effect on combat
rotation. Mid-encounter drag-drop = pointless.
- replay-combat.js calls reorderParticipants with WRONG signature
2026-06-30 16:30:26 -04:00
`(enc, reorderedArray)` --- swallowed by try/catch, silent no-op. So
replay never exercised real path either.
- Fix: reorder must also update turnOrderIds to match new participant order
(within same-initiative tie).
### BUG-7: reorderParticipants has no undo
- Test: `shared/tests/turn.undo.test.js` 'reorderParticipants has no undo' (GREEN doc).
- `reorderParticipants` returns `log: null`. Other ops return `log.undo`.
- Cannot undo drag-drop. Candidate for undo system (M6).
### BUG-8: ws adapter has no reconnect
- Test: `server/tests/ws-reconnect.test.js` (RED).
- WS dies (idle/error/close) → `wsReady=null`, subscribers dead forever.
- Display frozen until full reload.
- Fix: `onclose` → reconnect + re-subscribe existing paths.
### BUG-10: deact+reactivate same round double-acts participant
- Discovered in 500-round replay (3 occurrences). DISTINCT from BUG-5.
- Pattern: participant acts → DM deactivates them → DM reactivates them
same round → `computeTurnOrderAfterAddition` re-inserts by initiative
(front) → acts AGAIN before round ends.
- No "acted-this-round" guard. Slot-array model has no per-round-acted set.
- Edge case (DM deact+reactivate same participant same round).
- Fix candidate: track actedThisRound set, skip re-acted; OR insertion
places reactivate AFTER current position (not by initiative).
- Parser now discounts deact-current advances, so this surfaced real.
### BUG-11: FE Combat.scenario test crashes (pre-existing)
- `src/tests/Combat.scenario.test.js:254` deathSave query helper throws
(button not found).
- Baseline (my changes removed) also exit=1. Pre-existing, not regression.
- Crashes whole FE test run (process dies).
### BUG-13: reorderParticipants crossing current pointer = ambiguous acted-semantics
- Discovered 7/1 replay. `reorderParticipants` (shared/turn.js:522) = pure
drag, no pointer logic. Swapping two actors across current pointer mid-round
= ambiguous who-acted-this-round. Earlier replay arbitrary swaps showed
skip/double (R9 Summon3 2x, R11 Goblin1 2x) before fix restricted swaps to
upcoming-only.
- Replay now avoids crossing (adjacent upcoming pair only, commit af165f4).
Real app untested: if DM drags actor past current pointer mid-round, skip/
double behavior undefined.
- Decide: block cross-pointer reorder, or define acted-semantics. RED needed.
### BUG-14: addParticipant init-insertion breaks after drag-reorder
- Discovered 7/1 replay. `computeTurnOrderAfterAddition` scans for first id
with init < addedInit, assumes list init-sorted. After drag, list NOT sorted
→ scan hits wrong slot.
- Trace turn 30→31: list `[Goblin1:20,Goblin2:22,...]` (drag moved Goblin1
before Goblin2). Add Reinforce3 init 21 → scan hits Goblin1:20 (idx 0, <21)
first → insert at 0. Should slot after Goblin2:22. WRONG.
- Root conflict: 1-list model = drag source of truth (no re-sort); addParticipant
= init-based insertion (needs sorted list). After ANY drag, add-insertion
meaningless.
- Proposed fix: append to end always (option A). DM drags to position. Matches
drag = source of truth. Makes `computeTurnOrderAfterAddition` trivial.
- Related: FEAT-3 (initiative first-class field).
2026-06-30 16:30:26 -04:00
## Pipeline (bugs only --- milestones live in REWORK_PLAN.md)
2026-06-30 16:22:13 -04:00
- [ ] BUG-4: fix setDoc→updateDoc for all 5 activeDisplay sites
- [x] BUG-5: fixed (1-list model, 500 rounds clean)
- [x] BUG-6: fixed structurally (1-list model)
- [x] BUG-12: fixed --- campaign selection follows activeDisplay
- [x] BUG-15: fixed --- DisplayView no longer re-sorts (drag order preserved)
- [x] BUG-8: ws adapter reconnect (implemented + GREEN)
- [ ] BUG-10: deact+reactivate double-act
- [ ] BUG-11: FE Combat.scenario crash
- [ ] BUG-13: reorder cross-pointer semantics (RED + decide block/allow)
- [ ] BUG-14: addParticipant init-insert breaks post-drag (append? + RED)