Empty a deleted tile's cells on every tilemap (#268)
Deleting a tile left the cells painted with it in the tilemaps. They drew nothing, blocked nothing and did not show in the delete dialog, but still read as a tile for "Tile at" and stopped a fill. - DeleteAssetCommand empties the cells of a deleted tile on every layer of every tilemap in the same step. Undo puts the tile and its cells back together. - The delete dialog counts them, for example "3 cell(s) on 1 tilemap(s) are painted with it and become empty." (cellsPainted). - presentTile treats a cell naming a missing tile as empty. "Tile at" (World.tileAt) and floodFill use it, so cells already in existing files read and fill as empty. There is no migration. - The brush, the rectangle and the fill check the tile in hand. A tile deleted after it was picked no longer paints new invisible cells, the editor says "Choose a tile to paint with." instead. - Unchanged on purpose: the picker still reports such a cell as naming a tile that no longer exists, and erasing removes it. Tests: - ./check.sh: 195 files, 2823 tests, CHECK EXIT 0 (+6). - GM_STEPS=39,40: ALL PASSED, 100 checks (4 new in 40t). - Full e2e sum is now 1590 (1586 confirmed by CI on round 8, plus 4). - Counter-checks, all fail as expected: 5 in unit tests (delete without emptying, undo without cells, fill and "Tile at" without presentTile, wrong map count) and 3 in the browser (dialog without count, delete without emptying, brush without the check). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
14972d057f
commit
9b5eeaab57
11 files changed
+310
-38
No files matched your search
@@ -6,6 +6,7 @@ import type {
|
||||
SoundAsset,
|
||||
StateMachineAsset,
|
||||
TextureAsset,
|
||||
TilemapLayer,
|
||||
VariablesAsset,
|
||||
} from '../model/Asset';
|
||||
import type { AssetType } from '../model/AssetTypes';
|
||||
@@ -26,6 +27,7 @@ import {
|
||||
} from '../model/Scope';
|
||||
import type { SceneObject } from '../model/SceneObject';
|
||||
import { isTextureFile } from '../model/texture';
|
||||
import { cellsWithout } from '../model/tilemap';
|
||||
import { coerceValue, defaultsFor, initialOf, initialPropsFor } from '../properties/PropertySchema';
|
||||
import { type Removed, removeById, removeFirst, restoreAt } from '../util/list';
|
||||
import { type Command, CommandError } from './Command';
|
||||
@@ -209,19 +211,32 @@ export class CreateSoundCommand extends ImportFileCommand<SoundAsset> {
|
||||
|
||||
/**
|
||||
* Deletes an asset and, as a cascade, every scene object referencing it, together with the objects
|
||||
* parented below those – in container templates as well as in scenes.
|
||||
* parented below those – in container templates as well as in scenes. A tile takes the cells it is
|
||||
* painted on along (#268), which would otherwise stay in the file and draw nothing.
|
||||
*/
|
||||
export class DeleteAssetCommand implements Command {
|
||||
readonly label = 'Delete asset';
|
||||
private removed?: Removed<Asset>;
|
||||
private removedObjects: { holderId: string; removed: Removed<SceneObject>[] }[] = [];
|
||||
private emptied: { layer: TilemapLayer; cells: Record<string, string> }[] = [];
|
||||
private previousActiveScene = '';
|
||||
|
||||
constructor(private readonly assetId: string) {}
|
||||
|
||||
execute(project: Project): void {
|
||||
requireAsset(project, this.assetId);
|
||||
const asset = requireAsset(project, this.assetId);
|
||||
this.removedObjects = [];
|
||||
this.emptied = [];
|
||||
if (asset.type === 'tile')
|
||||
for (const map of project.assets) {
|
||||
if (map.type !== 'tilemap') continue;
|
||||
for (const layer of map.layers) {
|
||||
const kept = cellsWithout(layer.cells, this.assetId);
|
||||
if (!kept) continue;
|
||||
this.emptied.push({ layer, cells: layer.cells });
|
||||
layer.cells = kept;
|
||||
}
|
||||
}
|
||||
// The cascade runs while the asset is still in the list: a container asset is an object
|
||||
// holder itself, so removing it first would change what `objectHolders` reports.
|
||||
for (const holder of objectHolders(project)) {
|
||||
@@ -245,6 +260,7 @@ export class DeleteAssetCommand implements Command {
|
||||
const holder = holders.get(entry.holderId);
|
||||
if (holder) restoreAt(holder.objects, entry.removed);
|
||||
}
|
||||
for (const { layer, cells } of this.emptied) layer.cells = cells;
|
||||
project.activeSceneId = this.previousActiveScene;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -69,6 +69,21 @@ export function cellAt(layer: TilemapLayer, x: number, y: number): string {
|
||||
return layer.cells[cellKey(x, y)] ?? '';
|
||||
}
|
||||
|
||||
/**
|
||||
* The tile a cell holds as far as anything but the file is concerned: `''` for an empty cell and for
|
||||
* one naming a tile the project does not have (#268). Such a cell draws and blocks nothing, so it
|
||||
* reads and fills as empty too.
|
||||
*/
|
||||
export function presentTile(project: Project, tileId: string): string {
|
||||
return tileId !== '' && findAssetOf(project, tileId, 'tile') ? tileId : '';
|
||||
}
|
||||
|
||||
/** The cells without those naming `tileId`, or null when none does. */
|
||||
export function cellsWithout(cells: Record<string, string>, tileId: string): Record<string, string> | null {
|
||||
if (!Object.values(cells).includes(tileId)) return null;
|
||||
return Object.fromEntries(Object.entries(cells).filter(([, id]) => id !== tileId));
|
||||
}
|
||||
|
||||
export function isLayerVisible(layer: TilemapLayer): boolean {
|
||||
return layer.hidden !== true;
|
||||
}
|
||||
@@ -122,8 +137,8 @@ export interface TileDraw {
|
||||
|
||||
/**
|
||||
* Every cell of one layer that is inside the grid and names a tile that exists, in reading order
|
||||
* so the result is the same list twice. Cells naming a deleted tile are skipped rather than
|
||||
* dropped from the model: an undo brings the tile, and with it the drawing, back.
|
||||
* so the result is the same list twice. A cell naming a tile the project does not have is skipped
|
||||
* (see `presentTile`).
|
||||
*/
|
||||
export function layerTiles(project: Project, props: PropertyBag, layer: TilemapLayer): readonly TileDraw[] {
|
||||
const kinds = new Map<string, TileKind>();
|
||||
@@ -255,11 +270,24 @@ export function cellsInRect(props: PropertyBag, a: Cell, b: Cell): Cell[] {
|
||||
* The cells a fill from `start` changes (#133): every cell reached through its four edge
|
||||
* neighbours that holds what `start` holds – empty included – clipped to the grid. Diagonals do not
|
||||
* count, or a fill would leak through the gap between two corners. Filling with the tile already
|
||||
* there changes nothing, so that returns no cells.
|
||||
* there changes nothing, so that returns no cells. A cell naming a missing tile is empty here.
|
||||
*/
|
||||
export function floodFill(props: PropertyBag, layer: TilemapLayer, start: Cell, tileId: string): Cell[] {
|
||||
export function floodFill(
|
||||
project: Project,
|
||||
props: PropertyBag,
|
||||
layer: TilemapLayer,
|
||||
start: Cell,
|
||||
tileId: string,
|
||||
): Cell[] {
|
||||
if (!inGrid(props, start.x, start.y)) return [];
|
||||
const from = cellAt(layer, start.x, start.y);
|
||||
const present = new Map<string, string>();
|
||||
const tileAt = (x: number, y: number): string => {
|
||||
const id = cellAt(layer, x, y);
|
||||
let shown = present.get(id);
|
||||
if (shown === undefined) present.set(id, (shown = presentTile(project, id)));
|
||||
return shown;
|
||||
};
|
||||
const from = tileAt(start.x, start.y);
|
||||
if (from === tileId) return [];
|
||||
// The list is its own queue: everything in it is filled, and it is walked while it grows.
|
||||
const cells = [{ x: start.x, y: start.y }];
|
||||
@@ -275,7 +303,7 @@ export function floodFill(props: PropertyBag, layer: TilemapLayer, start: Cell,
|
||||
const x = cell.x + dx;
|
||||
const y = cell.y + dy;
|
||||
const key = cellKey(x, y);
|
||||
if (seen.has(key) || !inGrid(props, x, y) || cellAt(layer, x, y) !== from) continue;
|
||||
if (seen.has(key) || !inGrid(props, x, y) || tileAt(x, y) !== from) continue;
|
||||
seen.add(key);
|
||||
cells.push({ x, y });
|
||||
}
|
||||
|
||||
@@ -68,6 +68,22 @@ export function tilesUsing(project: Project, ids: readonly string[]): TileAsset[
|
||||
);
|
||||
}
|
||||
|
||||
/** The cells a tile is painted on, and on how many tilemaps – the cells are the maps' own data. */
|
||||
export function cellsPainted(project: Project, tileId: string): { tilemaps: number; cells: number } {
|
||||
let tilemaps = 0;
|
||||
let cells = 0;
|
||||
for (const asset of project.assets) {
|
||||
if (asset.type !== 'tilemap') continue;
|
||||
const here = asset.layers.reduce(
|
||||
(sum, layer) => sum + Object.values(layer.cells).filter((id) => id === tileId).length,
|
||||
0,
|
||||
);
|
||||
if (here > 0) tilemaps++;
|
||||
cells += here;
|
||||
}
|
||||
return { tilemaps, cells };
|
||||
}
|
||||
|
||||
/** The animations keying any of these ids – intrinsic data, which `findPropertyUsages` does not see. */
|
||||
export function animationsUsing(project: Project, ids: readonly string[]): AnimationAsset[] {
|
||||
const wanted = new Set(ids);
|
||||
|
||||
+21
-12
@@ -28,7 +28,13 @@ import { findAsset, findGraphAsset, type ObjectHolder, type Project } from '../c
|
||||
import type { SceneObject } from '../core/model/SceneObject';
|
||||
import { pixelArtRefIds } from '../core/model/pixelart';
|
||||
import { textureRefIds } from '../core/model/texture';
|
||||
import { animationsUsing, tilesUsing, findPropertyUsages, findUsages } from '../core/model/usages';
|
||||
import {
|
||||
animationsUsing,
|
||||
cellsPainted,
|
||||
tilesUsing,
|
||||
findPropertyUsages,
|
||||
findUsages,
|
||||
} from '../core/model/usages';
|
||||
import { Dialog } from '../ui';
|
||||
import { type EditorState, objectSelection } from './state';
|
||||
|
||||
@@ -189,20 +195,23 @@ export function confirmDeleteAsset(state: EditorState, assetId: string): void {
|
||||
const typed =
|
||||
asset.type === 'struct' ? structTypeUsages(state.project, assetId) : { places: 0, lists: 0, nodes: 0 };
|
||||
const lists = typed.lists > 0 ? `, ${typed.lists} list(s)` : '';
|
||||
const painted = asset.type === 'tile' ? cellsPainted(state.project, assetId) : { tilemaps: 0, cells: 0 };
|
||||
const detail =
|
||||
asset.type === 'scene'
|
||||
? `The scene and its ${asset.objects.length} object(s) are removed.`
|
||||
: eased.keys > 0 || eased.nodes > 0
|
||||
? `${eased.keys} animation key(s) and ${eased.nodes} Lerp node(s) use it and blend linearly instead.`
|
||||
: typed.places > 0 || typed.lists > 0 || typed.nodes > 0
|
||||
? `${typed.places} struct(s)${lists} and ${typed.nodes} node(s) use it and keep its fields as their own.`
|
||||
: references > 0 || animations > 0 || tiles > 0
|
||||
? wornDetail(references, animations, tiles, asset.type)
|
||||
: usages === 0
|
||||
? 'This cannot be undone with Ctrl+Z after reload.'
|
||||
: asset.type === 'container'
|
||||
? `${usages} placed container(s) are removed together with their content.`
|
||||
: `${usages} placed instance(s) are removed as well (scenes and container templates).`;
|
||||
: painted.cells > 0
|
||||
? `${painted.cells} cell(s) on ${painted.tilemaps} tilemap(s) are painted with it and become empty.`
|
||||
: eased.keys > 0 || eased.nodes > 0
|
||||
? `${eased.keys} animation key(s) and ${eased.nodes} Lerp node(s) use it and blend linearly instead.`
|
||||
: typed.places > 0 || typed.lists > 0 || typed.nodes > 0
|
||||
? `${typed.places} struct(s)${lists} and ${typed.nodes} node(s) use it and keep its fields as their own.`
|
||||
: references > 0 || animations > 0 || tiles > 0
|
||||
? wornDetail(references, animations, tiles, asset.type)
|
||||
: usages === 0
|
||||
? 'This cannot be undone with Ctrl+Z after reload.'
|
||||
: asset.type === 'container'
|
||||
? `${usages} placed container(s) are removed together with their content.`
|
||||
: `${usages} placed instance(s) are removed as well (scenes and container templates).`;
|
||||
new Dialog(`Delete "${asset.name}"?`, detail, [
|
||||
{ label: 'Cancel' },
|
||||
{ label: 'Delete', danger: true, onClick: () => state.execute(new DeleteAssetCommand(assetId)) },
|
||||
|
||||
@@ -14,6 +14,7 @@ import {
|
||||
findLayer,
|
||||
floodFill,
|
||||
inGrid,
|
||||
presentTile,
|
||||
tilemapColumns,
|
||||
tilemapRows,
|
||||
tilemapTileSize,
|
||||
@@ -140,12 +141,12 @@ export class TilemapPainter {
|
||||
/** Fills the connected cells of one tile in a single command, so one click is one undo step. */
|
||||
private fillFrom(target: PaintTarget, layer: TilemapLayer, at: Vec2): void {
|
||||
const state = this.host.state;
|
||||
const tileId = state.tilemapPaint.tileId;
|
||||
const tileId = presentTile(state.project, state.tilemapPaint.tileId);
|
||||
if (!tileId) {
|
||||
state.setNotice('Choose a tile to paint with.');
|
||||
return;
|
||||
}
|
||||
const cells = floodFill(target.item.props, layer, at, tileId);
|
||||
const cells = floodFill(state.project, target.item.props, layer, at, tileId);
|
||||
const command = paintTiles(
|
||||
layer,
|
||||
target.asset.id,
|
||||
@@ -157,7 +158,9 @@ export class TilemapPainter {
|
||||
|
||||
private startStroke(target: PaintTarget, global: Vec2): boolean {
|
||||
const paint = this.host.state.tilemapPaint;
|
||||
const tileId = paint.tool === 'erase' ? '' : paint.tileId;
|
||||
// The tile in hand may have been deleted since it was picked; painting it would leave cells that
|
||||
// draw nothing (#268).
|
||||
const tileId = paint.tool === 'erase' ? '' : presentTile(this.host.state.project, paint.tileId);
|
||||
if (paint.tool !== 'erase' && !tileId) {
|
||||
this.host.state.setNotice('Choose a tile to paint with.');
|
||||
return false;
|
||||
|
||||
@@ -8,7 +8,7 @@ import { bakeInstances, materializeInstance, objectRef, resolveObjects } from '.
|
||||
import { groupNamed, treeOrder } from '../core/model/hierarchy';
|
||||
import { liveTileAt, withTile } from '../core/model/liveTilemap';
|
||||
import { referenceCandidates } from '../core/model/objectRefs';
|
||||
import type { Cell } from '../core/model/tilemap';
|
||||
import { type Cell, presentTile } from '../core/model/tilemap';
|
||||
import { descendantsBelow } from '../core/model/walk';
|
||||
import { findAsset, findAssetOf, findScene, type Project } from '../core/model/Project';
|
||||
import { createSceneObject, objectName, type SceneObject } from '../core/model/SceneObject';
|
||||
@@ -189,10 +189,14 @@ export class World {
|
||||
return this.tilemapAssetOf(entity) !== undefined;
|
||||
}
|
||||
|
||||
/** The tile in a cell of a placed tilemap as the run left it (see `model/liveTilemap.ts`). */
|
||||
/**
|
||||
* The tile in a cell of a placed tilemap as the run left it (see `model/liveTilemap.ts`), `''` for
|
||||
* a cell naming a tile the project does not have, as it is drawn.
|
||||
*/
|
||||
tileAt(entity: Entity, layer: string, cell: Cell): string | null {
|
||||
const asset = this.tilemapAssetOf(entity);
|
||||
return asset ? liveTileAt(asset, entity.tileEdits, entity.props, layer, cell) : null;
|
||||
const tile = asset ? liveTileAt(asset, entity.tileEdits, entity.props, layer, cell) : null;
|
||||
return tile === null ? null : presentTile(this.project, tile);
|
||||
}
|
||||
|
||||
/** Sets a cell of this one placement for the rest of the run; `''` empties it. */
|
||||
|
||||
Reference in new issue
Block a user