Add procedural elevator system with floorplan, runtime, and first-person support#305
Conversation
# Conflicts: # packages/editor/src/components/tools/item/move-tool.tsx # packages/editor/src/components/tools/tool-manager.tsx # packages/editor/src/components/ui/panels/panel-manager.tsx # packages/editor/src/store/use-editor.tsx # packages/viewer/src/components/renderers/site/site-renderer.tsx # packages/viewer/src/components/viewer/ground-occluder.tsx # packages/viewer/src/components/viewer/index.tsx # packages/viewer/src/components/viewer/post-processing.tsx
wass08
left a comment
There was a problem hiding this comment.
Architectural review — feat/elevator-system
67 files / +7,017 / -139. Reviewed against the Pascal architecture rules (layer boundaries, hook hygiene, renderer/system separation, selector performance). Overall: clean architecture — one blocker, a few suggestions.
Blocker
Duplicate SurfaceHoleMetadata type definition — packages/core/src/systems/elevator/elevator-opening-sync.ts:7-11 and packages/core/src/systems/stair/stair-opening-sync.ts:14-18 each redeclare the type already exported from packages/core/src/schema/nodes/surface-hole-metadata.ts. Two parallel sources of truth — when the schema gains a field (e.g. another hole source), the sync files will silently drift.
Fix: Import SurfaceHoleMetadata from ../../schema in both *-opening-sync.ts files; delete the local type.
Suggestions
-
Asymmetric elevator lifecycle in
use-interactive.ts—initElevatorandsetElevatorStateexist, but there's noremoveElevatorto mirrorremoveDoorOpenState/removeWindowOpenState. Elevator entries leak when nodes are deleted. -
move-elevator-tool.tsx:59-60temporal pause/resume isn't StrictMode-safe. The tool callsuseScene.temporal.getState().pause()at mount and.resume()at unmount with no guard. Under StrictMode double-invoke (or if temporal is already paused upstream), you'll resume more times than you paused. Track locally:const wasPausedRef = useRef(false). -
Elevator level-bypass in
selection-manager.tsx:59—isNodeInCurrentLevelreturnstruefornode.type === 'elevator'regardless of level. This is correct (elevators are building-scoped, not level-scoped), but the behaviour is non-obvious next to the neighbouring level checks. Worth a one-line comment. -
resolveElevatorSupportLevelIdinpackages/editor/src/lib/elevator-support.tsacceptspreferredLevelIdbut ignores it when the node is undefined. Either honour the preferred ID in all branches or split placement-vs-resolution into two functions. -
surface-hole-metadata.tsenum gained'elevator'alongside'manual'/'stair'— no comment explains that elevator sources are tracked viaelevatorId(parallel tostairId) and consumed bysyncAutoElevatorOpenings. Worth a one-liner since the coupling is invisible from the schema alone.
Nits
post-processing.tsx:134—void projectId;looks like a leftover. Either drop the line or wire it into the effect's deps properly.elevator-tool.tsx:34-47—resolveCurrentBuildingIdreadsuseScene.getState()at module scope. Works, but it's a pure function in disguise — move intolib/elevator-support.tstaking(buildingId, levelId, nodes)as args.elevator-renderer.tsxat 1,310 lines is large but stays renderer-shaped: Three.js primitives +useNodeEvents+useRegistry, with geometry delegated to core. No rule violation. Flag for future: if accessibility/emergency panels push past ~1,500, split intoelevator-cab+elevator-shaftsub-renderers.
What's clean (notable)
- Zero editor imports in viewer code — no
apps/editor,packages/editor,useEditor, or phase/mode references. - Core stays editor-agnostic — no
Floorplan*/Paint*/Draft*types, no tool/phase vocabulary leaked into systems. elevator-runtime-system.tsx/elevator-opening-system.tsxare.tsxin core but only useuseFramefrom@react-three/fiber— the documented pattern. No Three.js objects.ElevatorNodeschema usesobjectId('elevator')+nodeType('elevator')and is registered inAnyNode.ElevatorEventtyped inbus.tsand added touseNodeEventsconfig — renderers useuseNodeEvents(node, 'elevator'), neveremitter.emitdirectly.ElevatorInteractionSystem(viewer system) does raycast + dispatch only — actual dispatch logic lives in core (resolveElevatorDispatchTarget,requestElevatorLevel,openElevatorDoor). Correct layering.polygon-union.tsin viewer is correctly placed — pure rendering-time geometry, no scene state.move-elevator-tool.tsxfollows the live-drag exception correctly: mirrors writes intouseLiveTransformsandsceneRegistry, cleans up on unmount.
Verdict
1 blocker, 5 suggestions, 3 nits — merge-ready once the duplicate type is removed.
Start with elevator-opening-sync.ts and stair-opening-sync.ts — same fix in both.
What does this PR do?
This PR introduces a complete procedural elevator system across core, editor, viewer, floorplan, and first-person workflows.
It enables:
Elevator Core System
ElevatorNodeschema with support for:Elevator Tools & Editor Integration
Added elevator creation and move tools.
Integrated elevators into:
Added full elevator side panel controls:
Live Editing & Performance
Elevator Rendering
Door Styles & Cab Controls
Added door opening styles:
Added door panel styles:
Added inside-cab controls:
Added landing call buttons outside elevator doors.
Runtime & Dispatch System
Added elevator runtime state:
Added elevator movement system:
Added shared dispatch system for multiple elevators:
Floor Restrictions & Indicators
Added support for:
Applied restrictions across:
Added floor indicators:
Fixed mirrored floor numbers and inverted direction arrows.
Slab & Ceiling Cutouts
Added automatic slab and ceiling cutouts for elevator shafts.
Cutouts generated from:
Added:
Allows multiple cutouts to merge cleanly.
First-Person Support
Added first-person elevator interaction:
Added player locking:
Floorplan Support
Added elevator rendering in floorplan:
Added floorplan interactions:
Added served-range visualization:
Architecture Improvements
Related Structural & Viewer Updates
Expanded procedural column support:
Updated related systems for:
Ensured compatibility with elevator cutouts and rendering behavior.
Commit Highlights