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.

Sign up for GitHub

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

Improve feedback stretch markers #1150

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Improve feedback stretch markers #1150

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1910ba3
Add feedback when inserting stretch markers if Report time movement d…
ScottChesworth Aug 27, 2024
1b3a74d
Add translator comments
ScottChesworth Aug 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 46 additions & 12 deletions src/reaper_osara.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3809,17 +3809,47 @@ void cmdDeleteTimeSig(Command* command) {
outputMessage(translate("time signature deleted"));
}

void cmdRemoveStretch(Command* command) {
MediaItem* item = GetSelectedMediaItem(0, 0);
if (!item)
return;
MediaItem_Take* take = GetActiveTake(item);
if (!take)
void cmdhAddOrRemoveStretch(int command) {
const int itemCount = CountSelectedMediaItems(nullptr);
if (itemCount == 0) {
outputMessage(translate("no selected items"));
return;
int count = GetTakeNumStretchMarkers(take);
Main_OnCommand(41859, 0); // Item: remove stretch marker at current position
if (GetTakeNumStretchMarkers(take) != count)
outputMessage(translate("stretch marker deleted"));
}
int oldCount = 0;
for (int i = 0; i < itemCount; ++i) {
MediaItem* item = GetSelectedMediaItem(nullptr, i);
Copy link
Owner

Choose a reason for hiding this comment

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

Just to clarify, these commands definitely insert stretch markers across all selected items? I guess that makes sense and we just never supported this properly, but I just wanted to check.

MediaItem_Take* take = GetActiveTake(item);
if (!take)
continue;
oldCount += GetTakeNumStretchMarkers(take);
}
Main_OnCommand(command, 0);
int newCount = 0;
for (int i = 0; i < itemCount; ++i) {
MediaItem* item = GetSelectedMediaItem(nullptr, i);
MediaItem_Take* take = GetActiveTake(item);
if (!take)
continue;
newCount += GetTakeNumStretchMarkers(take);
}
int difference = newCount - oldCount;
if (newCount >= oldCount) {
if (shouldReportTimeMovement()) {
// Translators: Reported when one or more stretch markers are added. {} will be replaced by the number of stretch markers, EG "2 stretch markers added".
outputMessage(format(
translate_plural("{} stretch marker added", "{} stretch markers added", difference),
difference));
}
} else {
// Translators: Reported when one or more stretch markers are removed. {} will be replaced by the number of stretch markers, EG "2 stretch markers removed".
outputMessage(format(
Copy link
Owner

Choose a reason for hiding this comment

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

Is it intentional that we respect shouldReportTimeMovement for addition, but not removal? I guess I kinda get this - you might well add them while playing, but you're less likely to remove them while playing, since you have to move to the marker first anyway - but I wanted to check because it does feel asymmetric.

translate_plural("{} stretch marker removed", "{} stretch markers removed", -difference),
-difference));
}
}

void cmdAddOrRemoveStretch(Command* command) {
cmdhAddOrRemoveStretch(command->gaccel.accel.cmd);
}

void cmdClearTimeLoopSel(Command* command) {
Expand Down Expand Up @@ -4273,6 +4303,7 @@ void cmdRemoveFocus(Command* command) {
cmdhRemoveItems(40006); // Item: Remove items
break;
case FOCUS_MARKER:

Copy link
Owner

Choose a reason for hiding this comment

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

Extraneous blank line.

cmdDeleteMarker(nullptr);
break;
case FOCUS_REGION:
Expand All @@ -4282,7 +4313,7 @@ void cmdRemoveFocus(Command* command) {
cmdDeleteTimeSig(nullptr);
break;
case FOCUS_STRETCH:
cmdRemoveStretch(nullptr);
cmdhAddOrRemoveStretch(41859);
Copy link
Owner

Choose a reason for hiding this comment

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

Please comment this command id.

break;
case FOCUS_ENVELOPE:
cmdhDeleteEnvelopePointsOrAutoItems(40333, true, false); // Envelope: Delete all selected points
Expand Down Expand Up @@ -5107,7 +5138,10 @@ Command COMMANDS[] = {
{MAIN_SECTION, {{0, 0, 40613}, nullptr}, nullptr, cmdDeleteMarker}, // Markers: Delete marker near cursor
{MAIN_SECTION, {{0, 0, 40615}, nullptr}, nullptr, cmdDeleteRegion}, // Markers: Delete region near cursor
{MAIN_SECTION, {{0, 0, 40617}, nullptr}, nullptr, cmdDeleteTimeSig}, // Markers: Delete time signature marker near cursor
{MAIN_SECTION, {{0, 0, 41859}, nullptr}, nullptr, cmdRemoveStretch}, // Item: remove stretch marker at current position
{MAIN_SECTION, {{0, 0, 41842}, nullptr}, nullptr, cmdAddOrRemoveStretch}, // Item: Add stretch marker at cursor
{MAIN_SECTION, {{0, 0, 41859}, nullptr}, nullptr, cmdAddOrRemoveStretch}, // Item: remove stretch marker at current position
{MAIN_SECTION, {{0, 0, 41844}, nullptr}, nullptr, cmdAddOrRemoveStretch}, // Item: Remove all stretch markers
Copy link
Owner

Choose a reason for hiding this comment

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

Do either of these need a readme update?

{MAIN_SECTION, {{0, 0, 41845}, nullptr}, nullptr, cmdAddOrRemoveStretch}, // Item: Remove all stretch markers in time selection
{MAIN_SECTION, {{0, 0, 40020}, nullptr}, nullptr, cmdClearTimeLoopSel}, // Time selection: Remove time selection and loop point selection
{MAIN_SECTION, {{0, 0, 40769}, nullptr}, nullptr, cmdUnselAllTracksItemsPoints}, // Unselect all tracks/items/envelope points
{MAIN_SECTION, {{0, 0, 40915}, nullptr}, nullptr, cmdInsertEnvelopePoint}, // Envelope: Insert new point at current position (remove nearby points)
Expand Down