Improve TreeItem API and allow to move nodes

[trollodel]

Partially fixes godotengine/godot-proposals#2088
Fixes #19796 by renaming get_children to get_first_child, as suggested in #19796 and in #16863 3 times.

This PR adds:

• Some getters to reduce the number of manual iteration over the tree.
• Tree get_tree().
• get_first_child to replace get_children.
• TreeItem.create_child.
• A pointer to the previous item to speed up get_prev() (the old code still exists but now is an edge case).
• move_before and move_after for moving items (see below), and remove move_to_top and move_to_bottom as they are not necessary anymore.

The new move_* methods allows moving items over the tree and (unlikely the current system), allow reparent and even moving the item and its children to another tree.
Those methods move self before (or after) the specified item. For me, this is the simplest (and less error-prone) approach, but I'm open to suggestions.

An example of cross-tree move

@tool
extends Control

@onready var root: TreeItem = $Tree.create_item()
@onready var level1: TreeItem = $Tree.create_item()
@onready var level2: TreeItem = level1.create_child()

func _ready():
    root.set_text(0, "Root")
    level1.set_text(0, "Level 1")
    level2.set_text(0, "Level 2")
    
    $Tree2.create_item()
    var dummy: TreeItem = $Tree2.create_item()
    await get_tree().create_timer(3.0).timeout
    level1.move_after(dummy)
    dummy.free()

[groud]

It looks good to me. My only worry would be about making sure that TreeItems are always freed correctly, but it' seems that there is no way to make them orphan (without a tree).
So I guess it should be fine.

[groud]

I think get_first_sibling and get_last_sibling should be remove.

Sorry for the double review I messed up the first one (you cannot edit the review status)

[groud]

Looks overral good to me.

My only concern would be the efficiency get_child_count/get_index/etc... TreeItem might benefit of using an internal array of pointers to all its children, that would avoid the need to iterate over all children every time. Those pointers would need to be updated, but I don't think it's a big deal since TreeItem is internal anyway.

[trollodel]

TreeItem might benefit of using an internal array of pointers to all its children, that would avoid the need to iterate over all children every time.

Implemented. Now when you call get_child, get_child_count and get_children, children are cached (the first time it iterates over all the children to construct the cache). And the cache is updated efficiently when adding and removing nodes.
But, the cache is not populated until a call to the methods listed above is done, so the cache is not populated if it is not necessary.

[groud]

Needs a rebase but looks good to me overall.

[akien-mga]

Thanks!

[trollodel]

I don't think this PR is enough to close godotengine/godot-proposals#2088. It is missing some feature listed there, like drag 'n' drop.
Also, should it be added in #16863?

[akien-mga]

I don't think this PR is enough to close godotengine/godot-proposals#2088. It is missing some feature listed there, like drag 'n' drop.

Reopened, but note that it was closed because you used a keyword that triggers closure:

See https://docs.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

Also, should it be added in #16863?

What should be added? This list is mostly TODOs, not an exhaustive overview of renames that did happen (there's a lot more than what the list includes).

[trollodel]

Reopened, but note that it was closed because you used a keyword that triggers closure

Thanks for reopening. I thought GH is smarter 😕.

This list is mostly TODOs, not an exhaustive overview of renames that did happen (there's a lot more than what the list includes).

Oh, ok. It's not important to me, especially if it not a tracker of all breaking changes.

[CsloudX]

I think it should be add_child and move_child like node's method.
LOOK:

|-TreeRoot
  |-NodeA
    |-Child
  |-NodeB

How can I move Child to as a NodeB's child by move_before and move_after, like this:

|-TreeRoot
  |-NodeA
  |-NodeB
    |-Child

[trollodel]

@CsloudX you can add a dummy item to NodeB, move_after Child to the dummy and then remove the dummy.
It's a workaround through, so it makes sense to have a way to handle that case. I'm a bit worried about performance of an add_node style method that appends the item to the end, especially when you have several items.
I'm not a maintainer, but can you open a proposal about this?

[CsloudX]

@trollodel Thanks

[NoodleSushi]

Is it possible to backport this feature to Godot 3?

[Calinou]

Is it possible to backport this feature to Godot 3?

This PR includes many compatibility-breaking changes, and therefore can't be backported as-is to Godot 3.x.

Feel free to work on a backport of the backwards-compatible features yourself, still.