Sign in to follow this  
JeremyB

JS Dice Roller

Recommended Posts

Just wondering if someone can help me with this JS Dice roller. For 5d100 I'm getting numbers like 1400.

this.roll = function(amount, sides, mod = 0)
	{
	   
				var min = amount;
				var max = sides * amount;
				var value = 0;
				
				for (i = 0; i < amount; i++) 
				{
					value = value + (Math.floor(Math.random() * (max - min + 1)) + min);
				}
		 
		value = value + mod; 
				
		return value;		
	// end of roll
	}

Share this post


Link to post
Share on other sites

never programmed JS, but 'min' would be 5 and 'max' would be 500, I guess, and then you're doing 5 rolls between 5 and 500, by the looks of it.

 

max-min = 500 - 5 is about 500, 500 * random is about 250, 250 * 5 times (for-loop) is 1250 on average.

 

Edit: Either do one draw between min and max, or do 5 draws but don't add 'amount' into min and max then.

Edited by Alberth

Share this post


Link to post
Share on other sites

That's weird. This function returns a single random number in a range.

	this.range = function(min, max, mod = 0)
	{
	
		   return (Math.floor(Math.random() * (max - min + 1)) + min) + mod;
	
		// end of range
	}

I figured to get a die roller you would do the amount of sides times that range, while adjusting minimum to be the amount of dice.

Edited by JeremyB

Share this post


Link to post
Share on other sites

So this is right

	this.roll = function(amount, sides, mod = 0)
	{
	   
				var min = amount;
				var max = sides;
				var value = 0;
				
				for (i = 0; i <= amount; i++) 
				{
					value = value + (Math.floor(Math.random() * (max - min + 1)) + min);
				}

		value = value + mod; 
				
		return value;		
	// end of roll
	}

Share this post


Link to post
Share on other sites

The 'min' and 'max' is about the value of a single roll.

They are not related to how many rolls you do.

 

It's like your money coins. They have a (sort of) random value, but their value is unrelated to how many coins you have. (Would be great though, oh man, I would be so much collecting coins then smile.png ).

 

"amount" is the number of rolls, which thus should not be assigned to "min". I guess it's 1 or some other lowest value of your die.

 

There is a simple way to test these things. Instead of using "Math.random()", try 0 (ie lowest value for all rolls) and 1 0.99999 (ie highest value for all rolls).

Then try a few cases, like 1d6, 5d100,  and 100d1.

 

You can even run the computation manually, just compute the values yourself. You can also use a debugger or add "print" statements to check how much you add each iteration.

If you do this, you will quickly see what value is wrong.

 

EDIT: You need to use 0.999999  instead of 1  for highest value of a roll  see also http://www.w3schools.com/jsref/jsref_random.asp

 

Hmm, I continue editing this post :)

To show you what I mean by the number replacement, see the code below

def unlucky(amount, sides, mod = 0):
    min = amount;
    max = sides;
    value = 0;

    for i in range(amount + 1): # for (i = 0; i <= amount; i++)
        value = value + int(0.0 * (max - min + 1)) + min;

    value = value + mod;

    return value;

def lucky(amount, sides, mod = 0):
    min = amount;
    max = sides;
    value = 0;

    for i in range(amount + 1): # for (i = 0; i <= amount; i++)
        value = value + int(0.99999 * (max - min + 1)) + min;

    value = value + mod;

    return value;

print("1d6   min={}, max={}".format(unlucky(1, 6), lucky(1, 6)))
print("2d6   min={}, max={}".format(unlucky(2, 6), lucky(2, 6)))
print("100d1 min={}, max={}".format(unlucky(100, 1), lucky(100, 1)))

