Remove strict class type in FunctionPickup for more flexibility in how controllers are used

[goatchurchprime]

In trying to get my own networking avatar code to work with the base toolkit again, I hit a blocker in the over-requirement of the FunctionPickup feature to point to a genuine ARVRController object, rather than a node that has the methods and signals it needs to work (ie duck-typing).

It is necessary to attach the FunctionPickup feature to a node whose transform position is not directly set by the hardware controller in order to implement features such as (1) physics hands that don't go through walls, (2) heavy hands that droop when carrying a large object, and (3) to make hand tracking and controller tracking inputs interchangeable by mapping their transforms to the same GripArea.

[Malcolmnixon]

With Godot 4 strong-typing is significantly faster than duck-typing, which is why we have been switching the code-base over to strong types everywhere.

There are large portions of the code-base that assume when they call "get_arvr_controller" that they're really given an ARVRController because they're going to do many other things with them:

• drive the rumble property
• query the controller_id
• use get_is_active() to verify the controller is on
• use get_joystick_axis() to read the joystick

As such just using a duck-type check for the "button_pressed" signal is going to potentially result in all these other objects being given something that doesn't meet their expected API contract.

We have been planning on adding physics-blocked hands for a while. The approach we were considering was to have a version of the hands which did set_as_toplevel(true) then attempted to move to its parent using move_and_slide. The FunctionPickup could then be childed to the hand object, so it would follow the physics hand, but still find its ARVRController as an ancestor.

[goatchurchprime]

That comes to the same thing. We need to be able to tell the Movement functions to get their button inputs from one node (the ARVRController), and the transform position from a different Node.

Could we do this sneakily with the following remotr transform node:
ARVRController/Physics_blocked_hand/RemoteTransform -> ARVRController/FunctionPickup

Then the FunctionPickup position is being set from the RemoteTransform from another object, but it still looks to the ARVRController parent for its controls?

This, however, doesn't help with trying to use the same FunctionPickup node for both the Controller tracking and the Hand tracking, but maybe we don't want that. Also, I guess we can use disabled ARVRControllers in the enemies or other player avatars if we want to use the same FunctionPickup node to pick up objects with their appendages while preserving the strong typing of the variables.

[BastiaanOlij]

@goatchurchprime Malcolm is pointing to the "is_top_level" switch, it's hidden but there in Godot 3, it is now exposed as a tickbox in Godot 4. It allows a node to be positioned independent of it's parent node and there is thus no need for remote transforms or nodes in other trees as far as XRTools is concerned. Combine that with the changes we've been making that allow functions to be a child of a child of the controller node you have a solution.

So your XR tree would look like this:

- XROrigin3D
  - XRController3D
    - XRPhysicsHand (is_top_level = true)
      - XRPickupFunction

Now the physics hand attempts to follow the controller but can be stopped by a wall. The pickup function works on this location.
If you have an avatar managed by a separate tree you can obviously add remote transforms to this to make that work, but as far as the XRTools nodes are concerned, it's all context based.

This is an important thing to us, and to Godot in general. As you may remember we started out with many functions which had a number of node paths as properties but it adds a lot of complexity and makes the toolkit much harder to understand and use, especially when talking about optional functionality that isn't immediately clear to the user, is optional.

Simply consistently making a node work based on placing it in a logical position (i.e. as a child of the controller its related to) is far more natural, so we're pretty adamant we follow that design guideline as much as we can.

[BastiaanOlij]

This, however, doesn't help with trying to use the same FunctionPickup node for both the Controller tracking and the Hand tracking, but maybe we don't want that.

This makes less sense to me, using the same pickup node for both hands would make the code far more difficult to have it work sensible, especially as the pickup function also interacts with grab points to position objects when they are held.
Suddenly you need to add a lot of extra control logic so it can handle multiple held objects and track which hand is being used for what.
Having this context driven by having a pickup function for each hand childed to the hand it functions for, is a major simplification code wise.

Also, I guess we can use disabled ARVRControllers in the enemies or other player avatars if we want to use the same FunctionPickup node to pick up objects with their appendages while preserving the strong typing of the variables.

This one is more difficult, because it can differ very much from implementation to implementation. At face value I think you'll find you're trying to fit a square peg in a round hole. Enemy AI pickup logic functions very differently to player logic. There is some overlap in needed a detection area and a mounting point, but how the pickup action is trigger and handled is structurally different.
It would very likely be far easier to create a purpose build enemy AI pickup function then to try and repurpose the XR tools one for the job.

[goatchurchprime]

I've hit a snag. The hand tracking seems to be attached to a Spatial node with a OpenXRPose.gnds script attached to it, not a ARVRController node. This means we can't put a FunctionPickup under it. I think that was my motivation for making up a Spatial node with enough of the ARVRController's API to make the FunctionPickup feature work.

