Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callable unbind doesn't actually unbind arguments #76141

Closed
lilizoey opened this issue Apr 16, 2023 · 18 comments · Fixed by #76158
Closed

Callable unbind doesn't actually unbind arguments #76141

lilizoey opened this issue Apr 16, 2023 · 18 comments · Fixed by #76158

Comments

@lilizoey
Copy link

lilizoey commented Apr 16, 2023

Godot version

4.0.2

System information

Linux 5.10.167-2-MANJARO

Issue description

Calling unbind on a callable doesn't actually unbind the arguments. However it makes godot think the arguments are unbound, thus requiring you to call the callable with fewer arguments. But some of the arguments passed along in the call will be ignored, and the original bound arguments are used instead.

Steps to reproduce

Run

func foo(a: int, b: int, c: int):
	print("a: ", a)
	print("b: ", b)
	print("c: ", c)

func _ready():
	var callable = foo.bind(1,2)
	callable = callable.unbind(1)
	callable.call(4,5)
	# errors
	# callable.call(4)

The printed output is:

a: 4
b: 1
c: 2

Whereas id expect to see something like

a: 4
b: 5
c: 1

Minimal reproduction project

N/A

@dalexeev

This comment was marked as outdated.

@AThousandShips
Copy link
Member

AThousandShips commented Apr 16, 2023

The implementation actively ignores called arguments, which seems to be the intention of unbind, it's not the reversal of a previous bind, it just ignores excess arguments which is helpful in a number of situations, here's the implementation

callable.callp(p_arguments, p_argcount - argcount, r_return_value, r_call_error);

Usecase for this is connecting to a signal that has some argument

This is also documented imo quite clearly:

Calling the returned Callable will call the method without the extra arguments that are supplied in the Callable on which you are calling this method.

To achieve this behavior (for callables that are actual named functions) you can do:

callable = Callable(callable.get_object(), callable.get_method()).bindv(callable.get_bound_arguments().slice(1))

@dalexeev
Copy link
Member

dalexeev commented Apr 16, 2023

You are right, I think this is a bug.

func foo(a: int, b: int, c: int):
    prints(a, b, c)

func _ready():
    var callable = foo.bind(88, 99)
    callable.call(11) # Prints "11 88 99".
    callable = callable.unbind(1)
    callable.call(22, 33) # Prints "22 88 99". Expected "22 33 99" (or "22 33 88"?).

@AThousandShips
Copy link
Member

AThousandShips commented Apr 16, 2023

I mean unbind does not seem to be an inverse to bind but the complement, it doesn't undo bind, it ignores arguments from the user, so it is the complement to bind

I'd say it isn't a bug but does cause confusion sure, also the use here isn't really something that's normal to do? Why would you remove bound arguments, like if you were given a callable from somewhere and don't have control over it

@AThousandShips
Copy link
Member

  • bind makes the callable call with extra arguments at the end beyond the arguments supplied to call
  • unbind makes the callable call while ignoring arguments at the end of the call function

@AThousandShips
Copy link
Member

A thing that would be useful though is extraction of the underlying callable, i.e. getting the subject of bind/unbind, it'd make operating on them easier (the base level one could just return itself)

@lilizoey
Copy link
Author

lilizoey commented Apr 16, 2023

This is also documented imo quite clearly:

Calling the returned Callable will call the method without the extra arguments that are supplied in the Callable on which you are calling this method.

Honestly this was unclear to me, i thought the "extra arguments" it was referring to here were the bound arguments in the callable supplied by a previous bind call.

Why would you remove bound arguments, like if you were given a callable from somewhere and don't have control over it

I agree that it's not very useful (outside of esoteric cases), but it's just how i'd expect unbind to work given the name. And the documentation didn't make clear that it didn't actually unbind previously bound arguments, but rather just changes how many arguments you can call the callable with.

But if this is the intended behavior then it should really not be named unbind. It should probably be named something closer to ignore_arguments or bind_ignored or change_arg_count or something. un-x usually means undo x, so for it to not mean that here is confusing.

