tvillarete / ipod-classic-js

An iPod Classic emulator that connects to Apple Music and Spotify. Built with React & Styled Components
http://tannerv.com/ipod
MIT License
1.45k stars 107 forks source link

[Brick Game] fix bouncing of the ball on the player #74

Closed nicosemp closed 3 years ago

nicosemp commented 3 years ago

fix: the ball now bounces off the player at an angle

fix: the ball now inverts x/y direction instead of setting to 1 or -1

fix: missing parenthesis in the Brick's fill color switch statement

p.s. sorry about all the edits I'm still learning github PRs

vercel[bot] commented 3 years ago

Someone is attempting to deploy a commit to a Personal Account owned by @tvillarete on Vercel.

@tvillarete first needs to authorize it.

vercel[bot] commented 3 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/tvillarete/ipod-classic-js/6oUHbWCAGY2CjLMPHDwZdtrcm6kZ
✅ Preview: https://ipod-classic-js-git-fork-nicosemp-master-tvillarete.vercel.app

nicosemp commented 3 years ago

This is fantastic – thanks for doing this @nicosemp!

You are very welcome! I really like this project.

One small thing I noticed was that the velocity of the ball is just slightly slower in these new changes. Ideally we should keep the velocity the same as the original. What are your thoughts?

I thought you might notice this. As you probably know, the length of the vector is the velocity. In your original file, the velocity is sqrt(2) because you define both the X and Y speed to be 1 or -1.

The velocity I used is exactly 1, just to make the math simpler. I made a quick (and ugly) drawing to demonstrate:

image

In the original version the vector length ends up being sqrt(2) (about 1.41), which is fairly larger than 1.

The solution

These variables are just implicitly multiplied by 1:

const vx = Math.cos(angle);
const vy = Math.sin(angle);

So if you want to keep the original speed you can just multiply by sqrt(2) instead of 1:

const vx = Math.cos(angle) * Math.sqrt(2);
const vy = Math.sin(angle) * Math.sqrt(2);

In case you do, remember to also change the starting speed both in the Ball class constructor and in its reset function!

tvillarete commented 3 years ago

Thanks for the great explanation! Seems simple enough to change later on, so I think we can leave the speed you've set in this PR for now and I'll file a followup PR with some velocity tweaks (and maybe a victory screen? 😄)

Again, thanks a bunch for these improvements!

nicosemp commented 3 years ago

It was a pleasure! Thank you for merging :)