Godot: Android sensor_portrait not working

[vicguedez]

Godot version:
3.2.1-stable-official

OS/device including version:
GLES2 project - Samsung A30 - Android 10

Issue description:
Setting "sensor_portrait" orientation on project settings does not work at all. Using "sensor" it does rotate to all directions.

Steps to reproduce:
Any android porject and use "sensor_portrait".

Minimal reproduction project:

[HEAVYPOLY]

To add to this, "sensor" does rotate to all directions but does not respect the user's Android lock rotation setting (rotates even if locked)

According to developer.android.com, the "fullUser" setting seems to respect lock:
https://developer.android.com/guide/topics/manifest/activity-element

"fullUser" | If the user has locked sensor-based rotation, this behaves the same as user, otherwise it behaves the same as fullSensor and allows any of the 4 possible screen orientations. Added in API level 18.

Also, the ability to change resizeableActivity to true would be great for split screen and android desktop mode. (I don't think we have the ability currently in Android export options)

[m4gr3d]

The fix for this should be easy, but while looking into the bug, I found out there are two non-compatible ways to set the app orientation:

1. Project -> Project Settings -> Display -> Window -> Orientation as described in this issue.
2. Project -> Export -> Android -> Screen -> Orientation in the Android export preset window.

Option 1 uses Activity#setRequestedOrientation to set the orientation at runtime when the app starts.
Option 2 updates the AndroidManifest to set the orientation at build time, but this doesn't work as it's being overridden by option 1.

So going forward, I'll update the logic to use the process used by option 2 and update the AndroidManifest at build time to set the desired orientation.
The remaining question is which of the orientation selector interfaces should be removed? Are users expecting the orientation settings to be in the project settings, or in the export preset window?

cc @akien-mga @pouleyKetchoupp

[HEAVYPOLY]

I've been asking @pouleyKetchoupp for help on this, and we found a fix, using option 1.

I think option 1 would be more user friendly because then the iOS and android orientation setting would be unified (vs having to set it twice in 2 different places for iOS and android)

if it's possible I'd like to make this change (replacing sensor with fullUser on android), I've never contributed to a software project before!

Only this part:
case SCREEN_SENSOR: {
activity.setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_FULL_USER);

[m4gr3d]

@HEAVYPOLY Sounds good to me, and I'd be happy to help if you have questions!

Though the change will involve more than replacing sensor with fullUser on android. As I mentioned, you'll also need to update the logic to use the AndroidManifest. The logic that uses the Activity#setRequestedOrientation will be deprecated.

[pouleyKetchoupp]

Concerning the two settings for orientation:
Updating the android manifest instead of calling setOrientation at runtime seems like a good idea, but could we do that based on the orientation from project settings, and just get rid of the export option? They seem redundant, and it might be still confusing to have two different places to set the same thing, and only one actually used on Android.

For the sensor taking user choice into account:
I imagine this could be an extra option (like sensor-user), possibly documented as Android only, rather than replacing the current behavior, which might be still needed for some people.
If it makes more sense semantically, we could also make sensor take user settings into account, and add a new option force-sensor for the current behavior that forces all directions to be possible.

[m4gr3d]

@pouleyKetchoupp Yes seems like using orientation from project settings is the way to go, with the logic being updated to update the AndroidManifest at build time!

[HEAVYPOLY]

@pouleyKetchoupp I agree that "sensor" behavior should be fullUser to match iOS "sensor" behavior and a new option like "force-sensor" or "sensor-ignore-lock" would be added for the current android behavior.

[pouleyKetchoupp]

@HEAVYPOLY It sounds good to make it consistent with iOS (and the new option would fall back to sensor on iOS I guess). This would work well for 4.0. If it's backported to 3.2 though, maybe the current behavior needs to be kept on Android to avoid compatibility breakage (it's hard to decide what expected behavior is in this case).

[m4gr3d]

@pouleyKetchoupp I think we should be fine on the backward compatibility front. There would be issues if we're changing values from portrait to landscape (or vice versa). But given we're just expanding the range for the current settings, I don't expect any regression (i.e: if the app works fine on portrait/landscape, it should also work fine on userPortrait/userLandscape).

[pouleyKetchoupp]

@m4gr3d For backward compatibility I was thinking more about changing the current sensor option that would take user lock into account while it didn't before, but if you feel like it's fine then I don't have any strong objection to it.

[pouleyKetchoupp]

@m4gr3d : @HEAVYPOLY has made good progress on the PR, but after hitting some issues with updating the AndroidManifest in custom build and checking the code again, I wonder if it's the right direction to go because:

• We probably need to keep support for setScreenOrientation in java, because it allows changing the orientation at runtime from scripts on Android
• It seems the call to set_screen_orientation when godot starts is needed at least for ios to set things up properly
• If we're keeping all of this, we might as well not bother fixing the AndroidManifest file and instead just remove the android export settings

So my question is: could we make this change easier by just removing the orientation option in export settings, not bother fixing the orientation in the android manifest at all during export, and just keep setting it when the application starts? Or is there a specific reason why it needs to be done in the android manifest?

[HEAVYPOLY]

My first PR!

43248

[m4gr3d]

@m4gr3d : @HEAVYPOLY has made good progress on the PR, but after hitting some issues with updating the AndroidManifest in custom build and checking the code again, I wonder if it's the right direction to go because:

• We probably need to keep support for setScreenOrientation in java, because it allows changing the orientation at runtime from scripts on Android
• It seems the call to set_screen_orientation when godot starts is needed at least for ios to set things up properly
• If we're keeping all of this, we might as well not bother fixing the AndroidManifest file and instead just remove the android export settings

So my question is: could we make this change easier by just removing the orientation option in export settings, not bother fixing the orientation in the android manifest at all during export, and just keep setting it when the application starts? Or is there a specific reason why it needs to be done in the android manifest?

@HEAVYPOLY Nice job!!
@pouleyKetchoupp Good point! I thought the setScreenOrientation method was only used by the main startup logic and didn't realize it was also exposed and used by scripts.

I was leaning toward the AndroidManifest option for clarity and a (supposedly) slight latency reduction.

• According to the documentation for setRequestedOrientation, the method call causes the activity to (possibly) be restarted. So using the manifest option would remove that behavior.
• The manifest option would also be a bit clearer since that's typically where the orientation is set for Android apps. I worry about user (game developers not end users) confusion about either not finding the selected orientation there, or the manifest having a different orientation than what is shown at runtime. In addition certain stores (e.g: Google Play games) use the orientation attribute to allow for apps filtering.

I think a hybrid approach should address both need:

1. We go forward with @HEAVYPOLY solution and update the current sensor values in setScreenOrientation. In addition, before updating the orientation, we check if the current value matches the requested value using getRequestedOrientation. That will allow to prevent restarting the activity if the runtime orientation is the same as the AndroidManifest orientation.
2. We also update the AndroidManifest to match the runtime value for the orientation.

@HEAVYPOLY I can tackle the second task as I'll also be doing some cleanups to fix up some AAB issues in the export logic.