[WIP] Multiple Tree Control issues

[follower]

[WIP: This is mostly a very quick dump of multiple issues from my notes about multiple issues with the Tree control--particularly in relation to keyboard navigation.

This write-up has been sitting in a tab for a couple of weeks & I diagnosed the original issues back in May but my "TODO" list isn't getting any smaller. :/

The primary motivation for posting this as is right now is because I've noticed that the Tree issues have been encountered by someone who is working on improving accessibility and hopefully the pointers/notes here will be better than nothing even in this state.]

Godot version: ~v3.1.0+

OS/device including version: N/A

Issue description:

• Multiple issues (particularly in relation to keyboard navigation) in the Tree control appear to have been introduced when moving from key press processing to action processing in this commit/PR:

  • "Tree now uses UI actions instead of keys " https://github.com/godotengine/godot/commit/e3eb6869068edb2617a311f89e93b92a9aa5b905 a.k.a https://github.com/godotengine/godot/pull/16947

  From one of the comments it seems the PR may not have been reviewed very closely--which was unfortunate given that tree controls are probably one of the more complex common UI controls.

  (Side note: Given the complexity of the control & implementation I'd recommend that a Tree control specific regression test example project/test script be created to help prevent this situation reoccurring in future. There's a huge combinatorial effect here when considering (mouse control + keyboard control) * (cell select + row select + multi-select) * (custom controls + non-selectable controls + confusing terminology) etc etc).

  Issues related to this PR include:

  (1) Setting cells to be "non-selectable" appears to break keyboard navigation.

[TODO: Finish this list from these notes:

# BUG: "selected" naming vs focus & "selected" vs "get_next_selected"
# BUG: Handling non-selectable cells--both selection & cursor focus.
# BUG: Keyboard selecting.
# BUG: Multi-select doesn't emit correct select signals on initial non-mouse selection.
# ~BUG: `set_as_cursor` isn't exposed to GDScript. Which means ~only way to set `selection_item` is via `ui_down` injection.
# BUG: Multi-select LMB click moves cursor column but not cursor row.
# BUG: Multi-select only selects entire rows, not cells in rows, or two partial rows.
# ~BUG: `selected_item = 0` vs `selected_col`
# BUG: Space bar/search issue.
# BUG: Handling of centering/hseparation etc?
# BUG: ...?
]

• Other issues:

  (1) Incorrect text centre alignment due to "The default hseparation value messes with an (apparent) incorrect centering calculation in Tree.cpp." (quoting myself :D )

  (2) Workaround is required to set the cursor position without also selecting the cell in multi-select mode (as set_as_cursor() is not exposed to GDScript).

Steps to reproduce:

Minimal reproduction project:

[follower]

Click disclosure triangle to see example code for the workaround I implemented for some of the issues I encountered

``` func _on_DataTreeHex_gui_input(event: InputEvent) -> void: # Workaround to enable non-selectable cells to be skipped via keyboard navigation. # # Tested & required for at least Godot v3.2.1. # # Underlying issue is that the *current* Godot code doesn't check if a cell # is selectable before trying to select it *and* also doesn't notice that # the selection failed. # # I think this is a regression from earlier code but there's a bunch of # inter-related issues I need to write up. # # TODO: Write up the aforementioned issues. # Note: Hooking `_unhandled_key_input` signal rather than `_gui_input` won't # work because the Tree thinks it *has* handled the input correctly, # so accepts the event. # TODO: Add Godot version check? if _DATA_TREE_HEX.select_mode in [Tree.SELECT_SINGLE, Tree.SELECT_MULTI]: # Note: `true` here is for "echos" a.k.a. key (auto-)repeats when the key is held down. if event.is_action_pressed("ui_left", true) or event.is_action_pressed("ui_right", true): # NOTE: The current Godot `Tree`/`TreeItem` mode-related terminology/mechanism # for item/cell "selection"/"focus" is *extremely* non-obvious/intuitive. # # I recommend reading the `Tree`/`TreeItem` reference documentation, # particularly for the various methods with "selected" in their # name--it's...enlightening. # # Hopefully the variable names used here are more easily understood. # # # In single selection mode the focused cell & selected cell always # refer to the same single cell (in a row identified by column index). var cursor_focused_item = _DATA_TREE_HEX.get_selected() if cursor_focused_item == null: # Hack to handle if no initial selection was made. var ui_down_event: InputEventKey = event.duplicate() ui_down_event.scancode = KEY_DOWN _DATA_TREE_HEX.call("_gui_input", ui_down_event) # `ui_down` action causes correct initialization. # Continuing here rather than returning is redundant but YOLO. cursor_focused_item = _DATA_TREE_HEX.get_selected() var actual_selected_item = _DATA_TREE_HEX.get_next_selected(null) var cursor_focused_column = _DATA_TREE_HEX.get_selected_column() # warning-ignore:unused_variable var cursor_focused_column_is_selected = cursor_focused_item.is_selected(cursor_focused_column) if cursor_focused_column >= 0 else false var cursor_focused_column_is_selectable = cursor_focused_item.is_selectable(cursor_focused_column) if cursor_focused_column >= 0 else false # Note: In single selection mode (after ensuring an initial selection) # if there is no actual selected item it means that an attempt # to select a non-selectable cell occurred & failed. if (_DATA_TREE_HEX.select_mode == Tree.SELECT_SINGLE) and (actual_selected_item == null): # TODO: Handle more properly and/or support multiple consecutive # unselectables & in first column. # e.g. by use of `get_next_visible()` & `is_selectable` if event.is_action_pressed("ui_left", true): cursor_focused_item.select(cursor_focused_column - 2) accept_event() elif event.is_action_pressed("ui_right", true): cursor_focused_item.select(cursor_focused_column + 2) accept_event() elif not cursor_focused_column_is_selectable: # Handle *cursor* skip over non-selectable for multi-select... # TODO: Add support for multiple consecutive non-selectables? _DATA_TREE_HEX.call("_gui_input", event) else: # TODO: Add workaround to support skipping non-selectable cells # in Tree.SELECT_MULTI select mode with keyboard navigation. push_warning("Tree.SELECT_MULTI not yet supported.") ```

[Giwayume]

@follower can you explain "ui_down" injection? I have an issue where I need to set the cursor in a multi select tree.