When ARVRController.controller_id=1 the node responds to the position of the Left Hand Controller and the left hand hand tracking -- but only when the palm of your hand is pointing away from your face.

The type ARVRController.controller_id=3 responds only when hands are being tracked and gives an orientation for all hand positions, and it can give thumb-and-finger pinching signals.

I can put a FunctionPickup node under an ARVRController.controller_id=3 and set pickup_axis_id to VR_SECONDARY_X_AXIS (which represents thumb-middle-finger pinch). This almost works, but for the clash caused by the ARVRController.controller_id=1 still operating, and the fact that it gives -1 for unpinched and +1 for pinched instead of a sensible range between 0 and 1, so can't be programmed with any hysteresis as done for the grip button regarding its grip_threshold.

I can overcome the erroneous active ARVRController controller nodes when hands are tracking with this:

	$FPController/LeftHandController/FunctionPickup.enabled = not $FPController/ARVRController3.get_is_active()
	$FPController/RightHandController/FunctionPickup.enabled = not $FPController/ARVRController4.get_is_active()

The place where the thumb-middle-pinch joystick value (which would preferably be a number between 0 and 1 instead of either -1 or +1) is set is this line:
https://github.com/GodotVR/godot_openxr/blob/a60519934f783d9153054a04cc3aa5e8a6c9b41f/src/openxr/extensions/xr_ext_hand_tracking_extension_wrapper.cpp#L366

The line which connects this value to the ARVRController.controller_id=3 node is here:
https://github.com/GodotVR/godot_openxr/blob/a60519934f783d9153054a04cc3aa5e8a6c9b41f/src/openxr/extensions/xr_ext_hand_tracking_extension_wrapper.cpp#L312 because HAND_CONTROLLER_ID_OFFSET=3.

The bad pinch strength number is from this class:
https://github.com/GodotVR/godot_openxr/blob/a60519934f783d9153054a04cc3aa5e8a6c9b41f/thirdparty/oculus_mobile_sdk/3rdParty/khronos/openxr/OpenXR-SDK/include/openxr/openxr.h#L3179

This is as far down that rabbit hole as I can go.

I think fundamentally we can't feed the raw signals from the controllers or the hand motions into this pickup (and hold) function. That's why it doesn't use the button_pressed signal to handle the grip, and instead runs this little cleanup and filtering algorithm:

https://github.com/GodotVR/godot-xr-tools/blob/master/addons/godot-xr-tools/functions/function_pickup.gd#L151

       # Handle our grip
	var grip_value = _controller.get_joystick_axis(pickup_axis_id)
	if (grip_pressed and grip_value < (_grip_threshold - 0.1)):
		grip_pressed = false
		_on_grip_release()
	elif (!grip_pressed and grip_value > (_grip_threshold + 0.1)):
		grip_pressed = true
		_on_grip_pressed()

If the hand tracking grip signal needs a different filtering algorithm (possibly calculating the grip value itself when the core library is giving the wrong values), then this function can't be reused.

[Malcolmnixon]

I'm going to have to familiarize myself with the hand-tracking logic (I personally hate using empty hands in VR). Godot-xr-tools is currently fairly strongly tied to controller input in order to read all the useful inputs (move/turn/strafe/grip/trigger/jump/sprint/etc.) and if the hand-tracking logic doesn't drive the controllers correctly then things will start to fail badly.

The -1/+1 grip-range sounds like an issue that was fixed for real controllers back in February of 2022 (GodotVR/godot_openxr#157) but apparently the hand-tracked input wasn't flagged as having the same problem. I would suggest filing a bug in godot_openxr for that issue.

I think it's also worth-while considering a new demo scene in godot-xr-tools-demo for testing hand-tracking. It may involve more work as most of the scenes won't function correctly with hand-tracking so we'd probably want the player blocked from entering them unless they're holding the real controllers.

[BastiaanOlij]

Controller ids 3 and 4 are Quest specific which is a PITA. They are how the hand aim extension on the Quest is exposed which is Metas alternative solution to hand tracking not being part of the action map system. I want to be very careful doing anything in XR tools that relies on it as any such solution isn't portable and I rather wait on an official solution in OpenXR that addresses that issue properly. This is also why this extension currently is not implemented in Godot 4.

That we're exposing the additional poses through a GDNS node in Godot 3 is also a PITA, this is solved properly in Godot 4. Here we're going to have to make a choice whether we put an interim solution in for Godot 3, or just accept this is a gap that won't be filled until Godot 4.

One solution that would possibly fit well in what we're trying to accomplish is that in the _ready function we can detect if our parent is an OpenXRPose node and if so, find the ARVRController with the same id.