From 642ee0d332a507562f35e3d999ab277fdfa45b95 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 16:34:33 +0000 Subject: [PATCH] Fix the nine remaining defects found in review Each reproduced before the change and verified after, with a test that fails without its fix. Step never refreshed the scrub slider, so its range sat one entry behind the history. Live, the thumb is already at max, so End and a rightward drag emitted no input event and the newest step could not be reached at all. Run/Pause and Reset already refreshed it. assertions was the one serialized Diagram field _clearAll missed, so design tests followed the user through File > New, every template load and the restored-session Discard, then into autosave, Save as JSON, the .econ export and the share link, where cli.js --check ran them against nodes that no longer existed. _exportFilename kept only ASCII, so a Chinese or Cyrillic name became a bare ".svg" that the browser renames to "svg.svg": every export of such a diagram collided on one filename, .econ lost its type to "econ.txt", and File > Save lost the name too. Unicode letters and digits are kept now, with a "diagram" fallback so the result can never begin with a dot. The Library's separate sanitizer is aligned on the same rule. Sparkline mapped v/max, putting anything below zero off the bottom of the canvas. A register holding a net or deficit, which is most of what a register computes, drew nothing at all under a false "max: 1"; a series crossing zero drew only its positive half, which looks like real data. The scale spans zero now, positive-only series are unchanged, and the caption reports the true extremes. The properties panel had no scrub-aware read path at all. The canvas and the timeline each had one, so scrubbing showed the replayed step beside a hero card still reporting the end of the run: two numbers for one node. The hero, the live readout and the sparkline now read the previewed entry, and the amount field stops mirroring while scrubbing so a past value cannot be typed back into the model. A node labelled with a .econ head keyword broke its own connections, because dslParse dispatches on tokens[0] before scanning for an arrow. Seventeen of the nineteen heads threw on reload; economy and assert matched their handlers and dropped the connection silently. Such a label serializes quoted now. Only the source side and only lowercase were ever affected, since default labels are capitalized. Trader flows were attributed backwards. The engine books what A pays under the incoming leg and what B pays under the outgoing one, so reading direction off sourceId/targetId called B's payment income, never credited either partner with what it received, and gave the trader two rows for resources it never held. Rebuilt from the real payer and payee, which the positional ins[i]/outs[i] pairing makes recoverable; both partners now account to a zero residual. Group and connection handles were painted below nodeLayer, so a node on a corner hid them while the press still resized. The hit-test precedence was never the bug and is unchanged; the handles move to an overlay above the content so the visible region and the hot region are the same again. Renaming a parameter onto an existing name overwrote the other and deleted this one. The check spans the whole shared store, params, custom variables, register labels and named state connections, since a duplicate collapses there whichever editor produced it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SH7ouoNymubArm7VEgALHg --- js/app-export.js | 13 ++- js/app-fields.js | 30 ++++++- js/app-library.js | 2 +- js/app-props.js | 40 +++++++++ js/app.js | 15 ++++ js/attribution.js | 33 ++++++++ js/charts.js | 48 ++++++++--- js/dsl.js | 20 ++++- js/renderer.js | 28 +++++-- test/run.js | 65 ++++++++++++++ test/smoke.js | 210 ++++++++++++++++++++++++++++++++++++++++++++++ 11 files changed, 478 insertions(+), 26 deletions(-) diff --git a/js/app-export.js b/js/app-export.js index daa0267..0a3b65f 100644 --- a/js/app-export.js +++ b/js/app-export.js @@ -10,9 +10,20 @@ class AppExport { // ── Export ──────────────────────────────────────────────────────────────── + // Backs every download in the app: SVG, PNG, CSV, .econ, the standalone + // module, File > Save, and the three analysis exports. The old ASCII-only + // class turned every character of a non-Latin name into an underscore and + // then stripped them, returning a bare ".svg" that the browser renames to + // "svg.svg", so a Chinese or Japanese diagram lost its name entirely and + // every export collided on one filename. Accented Latin fared little better + // ("Economie" losing its leading E). Unicode letters and digits are kept, and + // an empty stem falls back to "diagram" so the result can never start with a + // dot: a dotfile also cost .econ its extension, the browser rewriting it to + // "econ.txt" so it no longer matched the app's own file picker. _exportFilename(ext) { const raw = this.diagram.meta.name || 'diagram'; - return raw.replace(/[^a-z0-9_\-]/gi, '_').replace(/_+/g, '_').replace(/^_|_$/g, '') + '.' + ext; + const stem = raw.replace(/[^\p{L}\p{N}_-]+/gu, '_').replace(/_+/g, '_').replace(/^_|_$/g, ''); + return (stem || 'diagram') + '.' + ext; } // Build a standalone snapshot of the diagram, cloned from the live canvas so diff --git a/js/app-fields.js b/js/app-fields.js index cc51297..af557f2 100644 --- a/js/app-fields.js +++ b/js/app-fields.js @@ -86,7 +86,7 @@ class AppFields { const val = document.createElement('div'); val.className = 'hero-card-value'; val.id = 'props-hero-value'; - val.textContent = String(node.displayCount); + val.textContent = String(this._panelValueOf(node)); card.appendChild(val); let subText = ''; @@ -407,6 +407,21 @@ class AppFields { // ── Live update helpers ─────────────────────────────────────────────────── + // What the properties panel should display for a node right now. While the + // timeline is scrubbed that is the recorded value at the previewed step, not + // the live model. The canvas and the timeline both had a scrub-aware read + // path (Renderer._scrubSnap, Timeline.setScrub); the panel never did, so its + // hero card went on reporting the end of the run while the node beside it + // showed the replayed step. Two different numbers for the same node. + _panelValueOf(node) { + if (this._scrubIndex != null) { + const entry = this.engine.history[this._scrubIndex]; + const v = entry && entry.snap ? entry.snap[node.id] : undefined; + if (v !== undefined) return v; + } + return node.displayCount; + } + _refreshResourceCount() { // A diagram-rail feature (Parameters, Custom variables, Artificial player, // Design tests) borrows the properties panel without clearing the node @@ -420,7 +435,7 @@ class AppFields { // Big hero readout tracks the live value for every node kind. const hero = document.getElementById('props-hero-value'); - if (hero) hero.textContent = String(node.displayCount); + if (hero) hero.textContent = String(this._panelValueOf(node)); // Same exclusions the amount field is built under (app-props.js). A Trader // has no amount row, so its first number input is the End/goal value, and // the refresh below was overwriting that goal with the trader's count. @@ -429,7 +444,9 @@ class AppFields { // With a node owning the panel, the first number input is the Resources field. const inp = document.querySelector('#props-content input[type="number"]'); - if (inp && document.activeElement !== inp) inp.value = node.resources; + // Not while scrubbing: the field is editable, and writing a replayed value + // into it invites the user to commit a past step as the current amount. + if (inp && document.activeElement !== inp && this._scrubIndex == null) inp.value = node.resources; // That field means two different things either side of step 0, and the // panel is not rebuilt as the run advances, so its label has to be patched @@ -441,7 +458,12 @@ class AppFields { } } - _updateSparklines() { for (const sl of this._sparklines.values()) sl.update(); } + _updateSparklines() { + for (const sl of this._sparklines.values()) { + sl.scrubIndex = this._scrubIndex; + sl.update(); + } + } _clearSparklines() { for (const sl of this._sparklines.values()) sl.destroy(); this._sparklines.clear(); } } diff --git a/js/app-library.js b/js/app-library.js index 2929a56..a7e8e92 100644 --- a/js/app-library.js +++ b/js/app-library.js @@ -306,7 +306,7 @@ class AppLibrary { add('Export as JSON', 'download', () => { const a = Object.assign(document.createElement('a'), { href: URL.createObjectURL(new Blob([entry.json], { type: 'application/json' })), - download: `${(entry.name || 'diagram').replace(/[^\w\-]+/g, '_')}.json`, + download: `${(entry.name || 'diagram').replace(/[^\p{L}\p{N}_-]+/gu, '_') || 'diagram'}.json`, }); a.click(); }); diff --git a/js/app-props.js b/js/app-props.js index c4ecc70..694e2c1 100644 --- a/js/app-props.js +++ b/js/app-props.js @@ -624,6 +624,28 @@ class AppProps { } // Named constants available to all formulas. + // Every name that already resolves in the shared variable store, which the + // engine builds from parameters, custom variables, named state connections + // and register labels (SimEngine._updateVariables). Renaming has to consult + // all four, not just the editor being typed in: two rows agreeing on a name + // collapse to one value at simulation time whichever editor they came from. + // `except` is the name being edited, so a no-op rename is not a collision. + _nameTaken(name, except) { + const d = this.diagram; + if (name === except) return false; + for (const k of Object.keys(d.params || {})) if (k !== except && k === name) return true; + for (const rv of d.customVars || []) if (rv.name !== except && rv.name === name) return true; + for (const n of d.nodes.values()) { + if (n.type === NodeType.REGISTER && n.label !== except && n.label === name) return true; + } + for (const c of d.connections.values()) { + if (c.type !== ConnectionType.STATE) continue; + const vn = c.variableName || c.label; + if (vn && vn !== except && vn === name) return true; + } + return false; + } + _paramsEditor(panel) { this._info(panel, 'Named constants available to all formulas (e.g. growth_rate * pool).'); const params = this.diagram.params; @@ -637,6 +659,17 @@ class AppProps { const nk = ki.value.trim(); if (!nk || nk === key) { ki.value = key; return; } if (!VALID_IDENT.test(nk)) { ki.value = key; return; } + // Without this the assignment below overwrote the other parameter and + // deleted this one, so two rows silently became one: `gold` took this + // row's value, the original gold was gone, and every formula reading + // gold changed meaning with only undo to recover. The two checks above + // already reject a bad name by reverting the field, so a colliding name + // reverts the same way rather than destroying data. + if (this._nameTaken(nk, key)) { + ki.value = key; + this._toast(`"${nk}" is already used by another parameter or variable.`); + return; + } params[nk] = params[key]; delete params[key]; this._renderProps(); @@ -876,6 +909,13 @@ class AppProps { name.addEventListener('blur', () => { const nk = name.value.trim(); if (!nk || !VALID_IDENT.test(nk)) { name.value = rv.name; return; } + // Both rows survive here, but the shared store keeps one value for the + // name, so the duplicate silently shadows the other at run time. + if (this._nameTaken(nk, rv.name)) { + name.value = rv.name; + this._toast(`"${nk}" is already used by another parameter or variable.`); + return; + } if (nk !== rv.name) { rv.name = nk; this._commit(); } }); diff --git a/js/app.js b/js/app.js index 1b10e03..d3b9c09 100644 --- a/js/app.js +++ b/js/app.js @@ -647,6 +647,8 @@ class App { this.renderer.setScrub(entry.snap); if (this._timelineVisible) this.timeline.setScrub(entry.step); document.getElementById('step-counter').textContent = `Step ${entry.step} (replay)`; + this._refreshResourceCount(); + this._updateSparklines(); this._refreshScrubber(); } @@ -661,6 +663,8 @@ class App { if (wasScrubbing) { document.getElementById('step-counter').textContent = `Step ${this.engine.step}`; this.renderer.render(); + this._refreshResourceCount(); + this._updateSparklines(); if (this._activeFeature === 'monitor') this._renderProps(); } this._refreshScrubber(); @@ -721,6 +725,12 @@ class App { this.diagram.timeMode = 'sync'; this.diagram.seed = ''; this.diagram.aiPlayer = { enabled: false, rules: [] }; + // Assertions are a serialized Diagram field like the rest, and were the one + // this missed. They survived File > New, every template load and the + // restored-session Discard, then went straight into autosave, Save as JSON, + // the .econ export and the share link, so cli.js --check on that file ran a + // previous model's tests against nodes that no longer exist. + this.diagram.assertions = []; this.diagram.meta = Diagram.defaultMeta(); this._applyMeta(); this._dropScenarioState(); @@ -1241,6 +1251,11 @@ class App { document.getElementById('btn-step').addEventListener('click', () => { this._exitScrub(); this.engine.doStep(); + // Run/Pause and Reset both refresh the scrubber; Step did not, so its + // range stayed one entry behind the history. While Live the thumb already + // sits at max, so pressing End or dragging it right emitted no input + // event and the newest step could not be reached at all. + this._refreshScrubber(); }); const runBtn = document.getElementById('btn-run'); diff --git a/js/attribution.js b/js/attribution.js index b779e23..7b5e876 100644 --- a/js/attribution.js +++ b/js/attribution.js @@ -42,12 +42,45 @@ function attributeChange(diagram, history, nodeId, index) { const entries = []; const nameOf = id => { const n = diagram.nodes.get(id); return (n && n.label) || id || '?'; }; + // A trader holds nothing: it swaps between the two partners of a paired + // incoming/outgoing connection (SimEngine._fireTrader pairs ins[i] with + // outs[i], so the pairing is reconstructible here). Crucially the flow booked + // on the OUTGOING leg is what that partner PAID, not what it received, so + // reading direction off sourceId/targetId reported a payment as income, never + // credited either partner with what it got, and gave the trader itself two + // rows for resources that never touched it. Map each leg to its real payer + // and payee instead. + const traderLegs = new Map(); // connId -> { payer, payee, trader } + for (const t of diagram.nodes.values()) { + if (t.type !== NodeType.TRADER) continue; + const ins = diagram.incoming(t.id).filter(c => c.type === ConnectionType.RESOURCE); + const outs = diagram.outgoing(t.id).filter(c => c.type === ConnectionType.RESOURCE); + for (let i = 0; i < Math.min(ins.length, outs.length); i++) { + const cin = ins[i], cout = outs[i]; + traderLegs.set(cin.id, { payer: cin.sourceId, payee: cout.targetId, trader: t.id }); + traderLegs.set(cout.id, { payer: cout.targetId, payee: cin.sourceId, trader: t.id }); + } + } + const isRegister = node.type === NodeType.REGISTER; if (!isRegister) { for (const [connId, amt] of Object.entries(flows.conns)) { if (!amt) continue; const c = diagram.connections.get(connId); if (!c) continue; + const leg = traderLegs.get(connId); + if (leg) { + // The trader's own charted value is its trade count, not a balance, so + // these resources are none of its business. + if (nodeId === leg.trader) continue; + if (nodeId === leg.payer && node.type !== NodeType.DRAIN) { + entries.push({ kind: 'flow out', connId, amount: -amt, label: `to ${nameOf(leg.payee)}` }); + } + if (nodeId === leg.payee && node.type !== NodeType.SOURCE) { + entries.push({ kind: 'flow in', connId, amount: amt, label: `from ${nameOf(leg.payer)}` }); + } + continue; + } // A drain's charted value only ever grows with intake; a limited // source's stock only ever falls with output. Everything else counts // both directions. (A self-loop connection nets to zero via two rows.) diff --git a/js/charts.js b/js/charts.js index 290ae93..6531ba1 100644 --- a/js/charts.js +++ b/js/charts.js @@ -7,11 +7,17 @@ class Sparkline { this.canvas.width = 260; this.canvas.height = 60; this.canvas.className = 'sparkline'; + this.scrubIndex = null; // history index being previewed, or null when live container.appendChild(this.canvas); } update() { - const history = this.engine.history; + // While the timeline is scrubbed the panel must show the replayed step, not + // the end of the run. scrubIndex is set by the app before each update and + // cleared on exit; null means live. + const history = this.scrubIndex == null + ? this.engine.history + : this.engine.history.slice(0, this.scrubIndex + 1); // One point draws nothing but an empty box and a stray midline, which read // as a broken chart at the top of the properties panel. Stay collapsed // until there are two points to join, then reveal. @@ -23,14 +29,30 @@ class Sparkline { const w = this.canvas.width, h = this.canvas.height; ctx.clearRect(0, 0, w, h); - const max = Math.max(...values, 1); + // The scale has to span zero. Mapping v/max put anything below zero at + // y > h, off the bottom of the canvas: a register holding a net or deficit, + // which is most of what a register computes, drew nothing at all and was + // captioned "max: 1" because the old max floored at 1. A series crossing + // zero was worse, drawing only its positive half so the trace appeared to + // start mid-chart out of nowhere, looking like real data. Extending the + // range to include zero leaves a positive-only series scaled exactly as + // before (min lands on 0) and brings the rest onto the canvas. + const rawMax = Math.max(...values); + const rawMin = Math.min(...values); + const max = Math.max(rawMax, 0); + const min = Math.min(rawMin, 0); + const range = (max - min) || 1; + const yOf = (v) => h - ((v - min) / range) * (h - 4) - 2; + ctx.fillStyle = '#0d0e11'; ctx.fillRect(0, 0, w, h); - // Grid line at midpoint + // Reference line: the zero crossing once the series goes negative, where it + // actually means something, otherwise the midpoint as before. + const yRef = rawMin < 0 ? yOf(0) : h / 2; ctx.strokeStyle = '#22252e'; ctx.lineWidth = 1; - ctx.beginPath(); ctx.moveTo(0, h / 2); ctx.lineTo(w, h / 2); ctx.stroke(); + ctx.beginPath(); ctx.moveTo(0, yRef); ctx.lineTo(w, yRef); ctx.stroke(); const step = w / (values.length - 1); @@ -39,25 +61,29 @@ class Sparkline { ctx.lineWidth = 1.5; values.forEach((v, i) => { const x = i * step; - const y = h - (v / max) * (h - 4) - 2; + const y = yOf(v); if (i === 0) ctx.moveTo(x, y); else ctx.lineTo(x, y); }); ctx.stroke(); - // Fill under line - ctx.lineTo((values.length - 1) * step, h); - ctx.lineTo(0, h); + // Fill between the line and zero, so a dip below zero fills upward to the + // baseline rather than flooding the whole panel. + const yBase = yOf(0); + ctx.lineTo((values.length - 1) * step, yBase); + ctx.lineTo(0, yBase); ctx.closePath(); ctx.fillStyle = 'rgba(182,233,77,0.10)'; ctx.fill(); - // Current value label + // Labels use the real extremes, not the zero-extended ones: an all-negative + // series reading "max: 0" would be as wrong as the old "max: 1". + const fmt = (n) => (Number.isInteger(n) ? String(n) : n.toFixed(2)); ctx.fillStyle = '#b6e94d'; ctx.font = "11px 'JetBrains Mono', monospace"; const last = values[values.length - 1]; - ctx.fillText(`${last}`, w - 30, 12); + ctx.fillText(`${fmt(last)}`, w - 30, 12); ctx.fillStyle = '#8a90a0'; - ctx.fillText(`max: ${max}`, 4, 12); + ctx.fillText(rawMin < 0 ? `${fmt(rawMin)} to ${fmt(rawMax)}` : `max: ${fmt(rawMax)}`, 4, 12); } destroy() { this.canvas.remove(); } diff --git a/js/dsl.js b/js/dsl.js index 9a1f07a..508ce8b 100644 --- a/js/dsl.js +++ b/js/dsl.js @@ -150,6 +150,23 @@ function _econParseValue(raw, lineNo) { // Assign every node a unique, human-readable reference name derived from its // label, disambiguating duplicates with #2, #3, … in declaration order. +// Every word that starts a statement. dslParse dispatches on tokens[0] before it +// scans the line for an arrow, so a node whose label is one of these and which +// is the SOURCE of a connection would emit `pool -> Gold : 2` and be read back +// as a declaration. Seventeen of them throw on reload, which surfaces as a +// failed File > Open; `economy` and `assert` are worse, matching the version +// header and the assert handler so the connection silently disappears. Quoting +// the reference keeps such a label round-tripping, since _econReadRef already +// understands a quoted name. Case-sensitive on purpose: default labels are +// capitalized (MNode sets `Pool`, `Queue`), so only a hand-typed lowercase +// label is at risk. `name`, `desc`, `seed` and `timeMode` are absent because +// their handler requires a literal colon, which a connection line never has. +const ECON_RESERVED_HEADS = new Set([ + 'economy', 'meta', 'param', 'type', 'var', 'assert', 'player', + 'group', 'note', 'chart', + ...ECON_NODE_KINDS, +]); + function _econRefNames(nodes) { const used = new Map(); // base label → count const refs = new Map(); // node id → ref string @@ -157,7 +174,8 @@ function _econRefNames(nodes) { const base = n.label != null ? String(n.label) : ''; const count = (used.get(base) || 0) + 1; used.set(base, count); - refs.set(n.id, _econName(base) + (count > 1 ? '#' + count : '')); + const name = ECON_RESERVED_HEADS.has(base) ? _econQuote(base) : _econName(base); + refs.set(n.id, name + (count > 1 ? '#' + count : '')); } return refs; } diff --git a/js/renderer.js b/js/renderer.js index 0185f67..68f68ab 100644 --- a/js/renderer.js +++ b/js/renderer.js @@ -570,11 +570,22 @@ class Renderer { this.nodeLayer = svgEl('g'); this.chartLayer = svgEl('g'); this.noteLayer = svgEl('g'); + // Selection handles live here rather than inside the item they belong to. + // A group's element sits in groupLayer and a connection's in connLayer, + // both below nodeLayer, so a node parked near a corner covered the handle + // completely: the press still started a resize (the editor probes handles + // before the hit test, which is deliberate and correct), so the user + // pressed a node, the node did not move, and the group silently resized. + // A plain click was swallowed the same way, leaving the node unselectable + // there. Note and chart handles never had the problem because their layers + // already paint above nodeLayer. Drawing every handle above the content + // makes the hot region and the visible region the same thing again. + this.handleLayer = svgEl('g'); this.ballLayer = svgEl('g'); this.flowLayer = svgEl('g'); this.tempLayer = svgEl('g'); this.root.append(this.groupLayer, this.connLayer, this.nodeLayer, - this.chartLayer, this.noteLayer, this.ballLayer, this.flowLayer, this.tempLayer); + this.chartLayer, this.noteLayer, this.handleLayer, this.ballLayer, this.flowLayer, this.tempLayer); this.svg.appendChild(this.root); this._updateTransform(); @@ -722,6 +733,8 @@ class Renderer { } render() { + // Handles are re-emitted every frame by the item renderers below. + while (this.handleLayer.firstChild) this.handleLayer.removeChild(this.handleLayer.firstChild); this._renderGroups(); this._renderConns(); this._renderNodes(); @@ -1289,7 +1302,6 @@ class Renderer { // State-role badge (✷ trigger / ⊢ activator / Δ modifier), sitting on the // line just before the arrowhead. g.appendChild(svgEl('g', { class: 'conn-role', 'pointer-events': 'none' })); - g.appendChild(svgEl('g', { class: 'conn-handles' })); return g; } @@ -1499,9 +1511,10 @@ class Renderer { el.setAttribute('class', `conn${isSel ? ' selected' : ''}${isFlowing ? ' flowing' : ''}`); // Reshape handles — shown only while selected (and never on a self-loop). - const hg = el.querySelector('.conn-handles'); - while (hg.firstChild) hg.removeChild(hg.firstChild); + // Drawn into handleLayer for the same reason as the resize handles above. if (isSel && src.id !== tgt.id) { + const hg = svgEl('g', { class: 'conn-handles' }); + this.handleLayer.appendChild(hg); for (const h of this.getConnHandles(conn.id)) { hg.appendChild(svgEl('circle', { class: 'conn-cp-handle', r: '6', cx: h.x, cy: h.y, @@ -1557,10 +1570,9 @@ class Renderer { // Draw (or remove) the corner resize handles inside a selected item's group. // Kept as the last children so they paint above the item's own content. _updateResizeHandles(el, item, isSel) { - let hg = el.querySelector('.resize-handles'); - if (!isSel) { if (hg) hg.remove(); return; } - if (!hg) { hg = svgEl('g', { class: 'resize-handles' }); el.appendChild(hg); } - else { while (hg.firstChild) hg.removeChild(hg.firstChild); el.appendChild(hg); } + if (!isSel) return; // handleLayer was cleared at the top of render() + const hg = svgEl('g', { class: 'resize-handles' }); + this.handleLayer.appendChild(hg); const corners = [ { x: item.x, y: item.y, corner: 'nw' }, { x: item.x + item.w, y: item.y, corner: 'ne' }, diff --git a/test/run.js b/test/run.js index bbb7085..7c85780 100644 --- a/test/run.js +++ b/test/run.js @@ -734,6 +734,71 @@ test('a limited source emits its stock then runs dry', () => { eq(s.produced, 10, 'produced equals the emitted stock'); }); +test('.econ round-trip survives a node labelled with a DSL head keyword', () => { + // dslParse dispatches on tokens[0] before scanning for an arrow, so a node + // labelled `pool` that is the SOURCE of a connection emitted `pool -> Gold` + // and was read back as a declaration. Most heads threw on reload; `economy` + // and `assert` matched their handlers and dropped the connection silently. + const HEADS = ['economy', 'meta', 'param', 'type', 'var', 'assert', 'player', + 'group', 'note', 'chart', 'pool', 'source', 'drain', 'gate', 'converter', + 'register', 'delay', 'queue', 'trader']; + for (const kw of HEADS) { + const d = new Diagram(); + const a = new MNode(NodeType.POOL, 100, 100); a.label = kw; + const b = new MNode(NodeType.POOL, 400, 100); b.label = 'Gold'; + d.addNode(a); d.addNode(b); + d.addConnection(new MConnection(a.id, b.id, ConnectionType.RESOURCE)); + const back = new Diagram(); + back.loadJSON(dslParse(dslSerialize(d.toJSON()))); + eq(back.nodes.size, 2, `label "${kw}": both nodes survive`); + eq(back.connections.size, 1, `label "${kw}": the connection survives`); + } + + // An ordinary label must still serialize bare, not newly quoted. + const d = new Diagram(); + const a = new MNode(NodeType.POOL, 0, 0); a.label = 'Gold'; + const b = new MNode(NodeType.POOL, 100, 0); b.label = 'Silver'; + d.addNode(a); d.addNode(b); + d.addConnection(new MConnection(a.id, b.id, ConnectionType.RESOURCE)); + assert(/(^|\n)Gold -> Silver/.test(dslSerialize(d.toJSON())), 'ordinary labels stay unquoted'); +}); + +test('attribution reports trader swaps between the partners, not through the trader', () => { + // _fireTrader books what A pays under the INCOMING leg and what B pays under + // the OUTGOING one, so reading direction off sourceId/targetId called B's + // payment income, never credited either side with what it received, and gave + // the trader two rows for resources it never held. Asymmetric rates so the + // two legs cannot cancel and hide the error. + const d = new Diagram(); + const A = new MNode(NodeType.POOL, 0, 0); A.label = 'A'; A.setCount(50); + const B = new MNode(NodeType.POOL, 400, 0); B.label = 'B'; B.setCount(50); + const T = new MNode(NodeType.TRADER, 200, 0); T.label = 'Market'; + d.addNode(A); d.addNode(B); d.addNode(T); + const cin = new MConnection(A.id, T.id, ConnectionType.RESOURCE); cin.rate = 3; + const cout = new MConnection(T.id, B.id, ConnectionType.RESOURCE); cout.rate = 1; + d.addConnection(cin); d.addConnection(cout); + + const e = new SimEngine(d); e.reset(); + for (let i = 0; i < 3; i++) e.doStep(); + const last = e.history.length - 1; + + const ra = attributeChange(d, e.history, A.id, last); + eq(ra.residual, 0, 'A: every unit accounted for'); + const aOut = ra.entries.find(x => x.kind === 'flow out'); + const aIn = ra.entries.find(x => x.kind === 'flow in'); + eq(aOut.amount, -3, 'A paid 3 out'); + eq(aIn.amount, 1, 'A received 1 back'); + assert(/B/.test(aOut.label) && /B/.test(aIn.label), 'A trades with B, not with the trader'); + + const rb = attributeChange(d, e.history, B.id, last); + eq(rb.residual, 0, 'B: every unit accounted for'); + eq(rb.entries.find(x => x.kind === 'flow out').amount, -1, 'B paid 1 out, not received'); + eq(rb.entries.find(x => x.kind === 'flow in').amount, 3, 'B received 3'); + + const rt = attributeChange(d, e.history, T.id, last); + eq(rt.entries.length, 0, 'the trader holds nothing, so it gets no flow rows'); +}); + test('.econ round-trip keeps a limited source colored stock', () => { // dslSerialize writes the stock as `= N of color`, but the parser skipped the // colorMap for sources, so reconcile() refilled it as untyped grey. Any diff --git a/test/smoke.js b/test/smoke.js index 14d9196..9b9e582 100644 --- a/test/smoke.js +++ b/test/smoke.js @@ -285,6 +285,216 @@ const URL = process.env.SMOKE_URL || 'http://localhost:8080/'; ok('gesture released outside the canvas ends and commits, nothing follows a buttonless cursor'); else fail('outside release: ' + JSON.stringify(outsideRelease)); + // Run/Pause and Reset refresh the scrubber; Step did not, so its range sat one + // entry behind. While Live the thumb is already at max, so End or a rightward + // drag emitted no input event and the newest step was unreachable. + const stepScrub = await page.evaluate(() => { + window.app._clearAll(); + window.app._closeFeature(); + const d = window.app.diagram; + const s = d.addNode(new MNode(NodeType.SOURCE, 200, 300)); + const p = d.addNode(new MNode(NodeType.POOL, 500, 300)); + d.addConnection(new MConnection(s.id, p.id, 'resource')); + window.app.renderer.render(); + window.app._commit(); + // Leave the drawer as it was found: a later test toggles it unconditionally. + const wasOpen = window.app._timelineVisible; + if (!wasOpen) document.getElementById('btn-timeline').click(); + for (let i = 0; i < 4; i++) document.getElementById('btn-step').click(); + const range = document.getElementById('tl-range'); + const out = { max: +range.max, disabled: range.disabled, want: window.app.engine.history.length - 1 }; + if (!wasOpen) document.getElementById('btn-timeline').click(); + return out; + }); + if (stepScrub.max === stepScrub.want && !stepScrub.disabled) + ok('Step keeps the scrub slider in range, so the newest step is reachable'); + else fail('step scrubber: ' + JSON.stringify(stepScrub)); + + // assertions are a serialized Diagram field and were the one _clearAll missed, + // so design tests followed the user into File > New and every template load, + // then into autosave, Save as JSON, the .econ export and the share link. + const assertLeak = await page.evaluate(async () => { + window.app._confirmGuard = () => Promise.resolve(true); + const out = {}; + const seed = () => { window.app.diagram.assertions = ['always Gold < 500']; }; + seed(); + document.getElementById('btn-new').click(); + await new Promise(r => setTimeout(r, 60)); + out.afterNew = window.app.diagram.assertions.length; + seed(); window.app._installTemplate(window.app._templates[0]); + out.afterTemplate = window.app.diagram.assertions.length; + seed(); window.app._loadDemo(); + out.afterDemo = window.app.diagram.assertions.length; + return out; + }); + if (assertLeak.afterNew === 0 && assertLeak.afterTemplate === 0 && assertLeak.afterDemo === 0) + ok('design tests do not leak through New or a template load'); + else fail('assertion leak: ' + JSON.stringify(assertLeak)); + + // An ASCII-only sanitizer reduced a non-Latin name to nothing, returning a + // bare ".svg" that the browser renames to "svg.svg", so every export of a + // CJK or Cyrillic diagram collided on one filename and .econ lost its type. + const fnames = await page.evaluate(() => { + const out = {}; + for (const [k, nm] of [['cjk', '经济'], ['cyrillic', 'Экономика'], + ['accented', 'Économie'], ['ascii', 'Gold Rush'], + ['symbols', '***'], ['empty', '']]) { + window.app.diagram.meta.name = nm; + out[k] = window.app._exportFilename('svg'); + } + return out; + }); + const namesOk = Object.values(fnames).every(n => n && !n.startsWith('.') && n !== 'svg.svg') + && fnames.cjk.startsWith('经济') && fnames.accented.startsWith('É') + && fnames.symbols === 'diagram.svg' && fnames.empty === 'diagram.svg'; + if (namesOk) ok('export filenames keep non-ASCII names and never collapse to a dotfile'); + else fail('export filenames: ' + JSON.stringify(fnames)); + + // Sparkline mapped v/max, so anything below zero landed at y > h, off the + // bottom of the canvas. A register holding a net or deficit drew nothing at + // all under a false "max: 1" caption. Measured by ink per row rather than by + // colour, which is too sensitive to anti-aliasing to compare reliably. + const sparkNeg = await page.evaluate(() => { + const inkRows = (vals) => { + const host = document.createElement('div'); + document.body.appendChild(host); + const eng = { history: vals.map((v, i) => ({ step: i, snap: { n1: v } })) }; + const sp = new Sparkline(host, 'n1', eng); + sp.update(); + const ctx = sp.canvas.getContext('2d'); + const w = sp.canvas.width, h = sp.canvas.height; + const img = ctx.getImageData(0, 0, w, h).data; + let rows = 0; + // Skip the label band at the top; count rows carrying trace ink. + for (let y = 14; y < h; y++) { + for (let x = 0; x < w; x++) { + const i = (y * w + x) * 4; + if (img[i] > 40 || img[i + 1] > 60) { rows++; break; } + } + } + host.remove(); + return rows; + }; + return { + allNegative: inkRows([-20, -18, -15, -12, -10, -8]), + crossZero: inkRows([-6, -4, -2, 0, 2, 4, 6]), + allPositive: inkRows([0, 2, 4, 6, 8, 10]), + }; + }); + if (sparkNeg.allNegative > 5 && sparkNeg.crossZero > 5 && sparkNeg.allPositive > 5) + ok('sparkline draws negative and zero-crossing series instead of clipping them off the canvas'); + else fail('sparkline negatives: ' + JSON.stringify(sparkNeg)); + + // The canvas and timeline both had a scrub-aware read path; the properties + // panel never did, so its hero card reported the end of the run while the + // node beside it showed the replayed step. + const scrubPanel = await page.evaluate(() => { + window.app._clearAll(); + window.app._closeFeature(); + const d = window.app.diagram; + const s = d.addNode(new MNode(NodeType.SOURCE, 200, 300)); + const pool = d.addNode(new MNode(NodeType.POOL, 500, 300)); + pool.label = 'Gold'; + d.addConnection(new MConnection(s.id, pool.id, 'resource')); + window.app.renderer.render(); + window.app._commit(); + for (let i = 0; i < 12; i++) window.app.engine.doStep(); + window.app._onSelect(pool.id, 'node'); + const hero = () => document.getElementById('props-hero-value').textContent; + const wasOpen = window.app._timelineVisible; + if (!wasOpen) document.getElementById('btn-timeline').click(); + window.app._scrubTo(4); + const at4 = { hero: hero(), canvas: window.app.renderer._scrubSnap[pool.id] }; + window.app._exitScrub(); + const live = { hero: hero(), model: pool.resources }; + if (!wasOpen) document.getElementById('btn-timeline').click(); + return { at4, live }; + }); + if (String(scrubPanel.at4.hero) === String(scrubPanel.at4.canvas) + && String(scrubPanel.live.hero) === String(scrubPanel.live.model)) + ok('properties panel follows the scrub and returns to the live value'); + else fail('scrub panel: ' + JSON.stringify(scrubPanel)); + + // A selected item's handles beat whatever they sit on in the hit test, which + // is deliberate. The defect was the paint order: group handles lived in + // groupLayer and connection handles in connLayer, both under nodeLayer, so a + // node on a corner hid the handle while the press still resized. What is + // visible and what is hot have to be the same element. + const handleTop = await page.evaluate(() => { + window.app._clearAll(); + window.app._closeFeature(); + const d = window.app.diagram; + const g = d.addGroup(new MGroup(200, 200, 300, 220)); + g.label = 'Zone'; + d.addNode(new MNode(NodeType.POOL, 200, 200)); // sits exactly on the NW corner + window.app.editor.setTool('select'); + window.app.renderer.render(); + window.app._commit(); + window.app.editor._select(g.id, 'group'); + window.app.renderer.render(); + const canvas = document.getElementById('canvas'); + const r = canvas.getBoundingClientRect(), rn = window.app.renderer; + const top = document.elementFromPoint( + r.left + g.x * rn._scale + rn._panX, + r.top + g.y * rn._scale + rn._panY); + const rh = document.querySelector('.resize-handles'); + return { + topClass: top ? (top.getAttribute('class') || top.tagName) : null, + inOverlay: !!rh && rh.parentNode === rn.handleLayer, + }; + }); + if (handleTop.inOverlay && /resize-handle/.test(handleTop.topClass || '')) + ok('selection handles paint above nodes, so the hot region is the visible one'); + else fail('handle layer: ' + JSON.stringify(handleTop)); + + // Renaming a parameter onto an existing name overwrote the other and deleted + // this one, silently collapsing two rows into one. The shared variable store + // spans params, custom vars, register labels and named state connections, so + // the check has to as well. + const renameGuard = await page.evaluate(() => { + const openParams = () => { + window.app._clearAll(); + window.app._closeFeature(); + window.app.diagram.params = { gold: 5, rate: 2 }; + window.app.renderer.render(); + window.app._commit(); + document.querySelector('#diagram-rail .rail-btn[data-feature="params"]').click(); + }; + const box = (v) => [...document.querySelectorAll('#props-content input[type="text"]')] + .find(i => i.value === v); + openParams(); + const b1 = box('rate'); + b1.value = 'gold'; + b1.dispatchEvent(new Event('blur')); + const collision = { ...window.app.diagram.params }; + + openParams(); + const b2 = box('rate'); + b2.value = 'tempo'; + b2.dispatchEvent(new Event('blur')); + const renamed = { ...window.app.diagram.params }; + + // Collide with a register label rather than another parameter. + window.app._clearAll(); + window.app._closeFeature(); + const reg = window.app.diagram.addNode(new MNode(NodeType.REGISTER, 300, 300)); + reg.label = 'score'; + window.app.diagram.params = { rate: 2 }; + window.app.renderer.render(); + window.app._commit(); + document.querySelector('#diagram-rail .rail-btn[data-feature="params"]').click(); + const b3 = box('rate'); + b3.value = 'score'; + b3.dispatchEvent(new Event('blur')); + const acrossNamespace = { ...window.app.diagram.params }; + return { collision, renamed, acrossNamespace }; + }); + if (renameGuard.collision.gold === 5 && renameGuard.collision.rate === 2 + && renameGuard.renamed.tempo === 2 && !('rate' in renameGuard.renamed) + && renameGuard.acrossNamespace.rate === 2) + ok('a colliding rename is rejected across the shared namespace, a real one still works'); + else fail('rename guard: ' + JSON.stringify(renameGuard)); + // Navigation: zoom controls step the scale and update the readout; fit-to-content // re-frames without error. const nav = await page.evaluate(() => {