Switch PianoKeyboard between Notated Pitch (effective pitch) and Playback Pitch (completed).

[rpatters1]

Resolves: #15594

Supercedes: #22991, which should be closed.

Added buttons in the PianoKeyboardPanelContextMenuModel class that communicate with the PianoKeyboardController class to change a boolean variable. The aim is to address a limitation in the current implementation of MuseScore's keyboard view, which struggles with differently-tuned instruments. When users click on a key, the corresponding note used to be placed on the score as notated pitch, while the equivalent standard pitch was highlighted on the keyboard UI, leading to potential confusion.

This PR also adds a change to Score::noteVal so that the correct pitch is entered into the score, based on the menu option selected.

• 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)

[rpatters1]

This PR should be ready for review now. I moved the pianoKeyboardUseNotatedPitch getter and setter from INotationConfiguration to IEngravingConfiguration to allow accessing the property inside the Score class. Everything seems to be working., though I am not currently set up to test configuration persistence across executions of MuseScore

EDIT: I tested without the -F option and it indeed preserves the setting across executions of MuseScore.

[cbjeukendrup]

I'm not sure if we should do this check here. It might be preferable to move the setting back to notationConfiguration and retrieve its value outside the engraving module (since I'd rather consider it UI logic than business logic) and pass it as a bool parameter to Score::addMidiPitch and Score::noteVal.

[rpatters1]

I don't disagree with what you say at all, but I have trouble seeing any philosophical difference between the Display Concert Pitch setting and this new one as far as to whether they are business logic or UI. If we are going to move the new one, we should move the old one as well, and that may get into a rats' nest of refactoring. (You would know better than I whether it is a lot.)

Or maybe the noteVal function should not be part of Score. But that seems like gets into even more refactoring.

[cbjeukendrup]

I would say that the difference is the scope of these settings. Sid::concertPitch is a score setting, and affects also how the score is typeset, i.e. really the content; pianoKeyboardUseNotatedPitch is a UI setting, independent of the score, affecting only the behaviour of the UI.

There are absolutely a lot more opportunities for refactoring in the Score class and things around note input... but let's save that for different PRs!

[rpatters1]

Hmm. I hope you will get to know me well enough that I sometimes when write out a counter argument, I then begin arguing against my own argument in my head and eventually convince myself that I was wrong. I think I get how Display Concert Pitch could be construed as business logic (perhaps because it controls engraving output) while there is no possibility of the input being so construed. So I will investigate your suggestion.

[cbjeukendrup]

(when injecting a Configuration from a different module, we would usually make that clear in the name, like engravingConfiguration)

[cbjeukendrup]

See also the recent discussion in #15594; the scope of the issue has been extended a little bit. Personally I prefer the two "use notated pitch" / "use sounding pitch" options over a single checkbox though, because with these options you really know what you are choosing from. I'll add that as a comment at #15594.

[rpatters1]

The boolean (potentially) needs to be passed from three locations:

• NotationMidiInput::doRealtimeAdvance
• NotationMidiInput::addNoteToScore
• NotationMidiInput::makeNote

I am passing the boolean in from the latter 2 but not the 1st (doRealtimeAdvance). The only one I have figured out how to test is addNoteToScore. Do you think they all 3 should pass in the boolean?

[rpatters1]

I agree that the UI should be in Preferences. I'll tackle that on a different commit, though it can be on this PR if you wish. I would like to nail down the PR as it was.

[cbjeukendrup]

Hm, adding a bool parameter useNotatedPitch to Score::realtimeAdvance would also look a bit weird. One way to solve that would be to get rid of that method and move its content of that method to NotationMidiInput. Perhaps that's not a bad idea anyway.

Re. how to test: when you right-click the note input button in the toolbar, you get a menu where you can choose "realtime" note input mode. I've never studied how this works, but maybe the MuseScore 3 (!) handbook is useful: https://musescore.org/en/handbook/3/note-input-modes#realtime-auto

Making the preferences changes in separate commits to this PR seems good to me.

[rpatters1]

Given that Score is business logic, I provided a name for the boolean that makes no reference to the piano keyboard: allowTransposition. Does adding that parameter to Score::realtimeAdvance seem weird? Given how it works, it doesn't seem weird to me, but if you want me to I'll look at moving. It gets called exactly once by NotationMidiInput::doRealtimeAdvance.

Perhaps allowTransposition describes more precisely what the business logic is doing than useNotatedPitch would. Basically, if Display Concert Pitch is unchecked, the business logic transposes the note(s). This new parameter says only do that when it is allowed.

[rpatters1]

I have removed the menu items from the piano keyboard and added them to the playback settings menu, as shown.

I went ahead and changed the text to simple Anglo Saxon words instead of the fancy French words that we were using before. This is consistent with prior art. (I'm assuming menu item text is not copyrighted. If so, we can change it.)

[rpatters1]

Assuming the checks pass, it looks ready for review to me. (I've reviewed the cumulative file changes and they now all look as I intended them to.) The names have all been changed to match the new menu items and location of the menu.