diff --git a/packages/core/src/store/actions/node-actions.ts b/packages/core/src/store/actions/node-actions.ts index a855871d..d9e50b79 100644 --- a/packages/core/src/store/actions/node-actions.ts +++ b/packages/core/src/store/actions/node-actions.ts @@ -238,27 +238,29 @@ export const createNodesAction = ( const nextRootIds = [...state.rootNodeIds] for (const { node, parentId } of ops) { + const effectiveParentId = parentId ?? (node.parentId as AnyNodeId | null) ?? null + // 1. Assign parentId to the child (Safe because BaseNode has parentId) const newNode = { ...node, - parentId: parentId ?? null, + parentId: effectiveParentId, } nextNodes[newNode.id] = newNode // 2. Update the Parent's children list - if (parentId && nextNodes[parentId]) { - const parent = nextNodes[parentId] + if (effectiveParentId && nextNodes[effectiveParentId]) { + const parent = nextNodes[effectiveParentId] // Type Guard: Check if the parent node is a container that supports children if ('children' in parent && Array.isArray(parent.children)) { - nextNodes[parentId] = { + nextNodes[effectiveParentId] = { ...parent, // Use Set to prevent duplicate IDs if createNode is called twice children: Array.from(new Set([...parent.children, newNode.id])) as any, // We don't verify child types here } } - } else if (!parentId) { + } else if (!effectiveParentId) { // 3. Handle Root nodes if (!nextRootIds.includes(newNode.id)) { nextRootIds.push(newNode.id) diff --git a/packages/core/src/store/use-scene.ts b/packages/core/src/store/use-scene.ts index d4aa2c45..51e1caa9 100644 --- a/packages/core/src/store/use-scene.ts +++ b/packages/core/src/store/use-scene.ts @@ -11,8 +11,8 @@ import { SiteNode } from '../schema/nodes/site' import { StairNode as StairNodeSchema } from '../schema/nodes/stair' import { StairSegmentNode as StairSegmentNodeSchema } from '../schema/nodes/stair-segment' import type { AnyNode, AnyNodeId } from '../schema/types' -import { resetSceneHistoryPauseDepth } from './history-control' import * as nodeActions from './actions/node-actions' +import { resetSceneHistoryPauseDepth } from './history-control' function getFiniteNumber(value: unknown, fallback: number) { return typeof value === 'number' && Number.isFinite(value) ? value : fallback @@ -349,6 +349,67 @@ function migrateNodes(nodes: Record): Record { return patchedNodes as Record } +function getNodeChildIds(node: AnyNode): AnyNodeId[] { + if (!('children' in node) || !Array.isArray(node.children)) { + return [] + } + + return (node.children as unknown[]) + .map((child) => { + if (typeof child === 'string') return child + if (child && typeof child === 'object' && 'id' in child && typeof child.id === 'string') { + return child.id + } + return null + }) + .filter((id): id is AnyNodeId => typeof id === 'string') +} + +function normalizeRootNodeIds( + nodes: Record, + rootNodeIds: AnyNodeId[], +): AnyNodeId[] { + const existingRootIds = rootNodeIds.filter((id) => Boolean(nodes[id])) + const siteRootIds = existingRootIds.filter((id) => nodes[id]?.type === 'site') + + if (siteRootIds.length > 0) { + return siteRootIds + } + + return existingRootIds.filter((id) => nodes[id]?.parentId === null) +} + +function collectReachableNodeIds( + nodes: Record, + rootNodeIds: AnyNodeId[], +): Set { + const reachable = new Set() + const stack = [...rootNodeIds] + const childIdsByParentId = new Map() + + for (const node of Object.values(nodes)) { + if (!node.parentId) continue + const parentId = node.parentId as AnyNodeId + const children = childIdsByParentId.get(parentId) ?? [] + children.push(node.id as AnyNodeId) + childIdsByParentId.set(parentId, children) + } + + while (stack.length > 0) { + const id = stack.pop() + if (!id || reachable.has(id)) continue + + const node = nodes[id] + if (!node) continue + + reachable.add(id) + stack.push(...getNodeChildIds(node)) + stack.push(...(childIdsByParentId.get(id) ?? [])) + } + + return reachable +} + export type SceneState = { // 1. The Data: A flat dictionary of all nodes nodes: Record @@ -450,9 +511,19 @@ const useScene: UseSceneStore = create()( } } + const normalizedRootNodeIds = normalizeRootNodeIds(cleanedNodes, rootNodeIds) + const reachableNodeIds = collectReachableNodeIds(cleanedNodes, normalizedRootNodeIds) + if (normalizedRootNodeIds.length > 0) { + for (const node of Object.values(cleanedNodes)) { + if (reachableNodeIds.has(node.id as AnyNodeId)) continue + console.warn('[Scene] Removing unreachable node', node.id) + delete cleanedNodes[node.id] + } + } + set({ nodes: cleanedNodes, - rootNodeIds, + rootNodeIds: normalizedRootNodeIds, dirtyNodes: new Set(), collections: {}, }) diff --git a/packages/editor/src/components/ui/floating-level-selector.tsx b/packages/editor/src/components/ui/floating-level-selector.tsx index 67487cee..a11ff7be 100755 --- a/packages/editor/src/components/ui/floating-level-selector.tsx +++ b/packages/editor/src/components/ui/floating-level-selector.tsx @@ -293,13 +293,21 @@ export function FloatingLevelSelector() { const handleDuplicateLevel = useCallback( (level: LevelNode, preset: LevelDuplicatePreset = 'everything') => { - const { createOps, newLevelId } = buildLevelDuplicateCreateOps({ + const { createOps, newLevelId, shiftedLevels } = buildLevelDuplicateCreateOps({ nodes: useScene.getState().nodes, level, levels, preset, }) + if (shiftedLevels.length > 0) { + updateNodes( + shiftedLevels.map((shiftedLevel) => ({ + id: shiftedLevel.id as AnyNodeId, + data: { level: shiftedLevel.level } as Partial, + })), + ) + } createNodes(createOps) setSelection({ @@ -307,7 +315,7 @@ export function FloatingLevelSelector() { levelId: newLevelId as LevelNode['id'], }) }, - [createNodes, levels, resolvedBuildingId, setSelection], + [createNodes, levels, resolvedBuildingId, setSelection, updateNodes], ) if (levels.length === 0) return null diff --git a/packages/editor/src/components/ui/sidebar/panels/site-panel/index.tsx b/packages/editor/src/components/ui/sidebar/panels/site-panel/index.tsx index 8fb3d5c0..24742881 100755 --- a/packages/editor/src/components/ui/sidebar/panels/site-panel/index.tsx +++ b/packages/editor/src/components/ui/sidebar/panels/site-panel/index.tsx @@ -587,10 +587,19 @@ const LevelItem = memo(function LevelItem({ const [duplicateDialogOpen, setDuplicateDialogOpen] = useState(false) const [isEditing, setIsEditing] = useState(false) const createNodes = useScene((s) => s.createNodes) + const updateNodes = useScene((s) => s.updateNodes) const itemRef = useRef(null) const isSelected = selectedLevelId === level.id const canDeleteLevel = level.level !== 0 const [isExpanded, setIsExpanded] = useState(isSelected) + const buildingId = + typeof level.parentId === 'string' && level.parentId.startsWith('building_') + ? (level.parentId as BuildingNode['id']) + : undefined + + const selectLevel = (levelId: LevelNode['id']) => { + setSelection(buildingId ? { buildingId, levelId } : { levelId }) + } useEffect(() => { setIsExpanded(isSelected) @@ -603,7 +612,7 @@ const LevelItem = memo(function LevelItem({ }, [isSelected]) const handleSelect = () => { - setSelection({ levelId: level.id }) + selectLevel(level.id) } const handleDoubleClick = () => { @@ -611,15 +620,23 @@ const LevelItem = memo(function LevelItem({ } const handleDuplicateLevel = (preset: LevelDuplicatePreset = 'everything') => { - const { createOps, newLevelId } = buildLevelDuplicateCreateOps({ + const { createOps, newLevelId, shiftedLevels } = buildLevelDuplicateCreateOps({ nodes: useScene.getState().nodes, level, levels, preset, }) + if (shiftedLevels.length > 0) { + updateNodes( + shiftedLevels.map((shiftedLevel) => ({ + id: shiftedLevel.id as AnyNodeId, + data: { level: shiftedLevel.level } as Partial, + })), + ) + } createNodes(createOps) - setSelection({ levelId: newLevelId }) + selectLevel(newLevelId as LevelNode['id']) setDuplicateDialogOpen(false) } @@ -664,7 +681,7 @@ const LevelItem = memo(function LevelItem({ if (isSelected) { setIsExpanded(!isExpanded) } else { - setSelection({ levelId: level.id }) + selectLevel(level.id) } }} > @@ -869,7 +886,7 @@ const LevelsSection = memo(function LevelsSection({ parentId: building.id, }) createNode(newLevel, building.id) - setSelection({ levelId: newLevel.id }) + setSelection({ buildingId: building.id, levelId: newLevel.id }) } return ( diff --git a/packages/editor/src/lib/level-duplication.test.ts b/packages/editor/src/lib/level-duplication.test.ts new file mode 100644 index 00000000..539e2164 --- /dev/null +++ b/packages/editor/src/lib/level-duplication.test.ts @@ -0,0 +1,41 @@ +// @ts-expect-error — bun:test is provided by the Bun runtime; editor does not +// depend on @types/bun so the import type is unresolved at compile time. +import { describe, expect, test } from 'bun:test' +import { + type AnyNode, + type AnyNodeId, + BuildingNode, + LevelNode, + WallNode, +} from '@pascal-app/core/schema' +import { buildLevelDuplicateCreateOps } from './level-duplication' + +describe('buildLevelDuplicateCreateOps', () => { + test('parents a duplicated bootstrap level back to its building', () => { + const level = LevelNode.parse({ level: 0, children: [] }) + const building = BuildingNode.parse({ children: [level.id] }) + const wall = WallNode.parse({ + parentId: level.id, + start: [0, 0], + end: [4, 0], + }) + const sourceLevel = { ...level, children: [wall.id] } satisfies LevelNode + const nodes = { + [building.id]: building, + [sourceLevel.id]: sourceLevel, + [wall.id]: wall, + } as Record + + const { createOps, newLevelId } = buildLevelDuplicateCreateOps({ + nodes, + level: sourceLevel, + levels: [sourceLevel], + preset: 'everything', + }) + + const levelCreateOp = createOps.find((op) => op.node.id === newLevelId) + + expect(sourceLevel.parentId).toBeNull() + expect(levelCreateOp?.parentId).toBe(building.id) + }) +}) diff --git a/packages/editor/src/lib/level-duplication.ts b/packages/editor/src/lib/level-duplication.ts index 8f0f1040..a243bd7d 100644 --- a/packages/editor/src/lib/level-duplication.ts +++ b/packages/editor/src/lib/level-duplication.ts @@ -1,5 +1,5 @@ -import type { AnyNode, AnyNodeId, LevelNode } from '@pascal-app/core' -import { cloneLevelSubtree } from '@pascal-app/core' +import { cloneLevelSubtree } from '@pascal-app/core/clone-scene-graph' +import type { AnyNode, AnyNodeId, LevelNode } from '@pascal-app/core/schema' export type LevelDuplicatePreset = | 'everything' @@ -79,6 +79,20 @@ function stripMaterials(node: AnyNode): AnyNode { return next as AnyNode } +function findLevelBuildingId(nodes: Record, levelId: AnyNodeId) { + for (const node of Object.values(nodes)) { + if (node.type !== 'building' || !('children' in node) || !Array.isArray(node.children)) { + continue + } + + if ((node.children as AnyNodeId[]).includes(levelId)) { + return node.id as AnyNodeId + } + } + + return undefined +} + export function buildLevelDuplicateCreateOps({ nodes, level, @@ -91,7 +105,15 @@ export function buildLevelDuplicateCreateOps({ preset: LevelDuplicatePreset }) { const { clonedNodes, newLevelId } = cloneLevelSubtree(nodes, level.id) - const nextLevelNumber = Math.max(...levels.map((entry) => entry.level), -1) + 1 + const parentBuildingId = + (level.parentId as AnyNodeId | null) ?? findLevelBuildingId(nodes, level.id) + const nextLevelNumber = level.level + 1 + const shiftedLevels = levels + .filter((entry) => entry.id !== level.id && entry.level >= nextLevelNumber) + .map((entry) => ({ + id: entry.id, + level: entry.level + 1, + })) const filteredNodes = clonedNodes .filter((node) => shouldKeepNode(node, preset)) @@ -119,8 +141,12 @@ export function buildLevelDuplicateCreateOps({ level: nextLevelNumber, } as AnyNode) : node, - parentId: node.parentId as AnyNodeId | undefined, + parentId: + node.id === newLevelId + ? parentBuildingId + : ((node.parentId as AnyNodeId | null) ?? undefined), })), newLevelId, + shiftedLevels, } } diff --git a/packages/mcp/src/bridge/scene-bridge.test.ts b/packages/mcp/src/bridge/scene-bridge.test.ts index 087be39c..94ffa58d 100644 --- a/packages/mcp/src/bridge/scene-bridge.test.ts +++ b/packages/mcp/src/bridge/scene-bridge.test.ts @@ -401,6 +401,27 @@ describe('SceneBridge', () => { }) describe('setScene / exportJSON / loadJSON', () => { + test('setScene prunes duplicated levels that were accidentally saved as roots', () => { + const level0 = LevelNode.parse({ level: 0, children: [] }) + const building = BuildingNode.parse({ children: [level0.id] }) + const site = SiteNode.parse({ children: [building] }) + const orphanLevel = LevelNode.parse({ level: 1, children: [] }) + + bridge.setScene( + { + [site.id]: site, + [building.id]: building, + [level0.id]: level0, + [orphanLevel.id]: orphanLevel, + } as any, + [site.id, orphanLevel.id] as any, + ) + + expect(bridge.getRootNodeIds()).toEqual([site.id]) + expect(bridge.getNode(orphanLevel.id)).toBeNull() + expect(bridge.findNodes({ type: 'level' }).map((node) => node.id)).toEqual([level0.id]) + }) + test('exportJSON returns the scene shape', () => { const exp = bridge.exportJSON() expect(typeof exp.nodes).toBe('object')