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 scene reload crash related to mouse cursor update #69318

Merged
merged 1 commit into from
Apr 25, 2023

Conversation

Sauermann
Copy link
Contributor

@Sauermann Sauermann commented Nov 28, 2022

After a scene reload, a mouse cursor update is performed via a InputEventMouseMotion, that is exposed to the user. The state of Input is however not adjusted to this InputEventMouseMotion which can lead to inconsistencies:

  1. User requests scene change
  2. Mouse move event is sent
  3. User acts on mouse move event based on data from Input
  4. User requests scene change -> 1.

resolve #69227
Bugsquad edit: Fixes #71500
introduced in #58995

This PR makes sure, that the event is not exposed to the user. It makes sense, that events for internal concerns should not be exposed to the user.
It utilizes the same method that was already used in Viewport::_process_picking for marking events that are not sent to the user, so that this function doesn't need to be changed.

Updated 2023-02-10: Fix merge conflict

@Sauermann Sauermann requested review from a team as code owners November 28, 2022 22:30
@KoBeWi KoBeWi added this to the 4.0 milestone Nov 28, 2022
@Sauermann Sauermann force-pushed the fix-refresh-gui-events branch 4 times, most recently from 0e2ecac to f020198 Compare November 29, 2022 07:20
@Sauermann Sauermann changed the title Fix scene reload crash caused by mouse cursor update Fix scene reload crash related to mouse cursor update Dec 6, 2022
@Sauermann
Copy link
Contributor Author

This was discussed in a PR-review-meeting.
Makes generally sense, but will need to be reviewed in more detail.

After a scene reload a mouse cursor updates is performed via a
InputEventMouseMotion, that is exposed to the user.
The state of Input is however not adjusted to this InputEventMouseMotion
which can lead to inconsistencies.
This PR makes sure, that it is not exposed to the user.
It utilizes the method of Viewport::_process_picking for marking
events that are not sent to the user, so that this function doesn't
need to be changed.
@Sauermann Sauermann force-pushed the fix-refresh-gui-events branch from f020198 to 5bb66d3 Compare February 10, 2023 00:16
@akien-mga akien-mga modified the milestones: 4.0, 4.1 Feb 17, 2023
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

Seems fine.

I checked whether there are some internal events that could be relevant for the user and there are some Viewport events for unpressing mouse buttons. Probably not important tho 🤔

@akien-mga akien-mga requested a review from RandomShaper April 25, 2023 12:41
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.

Input is always tricky, with many corner cases and side effects. That said, this makes sense as far as I can tell without investigating everything myself.

@akien-mga akien-mga merged commit 6aac8af into godotengine:master Apr 25, 2023
@akien-mga
Copy link
Member

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.

InputEvent triggers immediately after reloading scene reload_current_scene crashes the game
5 participants