From fe4e0d9f1aa1c5192dd195c5407d68114465d594 Mon Sep 17 00:00:00 2001 From: MelekTaus Date: Sat, 10 Oct 2026 17:37:13 +0200 Subject: [PATCH] Snap to the grid from the start in every graph tab (#391) Laying out a graph by hand mostly means lining nodes up, so the magnet was switched on in nearly every session. Every graph tab now starts with it lit: Behavior, State machine, Shader, Particles, Sound FX, Instrument, the song's Routing and the Routing tab. - SnapToggle takes the state it starts in and lights its button to match. GRAPH_SNAPS_AT_FIRST sets it for the toggles and for GraphCanvas alike, so button and canvas cannot disagree. - Curve and Vector Art are no graphs and still start without it. - New hook __gm.graphSnapping(on?) reads every graph tab's magnet, or "mixed" where button and canvas differ, and switches them all. - The e2e harness reads the magnets after each navigate() and then switches them off, as it already sets the zoom transition to 0. Most checks drag a node to an exact spot. e2e 45 does the same itself, because the editor comes up there through the sign-in. - e2e 06 checks that all eight graph tabs started with the magnet on. The checks in 06, 26 and 65 that read it off now say they read what the harness left. Tests: - Unit snap-toggle: 4 tests, 1 new (lit at first, nothing asked of the target before a click). - e2e 06 (melektaus-atx): 150 checks, 1 new. e2e 34: 134 checks. - Gate (melektaus-atx, together with #390): 287 files, 4397 tests. - Pictures looked at: none, the state comes from the hook. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- src/editor/debug/uiHooks.ts | 26 +++++++++++++++++++ src/editor/graph/GraphCanvas.ts | 3 ++- src/editor/graph/GraphEditor.ts | 4 +-- src/editor/graph/SnapToggle.ts | 19 +++++++++++--- src/editor/instrument/InstrumentEditor.ts | 4 +-- src/editor/particles/ParticlesEditor.ts | 4 +-- src/editor/routing/RoutingEditor.ts | 4 +-- src/editor/shader/ShaderEditor.ts | 4 +-- src/editor/song/SongEditor.ts | 4 +-- src/editor/soundfx/SoundFxEditor.ts | 4 +-- src/editor/statemachine/StateMachineEditor.ts | 4 +-- tests/editor/snap-toggle.test.ts | 16 +++++++++++- tools/e2e/06-graph.mjs | 12 ++++++++- tools/e2e/26-state-machine.mjs | 5 +++- tools/e2e/45-login.mjs | 3 +++ tools/e2e/65-song.mjs | 3 ++- tools/lib/harness.mjs | 8 ++++++ 18 files changed, 104 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 91ebf9cf..0df9a404 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ npm run build # writes dist/ and regenerates index.html; the folder is wh The browser suite and the three example games need a machine with a GPU: ```bash -node tools/seed-e2e.mjs && node tools/e2e.mjs # the editor, 1965 checks (GM_STEPS=20 for one step) +node tools/seed-e2e.mjs && node tools/e2e.mjs # the editor, 1966 checks (GM_STEPS=20 for one step) # every tool signs in first โ€“ see the Accounts page node tools/seed-jumprun.mjs && node tools/smoke-jumprun.mjs node tools/seed-tetris.mjs && node tools/smoke-tetris.mjs diff --git a/src/editor/debug/uiHooks.ts b/src/editor/debug/uiHooks.ts index 04e987f9..7e74a1e9 100644 --- a/src/editor/debug/uiHooks.ts +++ b/src/editor/debug/uiHooks.ts @@ -373,6 +373,32 @@ export function uiHooks({ app, state, editor }: HookContext) { if (ms !== undefined) keepGeneralSettings({ zoomTransitionMs: ms }); return generalSettings().zoomTransitionMs; }, + /** + * Whether each graph tab's magnet is on (#391); with `on` they are all switched first. The + * harness reads them on every load and then switches them off, so a drag lands where it is let go. + */ + graphSnapping: (on?: boolean) => { + const tabs = { + logic: [editor.graphEditor.snap, editor.graphEditor.canvas], + stateMachine: [editor.stateMachineEditor.snap, editor.stateMachineEditor.canvas], + shader: [editor.shaderEditor.snap, editor.shaderEditor.canvas], + particles: [editor.particlesEditor.snap, editor.particlesEditor.canvas], + soundFx: [editor.soundFxEditor.snap, editor.soundFxEditor.canvas], + instrument: [editor.instrumentEditor.snap, editor.instrumentEditor.canvas], + song: [editor.songEditor.snap, editor.songEditor.routing], + routing: [editor.routingEditor.snap, editor.routingEditor.canvas], + } as const; + if (on !== undefined) for (const [snap] of Object.values(tabs)) snap.set(on); + // The magnet lit and the canvas snapping, or "mixed" where the two disagree. + return Object.fromEntries( + Object.entries(tabs).map(([tab, [snap, canvas]]) => [ + tab, + snap.button.toggled === canvas.snapEnabled && snap.snapping === canvas.snapEnabled + ? canvas.snapEnabled + : 'mixed', + ]), + ); + }, /** The "Open project" dialog (#183): the projects it lists and the row picked. */ projectPicker: () => editor.projectPickerDialog()?.info() ?? null, /** The "Snapshots" dialog (#231): its rows, newest first, the row picked and its name field. */ diff --git a/src/editor/graph/GraphCanvas.ts b/src/editor/graph/GraphCanvas.ts index dc23d8c4..c511cd85 100644 --- a/src/editor/graph/GraphCanvas.ts +++ b/src/editor/graph/GraphCanvas.ts @@ -82,6 +82,7 @@ import { type PaletteEntry, paletteEntries } from './paletteEntries'; import { NODE_WIDTH, NodeView, TITLE_HEIGHT } from './NodeView'; import type { NodeFace } from './nodeFace'; import { drawGraphGrid, FIT_MARGIN, inView, ZOOM_LIMITS, zoomedAbout } from './graphView'; +import { GRAPH_SNAPS_AT_FIRST } from './SnapToggle'; import { GraphWiring } from './wiring'; /** One running instance's answer for a machine: which state it holds, and whose instance it is. */ @@ -136,7 +137,7 @@ export class GraphCanvas extends Widget { draw: () => this.applyView(), }); private panning: { startX: number; startY: number; panX: number; panY: number } | null = null; - private snapping = false; + private snapping = GRAPH_SNAPS_AT_FIRST; private readonly wiring: GraphWiring; private readonly drags: GraphDrags; /** A rubber-band selection in progress, in graph coordinates. */ diff --git a/src/editor/graph/GraphEditor.ts b/src/editor/graph/GraphEditor.ts index 8943fb70..24d622cc 100644 --- a/src/editor/graph/GraphEditor.ts +++ b/src/editor/graph/GraphEditor.ts @@ -4,14 +4,14 @@ import { AssetTabEditor } from '../AssetTabEditor'; import type { EditorState } from '../state'; import { Breadcrumb } from './Breadcrumb'; import { GraphCanvas } from './GraphCanvas'; -import { SnapToggle } from './SnapToggle'; +import { GRAPH_SNAPS_AT_FIRST, SnapToggle } from './SnapToggle'; import { backTo, descend, follow, type Trail } from './trail'; /** Room kept for the "N nodes ยท M links" readout, wide enough for a three-digit graph. */ const INFO_WIDTH = 150; export class GraphEditor extends AssetTabEditor { - readonly snap = new SnapToggle(() => this.canvas); + readonly snap = new SnapToggle(() => this.canvas, GRAPH_SNAPS_AT_FIRST); readonly breadcrumb = new Breadcrumb(); private trail: Trail = []; /** A call followed into another behavior: the tab goes on to that document (#171). */ diff --git a/src/editor/graph/SnapToggle.ts b/src/editor/graph/SnapToggle.ts index 7eacbd0b..73667ebf 100644 --- a/src/editor/graph/SnapToggle.ts +++ b/src/editor/graph/SnapToggle.ts @@ -6,12 +6,20 @@ export interface Snappable { setSnapping(on: boolean): void; } -/** The magnet every tab that snaps carries in its header (#194): an icon toggle, off at first. */ +/** A graph, any of them, snaps from the start (#391); a curve and a vector art wait for the magnet. */ +export const GRAPH_SNAPS_AT_FIRST = true; + +/** + * The magnet every tab that snaps carries in its header (#194): an icon toggle, lit at first where + * its target starts snapping, as every graph does (#391). + */ export class SnapToggle { readonly button: Button; - private on = false; - constructor(private readonly target: () => Snappable) { + constructor( + private readonly target: () => Snappable, + private on = false, + ) { this.button = new Button('', { width: Theme.controlHeight, icon: HEADER_ICON_SVG.snap, @@ -19,6 +27,11 @@ export class SnapToggle { tooltip: 'Snap to the grid', onClick: () => this.set(!this.on), }); + this.button.setToggled(on); + } + + get snapping(): boolean { + return this.on; } get widgets(): readonly Button[] { diff --git a/src/editor/instrument/InstrumentEditor.ts b/src/editor/instrument/InstrumentEditor.ts index c1beeb7d..fa8ab6dd 100644 --- a/src/editor/instrument/InstrumentEditor.ts +++ b/src/editor/instrument/InstrumentEditor.ts @@ -28,7 +28,7 @@ import { Button, Dropdown, Label, Menu, VStack, type Widget } from '../../ui'; import { soundLibrary } from '../../viewport/SoundLibrary'; import { AssetTabEditor } from '../AssetTabEditor'; import { GraphCanvas } from '../graph/GraphCanvas'; -import { SnapToggle } from '../graph/SnapToggle'; +import { GRAPH_SNAPS_AT_FIRST, SnapToggle } from '../graph/SnapToggle'; import { TOOLBAR_ICON_SVG } from '../panels/toolbarIcons'; import { PreviewParams } from '../soundfx/PreviewParams'; import type { EditorState } from '../state'; @@ -51,7 +51,7 @@ const AUDITION_DELAY_MS = 150; * them at once, a preset is heard on the keys. */ export class InstrumentEditor extends AssetTabEditor { - readonly snap = new SnapToggle(() => this.canvas); + readonly snap = new SnapToggle(() => this.canvas, GRAPH_SNAPS_AT_FIRST); /** Public for the check scripts. */ readonly keyboard = new PianoKeyboard(); readonly stopButton: Button; diff --git a/src/editor/particles/ParticlesEditor.ts b/src/editor/particles/ParticlesEditor.ts index 5305a9d7..d41c0010 100644 --- a/src/editor/particles/ParticlesEditor.ts +++ b/src/editor/particles/ParticlesEditor.ts @@ -6,7 +6,7 @@ import { Button, HStack, Label, Slider, Spacer, type Splitter, Theme, VStack, ty import { AssetTabEditor } from '../AssetTabEditor'; import { GraphCanvas } from '../graph/GraphCanvas'; import { PreviewSplit } from '../PreviewSplit'; -import { SnapToggle } from '../graph/SnapToggle'; +import { GRAPH_SNAPS_AT_FIRST, SnapToggle } from '../graph/SnapToggle'; import type { EditorState } from '../state'; import { ParticlePreview } from './ParticlePreview'; @@ -25,7 +25,7 @@ const HOLD_WIDTH = 52; */ export class ParticlesEditor extends AssetTabEditor { readonly preview: ParticlePreview; - readonly snap = new SnapToggle(() => this.canvas); + readonly snap = new SnapToggle(() => this.canvas, GRAPH_SNAPS_AT_FIRST); /** Public for the check scripts: the one control they have to hit rather than read. */ readonly slider = new Slider(0, 0, MIN_SPAN, 120); private readonly holdButton = new Button('Hold', { diff --git a/src/editor/routing/RoutingEditor.ts b/src/editor/routing/RoutingEditor.ts index 110ee65f..8feaba77 100644 --- a/src/editor/routing/RoutingEditor.ts +++ b/src/editor/routing/RoutingEditor.ts @@ -2,7 +2,7 @@ import { masterRoutingProblems } from '../../core/music/projectRouting'; import { ROUTING_ID } from '../../core/model/Project'; import { Button, Label, Spacer, VStack } from '../../ui'; import { GraphCanvas } from '../graph/GraphCanvas'; -import { SnapToggle } from '../graph/SnapToggle'; +import { GRAPH_SNAPS_AT_FIRST, SnapToggle } from '../graph/SnapToggle'; import type { EditorState } from '../state'; import { TabHeader } from '../TabHeader'; @@ -22,7 +22,7 @@ export class RoutingEditor extends VStack { constructor(private readonly state: EditorState) { super(); this.canvas = new GraphCanvas(state, 'routing'); - this.snap = new SnapToggle(() => this.canvas); + this.snap = new SnapToggle(() => this.canvas, GRAPH_SNAPS_AT_FIRST); this.fitButton = new Button('Fit', { onClick: () => this.canvas.fit() }); this.info.minWidth = INFO_WIDTH; const header = new TabHeader(); diff --git a/src/editor/shader/ShaderEditor.ts b/src/editor/shader/ShaderEditor.ts index 64ae75d7..00584264 100644 --- a/src/editor/shader/ShaderEditor.ts +++ b/src/editor/shader/ShaderEditor.ts @@ -19,7 +19,7 @@ import { import { shaderLibrary } from '../../viewport/ShaderLibrary'; import { AssetTabEditor } from '../AssetTabEditor'; import { GraphCanvas } from '../graph/GraphCanvas'; -import { SnapToggle } from '../graph/SnapToggle'; +import { GRAPH_SNAPS_AT_FIRST, SnapToggle } from '../graph/SnapToggle'; import { PreviewSplit } from '../PreviewSplit'; import type { EditorState } from '../state'; import { @@ -50,7 +50,7 @@ const FUNCTION_SUBJECT = 'A function shows in the shaders calling it'; * frame โ€“ so the shader is judged on the image it is meant for; before that, a built-in test image. */ export class ShaderEditor extends AssetTabEditor { - readonly snap = new SnapToggle(() => this.canvas); + readonly snap = new SnapToggle(() => this.canvas, GRAPH_SNAPS_AT_FIRST); readonly preview: ShaderPreview; /** Assigned by `body`; public for the check scripts, which drag it. */ splitter!: Splitter; diff --git a/src/editor/song/SongEditor.ts b/src/editor/song/SongEditor.ts index 190de2d0..40f39a9e 100644 --- a/src/editor/song/SongEditor.ts +++ b/src/editor/song/SongEditor.ts @@ -40,7 +40,7 @@ import { import { activateAsset } from '../actions'; import { AssetTabEditor } from '../AssetTabEditor'; import { GraphCanvas } from '../graph/GraphCanvas'; -import { SnapToggle } from '../graph/SnapToggle'; +import { GRAPH_SNAPS_AT_FIRST, SnapToggle } from '../graph/SnapToggle'; import { BakeButton } from '../music/BakeButton'; import { clockText } from '../music/timeView'; import { claimTransport, releaseTransport } from '../music/transport'; @@ -93,7 +93,7 @@ export class SongEditor extends AssetTabEditor { readonly routingButton: Button; readonly bakeButton: BakeButton; readonly routing: GraphCanvas; - readonly snap = new SnapToggle(() => this.routing); + readonly snap = new SnapToggle(() => this.routing, GRAPH_SNAPS_AT_FIRST); private shown: SongView = 'timeline'; private readonly pane = new VStack(); /** Heard through the sound FX (#352), then straight into the master: a preview leaves the routing out (#368). */ diff --git a/src/editor/soundfx/SoundFxEditor.ts b/src/editor/soundfx/SoundFxEditor.ts index ade26462..674c8816 100644 --- a/src/editor/soundfx/SoundFxEditor.ts +++ b/src/editor/soundfx/SoundFxEditor.ts @@ -10,7 +10,7 @@ import { soundLibrary } from '../../viewport/SoundLibrary'; import { MASTER_MIX } from '../../core/music/mixer'; import { AssetTabEditor } from '../AssetTabEditor'; import { GraphCanvas } from '../graph/GraphCanvas'; -import { SnapToggle } from '../graph/SnapToggle'; +import { GRAPH_SNAPS_AT_FIRST, SnapToggle } from '../graph/SnapToggle'; import { TOOLBAR_ICON_SVG } from '../panels/toolbarIcons'; import type { EditorState } from '../state'; import { PreviewParams } from './PreviewParams'; @@ -27,7 +27,7 @@ const PREVIEW_OWNER = 'soundfx-preview'; * only there to listen with. */ export class SoundFxEditor extends AssetTabEditor { - readonly snap = new SnapToggle(() => this.canvas); + readonly snap = new SnapToggle(() => this.canvas, GRAPH_SNAPS_AT_FIRST); /** Public for the check scripts. */ readonly soundPicker: Dropdown; readonly playButton: Button; diff --git a/src/editor/statemachine/StateMachineEditor.ts b/src/editor/statemachine/StateMachineEditor.ts index c292d227..612e30a2 100644 --- a/src/editor/statemachine/StateMachineEditor.ts +++ b/src/editor/statemachine/StateMachineEditor.ts @@ -2,7 +2,7 @@ import { statesOf } from '../../core/graph'; import type { Asset, StateMachineAsset } from '../../core/model/Asset'; import { AssetTabEditor } from '../AssetTabEditor'; import { GraphCanvas } from '../graph/GraphCanvas'; -import { SnapToggle } from '../graph/SnapToggle'; +import { GRAPH_SNAPS_AT_FIRST, SnapToggle } from '../graph/SnapToggle'; import type { EditorState } from '../state'; /** Room kept for the "N states" readout. */ @@ -14,7 +14,7 @@ const INFO_WIDTH = 90; * and undo are all the graph's. */ export class StateMachineEditor extends AssetTabEditor { - readonly snap = new SnapToggle(() => this.canvas); + readonly snap = new SnapToggle(() => this.canvas, GRAPH_SNAPS_AT_FIRST); constructor(state: EditorState) { super(state, { infoWidth: INFO_WIDTH }); diff --git a/tests/editor/snap-toggle.test.ts b/tests/editor/snap-toggle.test.ts index 8b54b227..99328016 100644 --- a/tests/editor/snap-toggle.test.ts +++ b/tests/editor/snap-toggle.test.ts @@ -4,7 +4,7 @@ */ import { beforeAll, describe, expect, it, vi } from 'vitest'; import { CanvasTextMetrics } from 'pixi.js'; -import { SnapToggle } from '../../src/editor/graph/SnapToggle'; +import { GRAPH_SNAPS_AT_FIRST, SnapToggle } from '../../src/editor/graph/SnapToggle'; import { Theme } from '../../src/ui'; // Node has no DOMParser to read the file with; what the magnet looks like is the icon tests' job. @@ -40,6 +40,20 @@ describe('SnapToggle', () => { expect(canvas.calls).toEqual([true, false]); }); + it('starts lit where its target snaps from the start, as every graph does (#391)', () => { + const canvas = target(); + const snap = new SnapToggle(() => canvas, GRAPH_SNAPS_AT_FIRST); + expect(GRAPH_SNAPS_AT_FIRST).toBe(true); + expect(snap.button.toggled).toBe(true); + expect(snap.snapping).toBe(true); + // Nothing is asked of the target before a click: it snaps by itself from the start. + expect(canvas.calls).toEqual([]); + + snap.button.onClick?.(); + expect(snap.button.toggled).toBe(false); + expect(canvas.calls).toEqual([false]); + }); + it('asks for the target at the click, so a canvas mounted later is the one switched', () => { let canvas = target(); const snap = new SnapToggle(() => canvas); diff --git a/tools/e2e/06-graph.mjs b/tools/e2e/06-graph.mjs index 34bebe79..80970819 100644 --- a/tools/e2e/06-graph.mjs +++ b/tools/e2e/06-graph.mjs @@ -27,6 +27,7 @@ export async function graph(t) { asset, api, navigate, + graphSnapAtFirst, projectId, showTab, } = t; @@ -447,7 +448,16 @@ export async function graph(t) { // current zoom โ€“ the canvas is not wide enough here for a zoom of 1. const GRID = 24; const onGrid = (n) => n.x % GRID === 0 && n.y % GRID === 0; - check('snapping is off until it is asked for', (await json('__gm.graph.snapping()')) === false); + // Every graph tab comes up snapping, magnet lit (#391); the harness has switched them all off since. + const atFirst = graphSnapAtFirst(); + check( + 'every graph tab starts with snapping on and its magnet lit', + atFirst !== null && + Object.keys(atFirst).length === 8 && + Object.values(atFirst).every((on) => on === true), + JSON.stringify(atFirst), + ); + check('snapping is off as the harness left it', (await json('__gm.graph.snapping()')) === false); // The switch is a magnet (#194): a square icon button that stays lit while snapping is on. const magnet = await json('__gm.graph.snapButton()'); await click(magnet.x, magnet.y); diff --git a/tools/e2e/26-state-machine.mjs b/tools/e2e/26-state-machine.mjs index b3cc2cd7..e40b33e8 100644 --- a/tools/e2e/26-state-machine.mjs +++ b/tools/e2e/26-state-machine.mjs @@ -110,7 +110,10 @@ export async function stateMachine(t) { // 26b2. The header's magnet is the graph's: on, a dragged state lands on the 24 px grid. const GRID = 24; const stateXY = async (id) => (await states()).find((s) => s.id === id) ?? { x: NaN, y: NaN }; - check('snapping is off until it is asked for', (await json('__gm.stateMachine.snapping()')) === false); + check( + 'snapping is off as the harness left it (#391)', + (await json('__gm.stateMachine.snapping()')) === false, + ); const magnet = await json('__gm.stateMachine.snapButton()'); const preSnap = JSON.stringify(await states()); await click(magnet.x, magnet.y); diff --git a/tools/e2e/45-login.mjs b/tools/e2e/45-login.mjs index 0c8c9880..041508b9 100644 --- a/tools/e2e/45-login.mjs +++ b/tools/e2e/45-login.mjs @@ -285,4 +285,7 @@ export async function login(t) { restored.dialog === null && restored.notice.startsWith('Password changed'), restored, ); + // The editor came up through the sign-in screen, not `navigate()`: its graphs snap (#391), and + // the steps after this one drag nodes to exact spots. + await evaluate('__gm.graphSnapping(false)'); } diff --git a/tools/e2e/65-song.mjs b/tools/e2e/65-song.mjs index ecf74687..e2ca512e 100644 --- a/tools/e2e/65-song.mjs +++ b/tools/e2e/65-song.mjs @@ -954,7 +954,8 @@ export async function song(t) { p = await json("__gm.song.viewAt('routing')"); await click(p.x, p.y); await sleep(300); - // The magnet is the routing's, as in every graph tab: off at first, greyed out on the timeline. + // The magnet is the routing's, as in every graph tab: off as the harness left it (#391), greyed + // out on the timeline. const magnet = await json('__gm.song.routing.snapButton()'); const snapOff = await json('__gm.song.routing.snapping()'); await click(magnet.x, magnet.y); diff --git a/tools/lib/harness.mjs b/tools/lib/harness.mjs index ac1aa7ac..7f8e92bc 100644 --- a/tools/lib/harness.mjs +++ b/tools/lib/harness.mjs @@ -491,7 +491,14 @@ function makeTools({ cdp, projectId, baseUrl, consoleLog, check, shotPrefix }) { check('debug hook present', Boolean(await evaluate('Boolean(window.__gm?.state)'))); // The zoom is shown as a transition (#309); a check reads the step it lands on, not the way. await evaluate('__gm.zoomTransition?.(0)'); + // Every graph snaps from the start (#391); the checks drag nodes to exact spots, so the magnets + // go off here. What they started at stays readable for the check that wants it. + snapAtFirst = (await json('__gm.graphSnapping?.() ?? null')) ?? null; + await evaluate('__gm.graphSnapping?.(false)'); }; + /** Each graph tab's magnet as the last load left it, before the harness switched it off (#391). */ + let snapAtFirst = null; + const graphSnapAtFirst = () => snapAtFirst; return { cdp, @@ -545,6 +552,7 @@ function makeTools({ cdp, projectId, baseUrl, consoleLog, check, shotPrefix }) { signIn, sceneOf, navigate, + graphSnapAtFirst, }; }