-
Notifications
You must be signed in to change notification settings - Fork 39
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
Online client: build using MinGW and CMake #151
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CMake and Nix part LGTM.
Someone should confirm if it still works with MSVC/VisualStudio
I have confirmed it builds successfully on Visual Studio 2022 and detects Duckstation on Windows just fine like before. 😀
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested again, this time with really running it. 🏁
You could add artefacts to the CI by adding to .github/workflows/main.yml
:
- name: Build Server Release Mingw2 GCC
run: nix build -L --keep-going '.?submodules=1#online-server.release.mingw32.gcc' -o ${{ runner.temp }}/result/online-server.release.mingw32.gcc
+ - name: Build Client Release Mingw2 GCC
+ run: nix build -L --keep-going '.?submodules=1#online-client.release.mingw32.gcc' -o ${{ runner.temp }}/result/online-client.release.mingw32.gcc
- name: Upload artifact
uses: actions/upload-artifact@v4
with:
name: online-server-windows
path: |
${{ runner.temp }}/result/online-server.release.mingw32.gcc/bin
+ ${{ runner.temp }}/result/online-client.release.mingw32.gcc/bin
Yes I thought about doing that. On my machine I used the mingw64 build though. Not sure what is preferable? |
Sorry, only yesterday I learned that the online client does not need to be built in 32 bits (PC port of the game needs) Use the 64 one, that's what they're providing for users. |
I've updated it to build the new cpp and PINE based client. It needed a couple harmless changes, but I'm not 100% sure about the needed change in I could use the client (using wine) against the native linux Duckstation, but on linux duckstation uses a unix socket for pine while the client uses TCP so I had to proxy that using socat, running this on top of the client:
|
I'm not sure about global.h, as that static assert stuff wasn't mine. |
looks right to me, it's inside the GCC/Clang pre-processor-branch, and if it detects window.h (so it has to be Mingw's GCC and not Linux's GCC) then it does the same thing it does on MSVC. |
This adds support to build the client using CMake and MinGW, so it can be cross-built on Linux. I re-used the nix setup that was done for the server (fun to see nix in this project!). I tested the built client, using MinGW. I don't have Windows/VisualStudio to build it on Windows.
I would like to look into porting the client to Linux and porting the memory map to PINE, but I thought it would be better if I can build and test the Windows one first, to compare and make sure I don't break things.
Most of the CMake file is from #131
I had to do two changes to the client, one is to add
#include <psapi.h>
which seems harmless.The second one is to change
OpenFileMapping
toOpenFileMappingW
. This seems to be what's documented, but I'm not familiar with the Windows APIs so I'm not sure why it was needed (I assume for wide-char support) and whether it's fine for the Windows build. https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-openfilemappingw