Handle missing instance binding callbacks by finding the closest parent

[dsnopek]

This attempts to solve a problem that was brought up on issue #1160 (although, this PR doesn't solve that issue).

Basically, the problem is that we assume we'll have the instance binding callbacks for any valid wrapper class. If we don't have the wrapper class, we output an error message and use Object.

However, in either of these cases:

• the class is a new class that was added to Godot after the GDExtension was created (so it won't know about it), or
• the developer explicitly excluded the class from godot-cpp

... we want to do what we're already doing for unexposed classes, which is to find the closest parent class that we do know about.

This will only output an error message, if we're unable to find any ancestor classes that we know about.

This depends on PR #1164 which is currently a draft. So, marking this one as a draft too!

Fixes #1163

[DmitriySalnikov]

ERROR: Condition "singleton_obj == nullptr" is true. Returning: nullptr
   at: godot::ClassDBSingleton::get_singleton (.../godot_debug_draw_3d/godot-cpp/gen/src/classes/class_db_singleton.cpp:44)

The problem is that the class is now called ClassDBSingleton, and not ClassDB, as singleton is named.

I added this patch:

diff --git a/binding_generator.py b/binding_generator.py
index d04c698..7d6f6e7 100644
--- a/binding_generator.py
+++ b/binding_generator.py
@@ -1453,7 +1506,10 @@ def generate_engine_class_source(class_api, used_classes, fully_used_classes, us
 
     if is_singleton:
         result.append(f"{class_name} *{class_name}::get_singleton() {{")
-        result.append(f"\tconst StringName _gde_class_name = {class_name}::get_class_static();")
+        if class_name != "ClassDBSingleton":
+            result.append(f"\tconst StringName _gde_class_name = {class_name}::get_class_static();")
+        else:
+            result.append(f"\tstatic const StringName _gde_class_name = StringName(\"ClassDB\");")
         result.append(
             "\tstatic GDExtensionObjectPtr singleton_obj = internal::gdextension_interface_global_get_singleton(_gde_class_name._native_ptr());"
         )
@@ -1480,7 +1536,10 @@ def generate_engine_class_source(class_api, used_classes, fully_used_classes, us
             result.append(method_signature + " {")
 
             # Method body.
-            result.append(f"\tconst StringName _gde_class_name = {class_name}::get_class_static();")
+            if class_name != "ClassDBSingleton":
+                result.append(f"\tconst StringName _gde_class_name = {class_name}::get_class_static();")
+            else:
+                result.append(f"\tstatic const StringName _gde_class_name = StringName(\"ClassDB\");")
             result.append(f'\tconst StringName _gde_method_name = "{method["name"]}";')
             result.append(
                 f'\tstatic GDExtensionMethodBindPtr _gde_method_bind = internal::gdextension_interface_classdb_get_method_bind(_gde_class_name._native_ptr(), _gde_method_name._native_ptr(), {method["hash"]});'

And now everything seems to be working as before. No errors, no crashes, at first glance.
Now the difference in size is not so significant, and it is quite acceptable.
37 MB -> 48 MB

Here is the action in my repository with this PR. (+9 MB for editor libs)

I hope this shouldn't affect performance too much. I didn't notice a major performance decrease overall.

---

I decided to try to compare the performance of the code that previously displayed a lot of errors. I didn't see any noticeable difference.

Details

void DebugDraw::test_tree_process() {
	uint64_t start_time = TIME()->get_ticks_usec();

	String editor3d = "Node3DEditorViewportContainer";
	String subviewport = SubViewport::get_class_static();
	Node *res = Utils::find_node_by_class(SCENE_ROOT(), editor3d);
	Node *n = res->get_child(0)->get_child(0);

	UtilityFunctions::print("Test processed: ", TIME()->get_ticks_usec() - start_time);
}

Node *Utils::find_node_by_class(Node *start_node, const String &class_name) {
	for (int i = 0; i < start_node->get_child_count(); i++) {
		auto c = start_node->get_child(i);
		if (c->get_class() == class_name)
			return c;
		auto nc = find_node_by_class(c, class_name);
		if (nc)
			return nc;
	}
	return nullptr;
}

#define SCENE_ROOT() (Object::cast_to<SceneTree>(Engine::get_singleton()->get_main_loop())->get_root())
#define TIME() Time::get_singleton()

First runs:

PR 1165	master	avg PR	avg master
3474	3529	3547	3563
3244	3164		
3202	4092		
4128	3987		
3144	4244		
3162	3190		
4400	3130		
3280	3256		
3669	3803		
3223	3282		
3407	3491		
4183	3814		
3798	3610		
3330	3452		
3288	3200		
3558	3980		
3219	3888		
3784	3124		
3313	3585		
3436	4532		
3313	4977		
3195	3204		
3427	3215		
4950	3202		
3334	3524		
4125	3222		
3339	3150		
3250	3349		
3699	3153		

Second runs:

PR 1165	master	avg PR	avg master
3986	3169	3588	3551
3498	3174		
5570	3141		
3214	3184		
3362	3520		
3300	3145		
3332	3155		
3636	3223		
3271	3108		
3192	3425		
3480	3286		
3426	3260		
3236	4652		
3462	3142		
3286	3470		
3306	5472		
3245	3274		
3657	4339		
5406	3159		
3277	3238		
3939	3453		
4163	3360		
3251	3280		
3323	3269		
3324	5647		
3267	3381		
3170	3787		
3211	3920		
4281	3357		

Without this PR, with excluded unused classes and with fixed crashes, the output looks like this:

...
ERROR: Cannot find instance binding callbacks for class 'HSplitContainer'.
   at: (godot-cpp\src\core\class_db.cpp:321)
ERROR: Cannot find instance binding callbacks for class 'VSplitContainer'.
   at: (godot-cpp\src\core\class_db.cpp:321)
ERROR: Cannot find instance binding callbacks for class 'SubViewportContainer'.
   at: (godot-cpp\src\core\class_db.cpp:321)
Test processed: 3134184
Test processed: 5601
Test processed: 3309
Test processed: 3402
Test processed: 3638
Test processed: 4239

[dsnopek]

@DmitriySalnikov:

The problem is that the class is now called ClassDBSingleton, and not ClassDB, as singleton is named.

I added this patch:

Thanks for the patch! I've integrated it into PR #1164 and rebased this PR on that.

And now everything seems to be working as before. No errors, no crashes, at first glance.

That's awesome! Thanks for testing it ❤️

[dsnopek]

Discussed at the GDExtension meeting, and we think this PR makes sense! However, it needs to wait on getting access to the ClassDB singleton's function before it can be merged.

[dsnopek]

Now that PR #1164 has been merged, I've rebased it and taken this one of draft!

We already discussed this PR at a GDExtension meeting back in July and approved it pending the merge of PR #1164, so I think it just needs passing tests, and someone to formally press the "Approve" button :-)

[BastiaanOlij]

Reviewed during GDExtension meeting, this looks like a good fix.

[dsnopek]

Cherry-picked for 4.1 in PR #1261