GodotVR / godot_openxr_vendors

Godot 4 wrapper for OpenXR vendors loaders and extensions
MIT License
90 stars 19 forks source link

Implement XR_FB_hand_tracking_aim extension #81

Closed devloglogan closed 5 months ago

devloglogan commented 7 months ago

This PR depends on Godot PR 87546

Adds extension wrapper for the XR_FB_hand_tracking_aim extension. The added singleton has methods to determine gesture states / pinch strength while hand tracking, and also provides a number of signals indicating when certain gestures begin/end.

Since this extension wrapper will only be supported by 4.3 onward, as well as the others listed in the above Godot PR, maybe it would make since to merge this into a 4.3 branch instead of master?

Also, one thing that might be worth discussing is how we should implement demos of these extension wrappers moving forward. Right now they're being dropped in main.tscn, but these are only meta extensions, so it doesn't really make sense to have them always present. Maybe we want to start thinking of implementing a menu to navigate implemented extensions and load corresponding demos?

dsnopek commented 7 months ago

Note: This will fail CI until the Godot PR is merged, and we're using godot-cpp with an updated extension_api.json for it

BastiaanOlij commented 6 months ago

XR_FB_hand_tracking_aim has the dubious honour of being one of the FB extensions that isn't following the design of OpenXR very well. It's basically being replaced by XR_EXT_hand_interaction (which Meta sadly isn't supporting yet but for which we have an open PR that can be merged once we have a runtime on which we can test it). Together with the additions to the touch profiles Meta recently did, that means that there is a clean switch in a player controlling the game with a controller, or switching over to handtracking, with the action map.

I'm not against support for hand tracking aim, but it should come with a big warning that it's an older solution that is not portable, and that soon better alternatives are possible through the hand interaction API.

For getting this PR over the line I'd like to see two changes:

  1. The feature needs to be gated behind a project setting. People need to opt in to this extension being activated. Its very likely that in the long term, many people will leave it off
  2. I wouldn't go for direct access to the extension. Instead I would prefer if the extension manages two new positional trackers so normal XRController3D nodes can be used but with hardcoded actions that bypass the actionmap (similar to what we do with Skeleton). So we simply create two new paths, say /user/fb_hand_aim/left and /user/fb_hand_aim/right that only has a default pause (which is the provided aim pose) and various actions like pinch_index, pinch_middle, etc.
BastiaanOlij commented 6 months ago

Note that for point 2, we probably need to make an amendment to core so OpenXRInterface::get_suggested_tracker_names calls into extension wrappers to ask for additional entries to return. We can then just have the 3 OpenXR paths returned, move the HTC entries into the HTC extension, and have this extension return the two new trackers. This will ensure they are selectable from the dropdown.

Similarly, but this should be for later as its a much bigger job and a more structural change, get_suggested_pose_names should be implemented to actually query the action map for poses mapped in the action map and call into extensions to return hardcoded action names.

m4gr3d commented 6 months ago

Since this extension wrapper will only be supported by 4.3 onward, as well as the others listed in the above Godot PR, maybe it would make since to merge this into a 4.3 branch instead of master?

We follow the approach in Godot core where master tracks the tip of the development branch, so once approved this change can be merged into the master branch. We'll need to update the godot-cpp submodule accordingly.

devloglogan commented 6 months ago

Updated and rebased! Also see godotengine/godot#88311 for changes to get_suggested_tracker_names()

m4gr3d commented 6 months ago

@devloglogan Can you bump the compatibility_minimum value in plugin.gdextension to 4.3.

devloglogan commented 6 months ago

Rebased and bumped the compatibility_minimum value.

devloglogan commented 5 months ago

Rebased! I believe all requested changes have been addressed?