KSP-KOS / KOS

Fully programmable autopilot mod for KSP. Originally By Nivekk
Other
691 stars 229 forks source link

Parsing problem...Ubuntu, in cultures with comma decimals. #2163

Closed roxik0 closed 6 years ago

roxik0 commented 6 years ago

[LOG 00:25:33.262] kOS.Safe.Exceptions.KOSCompileException: Lock Throttle to 0.5 .

             ^

line 1, col 18: "0.5" is not in a recognized number string format. at kOS.Safe.Compilation.KS.Compiler.VisitDouble (kOS.Safe.Compilation.KS.ParseNode node) [0x00000] in :0

Any Idea? Environment: Ubuntu 16.04LTS

hvacengi commented 6 years ago

That's a new one for me. Have you had issues with any other decimal numbers in the past, or is this your first try?

The system we use to recognize the string value (regular expressions) is different from the system used to actually parse it (mono's built in parser). So it's possible to have a conflict between the two, but the string "0.5" shouldn't be one of those instances.

The only thought I have is that it appears that there is an extra space after the number in the script. That still shouldn't be an error but maybe it's the cause. I'll test it on my computer when I get home, but I don't have Ubuntu installed so @dunbaratu might need to fire up his copy to test.

tsholmes commented 6 years ago

we're not specifying the culture on the parsing, which will make it default to the system's culture. @roxik0 is your machine's locale set to one that uses commas as the decimal separator instead instead of periods?

roxik0 commented 6 years ago

Hi, again. So I change my culture(separator) into US and same story.. however my language is still polish. No change if there is this additional space or not. It;s high moded version If you want I will try to recreate problem tomorrow with only KOS installed.

//Edit: On Clean installation the same problem obraz

Dunbaratu commented 6 years ago

Is it possible the '0' character isn't an ASCII zero (hex 0x0030, or decimal 48), but some other Unicode character that looks like a zero? (Same question for the period). Can you do this to confirm? example.ks.

Lock Throttle to 0.5 .

run example.ks.

and it if has the error, then post an exact copy of example.ks for us to look at, in its raw binary form with no "cooking" of the text when you upload it. (i.e to be certain it doesn't get mangled by any text manipulation the website might do, ZIP it and attach the ZIP to a comment here.)

roxik0 commented 6 years ago

example.ks.zip As you wish still not working :(

Dunbaratu commented 6 years ago

When I run example.ks, it doesn't produce the error. This is very strange. I have no idea what step to take next to work on the problem. Can you verify that your numeric locale isn't set to use commas for decimals in some way? Perhaps by running some other application that obeys the formatting rules and seeing how it prints things? If it is a locale problem we probably would need to specify the culture explicitly in the kOS code. kerboScript isn't really set up to let you flip modes (it wouldn't know how to separate parameters passed to a function if comma doesn't always mean the delimiter between arguments).

roxik0 commented 6 years ago

Dunbaratu how can I verify that? from terminal after locale command:

LANG=pl_PL.UTF-8 LANGUAGE=pl LC_CTYPE="pl_PL.UTF-8" LC_NUMERIC=en_US.UTF-8 LC_TIME=en_US.UTF-8 LC_COLLATE="pl_PL.UTF-8" LC_MONETARY=en_US.UTF-8 LC_MESSAGES="pl_PL.UTF-8" LC_PAPER=en_US.UTF-8 LC_NAME=en_US.UTF-8 LC_ADDRESS=en_US.UTF-8 LC_TELEPHONE=en_US.UTF-8 LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=en_US.UTF-8 LC_ALL=

Dunbaratu commented 6 years ago

This is really confusing to me. If your LC_NUMBERIC is set to en_US.UTF-8, then that should mean that the number format libraries use US style decimal separators (".", not ","). Everything in the MSDN documentation claims that if the locale is not explicitly mentioned when calling double.TryParse() (which is the case in our code, I looked) then you get whatever the system's behaviour is (side note - we need to change that probably - we should explicitly override it so all players have the same result). But if you're getting it to expect comma instead of dot, then it's clearly NOT using your LC_NUMERIC=en_US.UTF-8" setting.

I wonder if this is a case where mono isn't quite right. I tried using google to see if anyone else is having similar problems and it seems people were complaining that the Unity3d editor always inherits from OS culture settings even if explicitly overridden, and this bug kept re-asserting itself.

Also, I know there are ways to change the locale for specific individual applications. I wonder if the locale environment variables are being set differently when running KSP than they are the rest of the time, in your setup (does it run a script that in turn runs KSP, and does that script alter LC_NUMERIC?)

Anyway, regardless of why it's expecting commas despite your LC_NUMERIC claiming to be en_US, and I can't figure out that bug without your computer in front of me, the point is that we could instruct the libraries to be invariant explicitly, and I think we missed that when calling TryParse.

Dunbaratu commented 6 years ago

I believe the correct answer will be to alter our code to explicitly state in all calls to TryParse that we need to use CultureInvariant.

That means we can't support the comma-decimal cultural styles, but then again the syntax of kerboscript makes it impossible to support it anyway because of this case:

local foo is list(1,2).

Is that a list of two items, 1, and 2? Or is it a list of one item who's value is "one and 2 tenths"?

As long as the language syntax says that it's legal for a list of arguments to have no whitespace between them, then we can't support comma-decimal styles.

So I think the only fix is to put CultureInvariant into all TryParse calls so at least the local locale doesn't end up taking over and asserting itself partway through the process of compiling the numbers.

@roxik0 - I can't easily test this on my own computer (I tried changing it to Polish language settings and couldn't re-create the problem, athough I had a really fun time trying to navigate the user interface after that to find the place to turn it back to English again.)

@roxik0 , If I give you a specially-made ZIP file with an experimental version of kOS in it, can you install it on your computer and tell me if it fixes the problem for you?

roxik0 commented 6 years ago

@Dunbaratu Yes you can give me this zip :)

Dunbaratu commented 6 years ago

Okay, @roxik0 it took a while for me to get back to this issue, but I did and I now have a ZIP file for you to try. Can you try this test version of kOS and tell me if it fixes the problem?

https://drive.google.com/open?id=1TBVwlgJkaxK_vjrYIPB5RdSMWdB2UKym

Please note what the README says:

This build of kOS is a test build that is just to check
issue 2163 (see github).  It is not compiled in the normal
way releases usually are so it shouldn't be used except to
test this problem.  The version number is deliberately
faked to be 1.1.3.29999 so as to leave clear evidence that
this was not a "normal" version should any logs ever be
submitted about it or bug reports be given about it.
pand5461 commented 6 years ago

Very weird. My locale number setting is Russian (which implies comma separator), and zero problems.

LANG=en_US.UTF-8
LANGUAGE=
LC_CTYPE="en_US.UTF-8"
LC_NUMERIC=ru_RU.UTF-8
LC_TIME=ru_RU.UTF-8
LC_COLLATE="en_US.UTF-8"
LC_MONETARY=ru_RU.UTF-8
LC_MESSAGES="en_US.UTF-8"
LC_PAPER=ru_RU.UTF-8
LC_NAME=ru_RU.UTF-8
LC_ADDRESS=ru_RU.UTF-8
LC_TELEPHONE=ru_RU.UTF-8
LC_MEASUREMENT=ru_RU.UTF-8
LC_IDENTIFICATION=ru_RU.UTF-8
LC_ALL=
pand5461 commented 6 years ago

Turns out, I've had no problems because I launch KSP through a bash script, as suggested in the forum thread for Linux users:

#!/bin/sh

export LC_ALL=C
exec taskset -c 2-3 ./KSP.x86_64

The "LC_ALL=C" line overrides the language settings.

Dunbaratu commented 6 years ago

Should close when PR #2196 is merged.