Make controllers for both Unity XR and WebVR #295

caseyyee · 2018-05-31T09:12:16Z

This PR adds the ability to map controller button and axis inputs to the appropriate Unity XR input and WebVR controller, depending on which context the project is running. Basically, it enables VR controllers to work between Unity and WebVR and fixes issue #233.

This involves change with how controllers work:

Removes WebVRManager and makes WebVRController a component of controller GameObject. (This binding was done auto-magically previously, this is way more straightforward and easier to understand and work with).

Handness is now defined on this component.
Component now deals with applying appropriate transforms for rotation and position (rather than user script).
Button mapping configuration defined using scriptable object asset (we'll get into how that's done down below)
Usage (as showcased in ControllerInteraction.cs example):

// Grab the WebVR controller component.
WebVRController controller = gameObject.GetComponent<WebVRController>();

// Check "Trigger" Input state.
if (controller.GetButtonDown("Trigger"))
{
    Debug.Log("Trigger is down!");
}

The Trigger action needs to be mapped to the appropriate Unity XR and WebVR controller inputs. To do this, we have a WebVRControllerInputSettings scriptable object that is used to create a configuration.

We bundle a basic configuration for Left and Right hand controllers: LeftControllerMap and RightControllerMap respectively. You can make your own using Assets > Create > WebVRControllerInputSettings:

For each action, we define a WebVR Controller button or axis ID and Unity Input definition. This is defined using Unity's Input Manager:

See Unity's XR Input docs https://docs.unity3d.com/Manual/OpenVRControllers.html to find the appropriate axis and button ID's to use. In this case, we want to use Axis1D.PrimaryIndexTrigger, which corresponds with joystick button 14.

caseyyee · 2018-05-31T09:18:43Z

