Add functionality to toggle visibility of TreeItems within a Tree

monkeyman192 commented 2 years ago

Describe the project you are working on

An audio editor/loader to load a common game audio pack format (wwise .bnk files). As part of this I am populating a tree with potentially 1000's of rows of data.

Describe the problem or limitation you are having in your project

To improve usability of my application I'd like to be able to filter the TreeItem's within the Tree so that it's easy to find the particular audio file or event the user wants. I was unable to find any easy way of filtering the entire tree without removing then re-adding nodes which was a little slow, as it required re-populating the entire tree every time just to keep everything in order. For large trees this becomes very cumbersome. Further I have a second tree which has links to the first (ie. clicking a row in one tree will select a row in the other tree). This is essentially impossible with the current restrictions as the referenced TreeItem will no longer exist if I filter the first tree.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

I propose a property and associated getter and setters:

bool visible

Setter	set_visible(value)
Getter	is_visible()

If true then the TreeItem will not be drawn in the tree. The TreeItem will also not be returned by the get_next_visible method and similar.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

I have done some basic work on trying to implement this, and the initial work to simply add the above methods and have it be visible or not works well and quickly. I am able to create a filterable tree very easily and it feels responsive even with 1000's of elements.

The getters/setters.

void TreeItem::set_visible(bool p_visible) {
    if (visible == p_visible || !tree) {
        return;
    }
    visible = p_visible;
    tree->update();

    _changed_notify();
}

bool TreeItem::is_visible() {
    return visible;
}

Using this is as simple as updating a line in the tree.cpp::Tree::draw_item method:

bool skip = ((p_item == root && hide_root) || !p_item->is_visible());

A number of other functions will need to be updated (I am still working on this) such as:

int Tree::compute_item_height(TreeItem *p_item) const {
    if ((p_item == root && hide_root) || !p_item->is_visible()) {
        return 0;
    }
...

If this enhancement will not be used often, can it be worked around with a few lines of script?

As far as I could find, there isn't really a good work around for this (especially in godot 3). In Godot 4 the new functionality to move a TreeItem from one Tree to another could be kind of made to work, but again, it is probably slower than this and also will make it hard to keep the tree in order when filtering it. Certainly not "a few lines of script".

Is there a reason why this should be core and not an add-on in the asset library?

This will require changes right in the TreeItem and Tree functions, so I don't think it's possible to implement this functionality with an add-on.

There are two other propsals which I would consider related to this one: https://github.com/godotengine/godot-proposals/issues/2088 https://github.com/godotengine/godot-proposals/issues/3607 The first is probably closer but doesn't actually cover this specific functionality I would like here.

I also think this should be in the core as it will allow for the concept of "filterable trees" to be implemented quickly and easily (as part of my testing I have loaded in a subset of the English dictionary and have a simple search bar at the top to filter by words. This however could be changed to filter by metadata or anything else associated with the row.

me2beats commented 2 years ago

This would be useful, now in Godot 3 I simply clear the tree and create/add items again if I want to filter them. But this isn't convenient.

monkeyman192 commented 2 years ago

Ok. I have got the code working well I think. My simple test program works really nicely. I see there are protocols for opening a PR, and I'd need to add documentation and probably unit tests to this functionality as well as probably fix inevitable failures due to the test, but am I free to open a PR (once aforementioned things are done), or is there more discussion needed here first?

Calinou commented 2 years ago

Ok. I have got the code working well I think. My simple test program works really nicely. I see there are protocols for opening a PR, and I'd need to add documentation and probably unit tests to this functionality as well as probably fix inevitable failures due to the test, but am I free to open a PR (once aforementioned things are done), or is there more discussion needed here first?

You can open a pull request directly :slightly_smiling_face:

Remember that feature development should happen on master first, then on 3.x if the feature can be backported without breaking compatibility.

godotengine / godot-proposals