I'm trying to code the game Simon in HTML/JS and it's all working, except for the part where the game flashes the sequence so you know what the new sequence is. Essentially what I have is:
for(var i in thePattern){
var obj = document.getElementById(thePattern[i]);
window.setTimeout(colorON(obj),500);
window.setTimeout(colorOFF(obj),1000);
}
where colorON and colorOFF are:
function colorON(obj){
if(obj.id == "red"){
obj.style.backgroundColor="#ff5555";
}else if(obj.id == "blue"){
obj.style.backgroundColor="#5555ff";
}else if(obj.id == "green"){
obj.style.backgroundColor="#88ff88";
}else{
obj.style.backgroundColor="#ffffaa";
}
}
function colorOFF(obj){
if(obj.id == "red"){
obj.style.backgroundColor="#ff0000";
}else if(obj.id == "blue"){
obj.style.backgroundColor="#0000ff";
}else if(obj.id == "green"){
obj.style.backgroundColor="#22ff22";
}else{
obj.style.backgroundColor="#ffff00";
}
}
What it seems to be doing is going through the entire for loop and starting all the timers and then then all the timers go off so quickly that the colors don't even seem to flash.
Any ideas? All help is greatly appreciated.
Now it is flashing correctly and the closure is working correctly, but it is flashing all the colors at the same time. I've tried putting the closure within another setTimeout, however, that just creates other problems.
SOLVED thanks for your help guys.
I'm trying to code the game Simon in HTML/JS and it's all working, except for the part where the game flashes the sequence so you know what the new sequence is. Essentially what I have is:
for(var i in thePattern){
var obj = document.getElementById(thePattern[i]);
window.setTimeout(colorON(obj),500);
window.setTimeout(colorOFF(obj),1000);
}
where colorON and colorOFF are:
function colorON(obj){
if(obj.id == "red"){
obj.style.backgroundColor="#ff5555";
}else if(obj.id == "blue"){
obj.style.backgroundColor="#5555ff";
}else if(obj.id == "green"){
obj.style.backgroundColor="#88ff88";
}else{
obj.style.backgroundColor="#ffffaa";
}
}
function colorOFF(obj){
if(obj.id == "red"){
obj.style.backgroundColor="#ff0000";
}else if(obj.id == "blue"){
obj.style.backgroundColor="#0000ff";
}else if(obj.id == "green"){
obj.style.backgroundColor="#22ff22";
}else{
obj.style.backgroundColor="#ffff00";
}
}
What it seems to be doing is going through the entire for loop and starting all the timers and then then all the timers go off so quickly that the colors don't even seem to flash.
Any ideas? All help is greatly appreciated.
Now it is flashing correctly and the closure is working correctly, but it is flashing all the colors at the same time. I've tried putting the closure within another setTimeout, however, that just creates other problems.
SOLVED thanks for your help guys.
Share Improve this question edited Aug 3, 2017 at 14:31 YakovL 8,36513 gold badges73 silver badges112 bronze badges asked May 13, 2013 at 17:29 joebobs0njoebobs0n 131 silver badge4 bronze badges 2- You really need to learn about switch statements. – epascarello Commented May 13, 2013 at 17:34
- First fix the problems pointed out in the answers here. Then when you find the next issue that fixing this will expose, have a look at this: stackoverflow./questions/750486/… – James Montagne Commented May 13, 2013 at 17:36
2 Answers
Reset to default 7You need to pass a function to setTimeout:
window.setTimeout(function() {
colorON(obj);
},500);
Right now, you're calling colorON(obj) immediately and passing its output to setTimeout, which makes it appear like setTimeout is firing immediately.
obj is also passed by reference, so by the time all of your functions will run, obj will refer to the last element in your loop. To get around that, you have to pass obj by value by shadowing it:
(function(obj) {
window.setTimeout(function() {
colorON(obj);
}, 500);
window.setTimeout(function() {
colorOFF(obj);
}, 1000);
})(obj);
You are calling the function, not assigning a reference to it! So the code runs right away and sets the setTimeout with whatever the function returns.
change
window.setTimeout(colorON(obj),500);
window.setTimeout(colorOFF(obj),1000);
to
for(var i in thePattern){
var obj = document.getElementById(thePattern[i]);
(function(obj) {
window.setTimeout(function(){colorON(obj);},500);
window.setTimeout(function(){colorOFF(obj);},1000);
})(obj);
}
and code showing you how to do with with a switch or an object to get rid of the if/else logic
function colorON(obj) {
var color = "";
switch (obj.id) {
case "red":
color = "#ff5555"
break;
case "blue":
color = "#5555ff"
break;
case "green":
color = "#88ff88"
break;
default:
color = "#ffffaa"
}
obj.style.background = color;
}
var colorsOff = {
"red": "#ff0000",
"blue": "#0000ff",
"green": "#22ff22",
"default": "#ffff00"
}
function colorOFF(obj) {
var color = colorsOff[obj.id] || colors["default"];
obj.style.backgroundColor = color;
}
var thePattern = {
"one": "red",
"two": "blue",
"three": "green"
}
for (var i in thePattern) {
var obj = document.getElementById(thePattern[i]);
(function (obj) {
window.setTimeout(function () {
colorON(obj);
}, 500);
window.setTimeout(function () {
colorOFF(obj);
}, 1000);
})(obj);
}
Example: http://jsfiddle/brjgc/