6

I made a minesweeper game in javascript, which was finally running pretty smoothly, until i added the "expand()" function (see below). I have 3 issues:

  1. When it expands it adds too many to "flippedCount" ( see code below ) - in the image below the div to the right displays "flippedCount", and its 39 instead of 35.
  2. As a result, if a player exceeds 90 squares ( amount to win ) during a "expand()", the winning screen doesnt show.
  3. It also doesnt expand properly ( see images below ).

The relevant code and a link is below these 2 images

Image showing a partly completed minesweeper field

Image showing an 'empty' minesweeper field

var flippedCount = 0;
var alreadySetAsZero = [];
var columnAmount = 10;

function processClick(clicked) { //i use a "(this)" to pass as "(clicked)"
  nextToBombCheck( parseInt(clicked.id) );
  checkWin();
}

nextToBombCheck( boxNum ) { 
  flippedCount++;
  document.getElementById("flipped").innerHTML = flippedCount;  
  //long function setting "bombCount" to # bombs around clicked square goes here
  if ( bombCount !== 0 ) { 
    //blah blah blah
  } else {
    alreadySetAsZero[ boxNum ] = "yes";
    expand(boxNum);
  } 
}

function expand( emptyBoxId ) {
  checkRightOfEmpty( emptyBoxId + 1 );
  checkLeftOfEmpty( emptyBoxId - 1 );
  checkAboveEmpty( emptyBoxId - columnAmount );
  checkBelowEmpty( emptyBoxId + columnAmount );
} 

function checkRightOfEmpty( boxToTheRightId ) {
  //check if already marked as zero
  if ( alreadySetAsZero[ boxToTheRightId ] === "yes" )
    return;
  //if box is at the edge
  if ( boxToTheRightId % columnAmount === ( 0 ) ) {
    //do nothing
  } else {
    nextToBombCheck( boxToTheRightId );
  }
}

//and the rest are 3 similar functions

I was not able to find a pattern with the lack of expansion or numbers added to flipped count.

link here

p.s. sorry about the title i dont know what else to call it

AstroCB
  • 12,337
  • 20
  • 57
  • 73
Math chiller
  • 4,123
  • 6
  • 28
  • 44
  • If I may give a suggestion: Format your code properly (proper indentation) and pay some attention to the grammar of phrases (a phrase starts with a capital for example and usually ends in a full stop). People are more likely to answer a question if it is easy to read. – Sumurai8 Aug 25 '13 at 20:12

3 Answers3

4

You are getting an incorrect count because your algorithm doesn't take into account squares that have already been flipped - resulting is squares getting counted twice.

The simplest way to keep an accurate count of the is to make use of that fact that 'getElementsByClassName' returns a live node list.

Note: getElementsByClassName has pretty good browser support, but if your browser requirements are different, you will need to adjust this a bit.

On entry, initialize the list (variable name just to differentiate from previous name):

var newFlippedCount = document.getElementsByClassName('flipped');

Each time you update a square with a bomb count class use this instead (adds the flipped class as well):

document.getElementById(boxNum).className = classList[bombCount] + ' flipped';

When you update the UI with the new flipped count use:

document.getElementById("flipped").innerHTML = newFlippedCount.length;

Your count is now magically correct :)

Note: you could also solve this by checking to see if the box has already been flipped before incrementing flippedCount.

You were also periodically getting an error in the console:

Uncaught TypeError: Cannot set property 'className' of null

at this line (around line: 298):

document.getElementById(boxNum).className = classList[bombCount];

You should protect that either by checking the bounds or just by checking the return value before using it:

var ele = document.getElementById(boxNum);
if(!ele) return;

ele.className = classList[bombCount] + ' flipped';

Demo

Edit: If you are interested, here is a high level overview of the Minesweeper Cascade Algorithm

Update - cascade bug found

One of the reasons the cascade wasn't working correctly on successive replays is that the variable used to track cells that had already been set to zero wasn't getting reset in a new game. For a fix:

function start() {
   …
   // Reset to empty array
   alreadySetAsZero = [];
   ...
}

Updated fiddle with additional debug statements to help find the problem:

Demo

dc5
  • 12,341
  • 2
  • 35
  • 47
  • i dont think "document.getElementById("flipped").innerHTML = newFlippedCount.length;" will work please edit answer or explain that code and thank you for your help – Math chiller Aug 25 '13 at 20:14
  • I didn't claim these were the only spots to change or that your code was completely debugged. There are several places where the logic checks flippedCount - each of those would need to be changed as well. This should get you started though... – dc5 Aug 25 '13 at 23:19
  • i fixed the flipped count mostly based on the other answer here it still seems to be bugged a bit. but its off by a lot less and i still got the problem with not flipping all the boxes it is supposed to. – Math chiller Aug 25 '13 at 23:26
  • thanks works perfectly, im gonna go compare it and try to understand the changes. – Math chiller Aug 25 '13 at 23:35
  • There was a capitalization bug in that version - check [fiddle v3](http://jsfiddle.net/HvdJR/3/). It was triggered when clicking on a cell that had already been flipped. With the capitalization fixed, it now causes you to lose the game as it did in your initial code. **Note** bombs are displayed initially to ease debugging in this version. – dc5 Aug 25 '13 at 23:49
  • i had something in place for clicking on a already flipped square line 116 (in "process(clicked)" ) "_if (gameOn && **clicked.className == "box"**)_" = if you click on already flipped function doesnt run. it must have been something else dont know what – Math chiller Aug 25 '13 at 23:57
1

Although you use "alreadySetAsZero" to stop squares being counted multiple times, this doesn't take into account squares with a bomb count. These will be counted multiple times in your expand method.

Stephen Cook
  • 171
  • 2
  • 11
0

i found the problem that was bothering me (the counter) it counted the squares "-1" and "100"

i then added || boxToTheRightId > ( cellAmount - 1 ) to checkRightOfEmpty and || boxToTheRightId < 0 to checkLeftOfEmpty (in their if statements) and it works just fine, thanks anyway guys.

Math chiller
  • 4,123
  • 6
  • 28
  • 44