Inefficient GDNative API - NodePath returns String there instead of StringName

[Evrey]

https://github.com/godotengine/godot/blob/6d380b04f27c648b1ebebf6afb0cce261625f205/modules/gdnative/include/gdnative/node_path.h#L61

I'm working on a custom Rust binding for Godot now and spotted this. For some reason, the godot_node_path_* functions return fresh and new godot_strings, whereas internally NodePath acts on and returns StringNames, as can be seen here:

https://github.com/godotengine/godot/blob/6d380b04f27c648b1ebebf6afb0cce261625f205/core/node_path.h#L65

So... why is that? Is it an error, or was there some specific thought put behind it?

In case there is no special reason for it, or in case the reason is "'cause that's how GDScript works" (Is it?), I suggest changing that API into using godot_string_name instead, before Godot 3 is stable. And by API here I mean all the godot_node_path_* functions returning godot_strings except for godot_node_path_as_string. Otherwise, I'd like to learn the reason why and see it documented somewhere. =)

[karroffel]

Cool that you are working on Rust bindings! Have something to show yet? :D

The rationale is that StringName is strictly speaking not a core type. You can't put a StringName into a Variant, which is why they are always converted from and to Strings.

There is #11780 which adds StringName to the GDNative API, but only those functions from the core types itself could make use of it since the whole class API has no idea that StringName exists.

We can merge the mentioned PR, but the usefulness is questionable.

So for now in every scripting language (or anything that isn't a module or part of the Godot source otherwise) there is no way to use StringName directly, it will always be converted from and to strings.

[Evrey]

Have something to show yet? :D

Not yet. I'm hand-crafting the core API right now with docs, Rust-style iterators 'n' stuff. Next step is the code generator for the remaining classes extracted from JSON, and finally a procedural macro for generating Godot-compatible "classes". Was rather tricky to get the dictionary iterators working, as I had to crawl through the sources to understand how godot_dictionary_next is meant to be used.

The rationale is that StringName is strictly speaking not a core type.

Okay, that's a surprise to me, as this chapter in the latest docs, the section about literals, suggests that while @"a/b" is a literal NodePath, @"n" would create a literal StringName. Literal value syntax does give it quite a core-ish feeling.

Anyways, that's the kind of explanation I was looking for. Personally, I'd like to see StringName being used more throughout the API, but I'm fine with using String as well. I'll notice if my code breaks because of sudden API changes. =)

[karroffel]

Well there is only NodePath, but no StringName.

https://github.com/godotengine/godot/blob/c5da28f24cbab915098165b13f50bcda049e273e/core/variant.h#L74-L115

So it's a documentation error 😛

[karroffel]

Closing as it's a documentation error and manually changing the whole API to find places where StringName could be used seems not worth it.