Skip to content

nodes: stop tracking elevator dirty state - #599

Merged
Aymericr merged 2 commits into
pascalorg:mainfrom
JimmyZheng-ZJU:codex/elevator-dirty-tracking-main
Aug 17, 2026
Merged

nodes: stop tracking elevator dirty state#599
Aymericr merged 2 commits into
pascalorg:mainfrom
JimmyZheng-ZJU:codex/elevator-dirty-tracking-main

Conversation

@JimmyZheng-ZJU

@JimmyZheng-ZJU JimmyZheng-ZJU commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Stops renderer-driven elevator nodes from entering the shared dirtyNodes rebuild queue. Elevators have no def.geometry, capabilities.floorPlaced, or legacy dirty consumer, so scene-load and mutation marks were never cleared and the performance HUD remained at DIRTY 1 (1 elevator). The definition now uses the existing dirtyTracking: false contract, with a regression test covering both the declaration and scene-load-style markDirty behavior.

How to test

  1. Run bun test packages/core/src/store/use-scene-dirty-tracking.test.ts packages/core/src/store/use-scene-elevator-migration.test.ts packages/core/src/systems/elevator packages/nodes/src/elevator/definition.test.ts; expect 17 passing tests.
  2. Run bun test packages/nodes/src; expect 946 passing tests, 1 skipped, and 0 failures.
  3. Run bunx turbo run build --filter=@pascal-app/nodes... and bun run check; expect the Core, Viewer, and Nodes builds plus Biome checks to pass.

Screenshots / screen recording

N/A — internal dirty-queue scheduling fix; no editor UI or elevator geometry output changes.

Checklist

  • I've tested this locally with bun dev
  • My code follows the existing code style (run bun check to verify)
  • I've updated relevant documentation (if applicable)
  • This PR targets the main branch

Note

Low Risk
Narrow change to dirty-queue bookkeeping for one node kind; no auth, data, or elevator geometry/rendering logic changes.

Overview
Sets dirtyTracking: false on the elevator node definition so scene-load and mutation markDirty calls no longer leave elevators in the shared dirtyNodes queue.

Elevators have no geometry rebuild or other dirty consumer that clears those marks, which previously kept the perf HUD at DIRTY 1, blocked idle-frame detection (elevator def.system counts as pending work), and skewed post-processing scheduling. A new definition.test.ts asserts the flag and that registered markDirty does not retain the elevator id.

Reviewed by Cursor Bugbot for commit 39c59ac. Bugbot is set up for automated code reviews on this repo. Configure here.

@Aymericr Aymericr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed against the current architecture and tested on the PR head plus a local merge with current main. The definition correctly opts out a renderer-driven kind with no dirty consumer; focused dirty-tracking/elevator tests (7) and the nodes dependency typecheck pass. Ready to merge after the maintainer's local release smoke.

@Aymericr

Copy link
Copy Markdown
Contributor

Thank you, @JimmyZheng-ZJU. This was a small diff backed by a precise diagnosis: elevator nodes were entering the shared dirty queue without any geometry, floor-placement, or legacy consumer that could clear them. The regression test makes that ownership rule durable.

I completed the final architecture and current-main compatibility pass. The focused tests, package typecheck, and CI are green, and the change stays entirely inside the public node-definition contract—no hosted/private integration surface is involved.

Merging this now. Credit for the contribution remains with Jimmy-Zheng in the squash commit and PR history.

@Aymericr
Aymericr merged commit 27b2eb2 into pascalorg:main Aug 17, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants