Set vararg methods' ptrcall of builtin classes, and let them can be called without arguments.

[Daylily-Zeleen]

1. Set VariantBuiltInMethodInfo::ptrcall for builtin vararg methods.
2. Let builtin vararg methods' ptrcall can be called without arguments.

I don't not the reason why the origin logic set vararg methods' VariantBuiltInMethodInfo::ptrcall to nullptr, I just set them like normal method, and in my test, it work fine.

This pr is made for implementing builtin classes' vararg methods in gdextension( refer to this pr #1091).

[dsnopek]

Thanks!

This works in my testing along with the godot-cpp PR godotengine/godot-cpp#1091

The one thing I'm unsure about is using the lambda function to make the argument pointer.

Why not convert this code:

LocalVector<Variant> vars;
vars.resize(p_argcount);
LocalVector<const Variant *> vars_ptrs;
vars_ptrs.resize(p_argcount);
for (int i = 0; i < p_argcount; i++) {
	vars[i] = PtrToArg<Variant>::convert(p_args[i]);
	vars_ptrs[i] = &vars[i];
}                                

... into something like:

LocalVector<Variant> vars;
LocalVector<const Variant *> vars_ptrs;
if (p_argcount > 0) {
	vars.resize(p_argcount);
	vars_ptrs.resize(p_argcount);
	for (int i = 0; i < p_argcount; i++) {
		vars[i] = PtrToArg<Variant>::convert(p_args[i]);
		vars_ptrs[i] = &vars[i];
	}
} else {
	vars.push_back(Variant());
	vars_ptrs.push_back(&vars[0]);
}

That seems much simpler and easier to understand to me, and it avoids a lambda, which we don't use much in Godot.

[Daylily-Zeleen]

That seems much simpler and easier to understand to me, and it avoids a lambda, which we don't use much in Godot.

Because zero argument calling sill need to memnew a Variant, which we know that this value has no real meaning.
To avoid memnew a unsued Variant for zero argument calling every time, I use a lambda to return a static const const Variant **, which means that it only mennew once if it is needed.

Of course, this only a micro optimization.
Accroding to your comment, I think the readability of code is more important, so I modified this pr to avoid using lamda function.

[dsnopek]

Thanks!

With the latest changes, this looks great to me! And it's working perfectly from the perspective of GDExtension (via PR godotengine/godot-cpp#1091).

However, it'd be great to get a review from someone on the @godotengine/core team, to double check that this wouldn't have any unexpected implications elsewhere.

[rburing]

The two ptrcall methods changed here are in the two macros VARARG_CLASS and VARARG_CLASS1, which are used in the two macros bind_custom and bind_custom1, which in turn are used in 6 places, so m_method_ptr can be one of

• _VariantCall::func_Callable_call
• _VariantCall::func_Callable_call_deferred
• _VariantCall::func_Callable_rpc
• _VariantCall::func_Callable_bind
• _VariantCall::func_Signal_emit
• _VariantCall::func_Callable_rpc_id

From a glance these all seem to support the case p_argcount == 0 and p_arguments == nullptr just fine, so the changes to the two ptrcall methods could just be reduced to:

    m_method_ptr(&base, vars_ptrs.ptr(), p_argcount, ret, ce);

avoiding the pointless Variant allocation.

If I'm wrong and one of the methods actually doesn't handle the case gracefully, then I propose handling it there (should be easy).

[Daylily-Zeleen]

@rburing Now I remove the branch of p_argcount<=0, and do some simple test about calling call, bind, call_deferred of Callable and Signal::emit without argument, the result is work fine.
I roughly traced the mechanism of Callable::rpc and Callable::rpc_id, it seems that calling callp, too. I think these two methods should work fine, too.

I'm not sure this change is safe or not, please review carefully.

[rburing]

Looks good to me.

[rburing]

This reordering (leftover from the previous version) is not necessary, but it doesn't hurt either.

[dsnopek]

Thanks for your continued work on this!

I just retested the latest PR with godot-cpp, and it seems to be working just fine, including when passing no arguments to a vararg function.

[akien-mga]

Thanks!