-
- Notifications
You must be signed in to change notification settings - Fork 688
Add variant_internal.hpp. #1655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
32f4df3 to 332690f Compare | Thanks! At a high-level, this looks great!
In order to discourage developers from using it (and create incompatibilities with Godot modules) you could put it in the
I don't know about this. Since I'd personally prefer to match Godot (even if it's wrong), and separately make an issue or PR for Godot to fix it there, and then we can match it in godot-cpp after.
Hm, it sounds like the problem is only for setting with |
2a05ce5 to 6a96858 Compare
That's fine by me. Done!
Ok. I changed it to a cast-return to match godot's interface.
Yep. I adjusted the PR accordingly. |
This module contains VariantInternalType, VariantInternal, VariantGetInternalPtr, VariantInternalAccessor and VariantDefaultInitializer, allowing to access and manipulate Variant's internal values.
6a96858 to daef7d4 Compare There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! The latest version of this looks good to me :-)
This module contains
VariantInternalType,VariantInternal,VariantGetInternalPtr,VariantInternalAccessorandVariantDefaultInitializer, facilitating access and manipulation of a Variant's internal value in various ways.The reference godot variant_internal.hpp is here. I chose a different, less boilerplate-y approach to implementation. It should work the same though. Doing it this way should decrease maintenance (and implementation) effort, since most of these are just copy-pasted code without any type specific logic (with the exception of
Object *).There are a few small differences:
VariantInternalTypedoes not exist in godot's implementation. I added it because it makes implementation of the other functions easier.get_objectreturnsObject * const *in my implementation, butconst Object **in godot's. Technically speaking, godot's implementation is wrong, because it allows manipulation of the containedObject *, even though aconst Variantwas supplied. Godot uses a C-style cast that acts as a const_cast to avoid cast issues.VariantInternalAccessoris not implemented forObject *, even though it is in Godot. I believe it is not possible to replicate the intended behavior without access toVariant::object_assign(or internalObjData). I made it fail statically using SFINAE.I think the rest should be the same.
PR contains a unit test for one of the internal accessors.
Depends on #1654.