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

Fix physics passive hovering with MOUSE_FILTER_IGNORE #79443

Merged
merged 1 commit into from
Jul 26, 2023

Conversation

Sauermann
Copy link
Contributor

@Sauermann Sauermann commented Jul 13, 2023

When the parent SubViewportContainer ignores mouse, then the SubViewport shouldn't create mouse-move-events for passive hovering.

resolve #79441
follow-up to #78017

Updated 2023-07-18: Fix also passive hovering during Drag and Drop as mentioned in #75120 (comment)

@Sauermann Sauermann added this to the 4.2 milestone Jul 13, 2023
@Sauermann Sauermann requested a review from a team as a code owner July 13, 2023 21:26
@Sauermann Sauermann force-pushed the fix-ingore-hovering branch 2 times, most recently from 8ffda10 to ac99e85 Compare July 14, 2023 05:27
Copy link
Member

@RandomShaper RandomShaper left a comment

Choose a reason for hiding this comment

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

When parent_ignore_mouse is true, the for loop can be skipped.

@Sauermann
Copy link
Contributor Author

I'm not entirely sure, if that is correct, because the loop covers also non-mouse events, like InputEventScreenDrag.
Skipping the loop in the case parent_ignore_mouse == true would mean, that InputEventScreenDrag-events don't get evaluated.
Also ... skipping would have the effect, that these events stay in Viewort::physics_picking_events to be executed during the next physics-frame(s).

@RandomShaper
Copy link
Member

My point was about putting the loop inside an if because it was only used to determine if has_mouse_event should be true. If other conditions didn't meet, the value of has_mouse_event was unused. But you have already changed the code, eliminating the loop altogether, and I can't say for sure now.

@Sauermann
Copy link
Contributor Author

Now I understand you. I was talking about a different loop, than you were.
With that in mind, I agree and will update the PR.

When the parent `SubViewportContainer` ignores mouse with
`MOUSE_FILTER_IGNORE` and also when the mouse is over a `Control`-node,
then the `SubViewport` shouldn't create mouse-move-events for passive
hovering.
@Sauermann Sauermann force-pushed the fix-ingore-hovering branch from 3e2c729 to b408b05 Compare July 19, 2023 17:27
Copy link
Member

@RandomShaper RandomShaper left a comment

Choose a reason for hiding this comment

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

Isn't the wording of the various filter modes inaccurate as they say things like "the control will not receive mouse button input [...]"? I mean, it's not only about buttons but also movement, isn't it?

@YuriSizov YuriSizov merged commit 1de9171 into godotengine:master Jul 26, 2023
@YuriSizov
Copy link
Contributor

Thanks!

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.

Physics Mouse Picking happens in SubViewport even with MOUSE_FILTER_IGNORE
3 participants