Skip to content

Conversation

@Ash-86
Copy link
Contributor

@Ash-86 Ash-86 commented Nov 19, 2025

Resolves: #27905

Allows T to work on list selections outside note entry. Works only on single chord selections as shown below.
(Would be cool to add multiple staves support later on)

Recording.2025-11-18.221523.mp4

Another option would be to add a tied note to the next chord without overwriting.

Third option would be to only allow cases where next chordRest is rest only. (see discussion in #27905)

@Ash-86 Ash-86 force-pushed the Allow-T-to-work-on-list-selection-outside-note-entry branch 2 times, most recently from 89a5b2a to be62a67 Compare November 19, 2025 01:38
Comment on lines 2185 to 2188
if (!tieNote && selection().isList() && sameChord) {
Tie* tie = nullptr;

if (sameChord) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking sameChord twice

And tie seems an unnecessary variable

It would be nice to move this outside for (size_t i = 0;… loop

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@Ash-86 Ash-86 force-pushed the Allow-T-to-work-on-list-selection-outside-note-entry branch from be62a67 to 03a3056 Compare November 19, 2025 19:35
@Ash-86 Ash-86 force-pushed the Allow-T-to-work-on-list-selection-outside-note-entry branch from b7d3101 to cd5a17a Compare November 19, 2025 19:46
std::vector<Note*> tieNoteList(notes);
const bool shouldTieListSelection = notes >= 2;
Note* tieNote = nullptr;
Note* n = nullptr;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n is only used for getting the Chord. So instead of extending the scope of n, let's add a Chord* chord variable, that's initialised as noteList[0]->chord(); then you can reuse that variable in the std::all_of predicate.

}
}

if (!tieNote && selection().isList() && sameChord) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now the logic has become the following:
if for the last selected note no tieable note was found, then cmdAddTie.

But I suppose the intention was:
if for any selected note [...].

I must say the meaning of the condition is not immediately clear in the way it's written right now. Is it supposed to be "If all selected notes are in the same chord, and for any one of them there is no existing note to tie to, then add new tied notes"? It might help to add a comment or introduce a descriptively named variable.

Copy link
Contributor Author

@Ash-86 Ash-86 Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're absolutely right! I shouId've made it clearer, but I intentionally left the condition a bit loose so we could discuss and agree on the ideal behavior before finalizing it. Once we settle on what makes the most sense, I can go ahead and refine the logic and add clarity through comments or better variable naming.

edit: The original behavior sent (i.e with the block inside the iteration loop) was: "If all selected notes are in the same chord, and for any one of them there is no existing note to tie to, then add new tied notes overwriting the next Chord"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like this:

    Chord* chord = noteList.front()->chord();
    bool someHaveExistingNoteToTieTo = false; // replaces `canAddTies`
    bool allHaveExistingNextNoteToTieTo = true;
    for (size_t i = 0; i < noteList.size(); ++i) {
        Note* n = noteList[i];
        if (chord && n->chord() != chord) {
            chord = nullptr;
        }
        if (n->tieFor()) {
            tieNoteList[i] = nullptr;
        } else {
            Note* tieNote = searchTieNote(n);
            tieNoteList[i] = tieNote;
            if (tieNote) {
                someHaveExistingNextNoteToTieTo = true;
            } else {
                allHaveExistingNextNoteToTieTo = false;
            }
        }
    }

    if (chord /* i.e. all notes are in the same chord */ && !allHaveExistingNextNoteToTieTo) {
         cmdAddTie(…

Some variables there are probably strictly speaking redundant, but they do increase clarity of the intention.

I would say, let's try that, and then send it to QA/design team to see what they think.

@Ash-86 Ash-86 force-pushed the Allow-T-to-work-on-list-selection-outside-note-entry branch 2 times, most recently from 2929da6 to f5422bb Compare November 21, 2025 00:19
@Ash-86 Ash-86 force-pushed the Allow-T-to-work-on-list-selection-outside-note-entry branch from f5422bb to 08c8ba0 Compare November 21, 2025 00:20
@avvvvve
Copy link

avvvvve commented Nov 21, 2025

Found a crash:

  1. Make a range selection of a single chord (easiest: select a measure containing a whole note chord)
  2. Press T
  3. Crash

It doesn't crash if you list-select the same chord. In 4.6.3, using T on this type of range selection does nothing, but we could make it so it has the same new effect as pressing T on a list selection (the resulting selection after T could still be a list-selection of the new notes).

Screen.Recording.2025-11-21.at.9.00.33.AM.mov

@avvvvve
Copy link

avvvvve commented Nov 21, 2025

Re: the question of what should happen if the next beat contains something already

So if this is my selection before I press T...
image

Option 1
...upon pressing T, right now the next chord is overwritten, like this:
image

Option 2
...but instead, it could just create a tied note and add it to the chord:
image

This would allow you to keep pressing T if you wanted to create a sustained note amongst changing chords:
image

@Ash-86 Ash-86 force-pushed the Allow-T-to-work-on-list-selection-outside-note-entry branch 3 times, most recently from 2c27084 to 988b7c3 Compare November 21, 2025 18:39
@Ash-86
Copy link
Contributor Author

Ash-86 commented Nov 21, 2025

Alright,

  • Crash fixed
  • behavior now follows "option 2"
  • single tick range selections have same effect as list selection
  • I also made some extra changes to support multi-measure-single-tick selections.

Would it be intereseting/useful to remove the same tick limitation? There might be a conflict with selections with 2-(or more)-same-track-non-consequtive-same-pitched notes, though.

@Ash-86 Ash-86 force-pushed the Allow-T-to-work-on-list-selection-outside-note-entry branch from 988b7c3 to 6bc967f Compare November 21, 2025 18:41
@avvvvve
Copy link

avvvvve commented Nov 24, 2025

Thank you! I think this is good to go as-is, but leaving some notes below for posterity.

If we completely lose the ability to tie non-consecutive-same-pitch notes by removing the same-tick limitation, I don't think we should do it. We should keep things that are possible now possible.

Also just wanted to note here that if you have a range selection of some notes that are tied and some that aren't, pressing T toggles each tie to be off if it was on and on if it was off (this is the same behavior as toggling accidentals).

Screen.Recording.2025-11-24.at.6.00.11.PM.mov

This works differently from how, say, articulations work (i.e. if I have 2 notes with staccatos and 2 without in a selection, one press of Shift+T will add staccatos to the notes that didn't already have it, leaving the existing ones intact, then a second press of Shift+T de-staccatos all the notes). But I don't think it's a problem that these work differently.

Screen.Recording.2025-11-24.at.6.16.14.PM.mov

@avvvvve
Copy link

avvvvve commented Nov 24, 2025

Actually after playing with it a bit more, I think we should revert the behavior I asked for to add notes to existing chords. It's simpler if T just overwrites everything in its path, for a few reasons:

  1. T in note input mode still overwrites everything
  2. If the chord is a different length than the selected note, the new note will match it, but then the next tied note is still the same length as the first one you selected
  3. Notes still get erased if they start within the length of a note you're adding via T

(Point 2)

Screen.Recording.2025-11-24.at.6.38.12.PM.mov

(Point 3)

Screen.Recording.2025-11-24.at.6.35.24.PM.mov

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.

pressing T to add tied note [outside note-input mode]

4 participants