godotengine / godot

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

With two ConfirmationDialog calls, shortly after each other, the second blocks when the first is canceled. #78798

Open AntonWedemeier opened 1 year ago

AntonWedemeier commented 1 year ago

Godot version

v4.0.2.stable.official [7a0977ce2] (also tested 4.1 RC)

System information

Windows 10, Vulkan

Issue description

I show two ConfirmationDialog (or AcceptDialogs) after each other, both waiting for a user response with await. The seconds is not shown if the the user cancel the first one (or click the close cross in the title)

Steps to reproduce

Create a Node3D with a ConfirmationDialog as child. At the next script to the Node3D:

extends Node3D

@onready var _confirmation_dialog: ConfirmationDialog = $ConfirmationDialog

signal on_user_action

var _confirmed: bool = false

# Called when the node enters the scene tree for the first time.
func _ready():
    _confirmation_dialog.connect("confirmed", on_confirmed)
    _confirmation_dialog.connect("canceled", on_canceled)

    _confirmation_dialog.dialog_text = "Test 1"
    _confirmation_dialog.popup_centered()
    await on_user_action
    _confirmation_dialog.dialog_text = "Test 2"
    _confirmation_dialog.popup_centered()
    await on_user_action

func on_confirmed():
    _confirmed = true
    _confirmation_dialog.hide()
    emit_signal("on_user_action")

func on_canceled():
    _confirmed = false
#   await get_tree().create_timer(0.02).timeout
    _confirmation_dialog.hide()
    emit_signal("on_user_action")

Run this and press cancel button with the first message. The second message will not be given. It seems someting to do with a deferred action. If I uncommend the timer all goes well.

Minimal reproduction project

TestConfimation.zip

AThousandShips commented 1 year ago

Pressing cancel already does a deferred call to hide, which will be called after the dialog has popped up again

In fact you manually hiding the dialog is unnecessary as it handles that by itself unless you've changed the settings for the ok button

This could do with some documentation, but I fail to see any real use case for reusing the same dialog in the exact same frame or the advisability of doing so

AntonWedemeier commented 1 year ago

I changed to the setting for the OK button (see project-file).

I am trying to make some general purpose script for Access and Confirmation Dialogs. I want to await for the user to respond when I show such a dialog. The project I inserted is just a minimal reproduction project.

I do not think this is a documentation problem. Somehow the close action is deferred. When I add a 'on_close_requested()' this signal is send a little later after the 'oncanceled' or 'on_confirmed()' signals.

AThousandShips commented 1 year ago

Yes, as I said, hide is deferred, i.e. the action to "close" the dialog, really hiding it, is called by a deferred call. By design I expect, but being able to hide and show a dialog on the same frame sounds weird to me

I argue this is indeed a documentation clarification, I doubt there's real reason for removing the deferred call to hide, as I assume it's there for a real reason

If you really want to have the same dialog be cancelled and then shown again at the same time, use a deferred call to pop up and that should just make it work, simple

AntonWedemeier commented 1 year ago

Now I understand my problem better: when i try using the dialog it is just (deferred) closed.

Perhaps I am programming in an oldfashion way. I ask the user a question (popup); wait for the answer; give a reaction (a new popup with the same dialog) directly following the players response.

I have tried to use your suggestion to use a deferred call. I tried both:

    call_deferred("_confirmation_dialog.popup_centered")
    _confirmation_dialog.call_deferred("popup_centered")

The first line gives an error saying it could not find the method. The second line runs but has the same problem as when called without deferred.

AThousandShips commented 1 year ago

call_deferred("_confirmation_dialog.popup_centered")

Yes this is not valid code

Unsure why the deferred call doesn't fire, but it's a bit of a specific case once again, I'm not sure about how useful being able to fire the same dialog after closing in the same frame

capnm commented 1 year ago

Here is the OK/Cancel difference. It looks like Cancel "hide" doesn't need to be called deferred here. I.e. with

-       call_deferred(SNAME("hide"));
+       set_visible(false);

the MRP works fine (tested only Linux). So the calls wouldn't be asymmetrical, as the OP (and I:) would intuitively expect. https://github.com/godotengine/godot/blob/c83f912bcb33d441b90afe36bad38880acbe5f15/scene/gui/dialogs.cpp#L105-L119

AntonWedemeier commented 1 year ago

Thank you all for the information. I've learned a lot from it.

I still think the difference between the OK and CANCEL button is not just a documentation issue, but I am not capable enough to judge this.

For now I have solved my situation by adding the next line inside the on_canceled function.

    await get_tree().process_frame
capnm commented 1 year ago

Thank you all for the information. I've learned a lot from it.

Thanks for raising this issue so I don't need to wonder myself :)

AThousandShips commented 1 year ago

Feel free to open a fix if this is indeed safe to change!

It's absolutely possible that the deferred call is a leftover from older behaviour that no longer applies, but it was most likely intentional at some point, and might still be

capnm commented 1 year ago

Thanks for the tip. I now have a lot of old starter projects to convert for our next workshops. Let's see if I can find something that will break.