godot_openxr
godot_openxr copied to clipboard
Fix the logic to retrieve and set the play space type
Fix for some of the logic errors in https://github.com/GodotVR/godot_openxr/pull/212
The reason we have that case is because the enums in OpenXR don't run sequentially. So as new types are added in OpenXR that we want to support, we would return different values for different OpenXR types as the property dropdown enums in Godot have to be sequential.
Say we want to support XR_REFERENCE_SPACE_TYPE_COMBINED_EYE_VARJO, that for us may become enum 3, not 1000121000.
This code is just a prelude to that conversion so that whomever makes the changes for new types we want to support, has a blueprint for adding that conversion.
The current logic returns 1 for LOCAL and 2 for STAGE but takes in 2 for LOCAL and 3 for STAGE:
// play space is STAGE
int type = get_play_space_type(); // == 2
set_play_space_type(type); // play space is now LOCAL when it should be STAGE
Ah! I see now, what a dumb mistake of mine, so the fix should just be that case 2 -> case 1, and case 3 -> case 2 in set_play_space_type
Right now its just "luck" that the values match the first 3 entries for the OpenXR constants.
When you look at: https://github.com/GodotVR/godot_openxr/blob/master/src/gdclasses/OpenXRConfig.cpp#L32
register_property<OpenXRConfig, int>("play_space_type", &OpenXRConfig::set_play_space_type, &OpenXRConfig::get_play_space_type, 2, GODOT_METHOD_RPC_MODE_DISABLED, GODOT_PROPERTY_USAGE_DEFAULT, GODOT_PROPERTY_HINT_ENUM, "View,Local,Stage"); // we don't support XR_REFERENCE_SPACE_TYPE_UNBOUNDED_MSFT and XR_REFERENCE_SPACE_TYPE_COMBINED_EYE_VARJO at this time.
You can see the enum defined for properties. In Godot core we would actually define a proper enum for this and type the return value and parameter for the getter and setter properly, but GDNative is still a bit iffy here.
I've fixed the issue for now by just addressing the problem so I can get a build out, but feel free to rebase this and we can have a further discussion of whether we want to simplify this so where the values in Godot match those of OpenXR constants we don't do specific cases.