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

Ensure OpenXR classes are declared properly #81037

Merged
merged 1 commit into from
Aug 28, 2023

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Aug 27, 2023

@akien-mga
Copy link
Member

Copying my question from the other PR:

@BastiaanOlij Since we're exposing this class properly now, it's worth thinking about the name. We use "metadata" as a single word throughout the Godot API, and this one is the only one using "meta data" (MetaData in the class name).

Is this a name derived from the OpenXR API, or something we chose ourselves? If the latter, it could be worth standardizing on Metadata / "metadata".

@BastiaanOlij
Copy link
Contributor

@BastiaanOlij Since we're exposing this class properly now, it's worth thinking about the name. We use "metadata" as a single word throughout the Godot API, and this one is the only one using "meta data" (MetaData in the class name).

@akien-mga nope, thats just me not knowing we wrote that as one word :) I'm more than happy with that change.

Copy link
Contributor

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

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

lgtm, just akiens requested change to make meta data -> metadata

@AThousandShips
Copy link
Member Author

Done! Renamed the file in question, to match the spelling

@akien-mga
Copy link
Member

akien-mga commented Aug 28, 2023

There's a use of "meta data" in the doc file, which would be worth changing too. Actually, there's a few more, most in OpenXR code, which we could change:

doc/classes/OpenXRInteractionProfileMetadata.xml:		This class allows OpenXR core and extensions to register meta data relating to supported interaction devices such as controllers, trackers, haptic devices, etc. It is primarily used by the action map editor and to sanitize any action map by removing extension-dependent entries when applicable.
modules/openxr/doc_classes/OpenXRInteractionProfile.xml:		This object stores suggested bindings for an interaction profile. Interaction profiles define the meta data for a tracked XR device such as an XR controller.
modules/openxr/openxr_api.cpp:	// If the io_path is not part of our meta data we've likely got a misspelled name or a bad action map, report
servers/xr/xr_interface.h:	enum Capabilities { /* purely meta data, provides some info about what this interface supports */
tests/scene/test_arraymesh.h:TEST_CASE("[SceneTree][ArrayMesh] Surface meta data tests.") {

(the .po file shouldn't be changed)

And finally, I also noticed that the doc file is in the wrong folder. It should be moved to the module's doc_classes (and for this to work, it should be listed in modules/openxr/config.py).

Edit: Actually, those three should move:

OpenXRAPIExtension.xml                OpenXRExtensionWrapperExtension.xml   OpenXRInteractionProfileMetadata.xml

scope_creep++

@AThousandShips
Copy link
Member Author

Got it!

@AThousandShips
Copy link
Member Author

I believe that's all now

Co-authored-by: Bastiaan Olij <mux213@gmail.com>
@akien-mga akien-mga merged commit 75bc686 into godotengine:master Aug 28, 2023
15 checks passed
@AThousandShips AThousandShips deleted the openxr_register branch August 28, 2023 10:15
@akien-mga
Copy link
Member

Thanks!

@AThousandShips
Copy link
Member Author

Thank you!

@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Sep 20, 2023
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.2.

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