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.

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

Can't give a segment the same name as the track #5528

Closed
gthzrsfk opened this issue Jun 7, 2020 · 10 comments · Fixed by #5621
Closed

Can't give a segment the same name as the track #5528

gthzrsfk opened this issue Jun 7, 2020 · 10 comments · Fixed by #5621
Assignees
Labels

Comments

@gthzrsfk
Copy link

gthzrsfk commented Jun 7, 2020

Affected LMMS versions

1.2.1

@gthzrsfk gthzrsfk added the bug label Jun 7, 2020
@Veratil
Copy link
Contributor

Veratil commented Jun 7, 2020

This isn't a bug, but normal behavior.

@gthzrsfk
Copy link
Author

gthzrsfk commented Jun 7, 2020

But for what?
When I rename a track to the same name as a segment and the window of instrument (what do you call it? Which appears when I click the track name) is viewed, all the same of the segments disappear. Is that expected, too?

@Veratil
Copy link
Contributor

Veratil commented Jun 7, 2020

It was done in this commit as part of this PR. As for why, I wasn't around then so I don't know. @PhysSong I see you were around when this PR was open, do you recall anything about this?

@PhysSong
Copy link
Member

PhysSong commented Jun 8, 2020

@Veratil No, it's was there way much before that PR.
dd1db7d

@Veratil
Copy link
Contributor

Veratil commented Jun 8, 2020

Interesting. I don't see a reason why other than "improve appearance".

@zonkmachine
Copy link
Member

zonkmachine commented Jun 8, 2020

Let me see If I get this right. When we create a new pattern we set the name to the track name here:

setName( _instrument_track->name() );

Then some people think that's a bit spammy (I tend to agree with that after removing the code to filter out the track name), so we forbid lmms to write that name on the pattern. It's better to not set the pattern name to the track name in the first place if we don't want it there. I think this should be fixed somehow.

@zonkmachine
Copy link
Member

zonkmachine commented Jun 8, 2020

Something like this. It will make a whole lot of tracks out there reveal hidden track names. I don't know what to do about that though.

(edit: please grab this code)

diff --git a/src/tracks/Pattern.cpp b/src/tracks/Pattern.cpp
index 26cc1c9..f6924cf 100644
--- a/src/tracks/Pattern.cpp
+++ b/src/tracks/Pattern.cpp
@@ -56,7 +56,6 @@ Pattern::Pattern( InstrumentTrack * _instrument_track ) :
        m_patternType( BeatPattern ),
        m_steps( MidiTime::stepsPerTact() )
 {
-       setName( _instrument_track->name() );
        if( _instrument_track->trackContainer()
                                        == Engine::getBBTrackContainer() )
        {
@@ -1049,12 +1048,7 @@ void PatternView::paintEvent( QPaintEvent * )
diff --git a/src/tracks/Pattern.cpp b/src/tracks/Pattern.cpp
index 26cc1c9..f6924cf 100644
--- a/src/tracks/Pattern.cpp
+++ b/src/tracks/Pattern.cpp
@@ -56,7 +56,6 @@ Pattern::Pattern( InstrumentTrack * _instrument_track ) :
        m_patternType( BeatPattern ),
        m_steps( MidiTime::stepsPerTact() )
 {
-       setName( _instrument_track->name() );
        if( _instrument_track->trackContainer()
                                        == Engine::getBBTrackContainer() )
        {
@@ -1049,12 +1048,7 @@ void PatternView::paintEvent( QPaintEvent * )
        // pattern name
        p.setRenderHint( QPainter::TextAntialiasing );
 
-       bool isDefaultName = m_pat->name() == m_pat->instrumentTrack()->name();
-
-       if( !isDefaultName && m_staticTextName.text() != m_pat->name() )
-       {
-               m_staticTextName.setText( m_pat->name() );
-       }
+       m_staticTextName.setText( m_pat->name() );
 
        QFont font;
        font.setHintingPreference( QFont::PreferFullHinting );
@@ -1064,13 +1058,10 @@ void PatternView::paintEvent( QPaintEvent * )
        const int textTop = TCO_BORDER_WIDTH + 1;
        const int textLeft = TCO_BORDER_WIDTH + 1;
 
-       if( !isDefaultName )
-       {
-               p.setPen( textShadowColor() );
-               p.drawStaticText( textLeft + 1, textTop + 1, m_staticTextName );
-               p.setPen( textColor() );
-               p.drawStaticText( textLeft, textTop, m_staticTextName );
-       }
+       p.setPen( textShadowColor() );
+       p.drawStaticText( textLeft + 1, textTop + 1, m_staticTextName );
+       p.setPen( textColor() );
+       p.drawStaticText( textLeft, textTop, m_staticTextName );
 
        // inner border
        if( !( fixedTCOs() && beatPattern ) )

@Spekular
Copy link
Member

Spekular commented Jun 8, 2020

It will make a whole lot of tracks out there reveal hidden track names. I don't know what to do about that though.

Should be possible to write an upgrade routine that clears the pattern name if it matches the track name. It wouldn't break anything, since those names are already invisible the behavior is unchanged for existing projects.

@zonkmachine
Copy link
Member

Yes. I have a rather poor track record with DataFile.cpp though, so I pass that on to someone else. ;)

@Spekular Spekular self-assigned this Jul 9, 2020
@Spekular
Copy link
Member

Spekular commented Jul 9, 2020

If anyone gets around to this before me, go ahead. Just assigning myself so I don't forget that I want to work on this.

Spekular added a commit that referenced this issue Sep 21, 2020
* 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 <[email protected]>
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this issue Jun 28, 2022
* 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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants