Trim needless comments and fix the picker width

Remove comments that only restate the code, keeping the section banners
and the notes that carry a reason, a measured fact or an external
contract.

ButtonOptions.width claimed to be a fixed width but only set minWidth,
so the Graph and Texture pickers grew with the name they showed. It is
now what it promises, with a separate minWidth for the buttons that have
two forms. Narrow buttons shrink their padding, otherwise a 24px icon
button had 8px left for its glyph and truncate dropped it entirely.

__gm.labelsDropped() guards that class: the overflow guard only saw text
that was too wide, never a label truncated to nothing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
rootandClaude Opus 5 committed 2026-09-08 15:52:05 +02:00
1 parent f16801aef3
commit 4d84231b9f
115 files changed
+707 -1564

No files matched your search

+3 -5
View File
@@ -1,7 +1,6 @@
/**
* The two pure halves of the `asset` property kind: how a stored id reads, and which assets a
* picker may offer for it. Both are used by the inspector row and by the graph node view, so
* they are pinned here rather than through a widget.
* picker may offer for it. Used by the inspector row and the graph node view alike.
*/
import { describe, expect, it } from 'vitest';
import { CreateAssetCommand } from '../../src/core/commands/assetCommands';
@@ -11,7 +10,6 @@ import type { Project } from '../../src/core/model/Project';
import { assetChoiceLabel, assetRefChoices, assetRefLabel } from '../../src/core/properties/assetRef';
import { makeProject, type ProjectFixture } from '../helpers';
/** Adds an asset and returns its id, so a test can reference it. */
function add(fixture: ProjectFixture, type: AssetType, name: string): string {
const command = new CreateAssetCommand(type, name);
fixture.stack.execute(command);
@@ -123,8 +121,8 @@ describe('assetRefChoices', () => {
});
describe('node asset params', () => {
// There used to be a second notion of an asset reference: an enum with empty options that the
// graph inspector filled in. This keeps the nodes on the real kind.
// Guards against the old second notion of an asset reference: an enum with empty options
// that the graph inspector filled in.
it('use the asset kind and offer every placeable asset', () => {
const params = BUILT_IN_NODES.flatMap((def) => def.params).filter((p) => p.key === 'asset');
expect(params.length).toBeGreaterThan(0);
+3 -6
View File
@@ -25,14 +25,12 @@ function makeSheet(width = 64, height = 64) {
return { ...fixture, textureId: create.asset.id, asset, frames: () => asset().frames };
}
/** Adds a frame through the command stack and returns its id. */
function addFrame(sheet: ReturnType<typeof makeSheet>, rect: FrameRect, name?: string): string {
const command = new AddFrameCommand(sheet.textureId, rect, name);
sheet.stack.execute(command);
return command.frameId;
}
/** A detached copy of the frame list, to compare a state against after an undo. */
function snapshot(frames: readonly TextureFrame[]): TextureFrame[] {
return frames.map((f) => ({ ...f }));
}
@@ -113,8 +111,8 @@ describe('SetFrameRectCommand', () => {
const id = addFrame(sheet, { x: 0, y: 0, w: 16, h: 16 });
const frame = sheet.frames()[0]!;
// What a live preview does: write into the frame, then hand the command the previewed
// rectangle and put the starting one back before executing (see `SceneViewport`).
// What a live preview does: hand the command the previewed rectangle, with the starting
// one put back before executing (see `SceneViewport`).
frame.x = 40;
const previewed = { x: frame.x, y: frame.y, w: frame.w, h: frame.h };
frame.x = 0;
@@ -251,8 +249,7 @@ describe('SliceFramesCommand', () => {
});
it('rejects a slice of an image whose size is unknown rather than doing nothing', () => {
// Width 0 is a texture whose upload has not been measured yet; slicing it would silently
// produce no frames at all.
// Width 0 is an unmeasured upload; slicing it would silently produce no frames at all.
const sheet = makeSheet(0, 0);
expect(() => sheet.stack.execute(new SliceFramesCommand(sheet.textureId, GRID_2X2, false))).toThrow(
CommandError,
+2 -4
View File
@@ -1,7 +1,6 @@
/**
* Container assets hold placed objects just like scenes do. Anything that walks "all placed
* objects" has to cover both – the tests here pin the places that used to look at scenes only.
* The history tests pin that a rejected undo leaves the stacks usable.
* Container assets hold placed objects just like scenes do, so anything that walks "all placed
* objects" has to cover both. Plus: a rejected undo leaves the stacks usable.
*/
import { describe, expect, it } from 'vitest';
import { type Command, CommandError } from '../../src/core/commands/Command';
@@ -71,7 +70,6 @@ describe('RenameVariableCommand', () => {
});
});
/** Applies a marker to the project; its undo refuses to run. */
class RefusingUndo implements Command {
readonly label = 'Refusing';
execute(project: { name: string }): void {
+13 -12
View File
@@ -1,8 +1,6 @@
/**
* One table covering every built-in node that has a `run`, plus a guard that keeps it that way:
* adding a node without a case here fails `covers every built-in node`. Nodes without `run`
* (the events) are seeded by the interpreter, so they are listed with the test that drives them
* and checked in tests/core/behavior-events.test.ts.
* One table covering every built-in node that has a `run`: adding a node without a case here
* fails `covers every built-in node`. The run-less event nodes are listed in `EVENT_NODES`.
*/
import { describe, expect, it } from 'vitest';
import { BUILT_IN_NODES } from '../../src/core/graph/nodes';
@@ -29,7 +27,6 @@ const run = (type: string, c: Case): { result: RunResult; ctx: FakeContext } =>
return { result: requireNodeDef(type).run!(ctx, c.inputs ?? {}, c.params ?? {}), ctx };
};
/** A handle with a fixed position, for the geometry nodes. */
const at = (name: string, x: number, y: number) => ({ ...fakeEntity(name), x, y });
const pointer = (overrides: Partial<NodeContext['pointer']>): Partial<NodeContext> => ({
@@ -38,6 +35,7 @@ const pointer = (overrides: Partial<NodeContext['pointer']>): Partial<NodeContex
const CASES: Record<string, Case[]> = {
// --- flow ------------------------------------------------------------------------------------
Branch: [
{ name: 'true takes then', inputs: { condition: true }, next: 'then' },
{ name: 'false takes else', inputs: { condition: false }, next: 'else' },
@@ -47,6 +45,7 @@ const CASES: Record<string, Case[]> = {
Sequence: [{ name: 'runs all three outputs in order', next: ['a', 'b', 'c'] }],
// --- variables -------------------------------------------------------------------------------
GetVariable: [
{ name: 'reads a variable', params: { variable: 'speed' }, outputs: { value: 100 } },
{ name: 'an unknown variable reads as null', params: { variable: 'nope' }, outputs: { value: null } },
@@ -69,6 +68,7 @@ const CASES: Record<string, Case[]> = {
],
// --- references and properties ----------------------------------------------------------------
Self: [{ name: 'outputs the behavior object', outputs: { object: fakeContext().self } }],
Owner: [
{ name: 'outputs the object the behavior is parented to', outputs: { object: fakeContext().owner } },
@@ -154,6 +154,7 @@ const CASES: Record<string, Case[]> = {
],
// --- literals and math -------------------------------------------------------------------------
Number: [
{ name: 'outputs its setting', params: { value: 7.5 }, outputs: { value: 7.5 } },
{ name: 'unset is 0', outputs: { value: 0 } },
@@ -210,8 +211,7 @@ const CASES: Record<string, Case[]> = {
{ name: 'modulo zero is 0, not NaN', inputs: { a: 125, b: 0 }, outputs: { result: 0 } },
],
Random: [
// The spread is checked in "Random stays inside [min, max)" below; an empty range is the
// one case with a single right answer.
// The spread is checked in "Random stays inside [min, max)" below.
{ name: 'an empty range always yields min', inputs: { min: 3, max: 3 }, outputs: { result: 3 } },
],
Time: [
@@ -223,6 +223,7 @@ const CASES: Record<string, Case[]> = {
],
// --- text and comparison -----------------------------------------------------------------------
ToText: [
{ name: 'formats with decimals', inputs: { value: 12.345, decimals: 1 }, outputs: { text: '12.3' } },
{ name: 'text passes through unformatted', inputs: { value: 'x', decimals: 2 }, outputs: { text: 'x' } },
@@ -267,6 +268,7 @@ const CASES: Record<string, Case[]> = {
],
// --- vectors and input --------------------------------------------------------------------------
Length: [
{ name: '3-4-5 triangle', inputs: { x: 3, y: 4 }, outputs: { result: 5 } },
{ name: 'the zero vector has length 0', inputs: { x: 0, y: 0 }, outputs: { result: 0 } },
@@ -390,6 +392,7 @@ const CASES: Record<string, Case[]> = {
],
// --- actions --------------------------------------------------------------------------------------
Move: [
{
name: 'moves by (dx, dy) × scale × dt',
@@ -479,8 +482,8 @@ function eqNamesakes(): Record<string, Value> {
}
/**
* Nodes without a `run`: the interpreter seeds their outputs when the BehaviorSystem fires them,
* so they are only observable through a running world. The value says which test drives it.
* Nodes without a `run`: only observable through a running world, since the BehaviorSystem
* seeds their outputs. The value names the test that drives each one.
*/
const EVENT_NODES: Record<string, string> = {
OnStart: 'graph.test.ts',
@@ -513,8 +516,7 @@ describe('node catalog', () => {
const keys = [...def.inputs, ...def.outputs].map((p) => p.key);
expect(def.label, `${def.type} has a label`).not.toBe('');
expect(def.description, `${def.type} has a description`).not.toBe('');
// Inputs and outputs are addressed by key within their own side; a key may repeat across
// sides (Normalize reads x/y and writes x/y), never within one.
// Keys are unique per side, but may repeat across sides (Normalize reads and writes x/y).
expect(new Set(def.inputs.map((p) => p.key)).size, `${def.type} input keys unique`).toBe(
def.inputs.length,
);
@@ -545,7 +547,6 @@ for (const [type, cases] of Object.entries(CASES)) {
});
}
// Cases whose point is the mutation they cause – stated here rather than squeezed into the table.
describe('action nodes write through to the object they are given', () => {
it('Move applies dx/dy × scale × dt', () => {
const target = at('m', 0, 0);
+3 -8
View File
@@ -18,7 +18,6 @@ import { makeSpriteScene, place, roundTrip } from '../helpers';
const FILE = `${'a1b2c3d4'.repeat(8)}.png`;
const OTHER_FILE = `${'0f'.repeat(32)}.webp`;
/** Adds a texture asset carrying `file` and returns it. */
function addTexture(fixture: ReturnType<typeof makeSpriteScene>, file = FILE, name = 'Bricks') {
const command = new CreateTextureCommand(name, { file, width: 64, height: 48 });
fixture.stack.execute(command);
@@ -51,7 +50,7 @@ describe('resolveTexture', () => {
const sprite = findAsset(fixture.project, fixture.spriteId)!;
sprite.props.texture = texture.id;
// A whole-texture reference carries no rectangle: the renderer then needs no matrix at all.
// A whole-texture reference carries no rectangle.
const whole = { file: FILE, rect: null, sourceWidth: 64, sourceHeight: 48 };
expect(resolveTexture(fixture.project, sprite.props)).toEqual({ ...whole, smooth: true });
texture.props.filter = 'nearest';
@@ -64,8 +63,6 @@ describe('resolveTexture', () => {
texture.frames.push({ id: 'f1', name: 'Idle', x: 16, y: 8, w: 32, h: 24 });
const sprite = findAsset(fixture.project, fixture.spriteId)!;
// The same property holds either kind of id, which is what keeps sprites written before
// frames existed meaning exactly what they meant.
sprite.props.texture = 'f1';
expect(resolveTexture(fixture.project, sprite.props)).toEqual({
file: FILE,
@@ -146,8 +143,7 @@ describe('findPropertyUsages', () => {
sprite.props.texture = texture.id;
const objectId = place(fixture, fixture.spriteId, { x: 0, y: 0 });
// A texture is never placed, so the "is it used?" question `findUsages` answers is blind
// to it – that is exactly the gap this function fills.
// A texture is never placed, so `findUsages` is blind to it – hence this function.
expect(findUsages(fixture.project, texture.id)).toHaveLength(0);
const usages = findPropertyUsages(fixture.project, texture.id);
@@ -165,8 +161,7 @@ describe('findPropertyUsages', () => {
texture.frames.push({ id: 'f1', name: 'Idle', x: 0, y: 0, w: 8, h: 8 });
findAsset(fixture.project, fixture.spriteId)!.props.texture = 'f1';
// Asking by the asset id alone under-reports exactly the sheets that were sliced – the
// ones with the most users – which is why deleting one asks about every id it owns.
// The asset id alone under-reports sliced sheets, so deleting one asks about every id it owns.
expect(findPropertyUsages(fixture.project, texture.id)).toHaveLength(0);
expect(findPropertyUsages(fixture.project, textureRefIds(texture))).toHaveLength(1);
});