Don't give clips a hidden default name (Fix #5528) (#5621)

* Automatic formatting changes

* Give clips an empty name by default, display all names

- Stop giving clips the same name as their parent track on creation
- Stop hiding clip names that match the parent track name
- Never rename clips on track rename
- Never clear clip name when a clip is copied to another track
- Create an upgrade routine to clear default names from old projects (< 1.3.0-alpha-1)
- Bump version to 1.3.0-alpha-1

* Revert now-unnecessary version bump

* Merge with master and fix conflicts

* Formatting changes from review

* Change weird for loop conditions

* Properly revert AutomationPatter.h changes

* Only clear names that match our parent track, be more generous with use of legacyFileVersion

Co-authored-by: Hyunjin Song <tteu.ingog@gmail.com>

include/DataFile.h

@@ -114,6 +114,7 @@ private:
 	void upgrade_1_1_91();
 	void upgrade_1_2_0_rc3();
 	void upgrade_1_3_0();
+	void upgrade_noHiddenClipNames();
 
 	// List of all upgrade methods
 	static const std::vector<UpgradeMethod> UPGRADE_METHODS;
@@ -141,4 +142,3 @@ private:
 
 
 #endif
-

include/ProjectVersion.h

@@ -45,6 +45,7 @@ public:
 	ProjectVersion(QString version, CompareType c = All);
 	ProjectVersion(const char * version, CompareType c = All);
 
+	const QString& getVersion() const { return m_version; }
 	int getMajor() const { return m_major; }
 	int getMinor() const { return m_minor; }
 	int getPatch() const { return m_patch; }

src/core/DataFile.cpp

@@ -59,7 +59,7 @@ const std::vector<DataFile::UpgradeMethod> DataFile::UPGRADE_METHODS = {
 	&DataFile::upgrade_0_4_0_beta1      ,   &DataFile::upgrade_0_4_0_rc2,
 	&DataFile::upgrade_1_0_99           ,   &DataFile::upgrade_1_1_0,
 	&DataFile::upgrade_1_1_91           ,   &DataFile::upgrade_1_2_0_rc3,
-	&DataFile::upgrade_1_3_0
+	&DataFile::upgrade_1_3_0            ,   &DataFile::upgrade_noHiddenClipNames
 };
 
 // Vector of all versions that have upgrade routines.
@@ -1355,6 +1355,35 @@ void DataFile::upgrade_1_3_0()
 	}
 }
 
