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

Fix parallelogram rotation #142

Merged

Conversation

Projects
None yet
3 participants
@jespirit
Copy link
Contributor

commented Nov 11, 2018

Fixes Issue #139

Will have to do more testing though.

@PopeSpaceous

This comment has been minimized.

Copy link
Owner

commented Nov 15, 2018

Awesome Job!
I'll have to test this when I get a chance.
I'll merge it after reviewing it.

@PopeSpaceous

This comment has been minimized.

Copy link
Owner

commented Nov 19, 2018

I've done some testing and the parallelogram is still flipping the same way.
parallelflip1
parallelflip2

Also I noticed that the tangrams can't be completed when the parallelogram must be flipped. See these two examples.

tangramnotsolved
tangramnotsolved2

I tested those two on the previous version and they worked there. What I'm thinking is that its having an issue trying to figure out it's orientation.

I would say, take a look at the code again and see what's happening.
Thanks.

@PopeSpaceous

This comment has been minimized.

Copy link
Owner

commented Nov 19, 2018

I'm going to close the PR, Let me know when it's ready for more testing 👍

@lejara

This comment has been minimized.

Copy link
Collaborator

commented Nov 19, 2018

This pr does not need to be close, if he add new commits on his branch jespirit:fix-parallelogram-rotation, it will automatically show in the pr

@lejara lejara reopened this Nov 19, 2018

@lejara
Copy link
Collaborator

left a comment

bump

@jespirit

This comment has been minimized.

Copy link
Contributor Author

commented Nov 24, 2018

It seems to work now. I only tested easy puzzles. This fix will have to include fixing the z-rotation values for medium and hard puzzles that flip the parallelogram.

I added the debug statements for easier testing. I can always remove them via rebase.

I won't explain how it works here. I wrote a blog post here for an in-depth explanation.

https://jespiritutech.wordpress.com/2018/11/24/parallelogram-rotation-fix/

@PopeSpaceous

This comment has been minimized.

Copy link
Owner

commented Nov 24, 2018

I'll take a look at it sometime this weekend. And i'll test the med, and hard tans too. 👍

@PopeSpaceous

This comment has been minimized.

Copy link
Owner

commented Dec 8, 2018

@jespirit I have played through the game. All the tangram puzzle can be completed with your code 👍

The only thing I noticed that might be wrong is that the flip is doing as the pictures show (below).
If this is how you think it should be we can keep it. Just let me know and I'll merge it.
Thanks again for your hard work and sorry it took so long to test.

tangram1
Tangram 2

@PopeSpaceous
Copy link
Owner

left a comment

I have tested this.
All Tangrams on all difficulty work and can be completed.
Awesome work @jespirit!

@lejara

lejara approved these changes Dec 8, 2018

@PopeSpaceous PopeSpaceous merged commit 8c2aaef into PopeSpaceous:master Dec 8, 2018

@jespirit

This comment has been minimized.

Copy link
Contributor Author

commented Dec 8, 2018

@jespirit I have played through the game. All the tangram puzzle can be completed with your code

The only thing I noticed that might be wrong is that the flip is doing as the pictures show (below).
If this is how you think it should be we can keep it. Just let me know and I'll merge it.
Thanks again for your hard work and sorry it took so long to test.

tangram1
Tangram 2

Hmm, it should flip properly.

No worries. Last few weeks of school can be tough with projects, assignments, and exams.

I had anticipated some changes needed to be made for puzzles where the parallelogram was initially flipped. Hmm, that is interesting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.