9
\$\begingroup\$

I am a newbie coder and I'm trying to learn good coding habits. I'm making a Simon Says game challenge from Free Code Camp in Angular JS.

Codepen

I have an array that keeps random integers between 1-4:

var simonSays = [];

This is how I'm currently animating and playing sounds. I've read that you're not to manipulate the DOM using Angular. But how would I handle it for my game?

var counter = 0, //for looping thru simon's array
    innerTimeOutSecs = 1000, //time during each iteration
    outerTimeOutSecs = 500, //time before next iteration
    simonTurn = false; //if simon is playing, player cannot interrupt simon

function animateSimonSays() {
    simonTurn = true;
    $timeout(function() {
        var simon = simonSays[counter];
        //find the current pad and current audio to light up/play
        var currentPad = document.getElementById("pad" + simon);
        var currentAudio = document.getElementById("pad" + simon + "_audio");
        angular.element(currentPad).addClass("lightUpPad");
        currentAudio.play();

        $timeout(function() {
            currentAudio.pause(); //stop audio
            currentAudio.currentTime = 0;
            angular.element(currentPad).removeClass("lightUpPad"); //'turn off' pad

            //get ready for the next simon
            counter++;
            if (counter < simonSays.length) { //recursion fail condition
                animateSimonSays();
            } else {
                counter = 0;
                simonTurn = false;
            }
        }, innerTimeOutSecs); //how long to stay 'lit'
    }, outerTimeOutSecs); //how long before the next simon
}

It's dirty but it works for my app. All the examples I found online were jQuery so I couldn't really follow their methodology.

var app = angular.module('App', []);

app.controller('MainCtrl', ['$scope', '$timeout', function($scope, $timeout) {
  var simonSays = [], //push what simon says
    copy = [], // compare user answer to simon answer
    counter = 0, //for looping thru simon's array
    innerTimeOutSecs = 1000, //time during each iteration
    outerTimeOutSecs = 500, //delay in resetting to original state of pad and call next pad
    simonTurn = false, //if simon is playing, play cannot interrupt simon
    currentResponse = true; //used to compare user answer to simon

  $scope.count = 0; //number of rounds
  $scope.isStrict = false; //is strict mode on/off

  $scope.startGame = function() { //start button calls restart
    restart();
  }

  function restart() { //initialize game by setting values in array to empty and start a new round
    simonSays = [];
    $scope.count = 0;
    newRound();
  }

  function newRound() { //begin next round 
    if ($scope.count === 20) { //victory alert if player reaches 20 steps
      alert("Congratulations. You win!");
      restart();
      return;
    }
    $scope.count++; //increase round by one
    if ($scope.count % 5 === 0) {
      innerTimeOutSecs -=100;
      outerTimeOutSecs -=50;
    }
    simonSays.push(Math.floor(Math.random() * 4 + 1)); //add random step to the end
    copy = simonSays.slice(0);
    animateSimonSays();
  }

  function animateSimonSays() { //light works and sound 

    //TODO: speedup animations after every 5 'count'
    simonTurn = true; //simon is playing, user cannot play function again by pressin start
    $timeout(function() { //outertimeout

      var simon = simonSays[counter]; //setting up iteration of array
      var currentPad = document.getElementById("pad" + simon); //setting up light show for each 
      var currentAudio = document.getElementById("pad" + simon + "_audio"); //setup audio
      angular.element(currentPad).addClass("lightUpPad"); //opacity change
      currentAudio.play(); //audio starts

      $timeout(function() { //inner timeout
        currentAudio.pause(); //stop sound
        currentAudio.currentTime = 0; //set audio back to start position
        angular.element(currentPad).removeClass("lightUpPad"); //return to original opacity 

        counter++; //setting up next iteration in array 

        if (counter < simonSays.length) { //test for recursiveness
          animateSimonSays(); //calling recursive function for next iteration
        } else { //if counter reaches end..
          counter = 0; //reset counter
          simonTurn = false; //simon turn over
        }
      }, innerTimeOutSecs); //how long it stays lit

    }, outerTimeOutSecs); //next pad

  }

  //three possible situations
  //1.user click wrong pad --> call animate again (last round)
  //2.user click right pad but seq not finish --> do nothing
  //3.user pick right pad and end of seq --> new round
  function checkResponse() {
    if (!currentResponse) { // case 1
      var buzzer = document.getElementById("buzzer");
      buzzer.play();

      if ($scope.isStrict) { //strict mode restarts game
        $timeout(function() {
          buzzer.pause();
          buzzer.currentTime = 0;
          restart();
        }, 1000);
        return;
      }

      $timeout(function() {
        buzzer.pause();
        buzzer.currentTime = 0;
        copy = simonSays.slice(0);
        animateSimonSays();
      }, 1000);

    } else if (currentResponse && copy.length === 0) { //case 3
      $timeout(newRound, 1000);

    }
  }

  $scope.registerClick = function(padId) {
    if (simonTurn) return; //click does not register during simon turn
    //var currentPad = document.getElementById("pad" + padId);
    var currentAudio = document.getElementById("pad" + padId + "_audio");
    currentAudio.play();

    $timeout(function() {
      currentAudio.pause();
      currentAudio.currentTime = 0;
    }, 500);

    var desiredResponse = copy.shift(); //pop out first index of the array
    var actualResponse = padId;
    currentResponse = desiredResponse === actualResponse; //compare click and simon
    checkResponse();
  }

}]);
.wrapper {
  position: relative;
  width: 640px;
  margin: 0 auto;
}

.back {
  position: absolute;
  top: 170px;
  width: 640px;
  height: 640px;
  z-index: 0;
  background-color: #000;
  border-radius: 310px;
}

.pad {
  width: 300px;
  height: 300px;
  float: left;
  z-index: 1;
  margin: 10px;
  -ms-filter: "progid:DXImageTransform.Microsoft.Alpha(Opacity=60)";
  filter: alpha(opacity=60);
  opacity: 0.6;
  cursor: pointer;
}

.pad:active {
  -ms-filter: "progid:DXImageTransform.Microsoft.Alpha(Opacity=100)";
  filter: alpha(opacity=100);
  opacity: 1;
}

.shape1 {
  border-top-left-radius: 300px;
  background-color: green;
}

.shape2 {
  float: left;
  border-top-right-radius: 300px;
  background-color: red;
  clear: right;
}

.shape3 {
  float: left;
  border-bottom-left-radius: 300px;
  background-color: yellow;
  clear: left;
}

.shape4 {
  float: left;
  border-bottom-right-radius: 300px;
  background-color: blue;
}

.circle {
  position: absolute;
  top: 195px;
  left: 195px;
  width: 250px;
  height: 250px;
  background: #000;
  border-radius: 125px;
  z-index: 10;
}

.simon {
  text-align: center;
  color: white;
  font-family: Impact;
  font-size: 5em;
  margin-top: 25px;
}

.startButton {
  width: 25px;
  height: 25px;
  border-radius: 50%;
  background: red;
  margin-left: 25%;
  margin-top: 10px;
}

.strictButton {
  width: 25px;
  height: 25px;
  border-radius: 50%;
  background: #ffff00;
  margin-top: 8px;
  margin-left: 10px;
}

.strictIndicator {
  height: 7px;
  width: 7px;
  background-color: #32050C;
  border-radius: 50%;
  margin-left: 20px;
  margin-top: -5px;
}

.strictIndicator-on {
  background-color: #DC0D29;
}

.lightUpPad {
  opacity: 1;
}

.count {
  text-align: center;
  width: 60px;
  height: 50px;
  background-color: #32050C;
  border-radius: 10px;
  margin-left: 15px;
  color: #DC0D29;
  font-size: 2.5em;
  border: 2px solid #222;
}

.controllabel {
  text-align: center;
  text-transform: uppercase;
  color: #fff;
}

.countlabel {
  margin-left: 15px;
}

.buttonlabel {
  margin-top: 13px;
}

.controls {
  float: left;
  margin-left: 13px;
}

