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

Improve Thread for better usability #3163

Open
qrrk opened this issue Aug 19, 2021 · 3 comments
Open

Improve Thread for better usability #3163

qrrk opened this issue Aug 19, 2021 · 3 comments

Comments

@qrrk
Copy link

qrrk commented Aug 19, 2021

Describe the project you are working on

A non-game desktop application using Control nodes.

Describe the problem or limitation you are having in your project

Sometimes it is needed to run a long task in a thread to avoid blocking the GUI, proceed to something else, and then react as soon as that task is done. I found this process to be a little convoluted is GDScript. There are two main issues:

  • No easy way to check if the function inside the thread has already returned. is_active() always returns true.
  • Having to always join your thread into the main thread from your code.

Therefore, if you want to execute a function in a thread, you need to implement:

  • A way to communicate that the task is done. This can be additional statements in your function emitting a signal or setting a flag, or a wrapper function around the task proper.
  • A method or block of code that detects task completion, collects the output and disposes of the thread.

This results in quite a bit of code for something I feel could be much simpler. I dove into this problem, and before I realized it, I had a whole wrapper class written to handle this (see below).

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Any of these three additions can drastically reduce the amount of boilerplate code required to use threads. Implementing all three of them would be ideal.

  • Add a method to Thread for easy checking if the function in the thread has returned.
  • Add a signal to Thread that is emitted after its function returns.
  • Add an argument to Thread.start indicating if the thread should join on its own when there is nothing else to do (same effect as thread.call_deferred("wait_to_finish") at the end of your threaded function).

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

I think the previous section already covers it.

If this enhancement will not be used often, can it be worked around with a few lines of script?

Yes, but it is kind of not trivial, especially for novices at programming. Here's what I came up with for a general solution:

class_name ThreadedFuncExecutor
extends Object

signal func_returned

var _worker: Thread
var _result = null


func _wrapper(data: Array) -> void:
	
	if data[2]:  # So you can execute functions that take no arguments.
		_result = data[0].call(data[1], data[2])
	else:
		_result = data[0].call(data[1])
		
	emit_signal("func_returned")


func execute(instance: Object, method: String, userdata = null,
		priority = Thread.PRIORITY_NORMAL) -> void:
	
	_worker = Thread.new()
	_result = null
	var data = [instance, method, userdata]
	_worker.start(self, "_wrapper", data, priority)
	

func collect():
	
	if _worker.is_active():
		_worker.wait_to_finish()
		return _result

Is there a reason why this should be core and not an add-on in the asset library?

These additions will help new users get the hang of threads, and they fit well with the way other things are done in Godot.

@Calinou Calinou changed the title Quality of life additions to Thread Improve Thread for better usability Aug 19, 2021
@Xrayez
Copy link
Contributor

Xrayez commented Aug 19, 2021

  • Add a signal to Thread that is emitted after its function returns.

See potential solution godotengine/godot#34862.

@qrrk
Copy link
Author

qrrk commented Aug 20, 2021

Yes, that will help a lot. Looking forward to 4.0. The other two items would still be nice to have, though.

@briansemrau
Copy link

briansemrau commented Oct 6, 2021

Discussing the point specifically about signals:

PRs for emitting a signal when a thread is complete have been rejected/closed several times (here and here), and I think for good reason (godotengine/godot#34862 (comment)).
Though, I think reduz's comment doesn't give the full picture of the issue:

Say that you spawn a worker thread that spawns additional subthreads. When those subthreads finish, you would expect the signal/callback to be deferred to the primary worker. A thread itself does not have a message queue to handle deferred calls, so call_deferred will always push it to the main thread (which is not the worker thread). Adding signals/callbacks to thread completion gives users a certain expectation that cannot be met within all reasonable use-cases, when the alternative is simple:

  • If you want to signal that a thread is completed (and you're not using nested threads), you can simply add call_deferred(my_thread_completed_callback) or call_deferred("emit_signal", "my_thread_completed_signal") at the end of your threaded function. Then you can wrap it with yield(thread, 'my_thread_completed_signal') (code for 3.x; not sure how it works with 4.0's await.)

  • If you are using nested threads, you must implement your own messaging loop (which was always unavoidable).

(Of course, a fair counter-argument is that people don't usually nest threads, and if they do, they likely understand the limitations/limitations can be documented.)

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

4 participants