0

I'm trying to make a simon says game and i need the background to change randomly every second. It changes, but the it does everything at once. I'm planning on adding a stylesheet later on, but for now, I just need this to work. Please help. I'm a beginner, so please be gentle.

HTML:

<!DOCTYPE HTML>
<html>
    <head>
        <title>Simon Says</title>
        <script src="script.js"></script>
    </head>

    <body>
        <button onclick="blue()" class="blue">Blue</button> 
        <button onclick="green()" class="green">Green</button>
        <button onclick="red()" class="red">Red</button>        
        <button onclick="yellow()" class="yellow">Yellow</button>
        <button onclick="ready()" class="ready">Ready</button>
    </body>
</html>

Javascript:

var seq = [0,1,2,1,3,2];
var rnd; 

function ready(){
    rnd = seq.length + 1;
    for(i = 0; i <= rnd;){
        seq[rnd] = Math.floor(Math.random()*4);
        setInterval(
        function () {
            switch(seq[i]){
                case 0:
                    document.body.style.backgroundColor = "rgb(0,0,255)";
                break;

                case 1:
                    document.body.style.backgroundColor = "rgb(0,255,0)";
                break;

                case 2:
                    document.body.style.backgroundColor = "rgb(255,0,0)";
                break;

                case 3:
                    document.body.style.backgroundColor = "rgb(255,255,0)";
                break;
            }
        }, 1000);
        console.log(i);
        i++;
    }
    rnd++;
}
  • You define `ready()` but never call it – hindmost Jan 21 '19 at 20:27
  • ``ready()`` is called ``onclick``. –  Jan 21 '19 at 20:28
  • 1
    Please check this answer: https://stackoverflow.com/a/7749109/4803039 I believe it answers your question – philip yoo Jan 21 '19 at 20:28
  • why are you calling setIntervals inside a for? After 1000ms, 6 setIntervals will be triggered, changing your color for same ones once `i` is stored in setInterval function scope... – guijob Jan 21 '19 at 20:37
  • Possible duplicate of [How to use setInterval function within for loop](https://stackoverflow.com/questions/7749090/how-to-use-setinterval-function-within-for-loop) – Wayne Phipps Jan 21 '19 at 21:12

2 Answers2

0

setInterval is not really what you want here. Try an alternative approach -

const buttons =
  { red: document.querySelector('.red')
  , green: document.querySelector('.green')
  , blue: document.querySelector('.blue')
  , yellow: document.querySelector('.yellow')
  }
  
const sleep = ms =>
  new Promise (r => setTimeout (r, ms))

const playSequence = async (seq, speed = 500) =>
{ for (const s of seq)
  { buttons[s].classList.toggle('lit', true)  
    await sleep (speed)
    buttons[s].classList.toggle('lit', false)
    await sleep (speed)
  }
}

playSequence ([ 'red', 'red', 'blue', 'green', 'yellow', 'red' ])
  .then(_ => console.log('done'))
.red { --rgb: 255, 0, 0; }
.green { --rgb: 0, 255, 0; }
.blue { --rgb: 0, 0, 255; }
.yellow { --rgb: 255, 255, 0; }


button {
  display: inline-block;
  width: 10vw;
  height: 10vw;
  background-color: rgba(var(--rgb), 0.25);
}

button.lit {
  background-color: rgba(var(--rgb), 1);
}
<button class="red"></button>
<button class="green"></button>
<button class="blue"></button>
<button class="yellow"></button>

Adjust the speed to increase difficulty -

// 200 ms delay
playSequence ([ 'red', 'red', 'blue', 'green', 'yellow', 'red' ], 200)
  .then(_ => console.log('done'))

Expand the snippet below to see it play faster -

const buttons =
  { red: document.querySelector('.red')
  , green: document.querySelector('.green')
  , blue: document.querySelector('.blue')
  , yellow: document.querySelector('.yellow')
  }
  
const sleep = ms =>
  new Promise (r => setTimeout (r, ms))

const playSequence = async (seq, speed = 500) =>
{ for (const s of seq)
  { buttons[s].classList.toggle('lit', true)  
    await sleep (speed)
    buttons[s].classList.toggle('lit', false)
    await sleep (speed)
  }
}

// 200 ms delay
playSequence ([ 'red', 'red', 'blue', 'green', 'yellow', 'red' ], 200)
  .then(_ => console.log('done'))
.red { --rgb: 255, 0, 0; }
.green { --rgb: 0, 255, 0; }
.blue { --rgb: 0, 0, 255; }
.yellow { --rgb: 255, 255, 0; }


button {
  display: inline-block;
  width: 10vw;
  height: 10vw;
  background-color: rgba(var(--rgb), 0.25);
}

button.lit {
  background-color: rgba(var(--rgb), 1);
}
<button class="red"></button>
<button class="green"></button>
<button class="blue"></button>
<button class="yellow"></button>
Mulan
  • 129,518
  • 31
  • 228
  • 259
0

You can make your code more DRY, and resolve the loop/timer issue, by simply adding the colours to an array and setting the body background to the index of that array given by the random index. Instead of setInterval I've used setTimeout which calls a function until the number of steps has been completed.

// Cache the DOM elements
const out = document.querySelector('#out');
const { body } = document;

const seq = [0, 1, 2, 1, 3, 2];

// Set up your color array
const cols = ["rgb(0,0,255)", "rgb(0,255,0)", "rgb(255,0,0)", "rgb(255,255,0)"];

function ready() {

  // `next` accepts a parameter c which is the current
  // step count. It's default is 0
  const next = function(c = 0) {

    // If the step count is less than 6
    if (c < 6) {

      // Update the DOM...
      out.textContent = `Step: ${c + 1}`;
      body.style.backgroundColor = cols[seq[c]];

      // ...and call the function again after a second
      // incrementing the step count
      setTimeout(next, 1000, ++c);
    }
  }

  // Call the loop the first time
  next();
}
#out { background-color: white };
<button onclick="ready()" class="ready">Ready</button>
<div id="out"></div>
Andy
  • 61,948
  • 13
  • 68
  • 95