godotengine / godot

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

Add a mechanism to GDScript to wait for signal's completion #3235

Closed ghost closed 7 years ago

ghost commented 8 years ago

This is feature request, and follows from the discussion in the issue #3223.

In GDScript, currently, there is no way to wait/block until a signal completes (in a general setting). This is in particular desirable in situations where the user wants to keep the game logic is synchronous, and the code structure flat (rather than sorting to a callback/futures/promises style game logic, which can be harder to develop and maintain).

It can be realized by adding a blocking alternative of signal-yield which blocks/waits until the signal is handled completely, or adding a blocking=false parameter to signal variant of yield for example.

ghost commented 8 years ago

Also, the blocking mechanism presented in the tutorial works properly only when the yield function is called from a top-level function in the GDScript call stack.

The tutorial needs to be updated either to mention this limitation or use the new feature requested in this issue (if it is accepted).

punto- commented 8 years ago

Do you mean "block" as in, the thread stops running? Is the code that eventually emits the signal running on a separate thread?

On 4 January 2016 at 21:07, Ferenc Arn notifications@github.com wrote:

Also, the blocking mechanism presented in the tutorial http://www.godotengine.org/projects/godot-engine/wiki/GDScript#Coroutines-38-Signals works only when the yield function is called from a top-level function in the GDScript call stack.

The tutorial needs to be updated either to mention this limitation or use the new feature requested in this issue (if it is accepted).

— Reply to this email directly or view it on GitHub https://github.com/godotengine/godot/issues/3235#issuecomment-168850113.

ghost commented 8 years ago

It's probably the same thread, but let me explain what I had in mind.

I'm trying to implement a function which presents a question with multiple answers to the users, and return user's choice:

# returns the selected answer
func question_modal(question, answers):
    # function body

question_modal will be a basic building block of the game, so it can be called anywhere. Like

func talk():
    var color = ui.question_modal("What color do you like", ["Red", "Blue", "Green"])
    if color == 0:
    # and so on...

If I implement it like this

# script of the class

var m_param

# return a result from a signal
func question_modal(question, answers):
    # stuff...
    yield(node, "button_selected")
    # more stuff here...
    return m_param

func _on_button_selected(param):
    m_param = param

question_modal will return a GDFunctionState rather than m_param, because yield here won't wait until _on_button_selected completes and signal is handled.

Back to your question, in my case, I'm not explicitly creating and threads from GDSCript. Since I'm not familiar with Godot's implementation, I don't know whether the ordinary GDScript code that will run just after yield (in the above example, that would be the line with if color == 0) and the code that handles input and signal runs in separate threads under the hood or not.

I think what I'm talking about here is a "longjmp/setjmp" kind of situation rather than truly blocking a thread in a parallel environment, sorry about creating a confusion with bad choice of words. I think I should say wait instead of blocking.

I'm not sure whether this makes sense inside Godot's C++ code, but one way to implement what I'm talking about is to make a yield-like function (let's call it wait), which behaves like yield, except when the signal handler completes, in jumps to the line right after wait is called (rather than yielding the control to the parent function in the GDScript call stack).

ghost commented 8 years ago

The mechanism I suggested in my previous post probably requires adding a new field (namely, a member variable which tells the GDScript address to jump to after the GDScript signal handler is complete; if it is null, then it is yield rather than wait, for example) to the class that represents signals.

ghost commented 8 years ago

