-1

I'm working on a game in JavaScript (I'm a novice), classic Lights Off puzzle. I managed to get the table to print out, and. However, I have a problem with checkAllOff function, which is supposed to test whether all buttons are in the off state (black), and if this is the case it should display a message "You win!". I want this function to also remove the message if, following a subsequent press, one or more buttons returns to the on state(yellow). I'm asking for pointing out my mistakes in that functions and an explenation of how it should be done properly. My code is included in images:

<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">

<!-- JavasScript code is contained within an HTML <script> element -->
<script>
function toggle(i,j) {
      b=document.getElementById("but_" + i + j)
      t = b.innerHTML
      if (t=="X") {b.innerHTML = "O";
                   b.setAttribute( "style", "color:red; background-color:yellow" )
                  }
      if (t=="O") {b.innerHTML = "X";
                   b.setAttribute( "style", "color:white; background-color:black" )
                  }
}

function checkAllOf(){
    //Check if board is solved
    var i=0
    var j=0
    var counter=0
    b=document.getElementById("but_" + i + j)
    t = b.innerHTML
    for(i=0;i<=5;i++)
	for (j=0; j<5; j++){
        if(t=="X") 
		counter++;
    }
    if (counter==25)
	return(false);
    return(true);
}


function press(i,j) {

 while(1)
 {
       toggle( i, j )
       if (j!=0)
       	    toggle(i, j-1)
       if (j+1<5)
            toggle(i, j+1)
       if (i+1<5)
	    toggle(i+1, j)
       if (i!=0)
	    toggle(i-1, j)

       if (checkAllOf){
	    alert(All lights are out! You win!)
		}
}		
}


function generateGrid() {
        var d = document.getElementById("button-grid");
        var table = document.createElement("table");
        d.appendChild(table);
        for (var i = 0; i < 5; i++) {
                var row = document.createElement("tr");
                for (var j = 0; j < 5; j++) {
                        var cell = document.createElement("td");
                        cell.innerHTML = "<button type=button id=but_" + i + j +
                                         " onclick=\"press(" +i + ',' +j + ")\"" + 
                                         " style=\"color:red; background-color:yellow\"" +
                                         ">O</button>" ;
                        row.appendChild(cell);
                }
                table.appendChild(row);
        }
        toggle(2,2) // Set middle button to off state (otherwise seems to be impossible).
}

window.onload = function() {
        generateGrid();
};
</script> 

<title>Lights Off Puzzle</title>
</head> 

<body>
<div align="center" id="button-grid">
<h1> *** Lights Off *** </h1>
<h2> Click on buttons until they all turn black </h2>
</div>

</body>

</html>

enter image description here

enter image description here

enter image description here

4
  • Hi Nika, you should add the actual code in your question instead of the screenshots. You can add a snippet by pressing CTRL+M and writing your HTML and CSS.
    – Ivan
    Nov 30, 2019 at 12:05
  • What are you trying to achieve with the while loop? Every time the user clicks, it creates an infinite loop!
    – Ivan
    Nov 30, 2019 at 12:35
  • But hwat would be the other way of making sure that when all buttons are black and communicate displayed, when user clicks again then the message would disappear? My thought process was for while() to make it so it infinitely goes though toggle and the checkforAll
    – Nika
    Nov 30, 2019 at 14:06
  • I have written an answer, right below.
    – Ivan
    Nov 30, 2019 at 17:58

1 Answer 1

0

∞ loop in press()

There is an issue in your press function. I see where you want to get at, but this won't work! Everytime you click on a button a while loop is initiated and it will start toggling the button you've pressed on and off as well as checking the conditions (j!=0), (i, j-1), (j+1<5), (i, j+1), (i+1<5), (i!=0). There is nothing stopping this loop!

