Painting over identical tiles no longer adds an empty undo step (#212)

A rectangle drawn over cells that already hold exactly that tile
pushed a command that changed nothing. Undo then seemed to do nothing
and the project was marked as modified. PaintTilesCommand.changes()
was meant to prevent that, but nothing called it since #128.

- New paintTiles() in tilemapCommands.ts returns the command for a
  finished stroke or fill, or null when no cell changes. It is the
  first caller of PaintTilesCommand.changes().
- All three paint paths in the Scene View go through it: brush and
  eraser, rectangle (both in endStroke) and flood fill (fillFrom).
- Flood fill was not affected, because floodFill already returns []
  for the same tile. The guard is there anyway, as the issue asked.
- A rectangle that changes even one cell stays a normal undo step.

Tests:
- ./check.sh: 145 files, 2190 tests, CHECK EXIT 0 (before 2187).
- 3 new unit tests in tests/core/tilemap.test.ts: same rectangle
  twice gives no command, a wider rectangle still does, a fill that
  finds nothing gives no command.
- 1 new e2e check in 40d: the same rectangle again, then one undo
  must take 9 cells to 0. A redo restores them for later blocks.
- GM_STEPS=39,40 on the final build: ALL PASSED, 81 checks.
- Counter-probe with changes(...) || true: exactly the two unit
  tests fail, and in the browser exactly the new check fails with
  {"afterUndo":9}. Original restored via cmp (IDENTICAL), rebuilt.
- Editor suite check total: CI measured 1312 at 0e85856, new total
  1313 (calculated) in README and wiki.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

README.md

@@ -43,7 +43,7 @@ npm run build        # writes dist/ and regenerates index.html; the folder is wh
 The browser suite and the three example games need a machine with a GPU:
 
 ```bash
-node tools/seed-e2e.mjs && node tools/e2e.mjs      # the editor, 1312 checks (GM_STEPS=20 for one step)
+node tools/seed-e2e.mjs && node tools/e2e.mjs      # the editor, 1313 checks (GM_STEPS=20 for one step)
                                                    #   every tool signs in first – see the Accounts page
 node tools/seed-jumprun.mjs && node tools/smoke-jumprun.mjs
 node tools/seed-tetris.mjs && node tools/smoke-tetris.mjs

src/core/commands/tilemapCommands.ts

@@ -82,6 +82,21 @@ export class PaintTilesCommand implements Command {
   }
 }
 
+/**
+ * The command for a finished stroke or fill, or null when it would change no cell: such a step
+ * would undo nothing and still count as an edit of the project (#212).
+ */
+export function paintTiles(
+  layer: TilemapLayer,
+  assetId: string,
+  edits: readonly CellEdit[],
+  label?: string,
+): PaintTilesCommand | null {
+  return PaintTilesCommand.changes(layer, edits)
+    ? new PaintTilesCommand(assetId, layer.id, edits, label)
+    : null;
+}
+
 /**
  * Where a placed tilemap's origin goes when `edge` gains `count` cells, starting from `from`, so the
  * painted cells stay where they are in the scene (#136). The shift is worked out in the tilemap's

src/editor/viewport/SceneViewport.ts

@@ -10,7 +10,7 @@ import {
   PlaceAssetCommand,
   SetOverrideCommand,
 } from '../../core/commands/sceneObjectCommands';
-import { type CellEdit, PaintTilesCommand } from '../../core/commands/tilemapCommands';
+import { type CellEdit, paintTiles } from '../../core/commands/tilemapCommands';
 import type { TilemapAsset, TilemapLayer } from '../../core/model/Asset';
 import { hasSceneIcon } from '../../core/model/AssetTypes';
 import { findObject, subtreeRoots } from '../../core/model/hierarchy';
@@ -503,15 +503,13 @@ export class SceneViewport extends FreeViewport {
       return;
     }
     const cells = floodFill(target.item.props, layer, at, tileId);
-    if (cells.length > 0)
-      this.state.execute(
-        new PaintTilesCommand(
-          target.asset.id,
-          target.layerId,
-          cells.map((cell) => ({ ...cell, tileId })),
-          'Fill tiles',
-        ),
-      );
+    const command = paintTiles(
+      layer,
+      target.asset.id,
+      cells.map((cell) => ({ ...cell, tileId })),
+      'Fill tiles',
+    );
+    if (command) this.state.execute(command);
   }
 
   private startStroke(
@@ -588,15 +586,10 @@ export class SceneViewport extends FreeViewport {
             tileId: stroke.tileId,
           }))
         : stroke.edits;
-    if (edits.length > 0)
-      this.state.execute(
-        new PaintTilesCommand(
-          stroke.assetId,
-          stroke.layerId,
-          edits,
-          stroke.tool === 'erase' ? 'Erase tiles' : 'Paint tiles',
-        ),
-      );
+    const command =
+      layer &&
+      paintTiles(layer, stroke.assetId, edits, stroke.tool === 'erase' ? 'Erase tiles' : 'Paint tiles');
+    if (command) this.state.execute(command);
     else this.state.preview();
     this.redraw();
   }

tests/core/tilemap.test.ts

@@ -10,6 +10,7 @@ import {
   DuplicateTilemapLayerCommand,
   MoveTilemapLayerCommand,
   PaintTilesCommand,
+  paintTiles,
   RemoveTilemapLayerCommand,
   RenameTilemapLayerCommand,
   SetTilemapLayerHiddenCommand,
@@ -26,6 +27,7 @@ import {
   cellOrigin,
   cellsAlong,
   cellsInRect,
+  floodFill,
   inGrid,
   layerTiles,
   neighboursOf,
@@ -151,6 +153,41 @@ describe('painting a tilemap', () => {
     expect(PaintTilesCommand.changes(layer, [{ x: 5, y: 5, tileId: f.tileId }])).toBe(true);
   });
 
+  it('makes no undo step for a rectangle over cells that already hold its tile', () => {
+    const f = makeTilemap();
+    const box = cellsInRect(f.assetOf().props, { x: 0, y: 0 }, { x: 2, y: 2 });
+    f.paint(box);
+    const again = box.map((at) => ({ ...at, tileId: f.tileId }));
+    expect(paintTiles(f.assetOf().layers[0]!, f.tilemapId, again)).toBeNull();
+    // The one undo left takes back the first rectangle, not an empty second one.
+    f.stack.undo();
+    expect(cellsOf(f.project, f.tilemapId)).toEqual({});
+  });
+
+  it('still makes one when a single cell of the rectangle changes', () => {
+    const f = makeTilemap();
+    f.paint(cellsInRect(f.assetOf().props, { x: 0, y: 0 }, { x: 1, y: 1 }));
+    const wider = cellsInRect(f.assetOf().props, { x: 0, y: 0 }, { x: 2, y: 1 });
+    const command = paintTiles(
+      f.assetOf().layers[0]!,
+      f.tilemapId,
+      wider.map((at) => ({ ...at, tileId: f.tileId })),
+    );
+    expect(command).not.toBeNull();
+    f.stack.execute(command!);
+    expect(Object.keys(cellsOf(f.project, f.tilemapId))).toHaveLength(6);
+    f.stack.undo();
+    expect(Object.keys(cellsOf(f.project, f.tilemapId)).sort()).toEqual(['0,0', '0,1', '1,0', '1,1']);
+  });
+
+  it('makes no undo step for a fill that finds nothing to fill', () => {
+    const f = makeTilemap();
+    f.paint([{ x: 0, y: 0 }]);
+    const layer = f.assetOf().layers[0]!;
+    expect(floodFill(f.assetOf().props, layer, { x: 0, y: 0 }, f.tileId)).toEqual([]);
+    expect(paintTiles(layer, f.tilemapId, [])).toBeNull();
+  });
+
   it('covers the cells between two pointer events', () => {
     expect(cellsAlong({ x: 0, y: 0 }, { x: 3, y: 0 })).toEqual([
       { x: 1, y: 0 },

tools/e2e/40-tilemap.mjs

@@ -117,6 +117,7 @@ export async function tilemap(t) {
     shot,
     settle,
     undo,
+    redo,
     select,
   } = t;
   const { saved, api, navigate, projectId } = t;
@@ -287,6 +288,20 @@ export async function tilemap(t) {
     Object.keys(cells).length === 9 && cells['1,1'] === 'at-grass' && cells['2,2'] === 'at-grass',
     JSON.stringify(Object.keys(cells).sort()),
   );
+  // The same box again changes no cell, so it leaves no step behind (#212): the one undo takes
+  // back the first rectangle, and redo brings it back for the blocks below.
+  await drag(corner.x, corner.y, far.x, far.y);
+  await settle();
+  await undo();
+  await settle();
+  const afterUndo = Object.keys(await painted()).length;
+  await redo();
+  await settle();
+  check(
+    'a rectangle over cells that already hold its tile is no undo step',
+    afterUndo === 0 && Object.keys(await painted()).length === 9,
+    JSON.stringify({ afterUndo, redone: Object.keys(await painted()).length }),
+  );
 
   // 40e. The pixels. While the animation runs the block under the pointer changes between the two
   //      steps, so it is cut down to one for the reading and put back afterwards (as step 39 does).