Core Refactor: Replace ``QVector`` with ``std::vector``
Essentially what was done in Veratil's Qt removal PR, only this time with more modernization (particularly with iterators) and testing. Changes on the GUI side were made as necessary to make the code compile. The remaining QVector's, if any, in the codebase should be in the GUI, but it is likely that I may have missed something.
The "Phase 2" is mainly because there were changes that needed to be made in order to make the code compile that I had missed on the first run.
:robot: Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! :tophat:
Linux
- Linux (AppImage):
lmms-1.3.0-alpha.1.274+gdcc49af09-linux-x86_64.AppImage(build link)
Windows
- Windows 32-bit:
lmms-1.3.0-alpha.1.274+gdcc49af09-mingw-win32.exe(build link) - Windows 64-bit:
lmms-1.3.0-alpha.1.274+gdcc49af09-mingw-win64.exe(build link)
:robot:
{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://output.circle-artifacts.com/output/job/4b9b8c46-60df-4348-b2e5-db6a17d35a8f/artifacts/0/lmms-1.3.0-alpha.1.274+gdcc49af09-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/sakertooth/lmms/428?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/d62c7e90-3e30-4146-aee5-f7dddb8e4ad8/artifacts/0/lmms-1.3.0-alpha.1.274+gdcc49af09-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/sakertooth/lmms/430?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/32f882ab-b6da-4527-9333-a1266081f0d0/artifacts/0/lmms-1.3.0-alpha.1.274+gdcc49af09-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/sakertooth/lmms/426?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "dcc49af0958c8b8a134cf56a1642a9a8e65e9bc7"}
I've decided that I'm just going to replace QVector with std::vector everywhere. There is no reason to leave QVector in the codebase as a whole even if this PR has a greater focus on the core.
There is no reason to leave
QVectorin the codebase as a whole even if this PR has a greater focus on the core.
One of the biggest differences of QVector and std::vector that I'm aware of is QVector uses copy-on-write and can implicitly share data between multiple containers. I'm not sure how this will affect our codebase, though.
One more thing worth discussing here is: do we want to changes in UI code in the same PR or not?
There is no reason to leave
QVectorin the codebase as a whole even if this PR has a greater focus on the core.One of the biggest differences of
QVectorandstd::vectorthat I'm aware of isQVectoruses copy-on-write and can implicitly share data between multiple containers. I'm not sure how this will affect our codebase, though.One more thing worth discussing here is: do we want to changes in UI code in the same PR or not?
I think changes to the UI code ultimately had to happen regardless because I had to make sure the code would compile. However, for the 20 most recent commits I just sent, it might be worthwhile to split this PR up because quite frankly it is huge. I've had doubts that this PR will be merged anytime soon just because of how large it is.
Let me know if you want to split this PR personally as well, and I'll do so. My only concern would be if we are going to run into merge conflicts, then I'm mostly not sure.
Edit: And on the topic of why I made QVector changes in the UI, I didn't like the idea of mixing two vector solutions together when the STL version is probably the better option. I also found it hard to make this PR because of my initial focus on just the core. I had to make sure that the core was the only part of codebase being altered (for the most part). With no kind of header separation for the core, checking to see if you are only changing the core files and changing the UI as needed is a pain. Leaving QVector also made this PR feel somewhat incomplete in my opinion.
Let me know if you want to split this PR personally as well, and I'll do so. My only concern would be if we are going to run into merge conflicts, then I'm mostly not sure.
You can backup this branch and create a new PR from it once this is merged. This won't lead to significant merge conflicts. One more reason for splitting is to review new changes separately, if required.
I will try to fix the conflicts within this PR soon, but the clang-tidy PRs that occurred are creating a lot more conflicts than what it would seem. That being said, it might take some time.
I also want to get rid of those "QVector -> std::vector UI" commits I pushed and then reverted, as its just clutter (Assuming this will not cause any major problems. Otherwise, I will just leave it as is).
@sakertooth Could you resolve merge conflicts? Once you do that, I will review and test this PR.
@sakertooth Could you resolve merge conflicts? Once you do that, I will review and test this PR.
@PhysSong Just got done doing that :+1: