Allow differently-bound callables from the same callable on the same signal

[wareya]

Describe the project you are working on

Helping a friend debug their game

Describe the problem or limitation you are having in your project

My friend ran into an issue where binding different callables to the same signal resulted in warnings/errors and semi-broken code:

mysignal.connect(mymethod.bind(1))
mysignal.connect(mymethod.bind(2))

As discussed in #63038 this is supposedly intentional behavior, as signal connection only checks the instance and method.

But I think this is a case of Chesterton's Fence gone wrong. Differently bound callables are non-equal in gdscript (i.e. mymethod.bind(2) == mymethod.bind(1) is false), and connecting multiple different-bound callables originating from the same method can be supported without breaking backwards compatibility.

This looks and feels like a bug, even if it's supposedly intentional behavior, and it can be fixed without breaking backwards compatibility or hurting performance.

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

First, treat non-equal callables in connect as being different callables.

It's very rare for signal connection or disconnection to be "hot" code, so this shouldn't significantly affect performance. However, for the sake of simplicity, the multi-connection check should use an autogenerated unique ID (an autoincrementing 64-bit or 128-bit number would work) instead of checking bindings.

Then, to retain backwards compatibility, make disconnect() disconnect all different-binded callables from the same instance and method. In fact, the single-connection case of this is what already happens, and the following example is working, valid code right now.

mysignal.connect(mymethod.bind(1))
mysignal.disconnect(mymethod)
#or:
mysignal.connect(mymethod)
mysignal.disconnect(mymethod.bind(1)) 

Disconnection would just need to potentially disconnect multiple callables instead of one. In other words, disconnection being weird would not be new.

Right now, connection and disconnection are done with a binary search using VMap and a comparator function. Making multiple connection work as explained here would be simple, but making multiple disconnection work as explained here would be slightly more involved.

The comparator for callables (which for normal callables is just the callable itself, with overridden equality and inequality operators) would need to take their bindings or ID into account, but only when instance and method are equal. This would make connections work without significantly affecting performance, only costing id management overhead and an additional branch in the comparator (i.e. callable's equality and inequality operators).

For disconnections, the disconnection code would search for the callable's comparator and take the resulting map index even if it points to a callable with a non-equal comparator. (This is just VMap's _find method.) Then it would check that index's instance and method against the disconnectee. Then it would search left and right for any callables that match the instance and method, stopping at the first callable in each direction that doesn't. This would allow it to find all attached callables with the same instance and method while retaining O(log(n)) complexity for disconnection.

As such, multiple connections to the same method with different bindings can be supported without hurting performance, breaking backwards compatibility, or introducing new confusing behavior that isn't already there.

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

The following code would become legal and do what it looks like it does:

mysignal.connect(mymethod.bind(1))
mysignal.connect(mymethod.bind(2))

The following code would bind two callables and then disconnect them both:

mysignal.connect(mymethod.bind(1))
mysignal.connect(mymethod.bind(2))
mysignal.disconnect(mymethod)

Note that this is already how disconnection works, except for specifically the change of disconnecting multiple signals at once.

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

Core gdscript functionality.

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

Core.

[dalexeev]

• Related to / duplicate of: #8341.

This would probably break compatibility.

[wareya]

For reference, I proposed a way of updating disconnect that tries to avoid breaking existing working code. I might not have succeeded, but I think it should work. (This also makes it not a duplicate; I'm proposing a specific implementation that differs from the other proposal.)