[v1.7.2.1] Voice Attack's doing the math wrong

[BlackJack]

What is {EXP:50*0.5}? Voice Attack says: 250!
Math wasn't my favorite subject at school, but that doesn't seem to be quite right.

[Pfeil]

Are you setting a variable to this value(of which type?), or using the token directly to write to the log?

Does your locale use "." or "," as a decimal separator?


On my machine

Code: [Select]

Write [Blue] '{EXP:50*0.5}' to log
writes "25" to the log.

[BlackJack]

Nope, I wrote the Expression directly into the Log.
My locale uses a comma as separator. But I get an error, when I use it for the expression.

[Gary]

Hi, BlackJack.  Good to see you back!   

Just curious... are you using a language in Windows that does not match the locale you have selected?  The component that's being used for the {EXP} token is Microsoft-authored, so, it's a mystery as to why that does not work straight away.

[BlackJack]

Hi, Gary.

I can't see any anomalies in my locale settings and normally I don't change these.Not sure if this came with a windows or a voice attack update, or if any other software made some locale changes. I've noticed this with some very old scripts, I've not used for a while.

This is a SpeedCrunch output:

Code: [Select]

50*0.5 = 250 ; 50*0,5 = 25So it must be something in this direction.
But VA don't like a comma as a decimal separator and it worked with a point in the past. now I'm a little at a loss.

[BlackJack]

okay, I found the reason.
It was a plugin.

Sorry about the false suspicions. I didn't expect that a plugin can do this...
Best Regards

[Gary]

Lol.  What plugin was causing this and how were you using it to make that happen?

[BlackJack]

It's called EDDI.The best thing is, it's got a localization, and this matches the time I registered the issue.

[Gary]

I think I've got this fixed up.  I'll have something for you soon, as the code is kind of guts-out at the moment.

[BlackJack]

Great! I'm looking forward to it. Most likely nothing will come from the EDDI side. But it is not really urgent either, as the problem can be easily workarounded.

[Tkael]

