godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Add a `Node.add_child_at()` method to add a node at a specific index #1152

Open andy-noisyduck opened 4 years ago

andy-noisyduck commented 4 years ago

Describe the problem or limitation you are having in your project: There is currently no way to directly add a child at an arbitrary index. The options we are have are either:

  1. add_child then move it with move_child
  2. Use add_child_below_node to specifiy a sibling. This method is already pretty poor and has been replaced with add_sibling in master, and it doesn't allow you to add a node at index 0 because the new node is added after the sibling.

Honestly, I was a little surprised to find this feature missing.

Describe the feature / enhancement and how it helps to overcome the problem or limitation: Implement a simple add_child_at which takes an index parameter in addition to the child. E.g.

# Add child at index 0
parent.add_child_at(child, 0)

If this enhancement will not be used often, can it be worked around with a few lines of script?: It's pretty fast to work around with add then move, but tree mutations seem like common enough operations to be worth adding. If we think add_sibling was a common enough use-case, then this seems pretty reasonable too.

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

Xrayez commented 4 years ago

How about insert_child(pos, child)? This would be consistent with other existing insert() methods for various Variant types.

rcorre commented 4 years ago

Out of curiosity, what's the use case for adding a child at a specific index? The only time I can think of where child order mattered to me was with viewport textures, which I still hope is a bug that can be fixed (https://github.com/godotengine/godot/issues/27790)

andy-noisyduck commented 4 years ago

Out of curiosity, what's the use case for adding a child at a specific index?

Any case where you want to make changes to the tree and that the order of the nodes matters.

Control nodes are a common use case for me. They have no z-index property (that's a separate discussion) so visual ordering is managed by tree position. Godot's input events are also cascaded in tree order. This means for almost all UI work the order of the nodes is important.

Zireael07 commented 4 years ago

ninja'd, basically what @andy-noisyduck said.

andy-noisyduck commented 4 years ago

How about insert_child(pos, child)? This would be consistent with other existing insert() methods for various Variant types.

I'm open to any reasonable implementation of this really. I think I slightly prefer _at to insert, but I wouldn't fight for it.

I actually think we actually shouldn't aim for consistency if the other operations are also not consistent. Variants with insert normally have a matching append method, compared to nodes which use add instead. I think deliberate inconsistency is less confusing than having partial consistency. Using the add_child_at also means anyone will be able to see the method when auto-completing add_child when editing.

I guess a third option is to just make the current implementation of add_child take a 2nd index parameter, and have it default to -1 or something to mean append? Though I think _at is cleaner.

aaronfranke commented 4 years ago

What about if add_child had a 2nd optional parameter for the index, defaulting to -1, which means auto/at the end?

Anyway, I think any implementation should have the argument order be (child, index).

Also, let's avoid the words pos and/or position since this can be confused with transforming/translating.

Xrayez commented 4 years ago

Anyway, I think any implementation should have the argument order be (child, index).

This would make it inconsistent with the rest of the Variant methods where the order is (index, child), so either the entire codebase would have to be changed, or the order must comply with the existing convention.

What about if add_child had a 2nd optional parameter for the index, defaulting to -1, which means auto/at the end?

It might be that the logic behind having (index, element) order is to prevent the default index specifically to prevent programming errors, dunno.

skejeton commented 4 years ago

Out of curiosity, what's the use case for adding a child at a specific index?

Any case where you want to make changes to the tree and that the order of the nodes matters.

Control nodes are a common use case for me. They have no z-index property (that's a separate discussion) so visual ordering is managed by tree position. Godot's input events are also cascaded in tree order. This means for almost all UI work the order of the nodes is important.

I can't really imagine use for 3d, for 2d there's ysort node

ghost commented 4 years ago

I can't really imagine use for 3d, for 2d there's ysort node

YSort isn't really helpful because it sorts via the y coordinates of the childs (Like if a tree is higher than a house on the y coordinate then it goes behind it). What if you have 2 WindowDialogs and you want to have the one, which is focused by the user, to go on the top of the screen.? You would have to move the focused Dialog below the 2nd Dialog which doesn't make sense (This is the exact same problem from #839 ).

I know this is not really contributing to this issue but I just wanted to say that :)

berarma commented 2 years ago

Node processing is done in tree order, so there's cases where order matters besides rendering.

alexander-yozzo commented 1 year ago

I ran into a case where this would be useful today. Looks like there is no resolution yet?

Calinou commented 1 year ago

I ran into a case where this would be useful today. Looks like there is no resolution yet?

To my knowledge, nobody is currently working on implementing this. It shouldn't be too difficult to do, but remember that we're in feature freeze so any feature PRs will have to wait until 4.3 at the earliest to be merged.

Blaine-McK commented 8 months ago

I came up against a scenario today that could be resolved with such a function

SteveSmith16384 commented 8 months ago

Let's not forget, as OP said, you can just do "add_child()" then "move_child()". It's only one extra line of code.

ben-rolfe commented 8 months ago

Ran into this today. My 2c is that making add_sibling add the node before rather than after would be sufficient. That way get_child(n).add_sibling(my_node) would do the job. At present, in addition to there being no way to add a node at the beginning, there are two ways to add one at the end (add_child and add_sibling to the last child)