I copied your roll_dice function, into 'unlucky', converted to Python (I don't have Javascript running) and replaced "Math.random()" by "0.0"

I did the same for "lucky", but used 0.999999 instead of 0.0.

 

Then I added a few test cases, to check results.

 

 

The problem with random is that you don't know what it returns, which makes it very difficult to check whether the code is correct. By the above change, you have fixed return values (namely "lowest possible" and "highest possible"), which makes checking much easier.

 

Once you are satisfied with the results, throw away one of the functions, and replace the constant again with Math.random(), and you're done.

 

 

As for tests: My results are

1d6   min=2, max=12
2d6   min=6, max=18
100d1 min=10100, max=303

so it seems it's still wrong.

 

The first result suggests you are throwing one die to many. (And indeed, 0 to .. <= 1 is 2 rolls rather than 1, I see now.)

Edited by Alberth

Share this post


Link to post
Share on other sites

Why would you use 0.999999? I don't think that's right.
That link says "Return a random number between 0 (inclusive) and 1 (exclusive):"

 

The "exclusive" means 1.0 is NOT included, so technically you need 1-(smallest representable value such that you end up just below 1).

I would say 0.000001 is a pretty good approximation for that small value, until you have a die with about 1,000,000 sides.

Share this post


Link to post
Share on other sites

Amount a min should be the same thing. There is 1 side for each dice. So if the first argument is 2 then min is 2, one for each side, and there are 2 rolls taking place. It's still a range between min max.. it's just min is also amount...

 

Right? Them loop for amount of rolls as well.. and add the mod.

 

I can't see where the code below would be wrong.

this.roll = function(amount, sides, mod = 0)
	{
	   
				var min = amount;
				var max = sides;
				var value = 0;
				
				for (i = 0; i <= amount; i++) 
				{
					value = value + (Math.floor(Math.random() * (max - min + 1)) + min);
				}

		value = value + mod; 
				
		return value;		
	// end of roll
	}

You're right though. It's giving constant 2 for 1d1.

Share this post


Link to post
Share on other sites

The correction is

for (i = 0; i <= amount - 1; i++) 
{
  value = value + (Math.floor(Math.random() * (max - min + 1)) + min)
}

Adding - 1 to amount in the loop condition.

Edited by JeremyB

Share this post


Link to post
Share on other sites

this.roll = function(amount, sides, mod = 0) {
    var min = amount;
    var max = sides * amount;
    var value = 0;

    for (i = 0; i <= amount; i++)
        value = value + (Math.floor(Math.random() * (max - min + 1)) + min);
}


This doesn't make any sense. To roll five 6-sided dice, you sum 5 numbers between 5 and 31? It should sum 5 numbers between 1 and 6. Therefore, 'min' should always be 1, and 'max' should always be 6:
 
this.roll = function(amount, sides, mod) {
	var sum = 0;
	for ( var idx = 0; idx < amount; ++idx )
		sum += Math.floor(Math.random()*sides) + 1;

	return sum + (mod||0);
};

And here's a test:
> Array.apply(null,{length: 100}).map(function(){return roll(2,6);})

[7, 6, 5, 5, 10, 8, 9, 6, 3, 8, 3, 11, 3, 6, 5, 10, 8, 10, 9, 5, 9, 7, 7, 7, 4, 7, 6, 6, 5, 8, 2, 6, 2, 7, 5, 8, 10, 10, 4, 9, 8, 7, 4, 4, 5, 9, 9, 10, 5, 6, 5, 7, 8, 11, 7, 10, 10, 12, 6, 6, 8, 4, 8, 8, 8, 9, 7, 5, 7, 9, 5, 7, 11, 8, 9, 3, 11, 9, 9, 11, 5, 8, 7, 6, 4, 7, 11, 3, 9, 3, 9, 8, 4, 8, 7, 9, 3, 6, 6, 5]

Additionally Javascript does not have default parameters. You'll either need to do this by hand, or since undefined is falsey, you can use the logical-or operator to select a zero instead:
var f = function(a,b,c) {
	if ( c === undefined )
		c = 0;
	// or:
	c = c || 0;
}
Edited by fastcall22

Share this post


Link to post
Share on other sites
Amount a min should be the same thing. There is 1 side for each dice. So if the first argument is 2 then min is 2, one for each side, and there are 2 rolls taking place. It's still a range between min max.. it's just min is also amount...

You are correct in thinking that the minimum result of N rolls is N, since each die roll has minimum 1, and you roll N times.

 

The point is that "min" is the minimum value of a single die roll. The "for" loop makes that happen N times, so you add N times "min".

 

"Math.floor(Math.random() * (max - min + 1)) + min" is the value added for a single die roll.

For Math.random being 0 (the lowest possible value), you get "Math.floor(0 * (max - min + 1)) + min" = Math.floor(min)" = "min".

 

So if you always draw the lowest possible value (random is always 0), the loop becomes

for (i = 0; i <= amount - 1; i++) 
{
    value = value + min
}

which is a difficult way to say "value = amount * min"  (yes, your -1 correction is correct).

So for "min=1" (the minimum value of a die), it says "value = amount * 1", which is "value = min amount", which is what you expect (throwing 2 times with a die is 2 as minimum value).

 

For "min=amount", the lowest possible value is "value = amount * amount", so a 2 roll has minimum 4, a 3 roll minimum 9, 4 roll 16 5 roll 25, 6 roll 36, etc. Clearly not what you want.

 

The looping part handles accumulating the minimum 1 value to obtain "amount" after N rolls already, you don't want a single die roll to do that as well.

 

 

As for the correction, another solution is to remove the "=" as in " for (i = 0; i < amount; i++)"  both are good, it's just a matter of preference.

Edited by Alberth

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this