Fix no cast in Ref<T>::ref()

[DmitriySalnikov]

Godot versions:
Windows 3.2.4 godotengine/godot#43554
Linux 3.2.3.stable
GDNative: #451

On Windows with the current Ref implementation on master brach, this code will crash after the first get_position because the reference is actualy nullptr for InputEventScreenTouch type and it has same address as iemb reference.

Ref<InputEventMouseButton> iemb(InputEventMouseButton::_new());
iemb->set_position(Vector2(12312312, 412421));

Ref<InputEventScreenTouch> iest = iemb;
if (iest.is_valid()) {
	Godot::print("cast 1");
	Godot::print(iest->get_position().operator godot::String());
}

Ref<InputEventScreenDrag> iesd(iemb);
if (iesd.is_valid()) {
	Godot::print("cast 2");
	Godot::print(iesd->get_position().operator godot::String());
}

Ref<InputEventScreenDrag> ie_null;
if (ie_null.is_valid()) {
	Godot::print("null");
	Godot::print(ie_null->get_position().operator godot::String());
}

Ref<InputEventMouseButton> iemb2(iemb);
if (iemb2.is_valid()) {
	Godot::print("same type");
	Godot::print(iemb2->get_position().operator godot::String());
}

On Linux no crash occurs but types not casted correctly too. Linux just use wrong memory segment and display zeroes or random values.

After this fix, the behavior of Windows and Linux is the same and correct as in normal modules

Also I tested this with my custom classes

Ref<GRPacketClientStreamAspect> a = GRPacketClientStreamAspect::_new();
a->set_aspect(0.99f);
if (a.is_valid()) {
	Godot::print("a");
	Godot::print(String::num_real(a->get_aspect()));
}

Ref<GRPacketServerSettings> b = a;
if (b.is_valid()) {
	Godot::print("b");
	Godot::print(b->get_settings()[0]);
}

Ref<GRPacketClientStreamOrientation> c = a;
if (c.is_valid()) {
	Godot::print("c");
	Godot::print(String::num_int64(c->is_vertical()));
}

Reference
└─GRPacket
  ├─GRPacketClientStreamAspect
  ├─GRPacketServerSettings
  ├─GRPacketClientStreamOrientation
  └─...

Without a fix Windows and Linux builds crashed on call to invalid Dictionary

a
0.99
b
handle_crash: Program crashed with signal 11
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[1] /lib/x86_64-linux-gnu/libc.so.6(+0x37840) [0x7f89afe3b840] (??:0)
[2] /home/dima/Desktop/Godot_v3.2.3-stable_x11.64() [0xde95cf] (??:?)
[3] godot::Dictionary::Dictionary(godot::Dictionary const&) (??:0)
[4] GRPacketServerSettings::get_settings() (??:0)
[5] GodotRemote::_init() (??:0)
[6] void* godot::_godot_class_instance_func<GodotRemote>(void*, void*) (??:0)
[7] /home/dima/Desktop/Godot_v3.2.3-stable_x11.64() [0x261c419] (<artificial>:?)
[8] /home/dima/Desktop/Godot_v3.2.3-stable_x11.64() [0xc63098] (??:?)
[9] /home/dima/Desktop/Godot_v3.2.3-stable_x11.64() [0xc6346b] (??:?)
[10] /home/dima/Desktop/Godot_v3.2.3-stable_x11.64() [0x12f99bf] (<artificial>:?)
[11] /home/dima/Desktop/Godot_v3.2.3-stable_x11.64() [0x12fb12e] (??:?)
[12] /home/dima/Desktop/Godot_v3.2.3-stable_x11.64() [0x2a77afa] (<artificial>:?)
[13] /home/dima/Desktop/Godot_v3.2.3-stable_x11.64() [0x85ce0d] (??:?)
[14] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xeb) [0x7f89afe2809b] (??:0)
[15] /home/dima/Desktop/Godot_v3.2.3-stable_x11.64() [0x86b68e] (??:?)
-- END OF BACKTRACE --

With fix:

Now the original Godot code and the code here are not the same, but now it works for me.

[DmitriySalnikov]

I'm not sure but maybe more leak tests are needed

[Zylann]

The cast makes sense, but looks like the location is not right. ref here expects a reference where T is the same between the source and destination references. I'm not even sure why it gets called (or even compiling?) while InputEventScreenTouch and InputEventMouseButton are incompatible types? Or perhaps the syntax without T means "any T"? I see it's used with the basic Reference somewhere so it might be a problem.
I would have expected this cast to occur in a function that expects a different T. If T is the same, no cast should occur, otherwise it would slow down every place the wrapper is assigned.

[Zylann]

Ok the real place to put the cast should actually be the places with // TODO We need a safe cast.
The reason ref ends up assigning the pointer is because of a cast that happened already, but it was done without any check.

[DmitriySalnikov]

Ok.
Can I close this pull request and let you fix this issue?

[Zylann]

Yeah, I'm working on a fix, though having a bit of trouble enforcing T being a Reference at compile-time... might be able to fix the cast but to have the extra nicety it might need more work

[Zylann]

New PR: #480