search

sarbian / ModuleManager

177 stars 96 forks source link

Double.TryParse is culture dependent by default #92

Closed pierrewillenbrock closed 6 years ago

pierrewillenbrock commented 6 years ago

This can lead to rather strange results when numbers like 1.001 turn into 1001 because Double.TryParse helpfully stripped '.' as thousands marker while expecting ',' as decimal separator.

I am experiencing this on my linux KSP install, but not on my windows KSP install, both using german culture settings. Either windows c-sharp is smarter at parsing numbers, or the culture should have been "fixed" long before ModuleManager got involved, but is not in my linux install, for some reason.

The use of Double.TryParse in ModuleManager has not changed in a long time, so there probably is no pressing need to change it right now. I would guess other places in KSP and its mods are similarly affected by non-english culture settings, so i will make sure my linux KSP is started with more appropriate settings.

blowfishpro commented 6 years ago

Interesting. Sounds like we need to use this version of the method, i.e.TryParse(string, NumberStyles.Float, CultureInfo.InvariantCulture.NumberFormat, out result)

sarbian commented 6 years ago

Damn, it had to be different on Linux... I will re-enable the resharper alerts about that and add the InvariantCulture everywhere... Technically this is a Unity bug and I wonder if they fixed it in the version that KSP 1.4 will use.

sarbian commented 6 years ago

Or I could force CultureInfo.CurrentCulture / Thread.CurrentThread.CurrentCulture... I ll see if I can create VM to test.

sarbian commented 6 years ago

There https://ksp.sarbian.com/jenkins/job/ModuleManager/137/ See if it is fixed

pierrewillenbrock commented 6 years ago

As far as i can tell, it is working fine now.

sarbian commented 6 years ago

Cool. There is one more thing I am curious about. What is the LANG of your Linux install ?

pierrewillenbrock commented 6 years ago

LANG (and LC_ALL) is set to "de_DE.utf8" on that machine. That may or may not be the correct setting for what i want. I notice that my other computer uses "de_DE.UTF-8".