+void DataFile::upgrade_noHiddenClipNames()
+{
+	QDomNodeList tracks = elementsByTagName("track");
+
+	auto clearDefaultNames = [](QDomNodeList clips, QString trackName)
+	{
+		for (int j = 0; j < clips.size(); ++j)
+		{
+			QDomElement clip = clips.item(j).toElement();
+			QString clipName = clip.attribute("name", "");
+			if (clipName == trackName) { clip.setAttribute("name", ""); }
+		}
+	};
+
+	for (int i = 0; i < tracks.size(); ++i)
+	{
+		QDomElement track = tracks.item(i).toElement();
+		QString trackName = track.attribute("name", "");
+
+		QDomNodeList instClips = track.elementsByTagName("pattern");
+		QDomNodeList autoClips = track.elementsByTagName("automationpattern");
+		QDomNodeList bbClips = track.elementsByTagName("bbtco");
+
+		clearDefaultNames(instClips, trackName);
+		clearDefaultNames(autoClips, trackName);
+		clearDefaultNames(bbClips, trackName);
+	}
+}
+
 
 void DataFile::upgrade()
 {
@@ -1430,62 +1459,45 @@ void DataFile::loadData( const QByteArray & _data, const QString & _sourceFile )
 	m_type = type( root.attribute( "type" ) );
 	m_head = root.elementsByTagName( "head" ).item( 0 ).toElement();
 
-	if( root.hasAttribute( "version" ) )
+	if (!root.hasAttribute("version") || root.attribute("version")=="1.0")
 	{
-		if( root.attribute( "version" ) == "1.0" ){
-			// The file versioning is now a unsigned int, not maj.min, so we use
-			// legacyFileVersion() to retrieve the appropriate version
-			m_fileVersion = legacyFileVersion();
-		}
-		else
-		{
-			bool success;
-			m_fileVersion = root.attribute( "version" ).toUInt( &success );
-			if( !success ) qWarning("File Version conversion failure.");
-		}
+		// The file versioning is now a unsigned int, not maj.min, so we use
+		// legacyFileVersion() to retrieve the appropriate version
+		m_fileVersion = legacyFileVersion();
+	}
+	else
+	{
+		bool success;
+		m_fileVersion = root.attribute( "version" ).toUInt( &success );
+		if( !success ) qWarning("File Version conversion failure.");
 	}
 
-	if( root.hasAttribute( "creatorversion" ) )
+	if (root.hasAttribute("creatorversion"))
 	{
 		// compareType defaults to All, so it doesn't have to be set here
-		ProjectVersion createdWith = root.attribute( "creatorversion" );
+		ProjectVersion createdWith = root.attribute("creatorversion");
 		ProjectVersion openedWith = LMMS_VERSION;
 
-		if ( createdWith != openedWith )
-		{
-			if( createdWith.setCompareType( ProjectVersion::Minor ) !=
-				openedWith.setCompareType( ProjectVersion::Minor ) )
-			{
-				if( gui != nullptr && root.attribute( "type" ) == "song" )
-				{
-					TextFloat::displayMessage(
-						SongEditor::tr( "Version difference" ),
-						SongEditor::tr(
-							"This %1 was created with "
-							"LMMS %2."
-						).arg(
-							_sourceFile.endsWith( ".mpt" ) ?
-								SongEditor::tr( "template" ) :
-								SongEditor::tr( "project" )
-						)
-						.arg( root.attribute( "creatorversion" ) ),
-						embed::getIconPixmap( "whatsthis", 24, 24 ),
-						2500
-					);
-				}
-			}
+		if (createdWith < openedWith) { upgrade(); }
 
-			// the upgrade needs to happen after the warning as it updates the project version.
-			if( createdWith.setCompareType( ProjectVersion::Build )
-								< openedWith )
-			{
-				upgrade();
-			}
+		if (createdWith.setCompareType(ProjectVersion::Minor)
+		 !=  openedWith.setCompareType(ProjectVersion::Minor)
+		 && gui != nullptr && root.attribute("type") == "song"
+		){
+			auto projectType = _sourceFile.endsWith(".mpt") ?
+				SongEditor::tr("template") : SongEditor::tr("project");
+
+			TextFloat::displayMessage(
+				SongEditor::tr("Version difference"),
+				SongEditor::tr("This %1 was created with LMMS %2")
+				.arg(projectType).arg(createdWith.getVersion()),
+				embed::getIconPixmap("whatsthis", 24, 24),
+				2500
+			);
 		}
 	}
 
-	m_content = root.elementsByTagName( typeName( m_type ) ).
-							item( 0 ).toElement();
+	m_content = root.elementsByTagName(typeName(m_type)).item(0).toElement();
 }
 
 

src/core/Track.cpp

@@ -1865,13 +1865,6 @@ bool TrackContentWidget::pasteSelection( MidiTime tcoPos, const QMimeData * md,
 		{
 			tco->selectViewOnCreate( true );
 		}
-
-		//check tco name, if the same as source track name dont copy
-		QString sourceTrackName = outerTCOElement.attributeNode( "trackName" ).value();
-		if( tco->name() == sourceTrackName )
-		{
-			tco->setName( "" );
-		}
 	}
 
 	AutomationPattern::resolveAllIDs();

src/tracks/BBTrack.cpp

@@ -290,10 +290,7 @@ void BBTCOView::openInBBEditor()
 
 
 
-void BBTCOView::resetName()
-{
-	m_bbTCO->setName( m_bbTCO->getTrack()->name() );
-}
+void BBTCOView::resetName() { m_bbTCO->setName(""); }
 
 
 

src/tracks/InstrumentTrack.cpp

@@ -151,7 +151,7 @@ InstrumentTrack::~InstrumentTrack()
 		autoAssignMidiDevice(false);
 		s_autoAssignedTrack = NULL;
 	}
-	
+
 	// kill all running notes and the iph
 	silenceAllNotes( true );
 
@@ -530,17 +530,6 @@ void InstrumentTrack::deleteNotePluginData( NotePlayHandle* n )
 
 void InstrumentTrack::setName( const QString & _new_name )
 {
-	// when changing name of track, also change name of those patterns,
-	// which have the same name as the instrument-track
-	for( int i = 0; i < numOfTCOs(); ++i )
-	{
-		Pattern* p = dynamic_cast<Pattern*>( getTCO( i ) );
-		if( ( p != NULL && p->name() == name() ) || p->name() == "" )
-		{
-			p->setName( _new_name );
-		}
-	}
-
 	Track::setName( _new_name );
 	m_midiPort.setName( name() );
 	m_audioPort.setName( name() );

src/tracks/Pattern.cpp

@@ -56,7 +56,6 @@ Pattern::Pattern( InstrumentTrack * _instrument_track ) :
 	m_patternType( BeatPattern ),
 	m_steps( MidiTime::stepsPerBar() )
 {
-	setName( _instrument_track->name() );
 	if( _instrument_track->trackContainer()
 					== Engine::getBBTrackContainer() )
 	{
@@ -647,10 +646,7 @@ void PatternView::setGhostInPianoRoll()
 
 
 
-void PatternView::resetName()
-{
-	m_pat->setName( m_pat->m_instrumentTrack->name() );
-}
+void PatternView::resetName() { m_pat->setName(""); }
 
 
 
@@ -885,8 +881,8 @@ void PatternView::paintEvent( QPaintEvent * )
 
 	// Check whether we will paint a text box and compute its potential height
 	// This is needed so we can paint the notes underneath it.
-	bool const isDefaultName = m_pat->name() == m_pat->instrumentTrack()->name();
-	bool const drawTextBox = !beatPattern && !isDefaultName;
+	bool const drawName = !m_pat->name().isEmpty();
+	bool const drawTextBox = !beatPattern && drawName;
 
 	// TODO Warning! This might cause problems if TrackContentObjectView::paintTextLabel changes
 	int textBoxHeight = 0;