godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.6k stars 21.1k forks source link

C++ can not expose properly typed properties that return Script classes #96266

Open 0x0ACB opened 1 month ago

0x0ACB commented 1 month ago

Tested versions

4.3-stable

System information

Windows 11

Issue description

I am currently trying to add some properties to a C++ class that return GDScript classes. While everything is linked correctly in the documentation and autocomplete is working, image

trying to access the properties or casting them to the GDScript type results in image

Maybe I am doing something wrong but it seems the issue is this part here:

https://github.com/godotengine/godot/blob/ce8a837aab2568f9cdc41b4b410c478b0cd711fc/modules/gdscript/gdscript_analyzer.cpp#L5710

Where p_source is SCRIPT and p_target is CLASS When I implement the same functionality in GDScript, both p_source and p_target are CLASS

Steps to reproduce

-

Minimal reproduction project (MRP)


class ManagerMethodBind : public MethodBind
{
protected:
    Variant::Type _gen_argument_type(int p_arg) const override 
    {
        return Variant::OBJECT;
    }

    PropertyInfo _gen_argument_type_info(int p_arg) const override 
    {
        return PropertyInfo(m_class);
    }

public:
#ifdef DEBUG_METHODS_ENABLED
    GodotTypeInfo::Metadata get_argument_meta(int p_arg) const override 
    {
        return GodotTypeInfo::METADATA_NONE;
    }
#endif

    Variant call(Object* p_object, const Variant** p_args, int p_arg_count, Callable::CallError& r_error) const override 
    {
        return static_cast<Manager*>(p_object)->get_manager(m_name);
    }

    void validated_call(Object* p_object, const Variant** p_args, Variant* r_ret) const override 
    {
        *r_ret = static_cast<Manager*>(p_object)->get_manager(m_name);
    }

    void ptrcall(Object* p_object, const void** p_args, void* r_ret) const override 
    {
        *static_cast<Object**>(r_ret) = static_cast<Manager*>(p_object)->get_manager(m_name);
    }

    ManagerMethodBind(StringName name, StringName cls)
        : m_class(cls)
    {
        m_name =Manager::get_manager_name(cls);
        set_name(name);
        _set_returns(true);
        _generate_argument_types(0);
        set_argument_count(0);
    }

private:
    String m_name;
    StringName m_class;
};

void get_script_managers(const String& p_class, List<StringName>& r_inheriters)
{
    Array script_classes = ProjectSettings::get_singleton()->get_global_class_list();
    for (int i = 0; i < script_classes.size(); i++) 
    {
        Dictionary c = script_classes[i];
        if (!c.has("class") || !c.has("language") || !c.has("path") || !c.has("base")) 
        {
            continue;
        }
        if (String(c["base"]) == p_class)
        {
            r_inheriters.push_back(c["class"]);
        }
    }
}

void Manager::_bind_methods()
{
    auto bind_manager = [](const StringName& mgr)
    {
        const auto method_name = "get_" + get_manager_name(mgr);
        ClassDB::bind_method_custom(get_class_static(), memnew(ManagerMethodBind(method_name, mgr)));
        ADD_PROPERTY(PropertyInfo(Variant::OBJECT, get_manager_name(mgr), 
            PROPERTY_HINT_RESOURCE_TYPE, mgr, PROPERTY_USAGE_READ_ONLY), "", method_name.utf8());
    };

    List<StringName> game;
    ClassDB::get_inheriters_from_class("GameManager", &game);
    get_script_managers("GameManager", game);
    ADD_GROUP("Game", "");
    for (const auto& mgr : game)
    {
        bind_manager(mgr);
    }
}
Maximilian-Seitz commented 1 month ago

Your MRP isn't complete (it's not even a complete class, the header for the Manager, along with many methods are certainly missing), making it pretty hard to decipher what's going on there. If you'd provide an entire example, that would make testing a lot easier.

0x0ACB commented 1 month ago

Well effectively I am just retrieving (Script) classes that inherit from GameManager and then generate a getter. The relevant part is that for GDScript classes I use PropertyInfo["class"] as both the name as well as the type. Ie

extends GameManager
class_name CombatManager

