godot-cpp
godot-cpp copied to clipboard
[3.4.2] register_method does not accept arguments passed by reference
Let suppose a member method like this:
void Foo::bar(godot::String const name);
If we want to make it uses for GDNative script, we have to export it:
godot::register_method("bar", &Foo::bar);
This makes pass the string by copy and this works fine ! But now let pass name by reference:
void Foo::bar(godot::String const& name);
But this reference will produce a warning:
...
/home/qq/godot/3.4.2-stable/cpp/include/core/Godot.hpp:163:10: warning: returning reference to temporary [-Wreturn-local-addr]
163 | return a;
| ^
And possibly gives a crash. So How to deal: passing C++ reference with register_method ?
PS: register_method and C++ references worked well within Godot modules (C++ code inside the Godot editor code source inside the modules/ folder). I had no warnings.
@Calinou I rephrase my sentence because it was not very understandable.
@Calinou Maybe a patch could be in godot-cpp/include/core/Godot.hpp (godot 3.4.2) where the current code is:
template <class T>
struct _ArgCast {
static T _arg_cast(Variant a) {
return a; <---- this is my line 163
}
};
template <class T>
struct _ArgCast<T *> {
static T *_arg_cast(Variant a) {
return (T *)T::___get_from_variant(a);
}
};
template <>
struct _ArgCast<Variant> {
static Variant _arg_cast(Variant a) {
return a;
}
};
To add this:
template <class T>
struct _ArgCast<T &> {
static T _arg_cast(Variant a) {
return a;
}
};
I have no more warning, but I very unsure if this is the good solution since in my opinion this will make an extra copy that we do not want.
PS: why not these methods are not constexpr this could help avoiding making copies ?
PS: why not these methods are not constexpr this could help avoiding making copies ?
constexpr is a C++11 feature (or C++14/C++17 depending on context), and GDNative in Godot 3.x was largely designed around C++03.
In contrast, GDExtension in 4.0 can mandate C++17 support since Godot 4.0 itself requires a C++ compiler to be built.
That makes sense