Hud Y rotation bug#688
Conversation
📝 WalkthroughWalkthroughThe PR adds an option to disable uniform-axis keyboard toggling, wires that option through gizmo setup, rewrites rotation drag updates to use a working Euler state, and renames the rotate and bounds helpers. ChangesAxis keyboard and gizmo rotation updates
Sequence Diagram(s)sequenceDiagram
participant "ui/gizmos.js" as gizmosJs
participant mesh
participant "physics target transforms" as physicsTransforms
participant "Blockly rotation block" as rotationBlock
gizmosJs->>mesh: rebuild quaternion from working Euler degrees
gizmosJs->>physicsTransforms: update target transform with the new quaternion
gizmosJs->>rotationBlock: setBlockAxisValue for changed axes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ui/gizmos.js (1)
817-825: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrim the historical rotation comment.
The implementation note is useful, but the “looked like no change / smeared every Euler component” history belongs in PR discussion rather than source.
As per coding guidelines, “Comments should reflect the current state of code only, keep discussion and historical notes in chat.”
Proposed cleanup
- // Track the rotation as Euler degrees (the block's own representation) rather - // than composing increments onto the quaternion and reading Euler back. A - // single-axis drag then changes only that axis's value, and the mesh is - // rebuilt with RotationYawPitchRoll — identical to what rotate_to applies — so - // the live view always matches the block. This also makes each axis a - // WORLD-axis rotation, like the drag arcs: rotating "Y" yaws a tilted mesh - // about the vertical, instead of spinning it about its own (local) axis, which - // on a shape symmetric about that axis (e.g. a capsule) looked like no change - // and smeared every Euler component across all three block values. + // Track rotation in the block's Euler-degree representation and rebuild via + // RotationYawPitchRoll so HUD/keyboard axis changes match rotate_to.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/gizmos.js` around lines 817 - 825, Trim the long rotation implementation note in gizmos.js so it only describes the current behavior of the rotation handling; keep the explanation around the Euler-degree tracking and RotationYawPitchRoll approach in the relevant gizmo rotation logic, but remove the historical “looked like no change / smeared every Euler component” discussion and other PR-style commentary from the comment near the rotation drag code.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/axis-keyboard.js`:
- Around line 15-17: The axis state can still be "all" even when allowUniform is
false, so keyboard navigation continues to affect all axes in non-uniform tools.
Update the axis handling in axis-keyboard.js to normalize any pre-existing "all"
value to a single permitted axis when uniform mode is disabled, including the
initialization path and the stop.setAxis("all") path, and make the Arrow/Page
key logic in the keyboard handler use that normalized axis rather than applying
all-axis movement.
---
Nitpick comments:
In `@ui/gizmos.js`:
- Around line 817-825: Trim the long rotation implementation note in gizmos.js
so it only describes the current behavior of the rotation handling; keep the
explanation around the Euler-degree tracking and RotationYawPitchRoll approach
in the relevant gizmo rotation logic, but remove the historical “looked like no
change / smeared every Euler component” discussion and other PR-style commentary
from the comment near the rotation drag code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 741572a1-0aa7-4885-a0cd-38f504970f8a
📒 Files selected for processing (2)
ui/axis-keyboard.jsui/gizmos.js
Summary
Fixes a bug where Y rotation didn't work properly using HUD - meshes should be Y rotated about the world axis in line with the drag handles.
Fixes a bug where U could be pressed for uniform on rotation/move gizmos, which makes no sense. If U mode is active when another gizmo is selected, axis reverts to X.
Summary by CodeRabbit
New Features
Bug Fixes