search

GodotVR / godot_openxr_vendors

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

Manually request eye tracking permission if it's included in the app manifest #71

Closed m4gr3d closed 7 months ago

m4gr3d commented 7 months ago

This is a follow-up of the changes made in https://github.com/godotengine/godot/pull/87080

m4gr3d commented 7 months ago

@BastiaanOlij @dsnopek This is ready for review.

@BastiaanOlij We'd want to release 2.0.3 with this for the Godot 4.3 dev 2 release.

m4gr3d commented 7 months ago

Thanks!

I tested it on the Quest 3. When I enable eye tracking, I get the expected log message, and when I don't enable it, I don't get that message. So, it seems to be more or less working.

However, I'm getting two different log messages about requesting this permission:

01-12 14:06:54.465 22862 22862 D PermissionsUtil: Requesting permission com.oculus.permission.EYE_TRACKING
01-12 14:06:54.573 22862 22862 D GodotOpenXRMeta: Requesting permission 'com.oculus.permission.EYE_TRACKING'

Is this intended? It seems like PermissionUtil.requestManifestPermission() may be requesting it before even getting to the new code added by this PR?

EDIT: Oh, nevermind, I get it! Godot 4.3-dev2 is removing the earlier code that requests the permission. I'll test again with that version in a moment.

EDIT 2: Hm, even with Godot 4.3-dev2, I'm still getting both log messages. I think I might just be misunderstanding what is supposed to be happening here?

@dsnopek I thought it was included, but looks like https://github.com/godotengine/godot/pull/87080 missed the dev2 cutout. So you'll need a master build to validate the change.

But even with a build that doesn't contain https://github.com/godotengine/godot/pull/87080, the change is safe. In that scenario as you noticed, the added logic will be a no-op given the permission would have already been requested.