C#: Skip methods with pointer parameters

[raulsntos]

Ignore methods with pointer parameters when generating the C# API. Currently these methods are generated using integer type parameters which makes them unusable.

Example of a method with pointer parameters: MultiplayerPeerExtension::_get_packet

godot/doc/classes/MultiplayerPeerExtension.xml

Lines 44 to 51 in 0ddd9c3

	<method name="_get_packet" qualifiers="virtual">
	<return type="int" enum="Error" />
	<param index="0" name="r_buffer" type="const uint8_t **" />
	<param index="1" name="r_buffer_size" type="int32_t*" />
	<description>
	Called when a packet needs to be received by the [MultiplayerAPI], with [param r_buffer_size] being the size of the binary [param r_buffer] in bytes.
	</description>
	</method>

This generates the following C#:

public virtual Error _GetPacket(long rBuffer, long rBufferSize)
{
    return default;
}

The parameters are Variant::INT, virtual methods currently don't have metadata so they are long (this is a separate issue unrelated to this PR), the important thing to notice is that these parameters are meant to be pointers because they are out parameters, the generated C# methods make them impossible to use.

This breaks compat because it removes methods from the public API but I don't think anyone is using them since they don't work.

For convenience, this is the diff of the generated C# API with this PR:

diff.zip

[RedworkDE]

These parameters can actually be used by casting them to the appropriate pointer type, so this should a somewhat useful implementation of that example method:

public override Error _GetPacket(long rBuffer, long rBufferSize)
{
	var buffer = GC.AllocateArray<byte>(1024, pinned: true);
	var len    = _stream.Read(buffer);
	_keepAlive = buffer;

	*(byte**)rBuffer = (byte*)Unsafe.AsPointer(ref buffer[0]);
	*(int*)rBufferSize = len;
	return default;
}

If proper type information about the pointer is not available, I would make these parameters void* or at least IntPtr to show that they are pointers and not just numbers without having to open the documentation of that method.

[neikeq]

Are these methods available in GDScript?

[raulsntos]

@RedworkDE I think using unsafe to make the method work is not the most intuitive so I'd rather not expose it like this. I agree that if we were to expose this a better way to do it would be to use pointers or IntPtr.

I'm not sure if we want to expose these methods but if we do we can always do it without breaking compatibility later, the main motivation for this PR is trying to remove potentially bad API before the stable release to avoid having to break compatibility later.

Are these methods available in GDScript?

@neikeq I don't think so, since GDScript doesn't support pointers. The API from the example in particular does have al alternative that is exposed so it can be used in GDScript:

godot/doc/classes/MultiplayerPeerExtension.xml

Lines 58 to 63 in 0ddd9c3

	<method name="_get_packet_script" qualifiers="virtual">
	<return type="PackedByteArray" />
	<description>
	Called when a packet needs to be received by the [MultiplayerAPI], if [method _get_packet] isn't implemented. Use this when extending this class via GDScript.
	</description>
	</method>

godot/scene/main/multiplayer_peer.h

Line 116 in 9ebb3e3

GDVIRTUAL0R(PackedByteArray, _get_packet_script); // For GDScript.

Which is also generated in C#:

public virtual byte[] _GetPacketScript()
{
    return default;
}

[neikeq]

If they're not usable from GDScript, I think it's fine to skip them. Especially, if there's no target type information. Not a big fan of those methods tbh. They're a poorly designed patch for exposing more advanced stuff to GDExtension.

[neikeq]

Wait, this is a bit different from the obscure struct pointer thing I had in mind. This one is declared as GDExtensionConstPtr<const uint8_t *>. Are we sure there's no way to retrieve that uint8_t type information?

[raulsntos]

Are we sure there's no way to retrieve that uint8_t type information?

There is, the hint_string gives you const uint8_t * in that case. But exposing them properly is a lot more involved than just skipping them for now, and we can always expose them later without breaking compat.

[neikeq]

the hint_string gives you const uint8_t * in that case

Never mind, it is what I'd thought initially. I don't have much interest in supporting that unless there's a case where it gives a great advantage over the normal API.

[akien-mga]

Thanks!