godotengine
Add explicit conversions from Variant to Packed array references.
Related to #98373: These two fill similar use-cases. The main differences are that this PR is lighter weight, but also more prone to failure, and less powerful.
Related to https://github.com/godotengine/godot-proposals/issues/10830. This PR does not close the proposal, but partially addresses its use-cases. For more information, see my discussion with @dsnopek in the proposal.
I needed to say goodbye to lvalue references. Besides the rather explicit crashes on conversions, the explicit lvalue conversions were being used automatically leading to unit test crashes already.
Pointers make somewhat more sense anyway because it can be nullable, which is fitting in this case, requiring no explicit crash.
Ok, I refactored to use variant.get_xxx notation instead. This is closer to std::get<XXX>(variant) syntax, with the main difference being that we return a pointer rather than an lvalue reference, which is null on type failure rather than crashing.
I also added getters for all the other types, except Object which already has such a getter (get_validated_object()). This will allow in-place modification and non-copy use of all held types. Keep in mind this is not new behavior: Variant can already be modified in-place with call. The main addition in this PR is to allow the modification to be typesafe.
These functions seem basically the same as the VariantInternal::get_*() functions.
Given that we want to expose something like this on the GDExtension side, I'm not sure what the API should be. We could certainly make matching VariantInternal::get_*() functions in godot-cpp, so long as Godot provided us with a way to get the underlying pointers, although, perhaps that isn't the friendliest API?
So, probably the first thing is we need to decide on what API we want within Godot, and then we can figure out how to make the same work in GDExtension and godot-cpp.
These functions seem basically the same as the
VariantInternal::get_*()functions.
Oh, I had no idea those existed! There remains a small difference in that my implementation checks for variant state first. This may or may not be wanted; the std::get<>() API crashes when it's in a wrong state, and most uses of this API would probably also not check for null before de-referencing. I'm not sure what I prefer.
Given that we want to expose something like this on the GDExtension side, I'm not sure what the API should be. We could certainly make matching
VariantInternal::get_*()functions in godot-cpp, so long as Godot provided us with a way to get the underlying pointers, although, perhaps that isn't the friendliest API?
This would be blocked by Variant not exposing PackedArrayRef, a change proposed in https://github.com/godotengine/godot/pull/98373 but not accepted (or sufficiently discussed) yet.
So, probably the first thing is we need to decide on what API we want within Godot, and then we can figure out how to make the same work in GDExtension and godot-cpp.
There seem to be a few scattered uses of the *VariantInternal::get_xxx(variant) API in some spots, but mostly *VariantGetInternalPtr<xxx>::get_ptr(variant) seems to be used (which uses the above internally). That's probably because a lot of the code using it is using templates, and using this code to abstract its logic.
Superseded by #99201 because Variant internal has the proposed functionality already, an equivalent just needs to be exposed to gdextension.