feat: Add VR HUD #367
feat: Add VR HUD #367
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
WalkthroughA new VR heads-up display (HUD) system is added to the VR rendering workflow. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant VRSystem
participant VRHud
participant VRCameraGroup
User->>VRSystem: Enable VR
VRSystem->>VRHud: Create VRHud instance
VRSystem->>VRCameraGroup: Attach VRHud
VRSystem->>VRHud: Set visible (true)
loop On each frame
VRSystem->>VRHud: update()
end
User->>VRSystem: End/Disable VR
VRSystem->>VRHud: Set visible (false)
VRSystem->>VRCameraGroup: Detach VRHud
VRSystem->>VRHud: dispose()
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
renderer/viewer/three/world/vrHud.ts (2)
87-87: Use modern Math.hypot API for velocity calculation.Math.hypot is more accurate and performant for calculating magnitudes.
- const vel = Math.sqrt(velocity.x ** 2 + velocity.z ** 2).toFixed(2) + const vel = Math.hypot(velocity.x, velocity.z).toFixed(2)🧰 Tools
🪛 ESLint
[error] 87-87: Prefer
Math.hypot(…)overMath.sqrt(…).(unicorn/prefer-modern-math-apis)
1-129: Fix formatting issues throughout the file.Multiple trailing spaces and missing spaces before function parentheses need to be addressed for code consistency.
The static analysis tool identified numerous formatting issues. Run your linter/formatter to address:
- Trailing spaces on multiple lines
- Missing spaces before function parentheses
- Missing newline at end of file
+🧰 Tools
🪛 ESLint
[error] 10-10: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 11-11: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
[error] 16-16: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 18-18: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 23-23: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 35-35: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 38-38: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 42-42: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 46-46: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 50-50: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 51-51: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
[error] 55-55: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 56-56: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
[error] 59-59: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 60-60: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
[error] 62-62: Use object destructuring.
(prefer-destructuring)
[error] 63-63: Use object destructuring.
(prefer-destructuring)
[error] 64-64: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 67-67: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 75-75: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 79-79: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 82-82: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 87-87: Prefer
Math.hypot(…)overMath.sqrt(…).(unicorn/prefer-modern-math-apis)
[error] 88-88: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 93-93: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 101-101: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 103-103: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 108-108: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 112-112: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 113-113: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
[error] 118-118: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 119-119: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
[error] 122-122: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 123-123: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
[error] 129-129: Newline required at end of file but not found.
(@stylistic/eol-last)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
renderer/viewer/three/world/vr.ts(4 hunks)renderer/viewer/three/world/vrHud.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
renderer/viewer/three/world/vr.ts (1)
renderer/viewer/three/world/vrHud.ts (1)
VRHud(4-129)
🪛 ESLint
renderer/viewer/three/world/vrHud.ts
[error] 10-10: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 11-11: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
[error] 16-16: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 18-18: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 23-23: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 35-35: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 38-38: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 42-42: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 46-46: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 50-50: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 51-51: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
[error] 55-55: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 56-56: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
[error] 59-59: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 60-60: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
[error] 62-62: Use object destructuring.
(prefer-destructuring)
[error] 63-63: Use object destructuring.
(prefer-destructuring)
[error] 64-64: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 67-67: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 75-75: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 79-79: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 82-82: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 87-87: Prefer Math.hypot(…) over Math.sqrt(…).
(unicorn/prefer-modern-math-apis)
[error] 88-88: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 93-93: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 101-101: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 103-103: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 108-108: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 112-112: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 113-113: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
[error] 118-118: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 119-119: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
[error] 122-122: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 123-123: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
[error] 129-129: Newline required at end of file but not found.
(@stylistic/eol-last)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-deploy
🔇 Additional comments (8)
renderer/viewer/three/world/vr.ts (7)
8-8: LGTM: Clean import addition.The VRHud import is properly added and will be used consistently throughout the file.
19-20: LGTM: VR HUD instantiation is well-placed.The HUD is created early in the VR initialization process, which allows it to be used throughout the VR session lifecycle.
44-46: LGTM: Proper HUD cleanup when VR is disabled.The HUD is correctly detached and hidden when VR is disabled, ensuring proper state management.
209-210: LGTM: HUD updated in the render loop.The HUD update is correctly placed in the animation loop, ensuring real-time information display during VR sessions.
216-218: LGTM: HUD properly attached when XR session starts.The HUD is correctly attached to the VR camera and made visible when the XR session begins.
222-224: LGTM: HUD properly detached when XR session ends.The HUD is correctly detached and hidden when the XR session ends, maintaining proper state consistency.
227-230: LGTM: Proper resource cleanup on abort.The HUD disposal is correctly handled when the abort signal is triggered, preventing memory leaks.
renderer/viewer/three/world/vrHud.ts (1)
123-128:❓ Verification inconclusive
Verify complete resource cleanup in dispose method.
Ensure all Three.js resources are properly disposed to prevent memory leaks.
The dispose method looks comprehensive, but let's verify that all resources are properly cleaned up:
🏁 Script executed:
#!/bin/bash # Description: Check Three.js disposal patterns in the codebase for completeness # Search for other disposal patterns to ensure consistency ast-grep --pattern $'dispose() { $$$ }' # Check for canvas cleanup patterns rg -A 5 "canvas.*remove"Length of output: 1084
#!/bin/bash # Locate the vrHud.ts file and print its key sections for review file=$(fd vrHud.ts) echo "Inspecting file: $file" echo -e "\n--- Class Declaration & Constructor ---" sed -n '1,80p' "$file" echo -e "\n--- Dispose Method Snippet ---" sed -n '100,150p' "$file"
#!/bin/bash # Locate vrHud.ts and inspect resource setup & disposal, plus any event listeners file=$(fd --basename vrHud.ts) echo "Inspecting file: $file" echo -e "\n--- Full dispose() method ---" sed -n '115,140p' "$file" echo -e "\n--- Search for event listeners on hudCanvas ---" rg -n "hudCanvas.*addEventListener" -A2 "$file" echo -e "\n--- hudCanvas creation & usage ---" grep -R "hudCanvas" -n "$file"
🏁 Script executed:
#!/bin/bash # Locate and inspect vrHud.ts for event listeners and animation loops file=$(fd --exact-name vrHud.ts) echo "File located at: $file" echo -e "\n--- dispose() method context (lines 100–160) ---" sed -n '100,160p' "$file" echo -e "\n--- Event listener registrations/removals ---" rg -n 'addEventListener|removeEventListener' "$file" echo -e "\n--- Animation loop (requestAnimationFrame/cancelAnimationFrame) usage ---" rg -n 'requestAnimationFrame|cancelAnimationFrame' "$file"Length of output: 928
Verify event listeners & animation-loop cleanup in vrHud.dispose
Although you’re disposing the texture, geometry, material, and removing the canvas element, please confirm that any of the following are also cleaned up to avoid leaks:• Cancelling any running requestAnimationFrame loops
• Clearing any setInterval/setTimeout timers
• Removing all hudCanvas (or window/document) event listeners
• Disposing of any additional render targets or helper objects created by vrHud🧰 Tools
🪛 ESLint
[error] 123-123: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
| const health = bot?.health || 10 | ||
| const maxHealth = 20 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make health configuration dynamic.
The hard-coded maxHealth = 20 should be retrieved from the game state or configuration.
- const health = bot?.health || 10
- const maxHealth = 20
+ const health = bot?.health || 0
+ const maxHealth = bot?.maxHealth || 20📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const health = bot?.health || 10 | |
| const maxHealth = 20 | |
| const health = bot?.health || 0 | |
| const maxHealth = bot?.maxHealth || 20 |
🤖 Prompt for AI Agents
In renderer/viewer/three/world/vrHud.ts around lines 97 to 98, replace the
hard-coded maxHealth value of 20 with a dynamic value fetched from the game
state or configuration. Identify where the game state or configuration stores
the maximum health value and update the code to retrieve maxHealth from that
source instead of using a fixed number.
| const bot = (window as any).bot | ||
| const playerState = this.worldRenderer.playerState |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety for global bot access.
Accessing (window as any).bot bypasses type safety and could lead to runtime errors if bot is undefined or doesn't have expected properties.
- const bot = (window as any).bot
- const playerState = this.worldRenderer.playerState
+ const bot = (globalThis as any).bot
+ const playerState = this.worldRenderer.playerState
+
+ // Add safety checks for bot existence
+ if (!bot) {
+ return // Skip update if bot is not available
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const bot = (window as any).bot | |
| const playerState = this.worldRenderer.playerState | |
| const bot = (globalThis as any).bot | |
| const playerState = this.worldRenderer.playerState | |
| // Add safety checks for bot existence | |
| if (!bot) { | |
| return // Skip update if bot is not available | |
| } |
🧰 Tools
🪛 ESLint
[error] 62-62: Use object destructuring.
(prefer-destructuring)
[error] 63-63: Use object destructuring.
(prefer-destructuring)
🤖 Prompt for AI Agents
In renderer/viewer/three/world/vrHud.ts around lines 62 to 63, the code accesses
the global bot object using (window as any).bot, which bypasses type safety and
risks runtime errors if bot is undefined or malformed. To fix this, define a
proper TypeScript interface for the bot object with expected properties, extend
the Window interface to include this bot type, and then access bot via
window.bot with type safety. Additionally, add a runtime check to ensure bot is
defined before using it to prevent potential errors.
| constructor(private worldRenderer: WorldRendererThree) { | ||
| // Create canvas for HUD | ||
| this.hudCanvas = document.createElement('canvas') | ||
| this.hudCanvas.width = 1024 | ||
| this.hudCanvas.height = 512 | ||
|
|
||
| this.hudContext = this.hudCanvas.getContext('2d')! |
There was a problem hiding this comment.
Add error handling for canvas context creation.
The non-null assertion on getContext('2d')! could fail if the context cannot be created, leading to runtime errors.
- this.hudContext = this.hudCanvas.getContext('2d')!
+ const context = this.hudCanvas.getContext('2d')
+ if (!context) {
+ throw new Error('Failed to create 2D canvas context for VR HUD')
+ }
+ this.hudContext = context📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| constructor(private worldRenderer: WorldRendererThree) { | |
| // Create canvas for HUD | |
| this.hudCanvas = document.createElement('canvas') | |
| this.hudCanvas.width = 1024 | |
| this.hudCanvas.height = 512 | |
| this.hudContext = this.hudCanvas.getContext('2d')! | |
| constructor(private worldRenderer: WorldRendererThree) { | |
| // Create canvas for HUD | |
| this.hudCanvas = document.createElement('canvas') | |
| this.hudCanvas.width = 1024 | |
| this.hudCanvas.height = 512 | |
| const context = this.hudCanvas.getContext('2d') | |
| if (!context) { | |
| throw new Error('Failed to create 2D canvas context for VR HUD') | |
| } | |
| this.hudContext = context |
🧰 Tools
🪛 ESLint
[error] 11-11: Missing space before function parentheses.
(@stylistic/space-before-function-paren)
[error] 16-16: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
🤖 Prompt for AI Agents
In renderer/viewer/three/world/vrHud.ts around lines 11 to 17, the code uses a
non-null assertion when getting the 2D context from the canvas, which can cause
runtime errors if the context creation fails. Modify the constructor to check if
the context is null after calling getContext('2d'), and handle the error
appropriately, such as throwing an error or logging a message, to prevent
unexpected failures.
| const fps = Math.round(1000 / this.worldRenderer.renderTimeAvg) || 0 | ||
| const ping = bot?._client?.latency || 0 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add fallback handling for undefined renderTimeAvg.
The FPS calculation could fail if renderTimeAvg is zero or undefined, leading to division by zero or NaN values.
- const fps = Math.round(1000 / this.worldRenderer.renderTimeAvg) || 0
+ const fps = this.worldRenderer.renderTimeAvg > 0
+ ? Math.round(1000 / this.worldRenderer.renderTimeAvg)
+ : 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fps = Math.round(1000 / this.worldRenderer.renderTimeAvg) || 0 | |
| const ping = bot?._client?.latency || 0 | |
| const fps = this.worldRenderer.renderTimeAvg > 0 | |
| ? Math.round(1000 / this.worldRenderer.renderTimeAvg) | |
| : 0 | |
| const ping = bot?._client?.latency || 0 |
🤖 Prompt for AI Agents
In renderer/viewer/three/world/vrHud.ts around lines 77 to 78, the FPS
calculation does not handle cases where renderTimeAvg is zero or undefined,
which can cause division by zero or result in NaN. Add a fallback check to
ensure renderTimeAvg is a positive number before performing the division; if it
is zero or undefined, set FPS to zero to avoid invalid calculations.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes