feat: add Fireplace node with animated procedural fire#488
Conversation
New architectural fireplace node for the Pascal 3D building editor: - 4 styles: wall, freestanding, corner, double-sided - Parametric mantel, hearth, surround, firebox with inspector controls - Animated instanced fire particle system with flickering point light - 5 fire intensity levels (none → roaring), 4 fire colors - Placement tool with grid snap, alignment guides, SFX - Translucent ghost preview during placement - Resize/rotate/move handles in 3D viewport Registered through the builtin plugin via the same NodeDefinition contract as all other node kinds. Follows the three-checkbox model: geometry (pure builder) + renderer (custom React with useFrame) + tool (lazy placement component).
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 11 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a8d3b37. Configure here.
| <group | ||
| ref={ref} | ||
| position={node.position} | ||
| rotation={[0, (node.rotation * Math.PI) / 180, 0]} |
There was a problem hiding this comment.
Rotation treated as degrees
High Severity
The FireplaceRenderer expects node.rotation to be in degrees and converts it to radians, but node.rotation is stored in radians elsewhere. This unit mismatch causes the fireplace to rotate by a much smaller amount than intended when adjusted via handles or other controls.
Reviewed by Cursor Bugbot for commit a8d3b37. Configure here.
| visible={node.visible} | ||
| {...handlers} | ||
| {...liveTransform} | ||
| > |
There was a problem hiding this comment.
Live transform breaks rotation prop
High Severity
Spreading liveTransform onto the <group> passes a scalar rotation value. However, the <group>'s rotation prop expects an Euler tuple, causing incorrect rotation behavior and visual jumps during interactive drags.
Reviewed by Cursor Bugbot for commit a8d3b37. Configure here.
| handles: fireplaceHandles, | ||
| parametrics: fireplaceParametrics, | ||
| geometry: buildFireplaceGeometry, | ||
| renderer: { kind: 'parametric', module: () => import('./renderer') }, |
There was a problem hiding this comment.
Duplicate fireplace geometry mounted
High Severity
The fireplace geometry is rendered twice. It's defined in NodeDefinition.geometry for GeometrySystem to handle, but the custom FireplaceRenderer also explicitly renders the same geometry. This results in doubled visuals and can cause transformation issues, like dropped corner yaw, especially since markDirty is called on every render.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a8d3b37. Configure here.
| mantel.name = 'fireplace-mantel' | ||
| const mantelY = height - mantelThickness / 2 | ||
| mantel.position.set(0, mantelY, mantelOverhang / 2) | ||
| group.add(mantel) |
There was a problem hiding this comment.
Mantel height control ignored
Medium Severity
The mantelHeight property is exposed and triggers rebuilds, but its value only determines if a mantel is rendered. The mantel's actual dimensions and vertical placement are controlled by mantelThickness and the overall height, so adjusting mantelHeight has no visual impact.
Reviewed by Cursor Bugbot for commit a8d3b37. Configure here.
| intensity={fireConfig.scale * 2} | ||
| color={fireColor} | ||
| /> | ||
| </> |
There was a problem hiding this comment.
Double-sided fire only on front
Medium Severity
For double-sided fireplaces, the geometry includes a rear firebox opening, but fire particles and light are only rendered at the front. This leaves the back opening dark and static, preventing the fireplace from appearing genuinely two-sided.
Reviewed by Cursor Bugbot for commit a8d3b37. Configure here.
| // Corner style — rotate the whole structure by cornerAngle. | ||
| if (style === 'corner') { | ||
| group.rotation.y = (cornerAngle * Math.PI) / 180 | ||
| } |
There was a problem hiding this comment.
Corner style desyncs handles
Medium Severity
For style: 'corner', yaw is applied on the builder Group via cornerAngle, while resize/rotate/move handles are placed in the outer node frame. The visible mesh rotates under the handles, so gizmos no longer line up with the fireplace body. GeometrySystem also drops that group rotation when reparenting children.
Reviewed by Cursor Bugbot for commit a8d3b37. Configure here.
| node.fireboxMaterialPreset, | ||
| shading, | ||
| ], | ||
| ) |
There was a problem hiding this comment.
Geometry resources never disposed
Low Severity
buildFireplaceGeometry results are swapped via useMemo with no dispose of child BoxGeometry/materials, and FireParticles creates a SphereGeometry plus MeshBasicMaterial without teardown. Parametric edits and fire toggles leak GPU resources over a session.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a8d3b37. Configure here.
| /// <reference types="next" /> | ||
| /// <reference types="next/image-types/global" /> | ||
| import "./.next/dev/types/routes.d.ts"; | ||
| import "./.next/types/routes.d.ts"; |
There was a problem hiding this comment.
Unrelated Next env change
Low Severity
This commit changes apps/ifc-converter/next-env.d.ts (an auto-generated Next.js types stub marked “should not be edited”) with no connection to the Fireplace feature. It looks like an accidental local regen included in the PR.
Reviewed by Cursor Bugbot for commit a8d3b37. Configure here.
| } | ||
|
|
||
| emitter.on('grid:move', onGridMove) | ||
| const unsubscribePlacementClicks = subscribeFloorPlacementClicks(commitAtCursor) |
There was a problem hiding this comment.
Placement ignores fireplace clicks
Medium Severity
subscribeFloorPlacementClicks only listens for clicks on kinds listed in FLOOR_PLACEMENT_CLICK_TRIGGER_KINDS, and that list was never updated for fireplace. Clicks on an existing fireplace do not commit floor placement for the fireplace tool or other floor tools that share this helper.
Reviewed by Cursor Bugbot for commit a8d3b37. Configure here.
| selectable: { hitVolume: 'bbox' }, | ||
| duplicable: true, | ||
| deletable: true, | ||
| }, |
There was a problem hiding this comment.
Missing slab elevation support
Medium Severity
Unlike shelf, column, spawn, and item, the fireplace definition never declares capabilities.floorPlaced. FloorElevationSystem therefore never lifts it onto overlapping slabs, so a fireplace on a raised slab stays at level Y and sinks through the slab. The placement tool still calls getFloorStackPreviewPosition, which cannot elevate correctly without a footprint.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a8d3b37. Configure here.
1 participant
What does this PR do?
Adds a new architectural Fireplace node kind to the editor — the first node with an animated procedural effect (instanced fire particles + flickering point light via
useFrame).wall,freestanding,corner,double-sidednone→roaring), 4 fire colors (orange,amber,blue,white)Registered through the builtin plugin via the same
NodeDefinitioncontract as all other node kinds. Follows the three-checkbox model fromwiki/architecture/node-definitions.md:geometry(pure builder) +renderer(custom React withuseFrame) +tool(lazy placement component).How to test
git clone https://github.com/toolucid/editor.git && cd editor && bun install && bun devhttp://localhost:3002in a browsernone→roaring), fire color, and fireplace stylebun check-typesandbun check— both pass with 0 errorsScreenshots / screen recording
Will add a screen recording before review.
Checklist
bun devbun checkto verify)mainbranchNote
Medium Risk
Adds a new discriminated union member and event surface area, plus per-instance
useFrameanimation and extra lights that could affect viewer performance with many fireplaces; otherwise mirrors existing furnish nodes with no auth or persistence changes.Overview
Introduces a new
fireplacefurnish node end-to-end: Zod schema in core,FireplaceEventon the editor bus, and registration in the builtin plugin.The node supports parametric dimensions (mantel, hearth, surround, firebox), four styles (
wall,freestanding,corner,double-sided), and inspector fire settings (intensity and color). Implementation follows the standardNodeDefinitionsplit: box-based geometry builder, a custom renderer with instanced particles and a flickering point light driven byuseFrame, floor placement tool (grid snap, alignment guides, ghost preview), and viewport resize/rotate/move handles.Also updates
apps/ifc-converter/next-env.d.tsto import Next route types from./.next/types/routes.d.tsinstead of the dev path.Reviewed by Cursor Bugbot for commit a8d3b37. Bugbot is set up for automated code reviews on this repo. Configure here.