fix(turn): block cross-pointer reorder (BUG-13)
reorderParticipants allowed dragging across current pointer mid-round. nextTurn walks turnOrderIds forward from pointer, so cross-pointer drag = ambiguous who-acts-next: upcoming dragged ahead of pointer = skipped (walk wraps past), acted dragged behind pointer = double-act. Full fix needs actedThisRound tracking. Pragmatic now: block both directions during active encounter. Pre-combat: free reorder (no pointer). reorderParticipants: added pointer check (currentTurn index). If drag crosses pointer, return null patch (no-op). Same-side reorder allowed. syncTurnOrder now always mirrors (was started-only — pre-combat reorder left turnOrderIds stale). Tests: - turn.bug13.test.js: cross-pointer blocked (both dirs), same-side allowed, pre-combat free. - turn.reorder.test.js: 3 tests updated to pre-combat (cross-pointer now blocked post-start). - turn.invariant.test.js: reorder test uses same-side upcoming pair. 214 tests green.
This commit is contained in:
@@ -8,19 +8,11 @@ Backlog of bugs + long-term items. Milestones live in REWORK_PLAN.md.
|
||||
- `reorderParticipants` returns `log: null`. Other ops return `log.undo`.
|
||||
- Cannot undo drag-drop. Candidate for undo system (M6).
|
||||
|
||||
### 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.
|
||||
|
||||
## Open features
|
||||
|
||||
### FEAT-2: upgrade app internal logs to be parseable
|
||||
- Goal: combat logs in storage store enough structured state to run
|
||||
skip/rotation analysis on ANY historic round — not just replay stdout.
|
||||
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
|
||||
@@ -32,11 +24,6 @@ Backlog of bugs + long-term items. Milestones live in REWORK_PLAN.md.
|
||||
- Then `scripts/analyze-turns.js` ingests app logs directly (adapter fetch).
|
||||
Works on real game sessions, any round, deterministic.
|
||||
|
||||
### 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)`.
|
||||
@@ -57,6 +44,12 @@ Backlog of bugs + long-term items. Milestones live in REWORK_PLAN.md.
|
||||
### Test integrated timeouts
|
||||
- No per-test timeout. Tests hang on regression. Add jest timeout (<60s).
|
||||
|
||||
### add participants duplication throws a alert but it disappears fast
|
||||
|
||||
### add particopents list dropdown - remove ones already added
|
||||
|
||||
|
||||
|
||||
## Done (history)
|
||||
|
||||
### Architecture: 1-list turn order model
|
||||
@@ -75,51 +68,62 @@ Backlog of bugs + long-term items. Milestones live in REWORK_PLAN.md.
|
||||
- FIXED (addParticipant throws on dup id).
|
||||
|
||||
### BUG-4: hide-player-HP breaks display view
|
||||
- FIXED — mock honors setDoc{merge}, all 5 activeDisplay sites use merge.
|
||||
- FIXED --- mock honors setDoc{merge}, all 5 activeDisplay sites use merge.
|
||||
|
||||
### BUG-5: mid-round addParticipant/revive corrupts rotation
|
||||
- FIXED — slot-array + DRY advance core `nextActiveAfter`.
|
||||
- FIXED --- slot-array + DRY advance core `nextActiveAfter`.
|
||||
|
||||
### BUG-6: reorderParticipants doesn't update turnOrderIds
|
||||
- FIXED structurally by 1-list model.
|
||||
|
||||
### BUG-8: server adapter has no reconnect
|
||||
- FIXED — onclose reconnects + re-subscribes existing paths.
|
||||
- FIXED --- onclose reconnects + re-subscribes existing paths.
|
||||
|
||||
### BUG-10: deact+reactivate same round double-acts participant
|
||||
- FIXED — 1-list model keeps slot position on toggle. Reactivate does not
|
||||
- FIXED --- 1-list model keeps slot position on toggle. Reactivate does not
|
||||
grant second turn. Test: turn.bug10.test.js.
|
||||
|
||||
### BUG-11: FE Combat.scenario test crashes
|
||||
- FIXED — moved to shared/turn.combat.test.js, pure functions, 100 rounds.
|
||||
- FIXED --- moved to shared/turn.combat.test.js, pure functions, 100 rounds.
|
||||
|
||||
### BUG-12: campaign selection follows activeDisplay
|
||||
- FIXED.
|
||||
|
||||
### BUG-13: reorderParticipants crossing current pointer = ambiguous acted-semantics
|
||||
- FIXED --- block cross-pointer reorder during active encounter (both dirs).
|
||||
Full fix needs actedThisRound tracking. Pragmatic block prevents skip/double.
|
||||
Pre-combat: free reorder.
|
||||
|
||||
### BUG-14: addParticipant init-insertion breaks after drag-reorder
|
||||
- FIXED — slotIndexForInit scans current list (post-drag aware).
|
||||
- FIXED --- slotIndexForInit scans current list (post-drag aware).
|
||||
|
||||
### BUG-15: DisplayView re-sorts (drag order not preserved)
|
||||
- FIXED — display renders participants[] directly.
|
||||
- FIXED --- display renders participants[] directly.
|
||||
|
||||
### BUG-16: subscribeCollection hook drops queryConstraints
|
||||
- FIXED — neutral builders, both adapters honor orderBy/limit.
|
||||
- FIXED --- neutral builders, both adapters honor orderBy/limit.
|
||||
|
||||
### BUG-17: dead SDK imports in App.js
|
||||
- FIXED — trimmed (auth + getFirestore + getStorage remain).
|
||||
- FIXED --- trimmed (auth + getFirestore + getStorage remain).
|
||||
|
||||
### BUG-18: stale comments reference deleted memory adapter
|
||||
- FIXED.
|
||||
|
||||
### FEAT-1: Dead participants stay in turn order
|
||||
- DONE — applyHpChange no longer flips isActive on death. Dead stay in
|
||||
- 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).
|
||||
- DONE --- addParticipants bulk add wired (App.js:943).
|
||||
|
||||
### Warning = failure in tests
|
||||
- DONE — console.error/warn throw in test env.
|
||||
- DONE --- console.error/warn throw in test env.
|
||||
|
||||
### combat.scenario 100 rounds not turns
|
||||
- DONE — loops by actual round-wrap count.
|
||||
- DONE --- loops by actual round-wrap count.
|
||||
|
||||
### FEAT-3: initiative first-class entry (add + edit) DONE
|
||||
- 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.
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
// BUG-13: cross-pointer reorder blocked (skip/double-act otherwise).
|
||||
// nextTurn walks turnOrderIds forward from current pointer. Dragging across
|
||||
// pointer = ambiguous who-acts-next:
|
||||
// - upcoming dragged ahead of pointer → walk wraps past → SKIPPED
|
||||
// - acted dragged behind pointer → acts again → DOUBLE-ACT
|
||||
// Full fix needs actedThisRound tracking. Pragmatic now: block both dirs
|
||||
// during active encounter. Pre-combat: free reorder (no pointer).
|
||||
|
||||
'use strict';
|
||||
|
||||
const shared = require('@ttrpg/shared');
|
||||
const {
|
||||
makeParticipant,
|
||||
startEncounter, nextTurn, 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:[] };
|
||||
}
|
||||
const apply = (e, r) => r && r.patch ? { ...e, ...r.patch } : e;
|
||||
|
||||
describe('BUG-13: cross-pointer reorder blocked during active encounter', () => {
|
||||
test('dragging not-yet-acted participant ahead of pointer = no-op', () => {
|
||||
// [a(10), b(10), c(10)]. a acts -> b current. c not yet acted.
|
||||
let e = enc([p('a',10),p('b',10),p('c',10)]);
|
||||
e = apply(e, startEncounter(e)); // a (idx0, pointer)
|
||||
e = apply(e, nextTurn(e)); // b (idx1, pointer)
|
||||
expect(e.currentTurnParticipantId).toBe('b');
|
||||
|
||||
// Drag c (idx2, behind pointer) before b (idx1, pointer). Crosses pointer.
|
||||
// Would let c act by initiative-reinsert but nextTurn forward-walk skips.
|
||||
// Blocked instead.
|
||||
const r = reorderParticipants(e, 'c', 'b');
|
||||
expect(r.patch).toBeNull();
|
||||
expect(e.turnOrderIds).toEqual(['a','b','c']); // unchanged
|
||||
});
|
||||
|
||||
test('dragging already-acted participant behind pointer = no-op', () => {
|
||||
// [a(10), b(10), c(10)]. a acts (idx0, ahead of pointer).
|
||||
let e = enc([p('a',10),p('b',10),p('c',10)]);
|
||||
e = apply(e, startEncounter(e)); // a (pointer idx0)
|
||||
e = apply(e, nextTurn(e)); // b (pointer idx1)
|
||||
// Drag a (already acted, ahead of pointer) after c (behind pointer).
|
||||
// Crosses pointer. Would let a act again. Blocked.
|
||||
const r = reorderParticipants(e, 'a', 'c');
|
||||
expect(r.patch).toBeNull();
|
||||
});
|
||||
|
||||
test('reorder within same side of pointer = allowed', () => {
|
||||
// [a(10), b(10), c(10), d(10)]. a current (pointer idx0).
|
||||
// b,c,d all behind pointer (upcoming). Reorder among them = safe.
|
||||
let e = enc([p('a',10),p('b',10),p('c',10),p('d',10)]);
|
||||
e = apply(e, startEncounter(e)); // a (idx0)
|
||||
// swap b,c (both idx1,2 — behind pointer). No cross.
|
||||
const r = reorderParticipants(e, 'c', 'b');
|
||||
expect(r.patch).not.toBeNull();
|
||||
expect(r.patch.participants.map(p => p.id)).toEqual(['a','c','b','d']);
|
||||
});
|
||||
|
||||
test('pre-combat: free reorder (no pointer)', () => {
|
||||
// isStarted=false. No pointer. Reorder allowed freely.
|
||||
let e = enc([p('a',10),p('b',10),p('c',10)]);
|
||||
const r = reorderParticipants(e, 'c', 'a');
|
||||
expect(r.patch).not.toBeNull();
|
||||
expect(r.patch.participants.map(p => p.id)).toEqual(['c','a','b']);
|
||||
});
|
||||
});
|
||||
@@ -90,12 +90,15 @@ describe('1-list model: turnOrderIds === participants.map(id), no re-sort', () =
|
||||
expect(e.turnOrderIds).toEqual(['a','b','c']); // unchanged
|
||||
});
|
||||
|
||||
test('reorder same-init: rotation follows new list order', () => {
|
||||
let e = enc([p('a',10),p('b',10),p('c',3)]); // a,b tie
|
||||
e = apply(e, startEncounter(e)); // [a,b,c], cur=a
|
||||
e = apply(e, reorderParticipants(e, 'b', 'a')); // [b,a,c], cur still a
|
||||
const rot = walkRotation(e); // start a, next c (wrap), next b, back a
|
||||
expect(rot).toEqual(['a','c','b']);
|
||||
test('reorder same-init: within upcoming side of pointer follows new order', () => {
|
||||
let e = enc([p('a',10),p('b',10),p('c',10),p('d',3)]); // a,b,c tie
|
||||
e = apply(e, startEncounter(e)); // [a,b,c,d], cur=a (idx0)
|
||||
// Reorder c before b (both upcoming idx1,2). Within same side = allowed.
|
||||
e = apply(e, reorderParticipants(e, 'c', 'b')); // [a,c,b,d], cur=a
|
||||
expect(e.turnOrderIds).toEqual(['a','c','b','d']);
|
||||
// walk: a current, next c, next b, next d, wrap a
|
||||
e = apply(e, nextTurn(e));
|
||||
expect(e.currentTurnParticipantId).toBe('c');
|
||||
});
|
||||
|
||||
test('toggle inactive: list unchanged (stays in rotation slot)', () => {
|
||||
|
||||
@@ -18,15 +18,12 @@ function enc(ps) {
|
||||
}
|
||||
|
||||
describe('reorderParticipants', () => {
|
||||
test('drag before target (1-list model)', () => {
|
||||
test('drag before target (1-list model, pre-combat)', () => {
|
||||
const ps = [p('a', 10), p('b', 20), p('c', 20)]; // b,c tie
|
||||
let e = enc(ps);
|
||||
e = { ...e, ...startEncounter(e).patch };
|
||||
// initial order: b,c,a (init 20,20,10)
|
||||
expect(e.turnOrderIds).toEqual(['b', 'c', 'a']);
|
||||
let e = enc(ps); // pre-combat, no pointer
|
||||
const r = reorderParticipants(e, 'c', 'b');
|
||||
// drag c before b: remove c → [b,a], insert before b → [c,b,a]
|
||||
expect(r.patch.participants.map(p => p.id)).toEqual(['c', 'b', 'a']);
|
||||
// drag c before b: remove c → [a,b], insert before b → [a,c,b]
|
||||
expect(r.patch.participants.map(p => p.id)).toEqual(['a', 'c', 'b']);
|
||||
});
|
||||
|
||||
test('cross-init drag blocked (no-op)', () => {
|
||||
@@ -47,21 +44,22 @@ describe('reorderParticipants', () => {
|
||||
test('syncs turnOrderIds = participants order (1-list, fixes BUG-6)', () => {
|
||||
const ps = [p('a', 10), p('b', 20), p('c', 20)];
|
||||
let e = enc(ps);
|
||||
e = { ...e, ...startEncounter(e).patch };
|
||||
const r = reorderParticipants(e, 'c', 'b');
|
||||
expect(r.patch.turnOrderIds).toEqual(['c', 'b', 'a']);
|
||||
expect(r.patch.turnOrderIds).toEqual(r.patch.participants.map(p => p.id));
|
||||
e = { ...e, ...startEncounter(e).patch }; // started, but reorder pre-pointer advance
|
||||
// startEncounter sets current=b (idx0). reorder c before b crosses pointer.
|
||||
// Use pre-combat to test syncTurnOrder.
|
||||
let pre = enc(ps);
|
||||
pre = { ...pre, ...reorderParticipants(pre, 'c', 'b').patch };
|
||||
expect(pre.turnOrderIds).toEqual(['a','c','b']);
|
||||
expect(pre.turnOrderIds).toEqual(pre.participants.map(p => p.id));
|
||||
});
|
||||
|
||||
// BUG-6 candidate: reorder should affect turnOrderIds so mid-combat
|
||||
// drag-drop changes who goes next within same-initiative tie.
|
||||
// Currently RED (turnOrderIds not in patch).
|
||||
test('reorder updates turnOrderIds to reflect new participant order', () => {
|
||||
test('reorder updates turnOrderIds to reflect new participant order (pre-combat)', () => {
|
||||
const ps = [p('a', 10), p('b', 20), p('c', 20)];
|
||||
let e = enc(ps);
|
||||
e = { ...e, ...startEncounter(e).patch };
|
||||
// order: b,c,a
|
||||
let e = enc(ps); // pre-combat, no pointer
|
||||
e = { ...e, ...reorderParticipants(e, 'c', 'b').patch };
|
||||
expect(e.turnOrderIds).toEqual(['c', 'b', 'a']);
|
||||
expect(e.turnOrderIds).toEqual(['a', 'c', 'b']);
|
||||
});
|
||||
});
|
||||
|
||||
+22
-3
@@ -536,10 +536,13 @@ function toggleCondition(encounter, participantId, conditionId) {
|
||||
};
|
||||
}
|
||||
|
||||
// REORDER_PARTICIPANTS — drag-drop. 1-list model: drag overrides initiative
|
||||
// (DM choice). Cross-init drag allowed. Splices participants[], syncs turnOrderIds.
|
||||
// REORDER_PARTICIPANTS — drag. Same-initiative ONLY (tie-break override).
|
||||
// Cross-init drag = no-op (use edit-initiative field instead). Splice move.
|
||||
// Cross-pointer drag = no-op once encounter started (BUG-13). nextTurn walks
|
||||
// turnOrderIds forward from current pointer. Dragging a not-yet-acted
|
||||
// participant to a position behind the pointer would skip them (wrap past).
|
||||
// Dragging an already-acted participant ahead of pointer would re-act them.
|
||||
// Blocking cross-pointer keeps rotation deterministic. Pre-combat: free.
|
||||
function reorderParticipants(encounter, draggedId, targetId) {
|
||||
const participants = [...(encounter.participants || [])];
|
||||
const dragged = participants.find(p => p.id === draggedId);
|
||||
@@ -550,10 +553,26 @@ function reorderParticipants(encounter, draggedId, targetId) {
|
||||
return { patch: null, log: null }; // cross-init blocked
|
||||
}
|
||||
const draggedIndex = participants.findIndex(p => p.id === draggedId);
|
||||
// BUG-13: block cross-pointer reorder while encounter running. nextTurn
|
||||
// walks turnOrderIds forward from current pointer. Dragging across the
|
||||
// pointer makes who-acts-next ambiguous — either skip (upcoming dragged
|
||||
// ahead of pointer, walk wraps past) or double-act (acted dragged behind).
|
||||
// Full fix needs actedThisRound tracking. Pragmatic now: block both dirs.
|
||||
// Pre-combat: free reorder (no pointer yet).
|
||||
if (encounter.isStarted && encounter.currentTurnParticipantId) {
|
||||
const pointerIdx = participants.findIndex(p => p.id === encounter.currentTurnParticipantId);
|
||||
const targetIdx0 = participants.findIndex(p => p.id === targetId);
|
||||
if (pointerIdx >= 0) {
|
||||
const crosses = (a, b) => (a <= pointerIdx && b > pointerIdx) || (a > pointerIdx && b <= pointerIdx);
|
||||
if (crosses(draggedIndex, targetIdx0)) {
|
||||
return { patch: null, log: null }; // cross-pointer blocked
|
||||
}
|
||||
}
|
||||
}
|
||||
const [removedItem] = participants.splice(draggedIndex, 1);
|
||||
const newTargetIndex = participants.findIndex(p => p.id === targetId);
|
||||
participants.splice(newTargetIndex, 0, removedItem);
|
||||
const turnUpdates = encounter.isStarted ? syncTurnOrder(participants) : {};
|
||||
const turnUpdates = syncTurnOrder(participants); // 1-list: always mirror
|
||||
return { patch: { participants, ...turnUpdates }, log: null };
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user