CODEWITHSUNDEEP
javascripthtmlcss
Mihir
Mihir

Reputation: 73

Variable changes its value inside function

I am trying to make an RGB color guessing game. It is working fine on the first run. On the second time using New Game button, new pickedColor and a set of random colors should be generated. Even though new pickedColor is generated every time (checked in line 30) but the value of pickedColor changes back to its previous value on line 42.

Why is it happening? How Can I get same value of pickedColor on line 30 and line 42?

function getRandomColor() {
  var arr = [];
  var r = 0,
    g = 0,
    b = 0;
  var temp;

  for (var i = 0; i < 6; i++) {
    r = Math.floor(Math.random() * 256);
    g = Math.floor(Math.random() * 256);
    b = Math.floor(Math.random() * 256);

    arr[i] = "rgb(" + r + ", " + g + ", " + b + ")";
  }

  return arr;
}

function pickColor(colors) {
  var num = Math.floor(Math.random() * colors.length);
  return num;
}

function newGame() {
  var colors = getRandomColor();
  var colorTile = document.querySelectorAll(".colorTile");
  var pickedColor = colors[pickColor(colors)];
  console.log(pickedColor);
  console.log(colors);

  var rgbDisplay = document.querySelector("#rgbDisplay");
  var h1 = document.querySelectorAll("h1");

  rgbDisplay.textContent = pickedColor;
  for (var i = 0; i < colorTile.length; i++) {
    colorTile[i].style.backgroundColor = colors[i];
    colorTile[i].addEventListener("click", function() {
      console.log(pickedColor);
      if (pickedColor === this.style.backgroundColor) {
        h1[1].textContent = "Correct";

        for (var j = 0; j < colorTile.length; j++) {
          colorTile[j].style.backgroundColor = this.style.backgroundColor;
        }
      } else {
        h1[1].textContent = "Incorrect";
        this.style.backgroundColor = "black";
      }
    });
  }
}

newGame();

var button = document.querySelectorAll("button");
button[0].addEventListener("click", function() {
  var h1 = document.querySelectorAll("h1");
  h1[1].textContent = "Click On Tile"
  newGame();
});
#container {
  width: 600px;
  height: 400px;
  margin: 0px auto;
}

.colorTile {
  background-color: red;
  width: 30%;
  margin: 1.6%;
  padding-bottom: 30%;
  float: left;
}

body {
  background-color: black;
}

h1 {
  color: yellow;
  text-align: center;
}

button {
  width: 20%;
  height: 30px;
  margin: 30px;
  margin-left: 40%;
  color: black;
  background-color: yellow;
}

HTML
<!DOCTYPE html>
<html lang="en" dir="ltr">
<link href="../css/ex160.css" rel="stylesheet">
<script defer src="../JavaScript/ex160.js"></script>

<head>
  <meta charset="utf-8">
  <title>Color Game</title>
</head>

<body>

  <h1>The Great <span id="rgbDisplay">RGB</span> Game</h1>

  <div id="container">
    <div class="colorTile"></div>
    <div class="colorTile"></div>
    <div class="colorTile"></div>
    <div class="colorTile"></div>
    <div class="colorTile"></div>
    <div class="colorTile"></div>
  </div>

  <h1>Click On Tile</h1>

  <button class="restart"><em>New Game</em></button>
</body>

</html>

Upvotes: 0

Views: 73

Answers (2)

Takit Isy
Takit Isy

Reputation: 10091

I played with your code and ended-up with a working snippet where I tried to enhance many things:

I tried to comment within the code during modifications.
With it, you can play with the amount of color tiles you want, and it's working nicely.

// Define global variables all at the same place
var rgbDisplay = document.querySelector("#rgbDisplay");
var h1 = document.querySelectorAll("h1");
var container = document.querySelector("#container");
var tilesNumber = 6; // Select how much tiles you want to start with!
var pickedColor;
var button = document.querySelector("button");

// Added s to Colors in the name
// Removed "temp" variable and r g b initial values
function getRandomColors(nb) {
  var arr = [];
  var r, g, b;
  for (var i = 0; i < nb; i++) {
    r = Math.floor(Math.random() * 256);
    g = Math.floor(Math.random() * 256);
    b = Math.floor(Math.random() * 256);
    arr[i] = "rgb(" + r + ", " + g + ", " + b + ")";
  }
  return arr;
}

// Removed variable used in this function
// Changed parameter to nb, that's the only thing used, no need to pass all the colors
function pickColor(nb) {
  return Math.floor(Math.random() * nb);
}

