Skip to content

Module #2 (Omar)#26

Open
riyadomf wants to merge 6 commits intorecruit-org:module-2from
riyadomf:module-2
Open

Module #2 (Omar)#26
riyadomf wants to merge 6 commits intorecruit-org:module-2from
riyadomf:module-2

Conversation

@riyadomf
Copy link
Copy Markdown

No description provided.

pages/index.js Outdated
// reset the game if the snake hit itself or hit poison
if (isSnake(newHead) || isPoison(newHead)) {
resetGame();
return getDefaultSnake();
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any particular reason to keep 'getDefaultSnake()' here instead of resetting the snake state inside 'resetGame()' ?
and how the snake state is resetted here without using setSnake() ?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you have a setSnake on line number 185 and providing a function inside that. now whatever you return from this function will be the new snakes value. that's why returning getDefaultSnake() resets the snake without any setSnake (actually there is a setSnake)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is more convenient to reset the snake inside resetGame function but we are already inside a setState and need to return a value there, otherwise, the newValue of the snake state will be undefined. If anyone else need to consume the resetSnake function, then it is a must to reset the snake inside the function.

pages/index.js Outdated
setPoisons((currentPoisons) =>
currentPoisons.filter((poison) => Date.now() - poison.createdAt <= 10 * 1000)
);
}, [])
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of seperate removeFoods and removePoisons we could create one removeObjects and remove any food/poison that lasted for 10s

pages/index.js Outdated
const addFood = useCallback(() => {
let newFood = getRandomCell();
while (isSnake(newFood) || isFood(newFood)) {
while (isSnake(newFood) || isFood(newFood) || isPoison(newFood)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an object can be put to a cell only if it isn't occupied yet.
why not create a reusable function like

const isOccupied =  (cell) => isSnake(cell) || isFood(cell) || isPoison(cell);

pages/index.js Outdated
// reset the game if the snake hit itself
if (isSnake(newHead)) {
// reset the game if the snake hit itself or hit poison
if (isSnake(newHead) || isPoison(newHead)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, poor snake! dies on one unit of poison!

Well, this works, there is nothing wrong with your approach. But you could improve it by decreasing the snake size by one after getting hit by poison, otherwise, the game would be extremely difficult for the player.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants