Port for windows #18

luizfernandonb · 2022-07-09T01:20:39Z

Well, it's functional and it compiles and runs perfectly and without warnings on Windows, I haven't tested it on Linux distros or on MacOS because I don't have it in my hands, but I would be grateful if someone could talk later if there was any problem compiling/running on Linux/MacOS.

The only defect is in the console, which in Windows the escape characters are different, so the text output looks like this:

I'll fix this and do a PR when I can.

Haxk20 · 2022-07-09T01:43:58Z

Source/Server.c

@@ -545,7 +545,7 @@ void StopServer()
    server.running = 0;
 }

-static void* serverConsole(void* arg)
+static void serverConsole(void* arg)


Leave it as pointer please. Dont pretype it in the actual passtrough.

Haxk20 · 2022-07-09T01:44:17Z

Source/Server.c

    exit(-1);
-
-    return 0;


Haxk20 · 2022-07-09T01:47:21Z

Source/Server.c

@@ -571,13 +571,14 @@ static void* serverConsole(void* arg)
    rl_clear_history();
 #endif

-    server_sleep(5); // seconds
+    StopServer();


This is not needed at all. Like it was there before. Rebase your commits. Dont fix up in later ones :|

Haxk20 · 2022-07-09T01:47:54Z

Source/Util/Functions.c

@@ -9,7 +9,7 @@
 void server_sleep(uint64 seconds)
 {
 #ifdef WIN32
-    Sleep(seconds * 1000);
+    Sleep(seconds * 1000); // The sleep function in Windows is in milliseconds


I mean kinda pointless comment. Its kinda obvious.

Haxk20 · 2022-07-09T01:48:39Z

Source/Server.c

        LOG_DEBUG("epic write fail");
    }
+


Pointless new line

Haxk20 · 2022-07-09T01:49:18Z

Source/Server.c

