From a26d53cbebb7fc824a2dd3e5e5e8b3b6142ac686 Mon Sep 17 00:00:00 2001 From: hyper Date: Sat, 12 Sep 2026 21:59:45 +0800 Subject: [PATCH] fix(journal): keep variable bases in the stream The stream only carried combined values, so hydrating a session lost the raw base and rebuildReactivityFromStore re-applied tag modifiers on top of already-combined values (e.g. $hp 70 became 90). Publish the raw base alongside the combined value on var messages, persist it in variableBases, and rebuild engine state from exact bases. Derive tag activations from base tagmaps so tagmap-valued modifier targets are not re-triggered on every rebuild. Seed declared variables only after hydration and only for keys absent from the store, so persisted values are no longer clobbered on load and reloads stop appending duplicate var messages. --- src/cli/completions/variable-system.test.ts | 132 +++++++++++++++++++ src/components/journal/command-dispatcher.ts | 12 +- src/components/journal/completions.ts | 65 ++++++--- src/components/journal/types/var.tsx | 10 ++ src/components/journal/var-reactivity.ts | 88 +++++++++---- src/components/stores/journalStream.ts | 29 +++- 6 files changed, 290 insertions(+), 46 deletions(-) diff --git a/src/cli/completions/variable-system.test.ts b/src/cli/completions/variable-system.test.ts index 4b9ae71..cff1e37 100644 --- a/src/cli/completions/variable-system.test.ts +++ b/src/cli/completions/variable-system.test.ts @@ -39,6 +39,7 @@ import { getMods, getDeclExpr, extractDependencies, + rebuildReactivityFromStore, } from "../../components/journal/var-reactivity"; import { parseInput } from "../../components/journal/command-parser"; import { @@ -539,6 +540,33 @@ describe("var-reactivity", () => { expect(hpResult?.value).toBe("60"); // 12 * 5 }); + test("reports the base alongside the combined value", () => { + initReactivity({ + declarations: [{ key: "$hp", expression: "$con * 5" }], + tagModifiers: [], + }); + + const results = computeInitialValues({ $con: "12" }); + const hp = results.find((r) => r.key === "$hp"); + expect(hp?.value).toBe("60"); + expect(hp?.base).toBe("60"); + }); + + test("skips variables already present in the store", () => { + initReactivity({ + declarations: [ + { key: "$hp", expression: "10 + 5" }, + { key: "$ac", expression: "15" }, + ], + tagModifiers: [], + }); + + const results = computeInitialValues({ $hp: "99" }, new Set(["$hp"])); + // Existing value is left untouched; only missing keys are computed. + expect(results.find((r) => r.key === "$hp")).toBeUndefined(); + expect(results.find((r) => r.key === "$ac")?.value).toBe("15"); + }); + test("returns empty array when no declarations", () => { initReactivity({ declarations: [], tagModifiers: [] }); const results = computeInitialValues(store()); @@ -570,6 +598,89 @@ describe("var-reactivity", () => { }); }); + describe("rebuildReactivityFromStore", () => { + test("restores bases exactly without double-counting modifiers", () => { + initReactivity({ + declarations: [], + tagModifiers: [ + { tag: "#warrior", target: "$mod_hp", expression: "20", threshold: 1 }, + ], + }); + + // `variables` holds combined values; `bases` holds what derived them. + const hydrated = { $class: "#warrior:1", $mod_hp: "20" }; + const bases = { $class: "#warrior:1", $mod_hp: "0" }; + rebuildReactivityFromStore(hydrated, bases); + + expect(getCombined("$mod_hp", hydrated)).toBe("20"); + }); + + test("recovers a non-zero base and keeps modifiers applied", () => { + initReactivity({ + declarations: [], + tagModifiers: [ + { tag: "#warrior", target: "$hp", expression: "20", threshold: 1 }, + ], + }); + + const hydrated = { $class: "#warrior:1", $hp: "70" }; // base 50 + 20 + const bases = { $class: "#warrior:1", $hp: "50" }; + rebuildReactivityFromStore(hydrated, bases); + + expect(getCombined("$hp", hydrated)).toBe("70"); + + // A later /set replaces the base; the modifier still applies. + setBase("$hp", "60"); + expect(getCombined("$hp", hydrated)).toBe("80"); + }); + + test("deactivates a hydrated modifier on a later tag change", () => { + initReactivity({ + declarations: [], + tagModifiers: [ + { tag: "#warrior", target: "$mod_hp", expression: "20", threshold: 1 }, + ], + }); + + const hydrated = { $class: "#warrior:1", $mod_hp: "20" }; + const bases = { $class: "#warrior:1", $mod_hp: "0" }; + rebuildReactivityFromStore(hydrated, bases); + + setBase("$class", "#mage:1"); + const cascade = computeCascade("$class", "#warrior:1", { + ...hydrated, + $class: "#mage:1", + }); + + expect(cascade.find((r) => r.key === "$mod_hp")?.value).toBe("0"); + expect(getCombined("$mod_hp", { $class: "#mage:1" })).toBe("0"); + }); + + test("rebuilds a tagmap variable's base from its published base", () => { + initReactivity({ + declarations: [], + tagModifiers: [ + { tag: "#warrior", target: "$pool", expression: "2", threshold: 1 }, + ], + }); + + // $pool is a tagmap variable that receives a +2 #warrior tagmap mod. + const hydrated = { $class: "#warrior:1", $pool: "#druid:3;#warrior:2" }; + const bases = { $class: "#warrior:1", $pool: "#druid:3" }; + rebuildReactivityFromStore(hydrated, bases); + + expect(getCombined("$pool", hydrated)).toBe("#druid:3;#warrior:2"); + }); + + test("is idempotent when called twice", () => { + initReactivity({ declarations: [], tagModifiers: [] }); + const hydrated = { $hp: "42" }; + rebuildReactivityFromStore(hydrated, { $hp: "42" }); + rebuildReactivityFromStore(hydrated, { $hp: "42" }); + expect(getCombined("$hp", hydrated)).toBe("42"); + }); + }); + describe("computeCascade — tag activation/deactivation", () => { test("activates tag modifiers when a tag value is set", () => { initReactivity({ @@ -605,6 +716,27 @@ describe("var-reactivity", () => { expect(modStr?.value).toBe("5"); }); + test("reports the raw base alongside the combined value", () => { + initReactivity({ + declarations: [], + tagModifiers: [ + { tag: "#warrior", target: "$hp", expression: "20", threshold: 1 }, + ], + }); + + setBase("$hp", "50"); + setBase("$class", "#warrior:1"); + const cascade = computeCascade( + "$class", + undefined, + store({ $class: "#warrior:1" }), + ); + + const hp = cascade.find((r) => r.key === "$hp"); + expect(hp?.value).toBe("70"); + expect(hp?.base).toBe("50"); + }); + test("deactivates tag modifiers when tag is removed from tagmap", () => { initReactivity({ declarations: [], diff --git a/src/components/journal/command-dispatcher.ts b/src/components/journal/command-dispatcher.ts index a7079d4..6eb1a58 100644 --- a/src/components/journal/command-dispatcher.ts +++ b/src/components/journal/command-dispatcher.ts @@ -236,6 +236,7 @@ function dispatchSet( action: "set", key: change.key, value: change.value, + base: change.base, }); } } catch (e) { @@ -244,10 +245,15 @@ function dispatchSet( } // Send the direct set last so cascade messages arrive first. - // Use getCombined with stream fallback so the stream store receives the - // correct value (base + active mods) rather than just the raw base. + // Publish the combined value for display plus the raw base so hydrated + // clients can rebuild the engine exactly (see rebuildReactivityFromStore). const combinedValue = getCombined(key, ctx.variables); - const r1 = sendMessage("var", { action: "set", key, value: combinedValue }); + const r1 = sendMessage("var", { + action: "set", + key, + value: combinedValue, + base: newValue, + }); const u1 = unwrap(r1); if (!u1.ok) return u1; diff --git a/src/components/journal/completions.ts b/src/components/journal/completions.ts index 17cae7c..4febea3 100644 --- a/src/components/journal/completions.ts +++ b/src/components/journal/completions.ts @@ -30,8 +30,13 @@ import type { VarDeclaration, TagModifier, } from "../../cli/completions/types"; -import { initReactivity, computeInitialValues } from "./var-reactivity"; -import { sendMessage, journalStreamState } from "../stores/journalStream"; +import { initReactivity, computeInitialValues, rebuildReactivityFromStore } from "./var-reactivity"; +import { + sendMessage, + journalStreamState, + hasHydrated, + setHydrationListener, +} from "../stores/journalStream"; export type { VarDeclaration, TagModifier }; @@ -133,6 +138,37 @@ async function scanClientSide(): Promise { // ------------------- Init (runs eagerly at import time) ------------------- +// Declarations finish loading asynchronously, as does worker hydration. Either +// may land first, so seeding is driven by whichever becomes ready last. +let _reactivityReady = false; + +/** + * Initialize the reactivity engine from the loaded declarations. If the store + * has already been hydrated, rebuild from it and seed missing values now. + */ +function bootstrapReactivity( + declarations: VarDeclaration[], + tagModifiers: TagModifier[], +): void { + initReactivity({ declarations, tagModifiers }); + _reactivityReady = true; + + if (hasHydrated()) { + rebuildReactivityFromStore(journalStreamState.variables); + seedDeclaredVariables(); + } +} + +// Seeding runs after every hydration snapshot so declared variables are +// computed against the real session store rather than an empty one. +setHydrationListener(() => { + try { + seedDeclaredVariables(); + } catch (e) { + console.warn("[completions] seed error:", e); + } +}); + // Using a top-level IIFE so the promise starts immediately const _initPromise: Promise = (async () => { // 1. Try server first (CLI mode) @@ -144,11 +180,7 @@ const _initPromise: Promise = (async () => { await tryServerRegistry(); setCompletionsState({ status: "loaded", data: serverData }); try { - initReactivity({ - declarations: serverData.declarations, - tagModifiers: serverData.tagModifiers, - }); - seedDeclaredVariables(); + bootstrapReactivity(serverData.declarations, serverData.tagModifiers); } catch (e) { console.warn("[completions] reactivity init error:", e); } @@ -161,11 +193,7 @@ const _initPromise: Promise = (async () => { if (data.dice.length > 0 || data.links.length > 0) { setCompletionsState({ status: "loaded", data }); try { - initReactivity({ - declarations: data.declarations, - tagModifiers: data.tagModifiers, - }); - seedDeclaredVariables(); + bootstrapReactivity(data.declarations, data.tagModifiers); } catch (e) { console.warn("[completions] reactivity init error:", e); } @@ -219,8 +247,15 @@ export function useJournalCompletions(): { // --------------------------------------------------------------------------- function seedDeclaredVariables(): void { - const initial = computeInitialValues(journalStreamState.variables); - for (const { key, value } of initial) { - sendMessage("var", { action: "set", key, value }); + if (!_reactivityReady) return; + // Only seed into a live session. Before connect the store is empty and + // publishing derived defaults would clobber the session once it joins. + if (!journalStreamState.connected) return; + + const store = journalStreamState.variables; + const existing = new Set(Object.keys(store)); + const initial = computeInitialValues(store, existing); + for (const { key, value, base } of initial) { + sendMessage("var", { action: "set", key, value, base }); } } diff --git a/src/components/journal/types/var.tsx b/src/components/journal/types/var.tsx index a99fc64..1371f5e 100644 --- a/src/components/journal/types/var.tsx +++ b/src/components/journal/types/var.tsx @@ -18,6 +18,10 @@ const schema = z.object({ action: z.enum(["set", "del"]), key: z.string().min(1), value: z.string().optional(), + /** The raw base value (before tag modifiers). The store's `variables` map + * holds combined values, so this is what the reactivity engine rebuilds + * from on hydration. */ + base: z.string().optional(), }); export type VarPayload = z.infer; @@ -53,8 +57,14 @@ registerMessageType({ produce((s) => { if (p.action === "del") { delete s.variables[p.key]; + delete s.variableBases[p.key]; } else if (p.value !== undefined) { s.variables[p.key] = p.value; + if (p.base !== undefined) { + s.variableBases[p.key] = p.base; + } else { + delete s.variableBases[p.key]; + } } }), ); diff --git a/src/components/journal/var-reactivity.ts b/src/components/journal/var-reactivity.ts index 1eea7da..71098f8 100644 --- a/src/components/journal/var-reactivity.ts +++ b/src/components/journal/var-reactivity.ts @@ -35,6 +35,18 @@ export interface VarReactivityState { export type VariableStore = Record; +/** + * A variable update produced by the engine: the combined `value` to publish + * for display, plus the raw `base` it was derived from. The base is carried on + * the stream so a hydrated client can rebuild the engine without having to + * reverse-engineer base values out of combined ones. + */ +export interface VarChange { + key: string; + value: string; + base: string; +} + /** A single modifier applied to a target variable. */ export interface ActiveMod { tag: string; // "#warrior" @@ -265,22 +277,34 @@ export function setBase(key: string, value: string): void { * from the hydrated stream variable store. Must be called after replayReducers * so that tag activations are correctly tracked for subsequent cascade * computations. + * + * The stream store holds *combined* values (base + active modifiers) alongside + * the raw `base` each was derived from, so bases are restored exactly rather + * than reconstructed by subtracting modifiers back out. */ -export function rebuildReactivityFromStore(variables: VariableStore): void { +export function rebuildReactivityFromStore( + variables: VariableStore, + bases?: VariableStore, +): void { if (!tagModMap) return; - for (const [key, value] of Object.entries(variables)) { - if (!baseValues.has(key)) { - baseValues.set(key, value); - } + // Reset, then restore exact bases from the store's base map. + baseValues.clear(); + numericMods.clear(); + tagMapMods.clear(); + sourceActivations.clear(); - const tagMap = isTagMapValue(value) ? parseTagMap(value) : null; - if (tagMap && !sourceActivations.has(key)) { - // Activate all tags in the tagmap - const added = applyTagMapActivations(key, {}, tagMap, variables); - // We don't return the added entries here — they'll be applied - // to the store separately - } + for (const [key, value] of Object.entries(variables)) { + baseValues.set(key, bases?.[key] ?? value); + } + + // Re-attach tag modifiers from each source's *base* tagmap. Using the + // combined value here would re-trigger a modifier that merely added a tag to + // the source (feedback), inflating counts on every rebuild. + for (const [key, value] of Object.entries(variables)) { + const source = bases?.[key] ?? value; + const tagMap = isTagMapValue(source) ? parseTagMap(source) : null; + if (tagMap) applyTagMapActivations(key, {}, tagMap, variables); } } @@ -296,12 +320,12 @@ export function computeCascade( changedVar: string, oldValue: string | undefined, currentVars: VariableStore, -): Array<{ key: string; value: string }> { +): VarChange[] { if (!depGraph || !declExprs || !tagModMap) { return []; } - const results: Array<{ key: string; value: string }> = []; + const results: VarChange[] = []; // ---- Tag activation/deactivation ---- const newValue = getCombined(changedVar, currentVars); @@ -325,19 +349,27 @@ export function computeCascade( /** * Compute initial values for all declared variables. - * Called once after initReactivity() to seed the store. + * Called after initReactivity() to seed the store. + * + * @param existing - declared keys already present in the store; these are + * skipped so hydrated/overridden session values are not clobbered. */ export function computeInitialValues( currentVars: VariableStore, -): Array<{ key: string; value: string }> { + existing?: ReadonlySet, +): VarChange[] { if (!declExprs) return []; const allKeys = [...declExprs.keys()]; const sorted = topoSortAffected(new Set(allKeys)); - const results: Array<{ key: string; value: string }> = []; + const results: VarChange[] = []; for (const key of sorted) { + // Variables already in the store were seeded (or overridden) previously; + // recomputing them would clobber session state on every reload. + if (existing?.has(key)) continue; + const expr = declExprs.get(key); if (!expr) continue; @@ -366,7 +398,7 @@ export function computeInitialValues( // Always emit the combined value for this key const combined = getCombined(key, currentVars); if (!results.some((x) => x.key === key)) { - results.push({ key, value: combined }); + results.push({ key, value: combined, base: rawValue }); } } catch { // skip failed evaluations at init time @@ -389,10 +421,10 @@ function applyTagMapActivations( oldTagMap: Record, newTagMap: Record, fallback: VariableStore, -): Array<{ key: string; value: string }> { +): VarChange[] { if (!tagModMap) return []; - const results: Array<{ key: string; value: string }> = []; + const results: VarChange[] = []; const allTags = new Set([...Object.keys(oldTagMap), ...Object.keys(newTagMap)]); for (const tag of allTags) { @@ -467,11 +499,12 @@ function applyTagMapActivations( // Emit new combined value const combined = getCombined(mod.target, fallback); + const base = baseValues.get(mod.target) ?? combined; const existing = results.findIndex((r) => r.key === mod.target); if (existing >= 0) { - results[existing] = { key: mod.target, value: combined }; + results[existing] = { key: mod.target, value: combined, base }; } else { - results.push({ key: mod.target, value: combined }); + results.push({ key: mod.target, value: combined, base }); } } catch { // skip failed modifier @@ -512,11 +545,12 @@ function applyTagMapActivations( // Emit updated combined value const combined = getCombined(entry.target, fallback); + const base = baseValues.get(entry.target) ?? combined; const existing = results.findIndex((r) => r.key === entry.target); if (existing >= 0) { - results[existing] = { key: entry.target, value: combined }; + results[existing] = { key: entry.target, value: combined, base }; } else { - results.push({ key: entry.target, value: combined }); + results.push({ key: entry.target, value: combined, base }); } } } @@ -537,7 +571,7 @@ function applyTagMapActivations( function reevaluateDependents( changedVar: string, fallback: VariableStore, -): Array<{ key: string; value: string }> { +): VarChange[] { if (!depGraph || !declExprs) return []; // Collect all dependents reachable from changedVar (BFS) @@ -556,7 +590,7 @@ function reevaluateDependents( } const sorted = topoSortAffected(affected); - const results: Array<{ key: string; value: string }> = []; + const results: VarChange[] = []; for (const key of sorted) { if (inFlight.has(key)) { @@ -599,7 +633,7 @@ function reevaluateDependents( // Emit combined value const combined = getCombined(key, fallback); if (!results.some((x) => x.key === key)) { - results.push({ key, value: combined }); + results.push({ key, value: combined, base: rawValue }); } } finally { inFlight.delete(key); diff --git a/src/components/stores/journalStream.ts b/src/components/stores/journalStream.ts index 6a15da3..5e9d710 100644 --- a/src/components/stores/journalStream.ts +++ b/src/components/stores/journalStream.ts @@ -46,6 +46,8 @@ export interface JournalStreamState { brokerUrl: string | null; players: Record; variables: Record; + /** Raw base values (before tag modifiers), keyed the same as `variables`. */ + variableBases: Record; } export interface SessionMeta { @@ -98,6 +100,7 @@ const [state, setState] = createStore({ brokerUrl: persisted.brokerUrl, players: {}, variables: {}, + variableBases: {}, }); if (initialName && !urlParams.playerName) syncUrlParam("player", initialName); @@ -128,11 +131,33 @@ function replayReducers(): void { // Reset derived state setState("revealedPaths", {}); setState("variables", {}); + setState("variableBases", {}); for (const msg of state.messages) { runReducer(msg); } } +// --------------------------------------------------------------------------- +// Hydration signal +// --------------------------------------------------------------------------- + +let _hydrated = false; +let _hydrationListener: (() => void) | null = null; + +/** True once the worker has delivered a full state snapshot. */ +export function hasHydrated(): boolean { + return _hydrated; +} + +/** + * Register a callback invoked after each full state snapshot has been applied + * (reducers replayed and reactivity rebuilt). Used to seed declared variables + * once the hydrated store is available. + */ +export function setHydrationListener(cb: (() => void) | null): void { + _hydrationListener = cb; +} + // --------------------------------------------------------------------------- // Worker patch handler // --------------------------------------------------------------------------- @@ -165,7 +190,9 @@ function handlePatch(patch: WorkerPatch): void { replayReducers(); // Rebuild local reactivity engine state so tag activations from // hydrated variables are tracked for subsequent cascade computations. - rebuildReactivityFromStore(state.variables); + rebuildReactivityFromStore(state.variables, state.variableBases); + _hydrated = true; + _hydrationListener?.(); break; }