Reading PhysicsRayQueryParameters3D.Exclude from C# causes Godot to exit

[rhofour]

Godot version

4.0.beta3.mono

System information

Fedora 36

Issue description

Reading PhysicsRayQueryParameters3D.Exclude appears to cause Godot to simply exit. Setting it to a new value doesn't, actually casting rays doesn't, but reading or modifying it does. Godot simply exits without any sign of an error.

This appears to be a C# issue. Doing something equivalent in GDScript works correctly.

Steps to reproduce

1. Create a 3D scene
2. Add this to any node:

public override void _PhysicsProcess(double delta)
{
	var rayParams = PhysicsRayQueryParameters3D.Create(
		from: new Vector3(0.0f, 0.0f, 0.0f),
		to: new Vector3(1.0f, 1.0f, 1.0f));
	GD.Print(rayParams.Exclude); // This line is the problem.
}

3. Run the scene and watch as it immediately exits (without throwing any errors).

Minimal reproduction project

No response

[rhofour]

I'm not sure if this is really a physics problem as the same code causes a crash when in _Ready and you never have to actually use the ray parameters. I think this is an issue with the C# code, but I don't know enough about how that works to debug any further.

I checked PhysicsRayQueryParameters2D and it doesn't seem to have this problem.

[raulsntos]

I looked into this, the get_exclude method returns Vector<RID>:

godot/servers/physics_server_3d.h

Line 850 in ad3f2a2

Vector<RID> get_exclude() const;

The vector type is a C++ type, in scripting this type does not exist so we use Array. This means that the ptrcall goes through PtrToArg::encode to convert the vector returned by get_exclude into an array that can be returned to a scripting language.

godot/core/variant/method_ptrcall.h

Lines 306 to 313 in ad3f2a2

	_FORCE_INLINE_ static void encode(Vector<m_type> p_vec, void *p_ptr) { \
	Array *arr = reinterpret_cast<Array *>(p_ptr); \
	int len = p_vec.size(); \
	arr->resize(len); \
	for (int i = 0; i < len; i++) { \
	(*arr)[i] = p_vec[i]; \
	} \
	} \

The PtrToArg::encode method takes a p_ptr parameter which is the pointer to an Array where the contents of the vector will be copied to, in order to do this it first calls Array::resize to ensure the size of the array is the same as the size of the vector. The Array::resize method just resizes the internal vector used by Array to store its elements, the problem is if the array hasn't been initialized this internal data is null.

So, when calling this method from C# it uses ptrcall with a pointer to a godot_array as the return argument but this array is uninitialized so when the method tries to copy the vector to the array it finds the unexpected null data and crashes. The solution would be to initialize the array properly (I see that GDScript seems to use VariantInternal::initialize for this).

    internal static unsafe Godot.Collections.Array godot_icall_0_96(IntPtr method, IntPtr ptr) {
        godot_array ret = default;
        if (ptr == IntPtr.Zero)
            throw new ArgumentNullException(nameof(ptr));
        NativeFuncs.godotsharp_method_bind_ptrcall(method, ptr, null, &ret);
        return Godot.Collections.Array.CreateTakingOwnershipOfDisposableValue(ret);
    }

However, it would be unfortunate to initialize an array that doesn't need to be initialized. The array that is passed to ptrcall doesn't need to be initialized if the method returns Array or TypedArray because in those cases it can just assign the returned value to the return argument pointer directly, it's only needed for Vector because since this type doesn't exist in scripting it needs to be copied to an array first and this array needs to be initialized. Since PtrToArg::encode assumes that it's already initialized, we need to initialize it before that.

In this particular case though, the 2D version exposes get_exclude as a TypedArray<RID> which is why, as you found out, it works there.

godot/servers/physics_server_2d.h

Line 635 in ad3f2a2

TypedArray<RID> get_exclude() const;

So I think we should change the bindings in the 3D version to be consistent with 2D and also use a TypedArray<RID> which would have the side-effect of fixing this issue, but that doesn't mean the original issue with Vector<T> parameters should be ignored.