Skip to content

Conversation

@keveleigh
Copy link
Contributor

Overview

This PR updates the rigged hands logic to not display when a transparent display is used, like over remoting. We'd been getting reports of rigged hands not aligning well with the physical hands and causing confusion.

This also fixes some potential null refs when the rigged hand material isn't set.
This also simplifies one of the controller visualization profiles that wasn't using "both" handedness for certain entries.
This also includes some like formatting and style updates for simplification.

@keveleigh keveleigh requested a review from RogPodge December 3, 2022 02:00
@keveleigh keveleigh self-assigned this Dec 3, 2022
@RogPodge
Copy link
Contributor

RogPodge commented Dec 5, 2022

Noticed we're still carrying around an "ObsoleteXRSDKProfile". Is this something we think we can just remove? Otherwise I'm not sure we should be changing any properties on that profile.

@MaxWang-MS
Copy link
Contributor

I am not fan of removing profiles in a patch release though.

@keveleigh
Copy link
Contributor Author

I am not fan of removing profiles in a patch release though.

Agreed. It could be a big breaking change if people currently have it assigned unless we introduce a migration path.

Otherwise I'm not sure we should be changing any properties on that profile.

I'm fine reverting the changes here. Mostly, one of the paths was referencing an also-marked-obsolete hand joint prefab and there were duplicate entries, but if we think that's risky I'm happy to revert!

@RogPodge
Copy link
Contributor

RogPodge commented Dec 5, 2022

No super strong opinions, I'll leave it to your judgement. I just wanted to raise the possibility of extra breaking changes.

@keveleigh keveleigh merged commit 7503ecc into microsoft:releases/2.8.3 Dec 5, 2022
@keveleigh keveleigh deleted the fix-rigged-hands-in-remoting branch December 5, 2022 23:14
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.

3 participants