0 votes
by (160 points)

Hello everyone, I am currently working on an item shop interface, and noticed that it does have a fatal flaw.

The passage generates a list of links, each triggering the buy menu, allowing the player to enter the amount of items to be bought into a textbox. As long as the player actually types in a number, that works as intended; however, there is no obvious way of preventing the player from trying to buy "zrgdfghjo" amount of potions, which in turn breaks both the player's money and the remaining number of potions in the shop's inventory. Now I'd be perfectly happy if there was a way to limit the damage - say, a check to see if a variable is not actually an integer, and overwriting it with 0 in that case, not buying anything in that particular instance. Is there any solution to this, or should I remove the textbox and replace it with something a bit safer?

 

Thank you for your time.

 

<<textbox "_buyThisMany" _buyThisMany>> 
<<button "Buy">>
	<<if (ndef Math.trunc(_buyThisMany))>> /*this is the part in question*/
		<<set _buyThisMany = 0>>
	<<else>>
		<<set _buyThisMany = Math.clamp(Math.clamp(_buyThisMany, 0, Math.trunc($player.money / $itemList[_activeShop.stock[_k].index].value)), 0, _activeShop.stock[_k].amount)>> /*works flawlessly from here on out*/
	<</if>>
	<<set _buyThis = {
		index: _activeShop.stock[_k].index,
		amount: _buyThisMany
	}>> /*define the object we're buying*/
	/*do the buying magic (code irrelevant to problem)*/
<</button>> (<<print _activeShop.stock[_i].amount>> in stock)

 

2 Answers

+2 votes
by (68.6k points)
selected by
 
Best answer

The Math.trunc() method does not return undefined, so that's one of your problems.  Also, it's probably not a good idea to truncate here anyway, since that only addresses one error (floats) and does it silently, which is probably not a good idea.

I'd suggest something like the following:

<<textbox "_buyThisMany" _buyThisMany>> <span id="buy-textbox-error"></span>
<<button "Buy">>
	<<replace "#buy-textbox-error">><</replace>>

	<<set _buyThisMany to Number(_buyThisMany)>>
	<<if !Number.isInteger(_buyThisMany)>>
		/%
			I suggest generating an error message here, though
			you could set `buyThisMany` to `0` if that's your
			want.
		%/
		<<replace "#buy-textbox-error">>Please enter a whole number.<</replace>>
	<<else>>
		<<set _buyThisMany = Math.clamp(Math.clamp(_buyThisMany, 0, Math.trunc($player.money / $itemList[_activeShop.stock[_k].index].value)), 0, _activeShop.stock[_k].amount)>> /*works flawlessly from here on out*/
		<<set _buyThis = {
			index  : _activeShop.stock[_k].index,
			amount : _buyThisMany
		}>> /* define the object we're buying */
		/* do the buying magic (code irrelevant to problem) */
	<</if>>
<</button>> (<<print _activeShop.stock[_i].amount>> in stock)

 

by (160 points)
Works like a charm. Thank you so much, you have no idea just how much help that is!
0 votes
by (63.1k points)

 You can try to turn the input into a number using Number(), which I've recently learned is better for this than parseInt(), and then use isNaN() to see if the result comes back as the value NaN (not a number).

I think it's best to put a span under your textbox to correct the player when they enter information incorrectly; a player trying to type out 'one' may not realize their mistake if it just silently fails, and then get frustrated or believe they've found a bug when they don't have the item later.

<<textbox "_buyThisMany" _buyThisMany>>
@@#textbox-reply;@@ /% for error messages %/
<<button "Buy">>
        <<set _buyThisMany to Math.trunc(Number(_buyThisMany))>>
	<<if isNaN(_buyThisMany)>>
         /% 
           I'm generating an error message here; 
           if you want to set it to 0 instead, you can do that.
         %/
        <<replace '#textbox-reply'>>\
            Please enter a number.
        <</replace>>
	<<else>>
		<<set _buyThisMany = Math.clamp(Math.clamp(_buyThisMany, 0, Math.trunc($player.money / $itemList[_activeShop.stock[_k].index].value)), 0, _activeShop.stock[_k].amount)>> /*works flawlessly from here on out*/
	<</if>>
	<<set _buyThis = {
		index: _activeShop.stock[_k].index,
		amount: _buyThisMany
	}>> /*define the object we're buying*/
	/*do the buying magic (code irrelevant to problem)*/
<</button>> (<<print _activeShop.stock[_i].amount>> in stock)

See this thread on the old forums for more ideas.

by (68.6k points)
edited by

Your error condition block doesn't terminate the <<button>> (it can't), so your code (as I write this) will proceed to attempt to <<set _buyThis = ...>> which you really don't want to do.  You should move the code after the <</if>> to just before it (i.e. at the end of the <<else>>).

Also.  A bit of FYI:

        <<set _buyThisMany to Math.trunc(Number(_buyThisMany))>>
	<<if isNaN(_buyThisMany)>>

 

  1. You don't really need the call to Number() there, as Math.trunc() will do the number type conversion for you (that said, being explicit isn't a bad thing).
  2. You're probably better off avoiding the the global number functions (i.e. isNaN(), isFinite(), parseInt(), parseFloat()) in favor of the Number object methods of the same names (i.e. Number.isNaN(), Number.isFinite(), Number.parseInt(), Number.parseFloat()) as the globals have some odd behaviors with regards to coercion.
  3. You're better off using Number.isInteger() here rather than isNaN() (or even Number.isNaN()) there since an integer is what's needed and, while it's highly unlikely, a player could enter something which would convert to Infinity (which would pass the NaN check, but obviously isn't useful here).
by (63.1k points)
Whoops. Yeah I forgot to move the <</if>> when i changed over to an error message approach. Thanks for the catch and the other advice.
...