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

SNAP App: support for Thumbstick of the right controller to capture picture and gif in HMD only. #1090

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AleziaKurdis
Copy link
Contributor

This PR adds the possibility to click on the thumbstick of the right controller to capture picture and gif, for HMD only.

It was a bit an irritant to capture picture and have to click on the tablet to do it. It is annoying for the posture if you are in the photo and it was quite a limitation to capture in movement.

It supports at least Vive and Oculus Touch.

This adds the possibility to click on the Thumbstick of the right controller to capture picture and gif, for HMD only.
Copy link
Member

@ksuprynowicz ksuprynowicz left a comment

Choose a reason for hiding this comment

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

Everything looks good :)

@ksuprynowicz ksuprynowicz added the CR approved This pull request has been successfully code reviewed label Jul 22, 2024
@ksuprynowicz
Copy link
Member

I think this PR is safe to merge

}
return;
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

There should be some sort of fallback for an unrecognized controller. At least log something, so that there's some clue as to what is wrong, but preferably an user-visible error message.

Also, the XBox controller has been used for VR, so it's also a possibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

3 things about it...
1- Fact: I simply bring the code from other camera app supporting the right thumb stick. This means that the same issue exists in those app too.

2- Xbox Controller: I don't have the hardware so I wouldn't be able to test it. I have honestly not consider support larger than that for that simple reason. If you have the "controllerType" string and which Controller.Standard.### it uses, then yes, I can add it.

3- Unrecognized controller: Currently, if a controller is not recognized, it simply do nothing. (No error, just not supported). The existence of this feature is not even written anywhere. This is not on the UI (a bit intentional). So people are currently find out accidentally or by learning from someone else. If a user has an unsupported controller, it won't tell him that the feature that he has never heard about before, is not working for him. He will be then as happy as all of us before this new implementation. In my opinion, It would be a bit mistake to advise directly the user that his controller is not taking change of this feature. It just cause the deception even before the need appears... It will just look clunkier as system for that user after that.
Instead, I would add a notice in the UI, but only if the controller is supporting it.

I didn't went there cause the cost in time was a bit too much for actually just a "shortcut".
Which means: changes in the Ui and what it needs between the app and the UI to hide/show the notice.
I can do it. Let me know what you think about this approach.

@JulianGro
Copy link
Member

JulianGro commented Sep 2, 2024

I tested this on desktop on Linux, both with and without a controller. Works fine in the sense that nothing breaks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR approved This pull request has been successfully code reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants