diff --git a/TODO.md b/TODO.md index 43cff87..337465a 100644 --- a/TODO.md +++ b/TODO.md @@ -38,9 +38,12 @@ Backlog of bugs + long-term items. Milestones live in REWORK_PLAN.md. 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. +### Death saves: D&D 5e model (FIXED) +- FIXED — separate success/fail tracking. `deathSave(enc, id, type, n)` + type='success'|'fail'. Fields: `deathSaves`, `deathFails`, `isStable`, + `isDying`. 3 success=stable, 3 fail=dead. Returns `{patch, log, status}`. + Old single-counter broken (successes missing, treated saves as fails). + Old data lost (user OK — feature didn't work). UI: green ✓ row + red ✕ row. ### Custom condition field - Conditions hardcoded list. No way for DM to add custom. diff --git a/shared/tests/turn.characterization.test.js b/shared/tests/turn.characterization.test.js index 9ec161f..83404e2 100644 --- a/shared/tests/turn.characterization.test.js +++ b/shared/tests/turn.characterization.test.js @@ -247,22 +247,22 @@ describe('applyHpChange', () => { }); describe('deathSave', () => { - test('increments saves', () => { - const ps = [p('a', 10, { currentHp: 0, deathSaves: 0 })]; - const { patch } = deathSave(enc(ps), 'a', 1); - expect(patch.participants[0].deathSaves).toBe(1); + test('increments fails', () => { + const ps = [p('a', 10, { currentHp: 0, deathFails: 0 })]; + const { patch } = deathSave(enc(ps), 'a', 'fail', 1); + expect(patch.participants[0].deathFails).toBe(1); }); - test('clicking same save decrements (toggle)', () => { - const ps = [p('a', 10, { currentHp: 0, deathSaves: 2 })]; - const { patch } = deathSave(enc(ps), 'a', 2); - expect(patch.participants[0].deathSaves).toBe(1); + test('clicking same fail decrements (toggle)', () => { + const ps = [p('a', 10, { currentHp: 0, deathFails: 2 })]; + const { patch } = deathSave(enc(ps), 'a', 'fail', 2); + expect(patch.participants[0].deathFails).toBe(1); }); - test('third save sets isDying', () => { - const ps = [p('a', 10, { currentHp: 0, deathSaves: 2 })]; - const result = deathSave(enc(ps), 'a', 3); - expect(result.patch.participants[0].deathSaves).toBe(3); + test('third fail sets isDying', () => { + const ps = [p('a', 10, { currentHp: 0, deathFails: 2 })]; + const result = deathSave(enc(ps), 'a', 'fail', 3); + expect(result.patch.participants[0].deathFails).toBe(3); expect(result.patch.participants[0].isDying).toBe(true); expect(result.isDying).toBe(true); }); diff --git a/shared/tests/turn.combat.test.js b/shared/tests/turn.combat.test.js index edf552d..eacc08f 100644 --- a/shared/tests/turn.combat.test.js +++ b/shared/tests/turn.combat.test.js @@ -162,7 +162,7 @@ describe('combat integrity (100 rounds, full op coverage)', () => { } // 5. deathSave if (actor && actor.currentHp <= 0 && !actor.isNpc) { - try { e = apply(e, deathSave(e, actor.id, 1)); } catch (err) {} + try { e = apply(e, deathSave(e, actor.id, 'fail', 1)); } catch (err) {} } // 6. removeParticipant if (totalTurns % 5 === 0) { diff --git a/shared/tests/turn.dead-skip.test.js b/shared/tests/turn.dead-skip.test.js index a56cb96..670bc35 100644 --- a/shared/tests/turn.dead-skip.test.js +++ b/shared/tests/turn.dead-skip.test.js @@ -56,10 +56,10 @@ describe('unified: death deactivates, dead skipped in rotation', () => { // kill b (current = a) e = { ...e, ...applyHpChange(e, 'b', 'damage', 100).patch }; // b is dead: DM can still fire deathSave (manual action) - const r = deathSave(e, 'b', 1); + const r = deathSave(e, 'b', 'fail', 1); expect(r.patch).toBeTruthy(); const b = r.patch.participants.find(x => x.id === 'b'); - expect(b.deathSaves).toBe(1); + expect(b.deathFails).toBe(1); }); // D1 unification: shared now matches App main — death flips isActive=false. diff --git a/shared/tests/turn.deathsave.test.js b/shared/tests/turn.deathsave.test.js new file mode 100644 index 0000000..7ae1d75 --- /dev/null +++ b/shared/tests/turn.deathsave.test.js @@ -0,0 +1,90 @@ +// Death saves: broken model. Current = single counter, 3=dying. D&D 5e needs +// separate success/fail tracking: +// 3 successes = stable (0hp unconscious, safe until healed) +// 3 failures = dead +// Current tracks "saves" (really fails via red X UI), no successes at all. +// "Stable" state doesn't exist. +// +// RED: prove current can't model 3-success stability. + +'use strict'; + +const shared = require('@ttrpg/shared'); +const { makeParticipant, startEncounter, applyHpChange, deathSave } = shared; + +function p(id) { + return makeParticipant({ id, name: id, type: 'character', + initiative: 10, 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; + +describe('death saves: missing success/stable model', () => { + test('3 successes makes character stable (not dead, not dying)', () => { + // Character at 0hp. Roll 3 successful death saves. + let e = enc([p('a')]); + e = apply(e, startEncounter(e)); + e = apply(e, applyHpChange(e, 'a', 'damage', 100)); // 0hp + expect(e.participants[0].currentHp).toBe(0); + + // Roll 3 successful saves. Should mark stable. + e = apply(e, deathSave(e, 'a', 'success', 1)); + e = apply(e, deathSave(e, 'a', 'success', 2)); + const r3 = deathSave(e, 'a', 'success', 3); + e = apply(e, r3); + + // RED: current deathSave signature = (enc, id, saveNumber). No type arg. + // Will throw or misbehave. Want: status field. + expect(r3.status).toBe('stable'); + expect(e.participants[0].isDying).toBe(false); + expect(e.participants[0].isStable).toBe(true); + expect(e.participants[0].deathFails).toBe(0); + }); + + test('3 failures makes character dead (isDying for removal)', () => { + let e = enc([p('a')]); + e = apply(e, startEncounter(e)); + e = apply(e, applyHpChange(e, 'a', 'damage', 100)); + + const r1 = deathSave(e, 'a', 'fail', 1); + e = apply(e, r1); + const r2 = deathSave(e, 'a', 'fail', 2); + e = apply(e, r2); + const r3 = deathSave(e, 'a', 'fail', 3); + e = apply(e, r3); + + expect(r3.status).toBe('dead'); + expect(e.participants[0].isDying).toBe(true); + expect(e.participants[0].deathFails).toBe(3); + }); + + test('successes and fails tracked independently', () => { + // Mixed: 1 success, 2 fails, then 2 more successes = stable. + let e = enc([p('a')]); + e = apply(e, startEncounter(e)); + e = apply(e, applyHpChange(e, 'a', 'damage', 100)); + + e = apply(e, deathSave(e, 'a', 'success', 1)); + expect(e.participants[0].deathSaves).toBe(1); + expect(e.participants[0].deathFails).toBe(0); + + e = apply(e, deathSave(e, 'a', 'fail', 1)); + e = apply(e, deathSave(e, 'a', 'fail', 2)); + expect(e.participants[0].deathSaves).toBe(1); + expect(e.participants[0].deathFails).toBe(2); + + // 2 more successes → total 3 successes → stable + e = apply(e, deathSave(e, 'a', 'success', 2)); + const r = deathSave(e, 'a', 'success', 3); + expect(r.status).toBe('stable'); + expect(e.participants[0].deathFails).toBe(2); // fails unchanged + }); + + test('deathSave signature: (enc, id, type, n)', () => { + // type = 'success' | 'fail'. n = 1|2|3. + expect(deathSave.length).toBe(4); + }); +}); diff --git a/shared/tests/turn.logging.test.js b/shared/tests/turn.logging.test.js index 82852a6..15793af 100644 --- a/shared/tests/turn.logging.test.js +++ b/shared/tests/turn.logging.test.js @@ -93,7 +93,7 @@ describe('Logging contract: mutating ops', () => { test('deathSave logs', () => { const started = apply(e, startEncounter(e)); const dying = apply(started, applyHpChange(started, 'b', 'damage', 100)); - const r = deathSave(dying, 'b', 1); + const r = deathSave(dying, 'b', 'fail', 1); expectLogged(r); }); diff --git a/shared/turn.js b/shared/turn.js index 6554926..b3fd753 100644 --- a/shared/turn.js +++ b/shared/turn.js @@ -129,7 +129,9 @@ function makeParticipant(opts) { isNpc: opts.isNpc || false, conditions: opts.conditions || [], isActive: opts.isActive !== undefined ? opts.isActive : true, - deathSaves: opts.deathSaves || 0, + deathSaves: opts.deathSaves || 0, // successes (0-3) + deathFails: opts.deathFails || 0, // failures (0-3) + isStable: opts.isStable || false, isDying: opts.isDying || false, }; } @@ -492,41 +494,63 @@ function applyHpChange(encounter, participantId, changeType, amount) { }; } -// DEATH_SAVE — verbatim from ParticipantManager.handleDeathSaveChange -// saveNumber: 1 | 2 | 3. Returns isDying flag if 3rd save hit (client triggers removal animation). -function deathSave(encounter, participantId, saveNumber) { +// DEATH_SAVE — D&D 5e model. Separate success/fail tracking. +// type: 'success' | 'fail' +// n: 1 | 2 | 3 (which pip clicked) +// 3 successes = stable (0hp unconscious, safe). 3 fails = dead (isDying, +// caller animates removal). Toggling same pip = undo that pip. +// Returns { patch, log, status } where status: 'stable'|'dead'|'pending'. +function deathSave(encounter, participantId, type, n) { const participant = (encounter.participants || []).find(p => p.id === participantId); if (!participant) throw new Error('Participant not found.'); - const currentSaves = participant.deathSaves || 0; - const newSaves = currentSaves === saveNumber ? saveNumber - 1 : saveNumber; + if (type !== 'success' && type !== 'fail') { + throw new Error(`deathSave type must be 'success' or 'fail', got "${type}".`); + } const name = participant.name; const undo = { participants: encounter.participants || [] }; - if (newSaves === 3) { - // Mark dying — caller waits for animation, then calls removeParticipant. - const updatedParticipants = (encounter.participants || []).map(p => - p.id === participantId ? { ...p, deathSaves: newSaves, isDying: true } : p - ); - return { - patch: { participants: updatedParticipants }, - log: { - message: `${name} failed 3 death saves — dying`, - undo, - }, - isDying: true, - }; + // Toggle: clicking current-value pip decrements (undo that pip). + const current = type === 'success' + ? (participant.deathSaves || 0) + : (participant.deathFails || 0); + const next = current === n ? n - 1 : n; + + const field = type === 'success' ? 'deathSaves' : 'deathFails'; + const updates = { [field]: next }; + + // Clear terminal flags on any change (re-eval below). + let status = 'pending'; + const newSuccesses = type === 'success' ? next : (participant.deathSaves || 0); + const newFails = type === 'fail' ? next : (participant.deathFails || 0); + + if (newSuccesses >= 3) { + updates.isStable = true; + updates.isDying = false; + status = 'stable'; + } else if (newFails >= 3) { + updates.isStable = false; + updates.isDying = true; + status = 'dead'; + } else { + updates.isStable = false; + updates.isDying = false; } const updatedParticipants = (encounter.participants || []).map(p => - p.id === participantId ? { ...p, deathSaves: newSaves } : p + p.id === participantId ? { ...p, ...updates } : p ); + + const message = status === 'stable' + ? `${name} stabilized (3 death saves)` + : status === 'dead' + ? `${name} failed 3 death saves — dead` + : `${name} death ${type}: ${next}/3`; + return { patch: { participants: updatedParticipants }, - log: { - message: `${name} death save: ${newSaves}/3`, - undo, - }, - isDying: false, + log: { message, undo }, + status, + isDying: status === 'dead', // back-compat for App.js handler }; } diff --git a/src/App.js b/src/App.js index c086a09..6504014 100644 --- a/src/App.js +++ b/src/App.js @@ -873,6 +873,8 @@ function ParticipantManager({ encounter, encounterPath, campaignCharacters }) { conditions: [], isActive: true, deathSaves: 0, + deathFails: 0, + isStable: false, isDying: false, }; @@ -930,6 +932,8 @@ function ParticipantManager({ encounter, encounterPath, campaignCharacters }) { isActive: true, isNpc: false, deathSaves: 0, + deathFails: 0, + isStable: false, isDying: false, }; }); @@ -1053,10 +1057,10 @@ function ParticipantManager({ encounter, encounterPath, campaignCharacters }) { } }; - const handleDeathSaveChange = async (participantId, saveNumber) => { + const handleDeathSaveChange = async (participantId, type, saveNumber) => { if (!db) return; try { - const { patch, isDying } = combatDeathSave(encounter, participantId, saveNumber); + const { patch, isDying } = combatDeathSave(encounter, participantId, type, saveNumber); await storage.updateDoc(encounterPath, patch); if (isDying) { setTimeout(async () => { @@ -1370,20 +1374,41 @@ function ParticipantManager({ encounter, encounterPath, campaignCharacters }) { HP: {p.currentHp}/{p.maxHp} - {/* Death Saves - only player characters make death saving throws */} + {/* Death saves - D&D 5e: successes + fails separate */} {isDead && encounter.isStarted && p.type === 'character' && ( -
- Death Saves: - {[1, 2, 3].map(saveNum => ( - - ))} +
+ {p.isStable && ( +
Stabilized
+ )} + {p.isDying && !p.isStable && ( +
Dying
+ )} +
+ Saves: + {[1, 2, 3].map(saveNum => ( + + ))} +
+
+ Fails: + {[1, 2, 3].map(failNum => ( + + ))} +
)} diff --git a/src/tests/Combat.scenario.test.js b/src/tests/Combat.scenario.test.js index b77610a..defd070 100644 --- a/src/tests/Combat.scenario.test.js +++ b/src/tests/Combat.scenario.test.js @@ -160,10 +160,10 @@ function removeParticipantByName(name) { if (!p) throw new Error(`no remove target: ${name}`); applyEnc(removeParticipant(enc, p.id).patch); } -function deathSaveAction(name, saveNum) { +function deathSaveAction(name, type, saveNum) { const p = any(name); if (!p) throw new Error(`no deathsave target: ${name}`); - applyEnc(combatDeathSave(enc, p.id, saveNum).patch); + applyEnc(combatDeathSave(enc, p.id, type, saveNum).patch); } function dragTie(draggedName, targetName) { const d = any(draggedName), t = any(targetName); @@ -254,16 +254,16 @@ test('full 100-round combat scenario (shared, no React)', async () => { // death scenarios: drop a PC to 0, death saves, revive if (r === 25 && turnInRound === 0) { await recordAsync(`r${r} drop Rogue`, () => applyDamage('Rogue', 99)); - record(`r${r} deathSave1 Rogue`, () => deathSaveAction('Rogue', 1)); + record(`r${r} deathSave1 Rogue`, () => deathSaveAction('Rogue', 'fail', 1)); record(`r${r} revive Rogue`, () => applyHeal('Rogue', 22)); } // round 50: full 3-success death-save path (isDying) on Cleric, then remove if (r === 50 && turnInRound === 0) { await recordAsync(`r${r} drop Cleric`, () => applyDamage('Cleric', 99)); await recordAsync(`r${r} deathSave Cleric x3 (isDying)`, async () => { - deathSaveAction('Cleric', 1); - deathSaveAction('Cleric', 2); - deathSaveAction('Cleric', 3); // → isDying true + deathSaveAction('Cleric', 'fail', 1); + deathSaveAction('Cleric', 'fail', 2); + deathSaveAction('Cleric', 'fail', 3); // → dead }); const cl = any('Cleric'); if (cl && cl.isDying) record(`r${r} remove dying Cleric`, () => removeParticipantByName('Cleric')); diff --git a/src/tests/Logs.characterization.test.js b/src/tests/Logs.characterization.test.js index 4cb070a..b8287ba 100644 --- a/src/tests/Logs.characterization.test.js +++ b/src/tests/Logs.characterization.test.js @@ -125,7 +125,7 @@ describe('DeathSave -> Firebase', () => { await waitFor(() => lastEncCall()?.data?.participants?.[0]?.currentHp === 0); // death save buttons appear - const save1 = screen.getByTitle('Death save 1'); + const save1 = screen.getByTitle('Success 1'); fireEvent.click(save1); await waitFor(() => lastEncCall()?.data?.participants?.[0]?.deathSaves === 1); expect(lastEncCall().data.participants[0].deathSaves).toBe(1); @@ -159,13 +159,13 @@ describe('DeathSave -> Firebase', () => { fireEvent.click(screen.getByTitle('Damage')); await waitFor(() => lastEncCall()?.data?.participants?.[0]?.currentHp === 0); - fireEvent.click(screen.getByTitle('Death save 1')); - await waitFor(() => lastEncCall()?.data?.participants?.[0]?.deathSaves === 1); - fireEvent.click(screen.getByTitle('Death save 2')); - await waitFor(() => lastEncCall()?.data?.participants?.[0]?.deathSaves === 2); - fireEvent.click(screen.getByTitle('Death save 3')); + fireEvent.click(screen.getByTitle('Fail 1')); + await waitFor(() => lastEncCall()?.data?.participants?.[0]?.deathFails === 1); + fireEvent.click(screen.getByTitle('Fail 2')); + await waitFor(() => lastEncCall()?.data?.participants?.[0]?.deathFails === 2); + fireEvent.click(screen.getByTitle('Fail 3')); await waitFor(() => lastEncCall()?.data?.participants?.[0]?.isDying === true); expect(lastEncCall().data.participants[0].isDying).toBe(true); - expect(lastEncCall().data.participants[0].deathSaves).toBe(3); + expect(lastEncCall().data.participants[0].deathFails).toBe(3); }); });