Pixel art tools show only useful options and remove any colour (#201)
The pixel art side bar showed every tool option all the time, the palette used only part of the column width and "Remove" could only drop the last colour. Now each tool shows what it needs, the palette fills the column and any colour can be removed. - Options per tool live in toolOptions.ts. Brush shows for pencil, eraser, line and unfilled shapes. Filled shapes shows for rectangle and ellipse. Contiguous shows for fill. Picker has none. - All options share one row with a fixed height, so nothing below jumps when the tool changes. - The palette picks its column count from the width and stretches the swatches until the row ends flush. At 1280 px that is 11 per row instead of 8. - The colour field takes the space the two buttons leave, so "Remove" is no longer cut off. - "Remove" deletes the selected colour. Pixels of that colour turn transparent, later indices move down by one in every image that uses the palette, and palette plus images change in one undo step. Transparent itself cannot be removed. Decisions: - Pixels of a removed colour become transparent, like erasing (chosen by the user before the build). Findings: - The Filled checkbox slid away under the pointer. The brush button sat to its left and vanished when Filled turned on, so the second click missed. The brush button now sits to the right, and a check clicks the same spot twice. - One start-up FAIL "debug hook present" came back never in three later runs. It is the known start flake. Tests: - ./check.sh: 141 files, 2080 tests, exit 0 (before 140 / 2070) - 10 new unit tests in tests/editor/pixel-palette.test.ts - GM_STEPS=48 (fresh seed): ALL PASSED, 40 checks, 4 of them new - Counter-check: indices not shifting and brush always visible make 4 unit tests and exactly the 2 matching e2e checks fail - Check total 1282 (1278 + 4) in README and wiki, computed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
41772e53f7
commit
fe894c630d
10 files changed
+424
-55
No files matched your search
@@ -0,0 +1,142 @@
|
||||
/**
|
||||
* The pixel art side panel after #201: the options a tool shows, the palette across the whole
|
||||
* width, and taking out the picked colour of a palette while every picture painted with it keeps
|
||||
* its colours.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { CreateAssetCommand, SetAssetPropertyCommand } from '../../src/core/commands/assetCommands';
|
||||
import { EditPixelArtCommand } from '../../src/core/commands/pixelArtCommands';
|
||||
import type { PaletteAsset, PixelArtAsset } from '../../src/core/model/Asset';
|
||||
import { createEmptyProject } from '../../src/core/model/Project';
|
||||
import { celOf, PALETTE_KEY } from '../../src/core/model/pixelart';
|
||||
import type { CelPixels } from '../../src/core/model/pixelCel';
|
||||
import { withoutColorIndex } from '../../src/core/model/pixelDraw';
|
||||
import { withCels, withCelsReplaced } from '../../src/core/model/pixelEdit';
|
||||
import { paletteLayout } from '../../src/editor/pixelart/PaletteGrid';
|
||||
import { removePaletteColor } from '../../src/editor/pixelart/pixelActions';
|
||||
import { pixelToolOptions } from '../../src/editor/pixelart/toolOptions';
|
||||
import { EditorState } from '../../src/editor/state';
|
||||
import { pixelCels } from '../../src/viewport/PixelCels';
|
||||
|
||||
function cel(data: number[]): CelPixels {
|
||||
return { width: data.length, height: 1, data: Uint8Array.from(data) };
|
||||
}
|
||||
|
||||
describe('tool options', () => {
|
||||
it('shows each option only with the tools it changes', () => {
|
||||
expect(pixelToolOptions('pencil', false)).toEqual({ brush: true, filled: false, contiguous: false });
|
||||
expect(pixelToolOptions('eraser', false)).toEqual({ brush: true, filled: false, contiguous: false });
|
||||
expect(pixelToolOptions('line', true)).toEqual({ brush: true, filled: false, contiguous: false });
|
||||
expect(pixelToolOptions('fill', false)).toEqual({ brush: false, filled: false, contiguous: true });
|
||||
expect(pixelToolOptions('picker', false)).toEqual({ brush: false, filled: false, contiguous: false });
|
||||
});
|
||||
|
||||
it('drops the brush once a shape is filled', () => {
|
||||
for (const tool of ['rect', 'ellipse'] as const) {
|
||||
expect(pixelToolOptions(tool, false)).toEqual({ brush: true, filled: true, contiguous: false });
|
||||
expect(pixelToolOptions(tool, true)).toEqual({ brush: false, filled: true, contiguous: false });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('palette layout', () => {
|
||||
it('fills the width with as many swatches as fit, flush to the right', () => {
|
||||
// 260 px hold 11 swatches of 20 with 2 between; what is left widens them to 21.
|
||||
expect(paletteLayout(260, 17)).toEqual({ columns: 11, cell: 21, rows: 2 });
|
||||
expect(paletteLayout(176, 17)).toEqual({ columns: 8, cell: 20, rows: 3 });
|
||||
});
|
||||
|
||||
it('lays out eight to a row before the width is known, and one row at least', () => {
|
||||
expect(paletteLayout(0, 17)).toEqual({ columns: 8, cell: 20, rows: 3 });
|
||||
expect(paletteLayout(100, 1)).toEqual({ columns: 4, cell: 23, rows: 1 });
|
||||
expect(paletteLayout(10, 3)).toEqual({ columns: 1, cell: 20, rows: 3 });
|
||||
});
|
||||
});
|
||||
|
||||
describe('a colour out of the middle', () => {
|
||||
it('turns its pixels transparent and moves the later ones down', () => {
|
||||
expect([...withoutColorIndex(cel([0, 1, 2, 3, 4]), 2)!.data]).toEqual([0, 1, 0, 2, 3]);
|
||||
});
|
||||
|
||||
it('leaves a cel alone that uses no colour from there on', () => {
|
||||
expect(withoutColorIndex(cel([0, 1, 1]), 2)).toBeNull();
|
||||
});
|
||||
|
||||
it('swaps only the cels it is given', () => {
|
||||
const state = {
|
||||
width: 1,
|
||||
height: 1,
|
||||
layers: [
|
||||
{ id: 'a', name: 'A', opacity: 1 },
|
||||
{ id: 'b', name: 'B', opacity: 1 },
|
||||
],
|
||||
frames: [{ id: 'f', cels: { a: 'one.px', b: 'two.px' } }],
|
||||
};
|
||||
const next = withCelsReplaced(state, { 'one.px': 'three.px', 'two.px': '' });
|
||||
expect(next.frames[0]!.cels).toEqual({ a: 'three.px' });
|
||||
expect(withCelsReplaced(state, {}).frames[0]!.cels).toEqual({ a: 'one.px', b: 'two.px' });
|
||||
expect(state.frames[0]!.cels).toEqual({ a: 'one.px', b: 'two.px' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('removing the picked colour', () => {
|
||||
function setup() {
|
||||
const state = new EditorState();
|
||||
state.loadProject(createEmptyProject('p', 'P'), 1);
|
||||
const create = <T>(type: 'pixelart' | 'palette', name: string): T => {
|
||||
const command = new CreateAssetCommand(type, name);
|
||||
expect(state.execute(command)).toBe(true);
|
||||
return command.asset as T;
|
||||
};
|
||||
const palette = create<PaletteAsset>('palette', 'Colours');
|
||||
const paint = (name: string, pixels: number[]) => {
|
||||
const art = create<PixelArtAsset>('pixelart', name);
|
||||
expect(state.execute(new SetAssetPropertyCommand(art.id, PALETTE_KEY, palette.id))).toBe(true);
|
||||
const change = {
|
||||
frameId: art.frames[0]!.id,
|
||||
layerId: art.layers[0]!.id,
|
||||
file: pixelCels.put(cel(pixels)),
|
||||
};
|
||||
expect(state.execute(new EditPixelArtCommand(art.id, 'Paint', (s) => withCels(s, [change])))).toBe(
|
||||
true,
|
||||
);
|
||||
return art;
|
||||
};
|
||||
const pixelsOf = (art: PixelArtAsset) => {
|
||||
const file = celOf(art.frames[0]!, art.layers[0]!.id);
|
||||
return file ? [...pixelCels.read(file)!.data] : null;
|
||||
};
|
||||
return { state, palette, paint, pixelsOf };
|
||||
}
|
||||
|
||||
it('rewrites every picture of the palette so each keeps its colours, in one undo step', () => {
|
||||
const { state, palette, paint, pixelsOf } = setup();
|
||||
const colors = [...palette.colors];
|
||||
const first = paint('First', [1, 2, 3, 0]);
|
||||
const second = paint('Second', [3, 3, 1, 2]);
|
||||
expect(removePaletteColor(state, palette, 2)).toBe(true);
|
||||
expect(palette.colors).toEqual(colors.filter((_, i) => i !== 1));
|
||||
expect(pixelsOf(first)).toEqual([1, 0, 2, 0]);
|
||||
expect(pixelsOf(second)).toEqual([2, 2, 1, 0]);
|
||||
// Each remaining pixel shows the colour it showed before.
|
||||
expect(palette.colors[2 - 1]).toBe(colors[3 - 1]);
|
||||
|
||||
state.undo();
|
||||
expect(palette.colors).toEqual(colors);
|
||||
expect(pixelsOf(first)).toEqual([1, 2, 3, 0]);
|
||||
expect(pixelsOf(second)).toEqual([3, 3, 1, 2]);
|
||||
});
|
||||
|
||||
it('empties a cel that held only that colour', () => {
|
||||
const { state, palette, paint, pixelsOf } = setup();
|
||||
const art = paint('Only', [4, 4]);
|
||||
expect(removePaletteColor(state, palette, 4)).toBe(true);
|
||||
expect(pixelsOf(art)).toBeNull();
|
||||
});
|
||||
|
||||
it('refuses transparent and an index past the end', () => {
|
||||
const { state, palette } = setup();
|
||||
expect(removePaletteColor(state, palette, 0)).toBe(false);
|
||||
expect(removePaletteColor(state, palette, palette.colors.length + 1)).toBe(false);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user