guusw / unnamed-sdvx-clone

A rhythm game written in C++
MIT License
68 stars 107 forks source link

[NOT FOR MERGE] make this compile on Linux #9

Closed magiruuvelvet closed 8 years ago

magiruuvelvet commented 8 years ago

Not for merge

This patch should give you some ideas of the futher work on the Linux version.

Font has been renamed to GlFont (better name?) because it conflicts with X11's Font typedef. Window has been renamed to DesktopWindow because it conflicts with X11's Window typedef.

Even more changes are done on the master ( GhettoGirl/unnamed-sdvx-clone@06d4dd7 ) branch. You should also take a look at the commit there.

About the Sort template function in Shared/include/Shared/Vector.hpp. You cannot pass a lambda function without being a object (std::function) when the argument takes a reference. There are a quite a lot of compile errors because of this. Why the hell does this even compile in MSVC????????? :confused:

Option A: Change it to a constant reference as I did. Option B: Add a overload which takes a constant reference. Option C: Store all lambda functions which are used for sorting into a std::function.

In Graphics/include/Graphics/Keys.hpp: None is defined in the X11 headers, since it is just a macro it can be safely undefined.

In Graphics/src/Linux/Window.cpp there is a missing KeyMap member variable.

In Main/KShootMap.hpp

-   static const uint32_t KShootMap::c_laserStart;
-   static const uint32_t KShootMap::c_laserEnd;
-   static const uint32_t KShootMap::c_laserRange;
+   static const uint32_t c_laserStart;
+   static const uint32_t c_laserEnd;
+   static const uint32_t c_laserRange;

I don't know what this is supposed to be, but in GCC and Clang this doesn't work! MSVC sure is a strange compiler :confused:

I guess GetCommandLine() is a Windows specific thing?

In Main/stdafx.h: <tchar.h> is a Windows-only header, it needs to be moved in the #ifdef _WIN32 block.

Make sure to checkout the comments in the diff too :wink:

magiruuvelvet commented 8 years ago

One more important thing to say :smile:

../Shared/libShared.a(Path.cpp.o): In function `Path::GetTemporaryPath()':
Path.cpp:(.text+0x138): warning: the use of `mktemp' is dangerous, better use `mkstemp' or `mkdtemp'

Please don't use mktemp.

guusw commented 8 years ago

Wow hey, thanks for taking the effort to look at everything. I'll try to move most things into the master branch when I have time to fix all those things. One thing I'm not really sure of is the find_package thing, but I'll look into that after it all compiles and runs. Indeed most things that still need changing are the windows specific filesystem stuff like the map searching and the embedded font (i'll probably move it just to the binary folder on both Windows and Linux for convenience).

Also, didn't the "Graphics" namespace fix the naming collisions with X11?

magiruuvelvet commented 8 years ago

Also, didn't the "Graphics" namespace fix the naming collisions with X11?

Most likely not. I'm not sure why the compiler complains about this. :confused: In theory this actually should work.

Regarding find_package, instead of hardcoding paths, find_package makes it possible to search for components and store its paths like include and library locations into a variable. Users may have installed dependencies somewhere else, then CMAKE_PREFIX_PATH can be used to tell CMake where to look for components.

On Linux X11, OpenGL and SDL2 (and others [zlib, lzma, etc.]) should be looked up using find_package rather than hardcoding the names and paths. You noticed the SDL include changes I did. On my system for example the SDL2 headers are in a subdirectory SDL2. Using find_package this problem can be solved in an easy way.

Regarding libvorbis and libogg, you need to link libvorbis before libogg, the GNU linker is not the smartes when it comes to symbol lookup... There were a lot of linker errors in libogg.

guusw commented 8 years ago

Yeah i'm not sure which ones it were but when I tried to replace all the libaries with their find_package equivalent there were some that didn't have an option for find_package. And even some libraries were a different version (I think freetype/jpeg).

magiruuvelvet commented 8 years ago

I updated the CMake files here GhettoGirl/unnamed-sdvx-clone@82abab0965a203310be055a11a0b98cb68b30551

The changes are rather primitive, but now there are no hardcoded names for libraries and include directories anymore.

The best solution would be to to put all the dependency stuff in one big CMakeList file and not mix everything togheter on different places.

This is how it looks like now

-- Found Freetype: /usr/lib64/libfreetype.so (found version "2.6.5") 
-- Looking for lzma_auto_decoder in /usr/lib64/liblzma.so
-- Looking for lzma_auto_decoder in /usr/lib64/liblzma.so - found
-- Looking for lzma_easy_encoder in /usr/lib64/liblzma.so
-- Looking for lzma_easy_encoder in /usr/lib64/liblzma.so - found
-- Looking for lzma_lzma_preset in /usr/lib64/liblzma.so
-- Looking for lzma_lzma_preset in /usr/lib64/liblzma.so - found
-- Found LibLZMA: /usr/include (found version "5.2.2") 
-- Found ZLIB: /usr/lib64/libz.so (found version "1.2.8") 
-- Found SDL2: /usr/include  

EDIT: Not sure about that jpeg-9b :confused: I moved it so that it is used on Linux too.