Remove song import global automation

[Veratil]

Completes step 2 from #403

I've also removed the global automation <track> node from the templates. I noticed in default.mpt that there were <object> nodes, but no related id in any other nodes like there would normally be if there was a pattern connected. I believe this might be a bug in the automation editor.

[PhysSong]

I think DataFile is a better place to handle the upgrade stuff rather than Song.

[Veratil]

I think DataFile is a better place to handle the upgrade stuff rather than Song.

I tried that initially, but it didn't do anything. I'll try playing with it some more.

[Veratil]

@PhysSong okay it took some playing around with finding nodes, but I finally managed to get it working in DataFile.

[PhysSong]

@Veratil Could you look into factory project files to see if any of them has global automation tracks?

[Veratil]

@Veratil Could you look into factory project files to see if any of them has global automation tracks?

@PhysSong I believe that's already taken care of in the second commit, or are there other ones that I missed?

[PhysSong]

It will be okay if you've checked all demo projects and confirmed that there are no global automation tracks that you need to remove.

[Veratil]

It will be okay if you've checked all demo projects and confirmed that there are no global automation tracks that you need to remove.

I've gone through and found that some of the demos are still adding tracks that aren't linked to anything. I've updated the check to look for more than one <time> node now which has prevented adding extraneous automation tracks which aren't linked to anything.

There is one demo that has an unlinked automation track which has mulitple time nodes.

@PhysSong Would you like me to go through and update all the demo files in this patch to the "newest" save file version then?

[PhysSong]

@PhysSong Would you like me to go through and update all the demo files in this patch to the "newest" save file version then?

If you want to do so, I'd recommend not touching files without global automations. I'm also okay if you do that in a separate pull request.

[Veratil]

@PhysSong Would you like me to go through and update all the demo files in this patch to the "newest" save file version then?

If you want to do so, I'd recommend not touching files without global automations. I'm also okay if you do that in a separate pull request.

Is there an easy way to read the mmpz files? That would be the easiest to see if there is any automation patterns that should be imported.

[PhysSong]

Is there an easy way to read the mmpz files?

lmms -d input.mmpz prints the decompressed version of input.mmpz.

[Veratil]

Is there an easy way to read the mmpz files?

lmms -d input.mmpz prints the decompressed version of input.mmpz.

Would it be okay to run lmms upgrade [in] [out] on all the files? 😄

[PhysSong]

That makes sense, but Qt will mix the order of attributes in XML tags. You can write a simple program to compress/decompress .mmpz files with QByteArray::qCompress() and QByteArray::qUncompress().

[PhysSong]

The result of qCompress() consists of the size of the uncompressed data(in 4byte big-endian integer) and the Zlib-compressed data. It means you can use some other software to (de)compress the project files.

[JohannesLorenz]

@Veratil Are you going to finish this PR?

[Spekular]

Seconded. It would be superb if we could get this and step 3 (remove from the GUI) done for 1.3, and step 3 should be a quick and safe fix.

[Veratil]

I need to go through all the demo files still.

[PhysSong]

@Veratil Bump.

[Veratil]

1. I rebased on master 😁
2. I updated the update function to the new format.
3. I removed the global automation save from Song's saveSettings, no more hidden automations will be saved after this at all.
4. I went through each demo file and actually loaded them in lmms. Any that had an unused hidden automation tracks that actually created visible automation tracks I removed and saved over. Anything else I didn't touch. If they do have the block, it's either ignored or will get removed on future updates where we update the demos for whatever reason.

[PhysSong]

Regarding global automation clips with only one point: In many cases, they are just holding the model's value at time 0, but it's not guaranteed that they always do.

[PhysSong]

The rest looks good to me.

[Veratil]

Regarding global automation clips with only one point: In many cases, they are just holding the model's value at time 0, but it's not guaranteed that they always do.

That's where thing are weird. Every global automation track that had one node was either a leftover that wasn't removed (file bloat because there wasn't a way to delete the global track), or didn't actually affect the model's value anymore (that value was saved as an attribute instead or didn't actually connect anymore).

I can make the change to load it regardless, and I'll go through all the projects again to see if anything else pops up.

[PhysSong]

I looked into this PR again. This PR will change some behaviors:

• Global automation patterns have lower priority than ordinary ones, but this PR will reverse it due to the order of XML nodes
• Global automation patterns with single point which "locks" the value of connected models will be removed
• Global automation patterns created by user will never be saved

The first one sounds undesirable, but the rest don't sound too bad once #5230 is merged and #5223 (comment) is addressed.

[Veratil]

Rebased on master and (hopefully) fixed conflicts (correctly). Tested build and it seemed to be working.
I'll go back through all the demos to double check everything, those are the biggest changes for removing the hidden automation tracks.

[Veratil]

Looks like everything is still good in the demos. 👍

[PhysSong]

Global automation patterns have lower priority than ordinary ones, but this PR will reverse it due to the order of XML nodes

I think this is now the only one which probably needs to be addressed. @Veratil any opinions?

[Veratil]

Global automation patterns have lower priority than ordinary ones, but this PR will reverse it due to the order of XML nodes

I think this is now the only one which probably needs to be addressed. @Veratil any opinions?

I'm honestly not sure what you mean by priority here. If two automation clips are connected to the same object the global wins?

[PhysSong]

If two automation clips are connected to the same object the global wins?

Yes, I mean that case.

lmms/src/core/Song.cpp

Lines 841 to 844 in 1f30ffc

	AutomatedValueMap Song::automatedValuesAt(TimePos time, int clipNum) const
	{
	return TrackContainer::automatedValuesFromTracks(TrackList{m_globalAutomationTrack} << tracks(), time, clipNum);
	}

Here you can see the global automation track is the first element before removal. You can also see that TrackContainer::automatedValuesFromTracks uses the value from the last track connected to an object.

[Veratil]

Okay I'll try to see what can be done for this to keep them in order. It never occurred to me that you could connect an object to multiple automations.

[Rossmaxx]

Update?

[PhysSong]

Update?

It's ready for review now.

[Rossmaxx]

Is the track order thing fixed now?

[PhysSong]

Is the track order thing fixed now?

I fixed that in the latest commit.

[Rossmaxx]

Got a few concerns from the diff.

[PhysSong]

@Rossmaxx Factory projects probably requires re-saving again, I think

[Rossmaxx]

Yeah, or unstaging them for now.

[PhysSong]

Yeah, or unstaging them for now.

I think we can defer re-saving to right before releasing 1.3 or any other sane point. Unstaging for now.

I also manually removed related parts in factory templates and tutorials.

[Rossmaxx]

Everything looks good for now.

[Rossmaxx]

Removing global automation shouldn't have waited this long.