[WIP] Summarise current feature state (especially Ref) vs app/branch versions

[follower]

---

TL;DR:

• After ~ 9eceb16 do use:

  • Ref<CustomClass> ref = Ref<CustomClass>(custom_class_instance);

  and don't use:

  • Ref<CusClass> ref = Ref<CusClass> ref::__internal_constructor(cus_class_inst);

  If you use __internal_constructor() directly--since the "extra reference" bug has been fixed--then there won't be a reference to keep the object/reference alive so it'll be deleted (either immediately, or when it gets passed back to GDScript--I can't remember which).

  This may result in a crash or Godot complaining in a manner similar to this:

  [Object:0]
  SCRIPT ERROR: _init: Invalid call. Nonexistent function '<function_name>' in base 'previously freed instance'.
  

---

Details / testing

(This would make more sense as a wiki but seems it's not configured for public/logged in editing?

godot-cpp ref vs Godot Version:	3.0.0	3.1.0	3.1.2	3.2.0	3.2.1	3.2.2
3.0 ref
3.1 ref
3.2 ref
master ref		[A]* [B]	[A]* [B]	[A] [B]	[A] [B]

Features to test:

• Ref<Custom Class>
• Pool*Array
• How to use Ref
• Lack of memory leaks. :D

Update: [A]* It turns out 9eceb16 does work with 3.1.x--the error I was seeing was...because the test.gd file couldn't be found because the "current path" CLI calculation has apparently changed... Not that the error message indicated the issue was "file not found". :D (Note that PoolByteArray doesn't have a hex_encode() method in 3.1 and as it also has no call() method I don't know of a backwards-compatible way of conditionally calling it.)

Example [A] @ 9eceb16

# Foreigner.h

# ...

# ...

# Foreigner.cpp

# ...

Ref<ForeignBuffer> Foreigner::new_buffer(uint32_t size_in_bytes) {

    ForeignBuffer *the_buffer = ForeignBuffer::_new();

#...
 
    Ref<ForeignBuffer> ref = Ref<ForeignBuffer>(the_buffer);
    return ref;
}
# ...

Example [B] @ 9eceb16

Note: This was my initial test code.

Although this approach works with __internal_constructor() (because we hold onto the object & the reference ourselves when returning it to GDScript) this approach is now no longer necessary and the simpler approach used in Example [A] can be used.

(There may be some benefit for source code compatibility to staying with this approach until the extra reference bug fix is backported to 3.1 godot-cpp branch.)

# Foreigner.h

# ...

class Foreigner : public Reference {
    GODOT_CLASS(Foreigner, Reference)

ForeignLibrary *library;
Ref<ForeignLibrary> ref;

# ...

Ref<ForeignLibrary> open(String path);

#...

# Foreigner.cpp

Ref<ForeignLibrary> Foreigner::open(String path) {

#...

    library = ForeignLibrary::_new();
    ref = Ref<ForeignLibrary>::__internal_constructor(library);

# ...

    return ref;
}

#...

Foreigner::~Foreigner() {
    // ref.free(); // <--- No!                                                                                                                                              
    //ref = nullptr;  //<--- No!                                                                                                                                               
    ref.unref(); //  <--- Yes!
    ////library->free(); // <--- No!                                                                                                                                    
    library = nullptr; //  <--- Yes!
}

• So far [B] with the above implementation approach appears to pass custom class references without crashing and/or leaking memory compiled with current master godot-cpp on Mac, test good with 3.2.0-3.2.1 but does not load 3.1.0-3.1.2.

Example [C]

[WIP: TODO]

If upgrading to a later godot-cpp version is not an option for you, other approaches are:

• Use a godot-cpp release which leaks memory but doesn't crash.
• Split the _new() call & Ref<> creation into two parts & try storing a reference to the Object and/or Reference in your own class. (As used in Example [B] above.)
• Attempt to back-port the later fixes to earlier godot-cpp version.

Related PRs

• Merged:

  • Rework the way custom class instances are created from C++ #408
  • Add C conversion constructors and fix new leak #356
  • Fix leaks caused by implicitly-called copy constructors #355
  • Fix String::operator+ memory leak #333

• Unmerged:

  • Fix dangling pointer in custom Reference _new #401
  • Fixed memory leak with String objects #307
  • Reset reference counter to fix memory leak #350
  • Preparation to fix memory leak when using Ref in NativeScript godot#33532
  • Add Reference::reset_ref to api.json godot-headers#60
  • Fix segmentation fault when using Ref #346

Related Issues

• ERROR: cleanup: There are still MemoryPool allocs in use at exit! #275 [BUG] Templates are messing everything up!! #276 Custom reference class instantiation from within other GDNative classs stopped working #274 Docs: GDnative Reference Counting #281 Custom GDNative class as class variable triggers crash #322 Custom type Ref<T> leaks #215 A problem with the Ref #343 How to instantiate custom class via GDNative (Question) #253

[WIP]

[dancullen]

@follower Thanks for the great summary of this info!

I just tested v3.2.2 and the master branch of godot-cpp (9eceb16). Approach [A] indeed works for me. i.e.,

...
godot::Ref<MyClass> MyLibrary::create(void)
{
    return godot::Ref<MyClass>(MyClass::_new());
}
...

The above commit fixes the issues I was seeing (e.g., #215).

I haven't had a chance to try [B].

[norton-corbett]

This causes a crash on Windows with the latest version of godot-cpp. Am i doing something wrong?

#pragma once

#include <Godot.hpp>
#include <Node.hpp>

using namespace godot;

class Thing : public Reference
{
	GODOT_CLASS(Thing, Reference);

public:

	static void _register_methods()
	{
		register_method("_init", &Thing::_init);
	}

	void _init() {}
};

class ThingMaker : public Node
{
	GODOT_CLASS(ThingMaker, Node);

public:

	static void _register_methods()
	{
		register_method("_init", &ThingMaker::_init);
		register_method("make_thing", &ThingMaker::make_thing);
	}

	void _init() {}

	Ref<Thing> make_thing()
	{
		Thing* new_thing = Thing::_new();

		Ref<Thing> ref = Ref<Thing>(new_thing); // crash

		return ref;
	}
};

[o01eg]

3.2 branch still segfaults on

Ref<CustomClass> ref = Ref<CustomClass>(custom_class_instance);

and works with __internal_constructor approach but it segfaults in Ref copy constructor.

[o01eg]

@norton-corbett Looks like inst->_owner contains an invalid pointer. I get in on Linux too #475