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

Send mouse-enter notification, when viewport receives a mouse motion event #90444

Conversation

Sauermann
Copy link
Contributor

@Sauermann Sauermann commented Apr 9, 2024

Ensure, that a SubViewport receives a NOTIFICATION_VP_MOUSE_ENTER, when an InputEventMouseMotion is sent to a viewport, that currently doesn't have the mouse inside.

In the following case the changed code makes a difference: By default the viewport has the state, that the mouse is not over the viewport.
When the user pushes an InputEventMouseMotion to a SubViewport, then because of the default state, the Viewport assumes, that the mouse is not over itself and acts accordingly, leading to the behavior described in #89757.
With this change, the Viewport automatically switches its state.

Make necessary adjustments to unit-tests.

The behavior got introduced by #88992, which by itself was a step in the right direction, but caused undesired behavior.
This change would have introduced problems before #89920, because sometimes it was possible, that a Viewport received mouse motion events even when the mouse was outside of the viewport.
Resolve #89757 and also duplicate #90413. (I have verified the MRPs of both reports)
Supersedes #89868

Instead of this change, the problem can also be solved, by updating the documentation and explaining, that the user not only needs to push input events to SubViewports, but is also responsible for sending NOTIFICATION_VP_MOUSE_ENTER and NOTIFICATION_VP_MOUSE_EXIT when necessary.

While I believe, that this documentation change would be conceptually the correct way to go forward, it would be less user-friendy and I don't forsee direct problems with the approach of this PR, because before #88992 the SubViewports be default had as state gui.mouse_in_viewport == true.

All in all, this PR seems a bit hacky to me.

@Sauermann Sauermann added this to the 4.3 milestone Apr 9, 2024
@Sauermann Sauermann requested a review from a team as a code owner April 9, 2024 18:16
…event

Ensure, that a `SubViewport` receives a `NOTIFICATION_VP_MOUSE_ENTER`,
when an `InputEventMouseMotion` is sent to a viewport, that currently
doesn't have the mouse inside.

In the following case the changed code makes a difference:
By default the viewport has the state, that the mouse is not over
the viewport.
When the user pushes an `InputEventMouseMotion` to a `SubViewport`,
then because of the default state, the Viewport assumes, that the mouse
is not over itself.
With this change, the Viewport automatically switches its state.

Make necessary adjustments to unit-tests.
@Sauermann Sauermann force-pushed the fix-push-input-mouse-notification branch from aed2146 to 3e14688 Compare April 9, 2024 19:21
@Sauermann Sauermann requested a review from a team as a code owner April 9, 2024 19:21
@KoBeWi
Copy link
Member

KoBeWi commented Apr 29, 2024

In the MRP and without this PR, when you hold mouse button, the motion events are received. It's happening without NOTIFICATION_VP_MOUSE_ENTER and releasing the button stops motion from being received. I wonder why is that 🤔

While you say these changes are a hack, technically receiving a mouse event means the mouse is over viewport, so I think it's correct. Though the mouse over is never dropped, not sure if it has undesirable effects. Maybe it should be documented that you should manually drop mouse over if you pushed mouse events to viewport.

@Sauermann Sauermann modified the milestones: 4.3, 4.4 Jul 24, 2024
@Sauermann Sauermann modified the milestones: 4.4, 4.x Dec 1, 2024
@Sauermann Sauermann marked this pull request as draft December 1, 2024 00:11
@Sauermann
Copy link
Contributor Author

Superseded by #99890

@Sauermann Sauermann closed this Dec 1, 2024
@Sauermann Sauermann removed this from the 4.x milestone Dec 1, 2024
@Sauermann Sauermann deleted the fix-push-input-mouse-notification branch December 1, 2024 00:42
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.

Pushing events to Viewports no longer triggers InputEventMouseMotion in _gui_input
2 participants