Skip to content

Tree recursive folding (like Scene Tree Dock)#62666

Merged
akien-mga merged 1 commit into
godotengine:masterfrom
AThousandShips:tree_folding
Sep 21, 2022
Merged

Tree recursive folding (like Scene Tree Dock)#62666
akien-mga merged 1 commit into
godotengine:masterfrom
AThousandShips:tree_folding

Conversation

@AThousandShips
Copy link
Copy Markdown
Member

@AThousandShips AThousandShips commented Jul 3, 2022

Moves the behavior of Scene Tree Dock recursively folding of trees into TreeItem:

  • Adds function set_collapsed_recursive to collapse/uncollapse a TreeItem and all its descendants
  • Adds function is_any_collapsed to check if a TreeItem (with children) or any of its children (with children) are collapsed
  • Shift clicking the fold collapses/uncollapses the entire tree under the TreeItem, depending on the fold state of the TreeItem
  • Ads disable property for this input behavior to TreeItem

Replaced the functionality in Scene Tree Dock with this, and I haven't found anywhere else it is used but if anyone does please point it out and I can remove it there as well.

(The original approach in Scene Tree Dock was doing some redundant folding on shift click as it would fold the TreeItem and all descendants, but this would trigger each of those descendants to fold recursively on their own, not major and mostly unavoidable with the approach used, but it is a thing)

@AThousandShips AThousandShips requested a review from a team as a code owner July 3, 2022 10:50
@AThousandShips AThousandShips requested a review from a team July 3, 2022 10:50
@AThousandShips AThousandShips requested a review from a team as a code owner July 3, 2022 10:50
@YuriSizov YuriSizov added this to the 4.0 milestone Jul 3, 2022
@AThousandShips
Copy link
Copy Markdown
Member Author

I went with is_any_collapsed over the is_collapsed_recursive of Scene Tree Dock as I felt it was ambiguous, as it could be seen as meaning "is this node and all descendants collapsed"

Comment thread doc/classes/TreeItem.xml Outdated
@AThousandShips
Copy link
Copy Markdown
Member Author

AThousandShips commented Jul 3, 2022

Wasn't aware of the doctool check but now I know to verify that (sorry only fixed what I immediately saw in the failed check before I ran it), this shoudl be correct

@akien-mga
Copy link
Copy Markdown
Member

Related (very old) PR: #36926.

@AThousandShips
Copy link
Copy Markdown
Member Author

AThousandShips commented Jul 3, 2022

Intriguing! I thought I had exhaustively searched for this but must have missed this old case

@AThousandShips
Copy link
Copy Markdown
Member Author

I'm not familiar with the process when something like this occurs is, so any directions would be appreciated

@AThousandShips AThousandShips force-pushed the tree_folding branch 3 times, most recently from 876f1d9 to 64ea5d5 Compare July 4, 2022 06:44
@AThousandShips
Copy link
Copy Markdown
Member Author

This should also fix #35529

@AThousandShips
Copy link
Copy Markdown
Member Author

Moved the disabling of recursive folding to the tree as I felt it doesn't make much sense in TreeItem, as one would have to disable it everywhere, and it would be confusing to decide what way that would control things anyway

Also added the option to limit is_any_collapsed to visible items only as it would otherwise cause some confusing behavior if you have collapsed but invisible items (an edge case but I felt it was useful, option defaults to false)

Will push once I've checked the last details on my end

@AThousandShips AThousandShips force-pushed the tree_folding branch 2 times, most recently from 003d3e2 to 0c769a9 Compare July 4, 2022 11:35
@AThousandShips
Copy link
Copy Markdown
Member Author

AThousandShips commented Jul 4, 2022

Note that with the addition of disabling it from the tree the original behavior in Scene Tree Dock could be kept for a subsequent PR by simply disabling it on the tree in Scene Tree, though I'm confident in my solution replicating previous the behavior reliably, i.e. if felt that the changes to Scene Tree Dock to accomodate this change should be done separately or by someone else

@AThousandShips AThousandShips force-pushed the tree_folding branch 2 times, most recently from 78342c3 to dbd439b Compare July 5, 2022 06:19
@AThousandShips
Copy link
Copy Markdown
Member Author

Made the context menu option in Filesystem Dock use recursive folding, leaving the instances in Visual Shader Editor alone as it is only one level deep, haven't found any other instance like this with a menu (will push once I've compiled locally)

@AThousandShips AThousandShips force-pushed the tree_folding branch 12 times, most recently from f6b75c1 to 1284fe2 Compare July 13, 2022 09:03
Comment thread doc/classes/TreeItem.xml Outdated
Comment thread doc/classes/TreeItem.xml Outdated
Comment thread scene/gui/tree.cpp Outdated
@KoBeWi
Copy link
Copy Markdown
Member

KoBeWi commented Sep 20, 2022

Looks good, but I'd rename disable_recursive_folding to enable_recursive_folding (true by default) and mention Shift in the docs.

@AThousandShips
Copy link
Copy Markdown
Member Author

Thank you, will take a look at all this tomorrow probably not doing so well today

Comment thread doc/classes/Tree.xml Outdated
@akien-mga akien-mga merged commit 057dd29 into godotengine:master Sep 21, 2022
@AThousandShips AThousandShips deleted the tree_folding branch September 21, 2022 13:43
@akien-mga
Copy link
Copy Markdown
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

5 participants