Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add XR tracking state-change signals #81239

Merged
merged 1 commit into from
Sep 4, 2023
Merged

Add XR tracking state-change signals #81239

merged 1 commit into from
Sep 4, 2023

Conversation

Malcolmnixon
Copy link
Contributor

This pull request adds signals to detect the start and stop of tracking for XR Trackers. Specifically it:

  • Adds a pose_lost_tracking(pose : XRPose) signal to XRPositionalTracker
  • Adds a tracking_changed(tracking : bool) signal to XRNode3D

@Malcolmnixon Malcolmnixon requested review from a team as code owners September 1, 2023 23:16
@akien-mga akien-mga added this to the 4.x milestone Sep 2, 2023
dsnopek
dsnopek reviewed Sep 2, 2023
View reviewed changes
Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Thanks! Overall, I think this is a great idea. I skimmed the code and for the most part it looks good, but I didn't actually test it.

scene/3d/xr_nodes.cpp Show resolved Hide resolved
BastiaanOlij
BastiaanOlij approved these changes Sep 3, 2023
View reviewed changes
Copy link
Contributor

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

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

This looks good, indeed makes it easier to react to tracking changes.

I'll leave it up to you whether you want to simplify the bind/unbind logic, I think you can keep it simple, esp in OpenXR trackers will exist from the start and be bound for the lifetime of the game (/scene).

@akien-mga akien-mga modified the milestones: 4.x, 4.2 Sep 3, 2023
@Malcolmnixon
Added XR tracking-state change signals
22de1b3 
Add "pose_lost_tracking" signal to XRPositionalTracker.
Add "tracking_changed" signal to XRNode3D.
AThousandShips
AThousandShips approved these changes Sep 3, 2023
View reviewed changes
Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

Looks good style wise

@Malcolmnixon Malcolmnixon requested a review from dsnopek September 3, 2023 16:22
dsnopek
dsnopek approved these changes Sep 3, 2023
View reviewed changes
Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Looks great to me! :-)

@akien-mga akien-mga merged commit f05c7f6 into godotengine:master Sep 4, 2023
@akien-mga
Copy link
Member

Thanks!

@Malcolmnixon Malcolmnixon mentioned this pull request Sep 6, 2023
Open
@Malcolmnixon Malcolmnixon deleted the xr-tracking-signals branch September 6, 2023 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AThousandShips AThousandShips AThousandShips approved these changes

@BastiaanOlij BastiaanOlij BastiaanOlij approved these changes

@dsnopek dsnopek dsnopek approved these changes

Assignees
No one assigned
Labels
enhancement topic:xr
Projects
None yet
Milestone
4.2
Development

Successfully merging this pull request may close these issues.
5 participants
@Malcolmnixon @akien-mga @dsnopek @BastiaanOlij @AThousandShips