Recurring cleanup round over the code since round 7 (bc688f3). The
issue now also asks to fix bugs and small inconsistencies found on the
way, so unlike the rounds before, this one changes behaviour in a few
places. Duplicated code in api went from 23 clones with 228 lines to
none (jscpd, at least 50 tokens). In src it stays at 22 clones with
271 lines, the same before and after.
Bugs:
- nginx served project and snapshot files as they were, so anyone who
knew a project id could read it without signing in. Every file the
API writes now goes through AtomicFile (temp file, mode, rename).
Projects, snapshots, releases, accounts and sessions are 0640, and
nginx answers 403 for them.
- Any signed-in account could delete a shared image, font or sound,
which projects of other accounts may wear. Deleting one is now for
administrators only. The editor never deleted a file, so the
client's delete is gone.
- Two saves of one project could run at once. The lock file was
removed after each save, so a waiting save and the next one locked
different files. The lock file now stays, and deleting a project
takes the lock as well.
- Bytes that are no picture got a 500 instead of a 422, because the
error handler also threw for warnings silenced with @.
- A session file was rewritten in place. A request reading it at that
moment found it empty and was signed out. It is replaced atomically.
- Delete did nothing for a shape picked in the vector art list. Delete
now goes through removeSelection, one case per kind of selection,
and a kind without a case fails the type check.
Inconsistencies:
- Escape on a question does what its Cancel does. Cancel on "Delete
project?" and "Restore snapshot?" leads back to the picker or the
list like the other questions did, and so does deleting a project.
- Editor texts use American spelling throughout, like the port names:
48 British spellings in 45 lines (colour, centre, initialised).
- objectName and groupName replace eleven copies of "own name, else
the asset's" and three of "Group". One copy fell back to the object
id instead of "?".
Duplicates, dead code, comments:
- routeFiles serves images, fonts and sounds instead of three copies.
HashedFileStore holds what the three stores shared: keeping a file
under its hash with its name, listing and deleting. The stores keep
only which bytes they take and what a record says.
- AtomicFile replaces nine copies of temp file plus rename.
- removeShapes is the one way the canvas, the inspector button and
Delete take shapes out.
- GraphInterpreter.fire was called by tests only. The 54 calls use a
helper in tests/helpers.ts now.
- FileStore.delete, HttpFileStore.delete and the PluginApi type were
unused. A doc comment in actions.ts sat above the wrong function.
Checked and left as is:
- solidCells is used by tests only. It is the per-cell view before the
merge into rectangles, the one place the largest-scale rule shows.
- An undo checker ran all 124 commands of the unit tests and compared
the project before and after undo. The only differences are empty
ports, groups and notes lists where there were none, which the file
format drops anyway.
Docs:
- README: the check sum.
- Wiki, own commit: API (admin-only deletes, 422, file modes, lock
file), Home (the "Not yet" list named three things that exist),
Concepts, Keyboard, Vector art, Architecture and Development, and
the 38 characters Gitea flagged as ambiguous on eight pages.
- .gitignore: the lock files, and pasted-*.png in any folder, which
keeps the screenshot archive out.
Tests:
- ./check.sh: 195 files, 2817 tests, exit 0 (+2 for the name helpers).
- PHP: accounts 52 in the web container and 53 in the CI image with
the race test (+2), snapshots 15 (+1), releases 20.
- Full e2e run: ALL PASSED, 1586 checks (+6 in 12, 47, 49 and 50).
- Smokes jumprun, tetris, actor: 24, 19 and 21 checks. The example
project files were restored byte for byte afterwards.
- The file stores of HEAD and of this commit give byte-identical
output for the live folders and for store, list and delete in a
throwaway folder.
- Counter-checks, all fail as expected: a 500 for bad bytes fails 12,
no admin check fails 49, removing the lock file again loses saves
in 3 of 3 runs (151 to 170 of 321), Delete ignoring shapes fails 47,
Escape that only closes fails 50, and dropping the shape case fails
the type check (TS2366).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
309 lines
13 KiB
TypeScript
309 lines
13 KiB
TypeScript
import { describe, expect, it } from 'vitest';
|
||
import { CreateAssetCommand } from '../../src/core/commands/assetCommands';
|
||
import { AddNodeCommand } from '../../src/core/commands/graphCommands';
|
||
import { AddPortCommand } from '../../src/core/commands/portCommands';
|
||
import {
|
||
AddVariableCommand,
|
||
RemoveVariableCommand,
|
||
RenameVariableCommand,
|
||
SetVariableKeptCommand,
|
||
} from '../../src/core/commands/variableCommands';
|
||
import {
|
||
BEHAVIOR_END,
|
||
BEHAVIOR_PARAM,
|
||
BEHAVIOR_START,
|
||
GraphInterpreter,
|
||
MACHINE_PORT,
|
||
RUN_BEHAVIOR,
|
||
resumedVariables,
|
||
STATE,
|
||
STATE_NAME_PARAM,
|
||
} from '../../src/core/graph';
|
||
import type { BehaviorAsset } from '../../src/core/model/Asset';
|
||
import { graphLookup } from '../../src/core/model/graphLookup';
|
||
import { deserializeProject, serializeProject } from '../../src/core/store/serialize';
|
||
import {
|
||
fakeContext,
|
||
graphEditor,
|
||
makeBehavior,
|
||
makeProject,
|
||
makeSpriteScene,
|
||
place,
|
||
type ProjectFixture,
|
||
roundTrip,
|
||
startWorld,
|
||
fire,
|
||
} from '../helpers';
|
||
|
||
/**
|
||
* A behavior that counts its calls in `count` and hands the new count out as `out1`. `steps`
|
||
* counts the same way but is left off, so it shows what an unkept variable does meanwhile.
|
||
*/
|
||
function counter(fixture: ProjectFixture): BehaviorAsset {
|
||
const callee = makeBehavior(fixture, 'Counter', ['count', 'steps']);
|
||
fixture.stack.execute(new SetVariableKeptCommand(callee.id, 'count', true));
|
||
const edit = graphEditor(fixture, callee);
|
||
const start = edit.addNode(BEHAVIOR_START);
|
||
const end = edit.addNode(BEHAVIOR_END);
|
||
fixture.stack.execute(new AddPortCommand(callee.id, end.id, { key: 'out1', kind: 'number' }));
|
||
fixture.stack.execute(new AddPortCommand(callee.id, end.id, { key: 'out2', kind: 'number' }));
|
||
const get = edit.variables(['count', 'steps']);
|
||
const plus = edit.addNode('Add', { b: 1 });
|
||
const plusSteps = edit.addNode('Add', { b: 1 });
|
||
const set = edit.variables(['count', 'steps'], { write: true });
|
||
edit.wire([
|
||
[start, 'exec', set, 'exec'],
|
||
[get, 'count', plus, 'a'],
|
||
[get, 'steps', plusSteps, 'a'],
|
||
[plus, 'result', set, 'count'],
|
||
[plusSteps, 'result', set, 'steps'],
|
||
[set, 'exec', end, 'exec'],
|
||
[plus, 'result', end, 'out1'],
|
||
[plusSteps, 'result', end, 'out2'],
|
||
]);
|
||
return callee;
|
||
}
|
||
|
||
/** A caller that runs `callee` once per `event` through each of `sites` nodes, storing `out1`. */
|
||
function callerOf(fixture: ProjectFixture, callee: BehaviorAsset, sites = 1, event = 'OnStart') {
|
||
const keys = Array.from({ length: sites }, (_, i) => `r${i}`);
|
||
const caller = makeBehavior(fixture, 'Caller', keys);
|
||
const edit = graphEditor(fixture, caller);
|
||
let previous = edit.addNode(event);
|
||
for (const key of keys) {
|
||
const call = edit.addNode(RUN_BEHAVIOR, { [BEHAVIOR_PARAM]: callee.id });
|
||
const store = edit.variables([key], { write: true });
|
||
edit.wire([
|
||
[previous, 'exec', call, 'exec'],
|
||
[call, 'exec', store, 'exec'],
|
||
[call, 'out1', store, key],
|
||
]);
|
||
previous = store;
|
||
}
|
||
return caller;
|
||
}
|
||
|
||
/** `out1` and `out2` of three calls from one node. */
|
||
function threeCalls(fixture: ProjectFixture, callee: BehaviorAsset): unknown[][] {
|
||
const caller = makeBehavior(fixture, 'Caller', ['r0', 'r1']);
|
||
const edit = graphEditor(fixture, caller);
|
||
const call = edit.addNode(RUN_BEHAVIOR, { [BEHAVIOR_PARAM]: callee.id });
|
||
const store = edit.variables(['r0', 'r1'], { write: true });
|
||
edit.wire([
|
||
[edit.addNode('OnStart'), 'exec', call, 'exec'],
|
||
[call, 'exec', store, 'exec'],
|
||
[call, 'out1', store, 'r0'],
|
||
[call, 'out2', store, 'r1'],
|
||
]);
|
||
const interpreter = new GraphInterpreter(caller.graph, graphLookup(fixture.project));
|
||
const ctx = fakeContext({ variables: { r0: 0, r1: 0 } });
|
||
const seen: unknown[][] = [];
|
||
for (let i = 0; i < 3; i++) {
|
||
fire(interpreter, 'OnStart', ctx);
|
||
seen.push([ctx.variables.r0, ctx.variables.r1]);
|
||
}
|
||
return seen;
|
||
}
|
||
|
||
describe('Keep on a behavior variable (#229)', () => {
|
||
it('starts off for a variable added to a behavior, and on a Variables asset', () => {
|
||
const fixture = makeProject();
|
||
const behavior = makeBehavior(fixture, 'B', ['count']);
|
||
expect('keptVariables' in behavior).toBe(false);
|
||
const shared = new CreateAssetCommand('variables', 'Shared');
|
||
fixture.stack.execute(shared);
|
||
fixture.stack.execute(new AddVariableCommand(shared.asset.id, 'score', 'number'));
|
||
expect('keptVariables' in shared.asset).toBe(false);
|
||
});
|
||
|
||
it('keeps none in a project saved before the switch existed', () => {
|
||
const fixture = makeProject();
|
||
const behavior = makeBehavior(fixture, 'Old', ['count']);
|
||
const raw = serializeProject(fixture.project) as { assets: Record<string, unknown>[] };
|
||
delete raw.assets.find((a) => a.id === behavior.id)!.keptVariables;
|
||
const loaded = deserializeProject(raw).assets.find((a) => a.id === behavior.id) as BehaviorAsset;
|
||
expect(loaded.keptVariables).toBeUndefined();
|
||
});
|
||
|
||
it('resumes a kept variable from the last call of this node, and restarts an unkept one', () => {
|
||
const fixture = makeProject();
|
||
// count is kept and counts 1, 2, 3; steps is not and reads 1 every time.
|
||
expect(threeCalls(fixture, counter(fixture))).toEqual([
|
||
[1, 1],
|
||
[2, 1],
|
||
[3, 1],
|
||
]);
|
||
});
|
||
|
||
it('switches off and on as one undo step each', () => {
|
||
const fixture = makeProject();
|
||
const callee = counter(fixture);
|
||
fixture.stack.execute(new SetVariableKeptCommand(callee.id, 'count', false));
|
||
expect(callee.keptVariables).toBeUndefined();
|
||
fixture.stack.undo();
|
||
expect(callee.keptVariables).toEqual(['count']);
|
||
fixture.stack.redo();
|
||
expect(callee.keptVariables).toBeUndefined();
|
||
expect(threeCalls(fixture, callee).map(([count]) => count)).toEqual([1, 1, 1]);
|
||
});
|
||
|
||
it('keeps one state per node: two nodes calling the same behavior count on their own', () => {
|
||
const fixture = makeProject();
|
||
const caller = callerOf(fixture, counter(fixture), 2);
|
||
const interpreter = new GraphInterpreter(caller.graph, graphLookup(fixture.project));
|
||
const ctx = fakeContext({ variables: { r0: 0, r1: 0 } });
|
||
fire(interpreter, 'OnStart', ctx);
|
||
expect([ctx.variables.r0, ctx.variables.r1]).toEqual([1, 1]);
|
||
fire(interpreter, 'OnStart', ctx);
|
||
expect([ctx.variables.r0, ctx.variables.r1]).toEqual([2, 2]);
|
||
// The caller's own bag never learns of `count`: the kept values stay the callee's.
|
||
expect('count' in ctx.variables).toBe(false);
|
||
});
|
||
|
||
it('keeps one state per object in a running world', () => {
|
||
const fixture = makeSpriteScene();
|
||
const callee = counter(fixture);
|
||
const caller = callerOf(fixture, callee, 1, 'OnUpdate');
|
||
const log = graphEditor(fixture, caller).addNode('Log');
|
||
const store = caller.graph.nodes.filter((n) => n.type === 'Variables').at(-1)!;
|
||
const call = caller.graph.nodes.find((n) => n.type === RUN_BEHAVIOR)!;
|
||
graphEditor(fixture, caller).wire([
|
||
[store, 'exec', log, 'exec'],
|
||
[call, 'out1', log, 'message'],
|
||
]);
|
||
place(fixture, caller.id, { x: 0, y: 0 });
|
||
place(fixture, caller.id, { x: 10, y: 0 });
|
||
const started = startWorld(fixture, { behaviors: true });
|
||
for (let i = 0; i < 3; i++) started.frame();
|
||
// Shared across objects it would read 1…6; per object each counts 1, 2, 3.
|
||
expect(started.world.log).toEqual(['1', '1', '2', '2', '3', '3']);
|
||
});
|
||
|
||
it('gives a call inside a call its own state per outer node', () => {
|
||
const fixture = makeProject();
|
||
const inner = counter(fixture);
|
||
// The middle behavior just passes the counter through.
|
||
const middle = makeBehavior(fixture, 'Middle');
|
||
const edit = graphEditor(fixture, middle);
|
||
const start = edit.addNode(BEHAVIOR_START);
|
||
const end = edit.addNode(BEHAVIOR_END);
|
||
fixture.stack.execute(new AddPortCommand(middle.id, end.id, { key: 'out1', kind: 'number' }));
|
||
const call = edit.addNode(RUN_BEHAVIOR, { [BEHAVIOR_PARAM]: inner.id });
|
||
edit.wire([
|
||
[start, 'exec', call, 'exec'],
|
||
[call, 'exec', end, 'exec'],
|
||
[call, 'out1', end, 'out1'],
|
||
]);
|
||
const caller = callerOf(fixture, middle, 2);
|
||
const interpreter = new GraphInterpreter(caller.graph, graphLookup(fixture.project));
|
||
const ctx = fakeContext({ variables: { r0: 0, r1: 0 } });
|
||
fire(interpreter, 'OnStart', ctx);
|
||
fire(interpreter, 'OnStart', ctx);
|
||
// One shared inner counter would have handed out 1, 2, 3, 4.
|
||
expect([ctx.variables.r0, ctx.variables.r1]).toEqual([2, 2]);
|
||
});
|
||
|
||
it('starts a local state machine of the callee over every call, kept variables or not', () => {
|
||
const fixture = makeProject();
|
||
const create = new CreateAssetCommand('statemachine', 'Doors');
|
||
fixture.stack.execute(create);
|
||
const open = new AddNodeCommand(create.asset.id, STATE, 0, 0, { [STATE_NAME_PARAM]: 'Open' });
|
||
fixture.stack.execute(open);
|
||
const machine = create.asset.id;
|
||
// No state yet → "first" and put it in Open; in Open → "open". The kept variable is only
|
||
// there to show that keeping one does not keep the machine along with it.
|
||
const callee = makeBehavior(fixture, 'Door', ['count']);
|
||
const edit = graphEditor(fixture, callee);
|
||
const start = edit.addNode(BEHAVIOR_START);
|
||
const branch = edit.addNode('StateBranch', { [MACHINE_PORT]: machine });
|
||
const first = edit.addNode('Log', { message: 'first' });
|
||
const set = edit.addNode('SetState', { [MACHINE_PORT]: machine });
|
||
const again = edit.addNode('Log', { message: 'open' });
|
||
edit.wire([
|
||
[start, 'exec', branch, 'exec'],
|
||
[branch, 'else', first, 'exec'],
|
||
[first, 'exec', set, open.node.id],
|
||
[branch, open.node.id, again, 'exec'],
|
||
]);
|
||
const caller = makeBehavior(fixture, 'Caller');
|
||
const cEdit = graphEditor(fixture, caller);
|
||
cEdit.connect(
|
||
cEdit.addNode('OnStart'),
|
||
'exec',
|
||
cEdit.addNode(RUN_BEHAVIOR, { [BEHAVIOR_PARAM]: callee.id }),
|
||
'exec',
|
||
);
|
||
const interpreter = new GraphInterpreter(caller.graph, graphLookup(fixture.project));
|
||
const ctx = fakeContext();
|
||
fire(interpreter, 'OnStart', ctx);
|
||
fire(interpreter, 'OnStart', ctx);
|
||
expect(ctx.logs).toEqual(['first', 'first']);
|
||
// A local machine of the callee is none of the caller's business either.
|
||
expect(ctx.states).toEqual({});
|
||
});
|
||
|
||
it('survives saving and loading, and a name that is not declared is dropped', () => {
|
||
const fixture = makeProject();
|
||
const callee = counter(fixture);
|
||
const raw = serializeProject(fixture.project) as { assets: Record<string, unknown>[] };
|
||
raw.assets.find((a) => a.id === callee.id)!.keptVariables = ['count', 'gone', 'count', 7];
|
||
const loaded = deserializeProject(raw).assets.find((a) => a.id === callee.id) as BehaviorAsset;
|
||
expect(loaded.keptVariables).toEqual(['count']);
|
||
const again = roundTrip(fixture.project).assets.find((a) => a.id === callee.id) as BehaviorAsset;
|
||
expect(again.keptVariables).toEqual(['count']);
|
||
});
|
||
|
||
it('follows its variable through a rename, and a removal gives it back on undo', () => {
|
||
const fixture = makeProject();
|
||
const callee = counter(fixture);
|
||
fixture.stack.execute(new RenameVariableCommand(callee.id, 'count', 'hits'));
|
||
expect(callee.keptVariables).toEqual(['hits']);
|
||
fixture.stack.execute(new RemoveVariableCommand(callee.id, 'hits'));
|
||
expect(callee.keptVariables).toBeUndefined();
|
||
fixture.stack.undo();
|
||
expect(callee.keptVariables).toEqual(['hits']);
|
||
fixture.stack.undo();
|
||
expect(callee.keptVariables).toEqual(['count']);
|
||
});
|
||
|
||
it('is gone from Run behavior, which has no switch of its own any more', () => {
|
||
const fixture = makeProject();
|
||
const caller = makeBehavior(fixture, 'Caller');
|
||
const added = new AddNodeCommand(caller.id, RUN_BEHAVIOR, 0, 0);
|
||
fixture.stack.execute(added);
|
||
expect(Object.keys(added.node.params)).toEqual([BEHAVIOR_PARAM]);
|
||
});
|
||
});
|
||
|
||
describe('resumedVariables', () => {
|
||
it('follows today’s declarations: new ones at their default, dropped ones gone', () => {
|
||
expect(resumedVariables({ count: 0, fresh: 5 }, { count: 7, gone: 1 }, ['count', 'fresh'])).toEqual({
|
||
count: 7,
|
||
fresh: 5,
|
||
});
|
||
});
|
||
|
||
it('restarts a variable that is not kept, whatever it was left at', () => {
|
||
expect(resumedVariables({ count: 0, steps: 0 }, { count: 7, steps: 4 }, ['count'])).toEqual({
|
||
count: 7,
|
||
steps: 0,
|
||
});
|
||
});
|
||
|
||
it('falls back to the default where the kept value is of another sort now', () => {
|
||
expect(
|
||
resumedVariables({ count: 0, name: 'a' }, { count: 'seven', name: 'b' }, ['count', 'name']),
|
||
).toEqual({
|
||
count: 0,
|
||
name: 'b',
|
||
});
|
||
});
|
||
|
||
it('is a fresh copy of the defaults when nothing was kept yet', () => {
|
||
const defaults = { count: 0 };
|
||
const variables = resumedVariables(defaults, undefined, ['count']);
|
||
expect(variables).toEqual(defaults);
|
||
expect(variables).not.toBe(defaults);
|
||
});
|
||
});
|