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

GDScript: Fix incorrect error message for utility functions #78882

Merged
merged 1 commit into from
Oct 16, 2023
Merged

GDScript: Fix incorrect error message for utility functions #78882

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
134 changes: 106 additions & 28 deletions core/variant/variant_utility.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,27 @@ Variant VariantUtilityFunctions::floor(Variant x, Callable::CallError &r_error)
case Variant::VECTOR2: {
return VariantInternalAccessor<Vector2>::get(&x).floor();
} break;
case Variant::VECTOR2I: {
return VariantInternalAccessor<Vector2i>::get(&x);
} break;
case Variant::VECTOR3: {
return VariantInternalAccessor<Vector3>::get(&x).floor();
} break;
case Variant::VECTOR3I: {
return VariantInternalAccessor<Vector3i>::get(&x);
} break;
case Variant::VECTOR4: {
return VariantInternalAccessor<Vector4>::get(&x).floor();
} break;
case Variant::VECTOR4I: {
return VariantInternalAccessor<Vector4i>::get(&x);
} break;
default: {
r_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD;
return Variant();
}
r_error.error = Callable::CallError::CALL_ERROR_INVALID_ARGUMENT;
r_error.argument = 0;
r_error.expected = Variant::NIL;
return R"(Argument "x" must be "int", "float", "Vector2", "Vector2i", "Vector3", "Vector3i", "Vector4", or "Vector4i".)";
} break;
}
}

Expand All @@ -154,16 +165,27 @@ Variant VariantUtilityFunctions::ceil(Variant x, Callable::CallError &r_error) {
case Variant::VECTOR2: {
return VariantInternalAccessor<Vector2>::get(&x).ceil();
} break;
case Variant::VECTOR2I: {
return VariantInternalAccessor<Vector2i>::get(&x);
} break;
case Variant::VECTOR3: {
return VariantInternalAccessor<Vector3>::get(&x).ceil();
} break;
case Variant::VECTOR3I: {
return VariantInternalAccessor<Vector3i>::get(&x);
} break;
case Variant::VECTOR4: {
return VariantInternalAccessor<Vector4>::get(&x).ceil();
} break;
case Variant::VECTOR4I: {
return VariantInternalAccessor<Vector4i>::get(&x);
} break;
default: {
r_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD;
return Variant();
}
r_error.error = Callable::CallError::CALL_ERROR_INVALID_ARGUMENT;
r_error.argument = 0;
r_error.expected = Variant::NIL;
return R"(Argument "x" must be "int", "float", "Vector2", "Vector2i", "Vector3", "Vector3i", "Vector4", or "Vector4i".)";
} break;
}
}

Expand All @@ -187,16 +209,27 @@ Variant VariantUtilityFunctions::round(Variant x, Callable::CallError &r_error)
case Variant::VECTOR2: {
return VariantInternalAccessor<Vector2>::get(&x).round();
} break;
case Variant::VECTOR2I: {
return VariantInternalAccessor<Vector2i>::get(&x);
} break;
case Variant::VECTOR3: {
return VariantInternalAccessor<Vector3>::get(&x).round();
} break;
case Variant::VECTOR3I: {
return VariantInternalAccessor<Vector3i>::get(&x);
} break;
case Variant::VECTOR4: {
return VariantInternalAccessor<Vector4>::get(&x).round();
} break;
case Variant::VECTOR4I: {
return VariantInternalAccessor<Vector4i>::get(&x);
} break;
default: {
r_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD;
return Variant();
}
r_error.error = Callable::CallError::CALL_ERROR_INVALID_ARGUMENT;
r_error.argument = 0;
r_error.expected = Variant::NIL;
return R"(Argument "x" must be "int", "float", "Vector2", "Vector2i", "Vector3", "Vector3i", "Vector4", or "Vector4i".)";
} break;
}
}

Expand Down Expand Up @@ -236,9 +269,11 @@ Variant VariantUtilityFunctions::abs(const Variant &x, Callable::CallError &r_er
return VariantInternalAccessor<Vector4i>::get(&x).abs();
} break;
default: {
r_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD;
return Variant();
}
r_error.error = Callable::CallError::CALL_ERROR_INVALID_ARGUMENT;
r_error.argument = 0;
r_error.expected = Variant::NIL;
return R"(Argument "x" must be "int", "float", "Vector2", "Vector2i", "Vector3", "Vector3i", "Vector4", or "Vector4i".)";
} break;
}
}

Expand Down Expand Up @@ -278,9 +313,11 @@ Variant VariantUtilityFunctions::sign(const Variant &x, Callable::CallError &r_e
return VariantInternalAccessor<Vector4i>::get(&x).sign();
} break;
default: {
r_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD;
return Variant();
}
r_error.error = Callable::CallError::CALL_ERROR_INVALID_ARGUMENT;
r_error.argument = 0;
r_error.expected = Variant::NIL;
return R"(Argument "x" must be "int", "float", "Vector2", "Vector2i", "Vector3", "Vector3i", "Vector4", or "Vector4i".)";
} break;
}
}

Expand Down Expand Up @@ -333,14 +370,40 @@ int VariantUtilityFunctions::step_decimals(float step) {
}

Variant VariantUtilityFunctions::snapped(const Variant &x, const Variant &step, Callable::CallError &r_error) {
r_error.error = Callable::CallError::CALL_OK;
if (x.get_type() != step.get_type() && !((x.get_type() == Variant::INT && step.get_type() == Variant::FLOAT) || (x.get_type() == Variant::FLOAT && step.get_type() == Variant::INT))) {
r_error.error = Callable::CallError::CALL_ERROR_INVALID_ARGUMENT;
r_error.argument = 1;
r_error.expected = x.get_type();
return Variant();
switch (x.get_type()) {
case Variant::INT:
case Variant::FLOAT:
case Variant::VECTOR2:
case Variant::VECTOR2I:
case Variant::VECTOR3:
case Variant::VECTOR3I:
case Variant::VECTOR4:
case Variant::VECTOR4I:
break;
default:
r_error.error = Callable::CallError::CALL_ERROR_INVALID_ARGUMENT;
r_error.argument = 0;
r_error.expected = Variant::NIL;
return R"(Argument "x" must be "int", "float", "Vector2", "Vector2i", "Vector3", "Vector3i", "Vector4", or "Vector4i".)";
}

if (x.get_type() != step.get_type()) {
if (x.get_type() == Variant::INT || x.get_type() == Variant::FLOAT) {
if (step.get_type() != Variant::INT && step.get_type() != Variant::FLOAT) {
r_error.error = Callable::CallError::CALL_ERROR_INVALID_ARGUMENT;
r_error.argument = 1;
r_error.expected = Variant::NIL;
return R"(Argument "step" must be "int" or "float".)";
}
} else {
r_error.error = Callable::CallError::CALL_ERROR_INVALID_ARGUMENT;
r_error.argument = 1;
r_error.expected = x.get_type();
return Variant();
}
}

r_error.error = Callable::CallError::CALL_OK;
switch (step.get_type()) {
case Variant::INT: {
return snappedi(x, VariantInternalAccessor<int64_t>::get(&step));
Expand All @@ -367,9 +430,8 @@ Variant VariantUtilityFunctions::snapped(const Variant &x, const Variant &step,
return VariantInternalAccessor<Vector4i>::get(&x).snapped(VariantInternalAccessor<Vector4i>::get(&step));
} break;
default: {
r_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD;
return Variant();
}
return Variant(); // Already handled.
} break;
}
}

Expand All @@ -382,14 +444,31 @@ int64_t VariantUtilityFunctions::snappedi(double x, int64_t step) {
}

Variant VariantUtilityFunctions::lerp(const Variant &from, const Variant &to, double weight, Callable::CallError &r_error) {
r_error.error = Callable::CallError::CALL_OK;
switch (from.get_type()) {
case Variant::INT:
case Variant::FLOAT:
case Variant::VECTOR2:
case Variant::VECTOR3:
case Variant::VECTOR4:
case Variant::QUATERNION:
case Variant::BASIS:
case Variant::COLOR:
break;
default:
r_error.error = Callable::CallError::CALL_ERROR_INVALID_ARGUMENT;
r_error.argument = 0;
r_error.expected = Variant::NIL;
return R"(Argument "from" must be "int", "float", "Vector2", "Vector3", "Vector4", "Quaternion", "Basis, or "Color".)";
}

if (from.get_type() != to.get_type()) {
r_error.error = Callable::CallError::CALL_ERROR_INVALID_ARGUMENT;
r_error.argument = 1;
r_error.expected = from.get_type();
return Variant();
}

r_error.error = Callable::CallError::CALL_OK;
switch (from.get_type()) {
case Variant::INT: {
return lerpf(VariantInternalAccessor<int64_t>::get(&from), to, weight);
Expand All @@ -416,9 +495,8 @@ Variant VariantUtilityFunctions::lerp(const Variant &from, const Variant &to, do
return VariantInternalAccessor<Color>::get(&from).lerp(VariantInternalAccessor<Color>::get(&to), weight);
} break;
default: {
r_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD;
return Variant();
}
return Variant(); // Already handled.
} break;
}
}

