make alt + up/down work on all screens

[woeps]

Fix #562.

Imlement jumpToNextSection in all sections.

Remarks

I'm not sure how to optimize the (somehow repetetive) code to reduce binary size. Using generics could at least reduce line of code in source code, but I'm not sure about the compiled binary. Any suggestions?

Since this is my very first attempt to modify the picoTracker code base, I'm thankfull for any advice.

Current State

• Song View (was already implemented, might get a little adapted, outlined in make alt + up/down work on all screens #562 (comment))
• Chain View
• Phrase View
• Table View

[maks]

@woeps thanks for working on this!
I've left a few minor comments, but in general your code style and approach is fine, but I would suggest holding off doing any more work on this until we've had a chance to agree to the details of the functionality we want implemented in the related issue.

[maks]

there is no need to change this as this is just a placeholder value that is replaced in GitHub CI (Actions)

[maks]

👍🏻

[maks]

this is fine to change in your local test builds but please dont commit this into the PR branch

[woeps]

Should I remove the commit changing this completely from the commit-history in the PR branch? Or is reverting it sufficient? (e.g. if PRs are being squashed anyways)

[maks]

just reverting or a new commit putting the old line back is fine since yes we do squash merge PRs so the commits in a PR branch arent kept anyways

[woeps]

I encountered a strange rendering, when the cursor is in the Phrase View and on one of both params columns. After jumpToNextSection the cursor is rendered twice on screen (previous position and result of the jump) until the next cursor movement.
I'm trying to understand if I did something wrong or if this surfaced a potential bug. What is strange about this is, that it works fine in the other columns and only happens when the cursor is in one of the param columns.

In this part of jumpToNextSection the handling for param1 column is done, basically just calculating the correct value for the current variable:

picoTracker/sources/Application/Views/PhraseView.cpp

Lines 392 to 408 in e937e70

	// Param1 column
	case 3: {
	for (int i = 0; i < PHRASE_ROW_COUNT; i++) {
	ushort param = phrase_->param1_[phraseStart + current];
	if (foundGap && param != 0) {
	break;
	} else if (param == 0) {
	foundGap = true;
	}
	current += direction;
	if (current < 0)
	current += PHRASE_ROW_COUNT;
	if (current >= PHRASE_ROW_COUNT)
	current -= PHRASE_ROW_COUNT;
	}
	break;
	}

Finally PhraseView::row_ is set to current and PhraseView::isDirty_ is set to true:

picoTracker/sources/Application/Views/PhraseView.cpp

Lines 454 to 456 in e937e70

	// update view
	row_ = current;
	isDirty_ = true;

[maks]

I encountered a strange rendering, when the cursor is in the Phrase View and on one of both params columns. After jumpToNextSection the cursor is rendered twice on screen (previous position and result of the jump) until the next cursor movement. I'm trying to understand if I did something wrong or if this surfaced a potential bug. What is strange about this is, that it works fine in the other columns and only happens when the cursor is in one of the param columns.

In this part of jumpToNextSection the handling for param1 column is done, basically just calculating the correct value for the current variable:

picoTracker/sources/Application/Views/PhraseView.cpp

Lines 392 to 408 in e937e70

	// Param1 column
	case 3: {
	for (int i = 0; i < PHRASE_ROW_COUNT; i++) {
	ushort param = phrase_->param1_[phraseStart + current];
	if (foundGap && param != 0) {
	break;
	} else if (param == 0) {
	foundGap = true;
	}
	current += direction;
	if (current < 0)
	current += PHRASE_ROW_COUNT;
	if (current >= PHRASE_ROW_COUNT)
	current -= PHRASE_ROW_COUNT;
	}
	break;
	}

Finally PhraseView::row_ is set to current and PhraseView::isDirty_ is set to true:

picoTracker/sources/Application/Views/PhraseView.cpp

Lines 454 to 456 in e937e70

	// update view
	row_ = current;
	isDirty_ = true;

Hmm @woeps sorry dont have time to look into this in detail just now but this sounds vaguely like what I fixed in #595 for the Table screen

[woeps]

I'm not expecting anybody to solve my issues. I just hoped to get a hint if something looks familiar.

Thank you @maks .