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

GUI declares input as handled only if it is really handled #54656

Closed
wants to merge 3 commits into from

Conversation

BimDav
Copy link
Contributor

@BimDav BimDav commented Nov 6, 2021

See #54529.

Input events were consumed by Control nodes as long as their filter was not set to MOUSE_FILTER_IGNORE. With this change they are consumed only if the Control handles the event or its filter is set to MOUSE_FILTER_STOP. Notably MOUSE_FILTER_PASS will not consume the event. Such unhandled event will then trigger _unhandled_input methods.

@golddotasksquestions
Copy link

Thanks you for this fix!!

@golddotasksquestions
Copy link

golddotasksquestions commented Nov 28, 2021

@akien-mga
Would it be possible to cherry-pick this fix for 3.4.1?
Besides #54529, I'm also having issues with mouse filter "pass" in my Control node only inventory and drag and drop scenes because pass is setting the input as handled. Issue documented here: #55432

Having to work around this bug is making my code very elaborate and complicated.

@golddotasksquestions
Copy link

@BimDav @YeldhamDev

Is this issue with Pass behaving like stop possibly related to this:
#55432

@BimDav
Copy link
Contributor Author

BimDav commented Nov 29, 2021

Is this issue with Pass behaving like stop possibly related to this: #55432

I tested your project and sadly, no, this PR does not fix #55432, which is apparently related to input handling between Control Nodes. I will have a look at it soon.

@KoBeWi
Copy link
Member

KoBeWi commented Nov 29, 2021

This isn't really a bugfix, because it changes established Control behavior. It breaks compatibility and might break lots of things if it resolves #55432 too.

But seems like the original behavior causes lots of confusion, so I wonder if changing it isn't maybe good.

@golddotasksquestions
Copy link

The intended use for "Pass" is not to behave like "Stop". Why would it? We already have "Stop" for the stop behaviour.
If "Pass" is expected by users to behave as the name indicates (to pass) and behaves like "Stop" it clearly is a bug. The fact that it is known behaviour to some, does not make it less of a bug.

@KoBeWi
Copy link
Member

KoBeWi commented Nov 29, 2021

Except it does pass the input, but under different rules than most people think. So the only problem is that it's unexpected. Pretty sure there was a reason for the original behavior.

@BimDav
Copy link
Contributor Author

BimDav commented Nov 29, 2021

Except it does pass the input, but under different rules than most people think. So the only problem is that it's unexpected. Pretty sure there was a reason for the original behavior.

I had the exact same reaction in #54529, but by looking at the code I was not able to find this reason, except that it was a little easier to do it that way. On the topic of breaking compatibility, I agree that it may cause some problems, so we may want to wait for 3.5 or even 4.0.

@golddotasksquestions
Copy link

golddotasksquestions commented Nov 29, 2021

Except it does pass the input, but under different rules than most people think.

Except when it does not. Like in #55432 or #54529

The definition of a bug is not when something works, but when it does not as it should.
Users don't need to care what the underlying reason for an implementation is. Either the feature works according to the design of it does not. Mouse filter "Pass" clearly does not work in a lot of situations as it should (from a user perspective).

If a programmer intentionally implements a feature in such a way it contradicts it's design and documents it, the code might work and be seen as "bug free" from the perspective of the programmer, but it still won't work and will be seen as a bug for the user as long as it contradicts the features purpose and design.

@groud
Copy link
Member

groud commented May 18, 2022

Closing as superseded by #61088.
Sorry this was not reviewed in depth before (and I did not see it before working on my PR :/), as the new implementation is quite similar.

@groud groud closed this May 18, 2022
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.

Control consumes CollisionObject events for Area2D despite mouse_filter set to "pass"
5 participants