From ab9f8fa2e735299236acc6d83b1d20f939f7ce54 Mon Sep 17 00:00:00 2001 From: david raistrick <1108844+keen99@users.noreply.github.com> Date: Sat, 4 Jul 2026 17:22:52 -0400 Subject: [PATCH] fix(turn): log reorder + deathSave (BUG-7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- TODO.md | 10 +- shared/tests/turn.bug7.test.js | 54 +++++++++ shared/tests/turn.logging.test.js | 188 ++++++++++++++++++++++++++++++ shared/turn.js | 28 ++++- src/App.js | 8 +- 5 files changed, 281 insertions(+), 7 deletions(-) create mode 100644 shared/tests/turn.bug7.test.js create mode 100644 shared/tests/turn.logging.test.js diff --git a/TODO.md b/TODO.md index ed61b97..43cff87 100644 --- a/TODO.md +++ b/TODO.md @@ -4,9 +4,13 @@ Backlog of bugs + long-term items. Milestones live in REWORK_PLAN.md. ## Open bugs -### BUG-7: reorderParticipants has no undo -- `reorderParticipants` returns `log: null`. Other ops return `log.undo`. -- Cannot undo drag-drop. Candidate for undo system (M6). +### BUG-7: reorderParticipants not logged +- FIXED — now returns `log: { message, undo }`. Handler calls logAction. + Undo snapshots participants[] + turnOrderIds + pointer. +- Found second gap: deathSave also returned null log. Fixed (message + undo). +- Logging contract test added: turn.logging.test.js (all mutating ops logged, + no-ops null, undo payloads valid). Documents addParticipants + + updateParticipant gaps (null log, intentional). ## Open features diff --git a/shared/tests/turn.bug7.test.js b/shared/tests/turn.bug7.test.js new file mode 100644 index 0000000..d68fa41 --- /dev/null +++ b/shared/tests/turn.bug7.test.js @@ -0,0 +1,54 @@ +// BUG-7: reorderParticipants not logged. +// Every combat op returns { patch, log }. Handler calls logAction(log.message, +// ctx, { updates: log.undo }) → writes entry to logs collection. +// reorderParticipants returns log: null → handler skips logAction → drag +// invisible in combat log + no undo payload (siblings all have one). +// +// RED: prove log is null (bug). Fix: return { message, undo }. + +'use strict'; + +const shared = require('@ttrpg/shared'); +const { makeParticipant, reorderParticipants } = shared; + +function p(id, init) { + return makeParticipant({ id, name: id, type: 'monster', + initiative: init, maxHp: 100, currentHp: 100 }); +} +function enc(ps) { + return { name:'t', participants:ps, isStarted:false, isPaused:false, + round:0, currentTurnParticipantId:null, turnOrderIds:[] }; +} + +describe('BUG-7: reorderParticipants logged', () => { + test('returns log object (not null) with message + undo', () => { + const e = enc([p('a', 10), p('b', 10), p('c', 10)]); + const r = reorderParticipants(e, 'c', 'b'); + expect(r.patch).not.toBeNull(); + expect(r.log).not.toBeNull(); + expect(typeof r.log.message).toBe('string'); + expect(r.log.message.length).toBeGreaterThan(0); + expect(r.log.undo).toBeDefined(); + }); + + test('undo restores original participants order', () => { + const e = enc([p('a', 10), p('b', 10), p('c', 10)]); + const orig = e.participants.map(p => p.id); + const r = reorderParticipants(e, 'c', 'b'); + const restored = { ...e, ...r.log.undo }; + expect(restored.participants.map(p => p.id)).toEqual(orig); + }); + + test('no-op (same id) returns null log', () => { + const e = enc([p('a', 10), p('b', 10)]); + const r = reorderParticipants(e, 'a', 'a'); + expect(r.log).toBeNull(); + }); + + test('no-op (cross-init blocked) returns null log', () => { + const e = enc([p('a', 10), p('b', 5)]); + const r = reorderParticipants(e, 'a', 'b'); + expect(r.patch).toBeNull(); + expect(r.log).toBeNull(); + }); +}); diff --git a/shared/tests/turn.logging.test.js b/shared/tests/turn.logging.test.js new file mode 100644 index 0000000..ecb3e82 --- /dev/null +++ b/shared/tests/turn.logging.test.js @@ -0,0 +1,188 @@ +// Logging contract: every mutating combat op returns { patch, log } where +// log = { message: string, undo: object|null }. No-op (no state change) +// returns { patch: null, log: null }. Display lifecycle ops return { patch } +// only (no log field expected). +// +// logAction handler does: +// if (log) logAction(log.message, ctx, { updates: log.undo }) +// → null log = skipped = gap in audit trail. +// +// Contract = every combat mutation MUST produce a log entry. + +'use strict'; + +const shared = require('@ttrpg/shared'); +const { + makeParticipant, buildMonsterParticipant, + startEncounter, nextTurn, togglePause, + addParticipant, addParticipants, updateParticipant, removeParticipant, + toggleParticipantActive, applyHpChange, deathSave, toggleCondition, + reorderParticipants, endEncounter, +} = shared; + +function p(id, init) { + return makeParticipant({ id, name: id, type: 'monster', + initiative: init, maxHp: 100, currentHp: 100 }); +} +function enc(ps, extra = {}) { + return { name:'t', participants:ps, isStarted:false, isPaused:false, + round:0, currentTurnParticipantId:null, turnOrderIds:[], ...extra }; +} +const apply = (e, r) => r && r.patch ? { ...e, ...r.patch } : e; + +// Helper: assert a result is a logged mutation (patch + valid log). +function expectLogged(r) { + expect(r.patch).not.toBeNull(); + expect(r.log).not.toBeNull(); + expect(typeof r.log).toBe('object'); + expect(typeof r.log.message).toBe('string'); + expect(r.log.message.length).toBeGreaterThan(0); +} +function expectNoOp(r) { + expect(r.patch).toBeNull(); + expect(r.log).toBeNull(); +} + +describe('Logging contract: mutating ops', () => { + let e; + beforeEach(() => { + e = enc([p('a', 10), p('b', 7), p('c', 3)]); + }); + + test('startEncounter logs', () => { + const r = startEncounter(e); + expectLogged(r); + }); + + test('nextTurn logs', () => { + const started = apply(e, startEncounter(e)); + const r = nextTurn(started); + expectLogged(r); + }); + + test('togglePause logs', () => { + const r = togglePause(apply(e, startEncounter(e))); + expectLogged(r); + }); + + test('addParticipant logs', () => { + const r = addParticipant(e, p('d', 5)); + expectLogged(r); + }); + + test('removeParticipant logs', () => { + const r = removeParticipant(e, 'b'); + expectLogged(r); + }); + + test('toggleParticipantActive logs', () => { + const r = toggleParticipantActive(e, 'b'); + expectLogged(r); + }); + + test('applyHpChange (damage) logs', () => { + const r = applyHpChange(e, 'b', 'damage', 10); + expectLogged(r); + }); + + test('applyHpChange (heal) logs', () => { + const r = applyHpChange(e, 'b', 'heal', 5); + expectLogged(r); + }); + + test('deathSave logs', () => { + const started = apply(e, startEncounter(e)); + const dying = apply(started, applyHpChange(started, 'b', 'damage', 100)); + const r = deathSave(dying, 'b', 1); + expectLogged(r); + }); + + test('toggleCondition logs', () => { + const r = toggleCondition(e, 'b', 'poisoned'); + expectLogged(r); + }); + + test('reorderParticipants logs (BUG-7)', () => { + // same-init tie (both 10) for valid reorder + const e2 = enc([p('a', 10), p('x', 10), p('c', 3)]); + const r = reorderParticipants(e2, 'x', 'a'); + expectLogged(r); + }); + + test('endEncounter logs', () => { + const started = apply(e, startEncounter(e)); + const r = endEncounter(started); + expectLogged(r); + }); +}); + +describe('Logging contract: no-ops', () => { + let e; + beforeEach(() => { + e = enc([p('a', 10), p('b', 7), p('c', 3)]); + }); + + test('reorder same-id = no-op', () => { + expectNoOp(reorderParticipants(e, 'a', 'a')); + }); + + test('reorder cross-init = no-op', () => { + expectNoOp(reorderParticipants(e, 'a', 'b')); + }); +}); + +describe('Logging undo payloads', () => { + let e; + beforeEach(() => { + e = enc([p('a', 10), p('b', 7), p('c', 3)]); + }); + + test('startEncounter undo restores pre-combat state', () => { + const r = startEncounter(e); + expect(r.log.undo).toBeDefined(); + expect(r.log.undo.isStarted).toBe(false); + }); + + test('endEncounter undo restores combat state', () => { + const started = apply(e, startEncounter(e)); + const r = endEncounter(started); + expect(r.log.undo).toBeDefined(); + expect(r.log.undo.isStarted).toBe(true); + }); + + test('applyHpChange undo restores prior hp', () => { + const r = applyHpChange(e, 'b', 'damage', 10); + expect(r.log.undo).toBeDefined(); + expect(r.log.undo.participants).toBeDefined(); + }); + + test('reorder undo restores prior order (BUG-7)', () => { + const e2 = enc([p('a', 10), p('x', 10), p('c', 3)]); + const orig = e2.participants.map(p => p.id); + const r = reorderParticipants(e2, 'x', 'a'); + expect(r.log.undo).toBeDefined(); + const restored = { ...e2, ...r.log.undo }; + expect(restored.participants.map(p => p.id)).toEqual(orig); + }); +}); + +describe('Logging: gaps (currently unlogged mutations)', () => { + // Document current behavior. addParticipants + updateParticipant return + // null log → invisible in combat log. May be intended (bulk/no-op) or bug. + let e; + beforeEach(() => { + e = enc([p('a', 10), p('b', 7)]); + }); + + test('addParticipants returns null log (bulk gap?)', () => { + const r = addParticipants(e, [p('c', 3)]); + // Currently null. Document so decision is explicit. + expect(r.log).toBeNull(); + }); + + test('updateParticipant returns null log (edit gap?)', () => { + const r = updateParticipant(e, 'b', { name: 'B' }); + // Currently null. Document so decision is explicit. + expect(r.log).toBeNull(); + }); +}); diff --git a/shared/turn.js b/shared/turn.js index f57b102..5415af2 100644 --- a/shared/turn.js +++ b/shared/turn.js @@ -477,6 +477,8 @@ function deathSave(encounter, participantId, saveNumber) { if (!participant) throw new Error('Participant not found.'); const currentSaves = participant.deathSaves || 0; const newSaves = currentSaves === saveNumber ? saveNumber - 1 : saveNumber; + const name = participant.name; + const undo = { participants: encounter.participants || [] }; if (newSaves === 3) { // Mark dying — caller waits for animation, then calls removeParticipant. @@ -485,7 +487,10 @@ function deathSave(encounter, participantId, saveNumber) { ); return { patch: { participants: updatedParticipants }, - log: null, + log: { + message: `${name} failed 3 death saves — dying`, + undo, + }, isDying: true, }; } @@ -493,7 +498,14 @@ function deathSave(encounter, participantId, saveNumber) { const updatedParticipants = (encounter.participants || []).map(p => p.id === participantId ? { ...p, deathSaves: newSaves } : p ); - return { patch: { participants: updatedParticipants }, log: null, isDying: false }; + return { + patch: { participants: updatedParticipants }, + log: { + message: `${name} death save: ${newSaves}/3`, + undo, + }, + isDying: false, + }; } // TOGGLE_CONDITION — verbatim from ParticipantManager.toggleCondition @@ -553,7 +565,17 @@ function reorderParticipants(encounter, draggedId, targetId) { const newTargetIndex = participants.findIndex(p => p.id === targetId); participants.splice(newTargetIndex, 0, removedItem); const turnUpdates = syncTurnOrder(participants); // 1-list: always mirror - return { patch: { participants, ...turnUpdates }, log: null }; + return { + patch: { participants, ...turnUpdates }, + log: { + message: `${removedItem.name} reordered before ${(participants.find(p => p.id === targetId) || {}).name || targetId}`, + undo: { + participants: encounter.participants || [], + turnOrderIds: encounter.turnOrderIds || [], + currentTurnParticipantId: encounter.currentTurnParticipantId ?? null, + }, + }, + }; } // END_ENCOUNTER — verbatim from InitiativeControls.confirmEndEncounter diff --git a/src/App.js b/src/App.js index f25b10c..186116c 100644 --- a/src/App.js +++ b/src/App.js @@ -1084,8 +1084,14 @@ function ParticipantManager({ encounter, encounterPath, campaignCharacters }) { return; } try { - const { patch } = reorderParticipants(encounter, draggedItemId, targetId); + const { patch, log } = reorderParticipants(encounter, draggedItemId, targetId); await storage.updateDoc(encounterPath, patch); + if (log) { + logAction(log.message, { encounterName: encounter.name }, { + encounterPath, + updates: log.undo, + }); + } } catch (err) { // drag invalid (id not found) — ignore }