fix(BUG-5): unify turn-advance core (DRY), 500 rounds skip-free

Extract shared nextActiveAfter() advance core. Both nextTurn and computeTurnOrderAfterRemoval delegate to it — single source of truth, eliminates drift risk where one path changes and the other doesn't. Previously two separate advance implementations computed the same target, but any future edit to one would silently desync deact-current advance from normal nextTurn advance. Replay (scripts/replay-combat.js): - Move turn-line print before mutations (event order = reality) - Emit [pointer X→Y] lines when a mutation advances currentTurnParticipantId - Emit [pointer X→Y wrap] when round bumps (removal-wrap case) - Skip pointer emission for nextTurn (label=null) — already logged via turn line Parser (scripts/analyze-turns.js): - Parse [pointer X→Y wrap] events - Credit pointer-target as acted (deact-current advance = turn pointer) - Wrap pointer credits NEXT round (not current) — fixes cross-round false skip - Drop currentRemoved special-case — pointer lines make skip check precise Tests: - shared/tests/turn.dry.test.js: 3 tests lock deact-current advance == nextTurn advance (mid-round, inactive-skipper, wrap+round-bump). RED catches future drift. Results: 500-round replay now 0 real skips, 0 double-acts (was 5+3). Shared suite: 79 green + 1 RED (BUG-6 reorder, intentional).
2026-07-01 14:22:02 -04:00
parent c72b88f8bb
commit 494327ff17
4 changed files with 160 additions and 63 deletions
@@ -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);
+  });
+});
@@ -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: {