godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.14k stars 93 forks source link

Allow multiple connections from a signal with different bound parameters to the same callable in inspector #8341

Open Lazy-Rabbit-2001 opened 11 months ago

Lazy-Rabbit-2001 commented 11 months ago

Describe the project you are working on

A project highly implemented by signal-calls

Describe the problem or limitation you are having in your project

Currently, when trying to connect a signal with the same callable in the inspector, we will receive an error showing that the signal has already connected to that callable and block the execution of connection. However, as we have bound parameters in advanced mode in a connection dock, some methods, taking set_indexed() as an example, will be called with different params input, and if ONE signal is trying connecting to this callable for multiple times, but each time the bound params(passed-in params) are different, the warning shows again and blocked the procession, too, which makes developers who are devoted to the signal-callable system go into madness.

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

Make signal connectable to the same method with different bound params to be passed in

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

When a signal that has connected a callable is trying to connect the callable again, check if there is any signal that has the same bound params (no bound params = 0 bound param), if no, then pass the detection and make the signal connect to the callable; if yes, then iterate each pair of params and compare if they are same, and if there is ONE pair that is different, then pass the detection and the signal connects to the callable; if no, then block the signal connection and pop the error.

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

No, because this is related to source code of Godot

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

This is related to core system of editor of Godot

dalexeev commented 11 months ago

This avoids having to list bound arguments to disconnect(). Changing this would break compatibility, but we could probably add a connect flag to take into account bound arguments.

Lazy-Rabbit-2001 commented 10 months ago

This avoids having to list bound arguments to disconnect(). Changing this would break compatibility, but we could probably add a connect flag to take into account bound arguments.

I think it also OK to make this feature a flag on the right of existing ones. However, as you mentioned, disconnect() would be called with bound arguments to make sure the mapped callable will be disconnected from the signal, I don't have any idea if flags will also lead it to encountering the issue you mentioned. This is really a question worth thinking about.

How about adding a label or an id of connections with different bound arguments, and make disconnect() contain a param with 0 or "" initially? For example:

my_signal.connect(_on_signal.bind(1), CONNECTION_DEFAULT, 0) # or label "a"
my_signal.connect(_on_signal.bind(2), CONNECTION_DEFAULT, 1) # or label "b"
...
my_signal.disconnect(_on_signal, 0) # or "a". Disconnect one passing in int 1
my_signal.disconnect(_on_signal, 1) # or "b". Disconnect one passing in int 2

This type avoids bound arguments from being passed in and only makes one more extra optional param, not breaking compability And as for the manual connection, makes the flag an edittable column to pass in a label that differs the connection versions. What do you think of this one?