Godot: TreeItem.get_children() iteration is non-intuitive

[valentinAlkan]

Godot version:
3.02stable.mono.official

OS/device including version:
Windows 10

Issue description:
Iterating over TreeItem.get_children() is counter intuitive and non pythonic. The docs state:

TreeItem get_children ( )
Returns the TreeItem’s child items.

But this is not what happens. One would expect that the function will return an array of children, since the name of the function is get_children and not get_child. Since the return value is a single TreeItem, attempts to iterate over the set of children can't succeed.

The docs do also state

TreeItem get_next ( )
Returns the next TreeItem in the tree.

But this doesn't mean much, and only after some pain does it become apparent that one needs to call get_children (to return a single child...) and then call get_next() to return the next child, and so on, until there are no more children, to wit:

var item = itemTree.get_root().get_children()
while (item):
    item = item.get_next()

This is a very odd pattern, and not one that I run into much anymore, especially not these days, and especially not in pythonic languages.

A far more intuitive interface (and one that would be more consistent with both python and GDScript) is for TreeItem.get_children() to return an array of child TreeItems. Then one could simply use a normal for loop iteration and not have to refer back to old projects to see how I did it a long time ago (cough cough).

While this does introduce the problem that inserting/removing TreeItems from within a For loop can cause unexpected behaviour (as a TreeItem that was just removed from the Tree would still exist as a reference in the list of children), that is up to the programmer to manage, and is what a python coder would expect to happen. The current method is confusing to everyone, while the proposed change may prove confusing, but only to relative newcomers.

Steps to reproduce:

var children = itemTree.get_root().get_children()
for child in children:
    do_something()

Produces error 'Unable to iterate on object of type Object', as a TreeItem is return, rather than an array of TreeItems.

Minimal reproduction project:

[Ranoller]

Yes, it´s very strange to use... but once you are used to, it´s easy to think in get down or get up the tree (get_children, get_next, get_parent or directly get_root) I got some plugins with tree, if somebody change that, can make me cry to strong...

The docs aren´t helpful, I explain:

TreeItem get_children ( )
Returns the TreeItem’s child items.

Should be something like that:

TreeItem get_children ( )
Returns first of the TreeItem’s child items, or null if the item don´t have any child.

Probably get_children should be renamed to get_child(index) or get_first_child(), and get_children should return an array of tree items. That it´s more coherent with the godot api....

More problematic is that you can´t get an item by it´s index, so inserting, deleting items is always by recursion o iterating (if you want to insert in exact position):
If create_item is by parent and index, treeitem should have "index" property.

But really, i think that this kind of things (breaking the api once and once over functionality that works....) should be only accomplished over a very strong and bug semi-free api, and now it´s not the case. Change that you propose break compatibility inside scripting and inside editor, it isn´t?

[valentinAlkan]

Change that you propose break compatibility inside scripting and inside editor, it isn´t?

Yes. Its perhaps best to save it for a 4.0 release. However I dont think too many people use Trees, as I would expect this to have been pointed out by now.

I mostly just wanted some documentation out there about this behavior for anyone who has issues with it. The docs do not explain how this is supposed to work.

[Ranoller]

Godot have now a problem with the docs... api grows, functionality change and doc´s sometimes don´t string along with...... There are a lot of methods that doesn´t have any description, and some method´s don´t explain it´s own parameters or have wrong return type... This is difficult to fix if you don´t have a deep knowledge of the API (I don´t).... So, when adding functionality to 3.1 ends i expect all developers to center in bug fix and docs... but you know, free software - free developers!!!

And you are right, no much people uses tree, nor editor plugins.... but really and sadly, no much people uses godot jet. My impressions are that indie developers are used to the oposite that godot does, exactly that game maker does, mantain compatibility and unuseful or aged functionality until death, this give a kind of sensation of "you must trust in this piece of software" that i think people desire (wrong of not).

[christoffersch]

I agree that Tree is very odd to work with.

Other thoughts:

• Getting a specific TreeItem by its text would also be a great addition
• Removing a TreeItem from a Tree requires the use of RemoveChild(), which can be confused by the Node function RemoveChild() - maybe renaming it to RemoveItem()?
• How about having the ability to add TreeItems to another TreeItem, making that a new parent?

[valentinAlkan]

How about having the ability to add TreeItems to another TreeItem, making that a new parent?

You can do this already, no?

https://godot.readthedocs.io/en/3.0/classes/class_tree.html#class-tree-create-item

[Ranoller]

Yes, a tree item can be the parent of other tree item. This is the way to do the tree structure.

[christoffersch]

@valentinAlkan @Ranoller My bad ;-)

[valentinAlkan]

no worries! thought I was confused for a minute lol

[TheDuriel]

Yeah this system needs changing.

func _retrieve_load_order() -> void:

    var item: TreeItem = _tree.get_root().get_children()

    _load_order.clear()
    var pos: int = 0

    while is_instance_valid(item):
        var mod_config: Dictionary = item.get_metadata(0)
        mod_config.position = pos

        _load_order.append(mod_config)

        pos += 1
        item = item.get_next()

seems to be the way to iterate though direct children. not intuitive.

[pycbouh]

I guess the main issue here is in the naming. get_children implies a collection of instances to be returned, not a single instance with further iteration. Since 4.0 roadmap has breaking changes to naming (#16863), maybe the simplest solution here is to have a more appropriate name for this function. The pattern itself I don't have a problem with.

[me2beats]

then imo get_children() should be called get_first_child() or something like that
because now it returns ItemList, even not an array.

I ran into this bug too, trying to get the immediate children of a tree item.

[me2beats]

that is to get a tree item real children, I need to do something like this:

func get_tree_item_children(item: TreeItem):
    var children: = []
    var child = item.get_children()
    if not child: return []

    children.append(child)

    while true:
        child = child.get_next()
        if not child: break
        children.append(child)

    return children

And for example to get tree item child by text- like this:

func get_tree_item_child_by_text(item:TreeItem, text: String):
    var child = item.get_children()
    if not child: return
    if child.get_text(0) == text:
        return child

    while true:
        child = child.get_next()
        if not child: return

        if child.get_text(0) == text:
            return child