-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
2048 the game #422
base: master
Are you sure you want to change the base?
2048 the game #422
Conversation
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.
Nice work 👍
Let's improve your code
return Math.random() < 0.9 ? 2 : 4; | ||
} | ||
|
||
function newCell() { |
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.
The condition 'clearCells.length === undefined' can be simplified to '!clearCells.length' which checks if the array is empty or not.
src/scripts/main.js
Outdated
|
||
switch (direction) { | ||
case 'ArrowUp': | ||
for (const column of columns) { |
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.
Avoid using a for...of
loop to iterate over an array. Instead, use forEach
or map
.
src/scripts/main.js
Outdated
}); | ||
} | ||
|
||
document.addEventListener('keyup', (evnt) => { |
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.
It's more efficient to listen to the keyup event only on the window or document object, not on the entire document. So you can change 'document.addEventListener' to 'window.addEventListener' or 'document.body.addEventListener'.
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.
I've decided to change it back to 'keydown'
. It feels more natural and more responsive while playing.
src/scripts/main.js
Outdated
function canBeMarged() { | ||
const wholeField = [...cellsInRow, ...columns]; | ||
|
||
for (const line of wholeField) { |
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.
Avoid using a for...of
loop to iterate over an array. Instead, use forEach
or map
.
src/scripts/main.js
Outdated
} | ||
|
||
switch (direction) { | ||
case 'ArrowUp': |
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.
Сreate a variable for these magic words
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.
case left: | ||
cellsInRow.forEach((row) => { | ||
flipLine([...row].reverse()); | ||
}); | ||
break; |
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.
Switch case should always have default case to handle possible errors
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.
Nice work 🔥
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.
Nice work 🔥
DEMO LINK
I've decided to use "keyup" event instead of "keydown" just in case to avoid continuous moves if press and hold any arrow key. Also, I`ve changed UI a little and added some sound effects for a better experience.
p.s. During development I have never won the game, maybe you will have more luck ;)