Skip to content

Conversation

@willeastcott
Copy link
Contributor

Adds an ESM script for managing XR sessions.

  • Starts/ends a WebXR session
  • Saves/restores camera state
  • Enables/disables skybox for AR sessions

Checklist

  • I have read the contributing guidelines
  • My code follows the project's coding standards
  • This PR focuses on a single change

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a WebXR session management script that provides a unified interface for starting/ending AR and VR sessions with automatic camera state preservation and environment adjustments.

  • Implements event-driven XR session control with configurable event names for AR, VR start/end operations
  • Adds camera transform state preservation and restoration when entering/exiting XR sessions
  • Includes AR-specific environment handling by disabling skybox and making camera background transparent

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +96 to +99
if (!this.cameraEntity.camera) {
console.error('XrSession: No cameraEntity.camera found on the entity.');
return;
}
Copy link

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

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

Potential null reference error. The code checks this.cameraEntity.camera but this.cameraEntity itself could be null if no camera component is found during initialization (line 44). This should check this.cameraEntity first before accessing its camera property.

Copilot uses AI. Check for mistakes.
}

endSession() {
if (!this.cameraEntity.camera) return;
Copy link

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

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

Same potential null reference issue as in startSession. Should check this.cameraEntity is not null before accessing its camera property.

Suggested change
if (!this.cameraEntity.camera) return;
if (!this.cameraEntity || !this.cameraEntity.camera) return;

Copilot uses AI. Check for mistakes.
if (this.app.xr.type === 'immersive-ar') {
// Make camera background transparent and hide the sky
this.clearColor.copy(this.cameraEntity.camera.clearColor);
this.cameraEntity.camera.clearColor = new Color(0, 0, 0, 0);
Copy link

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

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

Creating a new Color object on every XR session start is inefficient. Consider creating a static transparent color constant or reusing an instance variable to avoid repeated allocations.

Copilot uses AI. Check for mistakes.
@willeastcott willeastcott merged commit 6de5244 into main Sep 3, 2025
7 checks passed
@willeastcott willeastcott deleted the xr-session branch September 3, 2025 10:48
mvaligursky pushed a commit that referenced this pull request Sep 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants