linux and unix build support #37

arrayofchar · 2019-10-27T05:00:02Z

Merges #17 and #21 to conform to the new cross-platform abstraction. Build succeeded on OSX and Ubuntu.

Shugyousha · 2019-10-27T10:47:22Z

src/CMakeSources.cmake

+		set(GGPO_LIB_SRC_NOFILTER
+			${GGPO_LIB_SRC_NOFILTER}
+			"lib/ggpo/platform_linux.h"
+			"lib/ggpo/platform_linux.cpp"


platform_linux.* and platform_unix.* are almost identical. We can probably just use platform_unix.* on both MacOS and Linux (potentially with an #ifdef or two).

I totally agree.

Shugyousha · 2019-10-27T11:05:23Z

src/lib/ggpo/pevents.cpp

+#endif
+} // namespace neosmart
+
+#else //_WIN32


We only include pevents.h for Unix-like platforms so we could just delete everything in this else clause.

pond3r

Thanks for all the work! Most of these changes look good and can be pulled once some feedback has been addressed. There are two big issues which would prevent the pull:

It looks like this will conflict with some of the GGPO_EXPORT work done in a different pull request.
I'd prefer writing POSIX native implementations in the Platform object to pulling in Win32 Events for POSIX.

#1 can be fixed by pulling a new version of ponder:dev/crossplatform into your branch and resolving conflicts, I think. For #2, maybe it makes sense to revert those changes and re-submit the pull request. It won't actually compile, but it will get us closer and will avoid future conflicts as we move toward a cross platform world (specifically, the OSX people would be happy to get the other changes, I'm sure).

Thanks!

pond3r · 2019-10-27T19:09:44Z

src/apps/vectorwar/gdi_renderer.cpp

@@ -168,7 +168,7 @@ GDIRenderer::DrawConnectState(HDC hdc, Ship &ship, PlayerConnectionInfo &info, C

      case Disconnecting:
         sprintf(status, "Waiting for player...");
-         progress = (timeGetTime() - info.disconnect_start) * 100 / info.disconnect_timeout;
+         progress = (Platform::GetCurrentTimeMS() - info.disconnect_start) * 100 / info.disconnect_timeout;


The Platform type is an internal implementation detail of the GGPO SDK. Ideally the sample code wouldn't use it (or even be able to include those dependencies)

pond3r · 2019-10-27T19:11:05Z

src/include/ggponet.h

@@ -8,6 +8,21 @@
 #ifndef _GGPONET_H_
 #define _GGPONET_H_

+#if defined(_MSC_VER)


This pull request did something similar which will conflict with a lot of these changes. Could you re-pull dev/crossplatform into your fork and restage?

pond3r · 2019-10-27T19:12:20Z

src/lib/ggpo/backends/synctest.cpp

@@ -167,7 +167,9 @@ SyncTestBackend::RaiseSyncError(const char *fmt, ...)
   va_end(args);

   puts(buf);
+#ifdef _WIN32


We can just delete this instead of the #ifdef

pond3r · 2019-10-27T19:13:04Z

src/lib/ggpo/main.cpp

@@ -11,12 +11,18 @@
 #include "backends/spectator.h"
 #include "ggponet.h"

+#ifdef _WIN32


Can we move these functions into the platform specific files instead (e.g. DllMain goes into platform_windows.cpp)?

pond3r · 2019-10-27T19:18:20Z

src/lib/ggpo/network/udp.cpp

@@ -83,7 +90,11 @@ Udp::OnLoopPoll(void *cookie)
 {
   uint8          recv_buf[MAX_UDP_PACKET_SIZE];
   sockaddr_in    recv_addr;
+#ifdef _WIN32


I think the definition of socklen_t is in WS2tcpip.h. Let's just include that in platform_window.h and remove these ifdefs.

pond3r · 2019-10-27T19:20:01Z

src/lib/ggpo/network/udp_proto.cpp

@@ -296,8 +296,13 @@ UdpProtocol::HandlesMsg(sockaddr_in &from,
   if (!_udp) {
      return false;
   }
+#ifdef _WIN32


Windows has a #define s_addr S_un.S_addr in WinSock2.h, so I bet you can just use the else version on all platforms.

pond3r · 2019-10-27T19:23:38Z

src/lib/ggpo/platform_linux.cpp

-
-    return ((current.tv_sec - start.tv_sec) * 1000) +
-           ((current.tv_nsec  - start.tv_nsec ) / 1000000) +
+uint32_t Platform::GetCurrentTimeMS() {


Here's my understanding... GGPO will use this function to measure elapsed time (i.e. subtracting one time from another). gettimeofday will return the time according to the system clock, which might get adjusted based on an update from an NTP time source between the two calls. This could make it appear as if an operation took a negative amount of time. See https://blog.habets.se/2010/09/gettimeofday-should-never-be-used-to-measure-time.html for more information. Thoughts?

pond3r · 2019-10-27T19:24:37Z

src/CMakeSources.cmake

+		set(GGPO_LIB_SRC_NOFILTER
+			${GGPO_LIB_SRC_NOFILTER}
+			"lib/ggpo/platform_linux.h"
+			"lib/ggpo/platform_linux.cpp"


I totally agree.

pond3r · 2019-10-27T19:26:36Z

src/lib/ggpo/pevents.h

@@ -0,0 +1,42 @@
+/*


I think you only need this to implement Poll. IMO, it would be better to move the functionality of Poll into Platform and write a POSIX native implementation rather than mapping to the Win32 API.

pond3r · 2019-10-27T19:27:20Z

src/lib/ggpo/platform_linux.h

+
+#include "pevents.h"
+
+#define DebugBreak() (raise(SIGTRAP))


Ideally, none of the definitions from here to line 44 would be required, since they're specifically platform specific.

andrewdbanks · 2020-09-01T00:26:44Z

Merges #17 and #21 to conform to the new cross-platform abstraction. Build succeeded on OSX and Ubuntu.

I admit I'm not usually this far into the weeds on open source projects, so I'm sure there's something obvious I'm missing trying to build this on Kubuntu right now, but my build is failing on strncat_s and sprintf_s calls, which appear to have been committed to the repo specifically to resolve compiler issues. The best answers I can find on Google are that these are Microsoft-oriented C++ extensions, but the commits also coincide with the multiplatform efforts timestamps on this project. Is there a step I'm missing to get this building properly on Linux? With other operating systems labeled as still "in progress", the wiki build instructions are a little light.

henrebotha · 2022-10-28T12:30:43Z

What's the build process for this? cmake . && make?

Bo Qin added 2 commits October 26, 2019 19:24

Cross-platform support for *nix platforms

dbdc1bb

fix for linux build. linux build successful

86673e8

arrayofchar mentioned this pull request Oct 27, 2019

Add Linux support #19

Open

Shugyousha reviewed Oct 27, 2019

View reviewed changes

pond3r reviewed Oct 27, 2019

View reviewed changes

Shugyousha mentioned this pull request Jan 4, 2020

Linux/Unix support, continued from PR #37 #52

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

linux and unix build support #37

linux and unix build support #37

arrayofchar commented Oct 27, 2019

Shugyousha Oct 27, 2019

pond3r Oct 27, 2019

Shugyousha Oct 27, 2019

pond3r left a comment

pond3r Oct 27, 2019

pond3r Oct 27, 2019

pond3r Oct 27, 2019

pond3r Oct 27, 2019

pond3r Oct 27, 2019

pond3r Oct 27, 2019

pond3r Oct 27, 2019

pond3r Oct 27, 2019

pond3r Oct 27, 2019

pond3r Oct 27, 2019

andrewdbanks commented Sep 1, 2020

henrebotha commented Oct 28, 2022

linux and unix build support #37

Are you sure you want to change the base?

linux and unix build support #37

Conversation

arrayofchar commented Oct 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pond3r left a 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

andrewdbanks commented Sep 1, 2020

henrebotha commented Oct 28, 2022