Skip to content

Commit

Permalink
Reorganizes code to make it ready for review
Browse files Browse the repository at this point in the history
	This commit reorganizes the code a little bit to make it better suited for review. The code that checked whether a selection can be merged was extracted to a separate method called TrackContentObjectView::canMergeSelection(). Two conditionals inside that code were merged into a single one, since they both resulted in the same action.
	Also updated a comment next to a line of code that is probably going to be removed after the PR LMMS#5699 is merged (it's currently necessary because of a bug that is fixed on that PR).
  • Loading branch information
IanCaio committed Oct 4, 2020
1 parent 21dff27 commit ff76e0f
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 39 deletions.
3 changes: 3 additions & 0 deletions include/Track.h
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,9 @@ class TrackContentObjectView : public selectableObject, public ModelView
static void toggleMute( QVector<TrackContentObjectView *> tcovs );
static void mergeTCOs( QVector<TrackContentObjectView *> tcovs );

// Returns true if selection can be merged and false if not
static bool canMergeSelection( QVector<TrackContentObjectView *> tcovs );

public slots:
virtual bool close();
void cut();
Expand Down
75 changes: 36 additions & 39 deletions src/core/Track.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1138,44 +1138,7 @@ void TrackContentObjectView::contextMenuEvent( QContextMenuEvent * cme )

QMenu contextMenu( this );

// We can only merge InstrumentTrack's TCOs, so check if we only have those in the selection,
// and also if they all belong to the same track
bool canMergeTCOs = false;
if( !individualTCO )
{
// We can only merge if we have more then one TCO, so first set to true
canMergeTCOs = true;

// Variable to check if all TCOs belong to the same track
TrackView *previousOwnerTrackView = nullptr;

// Then we check every selected TCO to see if all of them are InstrumentTrack's TCOs.
// If any isn't, we set canMergeTCOs to false and quit the loop.
for( auto tcov: selectedTCOs )
{
TrackView *ownerTrackView = tcov->getTrackView();

// Set the previousOwnerTrackView to the first TrackView
if( !previousOwnerTrackView )
{
previousOwnerTrackView = ownerTrackView;
}

// Are all TCOs from the same track?
if( ownerTrackView != previousOwnerTrackView )
{
canMergeTCOs = false;
break;
}

// Is the TCO from an InstrumentTrack?
if( ! dynamic_cast<InstrumentTrackView *>(ownerTrackView) )
{
canMergeTCOs = false;
break;
}
}
}
bool canMergeTCOs = individualTCO ? false : canMergeSelection( selectedTCOs );

if( fixedTCOs() == false )
{
Expand Down Expand Up @@ -1374,6 +1337,40 @@ void TrackContentObjectView::toggleMute( QVector<TrackContentObjectView *> tcovs
}
}

bool TrackContentObjectView::canMergeSelection( QVector<TrackContentObjectView *> tcovs )
{
// We can only merge InstrumentTrack's TCOs, so check if we only have those in the selection,
// and also if they all belong to the same track

// We start assuming we can merge and if any condition is broken we set it to false
bool canMerge = true;

// Variable to check if all TCOs belong to the same track
TrackView *previousOwnerTrackView = nullptr;

// Then we check every selected TCO to see if all of them are InstrumentTrack's TCOs.
// If any isn't, we set canMerge to false and quit the loop.
for( auto tcov: tcovs )
{
TrackView *ownerTrackView = tcov->getTrackView();

// Set the previousOwnerTrackView to the first TrackView
if( !previousOwnerTrackView )
{
previousOwnerTrackView = ownerTrackView;
}

// If there are TCOs from different tracks or TCOs from tracks other than an InstrumentTrack, can't merge them
if( ownerTrackView != previousOwnerTrackView || !dynamic_cast<InstrumentTrackView *>(ownerTrackView) )
{
canMerge = false;
break;
}
}

return canMerge;
}

void TrackContentObjectView::mergeTCOs( QVector<TrackContentObjectView *> tcovs )
{
// Get the track that we are merging TCOs in
Expand Down Expand Up @@ -1406,7 +1403,7 @@ void TrackContentObjectView::mergeTCOs( QVector<TrackContentObjectView *> tcovs
return;
}

newPattern->movePosition( earliestPos ); // Kinda weird we need to call this even though we already give the Pos on the constructor right?
newPattern->movePosition( earliestPos ); // TODO: Won't be necessary once #5699 is merged!

// Add the notes and remove the TCOs that are being merged
for( auto tcov: tcovs )
Expand Down

0 comments on commit ff76e0f

Please sign in to comment.