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

Don't return Tree items outside visible rect #102262

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Jan 31, 2025

Fixes #102146
Likely needs some testing.

@KoBeWi KoBeWi added this to the 4.4 milestone Jan 31, 2025
@KoBeWi KoBeWi requested a review from a team as a code owner January 31, 2025 21:07
scene/gui/tree.cpp Outdated Show resolved Hide resolved
@KoBeWi KoBeWi force-pushed the outer_tree_is_void branch from ac9f483 to abdef36 Compare February 1, 2025 21:09
@KoBeWi
Copy link
Member Author

KoBeWi commented Feb 1, 2025

Added checks and early returns to all at_position methods, but it increased the scope a bit...
(also lots of noise due to indentation changes)

scene/gui/tree.cpp Outdated Show resolved Hide resolved
@KoBeWi KoBeWi force-pushed the outer_tree_is_void branch from abdef36 to 873142a Compare February 1, 2025 22:10
@KoBeWi KoBeWi force-pushed the outer_tree_is_void branch from 873142a to c14a57a Compare February 1, 2025 22:18
Copy link
Member

@kleonc kleonc left a comment

Choose a reason for hiding this comment

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

Overall LGTM, one more nitpick.

TreeItem *Tree::_find_item_at_pos(TreeItem *p_item, const Point2 &p_pos, int &r_column, int &r_height, int &r_section) const {
r_column = -1;
r_height = 0;
r_section = -1;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
r_section = -1;
r_section = -100;

-1 is a valid section indicator, get_drop_section_at_position returns -100 as invalid one, these should be consistent.
(and yeah, they could be constants...)

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.

Drag & Drop file to FileSystem dock saves it in the underlying directory behind the opened directory
2 participants