From ea17c5e26e8b4fb18a59ac738e9da2cef3569eed Mon Sep 17 00:00:00 2001 From: robert Date: Fri, 31 Jul 2026 16:30:00 -0400 Subject: [PATCH] fix: end combat failing when character sync enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Character writeback in endEncounter wrote to a bare campaigns/{id} path, which the firebase adapter passes straight to the SDK — the update hit a nonexistent top-level doc, threw, and aborted the whole end-combat action (round never reset, combat stayed running). - endEncounter now uses ctx.campaignPath (full artifacts/... path from App.js), falling back to the bare path for server/test adapters - writeback failures no longer block ending combat - regression tests: campaignPath routing, writeback-failure resilience, and participants never cleared on end Co-Authored-By: Claude Fable 5 --- shared/tests/turn.writeback.test.js | 49 +++++++++++++++++++++++++++++ shared/turn.js | 14 +++++++-- src/App.js | 3 +- 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/shared/tests/turn.writeback.test.js b/shared/tests/turn.writeback.test.js index 8fd099f..542796d 100644 --- a/shared/tests/turn.writeback.test.js +++ b/shared/tests/turn.writeback.test.js @@ -139,6 +139,55 @@ describe('character writeback on endEncounter', () => { expect(fighterOld.defaultCurrentHp).toBe(30); }); + test('writeback uses ctx.campaignPath when provided (firebase prefixed paths)', async () => { + const storage = makeMockStorage(); + const players = [{ id: 'char1', name: 'Fighter', defaultMaxHp: 30, defaultAc: 17, defaultCurrentHp: 30 }]; + const prefixed = 'artifacts/app1/public/data/campaigns/camp1'; + storage._docs.set(prefixed, { id: 'camp1', name: 'Camp', players, syncCharacters: true }); + const enc = makeEncounter(); + const ctx = { + storage, + encounterPath: 'campaigns/camp1/encounters/enc1', + campaignPath: prefixed, + logPath: 'logs/log1', + logCollection: 'logs', + displayPath: 'activeDisplay/status', + }; + await endEncounter(enc, ctx); + expect(storage._writes.find(w => w.path === 'campaigns/camp1')).toBeUndefined(); + const campWrite = storage._writes.find(w => w.path === prefixed); + expect(campWrite).toBeDefined(); + expect(campWrite.patch.players.find(p => p.id === 'char1').defaultMaxHp).toBe(28); + }); + + test('writeback failure does not block ending combat', async () => { + const storage = makeMockStorage(); + const players = [{ id: 'char1', name: 'Fighter', defaultMaxHp: 30, defaultAc: 17, defaultCurrentHp: 30 }]; + storage._docs.set('campaigns/camp1', { id: 'camp1', name: 'Camp', players, syncCharacters: true }); + storage.updateDoc = jest.fn(async (p, patch) => { + if (p === 'campaigns/camp1') throw new Error('No document to update'); + storage._writes.push({ path: p, patch }); + }); + const enc = makeEncounter(); + const ctx = { + storage, + encounterPath: 'campaigns/camp1/encounters/enc1', + encPath: 'campaigns/camp1/encounters/enc1', + logPath: 'logs', + logCollection: 'logs', + displayPath: 'activeDisplay/status', + }; + const newEnc = await endEncounter(enc, ctx); + expect(newEnc.isStarted).toBe(false); + expect(newEnc.currentTurnParticipantId).toBe(null); + // participants are preserved — ending combat never clears the roster + expect(newEnc.participants).toHaveLength(3); + const encWrite = storage._writes.find(w => w.path === 'campaigns/camp1/encounters/enc1'); + expect(encWrite).toBeDefined(); + expect(encWrite.patch.isStarted).toBe(false); + expect(encWrite.patch.participants).toBeUndefined(); + }); + test('writeback no-op if character missing from campaign', async () => { const storage = makeMockStorage(); storage._docs.set('campaigns/camp1', { id: 'camp1', name: 'Camp', players: [], syncCharacters: true }); diff --git a/shared/turn.js b/shared/turn.js index 0c41dec..1bf119f 100644 --- a/shared/turn.js +++ b/shared/turn.js @@ -1129,11 +1129,15 @@ async function endEncounter(encounter, ctx) { }; // Character writeback: sync maxHp/ac/currentHp to campaign players if enabled. + // ctx.campaignPath: adapter-specific doc path (firebase needs the full + // artifacts/{APP_ID}/public/data prefix); bare campaigns/{id} fallback for + // server/test adapters. Writeback failure must not block ending combat. const wbCampaignId = encounter.campaignId || ctx.campaignId; if ((wbCampaignId || ctx.campaign) && ctx.storage) { + const campaignPath = ctx.campaignPath || `campaigns/${wbCampaignId}`; let campaign; try { - campaign = ctx.campaign || await ctx.storage.getDoc(`campaigns/${wbCampaignId}`); + campaign = ctx.campaign || await ctx.storage.getDoc(campaignPath); } catch { campaign = null; } if (campaign && campaign.syncCharacters && Array.isArray(campaign.players)) { const charParticipants = (encounter.participants || []).filter(p => p.originalCharacterId); @@ -1153,8 +1157,12 @@ async function endEncounter(encounter, ctx) { return newPlayer; }); if (changed) { - await ctx.storage.updateDoc(`campaigns/${wbCampaignId}`, { players: updatedPlayers }); - log.undo.characterWriteback = oldValues; + try { + await ctx.storage.updateDoc(campaignPath, { players: updatedPlayers }); + log.undo.characterWriteback = oldValues; + } catch (err) { + console.error(`endEncounter: character writeback failed (${campaignPath}):`, err); + } } } } diff --git a/src/App.js b/src/App.js index e28cd5a..7712fd2 100644 --- a/src/App.js +++ b/src/App.js @@ -2281,8 +2281,9 @@ function InitiativeControls({ campaignId, encounter, encounterPath }) { try { const ctx = { ...buildCtx(encounterPath), campaignId }; if (campaignId) { + ctx.campaignPath = getPath.campaign(campaignId); try { - ctx.campaign = await storage.getDoc(getPath.campaign(campaignId)); + ctx.campaign = await storage.getDoc(ctx.campaignPath); } catch {} } await endEncounter(encounter, ctx);