Inconsistent `Signal.is_connected` behaviour for callables that combine `bind` and `unbind`

[romlok]

Godot version

v4.1.1.stable.official [bd6af8e]

System information

Godot v4.1.1.stable - Debian GNU/Linux trixie/sid trixie - Vulkan (Mobile) - dedicated NVIDIA GeForce GTX 1060 (nvidia) - Intel(R) Core(TM) i5-8400 CPU @ 2.80GHz (6 Threads)

Issue description

👍 For Callables which have had bind xor unbind called on them, Signal.is_connected will return true so long as the base callable is the same.
⚠️ For Callables which have had bind and unbind called on them, Signal.is_connected seems to only return true for the exact Callable object.

I would expect is_connected to return true in the same manner for either case.

Steps to reproduce

extends Node2D

signal boop(doot)
signal beep(doot)
signal baap(doot)

func _ready():
    boop.connect(translate.bind(Vector2.ZERO))
    print(boop.is_connected(translate)) # true
    print(boop.is_connected(translate.bind(Vector2.ZERO))) # true
    boop.disconnect(translate.bind(Vector2.ZERO))

    beep.connect(translate.unbind(1))
    print(beep.is_connected(translate)) # true
    print(beep.is_connected(translate.bind(Vector2.ZERO))) # true
    print(beep.is_connected(translate.unbind(1))) # true
    beep.disconnect(translate.unbind(1))

    var handler = translate.bind(Vector2.ZERO).unbind(1)
    baap.connect(handler)
    print(baap.is_connected(translate)) # false
    print(baap.is_connected(translate.bind(Vector2.ZERO))) # false
    print(baap.is_connected(translate.unbind(1))) # false
    print(baap.is_connected(translate.bind(Vector2.ZERO).unbind(1))) # false
    print(baap.is_connected(handler)) # true
    # Attempting this `disconnect` triggers "Attempt to disconnect a nonexistent connection..."
    baap.disconnect(translate.bind(Vector2.ZERO).unbind(1))

Minimal reproduction project

Self-contained scene: test.zip

[AThousandShips]

This is indeed incorrect behavior, the is_connected check should not care at all about binds/unbinds, unsure what happens here

[AThousandShips]

CC @dalexeev @kleonc

[kleonc]

Making these:

godot/core/variant/callable_bind.cpp

Lines 90 to 92 in 922689c

	const Callable *CallableCustomBind::get_base_comparator() const {
	return &callable;
	}

godot/core/variant/callable_bind.cpp

Lines 224 to 226 in 922689c

	const Callable *CallableCustomUnbind::get_base_comparator() const {
	return &callable;
	}

return callable.get_base_comparator() instead should make it work as suggested I think. It should result in returning the original method-Callable for nested binds/unbinds:

godot/core/variant/callable.cpp

Lines 194 to 204 in 922689c

	const Callable *Callable::get_base_comparator() const {
	const Callable *comparator = nullptr;
	if (is_custom()) {
	comparator = custom->get_base_comparator();
	}
	if (comparator) {
	return comparator;
	} else {
	return this;
	}
	}

[dalexeev]

What matters is not what both bind() and unbind() are used, but the chain length. Two bind() will give the same:

print(s.is_connected(f)) # true
print(s.is_connected(f.bind(11))) # true
print(s.is_connected(f.unbind(1))) # true
print(s.is_connected(f.unbind(1).bind(11))) # false
print(s.is_connected(f.bind(11).bind(22))) # false
print(s.is_connected(f.unbind(1).unbind(1))) # false

Also these callables are considered unequal. Perhaps an ignore_binds = true parameter is needed?

print(f == f) # true
print(f == f.bind(11)) # false
print(f == f.unbind(1)) # false
print(f == f.unbind(1).bind(11)) # false
print(f == f.bind(11).bind(22)) # false
print(f == f.unbind(1).unbind(1)) # false

[AThousandShips]

Can confirm, opening a PR