Merge pull request #98713 from dalexeev/core-fix-callable-get-bound-arguments

Core: Fix `Callable.get_bound_arguments{,_count}()` return incorrect data
This commit is contained in:
Thaddeus Crews 2024-11-12 12:13:04 -06:00
commit cc6ee3e956
No known key found for this signature in database
GPG Key ID: 62181B86FE9E5D84
9 changed files with 150 additions and 63 deletions

View File

@ -206,19 +206,17 @@ int Callable::get_bound_arguments_count() const {
}
}
void Callable::get_bound_arguments_ref(Vector<Variant> &r_arguments, int &r_argcount) const {
void Callable::get_bound_arguments_ref(Vector<Variant> &r_arguments) const {
if (!is_null() && is_custom()) {
custom->get_bound_arguments(r_arguments, r_argcount);
custom->get_bound_arguments(r_arguments);
} else {
r_arguments.clear();
r_argcount = 0;
}
}
Array Callable::get_bound_arguments() const {
Vector<Variant> arr;
int ac;
get_bound_arguments_ref(arr, ac);
get_bound_arguments_ref(arr);
Array ret;
ret.resize(arr.size());
for (int i = 0; i < arr.size(); i++) {
@ -227,6 +225,14 @@ Array Callable::get_bound_arguments() const {
return ret;
}
int Callable::get_unbound_arguments_count() const {
if (!is_null() && is_custom()) {
return custom->get_unbound_arguments_count();
} else {
return 0;
}
}
CallableCustom *Callable::get_custom() const {
ERR_FAIL_COND_V_MSG(!is_custom(), nullptr,
vformat("Can't get custom on non-CallableCustom \"%s\".", operator String()));
@ -464,9 +470,12 @@ int CallableCustom::get_bound_arguments_count() const {
return 0;
}
void CallableCustom::get_bound_arguments(Vector<Variant> &r_arguments, int &r_argcount) const {
r_arguments = Vector<Variant>();
r_argcount = 0;
void CallableCustom::get_bound_arguments(Vector<Variant> &r_arguments) const {
r_arguments.clear();
}
int CallableCustom::get_unbound_arguments_count() const {
return 0;
}
CallableCustom::CallableCustom() {

View File

@ -111,8 +111,9 @@ public:
CallableCustom *get_custom() const;
int get_argument_count(bool *r_is_valid = nullptr) const;
int get_bound_arguments_count() const;
void get_bound_arguments_ref(Vector<Variant> &r_arguments, int &r_argcount) const; // Internal engine use, the exposed one is below.
void get_bound_arguments_ref(Vector<Variant> &r_arguments) const; // Internal engine use, the exposed one is below.
Array get_bound_arguments() const;
int get_unbound_arguments_count() const;
uint32_t hash() const;
@ -158,7 +159,8 @@ public:
virtual const Callable *get_base_comparator() const;
virtual int get_argument_count(bool &r_is_valid) const;
virtual int get_bound_arguments_count() const;
virtual void get_bound_arguments(Vector<Variant> &r_arguments, int &r_argcount) const;
virtual void get_bound_arguments(Vector<Variant> &r_arguments) const;
virtual int get_unbound_arguments_count() const;
CallableCustom();
virtual ~CallableCustom() {}

View File

@ -100,44 +100,42 @@ int CallableCustomBind::get_argument_count(bool &r_is_valid) const {
}
int CallableCustomBind::get_bound_arguments_count() const {
return callable.get_bound_arguments_count() + binds.size();
return callable.get_bound_arguments_count() + MAX(0, binds.size() - callable.get_unbound_arguments_count());
}
void CallableCustomBind::get_bound_arguments(Vector<Variant> &r_arguments, int &r_argcount) const {
Vector<Variant> sub_args;
int sub_count;
callable.get_bound_arguments_ref(sub_args, sub_count);
void CallableCustomBind::get_bound_arguments(Vector<Variant> &r_arguments) const {
Vector<Variant> sub_bound_args;
callable.get_bound_arguments_ref(sub_bound_args);
int sub_bound_count = sub_bound_args.size();
if (sub_count == 0) {
int sub_unbound_count = callable.get_unbound_arguments_count();
if (sub_bound_count == 0 && sub_unbound_count == 0) {
r_arguments = binds;
r_argcount = binds.size();
return;
}
int new_count = sub_count + binds.size();
r_argcount = new_count;
int added_count = MAX(0, binds.size() - sub_unbound_count);
int new_count = sub_bound_count + added_count;
if (new_count <= 0) {
// Removed more arguments than it adds.
r_arguments = Vector<Variant>();
if (added_count <= 0) {
// All added arguments are consumed by `sub_unbound_count`.
r_arguments = sub_bound_args;
return;
}
r_arguments.resize(new_count);
if (sub_count > 0) {
for (int i = 0; i < sub_count; i++) {
r_arguments.write[i] = sub_args[i];
}
for (int i = 0; i < binds.size(); i++) {
r_arguments.write[i + sub_count] = binds[i];
}
r_argcount = new_count;
} else {
for (int i = 0; i < binds.size() + sub_count; i++) {
r_arguments.write[i] = binds[i - sub_count];
}
Variant *args = r_arguments.ptrw();
for (int i = 0; i < added_count; i++) {
args[i] = binds[i];
}
for (int i = 0; i < sub_bound_count; i++) {
args[i + added_count] = sub_bound_args[i];
}
}
int CallableCustomBind::get_unbound_arguments_count() const {
return MAX(0, callable.get_unbound_arguments_count() - binds.size());
}
void CallableCustomBind::call(const Variant **p_arguments, int p_argcount, Variant &r_return_value, Callable::CallError &r_call_error) const {
@ -242,22 +240,15 @@ int CallableCustomUnbind::get_argument_count(bool &r_is_valid) const {
}
int CallableCustomUnbind::get_bound_arguments_count() const {
return callable.get_bound_arguments_count() - argcount;
return callable.get_bound_arguments_count();
}
void CallableCustomUnbind::get_bound_arguments(Vector<Variant> &r_arguments, int &r_argcount) const {
Vector<Variant> sub_args;
int sub_count;
callable.get_bound_arguments_ref(sub_args, sub_count);
void CallableCustomUnbind::get_bound_arguments(Vector<Variant> &r_arguments) const {
callable.get_bound_arguments_ref(r_arguments);
}
r_argcount = sub_args.size() - argcount;
if (argcount >= sub_args.size()) {
r_arguments = Vector<Variant>();
} else {
sub_args.resize(sub_args.size() - argcount);
r_arguments = sub_args;
}
int CallableCustomUnbind::get_unbound_arguments_count() const {
return callable.get_unbound_arguments_count() + argcount;
}
void CallableCustomUnbind::call(const Variant **p_arguments, int p_argcount, Variant &r_return_value, Callable::CallError &r_call_error) const {

View File

@ -55,7 +55,8 @@ public:
virtual const Callable *get_base_comparator() const override;
virtual int get_argument_count(bool &r_is_valid) const override;
virtual int get_bound_arguments_count() const override;
virtual void get_bound_arguments(Vector<Variant> &r_arguments, int &r_argcount) const override;
virtual void get_bound_arguments(Vector<Variant> &r_arguments) const override;
virtual int get_unbound_arguments_count() const override;
Callable get_callable() { return callable; }
Vector<Variant> get_binds() { return binds; }
@ -84,7 +85,8 @@ public:
virtual const Callable *get_base_comparator() const override;
virtual int get_argument_count(bool &r_is_valid) const override;
virtual int get_bound_arguments_count() const override;
virtual void get_bound_arguments(Vector<Variant> &r_arguments, int &r_argcount) const override;
virtual void get_bound_arguments(Vector<Variant> &r_arguments) const override;
virtual int get_unbound_arguments_count() const override;
Callable get_callable() { return callable; }
int get_unbinds() { return argcount; }

View File

@ -3664,18 +3664,20 @@ String Variant::get_call_error_text(Object *p_base, const StringName &p_method,
String Variant::get_callable_error_text(const Callable &p_callable, const Variant **p_argptrs, int p_argcount, const Callable::CallError &ce) {
Vector<Variant> binds;
int args_bound;
p_callable.get_bound_arguments_ref(binds, args_bound);
if (args_bound <= 0) {
return get_call_error_text(p_callable.get_object(), p_callable.get_method(), p_argptrs, MAX(0, p_argcount + args_bound), ce);
p_callable.get_bound_arguments_ref(binds);
int args_unbound = p_callable.get_unbound_arguments_count();
if (p_argcount - args_unbound < 0) {
return "Callable unbinds " + itos(args_unbound) + " arguments, but called with " + itos(p_argcount);
} else {
Vector<const Variant *> argptrs;
argptrs.resize(p_argcount + binds.size());
for (int i = 0; i < p_argcount; i++) {
argptrs.resize(p_argcount - args_unbound + binds.size());
for (int i = 0; i < p_argcount - args_unbound; i++) {
argptrs.write[i] = p_argptrs[i];
}
for (int i = 0; i < binds.size(); i++) {
argptrs.write[i + p_argcount] = &binds[i];
argptrs.write[i + p_argcount - args_unbound] = &binds[i];
}
return get_call_error_text(p_callable.get_object(), p_callable.get_method(), (const Variant **)argptrs.ptr(), argptrs.size(), ce);
}

View File

@ -2116,6 +2116,7 @@ static void _register_variant_builtin_methods_misc() {
bind_function(Callable, get_argument_count, _VariantCall::func_Callable_get_argument_count, sarray(), varray());
bind_method(Callable, get_bound_arguments_count, sarray(), varray());
bind_method(Callable, get_bound_arguments, sarray(), varray());
bind_method(Callable, get_unbound_arguments_count, sarray(), varray());
bind_method(Callable, hash, sarray(), varray());
bind_method(Callable, bindv, sarray("arguments"), varray());
bind_method(Callable, unbind, sarray("argcount"), varray());

View File

@ -155,13 +155,21 @@
<method name="get_bound_arguments" qualifiers="const">
<return type="Array" />
<description>
Return the bound arguments (as long as [method get_bound_arguments_count] is greater than zero), or empty (if [method get_bound_arguments_count] is less than or equal to zero).
Returns the array of arguments bound via successive [method bind] or [method unbind] calls. These arguments will be added [i]after[/i] the arguments passed to the call, from which [method get_unbound_arguments_count] arguments on the right have been previously excluded.
[codeblock]
func get_effective_arguments(callable, call_args):
assert(call_args.size() - callable.get_unbound_arguments_count() &gt;= 0)
var result = call_args.slice(0, call_args.size() - callable.get_unbound_arguments_count())
result.append_array(callable.get_bound_arguments())
return result
[/codeblock]
</description>
</method>
<method name="get_bound_arguments_count" qualifiers="const">
<return type="int" />
<description>
Returns the total amount of arguments bound (or unbound) via successive [method bind] or [method unbind] calls. If the amount of arguments unbound is greater than the ones bound, this function returns a value less than zero.
Returns the total amount of arguments bound via successive [method bind] or [method unbind] calls. This is the same as the size of the array returned by [method get_bound_arguments]. See [method get_bound_arguments] for details.
[b]Note:[/b] The [method get_bound_arguments_count] and [method get_unbound_arguments_count] methods can both return positive values.
</description>
</method>
<method name="get_method" qualifiers="const">
@ -182,6 +190,13 @@
Returns the ID of this [Callable]'s object (see [method Object.get_instance_id]).
</description>
</method>
<method name="get_unbound_arguments_count" qualifiers="const">
<return type="int" />
<description>
Returns the total amount of arguments unbound via successive [method bind] or [method unbind] calls. See [method get_bound_arguments] for details.
[b]Note:[/b] The [method get_bound_arguments_count] and [method get_unbound_arguments_count] methods can both return positive values.
</description>
</method>
<method name="hash" qualifiers="const">
<return type="int" />
<description>

View File

@ -3055,11 +3055,12 @@ void Node::_duplicate_signals(const Node *p_original, Node *p_copy) const {
if (copy && copytarget && E.callable.get_method() != StringName()) {
Callable copy_callable = Callable(copytarget, E.callable.get_method());
if (!copy->is_connected(E.signal.get_name(), copy_callable)) {
int arg_count = E.callable.get_bound_arguments_count();
if (arg_count > 0) {
int unbound_arg_count = E.callable.get_unbound_arguments_count();
if (unbound_arg_count > 0) {
copy_callable = copy_callable.unbind(unbound_arg_count);
}
if (E.callable.get_bound_arguments_count() > 0) {
copy_callable = copy_callable.bindv(E.callable.get_bound_arguments());
} else if (arg_count < 0) {
copy_callable = copy_callable.unbind(-arg_count);
}
copy->connect(E.signal.get_name(), copy_callable, E.flags);
}

View File

@ -135,6 +135,70 @@ TEST_CASE("[Callable] Argument count") {
memdelete(my_test);
}
class TestBoundUnboundArgumentCount : public Object {
GDCLASS(TestBoundUnboundArgumentCount, Object);
protected:
static void _bind_methods() {
ClassDB::bind_vararg_method(METHOD_FLAGS_DEFAULT, "test_func", &TestBoundUnboundArgumentCount::test_func, MethodInfo("test_func"));
}
public:
Variant test_func(const Variant **p_args, int p_argcount, Callable::CallError &r_error) {
Array result;
result.resize(p_argcount);
for (int i = 0; i < p_argcount; i++) {
result[i] = *p_args[i];
}
return result;
}
static String get_output(const Callable &p_callable) {
Array effective_args;
effective_args.push_back(7);
effective_args.push_back(8);
effective_args.push_back(9);
effective_args.resize(3 - p_callable.get_unbound_arguments_count());
effective_args.append_array(p_callable.get_bound_arguments());
return vformat(
"%d %d %s %s %s",
p_callable.get_unbound_arguments_count(),
p_callable.get_bound_arguments_count(),
p_callable.get_bound_arguments(),
p_callable.call(7, 8, 9),
effective_args);
}
};
TEST_CASE("[Callable] Bound and unbound argument count") {
String (*get_output)(const Callable &) = TestBoundUnboundArgumentCount::get_output;
TestBoundUnboundArgumentCount *test_instance = memnew(TestBoundUnboundArgumentCount);
Callable test_func = Callable(test_instance, "test_func");
CHECK(get_output(test_func) == "0 0 [] [7, 8, 9] [7, 8, 9]");
CHECK(get_output(test_func.bind(1, 2)) == "0 2 [1, 2] [7, 8, 9, 1, 2] [7, 8, 9, 1, 2]");
CHECK(get_output(test_func.bind(1, 2).unbind(1)) == "1 2 [1, 2] [7, 8, 1, 2] [7, 8, 1, 2]");
CHECK(get_output(test_func.bind(1, 2).unbind(1).bind(3, 4)) == "0 3 [3, 1, 2] [7, 8, 9, 3, 1, 2] [7, 8, 9, 3, 1, 2]");
CHECK(get_output(test_func.bind(1, 2).unbind(1).bind(3, 4).unbind(1)) == "1 3 [3, 1, 2] [7, 8, 3, 1, 2] [7, 8, 3, 1, 2]");
CHECK(get_output(test_func.bind(1).bind(2).bind(3).unbind(1)) == "1 3 [3, 2, 1] [7, 8, 3, 2, 1] [7, 8, 3, 2, 1]");
CHECK(get_output(test_func.bind(1).bind(2).unbind(1).bind(3)) == "0 2 [2, 1] [7, 8, 9, 2, 1] [7, 8, 9, 2, 1]");
CHECK(get_output(test_func.bind(1).unbind(1).bind(2).bind(3)) == "0 2 [3, 1] [7, 8, 9, 3, 1] [7, 8, 9, 3, 1]");
CHECK(get_output(test_func.unbind(1).bind(1).bind(2).bind(3)) == "0 2 [3, 2] [7, 8, 9, 3, 2] [7, 8, 9, 3, 2]");
CHECK(get_output(test_func.unbind(1).unbind(1).unbind(1).bind(1, 2, 3)) == "0 0 [] [7, 8, 9] [7, 8, 9]");
CHECK(get_output(test_func.unbind(1).unbind(1).bind(1, 2, 3).unbind(1)) == "1 1 [1] [7, 8, 1] [7, 8, 1]");
CHECK(get_output(test_func.unbind(1).bind(1, 2, 3).unbind(1).unbind(1)) == "2 2 [1, 2] [7, 1, 2] [7, 1, 2]");
CHECK(get_output(test_func.bind(1, 2, 3).unbind(1).unbind(1).unbind(1)) == "3 3 [1, 2, 3] [1, 2, 3] [1, 2, 3]");
memdelete(test_instance);
}
} // namespace TestCallable
#endif // TEST_CALLABLE_H