godot icon indicating copy to clipboard operation
godot copied to clipboard

Add a flag to make the connection automatically emit the source object.

Open Rindbee opened this issue 2 years ago • 10 comments

Mainly used to improve the connection dialog.

It seems that the previous connection dialog only supports constant bindings. Though duplicating a node in the editor will also duplicate the existing connections, but we still need to adjust some parameters, and it's not very efficient to frequent operations in a pop-up dialog.

For existing connection-related parameters, these can be divided into three categories, listed in the order of use: the parameters of the signal, the binds arguments in the connection, the bound arguments of the callable.

It is probably well known that the binding parameters in the connection dialog are the bound arguments of the callable.

In the current situation, it seems difficult for the receiver to know the information of the signal sender when only use the connection dialog.

In this commit, add a flag bit ConnectFlags.CONNECT_SOURCE_AUTOBIND to make the connection automatically emit the source object.

If this flag bit is enabled, the source object will be emitted ~~at the end of the emitted parameters, right before the bound arguments of the callable.~~ at first, no matter what the other parameters are.

Hopefully this will make the connection dialog a bit more usable.

~~https://user-images.githubusercontent.com/30386067/162772481-c046b302-178d-41ba-ae8b-145c67ba3a39.mp4~~

https://user-images.githubusercontent.com/30386067/162824296-ea974077-20fe-4a4e-8a06-037341798362.mp4

  • Production edit: This closes https://github.com/godotengine/godot-proposals/issues/7166.

Rindbee avatar Apr 11 '22 14:04 Rindbee

Would work for the use case in https://github.com/godotengine/godot-proposals/issues/3775 albeit in a different way than proposed

Zireael07 avatar Apr 11 '22 16:04 Zireael07

I somewhat like this new flag, but the fact that it requires so much text to be explained properly concerns me and makes me question its simplicity.

"Auto-bind the source object during emission. Mainly used to improve the usability of the connection dialog. Connection-related parameters can be divided into three categories, listed in the order of use: the parameters of the signal, the [code]binds[/code] arguments in the connection, the bound arguments of the callable. If this flag is enabled, the order of the new parameter in the list is right before the bound arguments of the callable (that is, the last parameter to emit. By the way, the binding parameters in the connection dialog are the bound arguments of the callable.)."

Mickeon avatar Apr 11 '22 16:04 Mickeon

I somewhat like this new flag, but the fact that it requires so much text to be explained properly concerns me and makes me question its simplicity.

"Auto-bind the source object during emission. Mainly used to improve the usability of the connection dialog. Connection-related parameters can be divided into three categories, listed in the order of use: the parameters of the signal, the [code]binds[/code] arguments in the connection, the bound arguments of the callable. If this flag is enabled, the order of the new parameter in the list is right before the bound arguments of the callable (that is, the last parameter to emit. By the way, the binding parameters in the connection dialog are the bound arguments of the callable.)."

The picture will make it clearer. These three categories have existed before. Just rarely notice them.

captrue

This will affect the parameter list of the target method.

Emitting the new source object first may avoid some unnecessary trouble. ~~But the changes could be big.~~

Now, if the flag bit is enable, the source object will be the first argument to emit. It will be easy to write the parameter list of the target function by hand.

Rindbee avatar Apr 11 '22 18:04 Rindbee

The current feeling is that signal and connection binds know the parameters of the original callable, so their parameters are combined to be the parameters of the original callable; while callable.unbind hides the parameters of the original callable that are not needed, and callable.bind will append some. Now it seems to be expected to append a source object after the unbind/bind processing is completed.

~~The actual callable will usually be the function that receives all these arguments, which means that the function exists on demand, instead of an existing function with a known parameter list (that way, you need to use unbind to discard redundant parameters, which behaves like using an adapter).~~ Well, I'm a little confused about the cause and effect.

IMHO, right now, it actually feels a bit confusing about Callable, even without adding this new parameter:

  1. The result of Callable.bind and Callable.unbind is that we can't find the original Callable, only the Callable after these operations, so it seems that bind/unbind behaves like +=/resize.
  2. bind/unbind behaves more like an insert than an append. Because the calling order and the argument list order of the new callable are reversed. c.bind(A).bind(B) => func c(B, A).

On the other hand, it might make sense if we thought that the callable (the Receiver Method) was trying to receive these parameters. This is how I understand it.:

First, its parameter list size is 0, use bind/unbind to adjust the parameter list, receives (insert) parameters for connection binds, receives (insert) parameters for singal, and now also receives (insert) the source object. It becomes what we see.

Of course, it would be better if the order could be reversed.

Rindbee avatar Jul 28 '22 20:07 Rindbee

I seem to be stuck in a chicken-and-egg dilemma.

Let's go back to the beginning:

  1. This patch is to improve the flexibility of the connection dialog, making it possible to pass the source object. If it is through a script, there is no need to introduce it, just use bind(). Moreover, I personally feel that many users will not change the default connection flags when using the connect() function in most cases, at least I do. In complex cases, it is not pleasant to manually write the parameter list of the Receiver Method. I wrote the example in the picture several times, and finally did it without using type hints.
  2. If connection is finished through the connection dialog, I recommend automatically generating the Receiver Method. It will solve the problem of parameter arrangement. As long as there are no errors, the user doesn't have to care about the intermediate process and why. Compared to Callable.bind/Callable.unbind, this flag is simple. I guess users who understand Callable should have no trouble understanding this.
  3. Ease of use is more important. Compared to the original, when this flag is enabled, just add the parameter source to the beginning of the parameter list. Instead of bothering to check the result of callable bind/unbind and figure out where it is. In that case, it is better to use bind to bind source object.

Honestly, this patch doesn't introduce much complexity. I just briefly introduced the existing situation, and some people have already complained that it is too complicated. I believe they are the users who trust the connection dialog. In the process of use, they will not come into contact with these complicated things, before and after.

Rindbee avatar Jul 29 '22 01:07 Rindbee

Heyo, I already removed the connection binds in #63595 so feel free to rebase your PR. I think for merging, as was discussed, the source object should be appended (right after the original signal arguments, before the callable binds) and not prepended. Other than that it looks good!

reduz avatar Aug 06 '22 16:08 reduz

Now use a new commit to solve the callable.unbind problem: by changing the order of bind.

The source object will be appended, right before the callable binds (if it exists), after the original signal arguments, after the callable unbind (if it exists). This will ensure that the last unbind still only work on the original signal arguments.

Rindbee avatar Aug 07 '22 02:08 Rindbee

Okay, don't really agree, but following your advice. Let's wait for the feedback from the users.

Rindbee avatar Sep 02 '22 12:09 Rindbee

2. bind/unbind behaves more like an insert than an append. Because the calling order and the argument list order of the new callable are reversed. c.bind(A).bind(B) => func c(B, A).

For reference: #62891.

kleonc avatar Sep 11 '22 20:09 kleonc

I modified the usage condition of this option in connection dialog.

  • Using unbind in the connection dialog will automatically disable this option, in the same way that "Extra Call Argument" is disabled.

It is up to the user when used in scripts. It is more convenient to directly use bind() to bind the source object than to use this enumeration constant in scripts.

Rindbee avatar Mar 16 '23 03:03 Rindbee