Array & Vector: rename `remove()` to `remove_at()`, Array, Dictionary & Set: rename `erase()` to `remove()`

[AaronRecord]

See https://github.com/godotengine/godot/pull/49701

Describe the project you are working on

Godot

Describe the problem or limitation you are having in your project

Array's current erase() and remove() methods are poorly named because the words "erase" and "remove" are (for the most part) synonymous.

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

remove(position) -> remove_at(index)
erase(value) -> remove(value)

It was originally proposed to rename erase() to remove_value(), but people seem to prefer renaming it to remove() (https://github.com/godotengine/godot/pull/49701#issuecomment-863752431).

I think Set's and Dictionary's erase() methods should be renamed to remove()/remove_value() too for consistency.

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

n/a

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

No.

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

This is core.

[AaronRecord]

If you prefer remove() over remove_value(), +1 this comment, otherwise -1 it, or leave a comment if you have an alternative suggestion.

var array := [1, 2, 3]

array.remove(2)
# or
array.remove_value(2)

[AaronRecord]

Actually I'm thinking maybe erase should just be replaced with remove for all methods that have erase in their name erase. There's 3466 methods (including ones that aren't exposed to the API) that have remove in their name, and 903 that have erase (remove is about 5 times more common).

[Xrayez]

According to my understanding and thesaurus:

• "remove": we don't necessarily delete something forever, but clear away the value from the container. We already know the index of the element, so we can eliminate the element immediately ("move" part implies index-based access on some level).
• "erase": completely obliterate the value from the container, with no ability to recover the deleted item. Erasing may also imply that we don't necessarily know the exact element to delete, and we must first find it, which leads to:
  • find() method which returns an index of the first found element by value, and we can then remove(find(value)) instead. In my understanding, erase() is just a shorthand for the remove(find(value)) construct, but it greatly simplifies the code.

Depending on whether the element is passed by value or by reference, this may not always apply, though.

Generally, looks like remove() is used to "delete" elements by index, and erase() by value.

For example, Godot has List::erase(element) and List::Element::erase(), both deleting the element from the list (see memdelete), it does not have remove() because, unlike Vector, List does not use a contiguous array to store elements to be accessed by index.

When looking at other usages, Config has methods such as erase_section(). We cannot use remove() for those methods because those elements will always be deleted by value, and never by reference.

As you may already know, I prefer less verbosity in Godot API whenever possible.

That said, I'd rather use two different words than to rename using the same word to describe different actions.

To summarise

1. Use remove() when there's a possibility to delete an element by value and reference/index, especially when we deal with arrays.
2. Use erase() when there's no possibility to recover an element, and an element can be only removed by value.

Of course, both can be used, as already seen in Array API, unlike Dictionary which only has erase().

[Xrayez]

Curious: was my feedback above even useful?

I still think remove() and erase() were perfect words to describe the current functionality. While the words are synonymous, they are quite different.

I feel like godotengine/godot#49701 and godotengine/godot#50139 were supposed to be merged together, but know we have inconsistency again, something which this proposal supposed to solve. This already creates potential inconsistency with names such as insert(): https://github.com/godotengine/godot/pull/50139#issuecomment-915695704.

Also, it's not only about Array API consistency, this naming convention may and should affect other classes. For example: PopupMenu.remove_item(). Should it also be renamed to remove_item_at()?

Please don't get me wrong, I'm not necessarily against the change, but we have to understand the rationale behind it. If the reason is not clear, someone else in the future may rename those methods again, with the same reason of making API consistent. I say this because I've seen this happening in Godot already (2.0 → 3.0 → 4.0).

[Mickeon]

I'm personally glad remove() now behaves by the convention of Python and C#'s lists. It may be possible to take some inspiration from some other language to bring a consistent word for "removing at a specified index", but that could be somewhat arbitrary. The remove_at() as is now is lovely, though. It matches C#'s List.RemoveAt(). It just would be preferable to have the name of similar functions to be consistent across the board, but that may also cause their names to be annoyingly redundant.

[Xrayez]

The remove_at() as is now is lovely, though. It matches C#'s List.RemoveAt(). It just would be preferable to have the name of similar functions to be consistent across the board, but that may also cause their names to be annoyingly redundant.

Interesting, I didn't know about this! Still, I would be wary about inheriting naming conventions from other languages, if we do so, it has to be done systematically.

But, I think you may agree that GDScript is similar to Python rather than C#. GDScript is a first-party language in Godot and has existed before C# integration. This is not to say that C# is not a first-party language in Godot as well, but there are several reasons why GDScript exists in the first place.

I've just looked up Python's array.remove(), which is erase() is Godot's Array. I haven't found remove_at() in Python.

Since godotengine/godot#49701 was not merged, we have inconsistency in relation to Python's naming.

So, instead of being agnostic/skeptic to naming conventions in other languages, we simply choose some foreign convention. It's impossible to satisfy everyone, that's why I'm suggesting to think for ourselves and come up with a convention that would make sense specifically in Godot (or actually figuring out existing convention, which, yes, does take time reading documentation).

Some might even say that what I bring up is a bikeshedding, but recall that naming is the hardest problem in programming! Linguists spend their lives in order to make everything clear! 😃

Just my 50 cents! 🙂

[AaronRecord]

Curious: was my feedback above even useful?

I still think remove() and erase() were perfect words to describe the current functionality. While the words are synonymous, they are quite different.

I'm sorry but I personally don't see where you're coming from with this. Two methods with basically synonymous names that do different things is confusing to me.

This already creates potential inconsistency with names such as insert(): godotengine/godot#50139 (comment).

I feel like insert() is more okay because to me I think the word "insert" means to put something inside of something else, so it would make logical sense that it takes an index of where you want to insert said thing.

Also, it's not only about Array API consistency, this naming convention may and should affect other classes. For example: PopupMenu.remove_item(). Should it also be renamed to remove_item_at()?

Possibly

I've just looked up Python's array.remove(), which is erase() is Godot's Array.

In the future potentially erase() could be renamed to remove to match python, but I think this should probably be done after 4.0 to minimize confusion among people converting projects from 3.x to 4.0, even if automated tools could help with this.

I haven't found remove_at() in Python.

https://stackoverflow.com/questions/627435/how-to-remove-an-element-from-a-list-by-index

[Xrayez]

I'm sorry but I personally don't see where you're coming from with this. Two methods with basically synonymous names that do different things is confusing to me.

Could you explain what exactly makes it confusing?

In case you haven't noticed my previous messages, see https://github.com/godotengine/godot-proposals/issues/2885#issuecomment-864381319.

[AaronRecord]

I'm sorry but I personally don't see where you're coming from with this. Two methods with basically synonymous names that do different things is confusing to me.

Could you explain what exactly makes it confusing?

Because if the names of the methods mean basically the same thing then it makes distinguishing their functionality a matter of memorization, not intuition.

In case you haven't noticed my previous messages, see #2885 (comment).

Well about remove() you also said:

Generally, looks like remove() is used to "delete" elements by index

But you also said:

But, I think you may agree that GDScript is similar to Python rather than C#

I've just looked up Python's array.remove(), which is erase() is Godot's Array

I think it's pretty clear that the current 3.x names are not intuitive.

[Xrayez]

What I say now is about currently made decision, which created another set of inconsistency. I did this in a way to justify current decision and see if it still makes sense. I still don't quite understand your stance of memorization and intuition: those are not mutually exclusive concepts. Is memorization a problem, or relying on intuition is a problem?

I'm curious because English is not my native language.

Sorry in advance.

[AaronRecord]

What I say now is about currently made decision, which created another set of inconsistency.

Yeah I think it'd be a good idea to rename stuff like PopupMenu.remove_item(index) to PopupMenu.remove_item_at(index) for consistency, but those methods are a lot less common so they're less critical.

I still don't quite understand your stance of memorization and intuition: those are not mutually exclusive concepts. Is memorization a problem, or relying on intuition is a problem?

What I meant to say is that to someone who is not already familiar with Godot, without reading the documentation it'd be logical for them to assume that remove() is for removing a specified item from a list, and that erase() was just an alias for remove() or it did something slightly different like remove all of the specified item, not just the first occurrence. By renaming remove() to remove_at(), it makes it more clear that remove_at() removes from a specified location, not a specified item. That is, you don't need to memorize the difference between remove() and erase() whose names mean the same thing, you can use your intuition to figure out the difference.

Sorry in advance.

All good :)

[saminton]

As someone rather new to Godot, but very familiar with other languages such as php and typescript when I saw there was a remove_at function but no remove function I just assumed we had to remove by index. So for the past few weeks I have been using:

var index = arr.find_index(node)
arr.remove_at(index)

It never occured to me that erase would be what I was looking for...

We have functions such as pop_at pop_back and pop_front which all use pop in the naming as they all have the same basic functionality so imo it should either be remove_at and remove or erase_at and erase, but not a mix of the two. Having two different words is very unintuitive.

[Mickeon]

I wish this went through in a more notable way. Back when the "renaming season of 4.0" was in process, I thought this had already been changed, but that's not the case.