editor: fix wall item placement preview drifting off the item - #643
Merged
Conversation
The placement wireframe and the item it previews were computed from two different quantities: the node position snapped the hit in WALL-LOCAL coords, while the cursor snapped the raw WORLD hit independently per axis. Those are different lattices — wall-local X runs from `wall.start`, and wall-local Y is measured from the supporting slab's elevation, not world zero — so the box drifted off the item by the slab elevation plus up to a grid step, and the commit (which follows the node) landed where the box was not. `calculateCursorRotation` also returns the wall face's yaw + π, which is invisible for the z-symmetric door/window boxes it was written for but put the asymmetric `wall-side` item box on the far side of the wall. The coordinator already compensated for that π when publishing the 2D floorplan preview; that workaround goes away with the cause. Both now come from `resolveWallPlacementPose`, which maps one wall-local point through the hit object's frame — the exact inverse of how `localPosition` was measured. `z` follows the hosting convention rather than the hit depth, so `wall-side` items store the face they mount on (matching ItemSystem's per-frame push, no first-frame pop) and `wall` items stay centred in the thickness instead of being snapped out of a thick wall. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`FloorElevationSystem` documents that it "respects `floorPlaced.applies` so items with `asset.attachTo` (wall / ceiling mounted) are left alone", but it never called the predicate — only `getFloorPlacedElevation` did. For an opted-out node that resolver returns 0, so the system's write degenerated to `mesh.position.y = position[1]`. Harmless while `position` is the node's own host-local store value, but tools publish live transforms in WORLD space: the moment a wall item started moving, the system copied its world Y into the mesh's wall-local Y slot every frame and the ghost floated off the wall by the host frame's elevation. Hence the reported sequence — correct on `wall:enter` (no live transform yet), wrong throughout the move, correct again after commit (the coordinator clears the live transform first). Honour `applies` before the write. Covers the other three opt-outs too: ceiling items, cabinet modules inside a run, and non-floor duct terminals. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Placing an item on a wall drew the green placement box in a different spot from the item it previews, and the item then committed somewhere neither of them was. Three independent causes:
The box and the node came from different quantities.
wallStrategysnapped the node position in WALL-LOCAL coords but snapped the raw WORLD hit independently per axis for the cursor. Those are different lattices — wall-local X runs fromwall.start, and wall-local Y is measured from the supporting slab's elevation (mesh.position.set(start[0], slabElevation, start[1])), not world zero. Every sibling strategy (floor / ceiling / roof-wall) already derives its cursor from the point it stores; wall was the odd one out. Both now come fromresolveWallPlacementPose, which maps one wall-local point through the hit object's frame — the exact inverse of howlocalPositionwas measured.calculateCursorRotationreturns the wall face yaw + π. Invisible for the z-symmetric door/window boxes it was written for, but it put the asymmetricwall-sideitem box (z ∈ [0, depth]) on the far side of the wall. The coordinator was already compensating for that π when publishing the 2D floorplan preview; that workaround goes away with the cause. The shared helper is left alone for door/window.FloorElevationSystemnever calledfloorPlaced.applies. Its own doc comment claims it "respectsfloorPlaced.appliesso items withasset.attachTo(wall / ceiling mounted) are left alone", but onlygetFloorPlacedElevationdid. For an opted-out node that resolver returns 0, so the system's write degenerated tomesh.position.y = position[1]— harmless while that's the node's host-local store value, but tools publish live transforms in WORLD space, so the moment a wall item started moving it copied the world Y into the mesh's wall-local Y slot every frame and floated the ghost off the wall. Hence the reported sequence: correct onwall:enter(no live transform yet), wrong throughout the move, correct again after commit (the coordinator clears the live transform first). Honouringappliesalso covers the other three opt-outs: ceiling items, cabinet modules inside a run, and non-floor duct terminals.Two smaller consequences of deriving
zfrom the hosting convention rather than the hit depth:wall-sideitems now store the face they mount on (matchingItemSystem's per-frame push, so no first-frame pop), andwallitems stay centred in the thickness instead of being snapped clean out of a thick wall bysnapToHalf.How to test
bun devand open a scene with a room. A wall sitting on a slab (non-zero elevation) and a wall that is neither axis-aligned nor grid-aligned show the bug most clearly.attachTo: 'wall-side'catalog item and hover it over a wall face. The green box should wrap the item — same height off the floor, same spot along the wall, and extending out of the near face toward the room, not through the wall.attachTo: 'wall'item on a thick wall (≥ 0.6 m) — it should sit centred in the thickness rather than snapped out of it.Screenshots / screen recording
N/A — no recording captured (the Chrome extension wasn't connected in this session). The before-state is the reported bug: box below and behind the item, item floating above it while moving.
Checklist
bun devbun checkto verify)mainbranchNote
Medium Risk
Changes core 3D placement and per-frame mesh Y during drags for wall-mounted items; behavior is localized with new tests but regressions could affect doors/windows or floor stacking if
appliesgates are wrong.Overview
Fixes wall item placement where the green preview box, the moving ghost, and the committed position could disagree—especially on elevated slabs, diagonal walls, and
wall-sideassets.Wall strategy introduces
resolveWallPlacementPosesowallStrategyenter/move snap X/Y in wall-local space, set Z from hosting rules (wall-sideon the hit face,wallcentered in thickness) instead of snapping hit depth, and map the same point to worldcursorPositionvia the collision mesh. Preview rotation is now wall yaw + item rotation (replacingcalculateCursorRotationon this path), so asymmetricwall-sideboxes face outward and the 2D floorplan can usecursorRotationYdirectly.Placement coordinator drops redundant world-position patching during wall drag offset correction; pose math lives in the strategy.
Floor elevation system skips nodes when
floorPlaced.appliesreturns false, so live transforms published in world space no longer overwrite wall-local mesh Y during wall moves.Adds
placement-strategies.test.tscovering preview/commit alignment, face mounting, and validator-adjusted Y.Reviewed by Cursor Bugbot for commit c7bca37. Bugbot is set up for automated code reviews on this repo. Configure here.