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 switch_on_hover for MenuButton #99849

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

Sauermann
Copy link
Contributor

Previously, embedded Windows (the opened menu) were not accounted for when checking for switch_on_hover.

gui_get_hovered_control() is more appropriate to check for the hover status of other MenuButton nodes at the mouse position.

Explain the usage of the incorrectly used function in a comment.

Copy link
Member

@Geometror Geometror left a comment

Choose a reason for hiding this comment

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

Looks good! Tested locally and I can confirm that this fixes the linked issue.

Edit: Maybe gui_find_control should be fixed to handle (embedded) subwindows since the former comment implies that this is a bug, but that can be done separately from this PR.

@Sauermann
Copy link
Contributor Author

Maybe gui_find_control should be fixed to handle (embedded) subwindows since the former comment implies that this is a bug, but that can be done separately from this PR.

I would prefer to not make such a change, because currently this function is used only in Viewport::_gui_input_event and Viewport::_update_mouse_over and as far as I can tell in both functions it is made sure, that there is no Window at that position. So checking for embedded windows in gui_find_control would make that function more complex, while the added complexity is currently not needed.

Previously, embedded Windows (the opened menu) were not accounted for
when checking for `switch_on_hover`.

`gui_get_hovered_control()` is more appropriate to check for the hover
status of other `MenuButton` nodes at the mouse position.

Explain the usage of the incorrectly used function in a comment.
@Sauermann Sauermann force-pushed the fix-menu-button-hover branch from 26f5976 to 9c5886f Compare December 16, 2024 21:11
@akien-mga akien-mga merged commit 98ca63a into godotengine:master Dec 17, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@Sauermann Sauermann deleted the fix-menu-button-hover branch December 18, 2024 00:46
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.

MenuButton's switch_on_hover property ignores its own popup menu.
3 participants