Linux client

[MirageCode]

I have managed to get the client running on Linux. I can submit a pull request, however, there are a few issues that need ironing out so I thought I would document the necessary changes and the issues I found.

Winsock

Use SDL_net instead.

One problem is that send_info sends the local IP address to server. SDL_net abstracts the socket and doesn't seem to provide an API to retrieve the local address of the socket. It is possible to use SDL_GetLocalAddresses to retrieve all local addresses, but not to determine which of these is actually being used for the connection.

Sending the client machine's local IP address, which is likely on a LAN behind NAT, doesn't seem like a legitimate thing to be doing anyway. The server can obviously still determine the public IP of the client.

Can we just send a placeholder, like 0.0.0.0 or 127.0.0.1, instead?

Windows MessageBox

Use SDL_ShowSimpleMessageBox instead.

gui_keymode

This is a Windows-specific implementation but it can be directly replaced with SDL_GetModState.

gui_sdl_draghack

Don't understand the bug that this is intended to fix. The hack is Windows-specific, but don't know whether the bug is. It looks like the sort of thing that ought to have a better solution.

Removing gui_sdl_draghack works, but obviously risks reintroducing the bug.

SHGetFolderPath for APPDATA

Use SDL_GetPrefPath instead, e.g. SDL_GetPrefPath("Astonia","Astonia 3").

This will likely change the path on Windows from what it was previously, so it will be a breaking change.

save_unique and load_unique

Using the Windows registry obviously isn't portable. Perhaps we should store the values in e.g. unique.txt and usum.txt inside the APPDATA directory?

__declspec

Replace it with a macro from the GCC docs and add -fvisibility=hidden to CFLAGS in the Makefile.

resource.rc

I think this just sets the icon for the executable on Windows. Don't need it on Linux. Perhaps there should be a separate Makefile for Linux which doesn't include it.

SetProcessDPIAware

The documentation recommends setting this in the application manifest instead. Obviously this should only be built into the Windows executable. It isn't needed on Linux.

GetProcessMemoryInfo

This isn't portable. Removing it means losing the "Mem" line in the display output.

GlobalMemoryStatus

This isn't portable either. Removing it means losing list_mem, which just seems to write some notes to the standard output when pressing F10.

dwarfstack

This doesn't seem to be portable at all. Can it be removed?

It looks like what would be lost is the data dump to the log file when the application crashes.

LoadLibrary and GetProcAddress in modder.c

Use SDL_LoadObject and SDL_LoadFunction instead.

Don't know how to test this.

sharedmem.c

Don't know how to test any of this.

I think GetModuleHandle(NULL) and SDL_LoadObject(NULL) are equivalent.

It should be possible to use getpid instead of GetProcessId, possibly including process.h and defining getpid as _getpid on Windows and just including unistd.h on Linux.

Not sure what to do about CreateFileMapping, MapViewOfFile and UnmapViewOfFile. shm_open etc. might work on Linux but probably won't on Windows. Perhaps the solution is to provide separate sharedmem.c implementations for Windows and Linux.

timediff in gui.c main_loop

The client freezes a few seconds after starting but seems to work ok after removing an if-condition and always using the else statement (timediff=1;).

[DanielBrockhaus]

I'm not sure how best to proceed here. I currently do not have the means to test a Linux version.

I guess we could discuss the things you've mentioned one by one, and either switch to the SDL-based solution or introduce a bunch of #ifdefs to have separate solutions on Windows and Linux.

What do you suggest? And would you be willing to test / refine the Linux side of things?

[DanielBrockhaus]

Re: dwarfstack

This is purely because gcc on Windows does not generate usable core dumps on crashes. I'd put everything relating to this behind

ifdef _WIN32

[DanielBrockhaus]

Re: sharedmem.c

This is only used by a single modder. I'd make this Windows only as well.

[DanielBrockhaus]

Re: gui_sdl_draghack

The bug is that if the client window does not have focus and you click but do not release (for example, to try to drag the help window), SDL (or even Windows, I don't know) does not accept any cursor positioning commands until the mouse click is released.

The hack sends a button up / button down command to work around this.

This, too, I would leave Windows only.

[DanielBrockhaus]

Re: timediff in gui.c main_loop

This is supposed to skip drawing frames if the client is falling too far behind the server. If you remove it, you rely on the client being fast enough all the time. That will not be the case for slow machines or high resolutions.

Why would you want to touch this, anyway? It does not look Windows specific to me.

[DanielBrockhaus]

I'll stop here for now and wait for input from you.

[MirageCode]

I suppose I could factor some of the Windows-specific code into separate source files, and conditionally include it on Windows. This would be ok for things like dwarfstack which aren't going to be used on Linux. But I think, for the things which SDL provides cross-platform solutions, we should use them rather than have separate implementations.

I think the SDL_GetPrefPath change should be used on Windows as well. The documentation suggests the path would be similar to what it is currently, but probably not exactly the same (don't know how much of a problem that would be). SDL_ShowSimpleMessageBox might not appear identical either, but should function as intended.

I've also made some changes to the Makefile to use sdl2-config so that all e.g. #include <SDL2/SDL.h> are changed to #include <SDL.h> and the SDL-related include paths, other c flags and linker options are handled automatically.

I've managed to cross-compile the Windows executable on Linux as well, but I haven't fully tested this yet.

Sorry, I should have worded my comment regarding timediff more clearly. I wasn't suggesting this as a solution. I meant that, after I'd managed to get it to compile, I had an issue in running the client as it would freeze after a few seconds (or stop rendering, at least; I could still exit normally by pressing F12, so it was responding to input). I'm not sure why this happened given that it apparently doesn't happen on Windows. I'd narrowed it down to this line in the main loop, but I'll need to get a better understanding of it to come up with an actual solution.

I can submit separate pull requests for some of these things, so that you can review them one-by-one or in small groups, if you like? If so, you can let me know the ones you're happy to go ahead with, and the ones you'd like to discuss further first.

[MirageCode]

I found a solution to GetProcessMemoryInfo and GlobalMemoryStatus with separate platform-specific source files which can be compiled and linked conditionally, depending on the platform. There was a bit more to it than this, and I had to change HeapAlloc to malloc etc.

Just created pull request #19 with these changes, plus changes to make dwarfstack and sharedmem Windows-only and some other general clean-up.

[DanielBrockhaus]

I've had a short look, and it looks like I can merge almost all of this. I'll try to get to it soon. I'm a bit busy with family and friends at this time of year...

[MirageCode]

I've had another look at this and have now got something that is fairly clean and compiles natively on Linux and cross-compiles for Windows on Linux using MinGW-64 without producing any warnings on either. A few of the things I've changed might be opinionated, so perhaps it would be best for me to wait until you've had time to review the current pull request and then create more pull requests with batches of changes, as the changes might be dependent on each other and it might make it easier to discuss and potentially amend smaller pull requests rather than go through everything at once.

Aside from some fixes I've made which had clear solutions that probably don't warrant discussion, and others I've made based on your feedback so far, I'll note the things I have done here, if only for my reference.

Winsock

I've changed the target_server global to store the same as the server_url rather than the IP address, so that all SDL_net code is in client.c and the IP is resolved when connecting. The "Using login server" note uses the SDLNet_ResolveIP output, which might be a host name rather than an IP address as before. I think this is the only difference that is visible to the user.

I've left the first four bytes (the local address) of the send_info packet empty. As far as I can tell, the server just logs this value and it doesn't actually do anything.

save_unique and load_unique

I've kept the current implementation (using the Windows registry) and made it Windows-only. This means the unique value in the send_info packet on Linux is probably always 0. Don't know whether this matters.

SHGetFolderPath for APPDATA

Looking at the source code for the Windows implementation of SDL_GetPrefPath, it looks like the org is optional and so using SDL_GetPrefPath(NULL,"Astonia") will produce the same result as before and not be a breaking change.

LoadLibrary and GetProcAddress in modder.c

I haven't tested this but the source code for the Windows implementation of SDL_LoadObject and SDL_LoadFunction uses LoadLibrary and GetProcAddress internally anyway, so I'm confident it will be fine.

SetProcessDPIAware

I've created a moac.exe.xml application manifest file with the relevant dpiAware and dpiAwareness settings and referenced it in resource.rc. I'm not familiar with Windows application manifests or resource files in general. I've checked the output from windres and I think it is correct, but I haven't tested further than this.

timediff in gui.c main_loop

I didn't get as far as fully understanding the issue, but I found that changing the type of timediff from int to int64_t fixes it. This is probably an adequate solution.

[DanielBrockhaus]

I really appreciate your work. But I'm feeling not up to working on Astonia at the moment. Health issues...

I'll try to come back to this soon, but can't make any promises.

[MirageCode]

No problem. Take it easy. I can make any necessary changes whenever you get around to checking it.

Just in case anyone wants to play with it in the meantime, I've pushed all of my changes to a branch in my fork.