Question

Its roll the dice application. I want to sum up the dice results and present them to the user. Currently the dice image will change after I click the button "Click to roll the die".

However, When I rolled the dice 1, result will not be add (+0) and when i rolled dice 2, the result will only (+1). I have no idea what's wrong with my code:

public partial class PigForm : Form
{
    Image[] diceImages;
    int[] dice;
    Random roll;

    private void rollDieBotton_Click(object sender, EventArgs e)
    {
        RollDice();
    }

    private void RollDice()
    {
        for (int i = 0; i < dice.Length; i++)
        {
            var currentRoll = roll.Next(0, 6);
            dice[i] += currentRoll;
            dicePictureBox.Image = diceImages[currentRoll];
            playersTotal.Text = String.Format("{0}", dice[i]);
        }
    }

    private void PigForm_Load(object sender, EventArgs e)
    {
        diceImages = new Image[6];
        diceImages[0] = Properties.Resources.Alea_1;
        diceImages[1] = Properties.Resources.Alea_2;
        diceImages[2] = Properties.Resources.Alea_3;
        diceImages[3] = Properties.Resources.Alea_4;
        diceImages[4] = Properties.Resources.Alea_5;
        diceImages[5] = Properties.Resources.Alea_6;

        dice = new int[1] { 0 };
        roll = new Random();
    }
}
Was it helpful?

Solution 3

  • First of all, having an integer array for your dice is pointless because you need only one number which also means that you do not need a loop for it because it will never iterate for a second time.
  • Secondly your random goes from zero to five included and you want it to be from 1 to 6.

Your code with a few edits:

var currentRoll = roll.Next(1, 7); 
dice = currentRoll; // there should not be += operator because the result of the next roll will be absurd
dicePictureBox.Image = diceImages[currentRoll - 1]; // -1 because your array is zero-based which means that it starts from 0

That is how the random class really works. The initial value which in your case is 1, is included, but the one in the ending is not, so what you need is 1,7 because it will return a number between 1 and 6 included.

OTHER TIPS

A few remarks on your code:

  • Why use an array if it will always contain one integer? This also makes the for loop quite useless. Use a plain integer and remove the loop.
  • The Next() method of the Random class has two parameters. The first is inclusive lower-bound, the second one is exclusive upper-bound. Meaning in your case that the 0 will be a possible number, while 6 will never occur. (MSDN page: Random.Next Method (Int32, Int32))

Here's a slight modification of your code:

public partial class PigForm : Form
{
    Image[] diceImages;
    int dice;
    Random roll;

    private void rollDieBotton_Click(object sender, EventArgs e)
    {
        RollDice();
    }

    private void RollDice()
    {
        var currentRoll = roll.Next(1, 7);
        dice += currentRoll;
        dicePictureBox.Image = diceImages[currentRoll-1];
        playersTotal.Text = String.Format("{0}", dice);
    }

    private void PigForm_Load(object sender, EventArgs e)
    {
        diceImages = new Image[6];
        diceImages[0] = Properties.Resources.Alea_1;
        diceImages[1] = Properties.Resources.Alea_2;
        diceImages[2] = Properties.Resources.Alea_3;
        diceImages[3] = Properties.Resources.Alea_4;
        diceImages[4] = Properties.Resources.Alea_5;
        diceImages[5] = Properties.Resources.Alea_6;

        dice = 0;
        roll = new Random();
    }
}
var currentRoll = roll.Next(0, 6)

This will generate a random number from 0 to 5, inclusive. You probably want to generate from 1 to 6:

var currentRoll = roll.Next(1, 7)

Reference: Random.Next Method (Int32, Int32)

Edit:

dicePictureBox.Image = diceImages[currentRoll - 1]

You are allowing the currentRoll variable be anything between [0, 6]. That includes 0 but excludes 6. You should probably change to var currentRoll = roll.Next(1, 7);

Edit for comment: Then for accessing your array values(which is zero indexed) you should subtract one from your roll result.

As others have pointed out, Random.Next(a, b) generates a random number between a inclusive and b exclusive.

While it would be easy to just do

var currentRoll = roll.Next(1, 7);

that would break the array access line you have two lines later.

Instead, your best bet would be to modify the addition line, to do

dice[i] += currentRoll + 1;

Take a look at the documentation of the Random.Next(int, int) method:

http://msdn.microsoft.com/en-us/library/2dx6wyd4%28v=vs.110%29.aspx

There you will find that the lower bound is inclusive and the upper bound is exclusive. Hence Next(0,6) means that you get a 0, 1, 2, 3, 4 or 5.

I did not exactly understand what your problem is but I did spot something that looks wrong.

Try changing:

dice[i] += currentRoll;

With:

dice[i] += currentRoll+1;
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top