Refactoring round 6: shared sheet canvas, fewer copies (#85)
Recurring cleanup round. Behaviour is unchanged. Duplicated code in
src went from 27 clones with 299 lines to 6 clones with 56 lines
(jscpd, the two identical OFL licence texts not counted).
- SheetCanvas is the new base of the texture, pixel art and vector art
canvases. It holds background, mask, hint, panning, wheel zoom, fit,
view save and restore and the screen mapping. Pixel art keeps its
whole-pixel panning and fixed zoom steps.
- mixColor in src/core/util/color.ts replaces three copies in
animation, gradient and particles. It clamps t like the particle one.
- fingerprint now lives once in generated.ts, next to the revision
contract from #185.
- MoveCommand next to RenameCommand is the base of the three reorder
commands for tile pictures, tilemap layers and vector shapes.
- The two raycast nodes share inputs, settings and filter via rayOf.
- canvasRect() lives once in AssetTabEditor.
- Removed unused code: findPixelArt, isDrawableShape,
GeneratedPictures.has and SnapToggle.snapping. Tests now check the
same things through equivalent paths.
- 38 values used only in their own file lost their export.
- Three comments that told history now describe the current state.
The bundle stayed md5-identical for those edits.
- README lists vector art, pixel art and palette, and links the two
wiki pages.
Findings:
- PaintTilesCommand.changes is meant to prevent empty undo steps but
has not been called since #128. The rectangle tool therefore records
a no-op undo step when it paints the same tile over itself. Left as
is, because fixing it changes behaviour.
- No test covered middle-click panning in any sheet tab before. 13f
and 48a now check it.
- The default values of the ray inputs were untested. Length 1000
could become 10 without a red test. tests/core/raycast.test.ts now
covers it.
- CI measured 1308 editor checks at d7c0b7c, not the 1307 counted for
#164. The sum is now 1310 (1308 + 2), computed.
Tests:
- ./check.sh: 144 files, 2143 tests, exit 0 (before 2142).
- e2e 1-16,37,39-43,47,48 fresh: ALL PASSED, 729 checks.
- e2e 1-13,47,48 with the new checks: ALL PASSED, 518 checks.
- Counter-checks, each restored via cp and cmp: halved panning fails
both new pan checks; unclamped mix fails 1 test, no blue channel 3;
coarser hash 3; broken reorder undo 2; triggers setting forced on 2;
bent ray length 1.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
d7c0b7cfdb
commit
056a622524
51 files changed
+393
-525
No files matched your search
@@ -67,7 +67,7 @@ export function fixedPortKeys(def: NodeDef): string[] {
|
||||
return [...fixed.map((p) => p.key), ...def.params.map((p) => p.key)];
|
||||
}
|
||||
|
||||
export function declaredOn(
|
||||
function declaredOn(
|
||||
def: NodeDef,
|
||||
ports: readonly DeclaredPort[],
|
||||
side: 'inputs' | 'outputs',
|
||||
|
||||
@@ -1,11 +1,45 @@
|
||||
import { ALL_LAYERS, layerMaskOf } from '../../../model/collisionLayers';
|
||||
import type { NodeDef } from '../../NodeRegistry';
|
||||
import { asBoolean, asNumber } from '../../values';
|
||||
import type { PropertyBag } from '../../../model/Properties';
|
||||
import type { PropertyDef } from '../../../properties/PropertySchema';
|
||||
import type { NodeContext } from '../../NodeContext';
|
||||
import type { NodeDef, PortDef } from '../../NodeRegistry';
|
||||
import { asBoolean, asNumber, type Value } from '../../values';
|
||||
import { bool, exec, num, obj, withDefault } from '../shared';
|
||||
|
||||
/** The mask of layers a ray looks at – shared with `Raycast all` (#157) once that exists. */
|
||||
/** The mask of layers a ray looks at (#158). */
|
||||
export const RAY_LAYERS_PARAM = 'layers';
|
||||
|
||||
/** Where a ray starts, which way it points and how far it reaches – `Raycast all` takes the same. */
|
||||
export const RAY_INPUTS: readonly PortDef[] = [
|
||||
num('fromX', 'from x'),
|
||||
num('fromY', 'from y'),
|
||||
withDefault(num('dirX', 'dir x'), 1),
|
||||
num('dirY', 'dir y'),
|
||||
withDefault(num('length'), 1000),
|
||||
];
|
||||
|
||||
/** What a ray passes over and which layers it looks at, so both ray nodes are filtered alike. */
|
||||
export const RAY_PARAMS: readonly PropertyDef[] = [
|
||||
{ key: 'ignoreOwner', label: 'Ignore owner', kind: 'boolean', default: true },
|
||||
{ key: 'triggers', label: 'Hit triggers', kind: 'boolean', default: false },
|
||||
// Every layer, so a graph written before there was a choice answers as it did (#158).
|
||||
{ key: RAY_LAYERS_PARAM, label: 'Layers', kind: 'layers', default: ALL_LAYERS },
|
||||
];
|
||||
|
||||
/** The ray a node's inputs and settings describe, in the form `NodeContext.raycast` takes it. */
|
||||
export function rayOf(ctx: NodeContext, inputs: Record<string, Value>, params: PropertyBag) {
|
||||
return {
|
||||
from: { x: asNumber(inputs.fromX), y: asNumber(inputs.fromY) },
|
||||
direction: { x: asNumber(inputs.dirX), y: asNumber(inputs.dirY) },
|
||||
length: asNumber(inputs.length),
|
||||
options: {
|
||||
ignore: asBoolean(params.ignoreOwner ?? true) ? (ctx.owner ?? ctx.self) : null,
|
||||
triggers: asBoolean(params.triggers ?? false),
|
||||
layers: layerMaskOf(params[RAY_LAYERS_PARAM] ?? ALL_LAYERS),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const MISSED = {
|
||||
hit: false,
|
||||
object: null,
|
||||
@@ -28,14 +62,7 @@ export const Raycast: NodeDef = {
|
||||
'"Layers", triggers only with "Hit triggers", a one-way one only from the side it blocks from. What ' +
|
||||
'the ray starts inside is not met, and with "Ignore owner" neither is the owner or anything parented ' +
|
||||
'below it.',
|
||||
inputs: [
|
||||
exec(),
|
||||
num('fromX', 'from x'),
|
||||
num('fromY', 'from y'),
|
||||
withDefault(num('dirX', 'dir x'), 1),
|
||||
num('dirY', 'dir y'),
|
||||
withDefault(num('length'), 1000),
|
||||
],
|
||||
inputs: [exec(), ...RAY_INPUTS],
|
||||
outputs: [
|
||||
exec(),
|
||||
bool('hit'),
|
||||
@@ -47,23 +74,10 @@ export const Raycast: NodeDef = {
|
||||
num('normalY', 'normal y'),
|
||||
num('distance'),
|
||||
],
|
||||
params: [
|
||||
{ key: 'ignoreOwner', label: 'Ignore owner', kind: 'boolean', default: true },
|
||||
{ key: 'triggers', label: 'Hit triggers', kind: 'boolean', default: false },
|
||||
// Every layer, so a graph written before there was a choice answers as it did (#158).
|
||||
{ key: RAY_LAYERS_PARAM, label: 'Layers', kind: 'layers', default: ALL_LAYERS },
|
||||
],
|
||||
params: [...RAY_PARAMS],
|
||||
run: (ctx, inputs, params) => {
|
||||
const hit = ctx.raycast(
|
||||
{ x: asNumber(inputs.fromX), y: asNumber(inputs.fromY) },
|
||||
{ x: asNumber(inputs.dirX), y: asNumber(inputs.dirY) },
|
||||
asNumber(inputs.length),
|
||||
{
|
||||
ignore: asBoolean(params.ignoreOwner ?? true) ? (ctx.owner ?? ctx.self) : null,
|
||||
triggers: asBoolean(params.triggers ?? false),
|
||||
layers: layerMaskOf(params[RAY_LAYERS_PARAM] ?? ALL_LAYERS),
|
||||
},
|
||||
);
|
||||
const ray = rayOf(ctx, inputs, params);
|
||||
const hit = ctx.raycast(ray.from, ray.direction, ray.length, ray.options);
|
||||
if (!hit) return { next: 'exec', outputs: MISSED };
|
||||
return {
|
||||
next: 'exec',
|
||||
|
||||
@@ -1,9 +1,8 @@
|
||||
import { ALL_LAYERS, layerMaskOf } from '../../../model/collisionLayers';
|
||||
import type { RayOutcome } from '../../NodeContext';
|
||||
import type { NodeDef } from '../../NodeRegistry';
|
||||
import { asBoolean, asNumber, type ValueList } from '../../values';
|
||||
import { exec, int, list, num, withDefault } from '../shared';
|
||||
import { RAY_LAYERS_PARAM } from './Raycast';
|
||||
import { asNumber, type ValueList } from '../../values';
|
||||
import { exec, int, list, num } from '../shared';
|
||||
import { RAY_INPUTS, RAY_PARAMS, rayOf } from './Raycast';
|
||||
|
||||
/**
|
||||
* The hits as parallel lists: item 0 of every list is the nearest hit, item 1 the next, and so on
|
||||
@@ -30,15 +29,7 @@ export const RaycastAll: NodeDef = {
|
||||
'come out sorted by distance, one item per hit, and "item at" the same index in each of them ' +
|
||||
'belongs to the same hit. A collider counts once, however many pieces it is made of; a tilemap ' +
|
||||
'once per cell the ray goes through. "max hits" above 0 keeps that many of the nearest.',
|
||||
inputs: [
|
||||
exec(),
|
||||
num('fromX', 'from x'),
|
||||
num('fromY', 'from y'),
|
||||
withDefault(num('dirX', 'dir x'), 1),
|
||||
num('dirY', 'dir y'),
|
||||
withDefault(num('length'), 1000),
|
||||
int('maxHits', 0, 'max hits'),
|
||||
],
|
||||
inputs: [exec(), ...RAY_INPUTS, int('maxHits', 0, 'max hits')],
|
||||
outputs: [
|
||||
exec(),
|
||||
num('count'),
|
||||
@@ -50,24 +41,13 @@ export const RaycastAll: NodeDef = {
|
||||
list('normalsY', 'normals y'),
|
||||
list('distances'),
|
||||
],
|
||||
params: [
|
||||
{ key: 'ignoreOwner', label: 'Ignore owner', kind: 'boolean', default: true },
|
||||
{ key: 'triggers', label: 'Hit triggers', kind: 'boolean', default: false },
|
||||
// The same param as `Raycast` carries, so both nodes are filtered the same way (#158).
|
||||
{ key: RAY_LAYERS_PARAM, label: 'Layers', kind: 'layers', default: ALL_LAYERS },
|
||||
],
|
||||
params: [...RAY_PARAMS],
|
||||
run: (ctx, inputs, params) => {
|
||||
const hits = ctx.raycastAll(
|
||||
{ x: asNumber(inputs.fromX), y: asNumber(inputs.fromY) },
|
||||
{ x: asNumber(inputs.dirX), y: asNumber(inputs.dirY) },
|
||||
asNumber(inputs.length),
|
||||
{
|
||||
ignore: asBoolean(params.ignoreOwner ?? true) ? (ctx.owner ?? ctx.self) : null,
|
||||
triggers: asBoolean(params.triggers ?? false),
|
||||
layers: layerMaskOf(params[RAY_LAYERS_PARAM] ?? ALL_LAYERS),
|
||||
maxHits: Math.max(0, Math.floor(asNumber(inputs.maxHits))),
|
||||
},
|
||||
);
|
||||
const ray = rayOf(ctx, inputs, params);
|
||||
const hits = ctx.raycastAll(ray.from, ray.direction, ray.length, {
|
||||
...ray.options,
|
||||
maxHits: Math.max(0, Math.floor(asNumber(inputs.maxHits))),
|
||||
});
|
||||
return { next: 'exec', outputs: listsOf(hits) };
|
||||
},
|
||||
};
|
||||
@@ -110,7 +110,7 @@ export function variableBag(ctx: NodeContext, source: string): PropertyBag {
|
||||
}
|
||||
|
||||
/** Where a machine's current state is kept: shared unless the machine is `local`, else per instance. */
|
||||
export function stateBag(ctx: NodeContext, machineId: string): Record<string, string> {
|
||||
function stateBag(ctx: NodeContext, machineId: string): Record<string, string> {
|
||||
return machineId in ctx.shared.states ? ctx.shared.states : ctx.states;
|
||||
}
|
||||
|
||||
|
||||
@@ -26,7 +26,7 @@ export function isEntity(value: Value | undefined): value is EntityHandle {
|
||||
}
|
||||
|
||||
/** A list of anything a data edge carries – a handle among the items is what `isList` cannot say. */
|
||||
export function isValueList(value: Value | undefined): value is ValueList {
|
||||
function isValueList(value: Value | undefined): value is ValueList {
|
||||
return Array.isArray(value);
|
||||
}
|
||||
|
||||
@@ -63,7 +63,7 @@ export function asValueItem(value: Value | undefined): ValueList[number] {
|
||||
}
|
||||
|
||||
/** A value narrowed to what a list may hold: a struct stays one, handles and lists flatten. */
|
||||
export function asListItem(value: Value | undefined): number | string | boolean | StructValue {
|
||||
function asListItem(value: Value | undefined): number | string | boolean | StructValue {
|
||||
if (isStructValue(value)) return value;
|
||||
return asPrimitive(value);
|
||||
}
|
||||
@@ -128,7 +128,7 @@ function isStructValue(value: Value | undefined): value is StructValue {
|
||||
* A struct slot keeps the fields it was declared with: what arrives fills the ones it names and
|
||||
* the rest stay put. That is what lets one node set some values of a bundle without erasing it.
|
||||
*/
|
||||
export function mergeStruct(current: StructValue, value: Value | undefined): StructValue {
|
||||
function mergeStruct(current: StructValue, value: Value | undefined): StructValue {
|
||||
const incoming = asStruct(value);
|
||||
const merged: Record<string, PropertyValue> = {};
|
||||
for (const [field, own] of Object.entries(current))
|
||||
@@ -137,7 +137,7 @@ export function mergeStruct(current: StructValue, value: Value | undefined): Str
|
||||
}
|
||||
|
||||
/** `value` converted to the type the slot already holds, so a number stays a number. */
|
||||
export function asTypeOf(current: PropertyValue | undefined, value: Value | undefined): PropertyValue {
|
||||
function asTypeOf(current: PropertyValue | undefined, value: Value | undefined): PropertyValue {
|
||||
if (isList(current)) return storedList(asList(value));
|
||||
if (isStruct(current)) return mergeStruct(current, value);
|
||||
if (typeof current === 'number') return asNumber(value);
|
||||
|
||||
Reference in new issue
Block a user