Expand/collapse nodes recursively in scene tree dock

[Xrayez]

Jump to the most recent design proposal

Initial proposal with a button

Added a button to fold/unfold all of the nodes at selected node.

It will first check if any of the children are folded or not already. If any folded, it will first unfold them all. Then you have to press button for the second time to fold them all. If all of the children are unfolded, then you don't have to press the same button twice to collapse nodes.

While working on it I mostly haven't touched Nodes and only had to deal with Tree and TreeItem elements... makes me wonder whether TreeItem should have method like set_collapsed_recursive()...

All in all, it works fine.

[akien-mga]

The feature is nice, but I'm not convinced that putting it as a button in the top bar is a good idea. It's far from the location where you actually want to expand children recursively (the currently selected node), and most other such actions are provided in the right click popup.

[Xrayez]

Yeah, there could be several issues with the current design:

1. Having a new button there could be confusing when you reach for "Instance new Node" button instead.

2. Should the command try to expand or collapse children first? Currently one has to press the button twice in order to collapse children if any of the children are expanded.

3. If we remove the button, it might be better to create two separate commands for expanding and collapsing children, currently it has just one option combined:

   

4. If we separate the command to "Collapse" and "Expand", there are no "Expand" editor icon (at least I couldn't find such in the editor\icons\ folder).

5. We could leave and/or move the expand/collapse button there and it would act on the root node only, meaning "Collapse All" (idea taken from VS Code and similar editors):

   

If work should be done, is it ok to push commits to the branch and rebase later?

[Xrayez]

I made some improvements that solve some of the described usability issues (haven't pushed the changes yet, but the purposed design has allowed to simplify the code greatly as well):

1. In order to justify the importance of the button, I made it to only collapse nodes at the root. This means that all of the nodes get collapsed except for the root node, making its children visible by default (I don't see the point of seeing only the root node).

   

   It will also be very useful in case if you're too deep into the tree and the root is invisible, therefore making it difficult to reach. Having the button up there could help you reset the messy state of the tree without having to reach for the root node manually.

2. Split expand/collapse tool in the popup menu, making it explicit, working the same as before recursively expanding/collapsing nodes at selected node:

   

Demo:

Still need to find (or create) Expand editor icon...

[groud]

I'd rather not to have the button, it's already quite crowded in this area.
The menu entries are fine to me. :)

[groud]

Also, a single entry is better to me. As having too many options is bloat to me.
When collapsed -> expand all
When expanded -> collapse all

[Xrayez]

Bear in mind that if both options are combined, you'd have to call the popup menu twice in some cases with current implementation (because it checks whether any children are expanded/collapsed recursively to determine whether to expand or collapse first)...

Or it could be simplified to checking whether the selected item is expanded/collapsed without checking its children... So that:

• if the selected item is expanded (and only the selected item) -> collapse all children
• if the selected item is collapsed (and only the selected item) -> expand all children.
  (if that's what was meant)

That would simplify the implementation as well.

[groud]

Also, we can add a keyboard shortcut to solve the problem.

[toger5]

@groud @Xrayez yea I'm also advocating a shortcut. shift click seems to be totally obvious for me. if it is expanded collapse all and if it is collapsed expand all.
When it is partially expanded you can either use the RMB menu, or shift click it twice.

[Xrayez]

As suggested, I made it work to expand/collapse nodes by simply using a modifier shortcut. Just holding Shift button and clicking on the folding arrow will allow to expand/collapse nodes recursively (works exactly the same as single collapsing but recursive):

The popup menu option `Expand/Collapse All" is remained and allows to expand nodes first if not completely expanded, and collapse if all children expanded.

Needs rebase since I wanted to keep several versions.

[toger5]

Nice!
You were fast!. Also it is really good that you kept the popup.

[akien-mga]

That PR looks good to me now, could you squash the commits together and edit the resulting commit message to describe the final state?

[Xrayez]

Done. Also wanted to rename the branch to reflect the changes but figured it would trigger GitHub to close the PR!

[toger5]

Blender has a drag and collapse/"uncollapse" option.

If there is a good ux idea, that also would be worth a thought. (Ask if i need to be more precise in what i mean)

[Xrayez]

@toger5 You mean something like this?

The problem with implementing such feature is that dragging triggers the actual node dragging with LMB, and RMB alone triggers popup menu, so in either case the key modifier needs to be pressed to circumvent this behavior and to allow "drag-to-collapse/expand" feature, which kinda defeats the purpose imo.

Another problem might be that expanding a node might fill the scene tree dock entirely with its children so that "drag-to-collapse/expand" wouldn't be possible anymore for other nodes just because they will become out of reach in the scene tree dock (though this might be more suitable for instant collapsing).

[toger5]

I agree, that a key mod would be required.

Maybe this is also more suitable for the inspector.
It would be really fast to collapse/expand all, by going from the top to the bottom/ from the bottom to the top.

The scene tree than also should have the same behaviour, just for the sake of consitency.

[Xrayez]

Well the idea is good enough but I think this deserves another PR (which as of now is not a good idea because the engine is reaching feature freeze). What do you think @akien-mga? I could try implement suggested feature but I'm afraid this could take some time (if this "drag-to-collapse" feature is even desired at all).

[akien-mga]

Moving to 3.2 milestone as we're about to enter the beta stage and release freeze for Godot 3.1. Only bug fixes against issues of the 3.1 milestone (or enhancements to core features of the 3.1 roadmap) will be considered until 3.1-stable is released.

Note: If it's merged after 3.1-stable is released, this change might be considered for cherry-picking into a later 3.1.x release.

[Xrayez]

Resolved merge conflicts caused by recent feature by reduz (a20235a).

If scene tree and scripting editor are disabled, expand/collapse menu option will still be present:

Found this bug in the process #27866 which doesn't seem to be related to this PR.

[MunWolf]

Didn't see this when I reimplemented this in another PR. Only feedback I have here is that you could change it so the button also appears when you have multiple nodes selected and you just collapse / expand them all.

[Xrayez]

@MunWolf yeah, better to keep it in one place, besides I've tested it thoroughly and kept up with latest features added to the engine recently.

Only feedback I have here is that you could change it so the button also appears when you have multiple nodes selected and you just collapse / expand them all.

How does that differ from simply collapsing the whole branch? Unless you say you want to expand specific nodes on the branch.

[MunWolf]

Lets say you have a node with 5 child nodes all collapsed. This would mean you could select 2 or more of them with shift etc and expand them in one click, the only thing you would have to change is move the shortcut adding outside the if (selection.count == 1) and do a loop over the whole selection in your handling code.

[Xrayez]

Well I think this wouldn't hurt anything. I don't recall exactly but I think I've stumbled upon an issue with getting the actual nodes associated with tree items, so get_editor_selection() would not help much.

I'll take an opportunity and add you as a co-author since you've done some work as well, if you don't mind. 😃

[MunWolf]

You can if you want to but you have most of it here yourself ;)

Tree *tree = scene_tree->get_scene_tree();
if (tree->is_anything_selected()) {
	Node* root = EditorNode::get_singleton()->get_edited_scene();
	TreeItem *item = tree->get_selected();
	while (item != nullptr) {
		_set_fold_recursive(root, item, p_tool == TOOL_COLLAPSE_ALL);
		item = tree->get_next_selected(item);
	}
}

is the code I used to get the selection and set it recursively. You can have a look at _set_fold_recursive as well in my PR if you want to see how I get the Node* from TreeItem*

[Xrayez]

Oh I misread your message, you say that the actual button should appear when multiple nodes are selected. I've been removing and adding the button on the whim of anybody, so no consensus on this unfortunately...

[MunWolf]

Well I guess that depends on the ones with authority to decide then ;)

[Xrayez]

I've managed to fold/unfold all selected nodes in scene tree, but discovered that this could actually lead to unexpected behavior when selected nodes happen to be affected by recursive calls against parent nodes.

Details

case TOOL_EXPAND_COLLAPSE: {

	if (!scene_tree->get_selected())
		break;

	Tree *tree = scene_tree->get_scene_tree();
	TreeItem *selected_item = tree->get_selected();
	List<TreeItem *> selection;

	while (selected_item) {
		selection.push_back(selected_item);
		selected_item = tree->get_next_selected(selected_item);
	}

	if (selection.empty()) {
		selection.push_back(tree->get_root());
	}

	for (List<TreeItem *>::Element *E = selection.front(); E; E = E->next()) {

		bool collapsed = _is_collapsed_recursive(E->get());
		_set_collapsed_recursive(E->get(), !collapsed);
	}

	tree->ensure_cursor_is_visible();

} break;

This issue could be solved by sorting the selected items based on how deep they are in the scene tree, which would make the implementation quite complex for such a simple feature.

I personally don't see much issue with Shift-clicking multiple times on nodes that I want to unfold, if there are many nodes then it would be much easier to just fold/unfold the whole branch instead.

I didn't actually push any changes so the current version is merge-ready (added MunWolf as co-author, but for some reason GitHub can't detect...)

[Calinou]

I was looking to implement something like this recently (by using Ctrl + Click on the folding arrows, which seems to be a more common modifier in other software). Thanks for working on this feature 🙂

[Xrayez]

@Calinou I don't recall exactly but I avoided using Ctrl because one could accidentally select multiple nodes with this modifier. Same could be said regarding the use of Shift but it doesn't seem like it interferes with the scene tree dock much compared to other modifiers.

[akien-mga]

Thanks for your work and putting up with a slow review and merge process :)