refactor: release-review cleanup for roof wall openings

Dual review pass (Claude multi-angle + Codex release-quality). One
correctness fix and the agreed do-now cleanups:

- fix: clone-scene-graph remaps roofSegmentId like wallId in both
  clone paths — duplicated scenes/levels kept pointing roof-hosted
  children at the original segments.
- extract the settled, stateless roof target/cursor math shared by the
  four door/window tools into shared/roof-wall-opening-placement.ts
  (resolveRoofWallOpeningTarget + getRoofWallOpeningCursorPose +
  worldToSelectedBuildingLocal); tools keep the stateful lifecycle
  (drafts, undo/temporal, commit field lists). −199 net lines.
- rename host-generic state: currentWallId→currentHostId,
  markWallDirty→markHostDirty (they hold segment ids too); capability
  cascadesViaHostSegment→dirtyHandledByOwnSystem (behavior-facing,
  before the public API hardens).
- drop getRoofAccessoryKinds from core's public API — its only caller
  was the standalone Build tab, which now enumerates the registry
  inline with its app-specific filter.
- window move-tool uses the shared stripPlacementMetadataFlags; stale
  "segment-local" comment fixed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Aymeric Rabot
2026-06-10 14:33:13 -04:00
co-authored by Claude Fable 5
parent b8dfa90762
commit aa3b0ef758
16 changed files with 349 additions and 435 deletions
-1
View File
@@ -17,7 +17,6 @@ export type {
export {
discoverPlugins,
getHostRefFields,
getRoofAccessoryKinds,
getSelectableKinds,
isDrawnViaTool,
isDrawnViaToolKind,
-22
View File
@@ -114,28 +114,6 @@ export function isRegistrySelectable(kind: string): boolean {
return nodeRegistry.get(kind)?.capabilities.selectable !== undefined
}
/**
* Kinds whose definition declares the `roofAccessory` capability — the roof
* accessories (dormer, chimney, vents, gutter, …) that mount onto a roof
* segment via their own attach tool. Lets host UIs surface a "Features" group
* under the roof category without hardcoding the kind list (the standalone
* editor's Build tab; the roof inspector's add menu). Returned in builtin
* registration order (`packages/nodes/src/index.ts`), which is deterministic.
*
* Call at render time, not module-import time: the registry is populated by
* the host's bootstrap (`loadPlugin`), so a top-level `const` would race it
* and see an empty registry.
*/
export function getRoofAccessoryKinds(): string[] {
const result: string[] = []
for (const [kind, def] of nodeRegistry.entries()) {
if (def.capabilities.roofAccessory !== undefined) {
result.push(kind)
}
}
return result
}
/**
* Kinds whose `def.floorplanScope` matches the requested scope. Used by
* `FloorplanRegistryLayer` to discover building-scoped kinds (e.g.
+7 -7
View File
@@ -1214,14 +1214,14 @@ export type RoofAccessoryConfig = {
*/
cutScope?: 'all' | 'wall'
/**
* Set when the kind runs its own dirty-driven geometry system that
* already cascades to the host segment (door / window via the
* DoorSystem / WindowSystem `parentId` cascade). The roof-merge loop
* must then leave the kind's dirty marks alone — consuming them here
* would starve that system whenever it defers a rebuild (mesh not
* mounted yet, per-frame rebuild budget exhausted).
* The kind's own dirty-driven geometry system consumes its dirty
* marks (door / window via DoorSystem / WindowSystem, which already
* cascade to the host segment through `parentId`). The roof-merge
* loop must then leave those marks alone — consuming them would
* starve that system whenever it defers a rebuild (mesh not mounted
* yet, per-frame rebuild budget exhausted).
*/
cascadesViaHostSegment?: boolean
dirtyHandledByOwnSystem?: boolean
}
/**
@@ -76,6 +76,13 @@ export function cloneSceneGraph(sceneGraph: SceneGraph): SceneGraph {
| undefined
}
// Remap roofSegmentId (doors/windows/items hosted on roof wall faces)
if ('roofSegmentId' in clonedNode && typeof clonedNode.roofSegmentId === 'string') {
;(clonedNode as Record<string, unknown>).roofSegmentId = idMap.get(
clonedNode.roofSegmentId,
) as string | undefined
}
clonedNodes[newId] = clonedNode
}
@@ -220,6 +227,12 @@ export function cloneLevelSubtree(
;(cloned as Record<string, unknown>).wallId = idMap.get(cloned.wallId) ?? cloned.wallId
}
// Remap roofSegmentId (doors/windows/items hosted on roof wall faces)
if ('roofSegmentId' in cloned && typeof cloned.roofSegmentId === 'string') {
;(cloned as Record<string, unknown>).roofSegmentId =
idMap.get(cloned.roofSegmentId) ?? cloned.roofSegmentId
}
clonedNodes.push(cloned)
}