Expose Variant's internal generic pointer for GDNative

[ghost]

Describe the project you are working on

I am currently working on a game that heavily relies on the performance of GDNative, and so I use a lot of custom C structs (for example Steamworks'es callbacks are POD structs that I am forced to pass without a wrapper). I would really like to have the ability to put generic pointers in Variants, but API is designed against that.

Describe the problem or limitation you are having in your project

If I need to somehow send some arbitrary pointers, then I have to resort to reinterpreting uint64_t in order to work around this issue in this way:

// Example C struct allocation
CStruct* status_callback = new CStruct();
// New empty Variant
Variant ptr_variant;
// Putting arbitrary pointer in Variant as reinterpreted uint64_t
ptr_variant = reinterpret_cast<uint64_t>(status_callback);
uint64_t out_int = ptr_variant;
// Getting value out and reinterpreting it back to the original pointer
CStruct* out_callback = reinterpret_cast<CStruct*>(out_int);

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Implementing void constructor/assignment operator will allow for less code for the codebases that are forced to operate on top of a lot of C structs that won't be feasible to be translated into wrapper Objects just to be stored in Variants.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

// Example C struct allocationn
CStruct* status_callback = new CStruct();
// New empty Variant
Variant ptr_variant;
// Putting arbitrary pointer in Variant as void*
ptr_variant = status_callback;

// First option
void* out_ptr = ptr_variant;
// Getting pointer out and casting it ourselfs
CStruct* out_callback = static_cast<CStruct*>(out_ptr );

// Second option
// Make a small utility function that takes care of the cast for us
CStruct* out_ptr = ptr_variant.void_cast<CStruct>();

If this enhancement will not be used often, can it be worked around with a few lines of script?

It can be worked around, but, for example, Urho3D has an almost identical implementation of Variant to Godot's, and they decided to expose arbitrary pointer assignment to their version of Variant. [Example]

Is there a reason why this should be core and not an add-on in the asset library?

Because it requires implementing Variant::Type::VOID_PTR, and that can only be done on the engine level.

[ghost]

As far as I am aware, Variant only cares about what is being passed/requested but it does not really care about what is actually stored inside of it. There is even a line void *_ptr; //generic pointer that declares generic pointer as a member of the internal union, so some considerations about this were definetly made when Variant was written.

[Calinou]

Is this something that can be implemented with relative ease and assurance that this won't break existing projects?

GDNative has been removed in favor of a new native extension system in master, and sweeping changes are unlikely to be made in the 3.x branch.

[ghost]

It seems that _data member is actually private, which means that I would have to introduce Variant::Type::VOID_PTR at the engine level no matter what happens since there is no easy way of accessing that _ptr member that I was talking about. Should still be an additive introduction though.

[CedNaru]

I approve of that. Currently, in our FMOD Gdnative plugin, I need to return a handle to some internal structure to the calling code (GDScript in that case). I am using the same strategy of casting the pointer to an int64, not really the most elegant solution and it can be error-prone for the user who can manipulate it as a simple integer.

https://github.com/utopia-rise/fmod-gdnative/blob/89c2e9a1edfec56761c849bff1d35368f7e1cf7e/src/godot_fmod.cpp#L644

Normally when we work with modules/servers, this kind of need is handled by the use of RID. Sadly we can't manage our own RID from GDNative either. So exposing more RID-related methods could also be a solution.

Note: The current void pointer in the Variant was not put there because of future considerations, it's just the pointer used when Variant wraps a Godot object ( which doesn't make that incompatible with using it the way you said of course).

[ghost]

@CedNaru raises a good point, considering that in preparation for 4.0, a lot of natively bound projects will be able to stop using modules altogether and just use a new native extension system instead (not everyone likes the fact that if you provide a module's source code, beginner users will also expect you to provide precompiled engine in multiple flairs with that module compiled, which is exactly what happened to GodotSteam), and because of that, there will be A LOT of GDNative projects (or however they are going to be called after 4.0) that would require constant pointer passing for whatever reason (custom allocations, C API compatibility, etc). I believe that this should be implemented as soon as possible, I just need to hear a word from the core team first.

[Xrayez]

(not everyone likes the fact that if you provide a module's source code, beginner users will also expect you to provide precompiled engine in multiple flairs with that module compiled, which is exactly what happened to GodotSteam)

This happened to Goost project as well, but it's not necessarily a bad thing if we also talk about extending Godot's core features that may not necessarily be project specific. For example, implementing native languages on par with GDScript/C# (with full support in relation to editor integration and debugging) would be near impossible via GDNative/extensions.

GodotSteam provides both GDNative and custom modules versions. But one of the reasons why people choose to download precompiled editor/export templates with custom modules is the desired "batteries included" experience, the way I see it.

[ghost]

GodotSteam provides both GDNative and custom modules versions. But one of the reasons why people choose to download precompiled editor/export templates with custom modules is the desired "batteries included" experience, the way I see it.

I talked to the original creator of GodotSteam when I was contributing to his project some time ago, and his reasoning is that he won't be touching GDNative until it will finalize before 4.0 (which is why the GDNative version is abandoned for some time now), it does not have anything to do with his preference of modules, he just doesn't want to upkeep a version on a backend that will get completely redone in the future, it is really that simple.

His library is great, but there are a lot of problems that one experiences even with module bindings that pass pointers all the time, since for whatever reason a lot of the methods constantly change types in comparison to the original code (uint64_t becomes int32_t, and backward), hence, why I am writing my own GDNative version that uses raw pointers instead of ints, which is the direct reason for this proposal.

IMHO, for a lot of libraries that do not require deep engine integration but those that operate on a higher level (Steamworks, FMOD, even TTS demo from the official docs sometimes uses raw pointers, it just was not shown), this is a must-have feature for the integrity of raw pointers.

[Xrayez]

His library is great, but there are a lot of problems that one experiences even with module bindings that pass pointers all the time, since for whatever reason a lot of the methods constantly change types in comparison to the original code (uint64_t becomes int32_t, and backward), hence, why I am writing my own GDNative version that uses raw pointers instead of ints, which is the direct reason for this proposal.

Why not contribute further? Does the original creator refuse to accept those kind of patches? Author's own decision not to touch GDNative does not necessarily mean that he won't accept patches. Or are you talking about Godot's own limitations in relation to module bindings?

this is a must-have feature for the integrity of raw pointers.

This is probably one of those features that only a few understand or care about, but when you do, you really wish it was there. 😃

[reduz]

This may or may not solve your requirement:

https://github.com/godotengine/godot/pull/52036

If you just want to store a generic pointer in a variant a retrieve it later you can cast it to int (given integers in Variant are 64 bits), although its quite of a strange use case.

Keep in mind also GDNative is dead in 4.0, and it was entirely replaced by a new extension system.

[CedNaru]

Well, the point of that proposal was to find a way so we don't have to cast a pointer to an int. We have to reinterpreted_cast on one side (Gdnative or extension) and users can use the casted pointer as a regular Int on the other. That's not typesafe.

[reduz]

A generic pointer is not type safe either.

[CedNaru]

That's why for modules and servers we have RID in Godot, so we can make that pointer totally opaque and safe. But we can't do that in GDNative (soon extensions).

[reduz]

RID is not exactly for this, although it wouldnt be too complex to expose RID creation for extensions, maybe that would go more in line with this proposal.

[CedNaru]

I perfectly agree with that.

[reduz]

Here https://github.com/godotengine/godot/pull/52045. This needs support on the GodotCPP side, but that's outside the scope of the PR.