Repository navigation
Fix node dragging and refine pillars, elevators, and roofs - #1112
sudhir9297 wants to merge 7 commits into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
Thanks @sudhir9297, this is a big step up for pillars, elevators and roofs, really appreciate the work. One bug before we go further: elevator moves now go through the generic move tool ( Could you also add screenshots or short clips of the new roof menu, the thumbnails, the barrel roof and the pillars? A lot of this is visual and it's hard to judge from the diff. Last thing: at 158 files this is tough to review in one go. Would you be up for splitting it into three PRs (pillars, elevators, roofs)? Smaller ones will get reviewed and merged a lot faster. |
Conflicts resolved keeping main's behavior, carried into the scene runtime: - history-control: main's commit authors, write guards and derived-write notes; the derived-write notes and write guards are per scene environment, like the PR's other commit state, so a detached scene's writes never reach the live commit or end the editor's preview. - roof elevation: main's wall-top support (#1097) and roofsKeptOverStorey (#1112 era roof levels) run in both the editor's system and reconcileDocument; roofsKeptOverStorey selects roofs and reads level elevations only once a roof stands over a storey. - DECISIONS.md: main's E-017 stays; the scene runtime entry is E-018. - MCP: main's tool executor and recorder wrap the PR's scene scope; compileObject reads the scene before its first await and takes a pre-minted id; main's activeLevelId, briefs, bpy and materials go through the bridge's runtime. - agent operations: main's new code selects what it reads (queryNodes) or says why it reads the whole scene, per the raw-scene-enumeration rule.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c3edd17. Configure here.
| override: ({ node }) => | ||
| isManagedColumn(node as ColumnNodeType) | ||
| ? null | ||
| : { |
There was a problem hiding this comment.
Managed posts remain 3D movable
Medium Severity
movable.override returns null for lean-to managed posts, but resolveMovable treats null as “use the base config,” so isMovable stays true. 2D move is blocked, yet 3D direct drag and the generic move tool still translate the post and write position without going through columnEditPatch.
Reviewed by Cursor Bugbot for commit c3edd17. Configure here.
Aymericr
left a comment
Aymericr
left a comment
There was a problem hiding this comment.
Thanks @sudhir9297, the elevator fix through movable.resolvePosition looks right, and the screenshots help a lot.
Two things left before this can go further:
- Managed (lean-to) posts can still be dragged in 3D. In
packages/nodes/src/column/definition.tsthemovable.overridereturnsnullfor managed columns, butresolveMovableinpackages/core/src/services/movement.tstreatsnullas "use the base config" (return overridden ?? base). SoisMovablestays true and the generic move tool and 3D drag still writepositionwithout going throughcolumnEditPatch. Returning a config withaxes: []for managed columns (or makingresolveMovabletreat an explicitnullas not movable) would fix it. A test that a managed post isn't movable would lock it in. - Merge conflicts. The branch now conflicts with
main, so please mergemainin (no force-push needed).
And a gentle follow-up on the earlier question: are you up for splitting this into pillars, elevators and roofs? At 167 files it's still hard to review in one pass.
What does this PR do?
Fixes node placement, movement, and resize interactions and refines pillar, elevator, and roof editing on this branch:
The elevator movement review finding is addressed through the optional registry
movable.resolvePositionhook: the 3D and 2D movers resolve landing height aftersnapping and again at commit. Regression coverage checks an elevator crossing a
raised slab in both directions and a mounted mover whose support changes between
preview and drop. One undo still restores the gesture.
How to test
bun run check-types(13 tasks passed),bun run check(passed with warnings), andbun test packages/editor/src/components/systems/roof/roof-edit-system.test.tsx packages/nodes/src/roof-segment/renderer.test.tsx packages/nodes/src/roof/type-thumbnail.test.tsx(4 tests passed). The nodes package build also passed. The recovery regression injects a failed rebuild and verifies mesh retention and successful retry.Full
bun run cipassed after fixing test isolation: drag-session tests now restore registry registrations, and the floor-height gesture tests verify undo/redo rather than assuming history can grow past its 50-entry limit. The CI-ordered regression harness also passed all 564 tests.Latest validation (
c3edd17a8): full localbun run cipassed; GitHubquality,cli-smoke, andciall passed. The focused elevator/controller regression run passed 12 tests.Screenshots / screen recording
Editor captures from this branch's visual verification:
Four-column roof menu and thumbnails
Roof inspector thumbnails
Barrel roof
Pillar preset gallery
Checklist
bun devbun checkto verify)mainbranchNote
Medium Risk
Touches elevator runtime/opening sync, roof surface math, and broad column geometry/editing—important building logic with heavy test coverage but wide blast radius.
Overview
This PR tightens placement, pillars, elevators, and roofs end-to-end: interaction fixes, registry-owned tools, and clearer editor UX.
Pillars (columns) get lower-poly procedural geometry, a shared footprint/bounds model for plan view, collision, and handles, shape presets with hover preview and white default slot materials, inspector 3D highlights, and property previews that move hosted children without extra undo steps. Lean-to posts stay rotation-only; radius/width edits stay in sync via
columnEditPatch.Elevators drop legacy
ElevatorTool/MoveElevatorToolin favor of the registry path. Core adds dimension normalization, shaft-top clearance from cab height and wall thickness, runtime handling for disabled/removed landings and level elevation edits, simpler slab/ceiling opening rules (scoped to the owning building), and door leaf motion that stays inside the opening. New exports and tests cover these paths.Roofs add the barrel type, shared pitch/size limits, and more accurate
getRoofSegmentSurfaceYfor hip, gambrel, and mansard. Trim editing no longer usesmetadata.showTrimPlanes; it uses interaction scopereshape: 'trim', with Escape ignored while typing in fields. The build tab shows nine roof types in a 4-column grid withRoofTypeThumbnailicons.Editor polish: slider value controls are proper buttons/labels; right-stack layout keeps the inspector usable on short viewports; roof accessory movers use grid/magnetic snap (Alt to bypass) and strip placement metadata on commit. Column default library materials switch to
preset-white;showTrimPlanesis removed from the reference inventory.Reviewed by Cursor Bugbot for commit c4e7bcf. Bugbot is set up for automated code reviews on this repo. Configure here.