GodotVR / godot_openxr_vendors

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

Add Meta configs for the eye gaze interaction extension #48

Closed m4gr3d closed 10 months ago

m4gr3d commented 11 months ago

The first commit is a small change to address and fix https://github.com/GodotVR/godot_openxr_loaders/issues/46

The second commit contains the majority of the changes and is the counterpart to https://github.com/godotengine/godot/pull/82614:

BastiaanOlij commented 11 months ago

Owh, I'm starting to understand the approach you were planning now. Makes a lot of sense to have a platform agnostic feature for the permission, that we then implement for each platform that supports it.

We might want to add stubs to the other platforms that implement the supportsFeature function and just returns false.

The only bit that I don't completely see through, when I'm looking at also adding support on PICO, is selecting the setting itself in the export interface. I'm assuming we'll have a PICO section where we select whether eye gaze is required on PICO and it adds the same feature tag?

BastiaanOlij commented 11 months ago

Owh I noticed in the PR on the Godot side the supportsFeature already has a base implementation and we're overriding it :)

m4gr3d commented 11 months ago

I'm assuming we'll have a PICO section where we select whether eye gaze is required on PICO and it adds the same feature tag?

Yeah I can't test so I skipped that section but the changes will be similar (if not identical to the Meta changes):

BastiaanOlij commented 11 months ago

I'm kind of in the same boat as my PICO 4 doesn't have eye tracking. But I'm hoping I can test the extension with a Varjo headset at least.

I might add the PICO changes as I think they're needed and sent a test build off to PICO. But I think we can approve this work as is after I get a chance to test it on my Quest. Hopefully the production team will merge the upstream change in Godot for 4.2

m4gr3d commented 11 months ago

I'm kind of in the same boat as my PICO 4 doesn't have eye tracking. But I'm hoping I can test the extension with a Varjo headset at least.

I might add the PICO changes as I think they're needed and sent a test build off to PICO. But I think we can approve this work as is after I get a chance to test it on my Quest. Hopefully the production team will merge the upstream change in Godot for 4.2

We can also use the gating logic to validate this no longer causes a crash on the Pico 4.

m4gr3d commented 11 months ago

@BastiaanOlij This can be merged now that https://github.com/godotengine/godot/pull/82614 has been merged.