Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core: Fix Callable.get_bound_arguments{,_count}() return incorrect data #98713

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Core: Fix Callable.get_bound_arguments{,_count}() return incorrect data #98713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions core/variant/callable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand All @@ -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()));
Expand Down Expand Up @@ -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() {
Expand Down
6 changes: 4 additions & 2 deletions core/variant/callable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ class Callable {
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;

Expand Down Expand Up @@ -158,7 +159,8 @@ class CallableCustom {
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() {}
Expand Down
67 changes: 29 additions & 38 deletions core/variant/callable_bind.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);

r_argcount = sub_args.size() - argcount;
void CallableCustomUnbind::get_bound_arguments(Vector<Variant> &r_arguments) const {
callable.get_bound_arguments_ref(r_arguments);
}

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 {
Expand Down
6 changes: 4 additions & 2 deletions core/variant/callable_bind.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ class CallableCustomBind : public CallableCustom {
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; }

Expand Down Expand Up @@ -84,7 +85,8 @@ class CallableCustomUnbind : public CallableCustom {
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; }
Expand Down
16 changes: 9 additions & 7 deletions core/variant/variant.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3668,18 +3668,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);
}
Expand Down
1 change: 1 addition & 0 deletions core/variant/variant_call.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
19 changes: 17 additions & 2 deletions doc/classes/Callable.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand All @@ -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>
Expand Down
9 changes: 5 additions & 4 deletions scene/main/node.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
64 changes: 64 additions & 0 deletions tests/core/variant/test_callable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading