Add a method to construct a NodePath from a StringName

[aaronfranke]

This allows constructing a NodePath directly from a StringName, which skips the String parsing logic, if the given StringName does not contain a slash or colon and can be used as-is.

I considered a constructor, but this would break all uses in the C++ code that construct NodePath from char * literals, and we need to depend on two other constructors anyway, so this uses a static method instead.

[Zireael07]

I just ran into a situation where code that worked fine in 3.x doesn't due to this. Can this get merged into one of the patch releases?

[dalexeev]

I considered a constructor, but this would break all uses in the C++ code that construct NodePath from char * literals, and we need to depend on two other constructors anyway, so this uses a static method instead.

Perhaps we just need to add the StringName(NodePath) and NodePath(StringName) constructors for GDScript only?

godot/core/variant/variant.cpp

Lines 558 to 563 in e74bf83

	case STRING: {
	static const Type valid[] = {
	NODE_PATH,
	STRING_NAME,
	NIL
	};

godot/core/variant/variant.cpp

Lines 714 to 718 in e74bf83

	case STRING_NAME: {
	static const Type valid[] = {
	STRING,
	NIL
	};

godot/core/variant/variant.cpp

Lines 722 to 726 in e74bf83

	case NODE_PATH: {
	static const Type valid[] = {
	STRING,
	NIL
	};

[aaronfranke]

@dalexeev Good idea, pushed a change to add that, but we should also keep the method exposed for explicitness.

[dalexeev]

See also:

godot/core/variant/variant_construct.cpp

Lines 79 to 82 in fd31dc7

	add_constructor<VariantConstructNoArgs<String>>(sarray());
	add_constructor<VariantConstructor<String, String>>(sarray("from"));
	add_constructor<VariantConstructor<String, StringName>>(sarray("from"));
	add_constructor<VariantConstructor<String, NodePath>>(sarray("from"));

godot/core/variant/variant_construct.cpp

Lines 176 to 182 in fd31dc7

	add_constructor<VariantConstructNoArgs<StringName>>(sarray());
	add_constructor<VariantConstructor<StringName, StringName>>(sarray("from"));
	add_constructor<VariantConstructor<StringName, String>>(sarray("from"));
	add_constructor<VariantConstructNoArgs<NodePath>>(sarray());
	add_constructor<VariantConstructor<NodePath, NodePath>>(sarray("from"));
	add_constructor<VariantConstructor<NodePath, String>>(sarray("from"));

Perhaps it makes sense to add constructors only in GDScript, even if we can't add them in C++?

[raulsntos]

This looks good to me.

From the perspective of the C# bindings, I wonder if it'd be better to provide a static method or constructor that takes the StringName and just creates the NodePath assuming it contains no slashes or colons.

This would be an "unsafe" way to create a NodePath from StringName but I wouldn't expose this in the C# bindings. It would be used internally to create the NodePath when we can guarantee that the StringName doesn't contain slashes or colons. This means the logic to check this would be done in C# directly which would allow us to use IndexOfAny which is vectorized.

However, using IndexOfAny would require us to convert the StringName to a C# string first and that means creating a NodePath would take 2 interop calls instead of 1. I haven't benchmarked any of this so I don't know what would be better. It's possible that vectorizing the check for slashes and colons wouldn't bring much improvement over using this PR's static method, and I guess it's also possible to vectorize the C++ code in the future.

So, having said all that, I think this PR is still an improvement over the current way to create NodePaths from StringNames. I wonder if other language bindings authors are also interested in this API and would use it to either expose a way to create NodePaths from StringNames or, if it already exists, to improve their current implementation.

cc @Bromeon

[AThousandShips]

Makes me think it'd be interesting to see if we should add a findmc to complement findmk, and perhaps a contains_any or similar, there's many places that look for "is any of these present" and it'd be a good feature, just to think about I might look at an implementation later

[aaronfranke]

Wouldn't that require constructing a Vector<char32_t>, so less efficient than as it is now?

But in general, that seems like a good idea.

[AThousandShips]

For two arguments I'd tend to agree