[A17b] Possible Bug in Verse.Widgets.TextFieldNumericLabeled<T>

Started by neitsa, June 27, 2017, 12:39:44 AM

Previous topic - Next topic

neitsa

Hello,

I am trying to use Verse.Widgets.TextFieldNumericLabeled<T> is one of my mod with T as float.


        protected virtual void DrawCurrentMovementTime(ref float minMovementTime, ref float maxMovementType, ref bool checkedOn)
        {
            ListingStandard.Gap(DefaultGapHeight);
            ListingStandard.CheckboxLabeled("Current Movement Time", ref checkedOn, "Use Min/Max Current Movement Time");
            var currMovTimeMinRect = ListingStandard.GetRect(DefaultElementHeight);
            Widgets.TextFieldNumericLabeled(currMovTimeMinRect, "Min: ", ref minMovementTime, ref _currentMovTimeMinString);
            var currMovTimeMaxRect = ListingStandard.GetRect(DefaultElementHeight);
            Widgets.TextFieldNumericLabeled(currMovTimeMaxRect, "Max: ", ref maxMovementType, ref _currentMovTimeMaxString);
        }


The code kinda "works" but, if T is float (as in the above code snippet) you should be able to enter a decimal point in the number, which in fact is not possible (Also, please note that I think that the same bug applies to TextFieldNumeric<T> if T is float.)

So I decided to check where was the problem (ripped off the whole code and debugged it). It seems to lie in IsFullyTypedNumber<T> (string extension method in Verse.Widgets).

Say the user wants to type in "12.34" in the TextFieldNumericLabeled widget. As soon as we hit the '.' in "12.", we have the following state (note: code is from RimWorld, comments are my own. Please let me know if it's OK or not to post decompiled code):


private static bool IsFullyTypedNumber<T>(this string s)
{
if (s == string.Empty) // nope!
{
return false;
}
if (typeof(T) == typeof(float)) // yes
{
string[] array = s.Split(new char[]
{
'.'
});

// we have array.Length = 2
// we have array[0] = "12" and array[1] = ""

if (array.Length > 2 || array.Length < 1)  // none is true
{
return false;
}
if (!array[0].ContainsOnlyCharacters("-0123456789")) // false
{
return false;
}

// array.Length == 2 is true but the right hand of the statement is false, so we don't enter
if (array.Length == 2 && !array[1].ContainsOnlyCharacters("0123456789"))
{
return false;
}
}

//   the left hand of the statement is true, so we return true
return typeof(T) != typeof(int) || s.ContainsOnlyCharacters("-0123456789");
}


So, as commented in the above snippet, the code returns true while it should have returned false. Note that the string actually processed is "12" and the decimal part never make it into the GUI.

All in all I patched the code as follow:


                if (array.Length > 2 || array.Length < 1)
                {
                    return false;
                }

                // Patch: check that there is a decimal point and nothing else after it
                // return false to indicate that the number is *not* fully typed yet.
                if (array.Length == 2)
                {
                    if (array[1] == string.Empty)
                        return false;
                }
                // end patch

                if (!array[0].ContainsOnlyCharacters("-0123456789"))
                {
                    return false;
                }


With the above patch, I can enter a decimal number in a Verse.Widgets.TextFieldNumericLabeled<T> widget.

I might be wrong and didn't understand how the widget works though, in which case, I'd like to apologize. I really hate to report something that's not a bug.

Thank you!

ison