Fix nodes being renamed into garbage upon conflict

[KoBeWi]

Fresh new regression from #54072

(well maybe they aren't fresh anymore at this point)

Too fast.

[akien-mga]

Actually taking back my approval, changing Node::set_name defeats the whole purpose of having this be optional, no? This will mean that at runtime, costly human readable name conflict resolution will be performed.

[KoBeWi]

I checked the set_name() usages and they are mostly used for TabContainer tabs, the add_child() method itself doesn't use it. Node only internally uses it inside duplicate() and the most heavy usage I found is the instantiate() method.

set_name() is a setter, so not sure if it can have an optional argument. I could add set_name_nocheck() if performance is a concern.

[akien-mga]

I checked the set_name() usages and they are mostly used for TabContainer tabs, the add_child() method itself doesn't use it. Node only internally uses it inside duplicate() and the most heavy usage I found is the instantiate() method.

That's for the editor usage, but users can't set Node names from script and could write egregiously slow code like #54177 with e.g.:

func _ready():
	for i in range(40000):
		var node = Node.new()
		node.name = "MyNode"
		add_child(node)

But let's see what @godotengine/core folks think about it.

[KoBeWi]

Well my thinking is that if you use set_name(), you most likely want the name to be human-readable.

func _ready():
	for i in range(40000):
		var node = Node.new()
		add_child(node)

wouldn't have performance concerns.

[reduz]

@KoBeWi's reasoning makes sense to me, if you use set_name you most definitely want a human readable conflict resolution.

[akien-mga]

Thanks!