godotengine / godot-proposals

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

Add a method to swap array elements #10771

Open Cykyrios opened 1 month ago

Cykyrios commented 1 month ago

Describe the project you are working on

Not relevant to this feature, this is applicable to any time one may want to swap array elements.

Describe the problem or limitation you are having in your project

As there is no built-in swap method, one needs to create a buffer variable to swap array elements manually. This is mainly an inconvenience and may decrease readability. I am not entirely sure, but for specific needs to swap large amounts of elements, it may also be slower than a built-in method.

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

I propose adding the method Array.swap(first_idx, second_idx) to perform this swap in place.

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

Comparing current and proposed workflows:

var current := [1, 2, 3] as Array[int]
var buffer := current[0]
current[0] = current[2]
current[2] = buffer

var proposed := [1, 2, 3] as Array[int]
proposed.swap(0, 2)

Both snippets result in [3, 2, 1]. I implemented this change, allowing negative indices as is common for other methods, and will open a PR shortly.

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

As shown in the snippet above, it is quite easy to work around, but also somewhat inconvenient if needed in multiple places.

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

I believe this is common enough to warrant a built-in method.

clayjohn commented 1 month ago

Is the goal here to save two lines of code, or is it to improve performance?

If the goal is performance, can you elaborate more on your intended use cases? I suspect if you are running into a performance constraint with element swapping, a function that swaps one element at a time may be insufficient.

Cykyrios commented 1 month ago

This is mainly intended as convenience, I do not have a specific performance-critical use case, I was simply surprised not to find a swap function built-in when I needed to swap array elements, given there are many other utility functions for arrays.

girng commented 4 days ago

if there is a use case for advantageous performance increases i think it's worth it. until then, i'm not really sure how many game devs use swap (even if they need it, they can do it in gds).