From c1e467c91b84bcf8192021690337fa11e4952737 Mon Sep 17 00:00:00 2001 From: slaguru666 <111923774+slaguru666@users.noreply.github.com> Date: Thu, 6 Aug 2026 00:46:43 +0100 Subject: [PATCH] Prose wins over the field; the play sheet carries avoid; the guard is testable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Still unreleased, but module.json is at 0.6.5 now. initiativeWarning checked the field before the prose, so a hand-edited file carrying both printed the warning twice — the exact fault the migration was meant to end. Prose is checked first now and always wins. The play sheet was the only surface that did not render `avoid`, so three legacy rosters whose warning lived there — church, forest and temple at nightmare — lost it entirely: the helper suppressed its fallback on finding prose the play sheet never showed. It renders `avoid` now, which it should have anyway; it is how the table gets past a fight without having one, which is table-facing by definition. All five surfaces now render both prose fields, so "the prose already says it" means the same thing everywhere. Replayed all 64 rosters in their pre-field form through every surface: 26 need a warning, 130 of 130 checks say it exactly once. The current packs give the same result. commit() no longer mutates st before the write — it passes an incremented copy, so a failed write really does leave the state as it was, which is what the retry warning promises. The single-flight guard moved into stage.mjs as singleFlight() and has four tests: overlapping calls never run together, the second is refused rather than queued, the guard clears afterwards, and a throw does not wedge it. It closes the window within one client only; two GM browsers still race, and that needs a world-level lock rather than a module variable. 41 core tests, 60 adapter tests. Co-Authored-By: Claude Opus 5 --- core/render-authoring.mjs | 6 ++++- core/roster.mjs | 7 +++-- foundry-module/module.json | 2 +- .../module/core/render-authoring.mjs | 6 ++++- foundry-module/module/core/roster.mjs | 7 +++-- foundry-module/module/stage.mjs | 17 ++++++++++++ foundry-module/module/vanity-delve.mjs | 18 ++++++------- foundry-module/test.mjs | 26 ++++++++++++++++++- 8 files changed, 71 insertions(+), 18 deletions(-) diff --git a/core/render-authoring.mjs b/core/render-authoring.mjs index 8f351f0..b441daa 100644 --- a/core/render-authoring.mjs +++ b/core/render-authoring.mjs @@ -144,8 +144,12 @@ export function renderPlay(d) { L.push(`**${cap(a.decision.cue)}** — ${rv.roll ? `\`[${rv.roll}]\` ${rv.success}` : ''}${rv.failure ? ` · **miss** ${rv.failure}` : ''}${rv.orElse ? ` · **or** ${rv.orElse}` : ''}`); if (a.encounter?.roster) { const R = a.encounter.roster; + // `avoid` earns its place here — it is how the table gets past the fight without one, which + // is table-facing by definition. It also keeps every surface rendering both prose fields, so + // initiativeWarning's "the prose already says it" rule means the same thing everywhere; when + // the play sheet alone omitted `avoid`, a legacy warning living there vanished from it. L.push(`**${cap(a.encounter.heat)}:** ${R.foes.map(f => `${f.n}× ${f.name} \`${f.atk}/${f.def}/${f.grit}\``).join(' · ')} — harmed by ${R.harmedBy}.` - + `${initiativeWarning(R) ? ` **${initiativeWarning(R)}**` : ''}`); + + `${initiativeWarning(R) ? ` **${initiativeWarning(R)}**` : ''} *${R.avoid}.*`); } if (a.temptation) L.push(`**${cap(a.temptation.id)}:** ${a.temptation.benefit}. *Use: ${cost(a.temptation.useCost)}. ${a.temptation.standingDrawback}.*`); if (w.notes) L.push(`**Note:** ${w.notes}`); diff --git a/core/roster.mjs b/core/roster.mjs index e077502..a768fb7 100644 --- a/core/roster.mjs +++ b/core/roster.mjs @@ -25,8 +25,11 @@ export const RESTRICTS = /\bONLY\b|blessed|silvered|magical|only stays down|fire */ export function initiativeWarning(roster) { if (!roster) return null; - if (roster.beforeInitiative) return roster.beforeInitiative; + // Prose wins, and is checked first. A hand-edited file can carry both the field and the old + // sentence; returning the field before looking would print it twice, which is the fault this + // whole migration exists to remove. const prose = `${roster.harmedBy ?? ''} ${roster.avoid ?? ''}`; - if (/say so before initiative/i.test(prose)) return null; // a legacy file that says it itself + if (/say so before initiative/i.test(prose)) return null; + if (roster.beforeInitiative) return roster.beforeInitiative; return RESTRICTS.test(roster.harmedBy ?? '') ? 'Say so before initiative.' : null; } diff --git a/foundry-module/module.json b/foundry-module/module.json index 5a49152..5e092cc 100644 --- a/foundry-module/module.json +++ b/foundry-module/module.json @@ -2,7 +2,7 @@ "id": "vanity-delve", "title": "DELVE — a dungeon layer for VANITY", "description": "Generates a coherent delve and sequences VANITY's Forge to stage it, one area at a time.", - "version": "0.6.4", + "version": "0.6.5", "compatibility": { "minimum": "13", "verified": "14.365" diff --git a/foundry-module/module/core/render-authoring.mjs b/foundry-module/module/core/render-authoring.mjs index 8f351f0..b441daa 100644 --- a/foundry-module/module/core/render-authoring.mjs +++ b/foundry-module/module/core/render-authoring.mjs @@ -144,8 +144,12 @@ export function renderPlay(d) { L.push(`**${cap(a.decision.cue)}** — ${rv.roll ? `\`[${rv.roll}]\` ${rv.success}` : ''}${rv.failure ? ` · **miss** ${rv.failure}` : ''}${rv.orElse ? ` · **or** ${rv.orElse}` : ''}`); if (a.encounter?.roster) { const R = a.encounter.roster; + // `avoid` earns its place here — it is how the table gets past the fight without one, which + // is table-facing by definition. It also keeps every surface rendering both prose fields, so + // initiativeWarning's "the prose already says it" rule means the same thing everywhere; when + // the play sheet alone omitted `avoid`, a legacy warning living there vanished from it. L.push(`**${cap(a.encounter.heat)}:** ${R.foes.map(f => `${f.n}× ${f.name} \`${f.atk}/${f.def}/${f.grit}\``).join(' · ')} — harmed by ${R.harmedBy}.` - + `${initiativeWarning(R) ? ` **${initiativeWarning(R)}**` : ''}`); + + `${initiativeWarning(R) ? ` **${initiativeWarning(R)}**` : ''} *${R.avoid}.*`); } if (a.temptation) L.push(`**${cap(a.temptation.id)}:** ${a.temptation.benefit}. *Use: ${cost(a.temptation.useCost)}. ${a.temptation.standingDrawback}.*`); if (w.notes) L.push(`**Note:** ${w.notes}`); diff --git a/foundry-module/module/core/roster.mjs b/foundry-module/module/core/roster.mjs index e077502..a768fb7 100644 --- a/foundry-module/module/core/roster.mjs +++ b/foundry-module/module/core/roster.mjs @@ -25,8 +25,11 @@ export const RESTRICTS = /\bONLY\b|blessed|silvered|magical|only stays down|fire */ export function initiativeWarning(roster) { if (!roster) return null; - if (roster.beforeInitiative) return roster.beforeInitiative; + // Prose wins, and is checked first. A hand-edited file can carry both the field and the old + // sentence; returning the field before looking would print it twice, which is the fault this + // whole migration exists to remove. const prose = `${roster.harmedBy ?? ''} ${roster.avoid ?? ''}`; - if (/say so before initiative/i.test(prose)) return null; // a legacy file that says it itself + if (/say so before initiative/i.test(prose)) return null; + if (roster.beforeInitiative) return roster.beforeInitiative; return RESTRICTS.test(roster.harmedBy ?? '') ? 'Say so before initiative.' : null; } diff --git a/foundry-module/module/stage.mjs b/foundry-module/module/stage.mjs index f248e08..e2e8ba6 100644 --- a/foundry-module/module/stage.mjs +++ b/foundry-module/module/stage.mjs @@ -32,6 +32,23 @@ const attempt = async (fx, fn) => { catch (e) { fx.log?.(e); return { ok: false, value: null }; } }; +/** + * Wrap an async function so overlapping calls are refused rather than interleaved. + * + * enter() hangs off a button, and it awaits the Forge for seconds at a time. Two overlapping calls + * both read the same index and stage the same area twice. This closes that within one client; two + * separate GM browsers still race, and the durable fix for that is a world-level lock rather than + * a module variable. + */ +export function singleFlight(fn, onBusy) { + let busy = false; + return async (...args) => { + if (busy) return onBusy?.(); + busy = true; + try { return await fn(...args); } finally { busy = false; } + }; +} + /** The foe section of the GM card. Mirrors forge-app's foeSection; both switch on the same kind. */ export function foeBlock(c) { if (c.kind === 'forged') return `

${cap(c.heat)} — in the world:

${list(c.foes.map(foeLine))}${ diff --git a/foundry-module/module/vanity-delve.mjs b/foundry-module/module/vanity-delve.mjs index 4aaf73d..8e6783f 100644 --- a/foundry-module/module/vanity-delve.mjs +++ b/foundry-module/module/vanity-delve.mjs @@ -17,16 +17,13 @@ import { coinSeed, Rng } from './core/rng.mjs'; import { newWorkingFile, outstanding, readyToPlay } from './core/authoring.mjs'; import { DelveForgeApp, raiseDungeon, listDungeons, removeDungeon, removeDungeonDialog, setThemes } from './forge-app.mjs'; -import { stageArea } from './stage.mjs'; +import { stageArea, singleFlight } from './stage.mjs'; const MOD = 'vanity-delve'; const FLAG = 'state'; let PACK = null; let seamsPresent = false; -// enter() is bound to a button a GM can double-press, and it awaits the Forge for seconds at a -// time. Two overlapping calls would both read the same st.at and stage the same area twice. -let staging = false; let loadPack = async () => null; /** The geometries VANITY's Forge can actually build — mirrors validate-pack.mjs. */ @@ -122,11 +119,10 @@ async function draft(params = {}) { return load(d); } -async function enter() { - if (staging) return ui.notifications.warn('DELVE: already raising an area — wait for it to finish.'); - staging = true; - try { return await enterOnce(); } finally { staging = false; } -} +const enter = singleFlight( + () => enterOnce(), + () => ui.notifications.warn('DELVE: already raising an area — wait for it to finish.'), +); async function enterOnce() { const st = getState(); @@ -152,7 +148,9 @@ async function enterOnce() { ...(seamsPresent ? { hoard: false, post: false, folderId: st.folderId } : {}), }), hoard: size => game.vanity.forge.hoard({ size, ...(seamsPresent ? { post: false } : {}) }), - commit: async () => { st.at += 1; st.turn += 1; await setState(st); }, + // An incremented copy, never a mutation of st: a failed write must leave the in-memory state + // exactly as it was, or the retry advice in the warning is wrong. + commit: () => setState({ ...st, at: st.at + 1, turn: st.turn + 1 }), readAloud: readAloudCard, gm: gmCard, warn: m => ui.notifications.warn(m), diff --git a/foundry-module/test.mjs b/foundry-module/test.mjs index 3d51954..3afaca0 100644 --- a/foundry-module/test.mjs +++ b/foundry-module/test.mjs @@ -8,7 +8,7 @@ * Run: node foundry-module/test.mjs */ import { foeStats, foeLine, classifyFoes } from './module/foes.mjs'; -import { stageArea, areaCards, foeBlock } from './module/stage.mjs'; +import { stageArea, areaCards, foeBlock, singleFlight } from './module/stage.mjs'; import { initiativeWarning } from '../core/roster.mjs'; let pass = 0, fail = 0; @@ -175,8 +175,32 @@ t('a legacy Wraith roster spelled without ONLY still gets one', 'the spelling was never the semantics — barrow/nightmare had no ONLY'); t('a legacy roster that says it in prose is not given it twice', initiativeWarning({ harmedBy: 'blessed weapons ONLY — say so before initiative' }) === null); +t('prose in avoid also suppresses the fallback', + initiativeWarning({ harmedBy: 'blessed weapons ONLY', avoid: 'burn it — say so before initiative' }) === null); +t('the field never overrides prose that already says it', + initiativeWarning({ harmedBy: 'blessed weapons ONLY — say so before initiative', beforeInitiative: 'Say so before initiative.' }) === null, + 'a hand-edited file can carry both; returning the field first printed it twice'); t('null roster is safe', initiativeWarning(null) === null); +// ---------------------------------------------------------------- single flight +{ + let running = 0, peak = 0, refused = 0; + const guarded = singleFlight(async () => { + running++; peak = Math.max(peak, running); + await new Promise(r => setTimeout(r, 5)); + running--; return 'done'; + }, () => { refused++; return 'busy'; }); + const [a, b] = await Promise.all([guarded(), guarded()]); + t('two overlapping calls never run together', peak === 1, `peak concurrency ${peak}`); + t('the second is refused, not queued', a === 'done' && b === 'busy' && refused === 1); + t('the guard clears so a later call still runs', (await guarded()) === 'done'); +} +{ + const guarded = singleFlight(async () => { throw new Error('boom'); }, () => 'busy'); + await guarded().catch(() => {}); + t('a throw does not leave the guard stuck', (await guarded().catch(() => 'threw')) === 'threw'); +} + // ---------------------------------------------------------------- surface parity // forge-app destructures three Foundry symbols at import time. Everything under test is pure, so // the smallest possible shim makes the journal renderer importable and its parity with the chat