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.
This commit is contained in:
hypercross 2026-07-13 10:22:22 +08:00
parent c524dd5867
commit 4c2eb65470
2 changed files with 37 additions and 42 deletions

View File

@ -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;

View File

@ -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<string, Array<{ tag: string; target: string; v
/** Set of $vars currently being re-evaluated (cycle guard) */
const inFlight = new Set<string>();
/** 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 });
}