godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.79k stars 20.13k forks source link

`OS.request_permission` is now non-blocking #91194

Open scgm0 opened 4 months ago

scgm0 commented 4 months ago

Tested versions

v4.3.dev.mono.custom_build [00829d37f]

System information

HarmonyOS 4.0.0

Issue description

OS.request_permission will now not block the game process until the permission request is completed. This is inconsistent with the previous behavior and also breaks some functions of my game. My game requires local storage permissions to create some local folders and files for players to use. So you must make sure to complete the permission application before creating folders and files.

Steps to reproduce

Add dangerous permissions and use OS.request_permission. After exporting to Android, open the game and a permission application window will pop up. However, the game will not wait for the permission application to be completed and is still running.

Minimal reproduction project (MRP)

N/A

AdriaandeJongh commented 4 months ago

Making it non-blocking is desired behavior. If you need permissions to do something, you should wait to do that thing before you do it?

scgm0 commented 4 months ago

Making it non-blocking is desired behavior. If you need permissions to do something, you should wait to do that thing before you do it?

But in that case, how do I wait for the permission request to be completed? If I can't wait for the permission request to be completed, my game's folders and files will fail to be created until the game is launched a second time...

adamwych commented 4 months ago

@scgm0 It looks like there's a signal emitted once the request finishes: https://github.com/godotengine/godot/blob/86bf8354a06ab7b23a0ff6a81b48fd015e92ac94/platform/android/java_godot_lib_jni.cpp#L508C51-L508C80

BTW, won't this check fail now? It's synchronous. https://github.com/godotengine/godot/blob/86bf8354a06ab7b23a0ff6a81b48fd015e92ac94/platform/android/audio_driver_opensl.cpp#L271

scgm0 commented 4 months ago

@scgm0 It looks like there's a signal emitted once the request finishes: https://github.com/godotengine/godot/blob/86bf8354a06ab7b23a0ff6a81b48fd015e92ac94/platform/android/java_godot_lib_jni.cpp#L508C51-L508C80

BTW, won't this check fail now? It's synchronous.

https://github.com/godotengine/godot/blob/86bf8354a06ab7b23a0ff6a81b48fd015e92ac94/platform/android/audio_driver_opensl.cpp#L271

'OnRequestPermissionsResult' doesn't seem to meet my needs, and the following code still can't wait for the permission request to complete:

if (!OS.RequestPermissions()) {
    await ToSignal(Utils.Tree, MainLoop.SignalName.OnRequestPermissionsResult);
}
raulsntos commented 3 months ago

Are you sure this was blocking before? I tried in v4.2.2.stable.official [15073afe3] and it doesn't block.

akien-mga commented 3 months ago

This was changed intentionally in 4.3 and 4.2.2, fixing what was perceived by users as a regression in 4.2:

I don't think we'll change it yet again, it was pointed out that Google's guidelines require it to be non-blocking.

CC @m4gr3d

scgm0 commented 3 months ago

Are you sure this was blocking before? I tried in v4.2.2.stable.official [15073af] and it doesn't block.

Not sure, because earlier Android permissions were automatically applied for before godot started, and it was blocking. Later, pr was changed so that I had to use OS.request_permission to apply manually. However, my project at that time needed to apply for permission before godot started, so I applied for the permission myself in java, which was also blocking. Now I don't need to apply for permission before godot starts, and found that OS.request_permission is not blocking and there is no way to wait for it to complete the application.

scgm0 commented 3 months ago

This was changed intentionally in 4.3 and 4.2.2, fixing what was perceived by users as a regression in 4.2:

* [Android: Disable automatic permissions request #87080](https://github.com/godotengine/godot/pull/87080)

* [Godot 4.2.1 immediately asks for declared Android permissions upon launch. #87043](https://github.com/godotengine/godot/issues/87043)

I don't think we'll change it yet again, it was pointed out that Google's guidelines require it to be non-blocking.

CC @m4gr3d

I just need a way to wait for the permission request to complete, is it possible to add a signal if I can't block it?

raulsntos commented 3 months ago

This was changed intentionally [...] I don't think we'll change it yet again

Yeah, thank you. I agree with the change, just wanted to understand when the change happened to figure out where to document it, I added it to https://github.com/godotengine/godot-docs/pull/9250.

I just need a way to wait for the permission request to complete, is it possible to add a signal if I can't block it?

The signal should be on_request_permissions_result, as suggested previously, but it doesn't seem to work well in my experience. It was only emitting after locking and then unlocking the phone.

m4gr3d commented 3 months ago

OS.request_permission doesn't block on Android by design. The pattern to follow when you need a permission for a specific logic is to request the permission, then wait for the permission result signal and only proceed with your logic if the request was granted.

'OnRequestPermissionsResult' doesn't seem to meet my needs

You mention OnRequestPermissionsResult doesn't meet your needs, but you haven't specified why; if on_request_permission_result is not firing as expected, then that's a separate issue and I can look into it. Please do provide a MRP is that's the case to make it easy to reproduce the issue.

Not sure, because earlier Android permissions were automatically applied for before godot started

Requesting all permissions on start is an anti-pattern, and so it's unlikely we'll revert that change.

m4gr3d commented 3 months ago

@scgm0 It looks like there's a signal emitted once the request finishes: https://github.com/godotengine/godot/blob/86bf8354a06ab7b23a0ff6a81b48fd015e92ac94/platform/android/java_godot_lib_jni.cpp#L508C51-L508C80

BTW, won't this check fail now? It's synchronous.

https://github.com/godotengine/godot/blob/86bf8354a06ab7b23a0ff6a81b48fd015e92ac94/platform/android/audio_driver_opensl.cpp#L271

@adamwych OS.request_permission returns true if the permission was already granted, and false if a request was dispatched. There's logic here to restart the audio driver when the request is finally granted.

scgm0 commented 3 months ago

OS.request_permission doesn't block on Android by design. The pattern to follow when you need a permission for a specific logic is to request the permission, then wait for the permission result signal and only proceed with your logic if the request was granted.

'OnRequestPermissionsResult' doesn't seem to meet my needs

You mention OnRequestPermissionsResult doesn't meet your needs, but you haven't specified why; if on_request_permission_result is not firing as expected, then that's a separate issue and I can look into it. Please do provide a MRP is that's the case to make it easy to reproduce the issue.

Not sure, because earlier Android permissions were automatically applied for before godot started

Requesting all permissions on start is an anti-pattern, and so it's unlikely we'll revert that change.

if (!OS.RequestPermissions()) {
    await ToSignal(GetTree(), MainLoop.SignalName.OnRequestPermissionsResult);
}

The expectation is to wait for the permission application to be completed (after the player agrees) before executing the subsequent code.

m4gr3d commented 3 months ago

@scgm0 to clarify your last comment, do you mean the logic you showed there does not work?

scgm0 commented 3 months ago

@scgm0 to clarify your last comment, do you mean the logic you showed there does not work?

Yes.