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

B2ShapeCast #21

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

B2ShapeCast #21

wants to merge 3 commits into from

Conversation

Alexander-r
Copy link
Contributor

Perform a linear shape cast of shape B moving and shape A fixed.
Determines the hit point, normal, and translation fraction.

Ported from upstream.

Perform a linear shape cast of shape B moving and shape A fixed.
Determines the hit point, normal, and translation fraction.
@Alexander-r Alexander-r requested a review from xtuc February 9, 2020 18:09
@xtuc
Copy link
Member

xtuc commented Feb 11, 2020

Could you please add a few test cases? Ideally the same than upstream

@Alexander-r
Copy link
Contributor Author

I will look into adding tests. This was a rather recent addition so I have not checked how to use it.

But right now on go 1.13.7 all the tests fail because of Printf formatting. Maybe I should fix the formatting for existing tests first?

@netgusto
Copy link
Member

Hey @Alexander-r I went ahead and proposed a PR fixing the issue you put forward on go 1.13. Thanks for doing so!

@netgusto
Copy link
Member

I merged #24 into master. Tests should be good on go 1.13 now if you rebase.

@Alexander-r
Copy link
Contributor Author

Would something like that be enough for a test?
This is based on https://github.com/erincatto/box2d/blob/master/samples/tests/shape_cast.cpp but without actual drawing.

My C++ version for comparing the output:
https://gist.github.com/Alexander-r/937980a0bf7f04d8335db71f17b65d1d

Copy link
Member

@xtuc xtuc left a comment

Choose a reason for hiding this comment

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

thanks

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.

3 participants