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

fixes the wrong order of controller by loading a project #3438

Merged
merged 2 commits into from Mar 20, 2017

Conversation

@BaraMGB
Copy link
Contributor

BaraMGB commented Mar 19, 2017

fixes #2869

the problem:

Here's what happens:

  1. LMMS loads the FX Mixer from the file. Because the Peak Controller is a mix between an FX and a controller it gets loaded as an FX when the mixer "FX 1" is restored. In PeakControllerEffect::PeakControllerEffect the Peak Controller is added to the list of controllers by calling Engine::getSong()->addController( m_autoController ). Because no controllers have been loaded yet the Peak Controller is now the first controller in the list.
  2. LMMS loads the controllers and adds them via Song::addController. For each controller this method checks whether the controller is already contained in the list and only adds it if it's not:
    • The LFO controller is loaded first from the file. It is added because it is not contained in the list of controllers.
    • The Peak Controller is now also loaded from the file. It is not added because it is already contained in the list of controllers as it was added during the load of the FX chains.

What I did:

On adding a controller it checks if the controller is in the list. If so it removes the controller from the list and appends it again. So it is in the right order.

if( controller )
	{
		if( m_controllers.contains( controller ) )
		{
			int index = m_controllers.indexOf( controller );
			if( index != -1 )
			{
				m_controllers.remove( index );
				emit controllerRemoved( controller );
			}
		}
		m_controllers.append( controller );
		emit controllerAdded( controller );

		this->setModified();
	}
@BaraMGB
Copy link
Contributor Author

BaraMGB commented Mar 19, 2017

@zonkmachine
Copy link
Member

zonkmachine commented Mar 19, 2017

Tested, works! I've tested complex patterns, both saved from 1.1.3, master and this PR and they now open up correctly.

@jasp00
Copy link
Member

jasp00 commented Mar 20, 2017

The patch is incorrect. It changes the meaning of addController(). The (understandable) bug is PeakControllerEffect's fault. The constructor should check Engine::getSong()->isLoadingProject().

@BaraMGB
Copy link
Contributor Author

BaraMGB commented Mar 20, 2017

@jasp00 thank you.

@jasp00 jasp00 merged commit 17a6f37 into LMMS:master Mar 20, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

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