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

FIXES ISSUE #4018 More default EDOs for temperament #4023

Closed
wants to merge 1 commit into from

Conversation

omsuneri
Copy link
Contributor

@omsuneri omsuneri commented Oct 2, 2024

after the addition of some new temperament the define frequency block used in the define temperament block uses the division approach to calculate the frequency instead as per the requirement we have to use the exponential of 2 format for more understanding.
temperament-test-2-2024-10-01_14.04.51.zip

changes the define temperament block's frequency selection from division to exponential format
@@ -1774,9 +1774,11 @@ function TemperamentWidget() {
for (let i = 0; i < this.pitchNumber; i++) {
const idx = newStack.length;
if (
this.inTemperament === "equal" ||
(this.inTemperament === "equal" || this.inTemperament === "equal5" ||
Copy link
Member

Choose a reason for hiding this comment

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

I forget how to do this in JS, but why not check to see that the first 5 letters are "equal" (begins with).

Copy link
Contributor Author

@omsuneri omsuneri Oct 4, 2024

Choose a reason for hiding this comment

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

@walterbender ohh so you want me to reduce it to check if the string starts with equal must follow the if otherwise else

@walterbender
Copy link
Member

Yes... it will be easier to read and maintain.

@omsuneri
Copy link
Contributor Author

omsuneri commented Oct 4, 2024

@walterbender okay I'll make a new PR with desired changes

@omsuneri omsuneri closed this Oct 4, 2024
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