Minesweeper Game Problems

[mattwaab]

Hey, i am working on a project in which i am making a minesweeper game. The game part was written by my prof. all i had to do was make the class and header to run the background of the game. I have done so successfully, but i am having trouble with one part. When i try to place the mines into the board, it refuses to put the correct number of mines in. I dont know why. ere is the entire function that places the mines into the board:

void Minesweeper::init ()
{
    srand((unsigned)time(0));  
    int minesLeft = 0, cellsLeft = 0, randNum = 0;
    
    // sets minesLeft to the data member numMines
    minesLeft = numMines;      
    
    //sets cellsLeft to the number of rows and cols in the board
    //nrows and ncols are both data members
    cellsLeft = nRows * nCols;
  
    while (minesLeft > 0)
    {
        
         for (int r = 0; r < nRows; r++)//rows
            {
                for (int c = 0; c < nCols; c++)//cols
                {
                    //places mines with a probibility of numMines / cellsLeft
                    randNum = (rand()%cellsLeft)+1;
                   
                    if (randNum <= minesLeft)
                    {
                        mine [r][c] = true;
                        minesLeft --;
                    }
                    if (randNum > minesLeft)
                    {
                        mine [r][c] = false;
                    }
                    cellsLeft--;
                    mark [r][c] = NO_MARK;
                }
            }
    }       
}//Minesweeper::init

the problem that i have is that it never places the correct number of mines. here are some examples of what it does place.

No. of rows = 10
No. of columns = 10
Mo. of mines = 20

output of mine placement
_ _ _ _ _ M M _ _ M 
_ _ M _ M _ _ _ _ M 
_ _ _ _ _ _ _ _ _ M 
_ _ _ _ _ _ _ M _ _ 
_ _ M _ _ _ _ _ _ _ 
_ _ _ _ M M _ _ M _ 
_ M _ _ M M _ _ _ _ 
_ _ _ _ _ _ _ _ _ _ 
M _ _ _ _ _ _ _ _ _ 
_ _ _ _ _ _ _ _ _ _ 


~~~~~~~~~~~~~~~~
No. of rows = 10
No. of columns = 10
Mo. of mines = 20

output of mine placement
_ _ _ _ _ _ _ _ M _ 
M M _ _ _ M _ _ M _ 
_ M _ M _ _ _ _ _ _ 
_ M _ _ M _ M _ _ _ 
_ _ _ M _ _ _ _ _ _ 
_ _ _ _ _ _ _ _ _ _ 
_ M _ _ _ _ _ _ _ _ 
_ _ _ _ _ _ _ _ _ _ 
_ _ _ _ _ _ _ _ _ _ 
_ M _ M _ M _ _ _ _ 

~~~~~~~~~~~~~~~~
No. of rows = 10
No. of columns = 10
Mo. of mines = 20

output of mine placement
_ M _ _ _ _ _ _ _ _ 
_ _ M _ _ _ M _ _ _ 
_ _ _ _ _ _ _ _ _ _ 
M _ _ _ _ _ _ _ _ _ 
_ _ _ _ _ _ _ _ _ _ 
_ M _ _ _ _ _ _ _ _ 
_ _ M _ _ _ M M _ _ 
_ _ M _ M _ _ M M _ 
_ _ M _ _ _ M _ _ M 
_ M _ _ _ _ _ _ _ _

Thanks for any help.

Mattwaab;

[VernonDozier]

I have a few code snippets which may help.
http://www.daniweb.com/code/snippet1019.html
http://www.daniweb.com/code/snippet1034.html

Have a look at the GenerateRandomIntegers2 function. It's easier to follow. You need to generate 20 random numbers from 0 to 99 with no repeats (if you use my method).

However, your method could also work, with some fixing. Here is my guess as to what your problem is (or at least one of them):

if (randNum <= minesLeft)
                    {
                        mine [r][c] = true;
                        minesLeft --;
                    }
                    if (randNum > minesLeft)
                    {
                        mine [r][c] = false;
                    }

