Texture fills show in the vector tab as soon as they load (#196)
game-maker / gate (push) Successful in 2m21s
game-maker / suite (push) Failing after 7m29s

After signing out and back in, a texture fill stayed in its base
colour until something else redrew the vector tab. The texture
library is a singleton that outlives the editor. Four widgets
subscribed to it and never unsubscribed. After a new sign-in the
first loaded texture called a destroyed canvas, which threw and
stopped the loop before the vector tab heard about the image.

- TextureCanvas, TileCanvas, ShaderPreview and WorldViewport now
  unsubscribe in destroy(). This is the pattern FontPreview and
  VectorArtCanvas already use. WorldViewport leaves both the
  texture and the font library.
- FileLibrary.notifyChange catches errors per listener. A broken
  listener no longer starves the others. The error is still
  logged as "[textures] a listener failed".
- e2e 47i no longer forces a redraw. It waits up to 3 s for the
  tab to show the texture on its own.

Findings:
- The trigger was e2e module 45 (sign out, sign in), not the
  texture fill. My note in #196 that the browser showed no
  exception was wrong. The counter-check shows it clearly.

Tests:
- ./check.sh: 137 files, 2051 tests, exit 0 (before 136 / 2049)
- 2 new unit tests in tests/viewport/file-library.test.ts
- GM_STEPS=45,47 after a fresh seed: ALL PASSED, 71 checks
- Counter-check with the old source and the new check: 1 FAIL
  (shape stays #0000ff) plus the TypeError. Old FileLibrary.ts
  fails the first unit test.
- No full e2e run locally. The check count is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
adminandClaude Opus 5 committed 2026-09-21 17:48:10 +02:00
1 parent 66d6aafbb4
commit 6a2b06dd4b
7 files changed
+114 -14

No files matched your search

+3 -1
View File
@@ -32,6 +32,7 @@ export class ShaderPreview extends Widget {
private clockOrigin = 0; private clockOrigin = 0;
/** Null: the clock runs. A number: the preview stands at that second. */ /** Null: the clock runs. A number: the preview stands at that second. */
private held: number | null = null; private held: number | null = null;
private readonly stopPictures: () => void;
constructor(private readonly state: EditorState) { constructor(private readonly state: EditorState) {
super(); super();
@@ -39,7 +40,7 @@ export class ShaderPreview extends Widget {
this.content.addChild(this.mesh); this.content.addChild(this.mesh);
this.content.mask = this.clip; this.content.mask = this.clip;
this.addChild(this.bg, this.content, this.clip); this.addChild(this.bg, this.content, this.clip);
textureLibrary.onChange(() => this.invalidate()); this.stopPictures = textureLibrary.onChange(() => this.invalidate());
} }
override measure(): Size { override measure(): Size {
@@ -107,6 +108,7 @@ export class ShaderPreview extends Widget {
} }
override destroy(): void { override destroy(): void {
this.stopPictures();
disposeSpriteMesh(this.mesh); disposeSpriteMesh(this.mesh);
super.destroy(); super.destroy();
} }
+8 -1
View File
@@ -136,6 +136,7 @@ export class TextureCanvas extends Widget {
private collision = false; private collision = false;
/** The outline a click adds corners to; for another frame, or one gone, the frame's last one. */ /** The outline a click adds corners to; for another frame, or one gone, the frame's last one. */
private active = { frameId: '', index: -1 }; private active = { frameId: '', index: -1 };
private readonly stopPictures: () => void;
constructor(private readonly state: EditorState) { constructor(private readonly state: EditorState) {
super(); super();
@@ -159,7 +160,13 @@ export class TextureCanvas extends Widget {
this.redraw(); this.redraw();
}); });
// A picture arrives asynchronously; until then there is nothing to fit the view to. // A picture arrives asynchronously; until then there is nothing to fit the view to.
textureLibrary.onChange(() => this.redraw()); this.stopPictures = textureLibrary.onChange(() => this.redraw());
}
/** The library outlives a signed-out editor; a stale listener would redraw destroyed graphics. */
override destroy(options?: Parameters<Widget['destroy']>[0]): void {
this.stopPictures();
super.destroy(options);
} }
override measure(): Size { override measure(): Size {
+8 -1
View File
@@ -68,6 +68,7 @@ export class TileCanvas extends Widget {
private board = { w: 0, h: 0 }; private board = { w: 0, h: 0 };
private info: TileInfo | null = null; private info: TileInfo | null = null;
private needsFit = true; private needsFit = true;
private readonly stopPictures: () => void;
constructor(private readonly state: EditorState) { constructor(private readonly state: EditorState) {
super(); super();
@@ -90,7 +91,13 @@ export class TileCanvas extends Widget {
state.on('project', () => this.redraw()); state.on('project', () => this.redraw());
state.on('load', () => this.show('')); state.on('load', () => this.show(''));
// A picture arrives asynchronously; the board is empty until it does. // A picture arrives asynchronously; the board is empty until it does.
textureLibrary.onChange(() => this.redraw()); this.stopPictures = textureLibrary.onChange(() => this.redraw());
}
/** The library outlives a signed-out editor; a stale listener would redraw destroyed graphics. */
override destroy(options?: Parameters<Widget['destroy']>[0]): void {
this.stopPictures();
super.destroy(options);
} }
override measure(): Size { override measure(): Size {
+11 -2
View File
@@ -61,6 +61,7 @@ export abstract class WorldViewport extends Widget {
private visibleCount = 0; private visibleCount = 0;
private animated = false; private animated = false;
protected lastItems: DrawItem[] = []; protected lastItems: DrawItem[] = [];
private readonly stopFiles: (() => void)[];
protected constructor(protected readonly state: EditorState) { protected constructor(protected readonly state: EditorState) {
super(); super();
@@ -78,8 +79,16 @@ export abstract class WorldViewport extends Widget {
if (this.visible && !this.animated) this.redraw(); if (this.visible && !this.animated) this.redraw();
}); });
// Unconditional: a picture or a typeface arriving fires once per file, not per frame. // Unconditional: a picture or a typeface arriving fires once per file, not per frame.
textureLibrary.onChange(() => this.redraw()); this.stopFiles = [
fontLibrary.onChange(() => this.redraw()); textureLibrary.onChange(() => this.redraw()),
fontLibrary.onChange(() => this.redraw()),
];
}
/** The libraries outlive a signed-out editor; a stale listener would redraw destroyed graphics. */
override destroy(options?: Parameters<Widget['destroy']>[0]): void {
for (const stop of this.stopFiles) stop();
super.destroy(options);
} }
override measure(): Size { override measure(): Size {
+7 -1
View File
@@ -30,7 +30,13 @@ export abstract class FileLibrary<T> {
/** Runs the listeners: something they draw from has changed without a file arriving here. */ /** Runs the listeners: something they draw from has changed without a file arriving here. */
notifyChange(): void { notifyChange(): void {
for (const listener of this.listeners) listener(); // One listener that throws must not keep the file from the ones after it (#196).
for (const listener of this.listeners)
try {
listener();
} catch (error) {
console.error(`[${this.tag}] a listener failed`, error);
}
} }
protected abstract fetchFile(url: string, file: string): Promise<T>; protected abstract fetchFile(url: string, file: string): Promise<T>;
+67
View File
@@ -0,0 +1,67 @@
/**
* The base of the picture and font libraries: a file asked for is fetched once, and the listeners
* hear when it lands. The libraries are module singletons and outlive an editor that was signed out
* of, so a listener can belong to a widget that is gone (#196).
*/
import { describe, expect, it, vi } from 'vitest';
import { FileLibrary } from '../../src/viewport/FileLibrary';
/** A library whose loads a test resolves by hand. */
class HandLibrary extends FileLibrary<string> {
readonly kept = new Map<string, string>();
private readonly pending: (() => void)[] = [];
constructor() {
super('/files', 'test');
}
ask(file: string): void {
if (!this.kept.has(file)) this.startLoading(file);
}
land(): Promise<void> {
for (const resolve of this.pending.splice(0)) resolve();
return new Promise((resolve) => setTimeout(resolve, 0));
}
protected fetchFile(url: string): Promise<string> {
return new Promise((resolve) => this.pending.push(() => resolve(url)));
}
protected keep(file: string, loaded: string): void {
this.kept.set(file, loaded);
}
}
describe('the file library', () => {
it('tells every listener when a file lands, even after one of them threw', async () => {
const library = new HandLibrary();
const error = vi.spyOn(console, 'error').mockImplementation(() => {});
// First in line, as the canvas of an editor signed out of is: its graphics are destroyed.
library.onChange(() => {
throw new TypeError("Cannot read properties of null (reading 'clear')");
});
const later = vi.fn();
library.onChange(later);
library.ask('a.png');
await library.land();
expect(library.kept.get('a.png')).toBe('/files/a.png');
expect(later).toHaveBeenCalledTimes(1);
// Not swallowed: the broken listener still shows in the console.
expect(error).toHaveBeenCalledWith('[test] a listener failed', expect.any(TypeError));
error.mockRestore();
});
it('forgets a listener once its unsubscribe has run', async () => {
const library = new HandLibrary();
const gone = vi.fn();
const stop = library.onChange(gone);
stop();
library.ask('a.png');
await library.land();
expect(gone).not.toHaveBeenCalled();
});
});
+10 -8
View File
@@ -843,15 +843,17 @@ export async function vectorArt(t) {
const halvesEntry = Boolean(halvesName) && filteredOffers.includes(halvesName); const halvesEntry = Boolean(halvesName) && filteredOffers.includes(halvesName);
await press(halvesEntry ? 'Enter' : 'Escape'); await press(halvesEntry ? 'Enter' : 'Escape');
await settle(); await settle();
// The picture loads asynchronously and the shape keeps its plain fill until it is there. In the // The picture loads asynchronously and the shape keeps its plain fill until it is there; the tab
// full run the tab then waits for the next redraw instead of drawing when the picture lands // redraws by itself when it lands. Nothing here asks for that redraw: after module 45 had signed
// (#196), so the check asks for that redraw itself. Remove the preview() with #196. // out and in, a canvas of the old editor used to swallow the news (#196).
await sleep(300); let leftHalf = await tabPixel(60, 100);
await evaluate('__gm.state.preview()'); let rightHalf = await tabPixel(196, 100);
await settle(); for (let waited = 0; waited < 3000 && colourDistance(rightHalf, HALVES_RIGHT) >= 20; waited += 100) {
await sleep(100);
leftHalf = await tabPixel(60, 100);
rightHalf = await tabPixel(196, 100);
}
const tiled = (await sky())?.paint; const tiled = (await sky())?.paint;
const leftHalf = await tabPixel(60, 100);
const rightHalf = await tabPixel(196, 100);
check( check(
'Texture offers the uploaded pictures but no drawing, and fills the shape with the one picked', 'Texture offers the uploaded pictures but no drawing, and fills the shape with the one picked',
halvesEntry && halvesEntry &&