Samsung / GearVRf

The GearVR framework(GearVRf) is an Open Source VR rendering library for application development on VR-supported Android devices.
http://www.gearvrf.org
Apache License 2.0
407 stars 217 forks source link

OvrVrapiActivityHandler: onBack disabled if home button is present #1194

Closed brian-faber closed 7 years ago

brian-faber commented 7 years ago

Hi, Why is onBack disabled with the presence of a home button on a GearVR headset? I am wondering because this is against the oculus requirements.

https://developer3.oculus.com/documentation/publish/latest/concepts/publish-mobile-req/ "Meets all reserved interaction requirements:

brian-faber commented 7 years ago

Hi, Any update on this as I am close to submission and want to validate if there is a need for why it needs to be there.

thanks,

liaxim commented 7 years ago

Hi. Will investigate today.

liaxim commented 7 years ago

@brian-faber I guess it was our interpretation of the presence of the new home key. Hasn't been brought up before but you may very well be correct. The apps from the store I tried were all consistent with the wording you quoted.

Eliminating the check is easy. Are you using a certain version of the framework or the latest development snapshot?

brian-faber commented 7 years ago

Hi, Thanks for the reply. We are using the v3.2 release branch. I'll just have to build a local aar to use instead of the maven release.

liaxim commented 7 years ago

@brian-faber Attaching patched 3.2 release. framework-aars.zip

This is the diff:

diff --git a/GVRf/Framework/backend_oculus/src/main/java/org/gearvrf/OvrVrapiActivityHandler.java b/GVRf/Framework/backend_oculus/src/main/java/org/gearvrf/OvrVrapiActivityHandler.java
index 250857bd9..0f9c265c3 100644
--- a/GVRf/Framework/backend_oculus/src/main/java/org/gearvrf/OvrVrapiActivityHandler.java
+++ b/GVRf/Framework/backend_oculus/src/main/java/org/gearvrf/OvrVrapiActivityHandler.java
@@ -126,9 +126,7 @@ class OvrVrapiActivityHandler implements OvrActivityHandler {

     @Override
     public boolean onBack() {
-        if (!mActivity.getConfigurationManager().isHomeKeyPresent()) {
-            nativeShowConfirmQuit(mPtr);
-        }
+        nativeShowConfirmQuit(mPtr);
         return true;
     }

Unless I hear something else will include the change in the next release.

brian-faber commented 7 years ago

Thanks for the zip.

brian-faber commented 7 years ago

Hi @liaxim I noticed the fix is no longer in master or 3.2, is there a reason why it was put back?

liaxim commented 7 years ago

@brian-faber Created a pull request for master. Will be merged in few days and will become part of the 4.0 release. We never patched up 3.2. We would need a new release (3.2.2 I guess).

liaxim commented 7 years ago

@brian-faber Fix merged to master and will be part of the next release. Can we close the issue now?

brian-faber commented 7 years ago

Sorry for delay in seeing this. Yes we can close this again.