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

Allow binds and unbinds to be used at the same time in the signal connection dialog. #100554

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Macksaur
Copy link
Contributor

Fixes #98686 and behaves like source_signal.connect(target_function.bind(args).unbind(n)).

Enables the red highlighted options:
image

Copy link
Contributor

@quinnyo quinnyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems OK to me, it works.

I am not intimate with the Callable code (which is being called into) but the changes here don't seem to be affecting how each of those interactions work in a significant way.

The significant changes being made here remove the strict (perhaps overly-defensive?) mutual exclusion constraint on 'bind' and 'unbind'.

For the record, the 'if unbind then no bind' logic looks like the result of taking the two terms at face value as English words. Which is understandable, but is not, in my understanding, the correct behaviour.

I will mention that #102007 occurs in the CI build for this PR that I tested (as well as the official builds), which did limit testing in a very minor way. I don't think that should change anything for this PR, but I also don't know what will come of that issue, so I mention it for completeness.

@dalexeev
Copy link
Member

dalexeev commented Jan 25, 2025

behaves like source_signal.connect(target_function.bind(args).unbind(n))

This is correct, see the comment for reference. Any sequence of bind() and unbind() calls can be reduced to the form .bind(...).unbind(...). (But I didn't look at the code, just noted that the idea is correct. I would like to review this later if I find free time.)

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

Successfully merging this pull request may close these issues.

Cannot use Add Extra Call Argument and Unbind Signal Arguments at the same time in editor
4 participants