Move to CMake and add linux support

[GoGoOtaku]

Changes were done to rework most system specific code to be platform independent. This not only gives Linux support but also might open the doors for other operating systems. I do not own an Apple tho so I can not promise anything on that front (especially since Apple kicked out OpenGL iirc). There are also some bug fixes that I ran across while testing and playing the game. Finally I ran valgrind over the tests to check for memory leaks and fixed those. I do not use Windows anymore really so I haven't tested this with Visual Studio but it compiled fine using MinGW (see TC-mingw.cmake for a cross compilation template under Linux) and also ran fine with Wine.

I would REALLY like if somebody on the windows side of things could check if this still works with Windows and Visual Studio.

• Removed Visual Studio files
• Removed SDL2 files (CMake handles dependency)
• Removed googletest files (CMake handles dependency)
• Moved Windows resources (icon) out of src
• Reworked Win32 to compatible System
• Rework code to use C++17 std::filesystem::path
• Rework WinMain to standard main and let SDL2main and GTestmain handle Windows
• Add GNU/Linux support
• Fixed lightning effect in Catacombs Adventures
• Fixed several memory leaks (mostly in test code)

[GoGoOtaku]

This would fix #12 and would make #10 trivial (via CPack a feature of CMake)
I haven't tested it but since most raspberries run Linux and SDL2 supports raspberry it MIGHT also close #11

[ArnoAnsems]

Thank you so much, this is absolutely amazing! I can tell from all the code changes that you spent quite some time on this one. No doubt others will appreciate this feature as well.
I will need some time to go through the changes and check it out with Visual Studio. Regarding CMake I also have to go through a bit of a learning curve, to be honest. But I'm looking forward to having this feature merged in and published in a new version of the source port.
Thanks again and I'll keep you posted!

[ArnoAnsems]

Update: I did some testing with your branch. Generating Visual Studio project files with CMake was no problem. I did have to make some small changes in the source code to make it compile with the Visual Studio toolset, but I don't think those changes will break Linux support. I could start up the source port in both release and debug mode. Gameplay and visuals seem fine. The only bug that I spotted so far is that when browsing for game data, I cannot switch to a different drive, for example from C: to D:. I can imagine that this could be an OS specific thing.
As the next step, I would like to merge your pull request as-is into the main branch. Then I'll deliver a commit to make it build with the Visual Studio toolset. And then I'll just continue from there with some more testing and if needed some bug fixing. Do you think that's a good approach?

[GoGoOtaku]

I updated the lightning code as talked about. I think we can merge this now. There is one very specific problem under Linux but that is a bug in SDL which can be worked around very easily.

fyi: To be more specific it is the interaction between Wayland and SDL which causes SDL to think focus was lost. This causes the menu to pop up randomly. This can be worked around by setting SDL_VIDEODRIVER=x11 in the environment and like I said it is a bug in SDL itself.

PS: Do you need to generate Visual Studio files? Last I used Visual Studio you could just open the CMakelist.txt and Visual Studio would just do everything under the hood.

[NY00123]

Very nice! Last time I checked the option, I eventually got just to replacing backslashes with slashes in #include directives.

I could get this to run after making a few adjustments. I still use Ubuntu 20.04, and it includes CMake 3.16.3 out-of-the-box. Reverting the minimum required version to 3.16 in all CMakeLists.txt files was one step required for me.

Afterwards, I got this error:

  CMake Error at CMakeLists.txt:19 (add_library):
  Target "CatacombGL_Objects" links to target "SDL2::SDL2" but the target was
  not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?

I found a solution here: RobertBeckebans/RBDOOM-3-BFG#469 (comment)

Here's the relevant change to the top-level CMakeLists.txt file (at least for me):

-target_link_libraries( CatacombGL_Objects
-    SDL2::SDL2
-)
+if(SDL2_LIBRARIES)
+       include_directories(${SDL2_INCLUDE_DIRS})
+       target_link_libraries( CatacombGL_Objects
+           ${SDL2_LIBRARIES}
+       )
+else()
+       target_link_libraries( CatacombGL_Objects
+           SDL2::SDL2
+       )
+endif()

[NY00123]

Also, if you start Catacomb Abyss v1.24 for the very first time (while pathabyssv124 isn't yet set), CatacombGL will crash upon calling fs::create_directories with an empty path. It looks like code specific to this branch. I was running CatacombGL from a directory with a copy of the game.

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff788a859 in __GI_abort () at abort.c:79
#2  0x00007ffff7c64911 in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007ffff7c7038c in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007ffff7c703f7 in std::terminate() () from /lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00007ffff7c706a9 in __cxa_throw () from /lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x00007ffff7c697a2 in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6
#7  0x000055555557a0ac in SystemSDL::GetSavedGameNamesFromFolder (this=0x7fffffffcda0, 
    path=filesystem::path "", filesFound=std::vector of length 0, capacity 0)
    at /home/ny00123/hdd/Games/catacomb/CatacombGL/src/System/SystemSDL.cpp:53
#8  0x00005555555a6e6a in EngineCore::EngineCore (this=0x555555edf8f0, game=..., system=..., 
    keyboardInput=..., configurationSettings=...)
    at /home/ny00123/hdd/Games/catacomb/CatacombGL/src/Engine/EngineCore.cpp:139
#9  0x00005555555638bc in main ()
    at /home/ny00123/hdd/Games/catacomb/CatacombGL/src/System/CatacombGL.cpp:372

[GoGoOtaku]

OK. I have reduced the CMake version to 3.16. I had it set to 3.18 as I chose Debian Stable as a base. Tests at the time was set higher as I figured that running tests wouldn't be the norm. I checked again and the current code does run with 3.16 so I also lowered that.

