rock/paper/scissors game outputting random results and I have no idea why-CodePudding

I want my buttons to interact with JavaScript, so I added event listeners to them. The weird thing is when for example: I click a button 'rock' and the output is 'You win! Rock beats scissors' but I don't get a point. I will get a point if I click a random button again. Next example: Computer wins and computer doesn't get a point and let's say next click is a draw and then computer gets the point. I have no idea why.

const gameElements = ['rock', 'paper', 'scissors'];
function computerPlay() {
  let elementsRun =  gameElements[Math.floor(Math.random()*gameElements.length)];
  return elementsRun;
}

let playerScore = document.querySelector('.playerScore');
let computerScore = document.querySelector('.computerScore');
let playRounds = ''
let compRounds = ''

function singleRound(playerSelection, computerSelection) {
  if (playerSelection === computerSelection) {
    return "It's a draw!"
  } else if (playerSelection === 'rock' && computerSelection === 'scissors') {
    playRounds   ;
    return "You win! Rock beats scissors"
  } else if (playerSelection === 'paper' && computerSelection == 'rock') {
    playRounds   ;
    return "You win! Paper beats rock"
  } else if (playerSelection === 'scissors' && computerSelection == 'paper') {
    playRounds   ;
    return "You win! Scissors beat paper"
  } else if (playerSelection === 'scissors' && computerSelection === 'rock') {
    compRounds   ;
    return "You lose! Rock beat scissors"
  } else if (playerSelection == 'paper' && computerSelection == 'scissors') {
    compRounds   ;
    return "You lose! Scissors beat paper"
  } else if (playerSelection === 'rock' && computerSelection === 'paper') {
    compRounds   ;
    return "You lose! Paper beat rock"              
  }
}
            
const buttonRock = document.querySelector('.rock')
buttonRock.addEventListener('click', function() {
  computerSelection = computerPlay();
  playerScore.innerHTML = 'Player:'   playRounds;
  computerScore.innerHTML = 'Computer:'   compRounds;
  const result = singleRound('rock', computerSelection)
  console.log(result)
});

const buttonPaper = document.querySelector('.paper')
buttonPaper.addEventListener('click', function() {
  computerSelection = computerPlay();
  playerScore.innerHTML = 'Player:'   playRounds;
  computerScore.innerHTML = 'Computer:'   compRounds;
  const result = singleRound('paper', computerSelection)
  console.log(result)  
});

const buttonScissors = document.querySelector('.scissors')
buttonScissors.addEventListener('click', function() {
  computerSelection = computerPlay();
  playerScore.innerHTML = 'Player:'   playRounds;
  computerScore.innerHTML = 'Computer:'   compRounds;
  const result = singleRound('scissors', computerSelection)
  console.log(result)  
});

<div >
  <div >
    <div >
      <button ><span>Rock</span></button>
      <span >Player:</span>
    </div>
    <div >
      <button ><span>Paper</span></button>
      <span >Winner</span>
    </div>
    <div >
      <button ><span>Scissors</span></button>
      <span >Computer:</span>
    </div>
  </div>
</div>

CodePudding user response：

There's a lot going on here. You've got a lot of unnecessary variables and functions.

But basically you are updating the scores when a selection is made and not after comparing it to the computer to see who won.

See the comments inline below:

const gameElements = ['rock', 'paper', 'scissors'];
let playerScore = document.querySelector('.playerScore');
let computerScore = document.querySelector('.computerScore');
let playRounds = 0; // Needs to be initialized as a number
let compRounds = 0; // Needs to be initialized as a number
const output = document.getElementById("output");

// Since each of the buttons does almost the same thing, there's no need
// to create 3 functions. Instead, let the event bubble up to a common
// ancestor and handle it there.
document.addEventListener('click', function(event) {
  // Then, test to see if the event originated at an element we
  // care about (event.target)
  if(event.target.classList.contains("choice")){
    output.textContent = singleRound(event.target.textContent.toLowerCase(), computerPlay());
  }
});

function computerPlay() {
  // There's no need to set up a variable if you are only going to use it once.
  return gameElements[Math.floor(Math.random()*gameElements.length)];
}

function singleRound(playerSelection, computerSelection) {
  // Instead of returning at each branch, just record the result in a variable
  let result = "";
  if (playerSelection === computerSelection) {
    result =  "It's a draw!"
  } else if (playerSelection === 'rock' && computerSelection === 'scissors') {
    playRounds  ;
    result =  "You win! Rock beats scissors"
  } else if (playerSelection === 'paper' && computerSelection == 'rock') {
    playRounds  ;
    result =  "You win! Paper beats rock"
  } else if (playerSelection === 'scissors' && computerSelection == 'paper') {
    playRounds  ;
    result =  "You win! Scissors beat paper"
  } else if (playerSelection === 'scissors' && computerSelection === 'rock') {
    compRounds  ;
    result =  "You lose! Rock beat scissors"
  } else if (playerSelection == 'paper' && computerSelection == 'scissors') {
    compRounds  ;
    result =  "You lose! Scissors beat paper"
  } else if (playerSelection === 'rock' && computerSelection === 'paper') {
    compRounds  ;
    result =  "You lose! Paper beat rock"              
  }
  // Now that we know the outcome, update the screen
  
  // Don't use .innerHTML when the text you are working with isn't HTML
  // as .innerHTML has security and performance issues. Instead, use
  // .textContent
  playerScore.textContent = 'Player: '   playRounds;
  computerScore.textContent = 'Computer: '   compRounds;
  return result;
}

<div >
  <div >
    <div >
      <!-- Button elements are already inline elements.
           Wrapping their contents inside of a span is redundant. 
           
           Also, if we give each button the same class, we can 
           isolate only those buttons later. And we don't need
           unique class names to know what the button represents.
           We can just look at the .textContent of the button and
           convert it to lower case to compare against the computer's
           selection. -->
      <button >Rock</button>
      <span >Player:</span>
    </div>
    <div >
      <button >Paper</button>
      <span >Winner</span>
    </div>
    <div >
      <button >Scissors</button>
      <span >Computer:</span>
    </div>
  </div>
  <div id="output"></div>
</div>

CodePudding user response：

Your issue is that you're not updating the InnerHTML value of the playerScore and the computerScore elements after updating the playRounds and compRounds variables. Instead, you are only updating the innerHTML after the user clicks a button, which happens before the score is updated.

In Javascript, when you do something like:

element.InnerHTML = someVariable

If someVariable changes, the value of element.InnerHTML does not change with the variable, it stays the same as it was when you initially set it. You need to call element.InnerHTML = someVariable again after updating someVariable in order to see it updated on the page.

For your case, try adding another function updateScore() which will update the innerHTML of each of your elements after every call to singleRound().

Here's how I updated your Javascript, notice how updateScore() is called each time we play a round. I also adjusted the function so that there is only one return, which is the message to be displayed. This is just a best practice-type thing, but something to keep in mind where you can.

const gameElements = ['rock', 'paper', 'scissors'];
function computerPlay() {

    let elementsRun = gameElements[Math.floor(Math.random() * gameElements.length)];
    return elementsRun;
}


let playerScore = document.querySelector('.playerScore');
let computerScore = document.querySelector('.computerScore');
let playRounds = ''
let compRounds = ''

function singleRound(playerSelection, computerSelection) {
    let msg;
    if (playerSelection === computerSelection) {
        msg = "It's a draw!";
    }
    else if (playerSelection === 'rock' && computerSelection === 'scissors') {
        playRounds  ;
        msg = "You win! Rock beats scissors";
    }
    else if (playerSelection === 'paper' && computerSelection == 'rock') {
        playRounds  ;
        msg = "You win! Paper beats rock";
    }
    else if (playerSelection === 'scissors' && computerSelection == 'paper') {
        playRounds  ;
        msg = "You win! Scissors beat paper";
    }


    else if (playerSelection === 'scissors' && computerSelection === 'rock') {
        compRounds  ;
        msg = "You lose! Rock beat scissors";
    }
    else if (playerSelection == 'paper' && computerSelection == 'scissors') {
        compRounds  ;
        msg = "You lose! Scissors beat paper";
    }
    else if (playerSelection === 'rock' && computerSelection === 'paper') {
        compRounds  ;
        msg = "You lose! Paper beat rock";
    }
    updateScore();
    return msg;
}

function updateScore() {
    playerScore.innerHTML = 'Player:'   playRounds;
    computerScore.innerHTML = 'Computer:'   compRounds;
}

const buttonRock = document.querySelector('.rock')
buttonRock.addEventListener('click', function () {
    computerSelection = computerPlay();
    const result = singleRound('rock', computerSelection)
    console.log(result)

});

const buttonPaper = document.querySelector('.paper')
buttonPaper.addEventListener('click', function () {
    computerSelection = computerPlay();
    const result = singleRound('paper', computerSelection)
    console.log(result)
});

const buttonScissors = document.querySelector('.scissors')
buttonScissors.addEventListener('click', function () {
    computerSelection = computerPlay();
    const result = singleRound('scissors', computerSelection)
    console.log(result)
});