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.alert() not blocking execution on Android and iOS #49969

Open
dpensky opened this issue Jun 28, 2021 · 8 comments
Open

OS.alert() not blocking execution on Android and iOS #49969

dpensky opened this issue Jun 28, 2021 · 8 comments

Comments

@dpensky
Copy link

dpensky commented Jun 28, 2021

Godot version

v3.3.stable.official

System information

Ubuntu 20.04.2 LTS

Issue description

According to the documentation from OS.alert():

Displays a modal dialog box using the host OS' facilities. Execution is blocked until the dialog is closed.

This (blocking execution) is not happening on the android exported version of my code:

var locale = Settings.get_locale()
match locale:
  'en':
    OS.alert('Thank you!')
  'es':
    OS.alert('¡Gracias!')
_:
    OS.alert('Obrigado!')
queue_free()

queue_free() is called immediately.
Did I get something wrong?

Steps to reproduce

var locale = Settings.get_locale()
match locale:
  'en':
    OS.alert('Thank you!')
  'es':
    OS.alert('¡Gracias!')
_:
    OS.alert('Obrigado!')
queue_free()

Minimal reproduction project

No response

@Calinou
Copy link
Member

Calinou commented Jun 28, 2021

cc @m4gr3d

I'm not sure whether this is a missing feature or by design. While I agree that OS.alert() behavior should be as consistent as possible across platforms, mobile UI is often about being as non-blocking as possible.

@m4gr3d
Copy link
Contributor

m4gr3d commented Jun 28, 2021

@Calinou I would venture this is by design on mobile platforms since as you mention, mobile architecture is rather non-tolerant of blocking code (e.g: on Android, the app gets terminated if it's unresponsive for too long).

The action item here would be to update the documentation and remove the section about execution being blocked.

@dpensky For your use-case I would suggest refactoring your logic so that resource freeing is not conditional on interaction with the alert dialog. Such refactor should actually be valid on all targeted platforms unlike the currently expected behavior.

Let me know what you think.

@dpensky
Copy link
Author

dpensky commented Jun 28, 2021

I'm making this call within a game's auxiliary screen, which by itself already pauses the game and I'm using this alert as a sort of close button to close this screen without the player having to click again on another button just to close this auxiliary screen.
If this is intentional I will look for another way. Thanks.

@m4gr3d
Copy link
Contributor

m4gr3d commented Jun 28, 2021

If this is intentional I will look for another way. Thanks.

@dpensky It's indeed intentional on Android platforms. A feature that could help in this scenario would be to add a couple of signals which would be fired when the dialog is opened and closed. This way, you could listen to the dialog_close signal in order to trigger your exit logic.

This would be straightforward to do on Android, however I'm not familiar enough with the alert dialog logic on other platforms to know if this would be feasible (@Calinou any thoughts on the idea?), so for now, I'd recommend to look for another approach.

@Calinou
Copy link
Member

Calinou commented Jun 28, 2021

This would be straightforward to do on Android, however I'm not familiar enough with the alert dialog logic on other platforms to know if this would be feasible (@Calinou any thoughts on the idea?), so for now, I'd recommend to look for another approach.

This should be feasible on desktop platforms. However, for this to really make sense, we need to make OS::alert() non-blocking there, which is technically a breaking change. I'd wait for 4.0 to do this.

Also, some people may find the blocking behavior desirable on desktop platforms, so maybe there should be an optional blocking parameter (that would be ignored on mobile platforms). At this point, it may be better to open a proposal for this 🙂

@m4gr3d
Copy link
Contributor

m4gr3d commented Jun 28, 2021

This should be feasible on desktop platforms. However, for this to really make sense, we need to make OS::alert() non-blocking there, which is technically a breaking change. I'd wait for 4.0 to do this.

This could still be doable on 3.x; we leave the behavior for OS::alert() as is for desktop platforms, we just prepend and append the logic with the signals I described.

So for example after invoking OS::alert()

  • On desktop:
    • Fire alert_dialog_open signal
    • Run alert dialog blocking code
    • Fire alert_dialog_closed signal on completion of the blocking code
    • Return
  • On mobile:
    • Fire alert_dialog_open signal
    • Run alert dialog non-blocking code
    • Fire alert_dialog_closed signal on completion of the non-blocking code

So we don't make any breaking changes and just add additional notifications for when the alert dialog is opened/closed.
Game developers on desktop platforms can decide whether to stick with the blocking logic or use the newly added signals.

@timothyqiu
Copy link
Member

alert() not blocking has another issue.

We use OS::get_singleton()->alert() to show various error messages before exiting the program.

godot/main/main.cpp

Lines 1242 to 1245 in 1d910b1

const String error_msg = "Error: Can't run project: no main scene defined in the project.\n";
OS::get_singleton()->print("%s", error_msg.utf8().get_data());
OS::get_singleton()->alert(error_msg);
goto error;

In this case, Android will immediately exit without showing anything. Making it harder to debug simple mistakes.

@lostminds
Copy link

I just tested this on iOS and the problem seems to be the same there.

OS.alert("This is our message","Alert!")
OS.crash("Crash message")

Causes a crash before the alert is even shown on screen.

@akien-mga akien-mga changed the title OS.alert() not blocking execution on Android OS.alert() not blocking execution on Android and iOS Feb 27, 2023
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

6 participants