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.
This commit is contained in:
david raistrick
@@ -1,76 +1,35 @@
|
||||
# TODO
|
||||
|
||||
Backlog of bugs + long-term items, from user. Milestones live in
|
||||
REWORK_PLAN.md.
|
||||
Backlog of bugs + long-term items. Milestones live in REWORK_PLAN.md.
|
||||
|
||||
## Feature backlog
|
||||
## Open bugs
|
||||
|
||||
TODO: FIX: test for warnings. any warnings in tests, or compile or runtime == failure. like perl and php used to ahve. warning == error. we should lways solve them
|
||||
### BUG-7: reorderParticipants has no undo
|
||||
- `reorderParticipants` returns `log: null`. Other ops return `log.undo`.
|
||||
- Cannot undo drag-drop. Candidate for undo system (M6).
|
||||
|
||||
TODO: make tests have integrated timeout - even 60s is probabvly too long, should not take long.
|
||||
### 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 → 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).
|
||||
|
||||
TODO: combat.scenario doesnt do 100 rounds it does 100 turns. recover from kill-shared stash
|
||||
### BUG-13: reorderParticipants crossing current pointer = ambiguous acted-semantics
|
||||
- `reorderParticipants` (shared/turn.js) = pure drag, no pointer logic.
|
||||
- Swapping two actors across current pointer mid-round = ambiguous
|
||||
who-acted-this-round. Replay avoids crossing (adjacent upcoming pair only).
|
||||
- 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.
|
||||
|
||||
|
||||
|
||||
TODO: death saves wrong
|
||||
|
||||
TODO: custom condition field
|
||||
|
||||
|
||||
|
||||
|
||||
server = YOUR SQLite server. server/db.js = better-sqlite3. src/storage/server.js = client that talks to it (REST +
|
||||
websocket). Name bad. Should be "sqlite-adapter" or "server". You right.
|
||||
|
||||
memory = in-process JS Map. Useless? Mostly yes for prod. Value: fast contract tests, no server spinup. If
|
||||
contract test runs vs firebase-mock + live server backend.
|
||||
|
||||
Memory = optional convenience. === extra complication.
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
### 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.
|
||||
## Open features
|
||||
|
||||
### 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.
|
||||
- Goal: combat logs in storage 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
|
||||
@@ -81,172 +40,91 @@ Memory = optional convenience. === extra complication.
|
||||
```
|
||||
- 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)
|
||||
### 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.
|
||||
- Related: tie-break = drag order (current, works). Expose clearly.
|
||||
|
||||
### 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).
|
||||
|
||||
### Death saves: need fails counter
|
||||
- Current: saves only (`deathSaves` counter, 3 saves = revive). No fails.
|
||||
- D&D 5e: 3 saves = stable, 3 fails = dead. Track both.
|
||||
|
||||
### Custom condition field
|
||||
- Conditions hardcoded list. No way for DM to add custom.
|
||||
|
||||
### Test integrated timeouts
|
||||
- No per-test timeout. Tests hang on regression. Add jest timeout (<60s).
|
||||
|
||||
## Done (history)
|
||||
|
||||
### Architecture: 1-list turn order model
|
||||
- Single source: turnOrderIds === participants.map(id). No re-sort after
|
||||
startEncounter. nextTurn skips inactive (predicate), inactive stay in slot.
|
||||
- Drag (reorder) = same-init tie-break only. Cross-init blocked.
|
||||
- startEncounter sorts ALL participants by init once, then frozen.
|
||||
- addParticipant/updateParticipant slot by init (slotIndexForInit), preserve
|
||||
drag order. Display renders participants[] directly (no sort).
|
||||
- Static guard test errs if `.sort(` reintroduced.
|
||||
|
||||
### 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).
|
||||
- RESOLVED as side effect of BUG-2 fix.
|
||||
|
||||
### 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.
|
||||
- FIXED (addParticipant throws on dup id).
|
||||
|
||||
### bug-3 was a halucination has been removed
|
||||
### BUG-4: hide-player-HP breaks display view
|
||||
- FIXED — mock honors setDoc{merge}, all 5 activeDisplay sites use merge.
|
||||
|
||||
### 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, server 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 — slot-array + DRY advance core `nextActiveAfter`.
|
||||
|
||||
### 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-6: reorderParticipants doesn't update turnOrderIds
|
||||
- FIXED structurally by 1-list model.
|
||||
|
||||
### BUG-8: server adapter has no reconnect
|
||||
- Test: `server/tests/server-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.
|
||||
- FIXED — onclose reconnects + re-subscribes 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
|
||||
- FIXED — moved to shared/turn.combat.test.js, pure functions, 100 rounds.
|
||||
|
||||
### 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-12: campaign selection follows activeDisplay
|
||||
- FIXED.
|
||||
|
||||
### 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).
|
||||
- FIXED — slotIndexForInit scans current list (post-drag aware).
|
||||
|
||||
## Pipeline (bugs only --- milestones live in REWORK_PLAN.md)
|
||||
### BUG-15: DisplayView re-sorts (drag order not preserved)
|
||||
- FIXED — display renders participants[] directly.
|
||||
|
||||
### BUG-16: subscribeCollection hook drops queryConstraints
|
||||
- SS `useFirestoreCollection` (App.js ~L233) calls
|
||||
`storage.subscribeCollection(collectionPath, cb)` --- NO constraints passed.
|
||||
- main (L251-253): `const q = query(collection(db,collectionPath), ...queryConstraints); onSnapshot(q, ...)`.
|
||||
- Effect: `LOG_QUERY = [orderBy('timestamp','desc'), limit(500)]` IGNORED. Logs
|
||||
unsorted + unbounded. Other collections unaffected (no constraints passed
|
||||
elsewhere) but interface broken.
|
||||
- firebase adapter `subscribeCollection(path, cb, queryConstraints=[])` HAS the
|
||||
param, hook just doesn't forward it.
|
||||
- Fix: hook pass `queryConstraints` as 3rd arg. RED first.
|
||||
- FIXED — neutral builders, both adapters honor orderBy/limit.
|
||||
|
||||
### BUG-17: dead SDK imports in App.js
|
||||
- L5 imports `doc,setDoc,collection,onSnapshot,writeBatch,query,getDocs` from
|
||||
`./storage` but App never calls them raw (all via `storage.*`). Only used as
|
||||
re-export passthrough. `getFirestore`,`initializeApp` still used in init flow.
|
||||
- Noise, not behavioral. Cleanup: trim dead imports OR keep for adapter test
|
||||
instrumentation. Decide after audit.
|
||||
- FIXED — trimmed (auth + getFirestore + getStorage remain).
|
||||
|
||||
### BUG-18: stale comments reference deleted memory adapter + phantom Phase B
|
||||
- `src/storage/firebase.js` header had Phase A/B comment (DELETED this session).
|
||||
- `src/storage/index.js` L3: "per-group refactor pending" --- stale.
|
||||
- `src/tests/StorageEsm.test.js`, `storage.factory.test.js` referenced memory
|
||||
(FIXED this session). Re-audit for stragglers.
|
||||
### BUG-18: stale comments reference deleted memory adapter
|
||||
- FIXED.
|
||||
|
||||
- [ ] 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: server 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)
|
||||
### FEAT-1: Dead participants stay in turn order
|
||||
- DONE — applyHpChange no longer flips isActive on death. Dead stay in
|
||||
rotation, nextTurn visits them, PCs get death-save turn.
|
||||
|
||||
### feat: add all characters to participants list
|
||||
- DONE — addParticipants bulk add wired (App.js:943).
|
||||
|
||||
### Warning = failure in tests
|
||||
- DONE — console.error/warn throw in test env.
|
||||
|
||||
### combat.scenario 100 rounds not turns
|
||||
- DONE — loops by actual round-wrap count.
|
||||
|
||||
Reference in New Issue
Block a user