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.

Sign up for GitHub

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

Jump to bottom

Add cross-product and angle #57

Merged
merged 7 commits into from
Oct 11, 2018
Merged

Add cross-product and angle #57

merged 7 commits into from
Oct 11, 2018

Conversation

nbraud
Copy link
Collaborator

@nbraud nbraud commented Oct 10, 2018

No description provided.

@nbraud
Copy link
Collaborator Author

nbraud commented Oct 10, 2018

I used this in mutant-games, but didn't yet add tests. Feel free to push directly commits to the feature branch to do so.

nbraud added 2 commits October 10, 2018 01:01
@nbraud
Add tests for angle()
d40005b
@nbraud
Vector2.angle: Use dot-product rather than cross-product
c0d0adb 
Avoids an issue where opposing vectors get an angle of 0 instead of 180.
@nbraud
Copy link
Collaborator Author

nbraud commented Oct 10, 2018

PS: I added a test for angle(), which uncovered a bug, and I fixed it.
This probably still needs a test for a ^ b.

pathunstrom
pathunstrom previously requested changes Oct 10, 2018
View reviewed changes
Copy link
Collaborator

@pathunstrom pathunstrom left a comment

Choose a reason for hiding this comment

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

The only thing I'm blocking on is the assert in non-test code.

All the rest of the comments can be turned into issues later.

README.md Outdated Show resolved Hide resolved
ppb_vector/vector2.py Outdated Show resolved Hide resolved
tests/test_vector2_rotate.py Show resolved Hide resolved
@nbraud
Copy link
Collaborator Author

nbraud commented Oct 11, 2018

@pathunstrom Thanks for the feedback, this should all be handled now.

FYI, you can automatically squash my fixup commits with git rebase -i --autosquash master before merging (or I can do that once you have reviewed the new changes)

@AstraLuma AstraLuma merged commit 7118259 into ppb:master Oct 11, 2018
@nbraud nbraud deleted the cross branch October 11, 2018 03:39
@AstraLuma AstraLuma mentioned this pull request Oct 13, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AstraLuma AstraLuma AstraLuma left review comments

@pathunstrom pathunstrom pathunstrom left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nbraud @AstraLuma @pathunstrom