godot icon indicating copy to clipboard operation
godot copied to clipboard

Add `insert_item` method to ItemList

Open djdanlib opened this issue 6 months ago • 5 comments
trafficstars

Hello, first-time PR here.

With this addition, you can insert an item into an ItemList at any index, such as 0 for the top, even including the new index at the bottom that will exist when an item is added. This is a pretty familiar idea for people coming from other GUI frameworks.

Hopefully I've done a decent enough job to consider adding this to the main codebase because I'd love to use this contribution myself. It does compile and work for me on Windows, VS2022, built with scons platform=windows dev_build=yes. I do need some help with the unit testing aspect of this because I don't quite understand how to write an appropriate test for this yet.

Rationale: While working with ItemList, I wanted a method to add/insert an item at any arbitrary index in the list, particularly at the top (index 0), so I created it. It's more efficient (in terms of CPU cycles and lines-of-code) than calling add_item, storing its return value, then calling move_item.

Code notes: This is short and simple, inspired by add_item and move_item. I thought about overloading add_item for this but landed on creating a separate insert_item method instead. Generally the index parameter is first in most languages' insert functions, so that's what I did here - and overloading add_item this way didn't feel good. The method doesn't return a value because you either know the index already or will be receiving an error, and as far as I can see, something like ERR_FAIL_INDEX isn't available for methods returning a value anyway.

edit: Following a code review, I reworded position to index, since position is commonly used for 2D coordinates. I also removed everything related to an unnecessary insert-at-end modality.

Bugsquad edit: Closes https://github.com/godotengine/godot-proposals/issues/12482

djdanlib avatar May 22 '25 05:05 djdanlib