Skip to content
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

Tess & Malin- Project Labyrinth #198

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

codeandjazz
Copy link

@codeandjazz codeandjazz changed the title Tess & Emma - Project Labyrinth Tess & Malin- Project Labyrinth Apr 30, 2023
Copy link

@dannebrob dannebrob left a comment

Choose a reason for hiding this comment

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

Hi Tess and Malin,
Great game you have made, I think proves that you have a solid understanding of how to work with APIs and Redux. I think your code is well structured and easy to read.
I really enjoyed your loading animation, great work!

// Daniel and Carol

},
reducers: {
addPlayer: (store, action) => {
store.username = `${new Date().getTime()} ${action.payload}`

Choose a reason for hiding this comment

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

nice way to get somewhat unique username 👍

Choose a reason for hiding this comment

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

Why not use "uniqid" on the username as well? This would make the reducer more readable. But hey it works 😋

Comment on lines +3 to +4
export const ApiStart = 'https://labyrinth.technigo.io/start'
export const ApiMove = 'https://labyrinth.technigo.io/action'

Choose a reason for hiding this comment

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

A clever way to not have to repeat the https

Comment on lines +13 to +15
{loading && <Loader loaderColor="#fff" textColor="#fff" />}
{!loading && currentPosition && <GamePlay />}
{!loading && !currentPosition && <StartingPage />}

Choose a reason for hiding this comment

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

Nice use of conditional redering

Comment on lines +37 to +57
switch (currentCoordinate) {
case '1,0':
backgroundImage = FirstImage;
break;
case '1,1':
backgroundImage = SecondImage;
break;
case '0,1':
backgroundImage = ThirdImage;
break;
case '0,2':
backgroundImage = FourthImage;
break;
case '0,3':
backgroundImage = FifthImage;
break;
case '1,3':
backgroundImage = SixthImage;
break;
default:
backgroundImage = DefaultImg;

Choose a reason for hiding this comment

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

Nice way to switch backgrounds, another way to do it is to have an object with all the background images as values, with the coordinates as keys.
const backgroundImages = { '0,0': '/assets/img2.jpg', '1,0': '/assets/img3.jpg', '1,1': 'assets/img4.jpg', '0,1': 'assets/img5.jpg', '0,2': 'assets/img6.jpg', '0,3': 'assets/img7.jpg', '1,3': 'assets/img8.jpg' };

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