Fix drag-dropping nodes to parent with internal nodes

[kleonc]

When reparenting nodes (or moving within the same parent) within the SceneTreeDock all indexes were taking into account internal nodes. However, Node::move_child does move the child only within the group of nodes the given node belongs to (internal-front / non-internal / internal-back), its parameter is supposed to be such relative index. Hence an absolute index (including internal nodes) shouldn't be passed to it.

This PR makes the SceneTreeDock::_do_reparent method ensure only non-internal nodes are being moved, and changes all indexes to not take internal nodes into account (hopefully I haven't missed any 🙃; please double check the changes).

Fixes #78810 (TabContainer has internal TabBar node).

[KoBeWi]

hopefully I haven't missed any 🙃

You did. Search the file for get_index(), seems like they all cause problem. Note that here

godot/editor/scene_tree_dock.cpp

Line 2627 in 4642448

for (int i = to_node->get_index() + 1; i < to_node->get_parent()->get_child_count(); i++) {

you also need to do get_child_count(false).

[kleonc]

hopefully I haven't missed any upside_down_face

You did. Search the file for get_index(), seems like they all cause problem. Note that here

godot/editor/scene_tree_dock.cpp

Line 2627 in 4642448

for (int i = to_node->get_index() + 1; i < to_node->get_parent()->get_child_count(); i++) {

you also need to do get_child_count(false).

Ah, saw that and concluded it's fine... but indeed it could result in finding a lower_sibling which is a back internal node. Which shouldn't happen.

[KoBeWi]

By "they all cause problem" I actually meant all calls in scene tree dock. Deleting nodes, changing node to root or instantiating all have the same bug.

[kleonc]

By "they all cause problem" I actually meant all calls in scene tree dock. Deleting nodes, changing node to root or instantiating all have the same bug.

Ok, I've restricted some more code to non-internal nodes only. Haven't tested it at all though, just went with find-read-update manner (again, not sure if I've handled everything). 🙃

Alternatively I guess I could maybe add a method like Node::get_internal_index so its result could be passed to Node::move_child (and nodes could be readded with the same internal mode as they had).

Current combination of how Node::get_index/Node::move_child work is unfortunate, I see no way to obtain internal node's "internal index" aka index only within the front/back internal nodes (for front ones it's the same as the absolute index but back ones are problematic). And that's what's needed for moving.

[akien-mga]

Thanks!