String::pad_zeros regression with padding less digits than length of string

[Grandro]

Godot version

v4.1.stable.official [9704596]

System information

Windows 11

Issue description

Using String::pad_zeros on a string containing digits with greater length than the argument digits raises the error Parameter count should be a positive number. of String::repeat, because it is called with a negative parameter.
(Actually this error is not 100% correct, it should be non-negative)

4.0 Code:

godot/core/string/ustring.cpp

Lines 4127 to 4155 in c8e0bd5

	String String::pad_zeros(int p_digits) const {
	String s = *this;
	int end = s.find(".");
	if (end == -1) {
	end = s.length();
	}
	if (end == 0) {
	return s;
	}
	int begin = 0;
	while (begin < end && !is_digit(s[begin])) {
	begin++;
	}
	if (begin >= end) {
	return s;
	}
	while (end - begin < p_digits) {
	s = s.insert(begin, "0");
	end++;
	}
	return s;
	}

Example: "2023".pad_zeros(2)
begin == 0
end == 4
end - begin < p_digits == false => No "0" is inserted, the string is returned unchanged.

4.1 Code:

godot/core/string/ustring.cpp

Lines 4266 to 4290 in ce5c615

	String String::pad_zeros(int p_digits) const {
	String s = *this;
	int end = s.find(".");
	if (end == -1) {
	end = s.length();
	}
	if (end == 0) {
	return s;
	}
	int begin = 0;
	while (begin < end && !is_digit(s[begin])) {
	begin++;
	}
	if (begin >= end) {
	return s;
	}
	int zeros_to_add = p_digits - (end - begin);
	return s.insert(begin, String("0").repeat(zeros_to_add));
	}

Example: "2023".pad_zeros(2)
begin == 0
end == 4
zeros_to_add == 2 - (4 - 0) == -2
=> s.insert(0, String("0").repeat(-2)) is called => Error.

I don't know why this was changed, but I think ensuring that zeros_to_add > 0 should be ensured internally and not by the user. If this condition is not fullfilled the string should be returned unchanged as in the 4.0 implementation.

Steps to reproduce

Try my above mentioned example in 4.0 and 4.1

Minimal reproduction project

Not necessary.

[raulsntos]

• This was changed in Use String.repeat() to optimize several String methods #72288.
• cc @MewPurPur

[MewPurPur]

I don't know why this was changed

It made the "adding zeros" part of the method from O(n) to O(logn), but I didn't foresee this situation. Onto it.

[kleonc]

raises the error Parameter count should be a positive number. of String::repeat, because it is called with a negative parameter.
(Actually this error is not 100% correct, it should be non-negative)

See #64489 (comment). 😄

• This was changed in Use String.repeat() to optimize several String methods #72288.

but I didn't foresee this situation.

And neither of 5 approvers did (including myself 🙃).