Account for changed native folder/file names when loading files

[Monospace-V]

System Information

Windows 10

LMMS Version(s)

master, all

Most Recent Working Version

No response

Bug Summary

Upgrade routine not working as expected on changed sample/folder names.

Files from 0.4 (pre #2712) had the bassloops folder titled "bassloopes".
Loading old project files made before this fix will throw the Sample not found error. If there was a data file routine it does not work (anymore)

At some point in 1.3 (#6747) bpm tags were added to beats in the sample folder.
Old projects do not load now, they also throw a Sample not found error.

Expected Behaviour

File upgrade routine should ensure file loads without errors of sample not found.

Steps To Reproduce

Find file linked in Screenshots / Minimum Reproducible Project.
Attempt to open it in latest master.

Logs

No response

Screenshots / Minimum Reproducible Project

Good example of file to trigger this:
https://lmms.io/lsp/?action=show&file=3625

Please search the issue tracker for existing bug reports before submitting your own.

• I have searched all existing issues and confirmed that this is not a duplicate.

[zonkmachine]

If there was a data file routine it does not work (anymore)

That would be this line:

lmms/src/core/DataFile.cpp

Line 1113 in bb6a77a

s.replace(QRegularExpression("/samples/bassloopes/"), "/samples/bassloops/");

[zonkmachine]

bassloopes -> bassloops upgrade routine is broken already on lmms-1.2.2

[michaelgregorius]

The files had the BPMs added and thus have been renamed with pull request #6747 by @mirk0dex. Might be that the upgrade routine that was also provided by the PR does not work. Or something has changed later.

The upgrade routine also looks needlessly complex for something that should be a straight mapping from an old name to a new one.

[michaelgregorius]

The tests in the upgrade routine DataFile::upgrade_loopsRename do not work. For example when checking for the replacement of beats/rave_hihat01.ogg it's testing against factorysample:beats/rave_hihat01.ogg which doses not match and thus the replacement never takes place.

[michaelgregorius]

Pull request #7235 fixes the problems that have been introduced by adding the BPM values to the names.

@mirk0dex, I have removed the code with regards to the factorysample: prefix. Are there any files where the src attribute starts with this prefix?

Only remaining problem with the linked test file is that the "bassloopes" file is still not found.

[mirk0dex]

The files had the BPMs added and thus have been renamed with pull request #6747 by @mirk0dex. Might be that the upgrade routine that was also provided by the PR does not work. Or something has changed later.

The upgrade routine also looks needlessly complex for something that should be a straight mapping from an old name to a new one.

We had tested it thoroughly, and it worked just fine.

Pull request #7235 fixes the problems that have been introduced by adding the BPM values to the names.

@mirk0dex, I have removed the code with regards to the factorysample: prefix. Are there any files where the src attribute starts with this prefix?

Only remaining problem with the linked test file is that the "bassloopes" file is still not found.

factorysample: was needed in order to Not replace any custom user samples with the same name. Removing it could be dangerous.

[zonkmachine]

We had tested it thoroughly, and it worked just fine.

Yeah, I remember spending quite some time on it. Should apparently have spent some more time on it. :(

[michaelgregorius]

factorysample: was needed in order to Not replace any custom user samples with the same name. Removing it could be dangerous.

The problem is that the example file that's linked above does not use factorysample: in its source attribute but the samples used do not seem to be user samples but factory samples.

The question is if the same files names with factorysample: need to be added to the map as well. When is a sample prefixed with factorysample:? I guess if I pull it from the samples tab? What happens if I load the same sample by another way? I guess it might not be prefixed with factorysample: even if it is one from a logical point?

To be frank, I think renaming the samples that already exist for a long time and that are likely used in tons of projects might have been the first dangerous step in the first place.

I think what LMMS could need is a dialog that opens when some samples cannot be found so that the users have a chance to efficiently point LMMS to the right sample(s). If I remember correctly in REAPER you can either specify the exact file to be used for a missing sample or point it to a directory from where it searches for samples matching by name.

[mirk0dex]

Not sure about the whole factorysample: deal at this point. What I can tell you that every time I've seen a stock LMMS sample in a project's XML, it was always prefixed with it.

To be frank, I think renaming the samples that already exist for a long time and that are likely used in tons of projects might have been the first dangerous step in the first place.

The loops, many of which are actually pretty good, were difficult to use because it was hard to tell what their BPM was. Especially now, that we have warping-like capabilities thanks to Slicert, which doesn't always guess the right BPM, I believe the tags are needed.

I think what LMMS could need is a dialog that opens when some samples cannot be found so that the users have a chance to efficiently point LMMS to the right sample(s). If I remember correctly in REAPER you can either specify the exact file to be used for a missing sample or point it to a directory from where it searches for samples matching by name.

+1 to this. Many programs have this feature already.

[michaelgregorius]

Not sure about the whole factorysample: deal at this point. What I can tell you that every time I've seen a stock LMMS sample in a project's XML, it was always prefixed with it.

@mirk0dex, do you have an example file at hand that has the factorysample: prefix in front of an adjusted file name? I tried to create one with LMMS 1.2 but it does not save with the prefix. If I create a file with master and adjust it manually by removing the BPM values from the file name in the src tags then it will obviously not go through the update routine because the file is too new.

[mirk0dex]

Here:

• https://github.com/LMMS/lmms/files/12641568/beatupgrade.mmp.txt
• https://github.com/LMMS/lmms/files/12664115/beatupgrade2.mmp.txt

zonkmachine'd created these and sent them to me for testing the routine.

Edit: wait, adjusted file name? These files were not run through the routine yet.

[michaelgregorius]

Thanks @mirk0dex! These files are exactly what I was asking for. 👍

[michaelgregorius]

With pull request #7236 factorysample: prefixes are supported again.

[michaelgregorius]

With commit d1fa6dd the "bassloopes" problem is fixed as well. So pull request #7236 should now close this issue.