Skip to content

Conversation

@sanket95droid
Copy link

Used Three.js Audio section for adding audio :

  • Sound after clicking the tile
  • background music

@sanket95droid sanket95droid marked this pull request as ready for review October 23, 2022 18:50
Copy link
Owner

@KonradLinkowski KonradLinkowski left a comment

Choose a reason for hiding this comment

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

Hi, thank you for your contribution :)

That's a pretty good job, but I would like you to fix the problems that I pointed out in the review

To make it more clear:

  • make sure that no errors appear in the console
  • remove old code in the comments
  • changes to package-lock.json, package.json and yarn.lock are redundant
  • we don't need two versions of each sound mp3 is enough

@sanket95droid
Copy link
Author

@KonradLinkowski did the requested changes except for the button please review it.

@KonradLinkowski
Copy link
Owner

@sanket95droid hey, please remove changes to package-lock.json and yarn.lock I will update them manually in the next commit

@sanket95droid
Copy link
Author

sanket95droid commented Oct 25, 2022

@KonradLinkowski ok, should I also remove changes to package.json because it is also causing merge conflict or only package-lock.json and yarn.lock???

@KonradLinkowski
Copy link
Owner

@sanket95droid yes I fixed the issue you mentioned earlier about node-gyp etc.

@sanket95droid
Copy link
Author

@KonradLinkowski please compare and merge my PR!!

@KonradLinkowski
Copy link
Owner

@sanket95droid I can't merge it because you didn't resolve the conflicts.
But I added hacktoberfest-accepted it should appear as being accepted for hacktoberfest

@sanket95droid
Copy link
Author

Ohk i'll try to resolve those conflits and make a commit @KonradLinkowski

@sanket95droid
Copy link
Author

Ohk i'll try to resolve those conflits and make a commit @KonradLinkowski

@KonradLinkowski Done ✅

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants