diff --git a/scripts/analyze-turns.js b/scripts/analyze-turns.js index 7f80938..a3195c5 100644 --- a/scripts/analyze-turns.js +++ b/scripts/analyze-turns.js @@ -20,7 +20,7 @@ const fs = require('fs'); // ---------- parsing ---------- -const TURN_RE = /^\s*turn\s+(\d+)\s+\(round\s+(\d+)\):\s+(.+?)\s*$/; +const TURN_RE = /^\s*turn\s+(\d+)\s+\(round\s+(\d+)\):\s+(.+?)(?:\s*\|.*)?\s*$/; const DEACTIVATE_RE = /^\s*\[(?:deactivate)\s+(.+?)\]\s*$/; const REACTIVATE_RE = /^\s*\[(?:revive-reactivate|reactivate)\s+(.+?)\]\s*$/; const ADD_RE = /^\s*\[(?:add)\s+(.+?)\]\s*$/; @@ -29,6 +29,7 @@ const PAUSE_RE = /^\s*\[pause\]\s*$/; const RESUME_RE = /^\s*\[resume\]\s*$/; const ROUND_COMPLETE_RE = /^\s*---\s*round\s+(\d+)\s+complete/; const FIRST_RE = /^combat started:\s+round\s+\d+,\s+first=(.+?)\s*$/; +const POINTER_RE = /^\s*\[pointer\s+(.+?)→(.+?)( wrap)?\]\s*$/; function parseLine(line) { if (TURN_RE.test(line)) { @@ -45,6 +46,10 @@ function parseLine(line) { if (REMOVE_RE.test(line)) return { kind: 'remove', name: line.match(REMOVE_RE)[1].trim() }; if (PAUSE_RE.test(line)) return { kind: 'pause' }; if (RESUME_RE.test(line)) return { kind: 'resume' }; + if (POINTER_RE.test(line)) { + const m = line.match(POINTER_RE); + return { kind: 'pointer', from: m[1].trim(), to: m[2].trim(), wrap: m[3] === ' wrap' }; + } if (ROUND_COMPLETE_RE.test(line)) return { kind: 'round-complete', round: +line.match(ROUND_COMPLETE_RE)[1] }; return null; } @@ -81,6 +86,11 @@ function reconstruct(events) { active.delete(ev.name); const r = rounds.get(curRound) || rounds.set(curRound, { turns: [], events: [], complete: false }).get(curRound); r.events.push({ ...ev, idx: r.events.length }); + } else if (ev.kind === 'pointer') { + // wrap pointer advances to next round — credit there. + if (ev.wrap) curRound += 1; + const r = rounds.get(curRound) || rounds.set(curRound, { turns: [], events: [], complete: false }).get(curRound); + r.events.push({ ...ev, idx: r.events.length }); } else if (ev.kind === 'round-complete') { if (rounds.has(ev.round)) rounds.get(ev.round).complete = true; } @@ -107,57 +117,72 @@ function analyze(rounds) { // Re-run per-round with active-set carry-over across rounds (module scope). function analyzeRounds(rounds) { - // Carry active set forward round to round. Reset at round 1 from scratch. + // Carry active set + current-name forward round to round. let activeCarry = new Set(); + let currentCarry = null; const reports = []; const sortedRounds = [...rounds.entries()].sort((a, b) => a[0] - b[0]); for (const [roundN, r] of sortedRounds) { if (!r.complete) continue; // incomplete final round — can't judge skips - if (roundN === 1) activeCarry = new Set(); - const result = analyzeRoundWithCarry(roundN, r, activeCarry); + if (roundN === 1) { activeCarry = new Set(); currentCarry = null; } + const result = analyzeRoundWithCarry(roundN, r, activeCarry, currentCarry); reports.push(result.report); activeCarry = result.activeAfter; + currentCarry = result.currentAfter; } return reports; } -function analyzeRoundWithCarry(roundN, r, activeAtStart) { +// When current participant is deactivated/removed, code advances current to +// next active. That target gets the turn pointer = acts. Parser can't see +// roster/order from stdout, so on deact-current the NEXT turn actor is the +// advance target and is credited an extra "pointer turn" (not a logged turn). +function analyzeRoundWithCarry(roundN, r, activeAtStart, currentAtStart) { // activeAtStart: Set copy. Mutations during round adjust a working copy. const active = new Set(activeAtStart); const activeWholeRound = new Set(activeAtStart); // participants never toggled off/removed const addedThisRound = new Set(); - const turns = []; // ordered actor names + const turns = []; // ordered actor names (logged) + const pointerTurns = new Set(); // names that got the turn pointer this round + let current = currentAtStart; // current participant name (carry) for (const ev of r.events) { if (ev.kind === 'turn') { turns.push(ev.actor); + pointerTurns.add(ev.actor); if (!active.has(ev.actor)) active.add(ev.actor); // first-ever sighting + current = ev.actor; + } else if (ev.kind === 'pointer') { + // mutation advanced current pointer: ev.to now holds it = got the turn. + // Credit ev.to. Update tracking. + pointerTurns.add(ev.to); + current = ev.to; } else if (ev.kind === 'deactivate' || ev.kind === 'remove') { - active.delete(ev.name); + // deact/REMOVE of current → code auto-advances (emitted as pointer line). + // Disqualify from whole-round (roster mutation = not "whole round"). activeWholeRound.delete(ev.name); + active.delete(ev.name); } else if (ev.kind === 'reactivate' || ev.kind === 'add') { + activeWholeRound.delete(ev.name); active.add(ev.name); - if (ev.kind === 'add') addedThisRound.add(ev.name); - // reactivated = was not active at start, so not eligible for "whole round" - activeWholeRound.add(ev.name); // gives benefit of doubt; refined below } } - // acted = unique names that took a turn this round - const acted = new Set(turns); + // acted = names that took a turn OR got pointer via mutation-advance + // (deact/remove of current advances to target — that target acts). + // Pointer lines from replay tell us the target explicitly. + const acted = new Set([...turns, ...pointerTurns]); - // double-acts: turns with count > 1 + // double-acts: logged turns with count > 1 (pointer-credits excluded — + // a deact-advance target acting once via pointer then once via nextTurn + // is correct, not a bug). const counts = {}; for (const n of turns) counts[n] = (counts[n] || 0) + 1; const doubleActs = Object.entries(counts).filter(([_, c]) => c > 1).map(([n, c]) => ({ name: n, count: c })); - // real skip: active at round start AND active at round end AND never acted. - // (deactivated/removed mid-round = legitimate skip, not a bug) - // also must have been active at END (revived back doesn't count as skip). - // Simplest defn matching the unit test: activeAtStart ∩ activeAtEnd ∩ ¬acted. - const activeAtEnd = active; - const realSkips = [...activeAtStart] - .filter(n => activeAtEnd.has(n) && !acted.has(n)); + // real skip: active for WHOLE round (no roster mutation) AND never got + // turn/pointer. Mutations disqualify from whole-round already. + const realSkips = [...activeWholeRound].filter(n => !acted.has(n)); return { report: { @@ -168,7 +193,8 @@ function analyzeRoundWithCarry(roundN, r, activeAtStart) { doubleActs, turns, }, - activeAfter: activeAtEnd, + activeAfter: active, + currentAfter: current, }; } diff --git a/scripts/replay-combat.js b/scripts/replay-combat.js index cbe20f7..f2855da 100644 --- a/scripts/replay-combat.js +++ b/scripts/replay-combat.js @@ -65,7 +65,21 @@ async function patch(encounterPath, enc, result, label) { if (!result || !result.patch) { if (label) console.log(` (${label}: no-op)`); return enc; } await storage.updateDoc(encounterPath, result.patch); if (label) console.log(` [${label}]`); - return { ...enc, ...result.patch }; + // emit pointer-advance line when a MUTATION changes currentTurnParticipantId. + // nextTurn passes label=null — it's a normal advance, already logged via + // the turn line. Emitting pointer for it double-counts. + const oldCur = enc.currentTurnParticipantId; + const oldRound = enc.round; + const newEnc = { ...enc, ...result.patch }; + const newCur = newEnc.currentTurnParticipantId; + const newRound = newEnc.round; + if (label && oldCur && newCur && oldCur !== newCur) { + const oldName = enc.participants.find(p => p.id === oldCur)?.name || oldCur; + const newName = newEnc.participants.find(p => p.id === newCur)?.name || newCur; + const wrap = oldRound !== newRound ? ' wrap' : ''; + console.log(` [pointer ${oldName}→${newName}${wrap}]`); + } + return newEnc; } function pick(arr) { return arr[Math.floor(Math.random() * arr.length)]; } @@ -147,7 +161,9 @@ async function main() { const cap = (enc.participants.length + 2) * 2; let guard = 0; while (enc.round < roundN + 1 && guard < cap) { - enc = await storage.getDoc(encounterPath); + // NOTE: do NOT getDoc here — async re-fetch can return stale state and + // cause nextTurn to compute off pre-mutation data (double-acts/skips). + // Trust the local enc returned by patch (sync spread of updateDoc). // 9. resume if paused: must happen BEFORE nextTurn or it throws. if (lastPaused) { @@ -162,6 +178,8 @@ async function main() { const actorName = firstActiveName(enc); const actor = currentParticipant(enc); + console.log(` turn ${totalTurns} (round ${enc.round}): ${actorName} | order=[${enc.turnOrderIds.map(id=>enc.participants.find(p=>p.id===id)?.name||id).join(',')}] cur=${enc.currentTurnParticipantId}`); + // 1. damage: actor hits a random living, active target. if (actor) { const foes = enc.participants.filter( @@ -300,7 +318,6 @@ async function main() { } } - console.log(` turn ${totalTurns} (round ${enc.round}): ${actorName}`); await sleep(DELAY); guard++; if (!enc.isStarted) { console.log('combat auto-ended'); break; } diff --git a/shared/tests/turn.dry.test.js b/shared/tests/turn.dry.test.js new file mode 100644 index 0000000..1438dca --- /dev/null +++ b/shared/tests/turn.dry.test.js @@ -0,0 +1,52 @@ +// DRY guard (BUG-5 fix): nextTurn and computeTurnOrderAfterRemoval share one +// advance core (nextActiveAfter). Both must pick the SAME next-active target +// for identical state. If this goes RED, the two paths drifted. + +'use strict'; + +const shared = require('@ttrpg/shared'); +const { makeParticipant, startEncounter, nextTurn, toggleParticipantActive } = shared; + +function p(id, init, extra = {}) { + return makeParticipant({ id, name: id, type: 'monster', + initiative: init, maxHp: 100, currentHp: 100, ...extra }); +} +function enc(ps, extra = {}) { + return { name:'t', participants:ps, isStarted:false, isPaused:false, + round:0, currentTurnParticipantId:null, turnOrderIds:[], ...extra }; +} + +describe('DRY: deact-current advance == nextTurn advance', () => { + test('mid-round: same target (not current)', () => { + // order a,b,c. a current. nextTurn → b. deact a → advance → b. + const e = enc([p('a',10),p('b',5),p('c',3)], { isStarted:true, + turnOrderIds:['a','b','c'], currentTurnParticipantId:'a' }); + const nt = nextTurn(e).patch.currentTurnParticipantId; + const deact = toggleParticipantActive(e, 'a').patch.currentTurnParticipantId; + expect(deact).toBe(nt); + expect(deact).toBe('b'); + }); + + test('mid-round with inactive skipper: same target', () => { + // order a,x,b,c; x inactive. a current. nextTurn → b. deact a → b. + const x = p('x',7,{ isActive:false }); + const e = enc([p('a',10),x,p('b',5),p('c',3)], { isStarted:true, + turnOrderIds:['a','x','b','c'], currentTurnParticipantId:'a' }); + const nt = nextTurn(e).patch.currentTurnParticipantId; + const deact = toggleParticipantActive(e, 'a').patch.currentTurnParticipantId; + expect(deact).toBe(nt); + expect(deact).toBe('b'); + }); + + test('wrap: same target + round bump', () => { + // order a,b,c. c current. nextTurn → wrap → a (r+1). deact c → wrap → a (r+1). + const e = enc([p('a',10),p('b',5),p('c',3)], { isStarted:true, + turnOrderIds:['a','b','c'], currentTurnParticipantId:'c', round:2 }); + const nt = nextTurn(e).patch; + const deact = toggleParticipantActive(e, 'c').patch; + expect(deact.currentTurnParticipantId).toBe(nt.currentTurnParticipantId); + expect(deact.currentTurnParticipantId).toBe('a'); + expect(deact.round).toBe(nt.round); + expect(deact.round).toBe(3); + }); +}); diff --git a/shared/turn.js b/shared/turn.js index 57c7835..89d94f8 100644 --- a/shared/turn.js +++ b/shared/turn.js @@ -43,6 +43,27 @@ const sortParticipantsByInitiative = (participants, originalOrder) => { }); }; +// SHARED ADVANCE CORE (BUG-5 DRY fix). +// Single source of truth for "who acts next". Both nextTurn and +// computeTurnOrderAfterRemoval delegate here — prevents drift where one path +// changes and the other doesn't. +// +// order: turnOrderIds (raw, may contain inactive/removed ids). +// fromPos: index of the last-acted slot (current participant, or the removed +// participant's old slot). Step +1 forward, skip fromPos itself. +// isActive: predicate id -> bool. +// Returns { nextId, wrapped }. wrapped = cycled past order end = new round. +const nextActiveAfter = (order, fromPos, isActive) => { + const n = order.length; + if (n === 0) return { nextId: null, wrapped: false }; + for (let step = 1; step < n; step++) { + const idx = (fromPos + step) % n; + const id = order[idx]; + if (isActive(id)) return { nextId: id, wrapped: idx <= fromPos }; + } + return { nextId: null, wrapped: false }; // no other active participant +}; + // Verbatim from src/App.js. Returns turnOrderIds/currentTurnParticipantId updates // when a participant leaves active combat. const computeTurnOrderAfterRemoval = (encounter, removedId, updatedParticipants) => { @@ -52,22 +73,12 @@ const computeTurnOrderAfterRemoval = (encounter, removedId, updatedParticipants) const updates = { turnOrderIds: newIds }; if (encounter.currentTurnParticipantId === removedId) { const removedPos = currentIds.indexOf(removedId); - // first try next-active AFTER removed (same round, no wrap) - const after = currentIds.slice(removedPos + 1); - const nextSameRound = after.find(id => - updatedParticipants.find(p => p.id === id && p.isActive)); - if (nextSameRound) { - updates.currentTurnParticipantId = nextSameRound; - } else { - // wrap: no active after removed → advance to first active at top of - // order AND bump round. Without the bump, nextTurn sees current already - // at order[0] and replays the whole round (BUG-5). - const before = currentIds.slice(0, removedPos); - const nextId = before.find(id => - updatedParticipants.find(p => p.id === id && p.isActive)) ?? null; - updates.currentTurnParticipantId = nextId; - if (nextId) updates.round = (encounter.round || 1) + 1; - } + const isActive = id => updatedParticipants.find(p => p.id === id && p.isActive); + // Delegate to shared core: advance from removed's old slot. Same math + // nextTurn uses → no drift. + const { nextId, wrapped } = nextActiveAfter(currentIds, removedPos, isActive); + updates.currentTurnParticipantId = nextId; + if (nextId && wrapped) updates.round = (encounter.round || 1) + 1; } return updates; }; @@ -220,35 +231,26 @@ function nextTurn(encounter) { }; } - let currentIndex = activePsInOrder.findIndex(p => p.id === encounter.currentTurnParticipantId); let nextRound = encounter.round; - - // Current participant was removed; find next after their old position in turnOrderIds. - if (currentIndex === -1) { - const rawPos = (encounter.turnOrderIds || []).indexOf(encounter.currentTurnParticipantId); - const candidateIds = [ - ...(encounter.turnOrderIds || []).slice(rawPos + 1), - ...(encounter.turnOrderIds || []).slice(0, rawPos), - ]; - const nextP = candidateIds.map(id => activePsInOrder.find(p => p.id === id)).find(Boolean); - currentIndex = nextP ? activePsInOrder.findIndex(p => p.id === nextP.id) - 1 : -1; - } - - let nextIndex = (currentIndex + 1) % activePsInOrder.length; let newTurnOrderIds = encounter.turnOrderIds; - // Round wrap: initiative is cyclic. Order is frozen at startEncounter and - // patched incrementally by add/remove/toggle. NO re-sort here — re-sorting - // displaces the current pointer and causes skips. - if (nextIndex === 0 && currentIndex !== -1) { - nextRound += 1; - } + // Delegate to shared advance core (BUG-5 DRY fix). Same math + // computeTurnOrderAfterRemoval uses → no drift. fromPos = current's slot + // in raw turnOrderIds; -1 path handles removed/stale current. + const order = encounter.turnOrderIds || []; + const fromPos = order.indexOf(encounter.currentTurnParticipantId); + const isActive = id => { + const p = encounter.participants.find(x => x.id === id); + return !!p && p.isActive; + }; + const { nextId, wrapped } = nextActiveAfter(order, fromPos, isActive); - const nextParticipant = activePsInOrder[nextIndex]; - - if (!nextParticipant) { + if (!nextId) { throw new Error('Could not determine next participant.'); } + if (wrapped) nextRound += 1; + + const nextParticipant = encounter.participants.find(p => p.id === nextId); return { patch: {