An alternative way to implement this could be to let users pass additional parameters to signal when calling yield, and adding a CSP-like channel mechanism (which complements yield/resume mechanism in GDScript's coroutines).

The caller of yield can create a channel and pass it as the additional parameter. The signal handler can communicate the result to the caller via send/recv.

IMHO, channels and wait aren't orthogonal to each other, and both can be considered.

ghost commented 8 years ago

Just to clarify what I mean by complementing yield: CSP-style channels are objects which you can pass around (in principle, you can even pass a channel over a channel), so you can do something like

# script of the class
# return a result from a signal
func question_modal(question, answers, chan):
    # stuff...
    yield(node, "button_selected", chan) # let's say we can pass additional parameters to signals via yield
    # more stuff here...

func _on_button_selected(selection, chan):
    chan.send(selection)
func talk():
    var chan = Channel.new()
    ui.question_modal("What color do you like", ["Red", "Blue", "Green"], chan)
    var color = chan.recv()
    if color == 0:
    # and so on...

Note that we no longer need the local state m_param this way, which might cause race conditions in a parallel environment in general. If wait is implemented to return the value that is returned from the signal handler, we won't have to use a local state either. In the worst case, one can use a mutex.

In principle, the first three lines of talk() can actually be wrapped to a function (but we can't put those inside the question_modal since yield will yield to talk) such that we can have a function which simply returns the selected answer.

This is not possible to implement using GDScript's yield. Channels would allow safe communication as well as synchronization, complementing yield.

AlexHolly commented 8 years ago

+1 quick example: _on_Area2D_body_enter( body ):

add player to moving platform node

_on_Area2D_body_exit( body ):

remove player from moving platform node

ProbDenis commented 8 years ago

I support this request. This would be really helpful in situations where you want things to happen in a more linear way without creating dozens of signals or putting a "yield" behind pretty much everything, because it only "blocks" the script when you call it from a top-level function (like tagcup said).

athalean commented 8 years ago

+1

I like the channels idea. I could also imagine this to be implemented like with an await keyword for sub-coroutines or GDFunctionStates emitting a signal when the function returns.

Sslaxx commented 8 years ago

Hopefully I don't sound like a complete idiot here, but is @reduz https://twitter.com/reduzio/status/762086309695479808 referring to this issue?

ghost commented 8 years ago

I don't think so. That post is only advertising the usage of get_tree.create_timer(n) with yield.

ghost commented 8 years ago

A few related posts:

https://coderwall.com/p/owym9g/yield-always-return-from-the-function-in-godot https://godotdevelopers.org/forum/discussion/15711/wait-for-return-value-form-function-in-a-yield

Could one of the godot developers share whether this feature is considered or not?

ghost commented 7 years ago

A nice write up how this can be done with Lua's coroutines (which behave differently than GDScript's yield/resume): http://lua.space/gamedev/using-lua-coroutines-to-create-rpg

supercom32 commented 7 years ago

This deficiency is pretty big in my humble opinion. It's pretty important for games like Visual Novels, RPGs, etc to be able to run code synchronously without blocking the main thread.

When you perform a yield, I would have expected the current method, as well as all callers to this method (especially if they expect a return value) to also yield. But alas, it wasn't designed like that. (^_^);

neikeq commented 7 years ago

I was playing a bit and got this: https://github.com/neikeq/godot/commit/eb274a0aab973b9a0a6efc0c7c453313ef1da4a9

This would allow the following:

func _ready():
    var ret
    var gdfs = do_something()
    if gdfs != null and typeof(gdfs) == TYPE_OBJECT and gdfs extends GDFunctionState and gdfs.is_valid():
        # do_something() did yield, so we yield too
        yield(gdfs, "completed")
        # resumed after do_something() resumes and completes
        ret = gdfs.get_func_ret()
    else:
        # do_something didn't yield. this doesn't happens in this example
        ret = gdfs
    prints("do_something returned:", ret)

func do_something():
    print("do_something began")
    yield(get_node("timer"), "timeout")
    print("do_something resumed")
    return 13

# Output
# do_something began
# do_something resumed
# do_something returned: 13

Of course, that's too much typing. The ideal would be to add syntax sugar to allow writing the previous code as something like this:

var ret = yield(do_something(), "completed")
# or
var ret = yield(do_something())

This is pretty similar to how it would work with await in C#: https://github.com/neikeq/GodotSharp/issues/10#issuecomment-294322458

ghost commented 7 years ago

As you pointed out, the call to get_func_ret may be eliminated in a nice form, but from a practical user perspective, I would already take this rather than nothing.

ghost commented 7 years ago

@neikeq Are you planning to submit a PR for this? Maybe this could be a first step toward a new yield opcode which does what you mentioned at the end?

athalean commented 7 years ago

@neikeq This is amazing, exactly what I imagined! Id love to see this make it into Godot somehow

akien-mga commented 7 years ago

cc @RandomShaper as he mentioned facing some yield() limitations too.

RandomShaper commented 7 years ago

@neikeq, I'm not sure it's a good idea hard-coding a signal name unless it's not visible by the user. But maybe I've not understood your intent/code well.

To everyone, what do you think about having a "recursive"/multilevel version of the yield()-for-signals that unwinds (and saves the state) of the whole call stack?

I wonder how could this be implemented since the state object wouldn't be returned to the caller and the script engine would then have to keep it, potentially forever.

But usability-wise it would be nice, wouldn't it?

supercom32 commented 7 years ago

I don't know how relevant this would be, but perhaps another possible consideration too. Consider this pseudo code:

draw "some_game_title_screen";
while (true) {
   if (keyboard_char = 32) {  
      print "The space bar was pressed";
      draw "some_png_to_screen";
   }
}

If this code was ported to Godot and called from some game class, you'll note that the very tight 'while' loop will starve the main thread and any UI, debug printing, or update events will not get a chance to run. In a sense, your application will look unresponsive (and maybe windows will ask you to close it?). In many programming languages the "yield()" command would allow you to give a slice of that time back for other processes to take place. For example:


while (true) {
   yeild();
   if (keyboard_char = 32) {  
      print "The space bar was pressed";
      draw "some_png_to_screen";
   }
}

In this case, since the 'while' loop is so tight, it probably runs 1000s of times per second before the user even has a chance to do anything. In addition, even if the user pressed space bar, the program couldn't respond to it because it has no cycles to print out text, draw any images, or perhaps even get keyboard_pressed events! By adding a yield() command, it tells the program, "at this time, check if anything else is outstanding and give them a slice of time to run". You may not think such a small amount of time is helpful, but in computing time it means a lot, especially when 1000s of cycles are dead because the user didn't push any key when the 'while' loop is waiting.

Using Godot, you can kind of achieve this yeild() behavior by spawning a separate thread and running all your synchronous code there. In such case, you can have a very tight 'while' loop running, like in my example, and your UI and everything will respond and update accordingly. However, I did notice that you experience a serious performance drop in updating the main application. That is, when your thread requests to do something like update the app, draw an image, or print something. For example, if using a pre-loaded texture and you tried to render 100 textureFrames to the screen, you'll see each texture drawn to the screen separately. Something like 20 textureFrames per second. Pretty slow, especially for updating a HUD or UI. But if you did the same code under the main loop, you can render literally 1000s of textureFrames, and they all appear instantly on screen! It happens so fast, the user can't even tell they were being draw separately in a loop! I assume these performance issues are caused by classic multi-threading problems (Data Races, Deadlocks, Live Locks, Mutex's, etc).

Hopefully, regardless of the way a solution is implemented, it will allow users to have the same synchronous behavior as I described since a large portion of games are not really suitable to be event driven.

neikeq commented 7 years ago

@RandomShaper with my example you can yield recursively. I'm not familiar with GDScript's code, so I don't know if there could be a better alternative to a "completed" signal. The point of my example was just to show it working, not to show an ideal solution. The only thing I would disagree with is making yield automatically yield all the caller functions. That's a source for potential hidden bugs.

supercom32 commented 7 years ago

Maybe something even simple like "promise" chain support would help make it easier in Godot than all these signal passing?

var totalResult = something(someParam).then(function(someResult) {
   something...
   something...
   return true
}).then(function(someResult) {
   something...
   return false
});

The only problem is, It helps with running code synchronously, but does nothing for you if you need branching code/execution paths.

RandomShaper commented 7 years ago

@neikeq, so you think there should be a way of marking an "unwind limit". Wouldn't that introduce additional complexity? That is, we would then have three flavors of yield-until-signal: immediate, partially recursive and fully recursive. Can you give an example on how fully recursive may mess things up?

@supercom32, for those situations you can try yield(get_tree(), "idle_frame"). That lets the normal frame processing flow continue and then you would be able to check and loop again if necessary.

supercom32 commented 7 years ago

@RandomShaper: Ah, that's pretty neat. It's too bad the current implementation of yield only blocks the current method, and not the calling method as well. It kind of forces you to in-line everything since making helper functions will just cause things to run wild and not run in order.

neikeq commented 7 years ago

@RandomShaper What I mean is something like this:


func _process(delta):
    # some code
    do_something() # notice there is no yield
    # resumed synchronously

func do_something():
    # some code
    yield(do_something_2(), "completed")
    # resumed asynchronously when do_something_2 completes

func do_something_2():
    # some code
    yield(timer, "timeout")
    # resumed asynchronously on timeout

In C# the async keywords specifies that a method can use await. If the method does not use await when calling another async method the execution continues synchronously like in my example. A warning is printed by the compiler in such cases: “warning CS4014: Because this call is not awaited, execution of the current method continues…”

RandomShaper commented 7 years ago

I get it now. Thank you for the example.

I'm very interested in seeing this supported (and helping to implement it if needed), but also I think it's being wise to consider all the possible design choices so this feature ends up being as good as possible.

El 23 abr. 2017 8:04 p. m., "Ignacio Etcheverry" notifications@github.com escribió:

@RandomShaper https://github.com/RandomShaper What I mean is something like this:

func _process(delta):

some code

do_something() # notice there is no yield
# resumed synchronously

func do_something():

some code

yield(do_something_2(), "completed")
# resumed asynchronously when do_something_2 completes

func do_something_2():

some code

yield(timer, "timeout")
# resumed asynchronously on timeout

In C# the async keywords specifies that a method can use await. If the method does not use await when calling another async method the execution continues synchronously like in my example. A warning is printed by the compiler in such cases: “warning CS4014: Because this call is not awaited, execution of the current method continues…”

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/3235#issuecomment-296469040, or mute the thread https://github.com/notifications/unsubscribe-auth/ALQCtmhinwGCUbyBhAmrNPQgEjYCiWUAks5ry5KQgaJpZM4G-eFX .

bojidar-bg commented 7 years ago

Actually while we are at it, I think we might add yeild() as a way to call_deferred the rest of the function, so as to leave other synchronous methods do their jobs, without having to make more methods. Example:

# Old code:
func set_something_expensive_to_recompute(new_value):
    value = new_value
    if !_dirty:
         call_deferred("_recompute")
         _dirty = true
func _recompute():
    if !_dirty: return
    _dirty = false
    # Compute stuff here

# New code:
func set_something_expensive_to_recompute(new_value):
    value = new_value
    if !_dirty:
         _dirty = true
         yield()
         _dirty = false
         # Compute stuff here

Only problem is that it collides with wanting to do yeild(null) shorter. Maybe we can do just yeild;?

supercom32 commented 7 years ago

@bojidar-bg: I think you inadvertently gave me a workaround idea. It might not be ideal, but I guess it works?

If you want to run some code synchronously you can spawn the method using a separate thread. There, it will be perfectly synchronous and won't block anything. However, as I mentioned previously, you'll seemingly incur a heavy performance penalty any time you attempt to create nodes, draw, or manipulate textures. However, if you do a "call_deferred" inside your thread (to your create node/drawing code), it will run just as fast as it would on the main thread. Someone else mentioned that call_deferred probably pushes messages and its data in a queue that is dequeued in the main thread (involving only one mutex lock) which is why it runs perfectly fast.

bojidar-bg commented 7 years ago

@supercom32 Uhm.. actually.... that's the way you usually get your results from a thread to the main thread, when not using wait_to_finish. Other than using mutexes, OFC.

RandomShaper commented 7 years ago

I can confirm call_deferred() does exactly that: inserts the call into a message queue that is flushed at the end of every fixed and idle frames (in the main thread).

neikeq commented 7 years ago

Actually, today I was reading GDScript source and noticed that yield on signal already allows syntax like this: var result = yield(obj, "something_happened"); In this example, the variable result will be set to the argument the signal was emitted with. If there is more than one argument, the variable result will be set to an array of the arguments the signal was emitted with.

RandomShaper commented 7 years ago

Interesting discovery!

I had been assuming it returned a GDFunctionState.

El 28 abr. 2017 8:17 p. m., "Ignacio Etcheverry" notifications@github.com escribió:

Actually, today I was reading GDScript source and noticed that yield on signal already allows syntax like this: var result = yield(obj, "something_happened"); In this example, the variable result will be set to the argument the signal was emitted with. If there is more than one argument, the variable result will be set to an array of the arguments the signal was emitted with.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/3235#issuecomment-298069976, or mute the thread https://github.com/notifications/unsubscribe-auth/ALQCtrzfjseWsRvU9MKkoKiekHxbBZwYks5r0i0rgaJpZM4G-eFX .

neikeq commented 7 years ago

@RandomShaper the function yields returning a GDFunctionState.

RandomShaper commented 7 years ago

Ah, OK. Anyway, I think I'll check this again tomorrow because my mind is a bit tired now. :)

El 28 abr. 2017 11:16 p. m., "Ignacio Etcheverry" notifications@github.com escribió:

@RandomShaper https://github.com/RandomShaper the function yields returning a GDFunctionState.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/3235#issuecomment-298108654, or mute the thread https://github.com/notifications/unsubscribe-auth/ALQCtolzA4opgrmUbJhgKQ5pTX0kOzbyks5r0lcpgaJpZM4G-eFX .

jay20162016 commented 3 years ago

The approach mentioned, while it works for calling a single function and yielding it works for single functions. However, it doesn't give a clean way of waiting for a signal's completion where multiple functions are connected to the signal, say in an event bus. This is particularly troublesome when the multiple functions are variable and aren't known during development. Is there a clean way of waiting for a signal's completion in this case?