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

await signal yields before its listeners get executed, if they were registered via anonymous function #75341

Open
AlexanderFarkas opened this issue Mar 26, 2023 · 5 comments

Comments

@AlexanderFarkas
Copy link
Contributor

AlexanderFarkas commented Mar 26, 2023

Godot version

v4.0.1.stable.official [cacf499]

System information

Mac OS 13.0.1

Issue description

Following tutortial https://docs.godotengine.org/en/stable/getting_started/first_2d_game/06.heads_up_display.html

Wanted to connect signal via code instead of editor. But it breaks the logic - signal callback is triggered after await Timer.timeout

"Dodge the\nCreeps!" doesn't appear on the screen

Steps to reproduce

class_name HUD
extends CanvasLayer

signal start_game

# Called when the node enters the scene tree for the first time.
func _ready():
	
	$MessageTimer.timeout.connect(func(): 
		$Message.hide()
		print("Hide")
	)
	$StartButton.pressed.connect(func():
		$StartButton.hide()
		start_game.emit()
	)


func show_message(text):
	$Message.text = text
	$Message.show()
	$MessageTimer.start()

func show_game_over():
	show_message("Game Over")
	# Wait until the MessageTimer has counted down.
	await $MessageTimer.timeout

	$Message.text = "Dodge the\nCreeps!"
	$Message.show()
	print("Show")
	# Make a one-shot timer and wait for it to finish.
	await get_tree().create_timer(1.0).timeout
	$StartButton.show()
	
func update_score(score):
	$ScoreLabel.text = str(score)
	
# Called every frame. 'delta' is the elapsed time since the previous frame.
func _process(delta):
	pass

Console output:

Hide
Show
Hide

Minimal reproduction project

Dodge_the_Creeps.zip

@AlexanderFarkas AlexanderFarkas changed the title Why signal yields before its listener is executro await signal yields before its listeners get executed Mar 26, 2023
@AlexanderFarkas
Copy link
Contributor Author

AlexanderFarkas commented Mar 26, 2023

If I replace this code:

$MessageTimer.timeout.connect(func(): 
		$Message.hide()
		print("Hide")
	)

with this:

$MessageTimer.timeout.connect(_timeout)

...

func _timeout():
	$Message.hide()
	print("Hide")

It starts working as expected
Output:

Hide
Hide
Show

@AlexanderFarkas AlexanderFarkas changed the title await signal yields before its listeners get executed await signal yields before its listeners get executed, if they were registered via anonymous function Mar 26, 2023
@AlexanderFarkas
Copy link
Contributor Author

AlexanderFarkas commented Mar 26, 2023

That happens due to how Godot handles connections.
object.cpp:1287 - VMap doesn't support order of insertion.

gscript_vm.cpp:2367 - VM connects callback for resumption. And get_base_comparator for lamda functions is low relative to methods.

So await returns before lambda functions have chance to execute.
object.cpp:1028 - If insertion order will be preserved, await-produced one-shot callback would always be the last one.

Hope, it helps @Calinou

@AlexanderFarkas
Copy link
Contributor Author

AlexanderFarkas commented Mar 26, 2023

If maintaining insertion order is cumbersome, I would just allow _signal_callback's connection to have lowest possible value. So, "await connection" will always be executed last.

So, this:

Error err = sig.connect(Callable(gdfs.ptr(), "_signal_callback").bind(retvalue), Object::CONNECT_ONE_SHOT);

Will become this:

Error err = sig.connect(Callable(gdfs.ptr(), "_signal_callback").bind(retvalue), Object::CONNECT_ONE_SHOT,  priority=NEGATIVE_INFINITY);

@AlexanderFarkas
Copy link
Contributor Author

Actually, insertion order is not a fix. You always want await connections to execute last, no matter when they were added. Lowest priority should be the fix, I believe.

@AlexanderFarkas
Copy link
Contributor Author

AlexanderFarkas commented Apr 2, 2023

@Calinou I want to PR, could we discuss how it should be implemented? I wanted to add new slot_map, but there are lots of things going on around slot_map. Adding late_slot_map doesn't seem suitable.

CONNECT_DEFERRED also doesn't seem to solve the problem, since other deffered connections could be added.

Adding new CallableCustom for await-resume exclusively doesn't work, since comparison functions don't work for different derived classes (they just compare functions???)

The only thing I understand - await-resume functions should always be executed last. There should be either separate vector for them or way to sort them last.

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

2 participants