For the record, I reached out to Gary via PM to discuss this issue on behalf of the EDDI team and proposed several possible solutions. Gary investigated and was able to find a solution that works with our existing code (because he's a hero). :-)

[BlackJack]

[...] (because he's a hero). :-)

He really is! 
Hi Tkael.Glad to see you're still on this topic. But both, don't waste too much time on it.
 

[mIRCon]

Ohhh.. there it goes.. the legendary "culture invariant" strikes again, right?
I remember that happening in the past with HCS plugin as well so it had to be changed a bit to convert the values to {exp} accepted variant or use the alternative way(s) for calculation which will go along with various regional settings.. 

[BlackJack]

Hi, Gary.
You've made some changes in the new beta, as I saw in the change log.
But now even disabling the plugins doesn't help anymore.The issue is still present.

[Gary]

Is it breaking outright ('expression error') or is it producing a bad result?

The {EXP} parser uses the invariant culture, which means it accepts decimal values with a, '.' and not a ','.  That means that if you provide a comma, it's going to not like it (error).  The result of the parser component is always a string, so I have to convert the value coming out of the parser.  Prior to the update, that value was not parsed using the invariant culture but rather the thread's culture - which was being explicitly set by the plugin to be 'de-DE'.  Since, 'de-DE' expects, ',' to be the decimal separator and the value produced by the expression parser was using, '.', you would get 250 for your value (the '.' was interpreted as a thousands separator).  So, I changed the decimal parser to use the invariant culture to match the expression parser, which seems to work on my end when I change both the UI and thread culture to, 'de-DE' (changing the culture and NOT using the invariant culture when parsing produced the same results on my machine that you were experiencing).

I uploaded a version to the, 'unofficial' bin:  http://www.voiceattack.com/unofficial

In this build, I added two tokens:  {STATE_CULTURE} and {STATE_UICULTURE}.   Could you do me a favor and write each of those tokens out to the log and tell me what you see?

Also, are you still using the expression indicated at the top of the thread?

[BlackJack]

Hi, Gary.

Here are the expression results with v1.7.2.4:
(the comma is only for completeness)

first with the setting "automatic" in EDDI:
{EXP:50*0.5}: 25 (EXP with point)
{EXP:50*0,5}: Expression error (EXP with comma)
{STATE_UICULTURE}: de-DE
{STATE_CULTURE}: de-DE

and with German explicitly:
{EXP:50*0.5}: 250 (EXP with point)
{EXP:50*0,5}: Expression error (EXP with comma)
{STATE_UICULTURE}: de
{STATE_CULTURE}: de

now with v1.7.2.1:

automatic:
{EXP:50*0.5}: 250 (EXP with point)
{EXP:50*0,5}: Expression error (EXP with comma)
{STATE_UICULTURE}: STATE_UICULTURE
{STATE_CULTURE}: STATE_CULTURE

German:
{EXP:50*0.5}: 25 (EXP with point)
{EXP:50*0,5}: Expression error (EXP with comma)
{STATE_UICULTURE}: STATE_UICULTURE
{STATE_CULTURE}: STATE_CULTURE

The results just seem to be reversed.Is this as you expected?Apparently I did not tried the second option.

[Gary]

Just to show what I've got going on over here...   First without using the invariant on the returned decimal parse after expression evaluation (v1.7.2.4 and v1.7.2):

First just straight away (default culture):
{STATE_UICULTURE}: en-US
{STATE_CULTURE}: en-US
{EXP:50 * 0.5}: 25

Which is what I would expect to see, since en-US uses '.' for decimal separators.

Then, I set the culture on the thread to German:
{STATE_UICULTURE}: de-DE
{STATE_CULTURE}: de-DE
{EXP:50 * 0.5}: 250

Which is a problem, as the expression parser's output is ONLY with the invariant culture (no ability to change this), and since the returned parsed decimal is using the local culture, the, '.' generated by the expression parser gets thrown out and you end up with 250 instead of 25.0.


So, in v1.7.2.3, I instructed the decimal parser to use the invariant culture:

Default culture:
{STATE_UICULTURE}: en-US
{STATE_CULTURE}: en-US
{EXP:50 * 0.5}: 25

Still good.


German:
{STATE_UICULTURE}: de-DE
{STATE_CULTURE}: de-DE
{EXP:50 * 0.5}: 25

Which seems solved to me.  Pfeil says that it's totally broken for him that way, so I reverted to whatever was in v1.7.2 (which is now v1.7.2.4).


@BlackJack - I put v1.7.2.5 out in unofficial for you to try all of this again if you would: 
http://www.voiceattack.com/unofficial
What you provided was thorough, so, thank you.

@Pfeil - can you tell me what your {STATE_UICULTURE} and {STATE_CULTURE} tokens resolve to?  You can PM if you want.  Also, that unofficial build will not work for you as it's just as it was in v1.7.2.3.  If you want to try both v1.7.2.4 and v1.7.2.5 and see what comes back that would be cool too o_O

Thanks to all for your work on this.  I'm really kind of at a loss, actually.  It should be rather simple:  Put in numbers with '.', parse with, '.', return with, '.'.



Edit - Now I'm wondering if it's not just the parser(s) at this point, but rather the value that is ultimately rendered by a token, which is a string that has something to do with this.  Looking into that as well.

[Pfeil]

String used for testing:

Code: [Select]

|{EXP: 50 * 0.5|}: {EXP: 50 * 0.5} |{STATE_UICULTURE|}: {STATE_UICULTURE} |{STATE_CULTURE|}: {STATE_CULTURE} |{STATE_VA_VERSION|}: {STATE_VA_VERSION}

Working(25) in v1.7.2.4, broken(250) in in v1.7.2.5

PM sent with locale info.

[Gary]

I don't think you know what a bed is, robot man o_O

Thanks for the info.

I'm uploading v1.7.2.6 -  could you do what you did one more time?  I can't tell if I uploaded a version with German culture set just in the EXP thread (I probably didn't, but I want to make sure).

I will test out using your specific and differing cultures... in the morning... because *I* know what a bed is.  Also, my wife is going to murder me if I dont... stop.... typi...  aaaack

[Pfeil]

Broken(250) in v1.7.2.6, PM sent.

[Gary]

Gentlemen - please try out the latest unofficial build (v1.7.2.7) and see if this finally solves the problem:  http://www.voiceattack.com/unofficial

Hoping this wraps up this issue.  It was never the parsers fully per se, but rather that the parser functions were contained in a static function.  I was able to finally reproduce Pfeil's locale issue and then correct it by moving code out of the static function and just putting it inline.  Not going to go dig thru the internet to see how this all fits together, as it *seems* like it works now.

[Pfeil]

Broken(250) in v1.7.2.7, unfortunately.

[BlackJack]

I will test out using your specific and differing cultures... in the morning... because *I* know what a bed is.  Also, my wife is going to murder me if I dont... stop.... typi...  aaaack

Hi, Gary.
I know you are working on very difficult, world-changing, intercultural locale-stuff here, but you should always listen to you wife, when she ask you to come to bed. Really! Every thing else can wait. Hope you are well?!

Okay, it's time for a new round:

automatic (and plugins off)
{EXP:50*0.5}: 250 (EXP with point)
{EXP:50*0,5}: Expression error (EXP with comma)
{STATE_UICULTURE}: de-DE
{STATE_CULTURE}: de-DE

German
{EXP:50*0.5}: 25 (EXP with point)
{EXP:50*0,5}: Expression error (EXP with comma)
{STATE_UICULTURE}: de
{STATE_CULTURE}: de

this is the result of: (in v1.7.2.7)

Code: [Select]

Write [Blank] '{STATE_CULTURE|}: {STATE_CULTURE}' to log
Write [Blank] '{STATE_UICULTURE|}: {STATE_UICULTURE}' to log
Write [Blank] '{EXP:50*0,5|}: {EXP:50*0,5} (EXP with comma)' to log
Write [Blank] '{EXP:50*0.5|}: {EXP:50*0.5} (EXP with point)' to log

[Gary]

I'll keep it short since, unlike Pfeil, I am not a machine and am being summoned off to slumberland...
New build out there in unofficial (v1.7.2.: http://www.voiceattack.com/unofficial

I've reverted {EXP:expression} pretty much back to whatever it was at the beginning of all this.  I've added an {EXPDEC:expression} since {EXP} is overloaded to parse a variety of types.  {EXPDEC} is not overloaded to do everything... just decimals.  There's way too many things that are going on in {EXP}, and then you throw in a third party changing the culture via dll and it's really not going to work right (when the locale is forced on the thread, the result is somehow mixed - invariant in, system culture out.  Kinda maddening & there's not much I can do about it).  {EXPDEC} still expects the input to be in the invariant culture, which is fine since these profiles are shared across borders.  Would you guys mind looking at these and see if this does anything for us?

[Pfeil]

Output from v1.7.2.8:

{EXP: 2.5 * 0.5}: 1.25
{EXPDEC: 2.5 * 0.5}: 1.25
{DEC:dec}: 2,5
{EXPDEC: {DEC:dec} * 0.5}: Expression error

So "{EXPDEC:}" seems to work just like regular "{EXP:}" in practice, though also in that it can only take decimal constants, as the "{DEC:}" token output is culture-dependent.

[Gary]

What I'm going to do is change {EXPDEC} to {EXPDECINV:expression} and add {DECINV:} that outputs invariant.  This way, depending on your setup, you have choices and clear indication of what you're going to get.  The biggest sore point is that this stuff has been around so long ({EXP} and {DEC}) that changing them in any way could end up breaking profiles, so, just adding more and more tokens is better than ruining setups.

So, in summary, {EXP} will be reverted and will still break in the ways that were discovered.  Going forward, it would be advised to use {EXPDECINV} and {DECINV} to avoid culture issues (especially when sharing profiles).

[Gary]

I changed '{EXPDEC}' to '{EXPDECINV}' and added, '{DECINV}':  http://www.voiceattack.com/unofficial if you guys want to try it out (v1.7.2.9).

[Pfeil]

{EXP:} has not reverted fully, as

Code: [Select]

{EXP: 2 * 0.75}results in

1.5

In 1.7.2, this would be

1,5

instead.


Also,

Code: [Select]

{EXPDECINV: 2 * 0.75}outputs

1.50

which shows {EXPDECINV:} displays an additional digit after the decimal separator.


{DECINV:} seems to work as intended, outputting "." as the decimal separator.

[Gary]

Ok, cool.  Thanks for the update.

I put v1.7.2.10 out there in hopes this is settled

http://www.voiceattack.com/unofficial

[Pfeil]

v1.7.2.10 appears to be working as intended; {EXP:} outputs the locale-dependent separator, {EXPDECINV:} outputs "." without trailing zeros.