jpcsupplies / Economy_mod

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

BUG! pay command crashes game if only 1 parameter used #14

Closed jpcsupplies closed 9 years ago

jpcsupplies commented 9 years ago

Started working on the pay command, have a test bank file entry for "bob" /pay = refer to help error /pay nobody 10 = unable to find user /pay bob 10 = debug message /pay bob 1000 = cant afford that debug message

BUT /pay fred seems to crash game with an array error (eh!?)

too tired to fix now, nearly face planted the keyboard but the "else" on the "if (split.Length >= 2)" which should respond " invalid number of parameters" seems to fail to trigger is best guess.. or somehow its trying to access [2] in array somehow but cant see how. lazy fix might be to assign nul as 0 then it treats it like they did /pay fred 0 which should respond with a unknown person error

to someone with a good nights sleep should be something obvious.. im off to bed here is the error message

2015-09-05 00:49:43.190 - Thread: 1 -> Exception occured: System.IndexOutOfRangeException: Index was outside the bounds of the array. at Economy.EconomyScript.processMessage(String messageText) at Economy.EconomyScript.gotMessage(String messageText, Boolean& sendToOthers) at Sandbox.Game.Gui.MyGuiScreenChat.HandleInput(Boolean receivedFocusInThisUpdate) at Sandbox.Graphics.GUI.MyScreenManager.HandleInput() at Sandbox.Graphics.GUI.MyDX9Gui.HandleInput() at Sandbox.MySandboxGame.Update() at Sandbox.Engine.Platform.Game.UpdateInternal() at Sandbox.Engine.Platform.FixedRenderLoop.<>cDisplayClass2.b1() at Sandbox.Engine.Platform.GenericRenderLoop.Run(VoidAction tickCallback) at Sandbox.Engine.Platform.FixedRenderLoop.Run(VoidAction tickCallback) at Sandbox.MySandboxGame.Run(Boolean customRenderLoop, Action disposeSplashScreen) at SpaceEngineers.MyProgram.RunInternal(String[] args) at SpaceEngineers.MyProgram.Main(String[] args) 2015-09-05 00:49:43.192 - Thread: 1 -> Hiding window 2015-09-05 00:49:43.607 - Thread: 1 -> Hiding window done 2015-09-05 00:49:43.607 - Thread: 1 -> Showing message

SaltPepp commented 9 years ago

I'm still up, I've been working on my private repo. I'll be up probably most of the night/morning

Sent from my Windows Phone