-    if (write(STDIN_FILENO, "\n", sizeof("\n")) != sizeof("\n")) {
+
+    // 0 = STDIN_FILENO
+    if (write(0, "\n", sizeof("\n")) != sizeof("\n")) {


No. Just hell no. If it doesnt exist on windows. Define it.

Haxk20 · 2022-07-09T01:50:55Z

Source/Protocol.c

 uint64 getNanos(void)
 {
    struct timespec ts;
+


No. Leave it as it is. It works on windows as it is. Adds pointless complexity.

You literally just copied what mingw does in the clock_gettime functions. Like why ?

clock_gettime is a POSIX function, not from Windows

since mingw is ok with it, perhaps wrap it in #ifdef MSVC ot something

Haxk20 · 2022-07-09T01:56:41Z

CMakeLists.txt

-  if (WIN32)
-    set_target_properties(enet::enet PROPERTIES
-      INTERFACE_LINK_LIBRARIES "ws2_32;winmm"
+if(WIN32)


Afaik ByteBit builds it without this mess ? And it works. You are also basically FORCING users to build with Visual Studio on windows. Which i do not like.

Haxk20 · 2022-07-09T01:58:08Z

Source/Util/Functions.c

@@ -0,0 +1,16 @@
+#include "Functions.h"


No need for 2 new files. Put it in 1 of the already existing files.

Haxk20 · 2022-07-09T01:58:17Z

Source/Util/Functions.h

@@ -0,0 +1,5 @@
+// Copyright Luiz Fernando 2022


Same. No need.

Haxk20 · 2022-07-09T02:00:21Z

Source/Protocol.c

@@ -22,10 +22,37 @@
 #include <string.h>
 #include <time.h>

+#ifdef WIN32


Including Windows api just for 1 function call. Like grrrrr. Let migw handle that.

#define WIN32_LEAN_AND_MEAN and we're good :)

Haxk20 · 2022-07-09T02:08:09Z

The entire PR is a mess. Truly. Commits are not properly squashed. Naming is completely off. Unneccessary changes at best. And i could go on. Also complaining that the escape character is wrong for windows while not actually fixing it in the PR.

The project DOES compile on windows AS IS. Without any additional other changes. In these changes you are FORCING users into Visual Studio and mingw or clang. Which they may not want to use or etc.

Considering that it builds on Windows AS IS this likely wont get merged unless there is a truly solid counter argument but even them im on the edge. Fix your build system. Not the project.

The main focus is Linux. And it will continue to be. Who wants to run game server on Windows ? You buy a cheapo VPS and run it on that.
That being said ByteBit did amazing changes to make it work on Windows which were minimal and they continue to work like that. No further tweaks were needed.
The recent builds failed because of pthreads. Which i just have to reinclude 1 library to fix that.

But lets see if it even builds on linux. (It doesnt).

But even then. As i said. It builds as is on Windows. And this just adds mess and complexity to something that already works.

luizfernandonb · 2022-07-09T02:38:10Z

The entire PR is a mess. Truly. Commits are not properly squashed. Naming is completely off. Unneccessary changes at best. And i could go on. Also complaining that the escape character is wrong for windows while not actually fixing it in the PR.

The project DOES compile on windows AS IS. Without any additional other changes. In these changes you are FORCING users into Visual Studio and mingw or clang. Which they may not want to use or etc.

Considering that it builds on Windows AS IS this likely wont get merged unless there is a truly solid counter argument but even them im on the edge. Fix your build system. Not the project.

The main focus is Linux. And it will continue to be. Who wants to run game server on Windows ? You buy a cheapo VPS and run it on that. That being said ByteBit did amazing changes to make it work on Windows which were minimal and they continue to work like that. No further tweaks were needed. The recent builds failed because of pthreads. Which i just have to reinclude 1 library to fix that.

But lets see if it even builds on linux. (It doesnt).

But even then. As i said. It builds as is on Windows. And this just adds mess and complexity to something that already works.

If so, do you compile with MinGW?

Haxk20 · 2022-07-09T08:12:06Z

The entire PR is a mess. Truly. Commits are not properly squashed. Naming is completely off. Unneccessary changes at best. And i could go on. Also complaining that the escape character is wrong for windows while not actually fixing it in the PR.
The project DOES compile on windows AS IS. Without any additional other changes. In these changes you are FORCING users into Visual Studio and mingw or clang. Which they may not want to use or etc.
Considering that it builds on Windows AS IS this likely wont get merged unless there is a truly solid counter argument but even them im on the edge. Fix your build system. Not the project.
The main focus is Linux. And it will continue to be. Who wants to run game server on Windows ? You buy a cheapo VPS and run it on that. That being said ByteBit did amazing changes to make it work on Windows which were minimal and they continue to work like that. No further tweaks were needed. The recent builds failed because of pthreads. Which i just have to reinclude 1 library to fix that.
But lets see if it even builds on linux. (It doesnt).
But even then. As i said. It builds as is on Windows. And this just adds mess and complexity to something that already works.

If so, do you compile with MinGW?

I do not but ByteBit does yes.

Haxk20 · 2022-07-09T09:21:46Z

But i would like to see MSVC support. Make the PR the bare minimum to build on MSVC WITHOUT breaking MinGW or Linux builds.

https://discord.gg/yegsgU2D
You can join this discord to discuss if you want :)

rndtrash · 2022-07-09T09:26:27Z

we sure need MSVC support since most of the users rely on it. plus, vcpkg is a great addition actually! just add the mingw support as an alternative, fix the linux build, perhaps add some missing #define-s and its ready to merge!

Haxk20 · 2022-07-09T09:27:00Z

we sure need MSVC support since most of the users rely on it. plus, vcpkg is a great addition actually! just add the mingw support as an alternative, fix the linux build, perhaps add some missing #define-s and its ready to merge!

And massive cleanup of the other unneccesary changes ;)

luizfernandonb · 2022-07-09T12:56:06Z

But i would like to see MSVC support. Make the PR the bare minimum to build on MSVC WITHOUT breaking MinGW or Linux builds.

https://discord.gg/yegsgU2D You can join this discord to discuss if you want :)

It's impossible to compile in MSVC without changing something, practically all POSIX functions are not in it, I don't think it's worth it, maybe a solution is to use MingGW, but I'm seeing if it's possible to use it in Visual Studio

luizfernandonb · 2022-07-09T18:23:25Z

I didn't find a viable way and that doesn't give so much work to use MinGW + CMake + VCPKG on Windows, I'm going to close this PR, I hope someone in the future can port the server to Windows.

Anyway, thanks everyone.

luizfernandonb added 4 commits July 8, 2022 21:12

Making the server compile on Windows

779e13d

Ignoring things that don't matter

dbb5b03

Latest changes

6510bf8

Fixes

bc2f28d

Haxk20 requested changes Jul 9, 2022

View reviewed changes

luizfernandonb closed this Jul 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port for windows #18

Port for windows #18

luizfernandonb commented Jul 9, 2022

Haxk20 Jul 9, 2022

Haxk20 Jul 9, 2022

Haxk20 Jul 9, 2022

Haxk20 Jul 9, 2022

Haxk20 Jul 9, 2022

Haxk20 Jul 9, 2022

Haxk20 Jul 9, 2022

luizfernandonb Jul 9, 2022

rndtrash Jul 9, 2022

Haxk20 Jul 9, 2022

Haxk20 Jul 9, 2022

Haxk20 Jul 9, 2022

Haxk20 Jul 9, 2022

rndtrash Jul 9, 2022

Haxk20 commented Jul 9, 2022 •

edited

luizfernandonb commented Jul 9, 2022

Haxk20 commented Jul 9, 2022

Haxk20 commented Jul 9, 2022

rndtrash commented Jul 9, 2022

Haxk20 commented Jul 9, 2022

luizfernandonb commented Jul 9, 2022

luizfernandonb commented Jul 9, 2022

Port for windows #18

Port for windows #18

Conversation

luizfernandonb commented Jul 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Haxk20 commented Jul 9, 2022 • edited

luizfernandonb commented Jul 9, 2022

Haxk20 commented Jul 9, 2022

Haxk20 commented Jul 9, 2022

rndtrash commented Jul 9, 2022

Haxk20 commented Jul 9, 2022

luizfernandonb commented Jul 9, 2022

luizfernandonb commented Jul 9, 2022

Haxk20 commented Jul 9, 2022 •

edited