See red. What if mine[r][c] was already picked as true? You've now "unpicked" it without changing your minesLeft count. Check to see if that is a problem.

[_adam_]

Hey,

What it looks like is probably happening is that you are overwriting previously set mines.

For example, You set a mine (to true) on the first iteration of your outer while loop at (2, 2).

However, at the end of this first iteration you have not placed enough mines. Due to this you have a second iteration.

One failed circumstance is that on the second pass the randomly generated condition returns false over a previously true mine location, thus removing it from the grid.

The second failed circumstance is that on the second iteration of the while loop the same cell (2,2) is set to true again and your code decreases the number of mines to place.

Ok, things to change:

1. The mine grid should initially all be set to false, i.e. no mines are present.
2. There is no need for the second if statement.
3. Before line 20 add the following line:

if( mine[r][c] )
    continue;

An interesting method of generating a random number I might add. However, the above three points should be enough to get you going.

All the best,
AdamEdit: Also put the lines 32 and 33 inside your if true condition.

[mattwaab]

ok... THANKS SO MUCH.

the problem was that i was setting some mines back to false and i did not realize it. i changed the code so that:

the array is cleared at the beginning of the init funct.
got rid of the redundant if that would unset mines

all works well now!!

i think what it was is that i have been staring at this code for so long i just would breeze right over the obvious...

again THANKS!!

mattwaab;

[VernonDozier]

I'm a little suspicious of the line in red below too.

void Minesweeper::init ()
{
    srand((unsigned)time(0));  
    int minesLeft = 0, cellsLeft = 0, randNum = 0;
    
    // sets minesLeft to the data member numMines
    minesLeft = numMines;      
    
    //sets cellsLeft to the number of rows and cols in the board
    //nrows and ncols are both data members
    cellsLeft = nRows * nCols;
  
    while (minesLeft > 0)
    {
        
         for (int r = 0; r < nRows; r++)//rows
            {
                for (int c = 0; c < nCols; c++)//cols
                {
                    //places mines with a probibility of numMines / cellsLeft
                    randNum = (rand()%cellsLeft)+1;
                   
                    if (randNum <= minesLeft)
                    {
                        mine [r][c] = true;
                        minesLeft --;
                    }
           /*         if (randNum > minesLeft)
                    {
                        mine [r][c] = false;
                    }*/
                    cellsLeft--;
                    mark [r][c] = NO_MARK;
                }
            }
    }       
}//Minesweeper::init

I commented out what I think you deleted above, and perhaps this red line is now different from before or has a different effect. But it seems to me that, while you have fixed the minesLeft problem, you potentially still have a cellsLeft problem (i.e. you are subtracting a cell WITHOUT subtracting a mine). I think the red line should be inside the if statement ( cellsLeft only adjusted when a new mine is placed), but I'd have to see the updated code.

Also, thanks for using code tags and formatting on your first Daniweb post.

[_adam_]

Wa-hoo first solve!

I'm a little suspicious of the line in red below too.

I would have to agree. The method for generating your random number is quite odd.

A simpleif( rand()%100 < 10 ) would be enough to produce a pseudo 10% chance.

mattwaab: Pleased to hear you managed to solve your problem though!

Cheers,
Adam

[mattwaab]

the part that you put into red is needed because of the way that the mines are being determined. The mine placement is based on the probability of numMines / cellsLeft the probability increases every time the loop occurs by reducing the number of cells (as it is the number that is working with the random number generator). So every time the loop happens the number of cells decreases, raising the probability of there being a mine on that spot. Below i will put how i ended up fixing the problem.

