Inconsistent Behavior For String/Bool Conversion

[Andrettin]

When converting a bool to a String, the String is set to "True" and "False" for the appropriate boolean values. However, when converting a String to a bool, the bool is set to true if the string is non-empty, regardless of the contents.

This inconsistent behavior means that if a bool set to false is converted to a string, and then is converted back, it will become true. Furthermore, the current string-to-bool conversion is not very useful for many (or even most) of the use cases for such conversions, e.g. custom text data parsing, where usually one wants to convert a string such as "true" to a bool.

I propose the following:

1. The bool to String conversion remains as it is.
2. For the String to bool conversion, the result is true if the String is "True", "TRUE", "true" or "1", and false if the String is "False", "FALSE", "false" or "0". If the string is set to anything else, then it is true if non-empty, and false otherwise.

If this proposal is accepted, then I will gladly work on the pull request to implement it.

[Zylann]

Which function are you using to do this conversion?

[Andrettin]

Which function are you using to do this conversion?

The Variant String() and bool() operators.

In the Variant String() operator bools are converted to strings in the manner I described:

		case BOOL: return _data._bool ? "True" : "False";

And the Variant bool() operator:

Variant::operator bool() const {

	return booleanize();
}

// We consider all uninitialized or empty types to be false based on the type's
// zeroiness.
bool Variant::booleanize() const {
	return !is_zero();
}

For a String, is_zero() returns true if it is empty (so that the behavior is as I described):

		case STRING: {

			return *reinterpret_cast<const String *>(_data._mem) == String();

The documentation on bool also describes it as working that way, note:
https://docs.godotengine.org/en/3.1/classes/class_bool.html#class-bool-method-bool

[Zylann]

Those operators are quite minimal, actually. In many languages there is a line between parsing and conversion, that's where it was decided to be, I guess^^
If you need such conversion, you shouldnt be using this IMO, but actual parsing to express your intent, because true, True or TRUE (or sometimes yes, no, 1 and 0) are words and they depend on your use case. It doesn't attempt to recognize more complex types like Vector3 either for that reason. String has to_int(), to_float(), and could be added to_bool() for expliciteness. Or that could be done in str2var? Something like true if s.to_lower() == "true" else false would do the trick, rather than logic that attempts to "be smart" in such a low-level implicit location (it could break areas of the engine already relying on it, without anyone noticing, like if(!value) { /* set default value */ }).

[Andrettin]

@Zylann I agree that adding a to_bool() function would be good, that was actually my original idea; as I saw that the to_int() and to_float() are both already consistent with the Variant conversion of a String to an int or float (by definition; the conversion from a String to an int simply calls to_int()), I thought it would be best for the Variant conversion to be changed, too.

I do see your point though about the Variant conversion being a different kind of location, though, and the sort of if conditions you mention could indeed be a reason for the Variant String-to-bool conversion to work differently from the int/float ones as it does presently.

In any case, adding a String::to_bool function would be enough to cover the use cases I mention, so that would solve the issue in my view.

Here is a suggested implementation:

bool String::to_bool() const {
	String lower = to_lower;

	if (lower == "true" || lower == "yes" || lower == "1")
		return true;
	else
		return false;
}

[KoBeWi]

Only "" is considered false, so that's the only one that should convert to false boolean. I guess the simplest conversion could work like this:

if self:
    return true
else:
    return false

EDIT:
I realized it's probably how it works now. Nevermind then.

[akien-mga]

What you want is var2str and str2var. Type casting is not meant for serialisation.

[Andrettin]

@akien-mga That functionality is unfortunately only available in gdscript; for C++ modules there is at present no simple and convenient way to convert strings to boolean values in the same way as for integers (besides, since "true" and "false" are extremely common keywords for those values, converting from those strings to the boolean values is IMO little different from converting the "5" string to the integer 5).

Converting a string to a variant doesn't quite cover the use case, either, since it can return a non-boolean value.

It is of course not a big deal either way, since this is more of a matter of convenience (I can just use a custom function of my own); I made the suggestion because I thought this could be useful to others as well.

[akien-mga]

I guess var2str and str2var should be exposed in the Variant and String APIs then.

[Xrayez]

You can do this in C++ with VariantWriter and VariantParser, just needs more work to set it up:

Variant var;
String to_str;
VariantWriter::write_to_string(var, to_str);

VariantParser doesn't seem to be as convenient:

VariantParser::StreamString ss;
ss.s = str;

String parse_err;
int err_line;
Variant var;
Error err = VariantParser::parse(&ss, var, parse_err, err_line);

[Poobslag]

I've written Andrettin's proposed boolean conversion functionality as a gdscript utility function.

static func to_bool(s: String) -> bool:
	var result: bool
	match s:
		"True", "TRUE", "true", "1": result = true
		"False", "FALSE", "false", "0": result = false
		_: result = false if s.empty() else true
	return result