From: jpcsuppliesmailto:notifications@github.com Sent: ‎5/‎09/‎2015 1:16 AM To: jpcsupplies/Economy_modmailto:Economy_mod@noreply.github.com Subject: [Economy_mod] BUG! pay command crashes game if only 1 parameter used (#14)

Started working on the pay command, have a test bank file entry for "bob" /pay = refer to help error /pay nobody 10 = unable to find user /pay bob 10 = debug message /pay bob 1000 = cant afford that debug message

BUT /pay fred seems to crash game with an array error (eh!?)

too tired to fix now, nearly face planted the keyboard but the "else" on the "if (split.Length >= 2)" which should respond " invalid number of parameters" seems to fail to trigger is best guess.. or somehow its trying to access [2] in array somehow but cant see how. lazy fix might be to assign nul as 0 then it treats it like they did /pay fred 0 which should respond with a unknown person error

to someone with a good nights sleep should be something obvious.. im off to bed here is the error message

2015-09-05 00:49:43.190 - Thread: 1 -> Exception occured: System.IndexOutOfRangeException: Index was outside the bounds of the array. at Economy.EconomyScript.processMessage(String messageText) at Economy.EconomyScript.gotMessage(String messageText, Boolean& sendToOthers) at Sandbox.Game.Gui.MyGuiScreenChat.HandleInput(Boolean receivedFocusInThisUpdate) at Sandbox.Graphics.GUI.MyScreenManager.HandleInput() at Sandbox.Graphics.GUI.MyDX9Gui.HandleInput() at Sandbox.MySandboxGame.Update() at Sandbox.Engine.Platform.Game.UpdateInternal() at Sandbox.Engine.Platform.FixedRenderLoop.<>cDisplayClass2.b1() at Sandbox.Engine.Platform.GenericRenderLoop.Run(VoidAction tickCallback) at Sandbox.Engine.Platform.FixedRenderLoop.Run(VoidAction tickCallback) at Sandbox.MySandboxGame.Run(Boolean customRenderLoop, Action disposeSplashScreen) at SpaceEngineers.MyProgram.RunInternal(String[] args) at SpaceEngineers.MyProgram.Main(String[] args) 2015-09-05 00:49:43.192 - Thread: 1 -> Hiding window 2015-09-05 00:49:43.607 - Thread: 1 -> Hiding window done 2015-09-05 00:49:43.607 - Thread: 1 -> Showing message


Reply to this email directly or view it on GitHub: https://github.com/jpcsupplies/Economy_mod/issues/14

midspace commented 9 years ago

Well there is a level of complexity when it comes to interpreting what a player enters, and you may want to reconsider how you are doing it.

Currently you are using a tried and true approach to split strings from a command line. string[] split = messageText.Split(new Char[] { ' ' }, StringSplitOptions.RemoveEmptyEntries);

And then you need to reach each string from the array to find out if it contains what you need. if(split[0] == "/pay") ... string name = split[1]; decimal amount = Convert.ToDecimal(split[2]);

The problem really begin when you have complex names. take my player name "Screaming Angels". It has two words. A space in the middle. using String.Split will unnecessary split it up, leave you unsure how many parts of the array are the player name.

What if a player has spaces front and back? And then what if they use special character native to their language, which can be done on their keyboard (keyboards in Europe). ie., "Scrëamíng Anglës"

Using a Regular Expression, we could cover all these, except for the last part. If you have ever looked at my Admin script, you notice we use a lot of Regular Expressions, as it make it much easier to A, first check if the command entered matches the profile. B, extract the parameters without having to parse it manually.

If you haven't come across Regular Expressions before, they aren't a .Net only concept. You'll find them in use in many programming languages.

SaltPepp commented 9 years ago

Is it possible to check to see if the second parameter is a int or double, if it is not then combine split[1] and split[2] together to form the player name and use split[3] as the amount.

SaltPepp commented 9 years ago

I've done type checking in the past myself in C#. There is TypeOf or you could use a try and catch statement.

jpcsupplies commented 9 years ago

Good point on the nickname.. ive always created nicks with underscores or single words, didnt even occur to me players in steam may use spaces, at my (lacking) level of skill doing a check like saltpepp suggests would have been the direction i would most likely try. I may well have hit regular expressions before, i probably learnt about them years ago during my software development stream IT level 4 course at tafe, tho all this time later all the particular terminology is beginning to slip my mind!

jpcsupplies commented 9 years ago

.. ok i googled a vb6 era doc on regular expression.. ah yes, passing parameters to functions, which return a value in such a way as to allow a function to be used in the same logical context as a string.. usually you stuck them on uuhm what were they called, public code modules .. basically a stand alone bas file not linked to any frmform i used to use them for stuff like on error handling to return human readable interpretations of error codes. It is what i was attempting to do with the public void load () to populate the arrays with the bank data file, but couldn't work out how to make it work right in c# (you will probably just tell me all I needed was to call it public function load (filename as string) or something.. that terminates with a "return answer;" facepalm)

jpcsupplies commented 9 years ago

Might also be an idea once the pay command.. actually er pays .. for us to move all the doubled up code that checks steam ids and read/writes/creates our balance records into a similar function (or regular expression) so it only needs to be in one place, since if we start adding a bunch of balance effecting commands it will get used quite a lot.

eg if getbal(player)>= payment then pay them

calling something like public function getbal(name as string) check if name exists in file; if true return their balance else create a new default balance record and return the default balance on error 'file not found', create file; return 0

midspace commented 9 years ago

Regular Expression sample command.

If you want to use a configurable command prefix character, you can start with the following.

string commandPrefix = "/";
string payPattern = @"(?<command>" + Regex.Escape(commandPrefix) + @"(pay)|(payuser))\s+(?:(?:""(?<user>[^""]|.*?)"")|(?<user>[^\s]*))\s+(?<value>[+-]?((\d+(\.\d*)?)|(\.\d+)))\s*$";

Otherwise the following example to define the pay command pattern.

string payPattern = @"(?<command>/(pay)|(payuser))\s+(?:(?:""(?<user>[^""]|.*?)"")|(?<user>[^\s]*))\s+(?<value>[+-]?((\d+(\.\d*)?)|(\.\d+)))\s*$";

My sample commands...

messageText = "/pay midspace -123.123";
messageText = "/pay \"Screaming Angels\" 45663.23443";

The core code.

match = Regex.Match(messageText, payPattern, RegexOptions.IgnoreCase);
if (match.Success)
{
    var user = match.Groups["user"].Value;
    var value = Convert.ToDecimal(match.Groups["value"].Value, CultureInfo.InvariantCulture);

    //if (!player.IsAdmin)
    value = Math.Abs(value);
}

One thing I just remembered, some of those European countries use "," for their decimal place, and "." for their thousand separator. So instead of "1,234,567.887766", they have "1.234.567,887766"

by using Convert.ToDecimal(match.Groups["value"].Value, CultureInfo.InvariantCulture); it will enforce that they enter "." for the decimal place separator, and not crash. Otherwise we will have to cater for it.

The way to do that, would be like this.

// Setting sample to French UI.
System.Threading.Thread.CurrentThread.CurrentUICulture = CultureInfo.GetCultureInfoByIetfLanguageTag("fr");

string commandPrefix = "/";
var decSeperator = Regex.Escape(CultureInfo.CurrentUICulture.NumberFormat.CurrencyDecimalSeparator);
var positiveSign = Regex.Escape(CultureInfo.CurrentUICulture.NumberFormat.PositiveSign);
var negativeSign = Regex.Escape(CultureInfo.CurrentUICulture.NumberFormat.NegativeSign);
string payPattern = @"(?<command>" + Regex.Escape(commandPrefix) + @"(pay)|(payuser))\s+(?:(?:""(?<user>[^""]|.*?)"")|(?<user>[^\s]*))\s+(?<value>[" + positiveSign + negativeSign + @"]?((\d+(" + decSeperator + @"\d*)?)|(" + decSeperator + @"\d+)))\s*$";

messageText = "/pay midspace -123,123";

match = Regex.Match(messageText, payPattern, RegexOptions.IgnoreCase);
if (match.Success)
{
    var user = match.Groups["user"].Value;
    var value = Convert.ToDecimal(match.Groups["value"].Value, CultureInfo.CurrentUICulture);

    //if (!player.IsAdmin)
    value = Math.Abs(value);
}
jpcsupplies commented 9 years ago

oh wow git hub auto closed this when i fixed the number of parms issue.. but yes, pattern matching support for long nicknames good idea..

The typical cmdline convention for spaced parsing is usually double quote ie "Screaming Angels" there should be a a c style escape code for quotes like \q or something ill look it up.

For international support, cant we just strip ',' out? although with decimal places we would need to be clear in the documentation to use whole numbers with "period" as the decimal point. Unless we have an english professor using the mod and using , at each 000's when he moves money around.. i imagine it would be rare for a player to comma it. I only use commas when hand writing numbers myself most of the time.

jpcsupplies commented 9 years ago

. or not.. does C# use the perl/pascal /actual/ identifier style ' ' as well? eg if (input = '"') then do stuff { print 'hello "world" ....'; } ie ( ' " ') etc

midspace commented 9 years ago

the ' is a char limiter. the " is a string limiter.

char alpha = 'a'; string bravo = "aaaaaaa";

C# uses escape characters. so string charlie = "\""; there is a special notation in .net using @ which allows a double quote to be used instead of the back-slash escape character. string delta = @"""";

https://msdn.microsoft.com/en-us/library/aa691090(v=vs.71).aspx

jpcsupplies commented 9 years ago

ok so our logic here is check messageText for " if it exists, we process the split against " to capture the nickname, and do some voodoo to catch the amount and reason text too. or we simply strip spaces from names, and the " when we detect " (or substitute underscore)

if it doesnt exist use the logic it has now which splits on space

jpcsupplies commented 9 years ago

wouldnt char alpha ='"'; or string beta="\""; give us essentially the same scan code ?

pity c# hasnt got a tidy equivalent of the VB chr() nearest i can find is here: http://stackoverflow.com/questions/721201/whats-the-equivalent-of-vbs-asc-and-chr-functions-in-c Which offers some ways to do it as well, but is mostly people arguing with each other lol.

midspace commented 9 years ago

It really comes down to a few details.

What you are using it for. And how efficient do you need it?

using char to build a string is inefficient. If you are doing string comparisons, you need a string, not a char.

The following are all equal. var xxx = "ab!cd"; var xxx = "ab\x0021cd"; var xxx = "ab\u0021cd"; var xxx = String.Format("ab{0}cd", (char)33);

jpcsupplies commented 9 years ago

all we really need to do is use it for an if exist in string split by " assign the result (the nickname) into the nickname we want to pay or check balance on.. then process the pay or bal like we do normally just with the spaced nickname

probably need some sort of hash check comparasin (or whatever its called) type c# voodoo tho otherwise anything i come up with will be long winded..

not urgent I suppose main issue now is the mod being happy on a server then update the workshop and see what sort of feedback we get

Tobys reference code had a nice full list of ores and materials too, have to start thinking about the RRP price table once we hit our current milestone