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

Disable packet tagging on MingW by default, fix TLS qualifier #134

Merged

Conversation

jclc
Copy link
Contributor

@jclc jclc commented Jan 25, 2025

Added back netcode_enable_packet_tagging() even when NETCODE_PACKET_TAGGING=0 because otherwise code calling it (Yojimbo) would not compile.

@gafferongames
Copy link
Contributor

Thank you! ps. To make sure netcode keeps working with MingW do you think you could modify the GitHub actions script to have a build in MingW test? This way the work you have done will get locked in and keep working even when other people make changes.

@jclc
Copy link
Contributor Author

jclc commented Jan 25, 2025

I can check that out, but I'm not at all familiar with GitHub actions.

@gafferongames
Copy link
Contributor

They're pretty simple. They are defined here:

https://github.com/mas-bandwidth/netcode/tree/main/.github/workflows/ci.yml

@jclc
Copy link
Contributor Author

jclc commented Jan 26, 2025

Could you let me know if this makes any sense? Let's say I add the following after the vs2019 steps:

    # Set up msys2/MingW-w64 toolchain
    - name: Setup (msys2)
      if: runner.os == 'Windows'
      uses: msys2/setup-msys2@v2

    - name: Build (msys2)
      if: runner.os == 'Windows'
      run: |
        premake5 gmake
        make config=${{ matrix.configuration }}

    - name: Test (msys2)
      if: runner.os == 'Windows'
      run: "& ./bin/test.exe"

Or should this be made an entirely separate job? Also, can I run the pipelines on my fork to test this?

@gafferongames
Copy link
Contributor

gafferongames commented Jan 26, 2025 via email

@gafferongames
Copy link
Contributor

Almost there!

@jclc
Copy link
Contributor Author

jclc commented Jan 30, 2025

Sorry about the spam. I don't have a local msys2 environment available so I can't really run these locally.

All that's missing is linking to ws2_32 and IPHLPAPI. I've never used premake before, could you tell me how to do this only when compiler is MingW (or target is Windows but compiler is not MSVC)?

@jclc
Copy link
Contributor Author

jclc commented Jan 30, 2025

Okay the workflow passes now. Sorry for the spam again.

@gafferongames
Copy link
Contributor

No problem at all!

@gafferongames gafferongames merged commit 4398d61 into mas-bandwidth:main Jan 30, 2025
8 checks passed
@gafferongames
Copy link
Contributor

Woo!

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.

2 participants