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

LMMS displays templates as last opened project #1812

Closed
Sti2nd opened this issue Mar 2, 2015 · 11 comments
Closed

LMMS displays templates as last opened project #1812

Sti2nd opened this issue Mar 2, 2015 · 11 comments
Labels
Milestone

Comments

@Sti2nd
Copy link
Contributor

@Sti2nd Sti2nd commented Mar 2, 2015

Since I have edited the default template, and that is loaded on startup, LMMS ALWAYS says that default.mpt was the last project. So it should not say it for default.mpt, at least

image

@Sti2nd
Copy link
Contributor Author

@Sti2nd Sti2nd commented Mar 2, 2015

PS: I like how the slashes are mixed between forward and backward

@tresf tresf added the enhancement label Mar 2, 2015
@tresf tresf added this to the 1.2.0 milestone Mar 2, 2015
@tresf tresf added the bug label Mar 2, 2015
@tresf
Copy link
Member

@tresf tresf commented Mar 2, 2015

PS: I like how the slashes are mixed between forward and backward

Yeah that is a cleanup task for us to tackle on a rainy day. :)

LMMS [...] says that default.mpt was the last project.

Good find. We should remove all templates from that list via ConfigManager.cpp#L192.

Something like this perhaps...

void ConfigManager::addRecentlyOpenedProject( const QString & _file )
{
    ////  ADD TEMPLATE EXTENSION CHECK
    if ( ! _file.endsWith( ".mpt" ) ) {
        m_recentlyOpenedProjects.removeAll( _file );
        if( m_recentlyOpenedProjects.size() > 15 )
        {
            m_recentlyOpenedProjects.removeLast();
        }
        m_recentlyOpenedProjects.push_front( _file );
        ConfigManager::inst()->saveConfigFile();
    }
}
@M374LX
Copy link
Contributor

@M374LX M374LX commented Mar 13, 2015

Actually, the template used is shown as the last opened project every time one creates a project from a template, not only from the default one. This should not happen with any template at all.

A better solution than @tresf's suggestion is to check the file name within Song::loadProject(), which is called from Song::createNewProjectFromTemplate().

At Song.cpp line 1045, we could replace:

ConfigManager::inst()->addRecentlyOpenedProject( fileName );

with

if ( !fileName.endsWith( ".mpt" ) )
{
    ConfigManager::inst()->addRecentlyOpenedProject( fileName );
}
@tresf
Copy link
Member

@tresf tresf commented Mar 13, 2015

Yes, much cleaner!

@M374LX
Copy link
Contributor

@M374LX M374LX commented Mar 13, 2015

One more detail is that creating a new project from a template created with an old version of LMMS show a message like
This project was created with LMMS version x.x.x, but version x.x.x is installed
It is better to use the word "template" instead of "project".

The method where the message is shown is DataFile::loadData().

@tresf
Copy link
Member

@tresf tresf commented Mar 13, 2015

One more detail is that creating a new project from a template created with an old version of LMMS show a message like
This project was created with LMMS version x.x.x, but version x.x.x is installed
It is better to use the word "template" instead of "project".

We should probably file a separate issue for that.

@tresf
Copy link
Member

@tresf tresf commented Mar 13, 2015

@M374LX actually, I think my proposed solution is more scalable. Why put the logic in an unrelated class?

@M374LX
Copy link
Contributor

@M374LX M374LX commented Mar 13, 2015

@tresf
I have just seen that ConfigManager::addRecentlyOpenedProject() is also called from Song::guiSaveProject(). So, in case you prefer saved templates not to be listed as recent projects, your solution is really better.

@tresf
Copy link
Member

@tresf tresf commented Mar 13, 2015

👍

@M374LX
Copy link
Contributor

@M374LX M374LX commented Mar 13, 2015

We should probably file a separate issue for that.

Filed: #1869

@tresf
Copy link
Member

@tresf tresf commented Jun 11, 2015

Closed via: #2091. Added to 1.2.0 changelog.

-Tres

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.