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

Use MOUSE_FILTER_PASS for all containers #35068

Merged

Conversation

akien-mga
Copy link
Member

Containers are meant to forward mouse input to their the Controls
they contain.

Fixes #34933.


Putting this up for discussion, I haven't thoroughly tested yet.

Apart from GridContainer and BoxContainer-derived classes which already defaulted to PASS, this changes the behavior for CenterContainer, MarginContainer, PanelContainer, ScrollContainer, SplitContainer, TabContainer, ViewportContainer and the base Container. Might have unforeseen consequences if nobody noticed this discrepancy in 6 years.

@akien-mga
Copy link
Member Author

Related: #24083 (but that one would be for 4.0).

@YeldhamDev
Copy link
Member

Parts of the editor code that sets the filtering to pass in containers should be removed too, these are the ones that I found at first glance:

audioprev_hbc->set_mouse_filter(MOUSE_FILTER_PASS);

library_vb_border->set_mouse_filter(MOUSE_FILTER_PASS);

@akien-mga
Copy link
Member Author

Thanks, removed all occurrences I found in editor/ and scene/ (notably GraphNode which is a container too).

@@ -1335,7 +1334,6 @@ GraphEdit::GraphEdit() {
top_layer->set_mouse_filter(MOUSE_FILTER_PASS);
top_layer->set_anchors_and_margins_preset(Control::PRESET_WIDE);
top_layer->connect("draw", this, "_top_layer_draw");
top_layer->set_mouse_filter(MOUSE_FILTER_PASS);
Copy link
Member Author

Choose a reason for hiding this comment

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

This one is not a Container, but the mouse filter was already set to Pass on line 1335.

Containers are meant to forward mouse input to their the Controls
they contain.

PanelContainer has a visible Panel stylebox, so it still defaults
to STOP.

Fixes godotengine#34933.
@akien-mga akien-mga modified the milestones: 3.2, 4.0 Jan 13, 2020
@akien-mga akien-mga merged commit f003b3e into godotengine:master Feb 6, 2020
@akien-mga akien-mga deleted the containers-mouse-filter-pass branch February 6, 2020 11:23
akien-mga added a commit to akien-mga/godot that referenced this pull request Feb 7, 2020
PR godotengine#35068 made Container (which GraphNode inherits) default to
MOUSE_FILTER_PASS, so I removed the manual override, but it turns out
that GraphNode's constructor still overrides it to MOUSE_FILTER_STOP.

Another fix could be to remove the STOP in the constructor, but I don't
know if it's there for a specific reason (e.g. to have GraphNodes STOP
by default, but PASS in a specific case).

Fixes godotengine#35978.
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.

CenterContainer blocks parent from receiving drag and drop
2 participants