GDExtension Vector*i to Vector* conversion results in wrong values

[ruedoux]

Godot version

4.0.3

godot-cpp version

4.0.3

System information

Windows 11

Issue description

When you declare a function that takes Vector2i as an argument it works correctly if you pass Vector2i to the function, but the moment you pass Vector2 instead it gives some crazy high numbers (likely a conversion problem).

Basically calling do_stuff_with_vec2i(Vecotr2i(1,1)) works fine but do_stuff_with_vec2i(Vecotr2(1,1)) reads Vector2(1,1) as Vector2i(213213213,213213213)

Steps to reproduce

If you write a simple function that takes Vector2i as an argument

Vector3i vec2i_vec3i(const Vector2i &vec, int32_t z)
{
    UtilityFunctions::print(vec);
    return Vector3i(vec.x, vec.y, z);
}

Everything is fine if you for example call it via vec2i_vec3i(Vector2i(1,1), 1), but calling vec2i_vec3i(Vector2(1,1), 1) results in very high x and y values in vector being printed

Minimal reproduction project

Example minimalistic source code to reproduce the above bug
src.zip

[dsnopek]

Thanks for the bug report!

This may be related to PR godotengine/godot#75758 - if you have time, can you try that PR and see if it fixes the issue for you?

[ruedoux]

@dsnopek Yes it seems to solve the issue!

I have downloaded godot 4.0.3 source code and changed the memnew_placement(r_value, T(VariantInternalAccessor<T>::get(reinterpret_cast<Variant *>(p_variant)))); to memnew_placement(r_value, T(*reinterpret_cast<Variant *>(p_variant)));.

I ran some GUT unit tests on a simple function that converts vector2 to vector3 and got something interesting you might want to look into.

func test_vector_conversion() -> void:
   var Rng := RandomNumberGenerator.new()
   for i in range(10):
       var x:int = Rng.randi_range(-10_000_000, 10_000_000)
       var y:int = Rng.randi_range(-10_000_000, 10_000_000)
       var z:int = Rng.randi_range(-10_000_000, 10_000_000)
       assert_eq(VectorUtilsExt.vec2i_vec3i(Vector2i(x,y), z), Vector3i(x,y,z),
           "Failed for Vector2i")
       assert_eq(VectorUtilsExt.vec2i_vec3i(Vector2(x,y), z), Vector3i(x,y,z),
           "Failed for Vector2")

This code will always work for the below gdextension function:

Vector3i VectorUtilsExt::vec2i_vec3i(const Vector2i &vec, int32_t z)
{
   return Vector3i(vec.x, vec.y, z);
}

The unit test starts failing the moment I take off the limit of 10_000_000, any bigger numbers will make the function fail, I suspect it has to do with float -> int conversion overflowing for numbers of certain size. So as long as your input numbers in Vector2 are lower than roughly 10_000_000 everything works fine, the function fails for bigger numbers. Maybe there should be a maximum intiger value you can put into Vector2 to solve this issue or is it intended behaviour? Anyway I think there should be some kind of warning for the user when the input number is too big for Vector2 to handle.

[dsnopek]

Great, thanks for testing!

The unit test starts failing the moment I take off the limit of 10_000_000, any bigger numbers will make the function fail, I suspect it has to do with float -> int conversion overflowing for numbers of certain size.

Hm. Can you give some example numbers where this happens?

In any case, your theory could be correct. An int in GDScript is int64_t whereas Vector2i and Vector3i hold int32_ts. In the 2nd assertion, you're creating a Vector2 which holds floats, and floats can hold numbers much larger than int32_t, so it wouldn't be that weird that converting int64_t to float to int32_t would give different results than int64_t to int32_t with very large numbers.

Seeing some examples of numbers that fail would help to confirm that!

[ruedoux]

Ok so I ran a simple while loop that tried numbers until it broke. The result is quite weird since limit is 16_777_216 for both positive and negative numbers. What is interesting here is that after this number the function converts Vector2 input correctly every second number. If you run a simple test:

func test() -> void:
	print("+")
	print(try_num(16_777_216))
	print(try_num(16_777_217))
	print(try_num(16_777_218))
	print(try_num(16_777_219))
	print(try_num(16_777_220))
	
	print("-")
	print(try_num(-16_777_216))
	print(try_num(-16_777_217))
	print(try_num(-16_777_218))
	print(try_num(-16_777_219))
	print(try_num(-16_777_220))

func try_num(num:int) -> bool:
	var isOK := VectorUtilsExt.vec2i_vec3i(Vector2(num,num), num) == Vector3i(num,num,num)
	return isOK

Output for this test is:

+
true
false
true
false
true
-
true
false
true
false
true

To find out where the back and forth correct and failed conversion ends i ran another test and the upper bound for it is 33_554_434 for both positive and negative numbers. To find out what happens after that point I ran a simple for loop that goes for another 100 numbers and we get another very weird behaviour:

for i in range(100):
    print(try_num(33_554_439+i))

output:

false
true
false
false
false
true
false
false
false
true
false
false
false
true
false
false
false
[...]

You can notice that:
33_554_438 / 2 == 16 777 219
So its safe to assume this number is a divisor of something causing trouble there. The true-false output has to do with 16_777_216 * 2^x because for for loop 16_777_216 * 3 the output is the same as in 16 777 216 * 2 and it switches again for 16 777 216 * 4 :

for i in range(100):
    print(try_num(16_777_216*4+i))

output

true
false
false
false
false
false
false
false
true
false
false
false
false
false
false
false
true
[...]

[ruedoux]

Ah nevermind seems like this is just a general floating point number problem like shown in this reddit thread, seems like this is a no-issue then. So I can assume the PR you linked earlier fully solves the problem with conversion

[DmitriySalnikov]

A similar issue has been encountered here at least 2 times #907 #1106

I also keep running into these conversion issues. The last time it was related to the Dictionary. It was used to read data from JSON, after which it was necessary to convert the values to int or enum. Variant tried to do this through int64_t and returned 0. So I had to convert Variant to float first, then to int, then to enum if needed... -_-
bool was converted without any problems.