There's an easier way of going about this problem: simply call toggle on all adjacent buttons. Then in toggle check if the button you're wanting to toggle actually exists (if it does exist change its state, if it doesn't, just return). Indeed you will be expecting press to call toggle on buttons that don't exist.

For instance: if you press but_01 you will call press(0,1) in turn calling toggle(0,1), toggle(-1,1), toggle(1,1), toggle(0,0) and toggle(0,2). However but_-11 doesn't exists and only but_01, but_11, but_00 and but_02 will see their state change.


checkAllOf() will always return the state of but_00

You have several issues when checking the state of the board:

First, you are initializing i, j and counter:

var i = 0
var j = 0
var counter = 0

Then you select the content of but_00

b = document.getElementById("but_" + i + j)
t = b.innerHTML

You loop over ixj i.e. [0;5[x[0;5[ (5[ means 5 is excluded) i.e. [0;4]x[0;4]. Here I should mention you are out of bound (i needs to be smaller than 5, it can't be equal as you only have 5 rows: 0, 1, 2, 3, 4.

for (i=0 ; i<5 ; i++)
  for (j=0 ; j<5 ; j++){
    if (t=="X") 
      counter++;
    }
  }
}

The problem is: the value of t is whatever b.innerHTML returned but it will not change over the period of these loops. You're essentially checking if t=="X" 25 times! Instead, we need to update its value then check if it's equal to "X".

Lastly, you're checking for the total number of "X"s on the board to be equal to 25. This is simply not necessary.

To know if you've won, you need to check all buttons, but if at least one button is an O then you already know you didn't win, right? So when looping over the buttons, if you find an "X" you can directly return false. If the loop as passed, i.e. all values are not "X"s then you know you've won and you can return true:

for (i=0 ; i<5 ; i++)
  for (j=0 ; j<5 ; j++){
    b=document.getElementById("but_" + i + j)
    t = b.innerHTML
    if (t=="O")
      return false;
    }
  }
}

So you don't need to keep in memory the number of "X"s on the board.

We can write this even better by declaring the variables i and j inside the loop's head.

Also, you might want to use the let declaration statement...


Side note, you have a syntax error on your alert, you need quotes. But that wasn't really the main concern here I presume.

Also, you need to actually call checkAllOf with checkAllOf() in the if condition as checkAllOf will return the function itself, not the result of the function.


Possible solution

function toggle(i, j) {
  b = document.getElementById("but_" + i + j)
  if (!b) return;
  t = b.innerHTML
  if (t == "X") {
    b.innerHTML = "O";
    b.setAttribute("style", "color:red; background-color:yellow")
  }
  if (t == "O") {
    b.innerHTML = "X";
    b.setAttribute("style", "color:white; background-color:black")
  }
}

// Check if board is solved
function checkAllOff() {
  for (var i = 0; i < 5; i++)
    for (var j = 0; j < 5; j++) {
      t = document.getElementById("but_" + i + j).innerHTML
      if (t == "O")
        return false
    }
  return (true);
}


function press(i, j) {
  toggle(i, j)
  toggle(i, j - 1)
  toggle(i, j + 1)
  toggle(i + 1, j)
  toggle(i - 1, j)

  if (checkAllOff()) {
    console.log("All lights are out!You win!")
  }
}


function generateGrid() {
  var d = document.getElementById("button-grid");
  var table = document.createElement("table");
  d.appendChild(table);
  for (var i = 0; i < 5; i++) {
    var row = document.createElement("tr");
    for (var j = 0; j < 5; j++) {
      var cell = document.createElement("td");
      cell.innerHTML = "<button type=button id=but_" + i + j +
        " onclick=\"press(" + i + ',' + j + ")\"" +
        " style=\"color:red; background-color:yellow\"" +
        ">O</button>";
      row.appendChild(cell);
    }
    table.appendChild(row);
  }
}

window.onload = function() {
  generateGrid();
};
<div align="center" id="button-grid">
  <h1> *** Lights Off *** </h1>
  <h2> Click on buttons until they all turn black </h2>
</div>

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.