Calling a function taking a Ref<Resource> crashes

[Zylann]

I have a set_data function, which does nothing yet:

void HeightMap::set_data(Ref<Resource> new_data_ref) {
}

Which is registered this way:

	godot::register_method("set_data", &HeightMap::set_data);

If I call it in GDScript:

func test():
	var terrain = HeightMap2.new()
	add_child(terrain)
	var data = Resource.new()
	terrain.set_data(data)
	print(terrain.get_data())

It produces a crash as soon as the function ends (the print gets printed before).
It looks like a double-delete.

It doesn't reproduce if I try fully in C++:

void test(Ref<Resource> r) {

}

void HeightMap::_register_methods() {

	Ref<Resource> res(memnew(Resource));
	test(res);

Because I am unable to debug I tried putting an assert(0)

But that's all I can see about the state of the ref:

[Zylann]

I have a guess...
The pointer constructor of Ref<T> must increase the reference count, but in a special way. If it doesnt, as it is now, it will unref badly.
We decided to not reference in the pointer constructor because constructing a ref this way would produce a leak:

// Resource would have two refs, despite being only referenced in ref
Ref<T> ref(new Resource());

If we reference, we get a leak.
If we don't reference, we get a crash.

But Godot has a trick. I think we miss something, here is the Godot implementation:

	void ref_pointer(T *p_ref) {

		ERR_FAIL_COND(!p_ref);

		if (p_ref->init_ref())
			reference = p_ref;
	}

here is the C++ bindings implementation:

	Ref(T *r)
	{
		reference = r;
	}

Which means we... undo 2e4de7b ? That means there is something we missed at this step. The initial implementation might have been valid, but somehow a detail made it wrong...

[Zylann]

The initial Ref<T> implementation may have been correct after all... the leak could have originated from here instead #79

Let's rewrite Ref<T> to replicate exactly Godot's behavior to see what actually doesn't work...

[Zylann]

I finished rewriting it... and now everything works fine. getting, setting or instance() a Ref<T> triggers no leaks and no crashes using my test classs, with or without GodotScript<T> attached to it (though I'm still casting the userdata out of it).
I'll make a PR soon