Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug when using the String module operator for Variant type. #1605

Open
YooRarely opened this issue Sep 24, 2024 · 2 comments
Open

Bug when using the String module operator for Variant type. #1605

YooRarely opened this issue Sep 24, 2024 · 2 comments
Labels
bug This has been identified as a bug

Comments

@YooRarely
Copy link

YooRarely commented Sep 24, 2024

Tested versions

  • 4.3.stable

System information

Windows 11

Issue description

I found that the module_Variant method for the String type in GDExtension did not return the correct result. They all returned an incorrect result <null>. Could this be a bug? I haven't found any posts mentioning a similar issue.

Steps to reproduce

I tried the following code. The output on the left is correct, while the result on the right is incorrect.

		Variant iv = (int64_t) 10;
		Variant dv = 99.9;
		Variant bv = true;
		GD::print(iv," ",String("%s") % (int64_t)10, "  ", String("%s") % iv);
		GD::print(dv," ",String("%s") % 99.9, "  ", String("%s") % dv);
		GD::print(bv," ",String("%s") % true, "  ", String("%s") % bv);
10 10  <null>
99.9 99.9  <null>
true true  <null>

They each used different overloaded operators, as shown below.

// [incorrect]
String operator%(const Variant &p_other) const;
// [correct]
String operator%(bool p_other) const;
String operator%(int64_t p_other) const;
String operator%(double p_other) const;

source_code

String String::operator%(const Variant &p_other) const {
	return internal::_call_builtin_operator_ptr<String>(_method_bindings.operator_module_Variant, (GDExtensionConstTypePtr)&opaque, (GDExtensionConstTypePtr)&p_other);
}
String String::operator%(bool p_other) const {
	int8_t p_other_encoded;
	PtrToArg<bool>::encode(p_other, &p_other_encoded);
	return internal::_call_builtin_operator_ptr<String>(_method_bindings.operator_module_bool, (GDExtensionConstTypePtr)&opaque, (GDExtensionConstTypePtr)&p_other_encoded);
}

String String::operator%(int64_t p_other) const {
	int64_t p_other_encoded;
	PtrToArg<int64_t>::encode(p_other, &p_other_encoded);
	return internal::_call_builtin_operator_ptr<String>(_method_bindings.operator_module_int, (GDExtensionConstTypePtr)&opaque, (GDExtensionConstTypePtr)&p_other_encoded);
}

String String::operator%(double p_other) const {
	double p_other_encoded;
	PtrToArg<double>::encode(p_other, &p_other_encoded);
	return internal::_call_builtin_operator_ptr<String>(_method_bindings.operator_module_float, (GDExtensionConstTypePtr)&opaque, (GDExtensionConstTypePtr)&p_other_encoded);
}

So, I had to resort to the most cumbersome way, using a switch statement to solve the issue where the Variant type couldn't be handled correctly.

switch (p_value.get_type())
		{
		case Variant::NIL:
			s = str % nullptr;
			break;
		case Variant::BOOL:
			s = str % bool(p_value);
			break;
		case Variant::INT:
			s = str % int64_t(p_value);
			break;
		case Variant::FLOAT:
			s = str % double(p_value);
			break;
.....
.....
.....
		case Variant::PACKED_COLOR_ARRAY:
			s = str % PackedColorArray(p_value);
			break;
		case Variant::PACKED_VECTOR4_ARRAY:
			s = str % PackedVector4Array(p_value);
			break;
		default:
			s = str % p_value;
			break;
		}

Minimal reproduction project (MRP)

1

@akien-mga akien-mga changed the title [gdextension]I found a bug when use the String module operator for Variant type. Bug when using the String module operator for Variant type. Sep 24, 2024
@akien-mga
Copy link
Member

Moving to the godot-cpp as it's related to its own implementation of String.

@akien-mga akien-mga transferred this issue from godotengine/godot Sep 24, 2024
@dsnopek
Copy link
Collaborator

dsnopek commented Oct 30, 2024

I'm really surprised to learn we have these operator%(...) methods in godot-cpp!

Since this isn't supported in Godot itself, having them goes against one of godot-cpp design goals, which is to support the same API as Godot. I think it might be better to remove these operators, rather than fix them. We appear to be automatically generating them based on the extension_api.json, so it'd be a matter of filtering them out in binding_generator.py.

Using vformat() would be the recommended way to do this kind of string formatting in godot-cpp

@dsnopek dsnopek added the bug This has been identified as a bug label Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug
Projects
None yet
Development

No branches or pull requests

3 participants