From 150f2e0906d723ff07d5c81e695075ee09f6b209 Mon Sep 17 00:00:00 2001 From: MelekTaus Date: Tue, 15 Sep 2026 18:43:34 +0200 Subject: [PATCH] A row scrolled out of its list is not on the screen, so no test may click it CI found the gap the hard way: with every kind of asset in the project the new scene sits below the panel, and the right click for "Set as start scene" landed somewhere under the list instead of on the row. locate() only asked whether a text was visible, and a masked row still is - it is merely clipped away. locateAll and overlayTexts now walk the tree together (eachText) and carry the clip of every ancestor, measured on the widget that owns the mask: Pixi v8 hands a Graphics set as a mask to the mask effect and reports empty bounds for it, so asking the mask itself declares every masked text in the editor missing. Two hooks with two notions of "visible" was the second half of the same bug. The panel also scrolls the selected row into view now, the way the hierarchy has always done - an asset selected elsewhere has to be findable in the list, test or no test. And step 21 learned that scene is a property kind. --- src/editor/debugHook.ts | 74 +++++++++++++++++++++++++-------- src/editor/panels/AssetPanel.ts | 9 +++- tools/e2e/21-list.mjs | 4 +- tools/e2e/35-load-scene.mjs | 11 ++++- 4 files changed, 76 insertions(+), 22 deletions(-) diff --git a/src/editor/debugHook.ts b/src/editor/debugHook.ts index c8f0e7f8..b3f090a3 100644 --- a/src/editor/debugHook.ts +++ b/src/editor/debugHook.ts @@ -27,25 +27,66 @@ export function installDebugHook(app: Application, state: EditorState, editor: E const p = widget.getGlobalPosition(); return { x: p.x + widget.w / 2, y: p.y + widget.h / 2 }; }; + /** The overlap of two global rectangles; an empty one when they do not meet. */ + const intersect = (a: Rectangle, b: Rectangle): Rectangle => { + const x = Math.max(a.x, b.x); + const y = Math.max(a.y, b.y); + return new Rectangle( + x, + y, + Math.max(0, Math.min(a.right, b.right) - x), + Math.max(0, Math.min(a.bottom, b.bottom) - y), + ); + }; const rectOf = (widget: { getGlobalPosition(): { x: number; y: number }; w: number; h: number }) => { const p = widget.getGlobalPosition(); return { x: p.x, y: p.y, w: widget.w, h: widget.h }; }; - /** Centres of every visible text equal to `text`, in drawing order (the topmost last). */ - const locateAll = (text: string): { x: number; y: number }[] => { - const hits: { x: number; y: number }[] = []; - const walk = (node: Container): void => { + /** + * Every text under `root` that is really on screen, in drawing order (the topmost last), handed + * to `visit` with the centre a click on it would use. + * + * A row scrolled out of its list is still `visible` – only masked away – and reporting it would + * hand a check a point outside the panel, where the click lands on whatever is underneath. That + * is how a step that passes on a short list fails on a long one (#39 in CI), so a text whose + * centre falls outside the clipping it sits in counts as not being there at all. + * + * Every hook that reads text goes through here. A `locate` that refuses a scrolled-away row + * while `overlayTexts` still lists it is worse than either rule alone: a check picks a row off + * the one hook and then cannot find it with the other. + * + * The clip is the box of the *widget* that owns the mask, not the mask itself: Pixi hands a + * graphics that serves as a mask to the mask effect, and `getBounds()` on it then reports an + * empty rectangle at the origin – which would throw away every masked text in the editor. + */ + const eachText = (root: Container, visit: (node: Text, point: { x: number; y: number }) => void): void => { + const walk = (node: Container, clip: Rectangle | null, owner: Widget | null): void => { if (!node.visible) return; - if (node instanceof Text && node.text === text) { + const holder = node instanceof Widget ? node : owner; + let inner = clip; + if (node.mask && holder) { + const r = rectOf(holder); + const box = new Rectangle(r.x, r.y, r.w, r.h); + inner = clip ? intersect(clip, box) : box; + } + if (node instanceof Text) { // Global bounds, not `width`/`height`: those are unscaled, and inside the graph canvas // (which is scaled by the zoom) that put the point right of the text it was meant to hit. const box = node.getBounds(); - hits.push({ x: box.x + box.width / 2, y: box.y + box.height / 2 }); + const point = { x: box.x + box.width / 2, y: box.y + box.height / 2 }; + if (!inner || inner.contains(point.x, point.y)) visit(node, point); return; } - for (const child of node.children) walk(child); + for (const child of node.children) walk(child, inner, holder); }; - walk(app.stage); + walk(root, null, null); + }; + /** Centres of every visible text equal to `text`, in drawing order (the topmost last). */ + const locateAll = (text: string): { x: number; y: number }[] => { + const hits: { x: number; y: number }[] = []; + eachText(app.stage, (node, point) => { + if (node.text === text) hits.push(point); + }); return hits; }; const locate = (text: string, last = false): { x: number; y: number } | null => { @@ -93,19 +134,14 @@ export function installDebugHook(app: Application, state: EditorState, editor: E }; /** * The visible text of the overlay layer, in drawing order – everything an open popup shows. - * Menus hide filtered-out entries, so this is also what a search box currently leaves. + * Menus hide filtered-out entries, so this is also what a search box currently leaves, and a + * list taller than its popup only reports the rows currently scrolled into it. */ const overlayTexts = (): string[] => { const out: string[] = []; - const walk = (node: Container): void => { - if (!node.visible) return; - if (node instanceof Text) { - if (node.text) out.push(node.text); - return; - } - for (const child of node.children) walk(child); - }; - walk(ui().overlay); + eachText(ui().overlay, (node) => { + if (node.text) out.push(node.text); + }); return out; }; /** @@ -236,6 +272,8 @@ export function installDebugHook(app: Application, state: EditorState, editor: E /** An asset row by id – two assets can share a name with the container picker's label. */ assetRow: (id: string) => assets.rowGlobal(id), assetRowColor: (id: string) => assets.rowColor(id), + /** The panel's own rectangle – what tells a row in view from one scrolled past its edge. */ + assetsRect: () => rectOf(assets), gameInfo: () => gameView.debugInfo(), /** Selection outlines in the Scene tab: a switched-off object is invisible but still framed. */ gizmos: () => viewport.gizmoInfo(), diff --git a/src/editor/panels/AssetPanel.ts b/src/editor/panels/AssetPanel.ts index 092a221b..a3cb0c8a 100644 --- a/src/editor/panels/AssetPanel.ts +++ b/src/editor/panels/AssetPanel.ts @@ -116,7 +116,14 @@ export class AssetPanel extends Panel { private syncSelection(): void { const s = this.state.selection; - this.list.select(s?.kind === 'asset' ? s.assetId : null, true); + const id = s?.kind === 'asset' ? s.assetId : null; + this.list.select(id, true); + // A selection made anywhere else – a graph node, a picker, a fresh asset – has to be reachable + // in the list, and with fifteen kinds of asset in a project the row is often below the fold. + if (id) { + const index = this.state.project.assets.findIndex((a) => a.id === id); + if (index >= 0) this.scroll.ensureVisible(index * this.list.rowHeight, this.list.rowHeight); + } } private createAsset(type: AssetType, name = ASSET_TYPE_LABELS[type], props: PropertyBag = {}): void { diff --git a/tools/e2e/21-list.mjs b/tools/e2e/21-list.mjs index b497232c..e55ddb97 100644 --- a/tools/e2e/21-list.mjs +++ b/tools/e2e/21-list.mjs @@ -90,8 +90,8 @@ export async function listTab(t) { 'a list of its own offers every kind, the struct included', (await json('__gm.list.info()')).offers.join(',') === // In the order of the kind table (`core/properties/kinds.ts`) – a new kind is a row there, - // and `sound` (#45) is the newest of them. - 'number,text,boolean,color,texture,statemachine,sound,struct', + // and `scene` (#39) is the newest of them. + 'number,text,boolean,color,texture,statemachine,sound,scene,struct', JSON.stringify((await json('__gm.list.info()')).offers), ); await pickKind('Text'); diff --git a/tools/e2e/35-load-scene.mjs b/tools/e2e/35-load-scene.mjs index 1c41811d..89b9ff67 100644 --- a/tools/e2e/35-load-scene.mjs +++ b/tools/e2e/35-load-scene.mjs @@ -114,7 +114,16 @@ export async function loadScene(t) { JSON.stringify(await playFrom()), ); - // 35c. Naming one: right-click the scene in the assets panel. + // 35c. Naming one: right-click the scene in the assets panel. By this point in the suite the + // project holds every kind of asset, so the new scene is the last row of a list far longer + // than the panel – selecting it brings it into view, which is what makes the click land. + await evaluate("__gm.state.select({ kind: 'asset', assetId: 'ls-scene' })"); + await sleep(200); + check( + 'selecting an asset scrolls its row into the panel', + await has('Level 2'), + JSON.stringify({ row: await json('__gm.assetRow("ls-scene")'), panel: await json('__gm.assetsRect()') }), + ); await clickText('Level 2', 'right'); await sleep(250); await clickText('Set as start scene');