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

Tuning temperaments plugin fix #26257

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

Conversation

Ash-86
Copy link
Contributor

@Ash-86 Ash-86 commented Jan 27, 2025

I had accidentally rebsed the branch in the old pr, and it closed automatically after i tried resetting. Couldnt reopen so i opened this one instead - sorry :(

  • The "annotation" option is fixed.
  • There are currently 3 tuning/temperaments plugins (tuning, modal tuning, and modal temperaments). They are the same plugin with different lists of temperaments. I merged them into a single plugin. The temperamentes are now organized by tab for western and middle eastern temperaments.
  • replaced broken "save" functionality with "add/remove temperament".
  • updated ui with MU components.
  • removed superflous redo/undo functions and cleaned up code.

@Ash-86
Copy link
Contributor Author

Ash-86 commented Jan 27, 2025

output.mp4

@Jojo-Schmitz
Copy link
Contributor

rebase needed

@Ash-86 Ash-86 force-pushed the Tuning-temperaments-plugin branch from 73ee4ed to 4fd4cb2 Compare February 20, 2025 16:45
@avvvvve
Copy link

avvvvve commented Feb 26, 2025

Thank you for taking this on, @Ash-86! I have some design feedback just based on what you showed in the video. Let me know if you have any questions about this or if anything seems too tricky to implement!

  1. Move Add/Remove buttons vertically in line with the list on the left. Right now, I expected those buttons to add/remove something on the right side of the panel because of where they are. Instead, let's move them above the left column. You could basically do what we have in the Instruments panel and have just an "Add" button and the trash can icon for remove (no arrows needed)

    image
  2. Rather than showing a warning when trying to "Remove" a built-in temperament, instead if a built-in temperament is selected, the "Remove" button should just be disabled. Only enable it when a user-created temperament is selected.

  3. "Annotate" could use a clearer title. Perhaps "Show tunings above notes in score". It could also use some more padding from the box above it—try 12px.

  4. Make all text sentence case, not title case (except for the actual names of the temperaments in the left column):

    • Western temperaments
    • Middle Eastern temperaments
    • Root note
    • Pure tone
    • Pure tone offset
    • Final offsets
  5. "OK" button at the bottom should say "Apply"

  6. Instead of Undo/Redo, we should add a 'Reset' button only for built-in temperaments if you've edited them (shown in the upper right corner of the right side, disabled if not yet edited).

  7. The 'Add' popup should just have 12px of space between the text field and 'OK', and 'OK' should be capitalized. Ideally, pressing Enter would confirm what you've written while you're still in the text field.

All of that would come together a bit like this (sorry, not a fully polished design, just edited over a screenshot, and I didn't replace all the title case with sentence case):

image

image
Old plugin screenshot from 4.4.4 for anyone looking for context: image

@avvvvve
Copy link

avvvvve commented Feb 26, 2025

I'm also seeing a functional issue maybe to do with removing Save/Load. If you add a new temperament, then change some of the values, the values do not save after hitting "Ok" and reopening the dialog.

I would expect that if I edit those values, they get saved to my custom temperament when I hit "OK".

(Didn't record this, but in the video I close and reopen the plugin via the menu)

Screen.Recording.2025-02-26.at.5.36.51.PM.mov

@Ash-86
Copy link
Contributor Author

Ash-86 commented Feb 27, 2025

Hi @avvvvve, I implemented most of the above and i have a couple of questions.

Regarding item 6, currently, temperament values are reset when a temperament is clicked. I think that was even the original behaviour. Considering that, you still prefer to add a reset button?

I'm also seeing a functional issue maybe to do with removing Save/Load.

RIght. I didn't implement autosave for changes when hitting 'OK' since you can just add a new temperament with the new changes and sometimes you wouldn't want to save whatever experimental tweaking you did. That said, if you think it is better, I could add a save button or have it automatically save temperament changes when hitting 'OK'.

Oh, and i the popup dialog just refused to have the same width as the add button and ended up slightly wider. Any tip on that would be great :)

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

Successfully merging this pull request may close these issues.

3 participants