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

Modify: rotate #140

Open
SeanArchibald opened this issue Oct 5, 2021 · 4 comments
Open

Modify: rotate #140

SeanArchibald opened this issue Oct 5, 2021 · 4 comments
Assignees

Comments

@SeanArchibald
Copy link
Owner

Add option to rotate a scale such that one of its inner intervals becomes 1/1.

This should be really really easy now that we have functions to handle transposition and modulo

@SeanArchibald
Copy link
Owner Author

A mostly-ok Rotate implementation is already committed to the develop branch. I see 2 issues that need fixing before it's ready for release.

  1. It does a sort ascending during the process, but it would be better if it preserved any non-ascending order the user might have.
  2. Unexpected output when using commadecimal lines. For example transposeLine("2,0", negateLine("2,0")) should equal 1,000000 but instead it equals 4,000000 (I have added a new test for this one)

Anybody who wants to help test can try https://sevish.com/scaleworkshop-dev/

@SeanArchibald SeanArchibald self-assigned this Nov 4, 2021
@SeanArchibald
Copy link
Owner Author

I'm preparing to release the new Rotate feature as it is.

  1. The sort ascending issue I haven't been able to figure out, but I have added a note on the UI to let the user know their scale will be sorted. It's a "won't fix"... for now.
  2. I believe the commadecimal issue will be fixed when transposeLine() function passes all its tests. I did try to fix this myself, but every time I fixed one test another one broke. @vsicurella if you get a chance in the future I'd appreciate your guidance on this. For now, very few people are aware of commadecimal so most will never encounter the bug.

@vsicurella
Copy link
Collaborator

Mostly good news! Sorry I missed this thread until yesterday. I'll look into these points as soon as I can!

@vsicurella
Copy link
Collaborator

The commadecimal issues should be sorted out now. I'll take a closer look at the sort ascending issue and see if I can figure it out.

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

No branches or pull requests

3 participants