Expand Down
6 changes: 3 additions & 3 deletions doc/classes/@GlobalScope.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@
<return type="Variant" />
<param index="0" name="x" type="Variant" />
<description>
Rounds [param x] upward (towards positive infinity), returning the smallest whole number that is not less than [param x]. Supported types: [int], [float], [Vector2], [Vector3], [Vector4].
Rounds [param x] upward (towards positive infinity), returning the smallest whole number that is not less than [param x]. Supported types: [int], [float], [Vector2], [Vector2i], [Vector3], [Vector3i], [Vector4], [Vector4i].
[codeblock]
var i = ceil(1.45) # i is 2.0
i = ceil(1.001) # i is 2.0
Expand Down Expand Up @@ -421,7 +421,7 @@
<return type="Variant" />
<param index="0" name="x" type="Variant" />
<description>
Rounds [param x] downward (towards negative infinity), returning the largest whole number that is not more than [param x]. Supported types: [int], [float], [Vector2], [Vector3], [Vector4].
Rounds [param x] downward (towards negative infinity), returning the largest whole number that is not more than [param x]. Supported types: [int], [float], [Vector2], [Vector2i], [Vector3], [Vector3i], [Vector4], [Vector4i].
[codeblock]
var a = floor(2.99) # a is 2.0
a = floor(-2.99) # a is -3.0
Expand Down Expand Up @@ -1133,7 +1133,7 @@
<return type="Variant" />
<param index="0" name="x" type="Variant" />
<description>
Rounds [param x] to the nearest whole number, with halfway cases rounded away from 0. Supported types: [int], [float], [Vector2], [Vector3], [Vector4].
Rounds [param x] to the nearest whole number, with halfway cases rounded away from 0. Supported types: [int], [float], [Vector2], [Vector2i], [Vector3], [Vector3i], [Vector4], [Vector4i].
[codeblock]
round(2.4) # Returns 2
round(2.5) # Returns 3
Expand Down
34 changes: 18 additions & 16 deletions modules/gdscript/gdscript_analyzer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3134,12 +3134,16 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
GDScriptUtilityFunctions::get_function(function_name)(&value, (const Variant **)args.ptr(), args.size(), err);

