1

I'm trying to build a simple Minesweeper application using a 2D vector. First I fill the squares with 1's and 0's (1's meaning mines and 0's meaning clear). 

srand(time(NULL));
for (int i = 0; i < battlefield.size(); i++)
{
    for (int j = 0; j < battlefield[i].size(); j++)
    {
        int num = (rand() % 2);
        battlefield[i][j] = num;
    }
}`

Then, I go through the vector again and count the number of surrounding mines. This is where I'm having my problem. I think when it tries to check a square that is out of bounds, it blows up. However, if fails before any of those checks. If fails when it tries to see if the current square is equal to 1. 

for (int i = 0; i < battlefield.size(); i++)
{
    for (int j = 0; j < battlefield[i].size(); j++)
    {
        int count = 0;
        if (battlefield[i][j] == 1)//mine square
        {
            if (battlefield[i - 1][j - 1] != 0)
            {
                count++;
            }
            if (battlefield[i][j - 1] != 0)
            {
                count++;
            }
            if (battlefield[i + 1][j - 1] != 0)
            {
                count++;
            }
            if (battlefield[i - 1][j] != 0)
            {
                count++;
            }
            if (battlefield[i + 1][j] != 0)
            {
                count++;
            }
            if (battlefield[i - 1][j + 1] != 0)
            {
                count++;
            }
            if (battlefield[i][j + 1] != 0)
            {
                count++;
            }
            if (battlefield[i + 1][j + 1] != 0)
            {
                count++;
            }
            battlefield[i][j] = count;
        }
    }

I'm not really sure why it is failing there, any ideas?

Chris
  • 73
  • 1
  • 2
  • 8
  • "I think when it tries to check a square that is out of bounds, it blows up." What does running it through a debugger tell you? At the very least have added debug output to show what is going on? – graham.reeds Apr 30 '17 at 06:45
  • First it says that it triggered a breakpoint and then it says "Unhandled exception at 0x0FAB1FD5 (ucrtbased.dll) in MineSweeper.exe: An invalid parameter was passed to a function that considers invalid parameters fatal. " It stops at the line: _DEBUG_ERROR("vector subscript out of range"); – Chris Apr 30 '17 at 06:49
  • @Chris `vector subscript out of range` -- You have no idea what that error means? Seems clear enough. – PaulMcKenzie Apr 30 '17 at 06:50
  • I know what it means but I am confused why it is receiving that error at this line: if (battlefield[i][j] == 1). It would make more sense if it was at one of the lines below, but I don't understand why it would throw that error there. – Chris Apr 30 '17 at 06:52
  • at the last iteration of the outer loop this battlefield[i + 1] will be out of bounds. imagine its equal to battlefieldsize()-1 then if you +1 it will go past the bounds. – Lorence Hernandez Apr 30 '17 at 06:53
  • @Chris You are misreading your debugger. Put official boundary check in your code and you will see that the line you're claiming is the issue is not the issue. – PaulMcKenzie Apr 30 '17 at 06:57

3 Answers3

1

Make a "CheckTile" function

bool CheckTile(int i, int j)
{
    // check if both i and j are in the map bounds, return false if not
    if(i < 0 || i >= battlefield.size() || j < 0 || j >= battlefield.size()) 
        return false;

    // return true if the tile is a mine, false if not
    return (battlefield[i - 1][j - 1] != 0);
}

Basically replace the innards of all your if checks with calls to CheckTile. Your current code is breaking because you're not doing bounds checking on the array, so when it reads 

if(battlefield[i - 1][j - 1] != 0)

The first time, it's trying to read the array location (-1,-1) which is outside of allocated memory, thus an error. Replace that with

if(CheckTile(i - 1, j - 1))

And it will do the same thing, but with the added bounds checking protection.

Jason Lang
  • 1,079
  • 9
  • 17
  • I understand; however, when I was checking the square with an if statement it blew up. Why is it any different when checking it with a boolean function? – Chris Apr 30 '17 at 06:57
  • 1
    @Chris Don't check the square -- check the values of `i` and `j`. As soon as you say `battlefield[i-1]` in any context, even in an `if` statement, you've got an error. – PaulMcKenzie Apr 30 '17 at 07:00
  • because you didn't do bounds checking. The bounds checking checks that i and j are within the allow ranges before accessing the array. if the i/j don't fit then it avoids accessing the array at all. – Jason Lang Apr 30 '17 at 07:00
  • the reason for a function is so you don't have to replicate the bounds checking code in every single if statement. – Jason Lang Apr 30 '17 at 07:01
  • Oh I understand now! That should solve everything. I don't know why I never thought of that – Chris Apr 30 '17 at 07:03
1

If your mine is at 0,0 the first check you perform will check -1,-1 which is out of bounds, literally blowing up your program.

You need to check your boundaries first.

graham.reeds
  • 16,230
  • 17
  • 74
  • 137
1

You are indeed lucky. Accessing out of boundaries of an std::vector is "undefined behavior" and the program stops only if you are lucky.

When you are unlucky the program apparently "works" anyway and you keep adding features until is shipping day... and it will blow up only on the wide screen of customer machine during the stockholders meeting :-)

In my opinion a simpler implementation would be

int i0 = std::max(0, i-1), i1 = std::min(height-1, i+1);
int j0 = std::max(0, j-1); j1 = std::min(width-1, j+1);
int count = 0;
for (int i=i0; i<=i1; i++) {
    for (int j=j0; j<=j1; j++) {
        if (mine[i][j]) count++;
    }
}

basically computing first the boundaries that are safe and then looping in them instead of singly specifying all neighbors in code.

6502
  • 112,025
  • 15
  • 165
  • 265