jpcsupplies / Economy_mod

Basic Economy System for Space Engineers
13 stars 12 forks source link

Type checking needed on parameters #21

Closed SaltPepp closed 9 years ago

SaltPepp commented 9 years ago

The game will crash if you run: /bal -4483 playername You can either use try and catch statement or use typeOf

jpcsupplies commented 9 years ago

Not sure if the type checking is the issue on this crash; parameter (1) (ie -4483) will be treated as a string "-4483" which is then compared to the list of player names in our bank ledger file if you are an admin..

The third parameter [2] however may be causing an issue.. will do some further testing to confirm.

Whatever we need to fix these C2D's we need to nail them all before we push a test release to workshop. So that makes this a milestone.

I think there is instances where we directly convert chat output to a decimal - it might be an idea to process these as a string before hand and then type check it before converting to decimal to prevent the error conditions being able to occur.

Although we do need some sort of on error routine just so our mistakes dont kick players out of their game too. But ideally we still must test it enough all the C2D conditions are not possible to occur.

jpcsupplies commented 9 years ago

Bal command is fine i cant get it to fail.. but the pay command fails if i go /pay bob ooo ill have to add a check there

jpcsupplies commented 9 years ago

hmm bit of a spanner here - if( split[2].GetType() == typeof(int)) { reply = "we got an integer"; } else if (split[2].GetType() == typeof(string)) { reply = "we got a string"; } else { reply = "we got something else"; } always evaluates as string

using api quirks that ignore string elements test = "0" + split[2]; convert.todecimal (test); also fails..

converting to a string first is also pointless.

Looks like the only way to make this work is to use quirks to append a 0 to the start, then run a find/replace to write all numerical digits to a string, then todecimal that string.. that avoids the conditions that make the pay fail, but puts it outside my current c# skillset. Should be a simpler way, otherwise things like midspaces /tp command would crash the client every time someone used coordinates lol
but ill have to keep skimming reference docs to find said "easy way" try and catch maybe but i dont like using error handing routines for something we should cater for to begin with :/ nor am i familiar with try and catch to begin with lol

midspace commented 9 years ago

split[2].GetType() will always be string, because you are testing the Data Type of the variable, not the content.

You practically have to test each value.

decimal testValue;
if ((Decimal.TryParse(split[1], NumberStyles.Number, CultureInfo.InvariantCulture, out testValue))
{
}
else if ((Decimal.TryParse(split[2], NumberStyles.Number, CultureInfo.InvariantCulture, out testValue))
{
}
jpcsupplies commented 9 years ago

Hmmm, (googles tryparse) something like this then? Two scenarios - a: /pay bob 1 b: /pay bob xyz

decimal x = 0; if (decimal.TryParse(split[2], out x)) { reply = "we got a decimal of " + x; //x will equal 1 in scanario "a" } else { reply = "we probably got a string then"; //x will equal 0 in scenario "b" }

jpcsupplies commented 9 years ago

Looks like there is a few examples of a..z && A..Z "for" loops to extract numbers too.. strike me as inefficent tho.. make more sence to go the other way 0..9 = 10 loops per char VS 48 or more; same end result.. and wont exception on non alpha characters dont need that level tho, since if they type garbage an error indicating to learn to type and allowing them to remedy it themself, rather than to nanny state it and try to pull some sence out of their gibberish would be the safer option;

ill try something to that extent when i get home too..

midspace commented 9 years ago

This is why I prefer Regular Expressions. It will extract exactly what you want by the pattern, without having to loop through and test every string in the split array.