Controller will not saved in right order

[BaraMGB]

It's a little bit complicated to reproduce. Here's a step by step instruction:

1. new project / add 2 FX channels in Mixer
2. add lfo controller in Controller Rack
3. in BB editor make some steps in kicker and route kicker in FX1
4. add a Peak Controller on FX1
5. connect FX2 Fader with Controller2 (this should be the Peak Controller )
6. connect TripleOSC Volume knob to controller 1 (the LFO from step 2.)
7. save Project / restart LMMS and load Project again

Behavior: the controller connections are in reversed order. The lfo is now connected to the fader and the peak to the volume knob. Also the order in Controller Rack are reverse and both are named "controller1"

Kubuntu 14.04 64bit Qt4 build, latest master.

[zonkmachine]

This problem/regression started with PR #2222 (3548629) that caused a crash when loading a project with a Peak Controller in it, reported as Issue #2318 some two weeks later .
#2318 was closed in #2456.
Since we need to repopen a project as devised above to test the bug we can't actually bisect the code from this point to where the crash has been fixed. I'm trying to bisect this further to see if there are any changes in the save files that could hint to the cause but I haven't spotted anything yet.

Related Issues/Pull requests:
#2222 PR (@michaelgregorius)
#2407 Issue: PeakControllerEffect and PeakController circular destruction
#2408 (PR, closed #2407 @grejppi)
#2456 ( PR, fixed crash, closed #2318 @softrabbit)

[zonkmachine]

If you save the example project above in master and open it in stable-1.1.3 it will open correctly.

[michaelgregorius]

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.

The list of controllers now looks as follows:

1. Peak Controller (index 0)
2. LFO Controller (index 1)

Because the controllers seem to be referenced by their index in the list and not by an ID the order of connections is now switched compared to what we saved originally:

1. LFO Controller (index 0)
2. Peak Controller (index 1)

A potential solution

If I remove the following line in PeakControllerEffect::PeakControllerEffect the file loads correctly for me:

Engine::getSong()->addController( m_autoController );

However, looking at the long list of bugs above I assume that something else would break when doing so. Can someone else please test?

[zonkmachine]

However, looking at the long list of bugs above I assume that something else would break when doing so. Can someone else please test?

Tested. This only works for saved files. Any newly added Peak Controller won't be selectable from the list or show up in the Controller Rack.

[zonkmachine]

The list of controllers now looks as follows:

1.Peak Controller (index 0)
2.LFO Controller (index 1)

Because the controllers seem to be referenced by their index in the list and not by an ID the order of connections is now switched compared to what we saved originally:

1.LFO Controller (index 0)
2.Peak Controller (index 1)

Yes, and If you instead add the peak controller first they will reload correctly.

Engine::getSong()->addController( m_autoController );

Is it possible to execute this after the constructor?
I think the old way to do this was that the Peak Controller removed itself from the controller list and then do it one more time with the other controllers properly.

PS. Thank's for looking into this!

[simonvanderveldt]

Because the controllers seem to be referenced by their index in the list and not by an ID the order of connections is now switched compared to what we saved originally

It sounds like this would be the thing to fix?
Or alternatively, to make sure that every item only every belongs to a single "category" ("FX", "controller", etc)?

[StakeoutPunch]

Possible solution: Because there are currently only two saved controller types, why not separate them into separate lists? Displayed either by a tabbed controller window, or having two controller areas in the controller window, corresponding to type.

[tresf]

@StakeoutPunch I find the UI proposal to be a separate enhancement. From what I'm reading this is a simple matter of not honoring (or not storing) index.