Assets/WebVR/Scripts/ControllerInteraction.cs

    private FixedJoint attachJoint = null;
    private Rigidbody currentRigidBody = null;
    private List<Rigidbody> contactRigidBodies = new List<Rigidbody> ();

    void Awake()
    {
-        controllerManager = WebVRControllerManager.Instance;
+        //controllerManager = WebVRControllerManager.Instance;


oops, will remove this.

caseyyee · 2018-05-31T09:20:53Z

Assets/WebVR/Scripts/WebVRController.cs

    {
+        #if UNITY_EDITOR


actually, this might be better as !UNITY_WEBGL since XR can be used outside of UNITY_EDITOR as well.

yep, good call 👍 can you change this? as well as in GetButton?

can you add a comment explaining the #if/#else/#endif block?

caseyyee · 2018-05-31T09:21:35Z

Assets/WebVR/Scripts/WebVRController.cs

+
+    void Update()
+    {
+		#if UNITY_EDITOR


fix casing.

same as above, can you fix? and also can you add a comment explaining the conditional logic? thanks!

cvan

great work - refactor looking good!

on both master and on this branch, I tested the following:

Oculus Rift with Touch controllers in Unity's XR preview mode
Oculus Rift with Touch controllers in WebVR in Firefox for Windows
HTC Vive with controllers in Unity's XR preview mode
HTC Vive with controllers in WebVR in Firefox for Windows

in the Vive, my right-hand controller could not pick up objects. both hands appeared correctly, and I was able to pick up the interactable objects with my left hand. but my right hand did not work in Unity's XR preview mode. surprisingly or not, my right hand (and left hand) did work in WebVR in Firefox for Windows. (I'm not sure if this is an Oculus vs. OpenVR bug, or whether it's reproducible with Oculus as well. I encountered some problematic tracking issues with my Oculus Rift that seemed unrelated to your patch/this project, so I was unable to grab objects to test if Oculus is also affected.)

👍 fantastic progress here. this makes development easy breezy. I can't believe how simple it is now to develop whilst testing in VR and rapidly making adjustments.

cvan · 2018-06-05T02:30:01Z

Assets/WebVR/Scripts/WebVRController.cs

 {
+    [Tooltip("Map GameObject to controller hand name.")]


could you change this to Hand of controller to map to. (or something succincter, of course)?

cvan · 2018-06-05T02:30:37Z

Assets/WebVR/Scripts/WebVRController.cs

 {
+    [Tooltip("Map GameObject to controller hand name.")]
+    public WebVRControllerHand hand = WebVRControllerHand.NONE;
+    [Tooltip("Controller Input settings")]


nit: Controller input settings. (including the trailing period)

cvan · 2018-06-05T02:30:50Z

Assets/WebVR/Scripts/WebVRController.cs

    public int index;
-    public Enum hand;
+    [HideInInspector]


👍 good call

cvan · 2018-06-05T02:31:29Z

Assets/WebVR/Scripts/WebVRController.cs


-    private Dictionary<WebVRInputAction, bool[]> buttonStates = new Dictionary<WebVRInputAction, bool[]>();
+    private Dictionary<string, bool[]> buttonStates = new Dictionary<string, bool[]>();


👍 this is probably better to use native strings than the WebVRInputAction interface

cvan · 2018-06-05T02:33:09Z

Assets/WebVR/Scripts/WebVRController.cs

-                if (i == (int)action) {
-                    if (buttonStates.ContainsKey(action))
-                        buttonStates[action][0] = button.pressed;
+                if (input.gamepadId == i)


if a gamepad is removed and/or added, gamepadId can be guaranteed to reflect the same Gamepad object as the one that was initially detected? (I'm pretty sure Gecko and Chromium handle this properly, but I haven't checked all the implementations.)

Actually, I messed this one up. This should be gamepadButtonId, not gamepadId.

cvan · 2018-06-05T02:49:53Z

Assets/WebVR/Scripts/WebVRControllerInputSettings.cs

+
+[CreateAssetMenu(menuName = "WebVRControllerInputSettings")]
+public class WebVRControllerInputSettings : ScriptableObject {
+	[Header("WebVR Controller Input Settings")]


nit: do we need to say WebVR?

I just want to be clear since there are two different input systems in use here.

Following my former comments, we should change this to: WebVR Input Map.

Agree, WebVR Input Map is clearer.

cvan · 2018-06-05T02:50:19Z

Assets/WebVR/Scripts/WebVRControllerInputSettings.cs

+
+[System.Serializable]
+public class WebVRControllerInput {
+	public string actionName;


can you add [Tooltip(…)]s or helpful descriptions?

cvan · 2018-06-05T02:51:19Z

Assets/WebVR/Scripts/WebVRController.cs

+    }
+
+    private void onControllerUpdate(
+		int index, string h, Vector3 position, Quaternion rotation, Matrix4x4 sitStand, WebVRControllerButton[] b)


where is index used here?

cvan · 2018-06-05T02:52:40Z

Assets/WebVR/Scripts/WebVRController.cs

+    private void onControllerUpdate(
+		int index, string h, Vector3 position, Quaternion rotation, Matrix4x4 sitStand, WebVRControllerButton[] b)
+	{
+        // convert string to enum


would it be possible to just pass in the enum instead of passing and parsing the string on every call?

cvan · 2018-06-05T02:55:46Z

Assets/WebVR/Scripts/WebVRController.cs

+		int index, string h, Vector3 position, Quaternion rotation, Matrix4x4 sitStand, WebVRControllerButton[] b)
+	{
+        // convert string to enum
+        WebVRControllerHand updateHand;


wherever you decide to keep this string-to-enum parsing, could you do something like this to catch any possible enum-parsing errors?

WebVRControllerHand updateHand = WebVRControllerHand.NONE; if (!String.IsNullOrEmpty(h)) { try { updateHand = (WebVRControllerHand) Enum.Parse(typeof(WebVRControllerHand), h.ToUpper(), true); } catch { } }

update: it looks like you can use Enum.TryParse

TryParse doesn't work with Unity apparently: https://answers.unity.com/questions/830527/enumtryparse-method-not-support.html

caseyyee · 2018-06-08T18:47:11Z

@cvan ready for another review, fixed review issues + nits.

cvan

grabbing objects with the right controller now works now. 👍

great work! I tested with both Rift and Vive in both Unity's XR player and WebVR-exported builds in Firefox. ship it! 🚢

P.S. see my comment about the changed default camera position in non-VR mode. also, we should address the varying camera positions in Oculus and in Windows Mixed Reality.

cvan · 2018-06-08T21:33:00Z

Assets/WebVR/Prefabs/WebVRCameraSet.prefab

@@ -515,6 +515,7 @@ GameObject:
  - component: {fileID: 54027100335586508}


something in here caused the default camera position to be different (compare it to the position in master)

master

this branch

@cvan hm. not sure I understand. Only when entering VR?

regarding the default camera position. It's the transform that the browser supplies. Not sure if there is anything that we need to do specifically to accommodate different devices, but I suspect this may be an issue with WebVR with Edge. Will need to do some testing.

I will give it a check with the Samsung HMD.

okay, I just filed issue #303 to track this

okay, so I tried rearranging the order of the Virtual Reality SDKs in the XR Settings, but it seems to set the camera position below the desk when rendered via both Oculus and SteamVR.

rendered with Oculus SDK:

delapuente

Super nice job @caseyyee ! This is great, we should provide full mappings for not only the trigger and grip but including all the buttons but is a great enhancement.

Please, make the changes I've requested you and feel free to discuss. Although the ease of use is mainly provided by the default mappings, we should make super clear that the user understand what is she doing.

Another thing, there is a Input folder inside the script. Do you mind to move the controller-related scripts inside that folder?

delapuente · 2018-06-13T17:27:43Z

Assets/WebVR/Scripts/WebVRController.cs

@@ -81,13 +113,56 @@ public bool GetButtonUp(WebVRInputAction action)
        return isUp;
    }

-    public WebVRController(int index, Enum hand, Vector3 position, Quaternion rotation, Matrix4x4 sitStand)
+    private void onControllerUpdate(


Public names should be in UpperCamelCase.

It's internal. The public function is on the WebVRManager.

delapuente · 2018-06-13T17:29:04Z

Assets/WebVR/Scripts/WebVRController.cs

@@ -81,13 +113,56 @@ public bool GetButtonUp(WebVRInputAction action)
        return isUp;
    }

-    public WebVRController(int index, Enum hand, Vector3 position, Quaternion rotation, Matrix4x4 sitStand)
+    private void onControllerUpdate(
+        int index, string h, Vector3 position, Quaternion rotation, Matrix4x4 sitStand, WebVRControllerButton[] b)


What is h? Can you provide a more meaningful name here?

delapuente · 2018-06-13T17:29:59Z

Assets/WebVR/Scripts/WebVRController.cs

+    private void onControllerUpdate(
+        int index, string h, Vector3 position, Quaternion rotation, Matrix4x4 sitStand, WebVRControllerButton[] b)
+    {
+        // convert string to enum


Extract this into a meaningful function handFromString() and remove the comment.

delapuente · 2018-06-13T17:30:36Z

Assets/WebVR/Scripts/WebVRController.cs

+                handValue = (WebVRControllerHand) Enum.Parse(typeof(WebVRControllerHand), h.ToUpper(), true);
+            }
+            catch
+            {


At least, we should show a warning explaining what happened, hinting ourselves about how to solve the problem.

delapuente · 2018-06-13T17:32:24Z

Assets/WebVR/Scripts/WebVRController.cs

@@ -81,13 +113,56 @@ public bool GetButtonUp(WebVRInputAction action)
        return isUp;
    }

-    public WebVRController(int index, Enum hand, Vector3 position, Quaternion rotation, Matrix4x4 sitStand)
+    private void onControllerUpdate(
+        int index, string h, Vector3 position, Quaternion rotation, Matrix4x4 sitStand, WebVRControllerButton[] b)


Please, provide a meaningful name for the b parameter.

delapuente · 2018-06-13T18:10:43Z

Assets/WebVR/Scripts/WebVRControllerInputSettings.cs

+using UnityEngine;
+
+[CreateAssetMenu(menuName = "WebVRControllerInputSettings")]
+public class WebVRControllerInputSettings : ScriptableObject {


delapuente · 2018-06-13T18:11:10Z

Assets/WebVR/Scripts/WebVRControllerInputSettings.cs

+
+[CreateAssetMenu(menuName = "WebVRControllerInputSettings")]
+public class WebVRControllerInputSettings : ScriptableObject {
+	[Header("WebVR Controller Input Settings")]


Following my former comments, we should change this to: WebVR Input Map.

delapuente · 2018-06-13T18:13:38Z

Assets/WebVR/Scripts/WebVRControllerInputSettings.cs

+}
+
+[System.Serializable]
+public class WebVRControllerInput {


Following the naming from before, the former is the map containing individual mappings so this should be WebVRInputMapping. I'm not enforcing these names but try to make clear that these structures are mapping to different sources (the one from the Unity Input System and the one coming from the Web platform) to a common name. Clever idea and very nice from your side to provide some defaults but let make it super-clear.

delapuente · 2018-06-13T18:14:22Z

Assets/WebVR/Scripts/WebVRControllerInputSettings.cs

+
+[System.Serializable]
+public class WebVRControllerInput {
+	[Tooltip("Name of input action to be performed.")]


Since these are tooltips, I would say for this: A meaningful name describing the gesture performed on the controller.

delapuente · 2018-06-13T18:15:18Z

Assets/WebVR/Scripts/WebVRControllerInputSettings.cs

+public class WebVRControllerInput {
+	[Tooltip("Name of input action to be performed.")]
+	public string actionName;
+	[Tooltip("WebVR Gamepad button ID.")]


I'd rather say: Button ID coming from Web Gamepad API.

cvan · 2018-06-18T23:59:05Z

@caseyyee let me know when you've addressed @delapuente's review feedback (excellent comments, btw 👍). and I can take another look, test it, and we can merge this. let me know if you're stuck on anything or need a hand. otherwise, this looks very close to ready. great work, @caseyyee!

cvan

almost there!

two issues:

I cannot pick up objects anymore (seems to be a regression, but it seems fixed somewhat in your follow-up PR Cross platform controller support for Unity XR and WebVR. #306)
the starting camera position in the Rift (regardless of the order of OpenVR vs. Oculus in Unity's XR Settings) starts below the desk

looking good 👍 let me know what else you'd like me to test

cvan · 2018-06-25T21:12:36Z

Assets/WebVR/Prefabs/WebVRCameraSet.prefab

@@ -515,6 +515,7 @@ GameObject:
  - component: {fileID: 54027100335586508}


okay, so I tried rearranging the order of the Virtual Reality SDKs in the XR Settings, but it seems to set the camera position below the desk when rendered via both Oculus and SteamVR.

rendered with Oculus SDK:

caseyyee · 2018-06-26T23:40:55Z

Closing in favor of #306.

caseyyee force-pushed the buttons-editor branch from 8cf2134 to dd5a4cf Compare May 31, 2018 09:15

caseyyee requested a review from cvan May 31, 2018 09:16

caseyyee commented May 31, 2018

View reviewed changes

cvan assigned caseyyee Jun 5, 2018

cvan suggested changes Jun 5, 2018

View reviewed changes

cvan mentioned this pull request Jun 5, 2018

physics interactions do not work in XR Unity Editor preview mode #289

Closed

caseyyee force-pushed the buttons-editor branch 2 times, most recently from 2a70a6c to 20703e3 Compare June 8, 2018 18:46

cvan previously approved these changes Jun 8, 2018

View reviewed changes

cvan mentioned this pull request Jun 12, 2018

starting camera position is different in Unity XR player vs. WebVR export #303

Closed

cvan changed the title ~~Make controllers for both Unity XR and WebVR.~~ Make controllers for both Unity XR and WebVR Jun 12, 2018

delapuente suggested changes Jun 13, 2018

View reviewed changes

caseyyee added 6 commits June 20, 2018 17:11

simplify controller handling by eliminating ControllerManager

1529cba

Scriptable object for input mapping

d0b47f1

Adds input manager settings for XR controller inputs

1aff3ad

Adds Left and Right controller input maps

143b373

Updates prefab

37fdff0

Updates scene

d7c4c36

caseyyee dismissed cvan’s stale review via d7c4c36 June 21, 2018 00:18

caseyyee force-pushed the buttons-editor branch from 20703e3 to d7c4c36 Compare June 21, 2018 00:18

caseyyee mentioned this pull request Jun 22, 2018

Cross platform controller support for Unity XR and WebVR. #306

Merged

cvan suggested changes Jun 25, 2018

View reviewed changes

caseyyee closed this Jun 26, 2018


		private Dictionary<WebVRInputAction, bool[]> buttonStates = new Dictionary<WebVRInputAction, bool[]>();
		private Dictionary<string, bool[]> buttonStates = new Dictionary<string, bool[]>();

		@@ -515,6 +515,7 @@ GameObject:
		- component: {fileID: 54027100335586508}

Make controllers for both Unity XR and WebVR #295

Make controllers for both Unity XR and WebVR #295

Conversation

caseyyee commented May 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cvan left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caseyyee commented Jun 8, 2018

cvan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

master

this branch

Choose a reason for hiding this comment

caseyyee Jun 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

delapuente left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cvan commented Jun 18, 2018

cvan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caseyyee commented Jun 26, 2018

caseyyee commented May 31, 2018 •

edited

Loading

cvan left a comment •

edited

Loading

`master`

caseyyee Jun 9, 2018 •

edited

Loading