Merge pull request #433 from pascalorg/chore/editor-polish

Editor polish: paint SFX, duplicate keeps slots, review-skill convention

.agents/skills/review-architecture/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: review-architecture
-description: Review a PR against the Pascal architectural rules — package boundaries (core/viewer/editor/nodes), the registry-driven composition model (def.geometry / def.renderer / def.system), legacy-dispatch regressions, hook hygiene (useEditor/useScene/useViewer), and selector performance. Use when the user asks to review a PR, audit a branch, or check that changes respect the codebase's architecture.
+description: Review a PR against the Pascal architectural rules — package boundaries (core/viewer/editor/nodes), the registry-driven composition model (def.geometry / def.renderer / def.system), legacy-dispatch regressions, the slots + world-scale-UV convention for new nodes/geometry, hook hygiene (useEditor/useScene/useViewer), and selector performance. Use when the user asks to review a PR, audit a branch, or check that changes respect the codebase's architecture.
 allowed-tools: Bash(git *) Bash(gh *) Read Grep Glob
 ---
 
@@ -101,6 +101,9 @@ If the PR adds or modifies a node kind, check against `wiki/architecture/node-de
 - **Host kinds need `children` on the schema.** If `def.relations.hosts` is set, the schema must declare `children: z.array(z.string()).default([])` (and `migrateNodes` must patch existing scenes). Otherwise `useScene.createNode(child, parentId)` writes a `parent.children` entry into nothing and the host never sees the new child.
 - **Movable opt-in.** `MoveTool` dispatches to `MoveRegistryNodeTool` only when `def.capabilities.movable` is set. Kinds with bespoke move semantics (wall endpoint drag with linked-wall cascade, slab vertex edit, etc.) deliberately omit `movable` and supply `def.affordanceTools.move` instead. Force-routing a bespoke-move kind through generic dispatch (`nodeRegistry.has(kind)` instead of `def.capabilities.movable`) is a regression — call it out. The bug history is documented in `plans/editor-node-registry.md` ("Capability-driven move dispatch").
 - **Paint dispatch lives on `def.capabilities.paint`.** A paintable kind declares `resolveRole` / `buildPatch` / `applyPreview` (+ optional `getEffectiveMaterial`) on `PaintCapability`; the editor's selection-manager routes hover / click / preview through the generic dispatcher. A PR that adds an `if (node.type === '<kind>')` arm to paint-mode handling, paint-preview application, or material picker resolution is a regression — the behaviour belongs on the kind's `paint` capability. See `packages/core/src/registry/types.ts` (`PaintCapability`).
+- **Slots + world-scale UVs for paintable surfaces.** A new kind (or geometry change) that exposes paintable parts must follow the unified slot convention, not reinvent it:
+  - **Paintable parts are slots, carried on the node.** Overrides live in a `slots` record (`slotId → MaterialRef`, `scene:`/`library:`) on the schema, resolved via `def.capabilities.paint` — not ad-hoc per-surface `material` / `materialPreset` fields, and not a parallel store. A new paintable kind whose schema lacks `slots` (or whose duplicate / preset / clone path drops it) is a blocker: it silently loses painted materials. (Slots are plain data — generic clone/parse preserves them; bespoke draft-rebuild placement paths must thread them through explicitly. Reference bug: item duplicate rebuilt the draft from `asset` and dropped `slots`.)
+  - **Texturable geometry emits UVs in metres (1 UV unit = 1 m).** Any `def.geometry` producing a surface a finish can tile onto must generate UVs at the same world scale walls / slabs / roofs use, because catalog finishes set `repeat` as tiles-per-metre. Unitless, bounding-box-normalised, or hardcoded UVs that don't scale with the surface are a blocker — finishes won't tile consistently. Flat-colour-only surfaces need no UVs. GLB item authoring follows the same contract via `slot_`-prefixed materials (case-insensitive, `slot_` → slot id). See `wiki/architecture/materials-and-themes.md` § "Texture world scale" and `wiki/architecture/item-authoring.md`.
 - **Floor elevation lives on `def.capabilities.floorPlaced`.** Kinds that rest on a level and lift over overlapping slabs declare a `footprint` (and optional `applies` predicate) on `FloorPlacedConfig`; the generic `<FloorElevationSystem>` writes `slabElevation + node.position[1]` onto the registered mesh on each dirty mark. A new per-kind `useEffect` / per-kind system that recomputes Y from slab overlap is a regression — the per-kind block was lifted out of `ItemSystem` in Phase 6.1. See `packages/core/src/registry/types.ts` (`FloorPlacedConfig`).
 - **Render-mode behaviour goes through `def.surfaceRole`.** Solid / Rendered / Clay viewer modes look up the kind's `surfaceRole` on `NodeDefinition` to pick the right material strategy (clay overrides, edge passes, theme overlays). New `if (node.type === '<kind>')` arms inside the render-mode pipeline or theme application are a regression — the behaviour belongs on `def.surfaceRole`. See `packages/core/src/registry/types.ts:529`.
 - **Capability names must describe verbs, not host kinds.** `movable` / `paint` / `floorPlaced` / `cuttable` / `selectable` describe *what the node does*. A new capability named after a host kind (`slabAccessory`, `wallAccessory`, `siteAccessory`, anything `Xaccessory` / `Xhosted` shaped) couples the registry's type surface to one specific host and reads as precedent for the next reviewer. Blocker. Push back: generalise into a paired *host-side* capability ("I merge subtractive accessories from my children") + *accessory-side* capability ("I provide a cut geometry, cascade my dirty mark to my host's parent"). The single existing case — `capabilities.roofAccessory` (`packages/core/src/registry/types.ts:791`, consumed by `packages/viewer/src/systems/roof/roof-system.tsx`) — is documented tech debt; do not extend the pattern.

packages/editor/src/components/editor/selection-manager.tsx

@@ -1157,6 +1157,7 @@ export const SelectionManager = () => {
       }
 
       interaction.apply()
+      sfxEmitter.emit('sfx:paint-apply')
       if (activePreview?.key === interaction.key) {
         activePreview = null
       } else {

packages/editor/src/components/tools/item/use-draft-node.ts

@@ -27,12 +27,14 @@ export interface DraftNodeHandle {
   readonly current: ItemNode | null
   /** Whether the current draft was adopted (move mode) vs created (create mode) */
   readonly isAdopted: boolean
-  /** Create a new draft item at the given position. Returns the created node or null. */
+  /** Create a new draft item at the given position. Returns the created node or null.
+   *  `slots` seeds painted slot overrides so duplicates keep their materials. */
   create: (
     gridPosition: Vector3,
     asset: AssetInput,
     rotation?: [number, number, number],
     scale?: [number, number, number],
+    slots?: ItemNode['slots'],
   ) => ItemNode | null
   /** Take ownership of an existing scene node as the draft (for move mode). */
   adopt: (node: ItemNode) => void
@@ -61,6 +63,7 @@ export function useDraftNode(): DraftNodeHandle {
       asset: AssetInput,
       rotation?: [number, number, number],
       scale?: [number, number, number],
+      slots?: ItemNode['slots'],
     ): ItemNode | null => {
       const currentLevelId = useViewer.getState().selection.levelId
       if (!currentLevelId) return null
@@ -73,6 +76,7 @@ export function useDraftNode(): DraftNodeHandle {
         asset,
         parentId: currentLevelId,
         metadata: { isTransient: true },
+        ...(slots ? { slots } : {}),
       })
 
       useScene.getState().createNode(node, currentLevelId)
@@ -180,6 +184,8 @@ export function useDraftNode(): DraftNodeHandle {
       rotation: updateProps.rotation ?? draft.rotation,
       scale: updateProps.scale ?? draft.scale,
       side: updateProps.side ?? draft.side,
+      // Carry painted slot overrides so a duplicated item keeps its materials.
+      ...(draft.slots ? { slots: draft.slots } : {}),
       // Roof host — see the move-mode commit above for why this must be
       // forwarded explicitly.
       roofSegmentId: updateProps.roofSegmentId,

packages/editor/src/components/tools/item/use-placement-coordinator.tsx

@@ -192,6 +192,9 @@ export interface PlacementCoordinatorConfig {
   initialState?: PlacementState
   /** Scale to use when lazily creating a draft (e.g. for wall/ceiling duplicates). Defaults to [1,1,1]. */
   defaultScale?: [number, number, number]
+  /** Painted slot overrides to seed onto a lazily-created draft (wall/ceiling
+   *  duplicates) so the duplicate keeps its materials. */
+  slots?: ItemNode['slots']
   /** Move-mode sessions keep the grabbed item offset from the first surface hit
    *  (floor / wall / ceiling / item-surface / shelf) instead of snapping the
    *  item's origin under the cursor. */
@@ -512,7 +515,13 @@ export function usePlacementCoordinator(config: PlacementCoordinatorConfig): Rea
         0,
       ]
 
-      draftNode.create(gridPosition.current, asset, initRotation, configRef.current.defaultScale)
+      draftNode.create(
+        gridPosition.current,
+        asset,
+        initRotation,
+        configRef.current.defaultScale,
+        configRef.current.slots,
+      )
 
       const draft = draftNode.current
       if (draft) {
@@ -857,7 +866,13 @@ export function usePlacementCoordinator(config: PlacementCoordinatorConfig): Rea
 
       draftNode.commit(result.nodeUpdate)
       if (configRef.current.onCommitted()) {
-        draftNode.create(gridPosition.current, asset, currentRotation)
+        draftNode.create(
+          gridPosition.current,
+          asset,
+          currentRotation,
+          configRef.current.defaultScale,
+          configRef.current.slots,
+        )
         const previewBounds = expandBoundsToGrid(
           getFallbackPreviewBounds(draftNode.current, asset, asset.attachTo),
           asset.attachTo,

packages/editor/src/lib/sfx-bus.ts

@@ -16,6 +16,7 @@ type SFXEvents = {
   'sfx:snapshot-capture': undefined
   'sfx:menu-hover': undefined
   'sfx:menu-click': undefined
+  'sfx:paint-apply': undefined
 }
 
 /**
@@ -45,6 +46,7 @@ export function initSFXBus() {
   sfxEmitter.on('sfx:snapshot-capture', () => playSFX('snapshotCapture'))
   sfxEmitter.on('sfx:menu-hover', () => playSFX('menuHover'))
   sfxEmitter.on('sfx:menu-click', () => playSFX('menuClick'))
+  sfxEmitter.on('sfx:paint-apply', () => playSFX('paintApply'))
 }
 
 /**

packages/editor/src/lib/sfx-player.ts

@@ -100,6 +100,15 @@ export const SFX: Record<string, SFXConfig> = {
     volumeRange: [0.5, 0.6],
     panJitter: 0.1,
   },
+  // Fired when a material is applied to a surface in paint mode. Painting can
+  // fire in quick succession across faces, so keep variation + a small gap.
+  paintApply: {
+    src: '/audios/sfx/paint_apply.mp3',
+    rateRange: [0.95, 1.05],
+    volumeRange: [0.85, 1.0],
+    panJitter: 0.12,
+    minIntervalMs: 60,
+  },
 } as const
 
 export type SFXName = keyof typeof SFX

packages/nodes/src/item/move-tool.tsx

@@ -115,6 +115,9 @@ export function MoveItemTool({ node }: { node: ItemNode }) {
   const cursor = usePlacementCoordinator({
     asset: node.asset,
     draftNode,
+    // Carry painted slot overrides onto the duplicate's draft (wall/ceiling
+    // items create their draft lazily inside the coordinator).
+    slots: node.slots,
     // Duplicates start fresh in floor mode; wall/ceiling draft is created lazily by ensureDraft.
     initialState: isNew
       ? {
@@ -135,7 +138,7 @@ export function MoveItemTool({ node }: { node: ItemNode }) {
         // items are created lazily on surface entry.
         gridPosition.copy(new Vector3(...node.position))
         if (!node.asset.attachTo) {
-          draftNode.create(gridPosition, node.asset, node.rotation, node.scale)
+          draftNode.create(gridPosition, node.asset, node.rotation, node.scale, node.slots)
         }
       } else {
         draftNode.adopt(node)