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

Clarify XRController3D get_input name corresponds to actions in the action set #104103

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lodetrick
Copy link

Clarify that the parameter name for the methods get_input, get_float, get_vector2, and is_button_pressed corresponds to the internal name of an action in the current action map. Current docs call it the name of an input.

@lodetrick lodetrick requested a review from a team as a code owner March 14, 2025 05:47
@lodetrick lodetrick changed the title Clarify XRController3D get_input name corresponds to actions in the action set Clarify XRController3D get_input name corresponds to actions in the action set Mar 14, 2025
@dsnopek
Copy link
Contributor

dsnopek commented Mar 14, 2025

Thanks!

However, this is only true for OpenXR. With WebXR or any other interface from GDExtension, there is no action map. So, if we add something like this, it should specifically mention OpenXR

@AThousandShips AThousandShips added this to the 4.x milestone Mar 14, 2025
@dsnopek dsnopek requested a review from a team March 14, 2025 13:16
@lodetrick
Copy link
Author

For the other interfaces, is there any standard wording for what name the input is? The current documentation feels a bit vague and unclear, especially for OpenXR (which appears to be the most supported XR framework currently).

@dsnopek
Copy link
Contributor

dsnopek commented Mar 14, 2025

For the other interfaces, is there any standard wording for what name the input is?

The names are defined by the XRInterface, and that's all we can really say in general.

We could maybe write something like "The name of the input is defined by the [XRInterface]. In the case of OpenXR, these are the names of actions for the current action set."

We could conceivably also link to some docs on the names of the WebXR inputs, but I don't think we have any yet. :-)

@lodetrick
Copy link
Author

lodetrick commented Mar 15, 2025

I wonder if the clarification could be made in the class description, and each method just says to refer to that, rather than having the same description four times. The class description already mentions that inputs are named, so it could just expand upon that. Is there a convention for that sort of thing or should the docs avoid that kind of indirection?

@dsnopek
Copy link
Contributor

dsnopek commented Mar 17, 2025

Sure, we could add it to the class description - it does match the existing note about "a configurable action map"

@lodetrick lodetrick force-pushed the xr-controller-docs branch from 9f3e0c6 to 3e23989 Compare March 18, 2025 00:57
@@ -7,7 +7,7 @@
This is a helper 3D node that is linked to the tracking of controllers. It also offers several handy passthroughs to the state of buttons and such on the controllers.
Controllers are linked by their ID. You can create controller nodes before the controllers are available. If your game always uses two controllers (one for each hand), you can predefine the controllers with ID 1 and 2; they will become active as soon as the controllers are identified. If you expect additional controllers to be used, you should react to the signals and add XRController3D nodes to your scene.
The position of the controller node is automatically updated by the [XRServer]. This makes this node ideal to add child nodes to visualize the controller.
As many XR runtimes now use a configurable action map all inputs are named.
As many XR runtimes now use a configurable action map all inputs are named. The name of the input is defined by the current [XRInterface]. In the case of OpenXR, these are the names of actions in the current action set.
Copy link
Contributor

@dsnopek dsnopek Mar 19, 2025

Choose a reason for hiding this comment

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

I wonder if we should just replace the old sentence, since the new stuff basically says the same thing?

With a little elaboration:

Suggested change
As many XR runtimes now use a configurable action map all inputs are named. The name of the input is defined by the current [XRInterface]. In the case of OpenXR, these are the names of actions in the current action set.
The names of inputs are defined by the current [XRInterface]. In the case of OpenXR, these are the names of actions in the current action set from the OpenXR action map.

Copy link
Author

Choose a reason for hiding this comment

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

I tried to combine the two sentences, because just having "The names of inputs are defined by the current [XRInterface]" felt a bit off, perhaps because it was in passive voice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants