GodotVR / godot_oculus_mobile

Godot Oculus mobile drivers (Oculus Go / Oculus Quest)
MIT License
169 stars 34 forks source link

Fix the controller vibration API #107

Closed m4gr3d closed 4 years ago

m4gr3d commented 4 years ago

The vrapi_SetHapticVibrationSimple(...) method is supposed to be called once per frame. To comply, the new logic batches the vibration requests (last one wins), and fire the method call once per frame during _process if a vibration request for the controller is available. The API exposed by the plugin is augmented to take a duration_in_ms parameter, which the logic uses to keep firing vrapi_SetHapticVibrationSimple(...) calls until the duration has been reached.

The controller demo has been updated to make the touch controllers vibrate when the A/X button is pressed.

NeoSpark314 commented 4 years ago

Later today I will be able to give this a test-run. Just a quick question. How does this new rumble API relate to the godot ARVRController.set_rumble(float) (https://docs.godotengine.org/en/stable/classes/class_arvrcontroller.html) that is implemented here https://github.com/GodotVR/godot_oculus_mobile/blob/88f7894400654541b1dfcf0b21536d78d0239874/plugin/src/main/cpp/ovr_mobile_controller.cpp#L43 ?

m4gr3d commented 4 years ago

Later today I will be able to give this a test-run. Just a quick question. How does this new rumble API relate to the godot ARVRController.set_rumble(float) (https://docs.godotengine.org/en/stable/classes/class_arvrcontroller.html) that is implemented here

https://github.com/GodotVR/godot_oculus_mobile/blob/88f7894400654541b1dfcf0b21536d78d0239874/plugin/src/main/cpp/ovr_mobile_controller.cpp#L43

?

It's a superset of that API. With this PR, there are now two ways to make the controllers vibrate:

Although I do see your point that now in GDScript there are two ways to do the same thing. I'll drop the GDScript API binding for this change, so that controller vibration is only done via the ARVRController node. I'll leave java/kotlin binding since that layer doesn't have access to the ARVRController node.