From 4c2eb65470afc67f89a331c2521764cbc739c244 Mon Sep 17 00:00:00 2001 From: hypercross Date: Mon, 13 Jul 2026 10:22:22 +0800 Subject: [PATCH] refactor: pass variable store explicitly to getCombined Remove the mutable `streamFallback` global state and instead pass the `VariableStore` as an explicit argument to `getCombined` and related functions. This ensures correctness by avoiding reliance on mutable module state during reactivity calculations. --- src/components/journal/command-dispatcher.ts | 16 ++--- src/components/journal/var-reactivity.ts | 63 +++++++++----------- 2 files changed, 37 insertions(+), 42 deletions(-) diff --git a/src/components/journal/command-dispatcher.ts b/src/components/journal/command-dispatcher.ts index a586c20..a849e8d 100644 --- a/src/components/journal/command-dispatcher.ts +++ b/src/components/journal/command-dispatcher.ts @@ -140,9 +140,9 @@ function dispatchSet( } const key = p.key; - // Use getCombined for old value so tag transitions are detected correctly - // even when the variable has active mods. - const oldValue = getCombined(key); + // Use getCombined with stream fallback for old value so tag transitions + // are detected correctly even when the variable has active mods. + const oldValue = getCombined(key, ctx.variables); let newValue: string; try { @@ -158,8 +158,8 @@ function dispatchSet( const result = evaluateExpression(p.expr, { lookup: (name: string) => { const k = "$" + name; - // Use getCombined for accurate values including active mods - return k === key ? undefined : getCombined(k); + // Use getCombined with stream fallback for accurate values + return k === key ? undefined : getCombined(k, ctx.variables); }, }); newValue = String(result.value); @@ -192,9 +192,9 @@ function dispatchSet( } // Send the direct set last so cascade messages arrive first. - // Use getCombined so the stream store receives the correct value - // (base + active mods) rather than just the raw base. - const combinedValue = getCombined(key); + // Use getCombined with stream fallback so the stream store receives the + // correct value (base + active mods) rather than just the raw base. + const combinedValue = getCombined(key, ctx.variables); const r1 = sendMessage("var", { action: "set", key, value: combinedValue }); const u1 = unwrap(r1); if (!u1.ok) return u1; diff --git a/src/components/journal/var-reactivity.ts b/src/components/journal/var-reactivity.ts index 6a63872..804b689 100644 --- a/src/components/journal/var-reactivity.ts +++ b/src/components/journal/var-reactivity.ts @@ -11,10 +11,9 @@ * Combined value = base + sum(activeMods). Tag values (starting with #) * are not numeric and don't receive mods. * - * Limitation: base snapshots are taken at tag activation time. If another - * user changes a variable's value remotely while a local tag is active, - * the deactivation will restore the snapshot base rather than the remote - * value. In practice this is rare — GMs set bases, players toggle tags. + * All functions that resolve variable values accept an explicit `fallback` + * (the stream VariableStore) rather than relying on mutable module state. + * This ensures correctness regardless of call order. */ import type { VarDeclaration, TagModifier } from "./declare-parser"; @@ -63,9 +62,6 @@ const sourceActivations = new Map(); -/** Fallback store for variables not tracked locally (set by other users) */ -let streamFallback: VariableStore = {}; - // --------------------------------------------------------------------------- // Public API // --------------------------------------------------------------------------- @@ -125,9 +121,9 @@ export function extractDependencies(expr: string): string[] { /** * Get the combined value of a variable (base + sum of active mods). - * Falls back to the stream store for variables not tracked locally. + * Falls back to the provided stream store for variables not tracked locally. */ -export function getCombined(key: string): string { +export function getCombined(key: string, fallback?: VariableStore): string { const base = baseValues.get(key); if (base !== undefined && isTagValue(base)) { return base; // tag values don't receive numeric mods @@ -142,12 +138,12 @@ export function getCombined(key: string): string { } // Fall back to stream store - const fallback = streamFallback[key]; - if (fallback !== undefined) { - if (isTagValue(fallback)) return fallback; - const fbNum = parseFloat(fallback); + const fb = fallback?.[key]; + if (fb !== undefined) { + if (isTagValue(fb)) return fb; + const fbNum = parseFloat(fb); if (!isNaN(fbNum)) return String(fbNum + modSum); - return fallback; + return fb; } // No base, but has mods @@ -191,19 +187,17 @@ export function computeCascade( return []; } - streamFallback = currentVars; - const results: Array<{ key: string; value: string }> = []; // ---- Tag activation/deactivation ---- - const newValue = getCombined(changedVar); + const newValue = getCombined(changedVar, currentVars); const newTag = isTagValue(newValue); const oldTag = isTagValue(oldValue); if (newTag !== oldTag) { // Deactivate old tag if (oldTag) { - const removed = deactivateTagFromSource(changedVar, oldTag); + const removed = deactivateTagFromSource(changedVar, oldTag, currentVars); for (const r of removed) { results.push(r); } @@ -211,7 +205,7 @@ export function computeCascade( // Activate new tag if (newTag) { - const added = activateTagFromSource(changedVar, newTag); + const added = activateTagFromSource(changedVar, newTag, currentVars); for (const r of added) { results.push(r); } @@ -219,7 +213,7 @@ export function computeCascade( } // ---- Declaration re-evaluation ---- - const reevaluated = reevaluateDependents(changedVar); + const reevaluated = reevaluateDependents(changedVar, currentVars); for (const r of reevaluated) { if (!results.some((x) => x.key === r.key)) { results.push(r); @@ -238,8 +232,6 @@ export function computeInitialValues( ): Array<{ key: string; value: string }> { if (!declExprs) return []; - streamFallback = currentVars; - const allKeys = [...declExprs.keys()]; const sorted = topoSortAffected(new Set(allKeys)); @@ -253,7 +245,7 @@ export function computeInitialValues( const result = evaluateExpression(expr, { lookup: (name: string) => { const k = "$" + name; - return getCombined(k); + return getCombined(k, currentVars); }, }); @@ -263,7 +255,7 @@ export function computeInitialValues( // Check if this is a tag value — if so, activate it const tag = isTagValue(rawValue); if (tag) { - const added = activateTagFromSource(key, tag); + const added = activateTagFromSource(key, tag, currentVars); for (const r of added) { if (!results.some((x) => x.key === r.key)) { results.push(r); @@ -272,7 +264,7 @@ export function computeInitialValues( } // Always emit the combined value for this key - const combined = getCombined(key); + const combined = getCombined(key, currentVars); if (!results.some((x) => x.key === key)) { results.push({ key, value: combined }); } @@ -298,6 +290,7 @@ function isTagValue(value: string | undefined): string | null { function activateTagFromSource( source: string, tag: string, + fallback: VariableStore, ): Array<{ key: string; value: string }> { if (!tagModMap) return []; @@ -312,13 +305,13 @@ function activateTagFromSource( // Snapshot the target's current combined value as its base before // adding the mod, so deactivation can restore it correctly. if (!baseValues.has(mod.target)) { - baseValues.set(mod.target, getCombined(mod.target)); + baseValues.set(mod.target, getCombined(mod.target, fallback)); } const result = evaluateExpression(mod.expression, { lookup: (name: string) => { const k = "$" + name; - return getCombined(k); + return getCombined(k, fallback); }, }); @@ -334,7 +327,7 @@ function activateTagFromSource( sourceEntries.push({ tag, target: mod.target, value: result.value }); // Emit new combined value for the target - const combined = getCombined(mod.target); + const combined = getCombined(mod.target, fallback); const existing = results.findIndex((r) => r.key === mod.target); if (existing >= 0) { results[existing] = { key: mod.target, value: combined }; @@ -354,6 +347,7 @@ function activateTagFromSource( function deactivateTagFromSource( source: string, _tag: string, + fallback: VariableStore, ): Array<{ key: string; value: string }> { const entries = sourceActivations.get(source); if (!entries) return []; @@ -388,7 +382,7 @@ function deactivateTagFromSource( // Emit new combined values for affected targets for (const target of affectedTargets) { - results.push({ key: target, value: getCombined(target) }); + results.push({ key: target, value: getCombined(target, fallback) }); } return results; @@ -400,6 +394,7 @@ function deactivateTagFromSource( function reevaluateDependents( changedVar: string, + fallback: VariableStore, ): Array<{ key: string; value: string }> { if (!depGraph || !declExprs) return []; @@ -436,21 +431,21 @@ function reevaluateDependents( const result = evaluateExpression(expr, { lookup: (name: string) => { const k = "$" + name; - return getCombined(k); + return getCombined(k, fallback); }, }); const rawValue = String(result.value); // Check for tag transition on this declared variable - const oldCombined = getCombined(key); + const oldCombined = getCombined(key, fallback); const oldTag = isTagValue(oldCombined); const newTag = isTagValue(rawValue); if (oldTag !== newTag) { // Deactivate old tag if (oldTag) { - const removed = deactivateTagFromSource(key, oldTag); + const removed = deactivateTagFromSource(key, oldTag, fallback); for (const r of removed) { if (!results.some((x) => x.key === r.key)) { results.push(r); @@ -460,7 +455,7 @@ function reevaluateDependents( // Activate new tag if (newTag) { baseValues.set(key, rawValue); - const added = activateTagFromSource(key, newTag); + const added = activateTagFromSource(key, newTag, fallback); for (const r of added) { if (!results.some((x) => x.key === r.key)) { results.push(r); @@ -473,7 +468,7 @@ function reevaluateDependents( baseValues.set(key, rawValue); // Emit combined value - const combined = getCombined(key); + const combined = getCombined(key, fallback); if (!results.some((x) => x.key === key)) { results.push({ key, value: combined }); }