Fix object_set_instance being wrongly called for built-in wrapped classes

[groud]

This used to cause errors to be printed out, such as:

Class 'Image' has no native extension.
Class 'ImageTexture' has no native extension.
...

This happened for all objects created from a Godot built-in class. In such cases, we are not supposed to set the _extension and _extension_instance properties of the associated Godot Object, which object_set_instance does.

For this, I renamed _get_class to _get_extension_class, which will return nullptr for all built-in classes, and the extension class name for extension-only classes.

Also, I remove CHECK_CLASS_CONSTRUCTOR, it is unused since my recent constructor rework.

[BastiaanOlij]

I think this looks good but I believe @reduz needs to confirm our findings first, we're trying to piece together how this is supposed to work while he designed the Godot side of it and should be able to tell us if our theory is correct.

If it is, this fix looks sound to me.

[saki7]

I still see errors after applying adbbf1a.

ERROR: Condition "_instance_bindings != nullptr" is true.
   at: Object::set_instance_binding (core\object\object.cpp:1781)
...
...
...

https://github.com/godotengine/godot-cpp/pull/668/files#diff-3a21ab62e86b3efcdec23f9472ae337ac6b9f44f306e0d29a3d773a71d5d7867R45-R48

I think, these lines:

	if (extension_class) {
		godot::internal::gdn_interface->object_set_instance(_owner, extension_class, this);
	}
	godot::internal::gdn_interface->object_set_instance_binding(_owner, godot::internal::token, this, _get_bindings_callbacks());

Should be:

	if (extension_class) {
		godot::internal::gdn_interface->object_set_instance(_owner, extension_class, this);
		godot::internal::gdn_interface->object_set_instance_binding(_owner, godot::internal::token, this, _get_bindings_callbacks());
	}

[saki7]

Here's the minimal reproducible example for the error below:

ERROR: Condition "_instance_bindings != nullptr" is true.
   at: Object::set_instance_binding (core\object\object.cpp:1781)

Scene tree:

• TestContainer
  • Node

Extension code:

class TestContainer : public Node
{
    GDCLASS(TestContainer, Node);

public:
    static void _bind_methods() {}

    void _ready() override
    {
        WARN_PRINT("TestContainer::_ready()");
        get_node<Node>("Node"); // <-----------------------------------
    }
};

[vnen]

I haven't touched this code in a while, but from what I can gather the change is good.

The issue mentioned by @saki7 seems unrelated to this change, so IMO it doesn't need to be fixed in this PR. For what I know the built-in classes still need the instance binding since that's what is used in the godot-cpp side.

[akien-mga]

Thanks, let's merge then.

@saki7 Can you open a dedicated issue for the bug you're running into?

[akien-mga]

Thanks!