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

20998: Added a midi mapping for 'Enter rest'; improved rest entry in 'Re-pitch existing notes' mode #26377

Merged

Conversation

krasko78
Copy link
Contributor

@krasko78 krasko78 commented Feb 7, 2025

Resolves: #20998

Adds a midi mapping for the "Enter rest" ui action which is also tied to the "Enter rest" shortcut (normally 0 and Num 0).

I think I am seeing a couple of issues with the "Enter rest" action in general:

  • In Rhythm only input mode, "Enter rest" acts as "Toggle rest". Is this expected? Does "Enter rest" make any sense in this mode at all?
  • More importantly, in Re-pitch only mode "Enter rest" enters a rest but changes the duration to the currently selected duration instead of retaining the whatever duration the underlying note has (similar to "Input by note name" mode). I don't think this is correct. Personally I enter my scores in Rhythm only mode, then Re-pitch only mode. During the first phase, I'd enter a rest as a note being interested only in its duration, and then during the second phase I'd change the note to a rest but I'd not expect the duration to change.

Thoughts?

  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)

@krasko78
Copy link
Contributor Author

See also the discussion in the issue itself.

@avvvvve
Copy link

avvvvve commented Feb 26, 2025

@krasko78 Just so I understand correctly, the only change to input modes is that in 'Re-pitch existing notes' mode, pressing 0 now replaces the current note with a rest that matches its duration rather than the duration selected in the note input bar.

Before, it did this:

nightly-repitch-rests.mov

Now, it does this:

pr-repitch-rests.mov

Definitely an improvement!


Looks good on my end but @DmitryArefiev would appreciate if you can give this a test

@avvvvve avvvvve changed the title 20998: Added a midi mapping for "Enter rest" 20998: Added a midi mapping for 'Enter rest'; improved rest entry in 'Re-pitch existing notes' mode Feb 26, 2025
@avvvvve
Copy link

avvvvve commented Feb 26, 2025

Update the title to describe the entry mode change.

Also re:

In Rhythm only input mode, "Enter rest" acts as "Toggle rest". Is this expected? Does "Enter rest" make any sense in this mode at all?

As discussed in other linked issues, this is expected.

@krasko78
Copy link
Contributor Author

@avvvvve That's all correct. :)

@DmitryArefiev DmitryArefiev removed their request for review February 27, 2025 09:26
@DmitryArefiev DmitryArefiev self-assigned this Feb 27, 2025
@DmitryArefiev
Copy link
Contributor

@krasko78 Hi! Can you rebase please?

@krasko78 krasko78 force-pushed the 20998-AddMidiMappingForEnterRest branch from 5370c50 to 556a2a9 Compare February 27, 2025 11:32
@krasko78
Copy link
Contributor Author

@DmitryArefiev Done. Thank you!

@DmitryArefiev
Copy link
Contributor

Tested #20998 on Win10, Mac13.6, LinuxUbuntu24.04.2 LTS with NI M32 MIDI keyaboard - PASS

@krasko78 Thank you!

@RomanPudashkin RomanPudashkin merged commit 892dc51 into musescore:master Feb 28, 2025
11 checks passed
@krasko78 krasko78 deleted the 20998-AddMidiMappingForEnterRest branch February 28, 2025 20:12
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.

MIDI remote control: add the "Enter rest" action
4 participants