-
Notifications
You must be signed in to change notification settings - Fork 659
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#9-1 Hong Xi - Base #137
base: main
Are you sure you want to change the base?
#9-1 Hong Xi - Base #137
Conversation
return `Player 1 Wins. Press submit to restart new game.`; | ||
} else { | ||
console.log(`P1: ${p1Number}. P2: ${p2Number}.`); | ||
gameMode = `rollingDice`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'gameMode' is not defined.
console.log(`P1: ${p1Number}. P2: ${p2Number}.`); | ||
gameMode = `rollingDice`; | ||
return `Player 1 Wins. Press submit to restart new game.`; | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary 'else' after 'return'.
if (gameMode == `checkWinner`) { | ||
if (p1Number > p2Number) { | ||
console.log(`P1: ${p1Number}. P2: ${p2Number}.`); | ||
gameMode = `rollingDice`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'gameMode' is not defined.
} | ||
} | ||
//Check Winners and loop game | ||
if (gameMode == `checkWinner`) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'gameMode' is not defined.
gameMode = `checkWinner`; | ||
console.log(`Locked in Reverse ${revCombineP2}`); | ||
console.log( | ||
`Current mode: ${gameMode}. Current player: ${currentPlayer}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'gameMode' is not defined.
Code Climate has analyzed commit c58bd76 and detected 35 issues on this pull request. Here's the issue category breakdown:
Note: there are 20 critical issues. View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall good use of conditionals! To improve I'd always prepare for multiple players (more than 2) when I start coding. This would help when the code needs to e refactored.
//Player 1 roll | ||
var p1Roll = [diceRoll(), diceRoll()]; | ||
//Player 2 roll | ||
var p2Roll = [diceRoll(), diceRoll()]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting choice of rolling the dice once the page loads!
I think for the purposes of this 2 player game it works! However always prepare for multiplayer so refactoring won't be such a headache . Heh!
//Starting game mode | ||
gameMode = `rollingDice`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing out var
when declaring gameMode :P
Please fill out the survey before submitting the pull request. Thanks!
🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀
How many hours did you spend on this assignment?
3
Please fill in one error and/or error message you received while working on this assignment.
What part of the assignment did you spend the most time on?
Comfort Level (1-5):
4
Completeness Level (1-5):
4
What did you think of this deliverable?
Is there anything in this code that you feel pleased about?