fix #280365: holding shift and entering notes with piano doesn't form chord #4473
Conversation
|
just an FYI, this commit seems to cause problem described in https://musescore.org/en/node/280715#comment-881692 And also fyi, you should have tested for cr to be non-NULL before dereferencing it...it cased a separate crash, which I've fixed in a separate pending PR: #4508 |
| Note* n = toChord(cr)->findNote(ev.pitch); | ||
| if (n) { | ||
| deleteItem(n->tieBack()); | ||
| deleteItem(n); |
There was a problem hiding this comment.
this line deleteItem(n) is what causes problems with https://musescore.org/en/node/280715 as far as I can tell. Removing it fixes that problem, while still allowing me to hold shift and enter notes with piano to form chord. I'm trying to figure out the reason why you would delete the actual previous note...
There was a problem hiding this comment.
maybe you meant to delete something else here related to the note? But I can't seem to figure out a reason why delete the note. I'm submitting a PR to remove this line...
There was a problem hiding this comment.
I see...I believe you wanted to prevent the situation of having multiple noteheads for identical pitches appear in the same chord. In that case, this deleteItem(n) should only happen if that n is in the same chordRest as being inputted currently (which happens when shift is held down, but is not the case when shift is not held down).
There was a problem hiding this comment.
I'm only going to delete that note if Shift is held. I think that is the intended behavior.
Hmm...not quite right....let me look closer....
There was a problem hiding this comment.
I think I figured it out...PR emminent.
A prior PR musescore#4473 (holding shift and entering notes with piano doesn't form chord) broke the ability to input identical midi pitches sequentially. That was because that PR did handling to ensure that identical pitches weren't inputted into the same chord by deleting the currently-existing note that had the same pitch. That deleting makes sense if user is holding Shift while inputting in the virtual midi piano keyboard, but that deleting doesn't make sense if user wasn't holding Shift. My commit only deletes the note if the Shift key is held. There is also an addition step to check if there are any remaining notes in the last segment of the input state, because if that deleted note was the only note in the chord, then it's deletion would have meant there is no longer a chord.
A prior PR musescore#4473 (holding shift and entering notes with piano doesn't form chord) broke the ability to input identical midi pitches sequentially. That was because that PR tried handling to ensure that identical pitches weren't inputted into the same chord by deleting the currently-existing note that had the same pitch. That deleting was problematic for a few reasons discussed in chat... This fix keeps the code simple by not worrying about deleting when input the same pitch via Shift + midi input.
A prior PR musescore#4473 (holding shift and entering notes with piano doesn't form chord) broke the ability to input identical midi pitches sequentially. That was because that PR tried handling to ensure that identical pitches weren't inputted into the same chord by deleting the currently-existing note that had the same pitch. That deleting was problematic for a few reasons discussed in chat... This fix keeps the code simple by not worrying about deleting when input the same pitch via Shift + midi input.
Resolves: https://musescore.org/en/node/280365