godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.07k stars 20.19k forks source link

TreeItem add move up/down function #30116

Open TheDuriel opened 5 years ago

TheDuriel commented 5 years ago

Godot version:

3.1

Issue description:

Right now TreeItem implements move_to_top() and move_to_bottom() to move the item to the first or last position among its siblings.

I would like the ability to move an Item by an arbitrary amount. I am currently building a modding system ala skyrim, and need to ability to move tree items to arbitrary positions in the list.

akien-mga commented 5 years ago

Makes sense, though I'm not sure if that can be added to TreeItem itself, I guess it would have to be a method of Tree? E.g. Tree.move_item(int p_item, int p_new_position). Or maybe it is possible in TreeItem, with something like TreeItem.move_up/down(int p_amount), as long as it's able to gracefully handle out of bound moves (e.g. you're moving TreeItem idx 3 up by 6, it should become the first item (0) as if you only moved it by 3).

TheDuriel commented 5 years ago

Either way is fine with me <3

Anutrix commented 5 years ago

Any comments on which one of these is better? Polls on github would be nice.

Makes sense, though I'm not sure if that can be added to TreeItem itself, I guess it would have to be a method of Tree? E.g. Tree.move_item(int p_item, int p_new_position). Or maybe it is possible in TreeItem, with something like TreeItem.move_up/down(int p_amount), as long as it's able to gracefully handle out of bound moves (e.g. you're moving TreeItem idx 3 up by 6, it should become the first item (0) as if you only moved it by 3).

akien-mga commented 5 years ago

Well it's not a matter of polling, but of checking the actual implementations of Tree, TreeItem and their interactions, to see what is possible / makes sense given their design.

The Tree/TreeItem APIs are a bit peculiar, and it seems item management mainly goes through TreeItem (where a TreeItem can give you a reference to its previous or next TreeItem in the parent Tree), so I suppose that's where the move methods would have to go, even if it stays a slightly unintuitive API.

But again I haven't checked the code, those are just some comments to point interested contributors in possible directions :)

girng commented 5 years ago

forgive me if i am mistaken, but since the node scene list in the editor lets you drag to any position, this should be possible already?

TheDuriel commented 5 years ago

@girng The editor implementation is not as straightforward. The entire drag and drop system for it is a custom implementation. Remember that it actually modifies a Scene Tree, the dock simply displays it after a change happens. This works because the tree structure itself is not actually managed using the Tree class.

https://github.com/godotengine/godot/blob/master/editor/scene_tree_dock.cpp

girng commented 5 years ago

@TheDuriel interesting, TIL. nvm my prev comment then :p. ty

marzecdawid commented 1 year ago

Is it still valid proposal after #46773? TreeItem.move_up/down(int p_amount) may still be more practical in some cases than move_before/after.

TheDuriel commented 1 year ago

I think that #46773 functionally solves this, though it's very awkward.

Say you want to move a node one down in the tree:

self.move_after(self.get_next())

And move it two up.

for i in 2:
    self.move_before(self)

A dedicated move_up(x) move_down(x) and raise() would be preferable.