From 3abb6894a7b1c3d3a9394ba575893fb23398b4d4 Mon Sep 17 00:00:00 2001 From: MelekTaus Date: Sun, 27 Sep 2026 12:12:07 +0200 Subject: [PATCH] Let a container sort its content with its parent (#243) A container is not drawn, so it had no place in the draw order. The chain from its content to the parent of the container broke there, and a container under a character could not be drawn together with it. - A container gets the switch Sort with parent under Z. It defaults to off, also for new containers, so projects draw as they did. - With the switch on, the whole content is drawn in the slot of the container's parent. The content's own switch does not count there. - Containers in a row pass the content on as long as each has the switch on. On the top level or under a tilemap it has no effect. - drawSlots follows the new sortsWith field of a draw item. Scene view, game and picking all use drawSlots, so all three agree. Found on the way: a scene object hung directly under a container spells the container out, and its template content is dropped. That is the existing spelledOut rule, the tests now keep both cases apart. Tests: - ./check.sh: 178 files, 2573 tests, CHECK EXIT 0 - 7 new unit tests in sort-with-parent.test.ts: the switch, template content, override per container, a chain, top level and tilemap, game and picking - e2e 5b-iii: a real click on the switch in the inspector, with undo - 4 counter-probes over #242 and #243 bite (2 unit, 2 browser) Co-Authored-By: Claude Opus 5.5 --- src/core/properties/PropertySchema.ts | 17 ++++- src/viewport/drawOrder.ts | 16 ++-- src/viewport/items.ts | 36 +++++++-- tests/viewport/sort-with-parent.test.ts | 98 ++++++++++++++++++++++++- tools/e2e/05-drawing.mjs | 36 +++++++++ 5 files changed, 188 insertions(+), 15 deletions(-) diff --git a/src/core/properties/PropertySchema.ts b/src/core/properties/PropertySchema.ts index e0f6b3f8..dfebca0d 100644 --- a/src/core/properties/PropertySchema.ts +++ b/src/core/properties/PropertySchema.ts @@ -274,13 +274,26 @@ export const SORT_PROPS: PropertySchema = [ { key: SORT_WITH_PARENT_KEY, label: 'Sort with parent', kind: 'boolean', default: false, initial: true }, ]; +/** + * A container is not drawn, but with the switch on it passes its content on to its own parent + * (#243). It starts off, so content placed before draws as it did. + */ +const CONTAINER_SORT_PROPS: PropertySchema = [ + { key: SORT_WITH_PARENT_KEY, label: 'Sort with parent', kind: 'boolean', default: false }, +]; + /** A tilemap has no single place in the draw order, and the other types are not drawn at all. */ -const SORT_TYPES: ReadonlySet = new Set(['sprite', 'text', 'particles']); +const SORT_TYPES: ReadonlySet = new Set(['sprite', 'text', 'particles', 'container']); export function hasSortProps(type: AssetType): boolean { return SORT_TYPES.has(type); } +function sortPropsOf(type: AssetType): PropertySchema { + if (!hasSortProps(type)) return []; + return type === 'container' ? CONTAINER_SORT_PROPS : SORT_PROPS; +} + export function isSortProp(key: string): boolean { return key === SORT_WITH_PARENT_KEY; } @@ -640,7 +653,7 @@ const FULL_SCHEMAS: Record = Object.fromEntries( ? [ ...COMMON_PROPS, ...(hasTransformProps(type) ? TRANSFORM_PROPS : []), - ...(hasSortProps(type) ? SORT_PROPS : []), + ...sortPropsOf(type), ...ASSET_SCHEMAS[type], ] : ASSET_SCHEMAS[type], diff --git a/src/viewport/drawOrder.ts b/src/viewport/drawOrder.ts index e3fb2af4..e63118b3 100644 --- a/src/viewport/drawOrder.ts +++ b/src/viewport/drawOrder.ts @@ -55,7 +55,8 @@ interface Candidate extends DrawSlot { * stay behind them, in that order, since neither has a single place to stand. * * A child that sorts with its parent takes no part in any of this: it is drawn in its parent's - * place, and its `z` only orders it against the parent and their siblings (#232). + * place, and its `z` only orders it against the parent and their siblings (#232). The content of a + * container that sorts with its parent is such a child of the container's parent (#243). */ export function drawSlots(items: readonly DrawItem[]): DrawSlot[] { const groups = parentGroups(items); @@ -103,11 +104,12 @@ export function drawSlots(items: readonly DrawItem[]): DrawSlot[] { */ function parentGroups(items: readonly DrawItem[]): Map { const drawn = new Map(items.filter((item) => !isSymbolType(item.type)).map((item) => [item.id, item])); + const parentOf = (item: DrawItem) => drawn.get(item.sortsWith || item.parentId); const leaderOf = (item: DrawItem): DrawItem => { let leader = item; - for (let parent = drawn.get(leader.parentId); parent && joinsParent(leader, parent);) { + for (let parent = parentOf(leader); parent && joinsParent(leader, parent);) { leader = parent; - parent = drawn.get(leader.parentId); + parent = parentOf(leader); } return leader; }; @@ -122,9 +124,13 @@ function parentGroups(items: readonly DrawItem[]): Map { return groups; } -/** A map has no single place to share, neither as the child nor as the parent. */ +/** + * A map has no single place to share, neither as the child nor as the parent. Content a container + * passes on joins by the container's switch, its own has no say (#243). + */ function joinsParent(child: DrawItem, parent: DrawItem): boolean { - return child.props[SORT_WITH_PARENT_KEY] === true && child.type !== 'tilemap' && parent.type !== 'tilemap'; + const joins = child.sortsWith !== '' || child.props[SORT_WITH_PARENT_KEY] === true; + return joins && child.type !== 'tilemap' && parent.type !== 'tilemap'; } /** A row of a layer sorted by Y, or an object other than a map. */ diff --git a/src/viewport/items.ts b/src/viewport/items.ts index f5506920..6fd272b9 100644 --- a/src/viewport/items.ts +++ b/src/viewport/items.ts @@ -29,6 +29,7 @@ import { liveTilemap, type TileEdits } from '../core/model/liveTilemap'; import { colliderFrame } from '../core/properties/collider'; import { enabledAlongChain } from '../core/properties/enabled'; import { fontOf } from '../core/properties/fonts'; +import { SORT_WITH_PARENT_KEY } from '../core/properties/PropertySchema'; import { resolveSceneObjectProps } from '../core/properties/resolve'; import { editorTransform } from '../core/properties/transform'; import type { World } from '../runtime/World'; @@ -47,6 +48,11 @@ export interface DrawItem { props: PropertyBag; /** Id of the parent item, empty when top-level. */ parentId: string; + /** + * The object a container between this one and it passes this one on to (#243): the container's + * switch "Sort with parent" is on, so this one is drawn in that object's place. Empty when none. + */ + sortsWith: string; /** Effective switch: this object and all its parents are on (see `properties/enabled.ts`). */ enabled: boolean; /** Resolved here because drawing has no access to the project. */ @@ -202,6 +208,7 @@ export function collectHolderItems( m, props, parentId: object.parentId, + sortsWith: '', enabled: true, texture: wearsPicture(asset.type) ? resolveTexture(project, props) : null, font: asset.type === 'text' ? fontFileOf(project, fontOf(props)) : '', @@ -215,12 +222,11 @@ export function collectHolderItems( }); } const byId = new Map(items.map((i) => [i.id, i])); - for (const item of items) - item.enabled = enabledAlongChain( - item, - (i) => i.props, - (i) => byId.get(i.parentId) ?? null, - ); + const parentOf = (i: DrawItem) => byId.get(i.parentId) ?? null; + for (const item of items) { + item.enabled = enabledAlongChain(item, (i) => i.props, parentOf); + item.sortsWith = passedOnTo(parentOf(item), parentOf)?.id ?? ''; + } return { items, background: Number(holder.props.backgroundColor ?? fallbackBackground) }; } @@ -229,6 +235,23 @@ function liveEdits(run: World | null, holder: ObjectHolder, objectId: string): T return run?.sceneId === holder.id ? run.findByObjectId(objectId)?.tileEdits : undefined; } +/** + * Where a chain of containers with "Sort with parent" on hands their content (#243): the first + * object above them, or null when no such container is in the way – or nothing is above it. + */ +function passedOnTo( + parent: T | null, + up: (at: T) => T | null, +): T | null { + let at = parent; + let passed = false; + while (at?.type === 'container' && at.props[SORT_WITH_PARENT_KEY] === true) { + passed = true; + at = up(at); + } + return passed ? at : null; +} + /** The running world: scene content only, since symbols are editor-only. */ export function collectWorldItems(world: World): DrawList { const items = world.entities @@ -247,6 +270,7 @@ export function collectWorldItems(world: World): DrawList { m, props: e.props, parentId: e.parent?.objectId ?? '', + sortsWith: passedOnTo(e.parent ?? null, (p) => p.parent ?? null)?.objectId ?? '', enabled: true, // Resolved per frame on purpose: a `Properties` node or an animation may swap `texture`. texture: wearsPicture(e.type) ? resolveTexture(world.project, e.props) : null, diff --git a/tests/viewport/sort-with-parent.test.ts b/tests/viewport/sort-with-parent.test.ts index aa079ab0..bf9c3885 100644 --- a/tests/viewport/sort-with-parent.test.ts +++ b/tests/viewport/sort-with-parent.test.ts @@ -4,13 +4,14 @@ import { ResetAssetPropertyCommand, SetAssetPropertyCommand, } from '../../src/core/commands/assetCommands'; -import { SetOverrideCommand } from '../../src/core/commands/sceneObjectCommands'; import { PaintTilesCommand, RenameTilemapLayerCommand, SetTilemapLayerSortByYCommand, } from '../../src/core/commands/tilemapCommands'; +import { PlaceAssetCommand, SetOverrideCommand } from '../../src/core/commands/sceneObjectCommands'; import type { TilemapAsset } from '../../src/core/model/Asset'; +import { instanceChildId } from '../../src/core/model/instance'; import type { AssetType } from '../../src/core/model/AssetTypes'; import { findAsset } from '../../src/core/model/Project'; import { schemaFor, SORT_WITH_PARENT_KEY } from '../../src/core/properties/PropertySchema'; @@ -55,7 +56,7 @@ describe('a child sorted with its parent (#232)', () => { type, ).toBe(true); } - for (const type of ['tilemap', 'container', 'camera', 'collider', 'behavior'] as const) + for (const type of ['tilemap', 'camera', 'collider', 'behavior'] as const) expect( schemaFor(type).some((d) => d.key === SORT_WITH_PARENT_KEY), type, @@ -204,3 +205,96 @@ describe('a child sorted with its parent (#232)', () => { expect(stackAt(items, hits).map((id) => s.names.get(id))).toEqual(['box', 'weapon', 'other']); }); }); + +describe('a container sorted with its parent (#243)', () => { + /** + * "other" and "box" on 0, a container "group" below the box, and in the container's template a + * "weapon" on -1. + */ + function withContainer() { + const s = makeScene(); + const groupAsset = s.asset('container', 'Group'); + const inTemplate = new PlaceAssetCommand(groupAsset, s.weaponId, { x: 0, y: 0, z: -1 }, ''); + s.f.stack.execute(inTemplate); + s.put('other', s.f.spriteId, { x: 300, y: 0, z: 0 }); + const box = s.put('box', s.f.spriteId, { x: 0, y: 0, z: 0 }); + const group = s.put('group', groupAsset, { x: 0, y: 0, z: 0 }, box); + s.names.set(instanceChildId(group, inTemplate.objectId), 'weapon'); + const sortGroup = (on: boolean) => + s.f.stack.execute(new SetAssetPropertyCommand(groupAsset, SORT_WITH_PARENT_KEY, on)); + return { ...s, groupAsset, group, box, sortGroup }; + } + + it('has the switch, off for a new container', () => { + const s = makeScene(); + const id = s.asset('container', 'Group'); + expect(schemaFor('container').some((d) => d.key === SORT_WITH_PARENT_KEY)).toBe(true); + expect(resolveAssetProps(s.f.project, findAsset(s.f.project, id)!)[SORT_WITH_PARENT_KEY]).toBe(false); + }); + + it('draws its content in the place of its parent once switched on', () => { + const s = withContainer(); + expect(s.order()).toEqual(['weapon', 'other', 'box']); + s.sortGroup(true); + expect(s.order()).toEqual(['other', 'weapon', 'box']); + s.f.stack.undo(); + expect(s.order()).toEqual(['weapon', 'other', 'box']); + }); + + it('passes content on whatever the content says itself', () => { + const s = withContainer(); + s.sortWithParent(s.weaponId, false); + s.sortGroup(true); + expect(s.order()).toEqual(['other', 'weapon', 'box']); + }); + + it('can be switched per placed container', () => { + const s = withContainer(); + s.f.stack.execute(new SetOverrideCommand(s.f.sceneId, s.group, SORT_WITH_PARENT_KEY, true)); + expect(s.order()).toEqual(['other', 'weapon', 'box']); + }); + + it('passes through unpacked containers in a row, and stops at one that is off', () => { + const s = makeScene(); + const outerAsset = s.asset('container', 'Outer'); + const innerAsset = s.asset('container', 'Inner'); + s.put('other', s.f.spriteId, { x: 300, y: 0, z: 0 }); + const box = s.put('box', s.f.spriteId, { x: 0, y: 0, z: 0 }); + const outer = s.put('outer', outerAsset, { x: 0, y: 0, z: 0 }, box); + const inner = s.put('inner', innerAsset, { x: 0, y: 0, z: 0 }, outer); + s.put('weapon', s.weaponId, { x: 0, y: 0, z: -1 }, inner); + s.f.stack.execute(new SetAssetPropertyCommand(innerAsset, SORT_WITH_PARENT_KEY, true)); + expect(s.order()).toEqual(['weapon', 'other', 'box']); + s.f.stack.execute(new SetAssetPropertyCommand(outerAsset, SORT_WITH_PARENT_KEY, true)); + expect(s.order()).toEqual(['other', 'weapon', 'box']); + }); + + it('draws alone at the top level, and not with a tilemap', () => { + const s = makeScene(); + const groupAsset = s.asset('container', 'Group'); + const map = s.put('map', s.asset('tilemap', 'Map'), { x: 0, y: 0, z: 0 }); + const top = s.put('top', groupAsset, { x: 0, y: 0, z: 0 }); + const underMap = s.put('underMap', groupAsset, { x: 0, y: 0, z: 0 }, map); + s.put('a', s.weaponId, { x: 0, y: 0, z: 1 }, top); + s.put('b', s.weaponId, { x: 0, y: 0, z: -1 }, underMap); + s.f.stack.execute(new SetAssetPropertyCommand(groupAsset, SORT_WITH_PARENT_KEY, true)); + expect(s.order()).toEqual(['b', 'map', 'map', 'a']); + }); + + it('holds in the running game and when picking', () => { + const s = withContainer(); + s.sortGroup(true); + const { world } = startWorld(s.f); + expect(drawSlots(collectWorldItems(world).items).map((slot) => s.names.get(slot.id))).toEqual([ + 'other', + 'weapon', + 'box', + ]); + const items = collectHolderItems(s.f.project, s.f.sceneOf(), 0).items; + const hits = ['other', 'box', 'weapon'].map((name) => ({ + id: [...s.names].find(([, n]) => n === name)![0], + layers: null, + })); + expect(stackAt(items, hits).map((id) => s.names.get(id))).toEqual(['box', 'weapon', 'other']); + }); +}); diff --git a/tools/e2e/05-drawing.mjs b/tools/e2e/05-drawing.mjs index 840f657c..95894a95 100644 --- a/tools/e2e/05-drawing.mjs +++ b/tools/e2e/05-drawing.mjs @@ -85,6 +85,42 @@ export async function drawing(t) { objs.splice(objs.findIndex(o => o.id === 'o-sort'), 1); __gm.state.select(null)`); + // 5b-iii. A container sorted with its parent (#243): a container under o1 holds a crate on -1 in + // its template, a third crate stands before o1. Off, as a new container starts, the crate + // inside drops behind everything on 0; the container's own switch draws it right behind o1. + const INSIDE = 'o-cont~sort-in'; + await mutate(` + const p = __gm.state.project; + const objs = __gm.state.activeScene.objects; + const make = (id, assetId, parentId, z) => ({ id, assetId, name: '', position: { x: 0, y: 0, z }, editorOffset: { x: 0, y: 0 }, overrides: {}, parentId, instanceEdits: {} }); + p.assets.push({ id: 'sort-box', type: 'container', name: 'sort-box', props: { enabled: true }, objects: [make('sort-in', 'crate', '', -1)] }); + objs.splice(objs.findIndex(o => o.id === 'o1'), 0, make('o-sort', 'crate', '', 0)); + objs.push(make('o-cont', 'sort-box', 'o1', 0));`); + const boxOrder = () => + json(`__gm.drawOrder().filter(id => ['o-sort', 'o1', ${JSON.stringify(INSIDE)}].includes(id))`); + const boxOff = await boxOrder(); + await select({ kind: 'object', holderId: 'scene1', objectId: 'o-cont' }); + const boxRow = await json("__gm.inspectorField('Sort with parent')"); + if (boxRow) await click(boxRow.x, boxRow.y); + const boxOverride = (await object('o-cont')).overrides.sortWithParent; + const boxOn = await boxOrder(); + await undo(); + const boxUndone = await boxOrder(); + check( + 'Sort with parent on a container draws its content right behind the parent of the container, and undo takes it back', + JSON.stringify(boxOff) === `["${INSIDE}","o-sort","o1"]` && + boxOverride === true && + JSON.stringify(boxOn) === `["o-sort","${INSIDE}","o1"]` && + JSON.stringify(boxUndone) === JSON.stringify(boxOff), + JSON.stringify({ boxRow, boxOff, boxOverride, boxOn, boxUndone }), + ); + await mutate(` + const p = __gm.state.project; + const scene = __gm.state.activeScene; + scene.objects = scene.objects.filter(o => o.id !== 'o-sort' && o.id !== 'o-cont'); + p.assets = p.assets.filter(a => a.id !== 'sort-box'); + __gm.state.select(null)`); + // 5c. The symbols are the asset panel's icons, white in a dark outline and without a plate // behind them: one of each in a lane of its own, the pixels read on a grid over each icon // (±12 world units round its centre). Each must show white and the outline, and none of the