The SDL2 issue was actually an oversight on my part. I prefer using SDL2::SDL2 as a target can have more properties than just the includes and libraries but I did not think about if SDL2 always had the target. Turns out the target was added in SDL2 Version 2.0.12. The RBDOOM3-BFG work around would not work tho as it would always find SDL2_LIBRARIES even if SDL2::SDL2 is available.

The crash should also be fixed and as a bonus I also find a very silly bug when saving the game.

Thank you for the feedback.

PS: Not wanting to throw shade at you but just a heads up that Canonical doesn't support your version of Ubuntu anymore and you should probably upgrade at some point. 🙂

[ArnoAnsems]

@NY00123 Thank you for your helpful and detailed feedback, I appreciate it!
@GoGoOtaku Thank you for going the extra mile and reworking the code based on NY00123's feedback. I actually wasn't aware that Visual Studio has built-in support for CMake and as a result can simply open CMakeLists.txt files. In my day job we generate the Visual Studio project files from the CMake files during an automated build process, so I didn't consider that it can be done faster. I learned something new today. :)

As discussed, I've merged the pull request and added a small commit to get it to build with Visual Studio.
I'll do some more testing to prepare for an updated release of the source port.

[NY00123]

Nice, thanks! The problems I had are now fixed. One other thing I've spotted is the path to the ini file being outputted to stdout on quit, is that on purpose? From CatacombGL.cpp:

    // Store configuration
    std::cout << configFilename.string() << "\n";

PS: Not wanting to throw shade at you but just a heads up that Canonical doesn't support your version of Ubuntu anymore and you should probably upgrade at some point. slightly_smiling_face

Actually, Ubuntu 20.04 is an LTS release, so it's expected to be supported until April 2025. Even Ubuntu 18.04 still has about 6 months left.

This might not be a common occurrence for me nowadays, but when I release Linux binaries of my own for a project, I actually have a habit of using a chroot with an older version of Ubuntu for building them; Continuous integration might also assist here if covering a relevant distribution.

[GoGoOtaku]

Actually, Ubuntu 20.04 is an LTS release,

Oh, Ubuntu has multiple LTS releases. I just saw that the current LTS was 22.04 (according to their download site)
Sorry for the confusion.

One other thing I've spotted is the path to the ini file being outputted to stdout on quit, is that on purpose?

Oh no. I think I added this quite early on when I added the config paths for Linux and after a while I forgot and thought that line was always there.

[stuaxo]

I tried this on Ubuntu 22.04, and it's complaining about SDL missing when I try and compile, is this something cmake could detect instead ?

❯ make                     
[  1%] Building CXX object CMakeFiles/CatacombGL_ThirdParty.dir/ThirdParty/RefKeen/be_st_sdl_audio_timer.cpp.o
/home/stu/projects/external/games/CatacombGL/ThirdParty/RefKeen/be_st_sdl_audio_timer.cpp:22:10: fatal error: SDL.h: No such file or directory
   22 | #include <SDL.h>
      |          ^~~~~~~
compilation terminated.
make[2]: *** [CMakeFiles/CatacombGL_ThirdParty.dir/build.make:76: CMakeFiles/CatacombGL_ThirdParty.dir/ThirdParty/RefKeen/be_st_sdl_audio_timer.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:192: CMakeFiles/CatacombGL_ThirdParty.dir/all] Error 2
make: *** [Makefile:156: all] Error 2

[GoGoOtaku]

This appears to be a regression in the recent rework of the CMakeList.txt
Thanks for the heads up

[ArnoAnsems]

Thank you @stuaxo for reporting this issue and thanks to @GoGoOtaku for the quick fix.

[stuaxo]

Thanks, that worked, need to sort out my files from GOG and will be ready to go.

Very cool to be able to revisit this.

[NY00123]

Good catch! I can still build a new binary on 20.04 after updating, so nothing is broken here.

I wonder a bit about the included header. Some projects use <SDL2/SDL.h>, others have "SDL.h" and more may use <SDL.h>

As I remembered, the general recommendation is to use "SDL.h", while adjusting a project/make file and/or compiler flags as needed:
https://wiki.libsdl.org/SDL2/FAQDevelopment#do_i_include_sdl.h_or_sdlsdl.h

For instance, if a custom makefile is used for building on Linux, then you can use sdl2-config to locate the headers.

[GoGoOtaku]

Here is my opinion:

<SDL2/SDL.h> is bad practice in my opinion. While a lot of Linux distributions have the headers in "/usr/include/SDL2/SDL.h" this is not the case for all, not the norm under BSD iirc, and Windows has no standard where anything is. It's much better to require to add the SDL2 include path as a compiler argument or have it in the working directory (hence why SDL themselves recommend not using the subfolder).

<SDL.h> vs "SDL.h" is compiler dependent but all mainstream compilers will look in the working directory first before going to system include paths when double quotes are used. System here means paths that were added with -I or system standard paths (not really a thing under windows). There are heated debates online what is better and no answer really is definitive. I used <SDL.h> since we added the paths with -I (or rather cmake does that for us) instead of them being relative to the working directory. I personally also like this since developers can clearly see in the source code what headers are external and thus not to be edited.

Using double quotes is only useful if you copy the headers into your repository or if you require each developer to handle their dependencies by themself as it gives more flexibility. The former might be required for closed source libraries or volatile library APIs for which sticking with one specific version that is being shipped with the sources is required. Since dependency management for SDL2 is now handled by cmake I felt it was not necessary to use double quotes.

sdl2-config --cflags is returning -I/usr/include/SDL2 -D_REENTRANT on my system which works with both <SDL.h> and "SDL.h"

These are just my opinions and how I would handle it in my repositories. I have no horse in this race either way tho and would just go along with what the repository maintainer wants.