search

projectM-visualizer / projectm

projectM - Cross-platform Music Visualization Library. Open-source and Milkdrop-compatible.
https://discord.gg/mMrxAqaa3W
GNU Lesser General Public License v2.1
3.22k stars 364 forks source link

Tweak to pmSDL.hpp and .gitignore #750

Closed serjykalstryke closed 6 months ago

serjykalstryke commented 6 months ago

I was having trouble building the library in totality due to some errors in the projectm-test-ui project. I was able to trace the errors to the includes in the file pmSDL.hpp. By tweaking the order to go in the order at which matches the call stack (this is I think the source of the issue. D needs C needs B needs A, so I set it to A, B, C, D, if that makes sense)

I also used vcpkg to install dependencies, so I added the ./vcpkg-install directory to .gitignore so that people who also use vcpkg don't accidentally push their dependencies to their own or the main repo.

--David Stinnett

kblaschke commented 6 months ago

Oh, ignore the failing Emscripten build. We've just fixed it in our master. You could rebase your master branch on that as well if you like, but it's not necessary for this PR.

serjykalstryke commented 6 months ago

Alright, all the commits are squashed down into one and then I force pushed to update.

kblaschke commented 6 months ago

Something seems to have gone wrong, there's now an additional merge commit in the opposite direction. To pull in upstream changes, I'd highly recommend always using git pull --rebase instead of the default merge operation.

In addition to that, the .gitmodules file was renamed to .gitmodules.txt, which would cause the submodules to fail.

If it's okay for you, I'd cherry-pick your changes into an extra branch and then create a PR for that one, even if that means eventually opening a third one. You can then simply reset your fork's master branch on upstream again to sync it without getting any merge commits or conflicts.

kblaschke commented 6 months ago

Cherry-picked and signed your commits into a new branch and created a new PR for this one: #753 Closing this here.

serjykalstryke commented 6 months ago

Ah yeah, ultimately that is fine with me, since I couldn’t do the merge anyways.

I still get credit for the contribution right? /j

kblaschke commented 6 months ago

Sure, the commits still bear your author credentials, I made sure to add these when committing. Git shows this as "authored by serjykalstryke, committed by kblaschke", as there are separate fields for this.