@AThousandShips
Copy link
Member

The documentation could do with a little clarification, will look into it tomorrow

But unbind is used in other libraries with callables so it's not arbitrarily here, it's kind of the usual concept of how to name it

@dalexeev
Copy link
Member

dalexeev commented Apr 16, 2023

It's funny that I found the correct use of this method in my project (I didn't think about the inconsistency), but in the context of this issue I got confused. Indeed, this is a very unfortunate naming.

func _ready() -> void:
    _scroll_bar.changed.connect(_update_scroll_bar)
    _scroll_bar.value_changed.connect(_update_scroll_bar.unbind(1))

func _update_scroll_bar() -> void:
    # ...

@lilizoey
Copy link
Author

The documentation could do with a little clarification, will look into it tomorrow

But unbind is used in other libraries with callables so it's not arbitrarily here, it's kind of the usual concept of how to name it

Which other scripting languages use it this way? im trying to look at some other scripting languages but they dont seem to use bind/unbind in this way, if they even have it/something comparable at all.

Ruby seems to use bind/unbind to bind a callable to an object/unbind a callable from an object, but nothing for arguments
Python seems to use functools.partial to do what bind does, and doesn't have anything named bind/unbind nor something similar to unbind
Javascript does have bind which can both change the receiver, and add arguments like our bind does, but doesn't seem to have unbind nor anything that does something similar to unbind
lisps in general use some variant of partial
i can't find anything similar in lua

@AThousandShips
Copy link
Member

My bad I forget where I saw it used before, might have gotten it confused

@dalexeev
Copy link
Member

I've thought about it a bit, and it seems to me that we can make it consistent.

func f(a: int, b: int, c: int) -> void:
    prints(a, b, c)

func _ready() -> void:
    var g := f.bind(33)
    var h := f.bind(22, 33)

    # All examples bellow should print "11 22 33".

    f.call(11, 22, 33)
    g.call(11, 22)
    h.call(11)

    h.unbind(1).call(11, 22)
    h.unbind(2).call(11, 22, 33)
    h.unbind(3).call(11, 22, 33, 44)

    g.unbind(1).call(11, 22, 33)
    g.unbind(2).call(11, 22, 33, 44)

    f.unbind(1).call(11, 22, 33, 44)
    f.unbind(2).call(11, 22, 33, 44, 55)
    f.unbind(3).call(11, 22, 33, 44, 55, 66)
    # ...

bind() adds arguments, and unbind() removes. Also unbind() can remove more than it was bound (remove from those passed to call()). But first unbind() must remove the bound arguments, and only then ignore the passed ones.

@AThousandShips

This comment was marked as outdated.

@AThousandShips
Copy link
Member

Documentation PR open

@AThousandShips
Copy link
Member

AThousandShips commented Apr 17, 2023

Thinking more about this I think it should be left as is, there are real usecases for keeping it like this, for example:

  • Signal that calls a single argument
  • Function that takes two arguments you want to supply yourself

Solution:

my_signal.connect(my_function.bind(1, 2).unbind(1))

This would be impossible otherwise, also a solution would be somewhat convoluted for a usecase that I'm not actually convinced is realistic

While I can think of real world situations where you want the current behavior, I can't really think of any situations where the proposed different functionality would be used

The only way to achieve the current functionality if we change it would be a really convoluted use of lambdas, which kind of defeats the purpose of having the bind/unbind in the first place as labmdas can achieve their functionality largely

@AThousandShips
Copy link
Member

I'd suggest opening a proposal for changing this behavior as it isn't a bug, and leave this to be closed by the documentation proposal.

@dalexeev
Copy link
Member

Solution:

my_signal.connect(my_function.bind(1, 2).unbind(1))

It probably should be

my_signal.connect(my_function.unbind(1).bind(1, 2))

@AThousandShips
Copy link
Member

AThousandShips commented Apr 17, 2023

No that would call it as my_function(..., 2), my example calls it as my_function(1, 2), if passed one argument, which is discarded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants