javascript minesweeper expand messing up counter
Asked Answered
O

3

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

Orbit answered 23/8, 2013 at 2:10 Comment(1)
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.Team
R
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

Ruvalcaba answered 25/8, 2013 at 20:0 Comment(6)
i dont think "document.getElementById("flipped").innerHTML = newFlippedCount.length;" will work please edit answer or explain that code and thank you for your helpOrbit
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...Ruvalcaba
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.Orbit
thanks works perfectly, im gonna go compare it and try to understand the changes.Orbit
There was a capitalization bug in that version - check fiddle v3. 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.Ruvalcaba
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 whatOrbit
T
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.

Thracian answered 25/8, 2013 at 20:10 Comment(1)
thanks the other answer didnt point out the reason my code was giving me that problemOrbit
O
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.

Orbit answered 29/9, 2013 at 5:9 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.