void Minesweeper::init ()
{
    //Clear all old mines and marks
    for (int r = 0; r < nRows; r++)
    {
        for (int c = 0; c < nCols; c++)
        {
            mine [r][c] = false;
            mark [r][c] = NO_MARK;
        }
    }
    //seed the random num generator with the time
    srand((unsigned)time(0));  
    
    //declare and initilize needed vars
    int minesLeft = 0, cellsLeft = 0, randNum = 0;
    
    minesLeft = numMines;      
    cellsLeft = nRows * nCols;
  
    
    //loop to keep going while there are still mines not placed
    while (minesLeft > 0)
    {
        for (int r = 0; r < nRows; r++)//rows
            {
                for (int c = 0; c < nCols; c++)//cols
                {
                    
                    //pick a random num between 0 and the number of cells left
                    randNum = (rand()%cellsLeft)+1;
                   
                    //probibility check. 
                    if (randNum <= minesLeft)
                    {
                        //if prob check is true set that mine to true
                        mine [r][c] = true;
                        minesLeft --;
                    }
                    cellsLeft--;
                }
            }
    }       
}//Minesweeper::init

thanks again for the help!

Mattwaab;

[VernonDozier]

the part that you put into red is needed because of the way that the mines are being determined. The mine placement is based on the probability of numMines / cellsLeft the probability increases every time the loop occurs by reducing the number of cells (as it is the number that is working with the random number generator). So every time the loop happens the number of cells decreases, raising the probability of there being a mine on that spot. Below i will put how i ended up fixing the problem.

while (minesLeft > 0)
    {
        for (int r = 0; r < nRows; r++)//rows
            {
                for (int c = 0; c < nCols; c++)//cols
                {
                    
                    //pick a random num between 0 and the number of cells left
                    randNum = (rand()%cellsLeft)+1;
                   
                    //probibility check. 
                    if (randNum <= minesLeft)
                    {
                             mine [r][c] = true;
                            minesLeft --;
                    }
                    cellsLeft--;
                }
            }
    }

thanks again for the help!

Mattwaab;

You are welcome. I understood why you had the line. My point was that I thought it should be here (red below), given that you had the while loop so that you could go through the while loop more than once without cellsLeft being smaller than minesLeft (the while loop confused me):

while (minesLeft > 0)
    {
        for (int r = 0; r < nRows; r++)//rows
            {
                for (int c = 0; c < nCols; c++)//cols
                {
                    
                    //pick a random num between 0 and the number of cells left
                    randNum = (rand()%cellsLeft)+1;
                   
                    //probibility check. 
                    if (randNum <= minesLeft)
                    {
                        if (!mine[r][c])
                        {
                            mine [r][c] = true;
                            minesLeft --;
                            cellsLeft--;
                        }
                    }
                }
            }
    }

It looks like it will work either way, and I think I prefer keeping it outside the if statement as you have it rather than my earlier suggestion, but the way you have it, I think it will always only go through the while loop only once, in which case you can get rid of the while loop to simplify the code (the way you have it now will not lead to any problems; you just have a while loop which only executes once, I think, so it's not really a loop. You'd have problems if it went through MORE than once, but I don't think you do):

// no while loop needed, I don't think
        for (int r = 0; r < nRows; r++)//rows
            {
                for (int c = 0; c < nCols; c++)//cols
                {
                    
                    //pick a random num between 0 and the number of cells left
                    randNum = (rand()%cellsLeft)+1;
                   
                    //probibility check. 
                    if (randNum <= minesLeft)
                    {
                        //if prob check is true set that mine to true
                        mine [r][c] = true;
                        minesLeft --;
                    }
                    cellsLeft--;
                }
            }

[mattwaab]

i can see what you mean about the while loop being unneeded. That was left over from an earlier revision of the code when i was attempting to do it in a different way. The cellsLeft-- has to be on the outside if the if (!mine [r][c]) though because the probability of getting a mine in each square needs to change even if a mine is not placed. that is where the randNum = (rand()%cellsLeft)+1; comes into play. Every time that cellsLeft goes down, there is a better chance that randNum <= minesLeft will be true.

mattwaab;