d00cc104c9
Reslot (stable sort by init desc, tie-break = original array index) now
fires on all 4 paths that can change order:
1. Add participant — sortParticipantsByInitiative([...parts, new], parts)
2. Edit modal save (handleUpdateParticipant) — reslot + syncTurnOrder
3. Drag reorder — splice move (already correct, untouched)
4. Inline init field — reslot (already committed 08c27c1)
Before: add appended (ignored init), edit modal overwrote value without
moving slot. Both caused list order to drift from init order until
startEncounter (sorts once). Now any init change immediately reslots
into correct position. Display + AdminView reflect order.
Stable sort preserves drag order within ties (tie-break = original index
= reflects prior drag). Move-one semantics: only changed element moves.
EditParticipantModal: added htmlFor/id link on Initiative label (was
missing — a11y + testable).
Tests: ReslotAllPaths.test.js (2). RED first (add appended, edit modal
no reslot), green after impl.
198 lines
10 KiB
Markdown
198 lines
10 KiB
Markdown
# 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
|
|
duplicate id' --- GREEN.
|
|
|
|
### bug-3 was a halucination has been removed
|
|
|
|
### BUG-4: hide-player-HP breaks display view (preexisting)
|
|
- **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.
|
|
- 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
|
|
`(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).
|
|
|
|
## Pipeline (bugs only --- milestones live in REWORK_PLAN.md)
|
|
- [ ] 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)
|