// Added nb as parameter of this function
function newGame(nb) {
  var colors = getRandomColors(nb); // Using nb
  pickedColor = colors[pickColor(nb)];
  rgbDisplay.textContent = pickedColor;
  container.innerHTML = ''; // Emptying container
  for (var i = 0; i < tilesNumber; i++) { // Adding new element for each tilesNumber
    var tile = document.createElement("div");
    tile.classList.add('colorTile');
    container.appendChild(tile);
    tile.style.backgroundColor = colors[i];
  }

  var colorTile = document.querySelectorAll(".colorTile");
  for (var i = 0; i < nb; i++) {
    colorTile[i].addEventListener("click", function() {
      if (pickedColor === this.style.backgroundColor) {
        h1[1].textContent = "Correct";
        tilesNumber++; // We could add that funny kind of things, now!
        for (var j = 0; j < nb; j++) {
          colorTile[j].style.backgroundColor = this.style.backgroundColor;
        }
      } else {
        h1[1].textContent = "Incorrect";
        this.style.backgroundColor = "black";
      }
    });
  }
}

// Event on button
// (Side question: Wouldn't be nice to disable it until correct answer?)
// Removed querySelection, because global variables…
button.addEventListener("click", function() {
  h1[1].textContent = "Click On Tile";
  newGame(tilesNumber);
});

// Launch game
newGame(tilesNumber);
body {
  background-color: black;
}

#container {
  width: 600px;
  margin: 0px auto;
}

.colorTile {
  /* Modified */
  background-color: red;
  margin: 1.6%;
  padding: 0 0 30% 30%;
  float: left;
}

h1 {
  color: yellow;
  text-align: center;
}

button {
  width: 20%;
  height: 30px;
  margin: 30px;
  margin-left: 40%;
  color: black;
  background-color: yellow;
}
<!DOCTYPE html>
<html lang="en" dir="ltr">
<link href="../css/ex160.css" rel="stylesheet">
<script defer src="../JavaScript/ex160.js"></script>

<head>
  <meta charset="utf-8">
  <title>Color Game</title>
</head>

<body>
  <h1>The Great <span id="rgbDisplay">RGB</span> Game</h1>
  <div id="container"></div>
  <h1>Click On Tile</h1>
  <button class="restart"><em>New Game</em></button>
</body>

</html>

Feel free to comment if anything.

I hope it helps. :)

Upvotes: 1

Nandita Sharma
Nandita Sharma

Reputation: 13417

You are binding multiple clicks on colorTile element with this line.

colorTile[i].addEventListener("click", function()

Try removeEventListener and then add new removeEventListener like this

function getRandomColor()
{
  var arr = [];
  var r=0, g=0, b=0;
  var temp;

  for(var i=0; i<6; i++)
  {
    r = Math.floor(Math.random() * 256);
    g = Math.floor(Math.random() * 256);
    b = Math.floor(Math.random() * 256);

    arr[i] = "rgb("+r+", "+g+", "+b+")";
  }

  return arr;
}

function pickColor(colors)
{
  var num = Math.floor(Math.random() * colors.length);
  return num;
}

var pickedColor = '';
var colorTile = document.querySelectorAll(".colorTile");
function newGame()
{
  var colors = getRandomColor();
  pickedColor = colors[pickColor(colors)];
  console.log(pickedColor);
  console.log(colors);

  var rgbDisplay = document.querySelector("#rgbDisplay");
  h1 = document.querySelectorAll("h1");

  rgbDisplay.textContent = pickedColor;
  for(var i=0; i<colorTile.length; i++)
  {
    colorTile[i].style.backgroundColor = colors[i];
    colorTile[i].removeEventListener('click', fn)
    colorTile[i].addEventListener("click", fn);

  }

}
function fn (){
      console.log(pickedColor);
      if(pickedColor === this.style.backgroundColor)
      {
        h1[1].textContent = "Correct";

        for(var j=0; j<colorTile.length; j++)
        {
          colorTile[j].style.backgroundColor = this.style.backgroundColor;
        }
      }

      else{
        h1[1].textContent = "Incorrect";
        this.style.backgroundColor = "black";
      }
    }
newGame();

var button = document.querySelectorAll("button");
button[0].addEventListener("click", function(){
  var h1 = document.querySelectorAll("h1");
  h1[1].textContent = "Click On Tile"
  newGame();
});

Upvotes: 1

Related Questions