Rename BB and TCO to clip and pattern in save files #6309

allejok96 · 2022-02-13T09:18:25Z

Upgrade method in DataFile + renamed some tags used internally to clone pattern tracks

SAVING FILES WITH THIS WILL MAKE INCOMPATIBLE TO OLDER LMMS (BB tracks will be empty)!

Things to test

open old project, check automation, instruments, samples and patterns aka BBs
save project as new, open and check again
midi export
cloning patterns aka BB tracks

LmmsBot · 2022-02-13T09:28:28Z

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Windows

Windows 32-bit: lmms-1.3.0-alpha.1.171%2Bge6b86eb49-mingw-win32.exe (build link)
Windows 64-bit: lmms-1.3.0-alpha.1.171%2Bge6b86eb49-mingw-win64.exe (build link)
Windows 32-bit: lmms-1.3.0-alpha-msvc2017-win32.exe (build link)
Windows 64-bit: lmms-1.3.0-alpha-msvc2017-win64.exe (build link)

Linux

Linux (AppImage): lmms-1.3.0-alpha.1.171%2Bge6b86eb49-linux-x86_64.AppImage (build link)

macOS

macOS : lmms-1.3.0-alpha.1.171%2Bge6b86eb49-mac10.14.dmg (build link)

🤖

{"platform_name_to_artifacts": {"Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://15746-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.171%2Bge6b86eb49-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15746?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://15744-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.171%2Bge6b86eb49-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15744?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/cf39n0wt0mla31ej/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/42570392"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/ng8ugl5ki2qft76m/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/42570392"}], "Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://15747-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.171%2Bge6b86eb49-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15747?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://15743-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.171%2Bge6b86eb49-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15743?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "111b4fdf8444e689a69014c42f1f44c30d89d3c3"}

JohannesLorenz

Code review finished. Only 2 minor comments.

The code covers the tags of all renamed classes, renames the tags correctly and consistently.

src/tracks/PatternTrack.cpp

src/core/DataFile.cpp

allejok96 · 2022-02-13T15:40:30Z

Should we replace Beat/Bassline with Pattern in the track names when opening an older project file? Like this older upgrade:

lmms/src/core/DataFile.cpp

Line 971 in 77fee49

s.replace( QRegExp( "^Beat/Baseline " ),

Veratil · 2022-02-13T15:49:34Z

Should we replace Beat/Bassline with Pattern in the track names when opening an older project file? Like this older upgrade:

lmms/src/core/DataFile.cpp

Line 971 in 77fee49

s.replace( QRegExp( "^Beat/Baseline " ),

My opinion is yes.

allejok96 · 2022-02-13T19:15:12Z

Replacing done for the English strings, I couldn't figure out how to get tr() working in this scope.

JohannesLorenz

Code is OK.

A bit ugly to use the constant "1" for type "PatternTrack", but I guess you wanted to save compile time to not include Track.h?

JohannesLorenz · 2022-02-13T20:38:32Z

I'll continue with test review.

allejok96 · 2022-02-13T20:47:16Z

A bit ugly to use the constant "1" for type "PatternTrack", but I guess you wanted to save compile time

Nah, laziness + relying on the fact that 1 will need to remain for backwards compatibility even if the enum changes in the future.

Veratil · 2022-02-13T20:57:51Z

A bit ugly to use the constant "1" for type "PatternTrack", but I guess you wanted to save compile time

Nah, laziness + relying on the fact that 1 will need to remain for backwards compatibility even if the enum changes in the future.

We should use the enum honestly.

JohannesLorenz

Test report:

A note from SampleClip does not survive the conversion MMP -> MIDI -> MMP => Same issue on master
Found one bug in PatternEditor: Open new project -> Clone PatternTrack in SongEditor -> Move clone over original (the reorder is necessary) -> Open PatternEditor -> Select original (if not already selected) -> Remove original from SongEditor -> PatternEditor shows a track without a grid => Same issue on master
No other issues found

IMO, ready to merge. If you want to change the "1", you can test it yourself, no further review for that.

allejok96 · 2022-02-13T21:27:39Z

U sure @Veratil ? A change to the enum would break this (admittedly very silly) upgrade routine. So it would be for the looks only. But on the other hand, why would it ever change...

Veratil · 2022-02-13T21:40:34Z

U sure @Veratil ? A change to the enum would break this (admittedly very silly) upgrade routine. So it would be for the looks only. But on the other hand, why would it ever change...

Yes, we should be explicit and not guess at the number it's suppose to be. I'm not going to call it a blocker at this point, but it's something we shouldn't overlook. It's not that it will never change, but that it's the correct way.

Veratil · 2022-02-13T21:42:17Z

We can still merge it now, to be clear. It's just something we should start focusing on later.

JohannesLorenz · 2022-02-14T00:05:30Z

We also still need a commit message. Does the current title fit? It does not name "pattern" yet, which is changed to "midiclip".

Let Veratil/me know when we should merge.

... and clarify how PatternTrack cloning works - pattern -> midiclip - automationpattern -> automationclip - *tco -> *clip - bb* -> pattern* - bbtrackcontainer -> patternstore

allejok96 · 2022-02-14T17:12:02Z

@JohannesLorenz, I've squashed the commits and given it a proper message.

The only lines changes since before the force-push is #include "Track.h" and line 1819 and 1820.

I think I found a golden middle way using static_assert in combination with TrackType. So now things like "Find references to symbol" works, but it can never silently break. Ok @Veratil ?

Veratil · 2022-02-14T17:53:32Z

I think I found a golden middle way using static_assert in combination with TrackType. So now things like "Find references to symbol" works, but it can never silently break. Ok @Veratil ?

I didn't even think about static_assert, but it doesn't hurt for sanity checks. 👍

JohannesLorenz · 2022-02-14T20:09:49Z

Perfect. Time to merge then 🚀

Thanks for the PR, again.

zonkmachine · 2022-06-03T17:59:34Z

@allejok96 This creates issues with some older projects. Try demos/DnB

allejok96 · 2022-06-04T08:00:31Z

Uhoh, thanks for the heads up. I've verified that DnB works before this PR and is broken after. Interestingly enough an older version of DnB still works after this PR... The search goes on.

allejok96 mentioned this pull request Feb 13, 2022

Reorganising LMMS, renaming editors and classes and everything in between (cont. #120) #5592

Closed

56 tasks

JohannesLorenz self-requested a review February 13, 2022 12:01

JohannesLorenz reviewed Feb 13, 2022

View reviewed changes

src/tracks/PatternTrack.cpp Show resolved Hide resolved

src/core/DataFile.cpp Outdated Show resolved Hide resolved

JohannesLorenz approved these changes Feb 13, 2022

View reviewed changes

JohannesLorenz self-requested a review February 13, 2022 20:38

JohannesLorenz approved these changes Feb 13, 2022

View reviewed changes

Rename TCO and BB to clip and pattern in save files

111b4fd

... and clarify how PatternTrack cloning works - pattern -> midiclip - automationpattern -> automationclip - *tco -> *clip - bb* -> pattern* - bbtrackcontainer -> patternstore

allejok96 force-pushed the bbTcoDataFile branch from 2825857 to 111b4fd Compare February 14, 2022 17:02

JohannesLorenz merged commit f56fc68 into LMMS:master Feb 14, 2022

allejok96 deleted the bbTcoDataFile branch March 22, 2022 19:36

allejok96 mentioned this pull request Jun 4, 2022

Correctly upgrade projects saved by LMMS forks #6424

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename BB and TCO to clip and pattern in save files #6309

Rename BB and TCO to clip and pattern in save files #6309

allejok96 commented Feb 13, 2022

LmmsBot commented Feb 13, 2022 •

edited

Loading

Windows

Linux

macOS

JohannesLorenz left a comment

allejok96 commented Feb 13, 2022

Veratil commented Feb 13, 2022

allejok96 commented Feb 13, 2022

JohannesLorenz left a comment

JohannesLorenz commented Feb 13, 2022

allejok96 commented Feb 13, 2022

Veratil commented Feb 13, 2022

JohannesLorenz left a comment

allejok96 commented Feb 13, 2022

Veratil commented Feb 13, 2022

Veratil commented Feb 13, 2022

JohannesLorenz commented Feb 14, 2022

allejok96 commented Feb 14, 2022

Veratil commented Feb 14, 2022

JohannesLorenz commented Feb 14, 2022

zonkmachine commented Jun 3, 2022

allejok96 commented Jun 4, 2022

Rename BB and TCO to clip and pattern in save files #6309

Rename BB and TCO to clip and pattern in save files #6309

Conversation

allejok96 commented Feb 13, 2022

LmmsBot commented Feb 13, 2022 • edited Loading

Windows

Linux

macOS

JohannesLorenz left a comment

Choose a reason for hiding this comment

allejok96 commented Feb 13, 2022

Veratil commented Feb 13, 2022

allejok96 commented Feb 13, 2022

JohannesLorenz left a comment

Choose a reason for hiding this comment

JohannesLorenz commented Feb 13, 2022

allejok96 commented Feb 13, 2022

Veratil commented Feb 13, 2022

JohannesLorenz left a comment

Choose a reason for hiding this comment

allejok96 commented Feb 13, 2022

Veratil commented Feb 13, 2022

Veratil commented Feb 13, 2022

JohannesLorenz commented Feb 14, 2022

allejok96 commented Feb 14, 2022

Veratil commented Feb 14, 2022

JohannesLorenz commented Feb 14, 2022

zonkmachine commented Jun 3, 2022

allejok96 commented Jun 4, 2022

LmmsBot commented Feb 13, 2022 •

edited

Loading