As explained in the title, for some reason the first time you click it doesn't work but the second click goes through and the game works fine onwards, can anyone explain why? can't figure it out with my noob brain:
The game logic works perfectly fine, I know I've gone about it in a roundabout way but forgive me I'm still a noob and plan to keep iterating on this to make it more efficient.
Am I applying the styling to display the results in the wrong place or can you point me to why its not instantly starting the game?
const pickIcon = document.getElementsByClassName('icons');
const pickWord = document.getElementsByClassName('word-choice');
const rockWord = document.getElementById('rock-word');
const paperWord = document.getElementById('paper-word');
const scissorsWord = document.getElementById('scissors-word');
// const resultContainer = document.getElementById('result');
let picksDisplay = document.getElementById('picks');
let resultDisplay = document.getElementById('winner-result');
let resultShow = document.getElementById('result-game');
let userPickIcon = ''
let userPick = ''
let compPick = ''
function playGame() {
//player pick
for (let i = 0; i < pickIcon.length; i ) {
pickIcon[i].addEventListener('click', function() {
document.getElementById('result').style.display = 'block';
if (pickIcon[i] === pickIcon[0] || pickIcon[i] === pickIcon[3]) {
userPickIcon = 'rock'
picksDisplay.innerText = `Player picked ${userPickIcon}`
} else if (pickIcon[i] === pickIcon[1] || pickIcon[i] === pickIcon[4]) {
userPickIcon = 'paper'
picksDisplay.innerText = `Player picked ${userPickIcon}`
} else if (pickIcon[i] === pickIcon[2] || pickIcon[i] === pickIcon[5]) {
userPickIcon = 'scissors'
picksDisplay.innerText = `Player picked ${userPickIcon}`
}
})
}
// computer randomised pick
let computerChoice = Math.floor(Math.random() * 3);
if (computerChoice === 0) {
compPick = 'rock'
resultDisplay.innerText = `Computer picked ${compPick}`
} else if (computerChoice === 1) {
compPick = 'paper'
resultDisplay.innerText = `Computer picked ${compPick}`
} else if (computerChoice === 2) {
compPick = 'scissors'
resultDisplay.innerText = `Computer picked ${compPick}`
}
//gamelogic
if (userPickIcon === compPick) {
resultShow.innerText = `The game is a tie!!!`
} else {
if (userPickIcon === 'rock') {
if (compPick === 'paper') {
resultShow.innerText = `Computer is the winner, Sorry :(`
} else {
resultShow.innerText = `The Player is the winner, Congratulations!!`
}
};
if (userPickIcon === 'paper') {
if (compPick === 'scissors') {
resultShow.innerText = `Computer is the winner, Sorry :(`
} else {
resultShow.innerText = `The Player is the winner, Congratulations!!`
}
};
if (userPickIcon === 'scissors') {
if (compPick === 'rock') {
resultShow.innerText = `Computer is the winner, Sorry :(`
} else {
resultShow.innerText = `The Player is the winner, Congratulations!!`
}
}
}
}
document.addEventListener('click', playGame);
@import url('https://fonts.googleapis.com/css2?family=Montserrat:wght@400;500&family=Roboto Mono&display=swap');
body {
background-color: #16213E;
font-family: 'Montserrat', sans-serif;
text-align: center;
}
.container {
background-color: #fff;
margin: 40px auto;
width: 500px;
color: #16213E;
}
.icons-div {
display: flex;
align-items: center;
justify-content: center;
}
i {
padding: 25px;
font-size: 70px;
color: #277BC0;
}
i:hover {
color: #1CD6CE;
cursor: pointer;
}
.word-choice:hover {
color: #1CD6CE;
cursor: pointer;
}
.title-div {
background-color: #277BC0;
padding: 10px 0;
color: #fff;
}
.results {
background-color: #fff;
width: 500px;
margin: 40px auto;
height: 200px;
display: none;
}
.result-items {
padding-top: 20px;
color: #16213E;
}
#result-game {
color: #495C83;
}
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.2.0/css/all.min.css" integrity="sha512-xh6O/CkQoPOWDdYTDqeRdPCVd1SpvCA9XXcUnZS2FmJNp1coAFzvtCN9BmamE 4aHK8yyUHUSCcJHgXloTyT2A==" crossorigin="anonymous" referrerpolicy="no-referrer"
/>
<div >
<div >
<h2>Rock, Paper, Scissors</h2>
</div>
<div >
<h2>Make your choice</h2>
</div>
<div >
<table>
<tr>
<td><i id="rock-btn"></i></td>
<td><i id="paper-btn"></i></td>
<td><i id="scissors-btn"></i></td>
</tr>
<tr>
<td>
<h3 id="rock-word" >Rock</h3>
</td>
<td>
<h3 id="paper-word" >Paper</h3>
</td>
<td>
<h3 id="scissors-word" >Scissors</h3>
</td>
</tr>
</table>
</div>
</div>
<div id="result">
<div >
<h3 id="picks"></h3>
<h3 id="winner-result"></h3>
<h2 id="result-game"></h2>
</div>
</div>
CodePudding user response:
You added an eventListener
to the whole document:
document.addEventListener('click', playGame);
Which then calls the function playGame
if anything is clicked. playGame
function adds the eventListeners
to your rock, paper and scissors only after you made your first click that called playGame
.
CodePudding user response:
As I stated already in the comments, there are a lot of issues. I not going to fix every issue such as semantics and accessibility issues as this would blow the proportion of an answer (take it as a hint to read into it and work on it in the future).
- You had a text and an icon that could be clicked. This means while you still have only 3 choices to make you had 6 elements that you could click on. This overcomplicated the necessary logic. In other words, you had twice as many elements to check than necessary. I change it now that the
eventListener
listen to the container that contains an icon and the text. - You selected all the elements to check for the
eventListener
by usinggetElementsById
. This unfortunately returns an HTML Collection Object. As such you had to iterate through that object with afor
-loop. This is highly inefficient and overcomplicated the issue as well. The smart and clean solution is to usequerySelectorAll
which will return a Node-List. The NodeList can be iterated withforEach
which works faster and is shorter than afor
-loop. - You nested a lot of
if/else
-statements. This is inefficient which aswitch
-statement and/or a ternary conditional operator can fix.
Now to the actual issue:
You have the entire logic running in a function. The eventListener
is inside the function. So you have to click in your document first to run the function that then will start listing to the click events of the choices. That's why you need the extra click at the start.
If you programmed clean as described above you have the EventListener
at the front and start the function when one of the choices is clicked on. That's what I meant in the comments, if you fix your major issues with your script performance-wise, the actual issue will resolve itself.
document.querySelectorAll('button').forEach(el =>
el.addEventListener('click', function(e) {
let playerChoice = e.currentTarget.id;
let userPick = '';
let computerPick = '';
// Get User Pick
switch (playerChoice) {
case 'rock-btn':
userPick = 'Rock';
break;
case 'paper-btn':
userPick = 'Paper';
break;
case 'scissors-btn':
userPick = 'Scissors';
break;
};
document.querySelector('#picks').textContent = `Player picked ${userPick}`;
// computer randomised pick
let computerChoice = Math.floor(Math.random() * 3);
switch (computerChoice) {
case 0:
computerPick = 'Rock';
break;
case 1:
computerPick = 'Paper';
break;
case 2:
computerPick = 'Scissors';
break;
};
document.querySelector('#winner-result').textContent = `Computer picked ${computerPick}`;
// game logic to check who wins;
let result = '';
if (userPick === computerPick) {
result = 'draw';
} else {
switch (userPick) {
case 'Rock':
result = (computerPick === 'Scissors') ? 'win' : 'lose';
break;
case 'Paper':
result = (computerPick === 'Rock') ? 'win' : 'lose';
break;
case 'Scissors':
result = (computerPick === 'Paper') ? 'win' : 'lose';
break;
};
};
// output for winner
let resultText = '';
switch (result) {
case 'draw':
resultText = 'The game is a tie!!!';
break;
case 'win':
resultText = 'The Player is the winner, Congratulations!!';
break;
case 'lose':
resultText = 'The Computer is the winner, Sorry :(';
break;
};
document.querySelector('#result-game').textContent = resultText;
// makes the result box visible
document.querySelector('#result').classList.remove('d-none');
})
)
@import url('https://fonts.googleapis.com/css2?family=Montserrat:wght@400;500&family=Roboto Mono&display=swap');
body {
background-color: #16213E;
font-family: 'Montserrat', sans-serif;
text-align: center;
}
.container {
background-color: #fff;
margin: 40px auto;
width: 500px;
color: #16213E;
}
.icons-div {
display: flex;
align-items: center;
justify-content: center;
}
i {
padding: 25px;
font-size: 70px;
color: #277BC0;
}
button {
border: none;
background: transparent;
}
.icons-div > button:hover,
.icons-div > button:hover i,
.icons-div > button:hover h3 {
color: #1CD6CE;
cursor: pointer;
}
.title-div {
background-color: #277BC0;
padding: 10px 0;
color: #fff;
}
.results {
background-color: #fff;
width: 500px;
margin: 40px auto;
height: 200px;
}
.result-items {
padding-top: 20px;
color: #16213E;
}
.d-none {
display: none;
}
#result-game {
color: #495C83;
}
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.2.0/css/all.min.css" integrity="sha512-xh6O/CkQoPOWDdYTDqeRdPCVd1SpvCA9XXcUnZS2FmJNp1coAFzvtCN9BmamE 4aHK8yyUHUSCcJHgXloTyT2A==" crossorigin="anonymous" referrerpolicy="no-referrer"
/>
<div >
<div >
<h2>Rock, Paper, Scissors</h2>
</div>
<div >
<h2>Make your choice</h2>
</div>
<div >
<button id="rock-btn">
<i ></i>
<h3 >Rock</h3>
</button>
<button id="paper-btn">
<i ></i>
<h3 >Paper</h3>
</button>
<button id="scissors-btn">
<i ></i>
<h3 >Scissors</h3>
</button>
</div>
</div>
<div id="result">
<div >
<h3 id="picks"></h3>
<h3 id="winner-result"></h3>
<h2 id="result-game"></h2>
</div>
</div>