.strictlabel {
  margin-top: 13px;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/angular.js/1.5.7/angular.min.js"></script>

<audio preload="auto" id="pad1_audio"><source src="https://s3.amazonaws.com/freecodecamp/simonSound1.mp3" type="audio/mp3"></audio>
<audio preload="auto" id="pad2_audio"><source src="https://s3.amazonaws.com/freecodecamp/simonSound2.mp3" type="audio/mp3"></audio>
<audio preload="auto" id="pad3_audio"><source src="https://s3.amazonaws.com/freecodecamp/simonSound3.mp3" type="audio/mp3"></audio>
<audio preload="auto" id="pad4_audio"><source src="https://s3.amazonaws.com/freecodecamp/simonSound4.mp3" type="audio/mp3"></audio>
<audio preload="auto" id="buzzer"><source src="http://soundbible.com/mp3/Basketball Buzzer-SoundBible.com-1863250611.mp3" type="audio/mp3"></audio>

<div ng-app="App" ng-controller="MainCtrl">
  <div class="wrapper">
    <div class="back">
      <div id="pad1" class="pad shape1" ng-click="registerClick(1)">
      </div>

      <div id="pad2" class="pad shape2" ng-click="registerClick(2)">
      </div>

      <div id="pad3" class="pad shape3" ng-click="registerClick(3)">
      </div>

      <div id="pad4" class="pad shape4" ng-click="registerClick(4)">
      </div>

      <div class="circle">
        <div class="simon">simon</div>
        <div class="controls">
          <div class="count">{{count}}</div>
          <div class="controllabel countlabel">COUNT</div>
        </div>
        
        <div class="controls">
          <button class="startButton" ng-click="startGame()"></button>
          <div class="controllabel buttonlabel">START</div>
        </div>
        
        <div class="controls">
          <div ng-class="isStrict ? 'strictIndicator-on' : ''" class="strictIndicator"></div>
          <button class="strictButton" ng-click="isStrict = !isStrict"></button>
          <div class="controllabel strictlabel">STRICT</div>
        </div>
      </div>
    </div>

  </div>
</div>

\$\endgroup\$
4
  • \$\begingroup\$ could you include the html? \$\endgroup\$
    – Pimgd
    Commented Jul 1, 2016 at 8:18
  • \$\begingroup\$ @Pimgd Working codepen is here: codepen.io/sharonghae/pen/dXOprE \$\endgroup\$ Commented Jul 1, 2016 at 11:11
  • \$\begingroup\$ then I cant review, code has to be in the question due to legal issues \$\endgroup\$
    – Pimgd
    Commented Jul 1, 2016 at 11:24
  • 1
    \$\begingroup\$ @Pimgd I added the full code into the question. \$\endgroup\$ Commented Jul 1, 2016 at 11:53

1 Answer 1

2
+100
\$\begingroup\$

Your controller could store a "padToLightUp" value. Then use ng-class={'lightUpPad': padToLightUp == 1} (where 1 is the pad number, so it could be 1, or 2, 3, 4...). This gets rid of the need to find a pad in the dom to light up.

Similarily, you could store the audio references all in an array, so that you can hook a selectPad function to the pad's ng-click, in which you play the audio file.

Lastly, instead of wrapping everything in timeout, make them actual functions that stand separately. Then you can use $timeout(lightUpNextPad), which means that the use of timeout affects your code structure far less.


//1.user click wrong pad --> call animate again (last round)
//2.user click right pad but seq not finish --> do nothing
//3.user pick right pad and end of seq --> new round
function checkResponse() {
  if (!currentResponse) { // case 1
    var buzzer = document.getElementById("buzzer");
    buzzer.play();

    if ($scope.isStrict) { //strict mode restarts game
      $timeout(function() {
        buzzer.pause();
        buzzer.currentTime = 0;
        restart();
      }, 1000);
      return;
    }

    $timeout(function() {
      buzzer.pause();
      buzzer.currentTime = 0;
      copy = simonSays.slice(0);
      animateSimonSays();
    }, 1000);

  } else if (currentResponse && copy.length === 0) { //case 3
    $timeout(newRound, 1000);

  }
}

This function is rather messy - there are three cases, but the comments only mention 2. And the random return in the middle there seems like weird flow. Let's fix it.

First, replace the return with else.

//1.user click wrong pad --> call animate again (last round)
//2.user click right pad but seq not finish --> do nothing
//3.user pick right pad and end of seq --> new round
function checkResponse() {
  if (!currentResponse) { // case 1
    var buzzer = document.getElementById("buzzer");
    buzzer.play();

    if ($scope.isStrict) { //strict mode restarts game
      $timeout(function() {
        buzzer.pause();
        buzzer.currentTime = 0;
        restart();
      }, 1000);
    } else {
      $timeout(function() {
        buzzer.pause();
        buzzer.currentTime = 0;
        copy = simonSays.slice(0);
        animateSimonSays();
      }, 1000);
    }
  } else if (currentResponse && copy.length === 0) { //case 3
    $timeout(newRound, 1000);
  }
}

Next, you have if(!currentResponse){ ... } else if(currentResponse && ... - that second check for currentResponse is not needed.

//1.user click wrong pad --> call animate again (last round)
//2.user click right pad but seq not finish --> do nothing
//3.user pick right pad and end of seq --> new round
function checkResponse() {
  if (!currentResponse) { // case 1
    var buzzer = document.getElementById("buzzer");
    buzzer.play();

    if ($scope.isStrict) { //strict mode restarts game
      $timeout(function() {
        buzzer.pause();
        buzzer.currentTime = 0;
        restart();
      }, 1000);
    } else {
      $timeout(function() {
        buzzer.pause();
        buzzer.currentTime = 0;
        copy = simonSays.slice(0);
        animateSimonSays();
      }, 1000);
    }
  } else if (copy.length === 0) { //case 3
    $timeout(newRound, 1000);
  }
}

You have multiple cases in your code where you have to "reset an audio element". This is done by calling pause and setting it's currentTime to 0.

You should make a separate function for that:

function stopAudio(audioElement) {
  audioElement.pause();
  audioElement.currentTime = 0;
}

Then you can use it for buzzer.

function checkResponse() {
  if (!currentResponse) { // case 1
    var buzzer = document.getElementById("buzzer");
    buzzer.play();

    if ($scope.isStrict) { //strict mode restarts game
      $timeout(function() {
        stopAudio(buzzer);
        restart();
      }, 1000);
    } else {
      $timeout(function() {
        stopAudio(buzzer);
        copy = simonSays.slice(0);
        animateSimonSays();
      }, 1000);
    }
  } else if (copy.length === 0) { //case 3
    $timeout(newRound, 1000);
  }
}
\$\endgroup\$

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.