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.

Sign up for GitHub

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

Jump to bottom

Fix event propagation to child after set_as_toplevel #67507

Merged
merged 1 commit into from
Jan 31, 2023
Merged

Fix event propagation to child after set_as_toplevel #67507

merged 1 commit into from
Jan 31, 2023

Conversation

Sauermann
Copy link
Contributor

In certain conditions events did not get propagated to Control-childs of Node2D-nodes when setting a parent of the Node2D to toplevel because the Controls did not get added as root-control-nodes in the Viewport.
fix #42822

Analysis of the problem is available here: #42822 (comment)

This patch makes sure that such Control nodes become root-control-nodes in the Viewport.

This is done by propagating the info that toplevel status has changed on a parent to all descendant CanvasItems passing through Node2D nodes and executing the code of NOTIFICATION_EXIT_CANVAS and NOTIFICATION_ENTER_CANVAS on all first reached Control nodes.

@Sauermann
Fix set_as_toplevel event propagation to child
13b87c1 
In certain conditions events did not get propagated to Control childs of
Node2D nodes when setting a parent of the Node2D to toplevel.

This patch makes sure that such Control nodes become root control in the
viewport.
@Sauermann Sauermann requested review from a team as code owners October 16, 2022 20:04
@KoBeWi KoBeWi added this to the 4.0 milestone Oct 16, 2022
akien-mga
akien-mga approved these changes Jan 31, 2023
View reviewed changes
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Approved in PR review meeting.

@akien-mga akien-mga merged commit 4cfdd25 into godotengine:master Jan 31, 2023
@akien-mga
Copy link
Member

Thanks!

@Sauermann Sauermann deleted the fix-toplevel-root-control-node branch January 31, 2023 21:43
@Iniquitatis
Copy link
Contributor

Not quite related, but toplevel and top_level identifiers are a bit inconsistent across the code. Which one is correct?

@akien-mga
Copy link
Member

It should be top_level in master (was toplevel in 3.x, and we changed it for 4.0 but some code still uses toplevel it seems, including the one added here).

@Sauermann
Copy link
Contributor Author

I wasn't aware of this convention. Will fix it.

@Sauermann Sauermann mentioned this pull request Feb 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akien-mga akien-mga akien-mga approved these changes

Assignees
No one assigned
Labels
bug topic:input topic:2d
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.

Mixing Node2D and Control causes issues with set_as_toplevel
4 participants
@Sauermann @akien-mga @Iniquitatis @KoBeWi