Would create a C++ property:

ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "combat", 
            PROPERTY_HINT_RESOURCE_TYPE, "CombatManager", PROPERTY_USAGE_READ_ONLY), "",  "get_combat");

Which shows up correctly in the doc. You just cant use it. Changing this to

ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "combat", 
            PROPERTY_HINT_RESOURCE_TYPE, "GameManager", PROPERTY_USAGE_READ_ONLY), "",  "get_combat");

works. However then the autocomplete is only considering GameManager properties.

I can see about getting a complete MRP. However I do not have much time at the moment.

AThousandShips commented 1 month ago

Have you been able to make a complete MRP?

0x0ACB commented 1 month ago

Here you go:

class ExposeGDScript : public Object
{
    GDCLASS(ExposeGDScript, Object);
public:
    static void _bind_methods();

    Object* get_gdscript()
    {
        return m_value;
    }

    void set_gdscript(Object* obj)
    {
        m_value = obj;
    }
private:
    Object* m_value = nullptr;
};

namespace
{
class GDScriptMethodBind : public MethodBind
{
protected:
    Variant::Type _gen_argument_type(int p_arg) const override
    {
        return Variant::OBJECT;
    }

    PropertyInfo _gen_argument_type_info(int p_arg) const override
    {
        return PropertyInfo(m_class);
    }

public:
#ifdef DEBUG_METHODS_ENABLED
    GodotTypeInfo::Metadata get_argument_meta(int p_arg) const override
    {
        return GodotTypeInfo::METADATA_NONE;
    }
#endif

    Variant call(Object* p_object, const Variant** p_args, int p_arg_count, Callable::CallError& r_error) const override
    {
        return static_cast<ExposeGDScript*>(p_object)->get_gdscript();
    }

    void validated_call(Object* p_object, const Variant** p_args, Variant* r_ret) const override
    {
        *r_ret = static_cast<ExposeGDScript*>(p_object)->get_gdscript();
    }

    void ptrcall(Object* p_object, const void** p_args, void* r_ret) const override
    {
        *static_cast<Object**>(r_ret) = static_cast<ExposeGDScript*>(p_object)->get_gdscript();
    }

    GDScriptMethodBind(const StringName& name, const StringName& cls)
        : m_class(cls)
    {
        set_name(name);
        _set_returns(true);
        _generate_argument_types(0);
        set_argument_count(0);
    }

private:
    StringName m_class;
};
}

void ExposeGDScript::_bind_methods()
{
    const auto method_name = "get_gdscript";
    ClassDB::bind_method_custom(get_class_static(), memnew(GDScriptMethodBind(method_name, "GDScriptProperty")));
    ClassDB::bind_method("set_gdscript", &ExposeGDScript::set_gdscript);
    ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "script_property",
        PROPERTY_HINT_RESOURCE_TYPE, "GDScriptProperty", PROPERTY_USAGE_READ_ONLY), "set_gdscript", method_name);
}
extends Node
class_name GDScriptProperty

## test func
func test() -> void:
    print("test")
extends Node

func _init() -> void:
    var expose := ExposeGDScript.new()
    expose.script_property = GDScriptProperty.new()
    var test : GDScriptProperty = expose.script_property

image

image image

AThousandShips commented 1 month ago

Please provide a complete project to help testing this

0x0ACB commented 1 month ago

I mean how do you want that? This needs to be part of the engine build. Just copy it into a random register_types part and you are good to go.

AThousandShips commented 1 month ago

Got it! Wasn't clear this was for a module, as it was tagged with GDExtension

Can't test and build this right now (and it might depend on where you register it, so the example is still incomplete)

But exports in GDScript only work with Resource, Node or built-in types, so can you test what happens if you make this class a Resource type? It isn't possible or supported to export other types because of how export serialization is done

0x0ACB commented 1 month ago

The type you inherit from does not matter. It fails with the same error. Like I said in my initial post the issue is this part here:

https://github.com/godotengine/godot/blob/ce8a837aab2568f9cdc41b4b410c478b0cd711fc/modules/gdscript/gdscript_analyzer.cpp#L5710

Where p_source is SCRIPT and p_target is CLASS When I implement the same functionality in GDScript, both p_source and p_target are CLASS.

And yes setting the return type to test/GDScriptProperty.gd or any variation thereof also does not work.