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.

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

Oculus Hand Fix #14889

Closed

Conversation

@SaracenOne
Copy link
Contributor

commented Feb 9, 2019

Changes the default behavior of hands to continue to be tracked when in the Oculus Dash menu, as well as to not go into the away state automatically when cursor loses focus, allowing you users to visibly interact with the virtual desktops and Oculus keyboards. It also by default prevents you from going into an away state when the cursor loses focus from the High Fidelity window while in VR.

https://highfidelity.fogbugz.com/f/cases/21167/

@hifi-gustavo

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2019

Can someone from High Fidelity approve this PR?

@ZappoMan

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2019

@SaracenOne - thanks for this pull request this looks interesting. Can you help us out by describing a little bit more about the feature, and how you expect it to work so that our QA team can design test plans for testing it and keeping it working in the future.

@shanzzam - This looks like an interesting feature, can we add it to our 80 project list?

@SaracenOne

This comment has been minimized.

Copy link
Contributor Author

commented Feb 9, 2019

@ZappoMan This was primarily implemented based on the feedback of a friend who doesn't speak in VR, instead using the Oculus Dash to communicate. She really didn't like the way that bringing up the Oculus Dash would remove control from your High Fidelity hands, since she likes people to be able to see that she was typing and interacting virtual desktops; it felt disconnected otherwise. I also disabled the default functionality that clicking on anything other than the High Fidelity window in the virtual desktop would automatically force you into an away state, since this is a part of it. I would say that one thing which might be worth forcing you into an away state though would be if the High Fidelity window was minimised rather than just defocused.

Both the behaviours described here have been derived into configurable settings though, so they can still be set to behave as they already do.

@ZappoMan
Copy link
Contributor

left a comment

project approved

@ZappoMan ZappoMan added this to the v0.80.0 milestone Feb 11, 2019

@ZappoMan ZappoMan requested a review from jherico Feb 11, 2019

@@ -65,6 +65,8 @@ var eventMapping = Controller.newMapping(eventMappingName);
var avatarPosition = MyAvatar.position;
var wasHmdMounted = HMD.mounted;

var goAwayOnMouseCaptureLoss = Settings.getValue('goAwayOnMouseCaptureLoss', false);

This comment has been minimized.

Copy link
@ZappoMan

ZappoMan Feb 12, 2019

Contributor

please make this default to true keeping the current behavior. Thanks.

This comment has been minimized.

Copy link
@ZappoMan

ZappoMan Feb 12, 2019

Contributor

Also - you don't appear to add a UI to these settings, I think you should add a checkbox hooked up to these settings, the Dash one should be in the oculus device settings, while the other should be in general settings. Can you add that to this PR as well?

This comment has been minimized.

Copy link
@SaracenOne

SaracenOne Feb 13, 2019

Author Contributor

I could add a menu button for it, though I was conscious of what was said about not wanting to bloat the menus with too many controls, especially for such a minor change which it seemed would be a universal positive. My feeling was somewhat along the line of putting it in as is, but then seeing if anyone actually complains about the change, which I felt unlikely to be the case.

If you feel having a setting which can only be altered from the config file is a problem though, do you think it would be better to add a menu control for it or simply take out the settings variable check?

@shanzzam

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

@SaracenOne just so you are aware, I'm closing this in favor of another PR #15090 that took the work you did (in a slightly different method) and created the checkbox to turn this on and off. Thank you for your work and let me know if you have any questions! This will roll out in 80

@shanzzam shanzzam closed this Mar 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.