godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.13k stars 92 forks source link

Add Tree `Multi Row` select mode #2161

Open me2beats opened 3 years ago

me2beats commented 3 years ago

Describe the project you are working on

Godot Editor plugins

Describe the problem or limitation you are having in your project

I need a Tree where user can easily select multiple items (multiple rows): when I select TreeItem any row, I need all its columns to be selected Just like SelectMode Row, but also with ability to select multiple rows. Now I have to use SelectMode Multi, but I find it a bit confusing for my current task.

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

Having Multi Row SelectMode would solve the problem

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

Just combine Multi and Row modes.

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

I believe this can be work-rounded but:

So I think the proposed mode is a "missing link"

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

see above

plyoung commented 9 months ago

Here is a simple workaround for those needing the described function. Sample in C# but should be same idea for gdscript.

// Hook to tree view multiselect in some init code, like _EnterTree
treeView.MultiSelected += OnMultiSelected;

// In there set a flag to check later
private void OnMultiSelected(TreeItem item, long column, bool selected)
{
    selectionChanged = true;
}

// check the flag in process. the idea is to wait until all the OnMultiSelected calls for this/(prev?) frame is done
public override void _Process(double dt)
{
    if (selectionChanged)
    {
        UpdateSelection();
    }
}

// now you can select each cell and perhaps update some var keeping track of what is selected
private void UpdateSelection()
{
    selectionChanged = false;

    selection.Clear();
    var row = treeView.GetNextSelected(null);
    while (row != null)
    {
        selection.Add(row);
        row = treeView.GetNextSelected(row);
    }

    // this is where the cells are set selected
    // c__max is just the max columns defined
    foreach (var selrow in selection)
    {
        for (int i = 0; i < c__max; i++) selrow.Select(i);
    }

    // maybe make a call to something which wants to know when selection changed
    UpdateInfoView();
}
Nlight91 commented 4 months ago

I second this request, plus the feature work is somehow already largely done with row_select so...

AThousandShips commented 4 months ago

@Nlight91 Please don't bump without contributing significant new information. Use the :+1: reaction button on the first post instead.

Nlight91 commented 4 months ago

I don't if you'd consider this new information... I don't consider it is, but I identified the portion of source where a patch could be made, that could give some idea to some, cause I know we dev can be lazy to dive into the source to find the problematic bit.

So here https://github.com/godotengine/godot/blob/master/scene/gui/tree.cpp function select_single_item() :

void Tree::select_single_item(TreeItem *p_selected, TreeItem *p_current, int p_col, TreeItem *p_prev, bool *r_in_range, bool p_force_deselect) {
    popup_editor->hide();

    TreeItem::Cell &selected_cell = p_selected->cells.write[p_col];

    bool switched = false;
    if (r_in_range && !*r_in_range && (p_current == p_selected || p_current == p_prev)) {
        *r_in_range = true;
        switched = true;
    }

    bool emitted_row = false;

    for (int i = 0; i < columns.size(); i++) {
        TreeItem::Cell &c = p_current->cells.write[i];

        if (!c.selectable) {
            continue;
        }

        if (select_mode == SELECT_ROW) {
            if (p_selected == p_current && (!c.selected || allow_reselect)) {
                c.selected = true;
                selected_item = p_selected;
                if (!emitted_row) {
                    emit_signal(SNAME("item_selected"));
                    emitted_row = true;
                }
            } else if (c.selected) {
                if (p_selected != p_current) {
                    // Deselect other rows.
                    c.selected = false;
                }
            }
            if (&selected_cell == &c) {
                selected_col = i;
            }
        } else if (select_mode == SELECT_SINGLE || select_mode == SELECT_MULTI) {
            if (!r_in_range && &selected_cell == &c) {
                if (!selected_cell.selected || allow_reselect) {
                    selected_cell.selected = true;

                    selected_item = p_selected;
                    selected_col = i;

                    emit_signal(SNAME("cell_selected"));
                    if (select_mode == SELECT_MULTI) {
                        emit_signal(SNAME("multi_selected"), p_current, i, true);
                    } else if (select_mode == SELECT_SINGLE) {
                        emit_signal(SNAME("item_selected"));
                    }

                } else if (select_mode == SELECT_MULTI && (selected_item != p_selected || selected_col != i)) {
                    selected_item = p_selected;
                    selected_col = i;
                    emit_signal(SNAME("cell_selected"));
                }
            } else {
                if (r_in_range && *r_in_range && !p_force_deselect) {
                    if (!c.selected && c.selectable) {
                        c.selected = true;
                        emit_signal(SNAME("multi_selected"), p_current, i, true);
                    }

                } else if (!r_in_range || p_force_deselect) {
                    if (select_mode == SELECT_MULTI && c.selected) {
                        emit_signal(SNAME("multi_selected"), p_current, i, false);
                    }
                    c.selected = false;
                }
                //p_current->deselected_signal.call(p_col);
            }
        }
    }

    if (!switched && r_in_range && *r_in_range && (p_current == p_selected || p_current == p_prev)) {
        *r_in_range = false;
    }

    TreeItem *c = p_current->first_child;

    while (c) {
        select_single_item(p_selected, c, p_col, p_prev, r_in_range, p_current->is_collapsed() || p_force_deselect);
        c = c->next;
    }
}

Rect2 Tree::search_item_rect(TreeItem *p_from, TreeItem *p_item) {
    return Rect2();
}

As I don't speak c++, Irm not sure about the meaning the arrow, but I'll find out later. I don't know how to make and propose a patch. But I'll see if I can come up with a code proposal. Now, someone more experienced than I (and there's a plethora of them), can quickly see that first a new entry should be add in an enum or a variable somehere else, something like SELECT_MULTI_ROW. reflect some of the logic from the else if (select_mode == SELECT_SINGLE || select_mode == SELECT_MULTI) part into the if (select_mode == SELECT_ROW) part.

Hopefully we can come up with something with just focusing efforts on that function

EDIT : Jesus, of course it could not have been that simple... there are other functions to look at :

Tree::draw_item()
Tree::gui_input()
Tree::edit_selected()
Tree::item_deselected()
Tree::deselect_all()
Tree::ensure_cursor_is_visible()

and gui_input() is a heavy cookie... damn...