(C++ Module) How to handle wrapper classes

:information_source: Attention Topic was automatically imported from the old Question2Answer platform.
:bust_in_silhouette: Asked By Paul Mauviel

So I’m currently working on extending the Dragonbones support to Godot.

I want to be able to wrap some dragonbones entities into a Godot class on request for return into the GDScripting system without having orphaned nodes lying about in memory.

As an example, here’s the function I’m using to get the wrapper object:


GDBone2D *GDArmatureDisplay::get_bone(const String &name) {
	GDBone2D *bone = memnew(GDBone2D)
	bone->set_data(getArmature()->getBone(name.ascii().get_data()));


	return bone->has_data() ? bone : nullptr;
}

Will this leak memory if, for example, I called it every frame? (i tried this and I saw memory going up very slowly but that could just be garbage collection not keeping up with the speed.

I’m registering the class with:

ClassDB::register_virtual_class<GDBone2D>

Thanks for the help

:bust_in_silhouette: Reply From: Zylann

It looks like this will leak memory. If you want this to not be the case, you should make GBone2D inherit Reference, and for safety, wrap it in a Ref<T> container:

Ref<GDBone2D> GDArmatureDisplay::get_bone(const String &name) {
    Ref<GDBone2D> bone;
    bone.instance();
    bone->set_data(getArmature()->getBone(name.ascii().get_data()));
    if(!bone->has_data()) {
    	// Sounds wasteful to have created a Bone2D object,
    	// if it has no data anyways. I suggest you check this earlier,
    	// before creating the wrapper.
    	bone.unref();
    }
    return bone;
}

Now about performance, there are two problems here:

  • You are instancing a heap-allocated object every time the user calls get_bone. That sounds quite wasteful because it will be re-created continuously if called every frame. It’s not a big deal if done 10 to 100 times per frame, but can become serious if done more, or if the data inside is actually heavy.

  • From the scripter’s point of view, a get_* function returning an object typically returns the same one. But here you create a different one each time, even if they reference the same bone. If they store it in a variable to later compare, it can be confusing. So either you could consider a different approach, or at least document that for the users to be aware of it.

About an alternative implementation:
You will notice in several places of the engine, that some APIs use an ID and many functions instead of full-blown objects. For example, the Skeleton class: https://docs.godotengine.org/en/stable/classes/class_skeleton.html
As you can see, there is no Bone wrapper class. This allows to further abstract the internal data structure, which can be useful, especially if it’s way too different to be easily exposed as full-blown objects.

Another way is to cache the wrapper (and have it reference the internal bone), but as any cache, you’ll need to be careful as to when it becomes invalidated.

You can also make it return a simple array or dictionary with the wanted info as well, if it’s meant for a short-lived use.

It looks like this will leak memory.

Yeah, it felt that way to me too.

Thanks for the reply. I was looking into Refbut I dont think I can have it be a Bone2D and a Reference at the same time.

Reading through, I think I’ll go in and create these Bone2Ds during my main armature’s creation and add them as children to the existing object so they get freed with the rest of it. It was going to be a step I took further along in development anyways so I might as well do it now.

This answered my question completely. I didn’t find many C++ resources that go into details about this kind of stuff.

Paul Mauviel | 2020-09-11 13:08

I dont think I can have it be a Bone2D and a Reference at the same time.

Oh, I thought Bone2D was a custom class you made. It’s actually a node. So you definitely can’t use it as reference, and you should not do what you were trying to do. Nodes should be in the scene then.

Zylann | 2020-09-11 18:01

Yeah, I went in and added them as children to the main node and now the get calls always reference the same object.

Also, Skeleton2D is a little different. That’s where the Bone2D has it’s own object defined:
Skeleton2D — Godot Engine (stable) documentation in English

Thanks :slight_smile:

Paul Mauviel | 2020-09-11 18:23