Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OS.request_permission is now non-blocking #91194

Open
scgm0 opened this issue Apr 26, 2024 · 14 comments
Open

OS.request_permission is now non-blocking #91194

scgm0 opened this issue Apr 26, 2024 · 14 comments

Comments

@scgm0
Copy link
Contributor

scgm0 commented Apr 26, 2024

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
Copy link
Contributor

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
Copy link
Contributor Author

scgm0 commented Apr 26, 2024

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
Copy link

adamwych commented Apr 26, 2024

@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.

if (OS::get_singleton()->request_permission("RECORD_AUDIO")) {

@scgm0
Copy link
Contributor Author

scgm0 commented Apr 26, 2024

@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.

if (OS::get_singleton()->request_permission("RECORD_AUDIO")) {

'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
Copy link
Member

raulsntos commented May 22, 2024

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

@akien-mga
Copy link
Member

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
Copy link
Contributor Author

scgm0 commented May 22, 2024

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
Copy link
Contributor Author

scgm0 commented May 22, 2024

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
Copy link
Member

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 godotengine/godot-docs#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
Copy link
Contributor

m4gr3d commented Jun 3, 2024

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
Copy link
Contributor

m4gr3d commented Jun 3, 2024

@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.

if (OS::get_singleton()->request_permission("RECORD_AUDIO")) {

@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
Copy link
Contributor Author

scgm0 commented Jun 3, 2024

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
Copy link
Contributor

m4gr3d commented Jun 3, 2024

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

@scgm0
Copy link
Contributor Author

scgm0 commented Jun 3, 2024

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

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants