Normalize root scene nodes and fix level duplication
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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<string, any>): Record<string, AnyNode> {
|
||||
return patchedNodes as Record<string, AnyNode>
|
||||
}
|
||||
|
||||
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<AnyNodeId, AnyNode>,
|
||||
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<AnyNodeId, AnyNode>,
|
||||
rootNodeIds: AnyNodeId[],
|
||||
): Set<AnyNodeId> {
|
||||
const reachable = new Set<AnyNodeId>()
|
||||
const stack = [...rootNodeIds]
|
||||
const childIdsByParentId = new Map<AnyNodeId, AnyNodeId[]>()
|
||||
|
||||
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<AnyNodeId, AnyNode>
|
||||
@@ -450,9 +511,19 @@ const useScene: UseSceneStore = create<SceneState>()(
|
||||
}
|
||||
}
|
||||
|
||||
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<AnyNodeId>(),
|
||||
collections: {},
|
||||
})
|
||||
|
||||
@@ -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<AnyNode>,
|
||||
})),
|
||||
)
|
||||
}
|
||||
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
|
||||
|
||||
@@ -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<HTMLDivElement>(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<AnyNode>,
|
||||
})),
|
||||
)
|
||||
}
|
||||
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 (
|
||||
|
||||
@@ -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<AnyNodeId, AnyNode>
|
||||
|
||||
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)
|
||||
})
|
||||
})
|
||||
@@ -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<AnyNodeId, AnyNode>, 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,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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')
|
||||
|
||||
Reference in New Issue
Block a user