Novum / vkQuake

Vulkan Quake port based on QuakeSpasm
GNU General Public License v2.0
1.72k stars 214 forks source link

Make vq_pak's Makefile update embedded_pak.c #651

Closed Macil closed 1 year ago

Macil commented 1 year ago

While working on MacOS, it took me a while to figure out how I could update vkquake.pak correctly. (I don't have any PRs planned that update vkquake.pak. I'm just experimenting with it in my own fork.) I eventually figured out it was a multi-step process: run make in the Misc/vq_pak directory, move both vkquake.pak and vkquake.pak.deflate to the Quake folder, and then use bintoc from the build directory to make a new embedded_pak.c file. The third step was automated inside of the Visual Studio project but not in any of the other platforms' build scripts.

This PR streamlines things by making Misc/vq_pak's Makefile produce embedded_pak.c directly in the proper place, killing steps 2 and 3. The third step in the VS project config and the committed copies of vkquake.pak and vkquake.pak.deflate are removed. (I made the change through Visual Studio and made sure the build still worked.)

The vq_pak Makefile does compile its own bintoc copy now. It was a bit convenient before that when the third step was in the (VS) build configuration it could easily use the same bintoc copy used for shaders, but even with this change the vq_pak Makefile is still very simple and I think the streamlining of this PR outweighs that small downside.

(Also fixes a minor issue where vq_pak's Makefile didn't specify vkquake.pak.deflate as a target, so if you previously created vkquake.pak but failed to create vkquake.pak.deflate (like because zopfli wasn't installed), then running make again would end immediately instead of creating vkquake.pak.deflate.)

Novum commented 1 year ago

I really disagree with messing with the Windows build. That's my main work environment, and I don't want to use makefiles for anything on Windows.

Macil commented 1 year ago

Do you use the vq_pak Makefile on Windows to make the vkquake.pak file, do you manually make it, or use another script/tool/config in the repo? If the vq_pak Makefile isn't part of your workflow then I definitely agree this PR is a bit too aggressive by centralizing steps in it.

Macil commented 1 year ago

I'll change the PR so that the VS project is untouched, and the Makefile also directly writes to Quake/vkquake.pak and Quake/vkquake.pak.deflate along with Quake/embedded_pak.c, so that Windows workflows work as before.

I think the ideal would be if I could make a manually-runnable task (that doesn't run automatically like on every build) within the VS project that directly generates Quake/embedded_pak.c, doing the same steps as the vq_pak Makefile, but I'm not familiar enough with VS to do that. Another route would be to make the VS project always produce Quake/embedded_pak.c during build from its source files in Misc/vq_pak, but if the build process isn't deterministic, we wouldn't want Quake/embedded_pak.c committed in the repo because it might change each build, and if it's not in the repo then all of the other platform build scripts will need to be updated to generate the file too, which would be consistent and a good setup but it's a bit more than I want to bite off right now.

Novum commented 1 year ago

I honestly almost never touch the pak file

Macil commented 1 year ago

I've changed the PR to be much less aggressive: Quake/vkquake.pak{,.deflate} are still checked into the repo and processed into Quake/embedded_pak.c by the VS build config. Any Windows workflow (where the user manually creates the Quake/vkquake.pak{,.deflate} files to be consumed by VS) will work as before, but anyone using the vq_pak Makefile will now find that it's a one-stop shop doing all that's necessary: it produces Quake/vkquake.pak{,.deflate} (for the benefit of future VS builds which expect these files) and Quake/embedded_pak.c (which wasn't previously buildable outside of VS).