Add C conversion constructors and fix new leak

[sheepandshepherd]

#355 fixed a leak caused by copy constructors. However, for Array and Dictionary, the fix now calls default constructors that allocate memory for an empty array/dictionary, again resulting in a leak. The other types fixed by #355 aren't affected - their default constructors do nothing.

The only way I can think of to convert a godot_array to a godot::Array without calling either constructor is to add a new Array(godot_array a) constructor to do the conversion. This way is clean and safe, but exposes part of the C API to C++. If we want to avoid that, maybe we can find a better way.

[zhangshiqian1214]

Yes, I test pool*array and found string, dictionary also be modified, so I test them. Use String will not leak. Array and Dictionay will leak. Now use your fix code, Dictionary and Array is ok.

[zhangshiqian1214]

@akien-mga can you merge this pull request

[Bromeon]

This seems to fix the problem, at least the leak warnings on GDNative/Godot shutdown disappear:

 WARNING: ObjectDB::cleanup: ObjectDB Instances still exist!
     At: core\object.cpp:2069
ERROR: ResourceCache::clear: Resources Still in use at Exit!
   At: core\resource.cpp:473

Code used to reproduce

var instance = A.new() # A is a custom GDNative class
var array = [
    B.new() # B is another GDNative class
]
	
instance.test_arrays(array)

void A::test_arrays(godot::Array arr)
{
	godot::Ref<B> elem = arr[0];
} // RAII should clean up here

---

Regarding exposing C types in the C++ API: if this is a problem, you can add the two Variant conversion operators as a friend of Array/Dictionary, respectively. I wouldn't add the entire Variant class, to still retain encapsulation.

[sheepandshepherd]

I wonder if reinterpret_cast would work - unlike the C casts that were originally used, it shouldn't call copy constructors. Will try that - it'd be a better option than adding any new API, since it's exactly what we're trying to do here anyway.

Edit: it does not. It's an invalid cast.

As for making the operators friend functions and the constructors private: that's a perfect idea, thanks! I forgot about friend.

That worked for Dictionary, but Array.hpp needs to forward-declare Variant instead of including its header. I don't know the structure of the headers well enough to reorganize them properly, so I used friend class Variant there instead.

[BastiaanOlij]

Any feedback on the last few changes? I'm happy to merge this in.

[Bromeon]

Two little things:

• You might want to make the constructor explicit
• Parameter should probably be a const-reference (even if sizeof(godot_array) is small in current implementation)

Same for the Dictionary constructor.

[sheepandshepherd]

Done, thanks again for the improvements.

[Zylann]

Thanks!