godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.7k stars 528 forks source link

Use explicit `::godot` namespace in gdvirtual.gen.inc #1412

Closed dsnopek closed 6 months ago

dsnopek commented 6 months ago

Related to this comment

Will hopefully allow extensions to use the GDVIRTUAL*() macros without requiring using namespace godot

Unfortunately, I don't currently have a project that isn't using using namespace godot to test this with, so I haven't tested if this is sufficient to fix the issue, but it does still build and work on the test project in godot-cpp.

Kehom commented 6 months ago

I haven't directly tested the PR yet, however looking at the changes I think there is a bunch of ::godot:: still missing. I have copied the macros that I needed and performed the changes until it's officially fixed. That said, I'm pasting here the const version for one argument with return:

#define GDVIRTUAL1RC(m_ret, m_name, m_type1)\
    ::godot::StringName _gdvirtual_##m_name##_sn = #m_name;\
    template <bool required>\
    _FORCE_INLINE_ bool _gdvirtual_##m_name##_call(m_type1 arg1, m_ret &r_ret) const {\
        if (::godot::internal::gdextension_interface_object_has_script_method(_owner, &_gdvirtual_##m_name##_sn)) { \
            ::godot::GDExtensionCallError ce;\
            ::godot::Variant vargs[1] = { Variant(arg1) };\
            const ::godot::Variant *vargptrs[1] = { &vargs[0] };\
            ::godot::Variant ret;\
            ::godot::internal::gdextension_interface_object_call_script_method(_owner, &_gdvirtual_##m_name##_sn, (const ::godot::GDExtensionConstVariantPtr *)vargptrs, 1, &ret, &ce);\
            if (ce.error == ::godot::GDEXTENSION_CALL_OK) {\
                r_ret = ::godot::VariantCaster<m_ret>::cast(ret);\
                return true;\
            }\
        }\
        if (required) {\
            ERR_PRINT_ONCE("Required virtual method " + get_class() + "::" + #m_name + " must be overridden before calling.");\
            (void)r_ret;\
        }\
        return false;\
    }\
    _FORCE_INLINE_ bool _gdvirtual_##m_name##_overridden() const {\
        return godot::internal::gdextension_interface_object_has_script_method(_owner, &_gdvirtual_##m_name##_sn); \
    }\
    _FORCE_INLINE_ static ::godot::MethodInfo _gdvirtual_##m_name##_get_method_info() {\
        ::godot::MethodInfo method_info;\
        method_info.name = #m_name;\
        method_info.flags = ::godot::METHOD_FLAG_VIRTUAL | ::godot::METHOD_FLAG_CONST;\
        method_info.return_val = ::godot::GetTypeInfo<m_ret>::get_class_info();\
        method_info.return_val_metadata = ::godot::GetTypeInfo<m_ret>::METADATA;\
        method_info.arguments.push_back(::godot::GetTypeInfo<m_type1>::get_class_info());\
        method_info.arguments_metadata.push_back(::godot::GetTypeInfo<m_type1>::METADATA);\
        return method_info;\
    }

As you can see, there are a lot of extra ::godot:: that must be added.

dsnopek commented 6 months ago

Some of those things aren't in the godot namespace, namely, anything in the gdextension_interface.h file, which is C, not C++. This includes, for example, GDExtensionCallError and GDEXTENSION_CALL_OK.

Kehom commented 6 months ago

Interesting. But for some reason here without prefixing those with the godot namespace I got errors (reported by VSCode). I didn't test if it at least compiled without though.

Kehom commented 6 months ago

Hi. I completely messed things up here while attempting to add the last push. Then each attempt to fix things triggered a full godot-cpp rebuild. In other words, it took me some time to finally test things. There is still one missing ::godot::!

There is a block that begins at line 77:

_FORCE_INLINE_ bool _gdvirtual_##m_name##_call($CALLARGS) $CONST {\\
        if (::godot::internal::gdextension_interface_object_has_script_method(_owner, &_gdvirtual_##m_name##_sn)) { \\
            GDExtensionCallError ce;\\
            $CALLSIARGS\\
            Variant ret;\\
            ::godot::internal::gdextension_interface_object_call_script_method(_owner, &_gdvirtual_##m_name##_sn, $CALLSIARGPASS, &ret, &ce);\\
            if (ce.error == GDEXTENSION_CALL_OK) {\\
                $CALLSIRET\\
                return true;\\
            }\\
        }\\

Inside of it there is a Variant! I did fix just that line (after refreshing the entire local codebase) and it worked. So I believe it's the last one that must be added.

dsnopek commented 6 months ago

Eep, sorry! That last one should be fixed in my latest push.

Kehom commented 6 months ago

Thank you very much! Just tested it. Seems to be working!

dsnopek commented 6 months ago

Thanks!