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 Tree::deselect_all not deselecting root #71405

Merged

Conversation

marzecdawid
Copy link
Contributor

As reported here: #20548 (comment)
It fixes deselecting the root.
Also after #71306 will be fixed the deselect_all does not need to iterate through every column in SELECT_ROW mode.

@marzecdawid marzecdawid requested a review from a team as a code owner January 14, 2023 16:12
@Calinou Calinou added this to the 4.0 milestone Jan 19, 2023
@YuriSizov YuriSizov modified the milestones: 4.0, 4.1 Feb 17, 2023
scene/gui/tree.cpp Outdated Show resolved Hide resolved
scene/gui/tree.cpp Outdated Show resolved Hide resolved
@marzecdawid marzecdawid force-pushed the deselect-root-in-deselect_all branch 2 times, most recently from 2154163 to 0f69433 Compare March 25, 2023 14:23
@marzecdawid marzecdawid force-pushed the deselect-root-in-deselect_all branch from 0f69433 to 45930e9 Compare March 25, 2023 15:03
@marzecdawid
Copy link
Contributor Author

@YuriSizov I changed the code to use root everywhere, removed the is_selected check as deselect() is not slow so it is rather unnecessary as you noted. I also moved the root deselecting to the other loop so it also uses the optimisation for SELECT_ROW mode. In this case it can have some small impact when you have a lot of columns, but I'm not expert to be able to evaluate it well.

@YuriSizov YuriSizov merged commit c0301b7 into godotengine:master Mar 27, 2023
@YuriSizov
Copy link
Contributor

Looks good to me now, thanks!

@marzecdawid marzecdawid deleted the deselect-root-in-deselect_all branch April 1, 2023 15:58
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.

3 participants