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

fix #47821, fix #153686: issues with voice and chords with drum input #6334

Merged
merged 1 commit into from
Aug 26, 2020

Conversation

MarcSabatella
Copy link
Contributor

Resolves: https://musescore.org/en/node/47821
Resolves: https://musescore.org/en/node/153686

When adding drum notes to a score by double-clicking in the palette,
the semantics have always been a bit odd,
and even with some bugs fixed, there have still be inconsistencies,
as well as needless limitations and outright bugs.
This fixes a number of interrelated problems,
that stem from two main issues.
The issues are conceptually separate,
but they exists in the exact same blocks of code,
so there is no practical way to separate these

  1. Shift+double-click should add note to chord, not replace

This mostly required checking for ShiftModifier in Note::drop()
and calling addNote instead of setNoteRest.
But also, it required correcting the management of the selection
and of the note input cursor, which was non-standard.
All other note input methods leave the note you just entered selected,
then move the input cursor.
Double-click was selecting the note or rest at the new input position.
That would have resulted in Shift adding to the next chord
rather than the one you just entered.
It also caused a bug in itself: entering a note before an existing one
caused playback of the existing note after the playback of the new one.
Changing the selection to stay on the new note helped,
but now it meant the new note got played twice -
once because you clicked it on the palette,
then again in endCmd because the note was selected.
Preventing the double playback was achieved
by turning off setPlayNote and setPlayChord in applyPaletteElement.
A change to that function was also needed
to make sure we use the selection when adding to the chord with Shift.
The code designed to favor the input cursor position over the selection
is now skipped when using Shift.

  1. Double-click should use the voice defined for the note in the drumset

This happened "sometimes" but it was unpredictable,
and in some of the cases where it didn't work,
it actually changed the voice of the note in the palette itself.
That turned out to be a bad side effect of a fix I made many years ago.
Removing the assignment of track of the element in applyPaletteElement
fixes that problem, then I made the corresponding change
to how we select the track in Note::drop.
But this still would leave us with the issue that the note
would be added in the wrong voice any time
there was no existing note or rest in that voice
at the current input position.
The solution to this was to borrow the code from putNote,
which is what gets used when entering a note via keyboard shortcut:
walking backwards until we find a place to enter the note.

Resolves: https://musescore.org/en/node/47821
Resolves: https://musescore.org/en/node/153686

When adding drum notes to a score by double-clicking in the palette,
the semantics have always been a bit odd,
and even with some bugs fixed, there have still be inconsistencies,
as well as needless limitations and outright bugs.
This fixes a number of interrelated problems,
that stem from two main issues.
The issues are conceptually separate,
but they exists in the exact same blocks of code,
so there is no practical way to separate these

1) Shift+double-click should add note to chord, not replace

This mostly required checking for ShiftModifier in Note::drop()
and calling addNote instead of setNoteRest.
But also, it required correcting the management of the selection
and of the note input cursor, which was non-standard.
All other note input methods leave the note you just entered selected,
then move the input cursor.
Double-click was selecting the note or rest at the new input position.
That would have resulted in Shift adding to the *next* chord
rather than the one you just entered.
It also caused a bug in itself: entering a note before an existing one
caused playback of the existing note after the playback of the new one.
Changing the selection to stay on the new note helped,
but now it meant the new note got played twice -
once because you clicked it on the palette,
then again in endCmd because the note was selected.
Preventing the double playback was achieved
by turning off setPlayNote and setPlayChord in applyPaletteElement.
A change to that function was also needed
to make sure we use the selection when adding to the chord with Shift.
The code designed to favor the input cursor position over the selection
is now skipped when using Shift.

2) Double-click should use the voice defined for the note in the drumset

This happened "sometimes" but it was unpredictable,
and in some of the cases where it didn't work,
it actually changed the voice of the note in the palette itself.
That turned out to be a bad side effect of a fix I made many years ago.
Removing the assignment of track of the element in applyPaletteElement
fixes that problem, then I made the corresponding change
to how we select the track in Note::drop.
But this still would leave us with the issue that the note
would be added in the wrong voice any time
there was no existing note or rest in that voice
at the current input position.
The solution to this was to borrow the code from putNote,
which is what gets used when entering a note via keyboard shortcut:
walking backwards until we find a place to enter the note.
@anatoly-os anatoly-os merged commit 0059c79 into musescore:3.x Aug 26, 2020
anatoly-os added a commit that referenced this pull request Aug 26, 2020
To both old and new version of the drumset note input interactor
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.

None yet

2 participants