switch (err.error) {
case Callable::CallError::CALL_ERROR_INVALID_ARGUMENT: {
PropertyInfo wrong_arg = function_info.arguments[err.argument];
push_error(vformat(R"*(Invalid argument for "%s()" function: argument %d should be "%s" but is "%s".)*", function_name, err.argument + 1,
type_from_property(wrong_arg, true).to_string(), p_call->arguments[err.argument]->get_datatype().to_string()),
p_call->arguments[err.argument]);
} break;
case Callable::CallError::CALL_ERROR_INVALID_ARGUMENT:
if (value.get_type() == Variant::STRING && !value.operator String().is_empty()) {
push_error(vformat(R"*(Invalid argument for "%s()" function: %s)*", function_name, value), p_call->arguments[err.argument]);
} else {
// Do not use `type_from_property()` for expected type, since utility functions use their own checks.
push_error(vformat(R"*(Invalid argument for "%s()" function: argument %d should be "%s" but is "%s".)*", function_name, err.argument + 1,
Variant::get_type_name((Variant::Type)err.expected), p_call->arguments[err.argument]->get_datatype().to_string()),
p_call->arguments[err.argument]);
}
break;
case Callable::CallError::CALL_ERROR_INVALID_METHOD:
push_error(vformat(R"(Invalid call for function "%s".)", function_name), p_call);
break;
Expand Down Expand Up @@ -3181,18 +3185,16 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
Variant::call_utility_function(function_name, &value, (const Variant **)args.ptr(), args.size(), err);

switch (err.error) {
case Callable::CallError::CALL_ERROR_INVALID_ARGUMENT: {
String expected_type_name;
if (err.argument < function_info.arguments.size()) {
expected_type_name = type_from_property(function_info.arguments[err.argument], true).to_string();
case Callable::CallError::CALL_ERROR_INVALID_ARGUMENT:
if (value.get_type() == Variant::STRING && !value.operator String().is_empty()) {
push_error(vformat(R"*(Invalid argument for "%s()" function: %s)*", function_name, value), p_call->arguments[err.argument]);
} else {
expected_type_name = Variant::get_type_name((Variant::Type)err.expected);
// Do not use `type_from_property()` for expected type, since utility functions use their own checks.
push_error(vformat(R"*(Invalid argument for "%s()" function: argument %d should be "%s" but is "%s".)*", function_name, err.argument + 1,
Variant::get_type_name((Variant::Type)err.expected), p_call->arguments[err.argument]->get_datatype().to_string()),
p_call->arguments[err.argument]);
}

push_error(vformat(R"*(Invalid argument for "%s()" function: argument %d should be "%s" but is "%s".)*", function_name, err.argument + 1,
expected_type_name, p_call->arguments[err.argument]->get_datatype().to_string()),
p_call->arguments[err.argument]);
} break;
break;
case Callable::CallError::CALL_ERROR_INVALID_METHOD:
push_error(vformat(R"(Invalid call for function "%s".)", function_name), p_call);
break;
Expand Down
11 changes: 7 additions & 4 deletions modules/gdscript/gdscript_utility_functions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ struct GDScriptUtilityFunctionsDefinitions {

} else {
Variant::construct(Variant::Type(type), *r_ret, p_args, 1, r_error);
if (r_error.error != Callable::CallError::CALL_OK) {
*r_ret = vformat(RTR(R"(Cannot convert "%s" to "%s".)"), Variant::get_type_name(p_args[0]->get_type()), Variant::get_type_name(Variant::Type(type)));
}
}
}
#endif // DISABLE_DEPRECATED
Expand Down Expand Up @@ -130,8 +133,8 @@ struct GDScriptUtilityFunctionsDefinitions {
}
Error err = arr.resize(count);
if (err != OK) {
*r_ret = RTR("Cannot resize array.");
r_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD;
*r_ret = Variant();
return;
}

Expand All @@ -155,8 +158,8 @@ struct GDScriptUtilityFunctionsDefinitions {
}
Error err = arr.resize(to - from);
if (err != OK) {
*r_ret = RTR("Cannot resize array.");
r_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD;
*r_ret = Variant();
return;
}
for (int i = from; i < to; i++) {
Expand Down Expand Up @@ -199,8 +202,8 @@ struct GDScriptUtilityFunctionsDefinitions {
Error err = arr.resize(count);

if (err != OK) {
*r_ret = RTR("Cannot resize array.");
r_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD;
*r_ret = Variant();
return;
}

Expand Down Expand Up @@ -370,7 +373,7 @@ struct GDScriptUtilityFunctionsDefinitions {
*r_ret = gdscr->_new(nullptr, -1 /*skip initializer*/, r_error);

if (r_error.error != Callable::CallError::CALL_OK) {
*r_ret = Variant();
*r_ret = RTR("Cannot instantiate GDScript class.");
return;
}

Expand Down
15 changes: 7 additions & 8 deletions modules/gdscript/gdscript_vm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2067,11 +2067,11 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
#ifdef DEBUG_ENABLED
if (err.error != Callable::CallError::CALL_OK) {
String methodstr = function;
if (dst->get_type() == Variant::STRING) {
if (dst->get_type() == Variant::STRING && !dst->operator String().is_empty()) {
// Call provided error string.
err_text = "Error calling utility function '" + methodstr + "': " + String(*dst);
err_text = vformat(R"*(Error calling utility function "%s()": %s)*", methodstr, *dst);
} else {
err_text = _get_call_error(err, "utility function '" + methodstr + "'", (const Variant **)argptrs);
err_text = _get_call_error(err, vformat(R"*(utility function "%s()")*", methodstr), (const Variant **)argptrs);
}
OPCODE_BREAK;
}
Expand Down Expand Up @@ -2123,13 +2123,12 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a

#ifdef DEBUG_ENABLED
if (err.error != Callable::CallError::CALL_OK) {
// TODO: Add this information in debug.
String methodstr = "<unknown function>";
if (dst->get_type() == Variant::STRING) {
String methodstr = gds_utilities_names[_code_ptr[ip + 2]];
if (dst->get_type() == Variant::STRING && !dst->operator String().is_empty()) {
// Call provided error string.
err_text = "Error calling GDScript utility function '" + methodstr + "': " + String(*dst);
err_text = vformat(R"*(Error calling GDScript utility function "%s()": %s)*", methodstr, *dst);
} else {
err_text = _get_call_error(err, "GDScript utility function '" + methodstr + "'", (const Variant **)argptrs);
err_text = _get_call_error(err, vformat(R"*(GDScript utility function "%s()")*", methodstr), (const Variant **)argptrs);
}
OPCODE_BREAK;
}
Expand Down
2 changes: 2 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/gd_utility_function_wrong_arg.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
func test():
print(len(Color())) # GDScript utility function.
2 changes: 2 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/gd_utility_function_wrong_arg.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Invalid argument for "len()" function: Value of type 'Color' can't provide a length.
2 changes: 2 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/utility_function_wrong_arg.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
func test():
print(floor(Color())) # Built-in utility function.
2 changes: 2 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/utility_function_wrong_arg.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Invalid argument for "floor()" function: Argument "x" must be "int", "float", "Vector2", "Vector2i", "Vector3", "Vector3i", "Vector4", or "Vector4i".
Loading
Loading