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

Correctly upgrade projects saved by LMMS forks #6424

Merged
merged 2 commits into from
Jun 14, 2022

Conversation

allejok96
Copy link
Contributor

Check version (of file format) instead of creatorversion (of LMMS) when deciding if a project file needs to be upgraded.

This fixes the currently broken demos/DnB.mmpz. It got saved by a fork 1.3.0-alpha.1.199 back in #6239. The current master is at 1.3.0-alpha.1.191 so - funny enough - it will be automatically un-broken in 9 commits.

@allejok96 allejok96 marked this pull request as ready for review June 4, 2022 10:03
@LmmsBot
Copy link

LmmsBot commented Jun 4, 2022

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

Linux

Windows

macOS

🤖
{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://output.circle-artifacts.com/output/job/46b3b37e-df92-4c65-9af5-a2e921ddae64/artifacts/0/lmms-1.3.0-alpha.1.194+g37081cf99-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/17060?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://output.circle-artifacts.com/output/job/f587ea27-6b07-4c7d-ab38-0bd9ae46e8a9/artifacts/0/lmms-1.3.0-alpha.1.194+g37081cf99-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/17064?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://output.circle-artifacts.com/output/job/8ca808d5-c995-4386-a60e-ed719f217761/artifacts/0/lmms-1.3.0-alpha.1.194+g37081cf99-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/17062?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/ox17hiiqjdxp009m/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/43757858"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/4ksd9q0pa32gkxri/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/43757858"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://output.circle-artifacts.com/output/job/6783d4d2-d4b3-4b29-89e6-2a8d929feab7/artifacts/0/lmms-1.3.0-alpha.1.194+g37081cf99-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/17061?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "3e615de2ac71ca516dde07af733398fded162fb7"}

@zonkmachine
Copy link
Member

Tested, fix confirmed!

src/core/DataFile.cpp Outdated Show resolved Hide resolved
@Rua
Copy link

Rua commented Jun 5, 2022

Does this also fix #6414?

@zonkmachine
Copy link
Member

zonkmachine commented Jun 5, 2022

Does this also fix #6414?

Can you please download a test build (LmmsBot, above) and test this? That would be much appreciated!

@Rua
Copy link

Rua commented Jun 5, 2022

I tested it, unfortunately the issue still exists. I guess something else is going on with that one.

@JohannesLorenz JohannesLorenz self-requested a review June 11, 2022 01:40
@allejok96
Copy link
Contributor Author

Merge? Or a second real world test? I don't know how you usually do for smaller PRs.

@Veratil
Copy link
Contributor

Veratil commented Jun 14, 2022

Confirmed fixes the import of demos (not just demos/DnB.mmpz) with B+B tracks introduced from f56fc68.

Okay to merge from me.

@Veratil Veratil merged commit ce2d898 into LMMS:master Jun 14, 2022
@allejok96 allejok96 deleted the dataFileVersionCheck branch July 2, 2022 18:05
sakertooth pushed a commit to sakertooth/lmms that referenced this pull request May 30, 2023
* Do project file upgrades based on file version and not on LMMS version

* Do upgrade after version check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants