Simon Game - mobile performance - JavaScript

Projects Personal Projects
1

Hello all!
I recently followed a tutorial from @CarnesBeau with freeCodeCamp.org. The project was to make a Simon Game ala the game from Hasbro from the late 70s, early 80s. I decided to make some improvements on it. I recorded my own sounds on my keyboard in Ableton Live, added an error sound and made the game responsive to work on mobile as his tutorial was not.
The game works perfectly on desktop, but I’ve noticed some glitches on mobile when I try to play it on my cellphone. Mainly, it seems that if the same color lights up twice in a row, the sound only happens the first time and occasionally the sounds seem to not play on the first move or randomly. (This was already happening before I added my own sounds, so that isn’t the problem.)
Some thoughts – I read something about mobile browsers having a 300ms delay, but I believe this is no longer the case, so I’m not sure if that has any bearing on the problem. There are some places in the code where setTimeout is used with certain delays, I’m not sure if that could be related to the problem. The final thought I had was that my event listeners are using ‘click’ events. I wasn’t sure if I need to add ‘touch’ events as well for mobile? It seems that the mobile browsers (I’ve tried both Chrome and Safari) treat the ‘click’ event as a ‘touch’ event anyway. I’ve searched a lot for help and have tweaked a few things in the code, but haven’t found the problem yet. Any help anyone can offer is much appreciated and enjoy a good game of Simon! If you get to 20 rounds, you win! Here’s a link to the code on Github:
GitHub - jasonfelton7/simon-game: A Simon Game with HTML, CSS and JavaScript

Thanks,
Jason

1 Like
2

Hi!
That’s really good work! I like it. I have some advices for make your code shorter and more structured.

It will be better to create a folder for sounds, it is good practice to separate files by types especially when your project (this or another) will grow up.

Your main.js file (it is usually nemed script.js) should perform a complementary role. I mean that it is better to split the logic of js: in one file You hold the functions and in another (main.js) You use these functions. For this in html-file connect the js-file with functions the first then connect main.js.

About the identical parts of code. For example, functions one(), two(), three() and four(). Try write the reusable code.
Here I can offer two ways for make it short:

function playClip(clip, color) {
    if (noise) {
        let audio = document.getElementById(clip);
        audio.play();
      }
      noise = true;
      topLeft.style.backgroundColor = color;
}
playClip('clip1', 'lightgreen');
playClip('clip2', 'tomato');
playClip('clip3', 'lyellow');
playClip('clip4', 'lightskyblue');

or

const clips = {
    one: {
        name: 'clip1',
        color: 'lightgreen'
    },
    two: {
        name: 'clip2',
        color: 'tomato'
    },
    three: {
        name: 'clip3',
        color: 'lyellow'
    },
    four: {
        name: 'clip4',
        color: 'lightskyblue'
    }
};

function playClip(clip) {
    if (noise) {
        let audio = document.getElementById(clips[clip]['name']);
        audio.play();
      }
      noise = true;
      topLeft.style.backgroundColor = color[clip]['color'];
}

playClip('one');
playClip('two');
playClip('three');
playClip('four');

And last, the UX and UI of you app is really great, so, show it in the README.md file. You can add a screenshot or use devices mockups. Mockup is the image looks like real device. You can show your app on it’s screen by changing default screen image. It is easy to do in PhotoShop or Figma.

I hope it was useful for You. I can’t say anything helpful about your problem with eventListeners :pensive:, but I have the same problem in Safari on my Mac.

1 Like
3

Thanks for the advice @liubowolkova ! Great ideas! I made some changes to reduce the repeated code. Instead of the functions one(), two(), three() and four(), I created one playClip() function like you suggested. I had add some logic into gameTurn() to make sure the right color lights when each of the four corners lights up.

function gameTurn() { on = false; if (flash == turn) { clearInterval(intervalId); compTurn = false; clearColor(); on = true; } if (compTurn) { clearColor(); setTimeout(() => { if (order[flash] == 1) { playClip("clip1", "lightgreen"); topLeft.style.backgroundColor = "lightgreen"; }; if (order[flash] == 2) { playClip("clip2", "tomato"); topRight.style.backgroundColor = "tomato"; }; if (order[flash] == 3) { playClip("clip3", "yellow"); bottomLeft.style.backgroundColor = "yellow"; }; if (order[flash] == 4) { playClip("clip4", "lightskyblue"); bottomRight.style.backgroundColor = "lightskyblue"; }; flash++; }, 200); } } function playClip(clip, color) { if (noise) { let audio = document.getElementById(clip); audio.play(); } noise = true; }

Making this change shaved off about 12 lines of code! :grinning_face_with_smiling_eyes: I also added a screenshot to my README file. I still need to figure out the performance issues on mobile though. Thanks for your help!

4

Great!
But it looks like you’ve forgotten to push your commits, README.md is steel previous version on GitHub.

There is unusable parameter in playClip() function - color. You cut the background setting from this function, so, this parameter is not needed anymore.

Let’s look at this:

if (order[flash] == 1) {
playClip("clip1", "lightgreen");
topLeft.style.backgroundColor = "lightgreen";
};
if (order[flash] == 2) {
playClip("clip2", "tomato");
topRight.style.backgroundColor = "tomato";
};
if (order[flash] == 3) {
playClip("clip3", "yellow");
bottomLeft.style.backgroundColor = "yellow";
};
if (order[flash] == 4) {
playClip("clip4", "lightskyblue");
bottomRight.style.backgroundColor = "lightskyblue";
};

This part of code can look like more readable:

if (order[flash] == 1) {
    playClip("clip1", "lightgreen");
    topLeft.style.backgroundColor = "lightgreen";
} else if (order[flash] == 2) {
    playClip("clip2", "tomato");
    topRight.style.backgroundColor = "tomato";
} else if (order[flash] == 3) {
    playClip("clip3", "yellow");
    bottomLeft.style.backgroundColor = "yellow";
} else {
    playClip("clip4", "lightskyblue");
    bottomRight.style.backgroundColor = "lightskyblue";
}

If you prefer using if and if-else statements that’s ok. You can also use switch-case statement here:

switch (order[flash]) {
    case 1: 
        playClip("clip1", "lightgreen");
        topLeft.style.backgroundColor = "lightgreen";
        break;
    case 2: 
        playClip("clip2", "tomato");
        topRight.style.backgroundColor = "tomato";
        break;
    case 3:
        playClip("clip3", "yellow");
        bottomLeft.style.backgroundColor = "yellow";
        break;
    default:
        playClip("clip4", "lightskyblue");
        bottomRight.style.backgroundColor = "lightskyblue";
        break;
}

I hope it was helpful too :slightly_smiling_face:

1 Like
5

I realize that I created a new version to experiment with the new ideas and didn’t include a link haha. Here it is:

1 Like
6

You were also right @liubowolkova! I had forgotten to push my commits as well. I changed to the if-else option to cut a little more extra code off and got rid of the parameter that wasn’t needed. Thanks again for the help!

1 Like