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

Rank-2 temperament line types #127

Merged

Conversation

vsicurella
Copy link
Collaborator

Here's a reimplementation of the generate rank-2 temperament feature to preserve line types when possible. I also added an output type if a user still wants to see the scale in cents (or even decimals).

You can easily end up with some crazy ratios this way, which isn't necessarily bad, but it could be useful if there was a 'Modify' option to convert intervals to cents or decimals, if for some reason you made some edits and didn't want to use the generate module again.

@vsicurella vsicurella changed the title Rank2 temperament line types Rank-2 temperament line types Sep 6, 2021
@SeanArchibald
Copy link
Owner

This is a great improvement - I will go through all your PRs some time this week. I'm looking to merge them all into develop and deploy to https://sevish.com/scaleworkshop-dev/ for testing and likely inclusion in the next release

Good point about adding a new Modify option to convert all lines to cents/commadecimal. This would be a really simple addition. I added an issue for it #128 and assigned to myself

@SeanArchibald SeanArchibald merged commit 53fe5d9 into SeanArchibald:develop Sep 11, 2021
@SeanArchibald
Copy link
Owner

Good stuff, this is all merged in and deployed on the dev instance: https://sevish.com/scaleworkshop-dev/
So this PR will make it into the next release! Thanks for sending it through.

I did make a change to simplifyRatio. Instead of x * gcdScalar I made it Math.round(x * gcdScalar) to prevent an issue I saw where simplifyRatioString("72409/31209") returned 702.9999999999999/303. Seems to have fixed it so hope it was the right approach

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.

2 participants