This fixes building on x86_64 as it sets proper compile-flags

[ljrk0]

If you have a better way to set CXXFLAGS or know why add_definitions only affects only CFLAGS, of course this can be changed.

[icculus]

Are you sure that add_definitions doesn't affect C++? It definitely does here (and the assembly code, too, which is why we had to use a custom command to fire off NASM without the add_definitions bits).

Run "make VERBOSE=1" when you build and it'll show you the actual command line for each thing it compiles.

[ljrk0]

The command is

/usr/bin/c++   -DDEBUG=1 -DPLATFORM_UNIX=1 -DPRAGMA_ONCE=1 -DSINGLE_THREADED=1 -DUSE_I386_ASM=1 -D_DEBUG=1 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE=1 -D_MT=1 -D_REENTRANT=1 -I/mnt/sdata/Serious-Engine/Sources/. -I/mnt/sdata/Serious-Engine/Sources/External/SDL12  -g   -O0 -ggdb3 -pthread -pipe -fsigned-char -o CMakeFiles/SeriousSam.dir/Engine/Engine.cpp.o -c /mnt/sdata/Serious-Engine/Sources/Engine/Engine.cpp

but I get:

/mnt/sdata/Serious-Engine/Sources/Engine/Engine.cpp: Assembler messages:
/mnt/sdata/Serious-Engine/Sources/Engine/Engine.cpp:147: Error: invalid instruction suffix for `push'
/mnt/sdata/Serious-Engine/Sources/Engine/Engine.cpp:153: Error: invalid instruction suffix for `pop'
/mnt/sdata/Serious-Engine/Sources/Engine/Engine.cpp:165: Error: invalid instruction suffix for `push'
/mnt/sdata/Serious-Engine/Sources/Engine/Engine.cpp:170: Error: invalid instruction suffix for `pop'
CMakeFiles/SeriousSam.dir/build.make:266: recipe for target 'CMakeFiles/SeriousSam.dir/Engine/Engine.cpp.o' failed

This is fixed via the add_definitions(-m32) but then I still have linkage errors:

/usr/bin/c++   -DDEBUG=1 -DPLATFORM_UNIX=1 -DPRAGMA_ONCE=1 -DSINGLE_THREADED=1 -DUSE_I386_ASM=1 -D_DEBUG=1 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE=1 -D_MT=1 -D_REENTRANT=1 -I/mnt/sdata/Serious-Engine/Sources/. -I/mnt/sdata/Serious-Engine/Sources/External/SDL12  -g   -O0 -ggdb3 -pthread -pipe -fsigned-char -m32 -o CMakeFiles/ecc.dir/Ecc/Scanner.cpp.o -c /mnt/sdata/Serious-Engine/Sources/Ecc/Scanner.cpp
[  2%] Linking CXX executable ecc
/usr/bin/cmake -E cmake_link_script CMakeFiles/ecc.dir/link.txt --verbose=1
/usr/bin/c++   -g   CMakeFiles/ecc.dir/Ecc/Main.cpp.o CMakeFiles/ecc.dir/Ecc/Parser.cpp.o CMakeFiles/ecc.dir/Ecc/Scanner.cpp.o  -o ecc -rdynamic 
/usr/bin/ld: i386 architecture of input file `CMakeFiles/ecc.dir/Ecc/Main.cpp.o' is incompatible with i386:x86-64 output
/usr/bin/ld: i386 architecture of input file `CMakeFiles/ecc.dir/Ecc/Parser.cpp.o' is incompatible with i386:x86-64 output
/usr/bin/ld: i386 architecture of input file `CMakeFiles/ecc.dir/Ecc/Scanner.cpp.o' is incompatible with i386:x86-64 output
collect2: error: ld returned 1 exit status
CMakeFiles/ecc.dir/build.make:161: recipe for target 'ecc' failed

With the CXX_FLAGS hack it runs successfully:

/usr/bin/c++   -DDEBUG=1 -DPLATFORM_UNIX=1 -DPRAGMA_ONCE=1 -DSINGLE_THREADED=1 -DUSE_I386_ASM=1 -D_DEBUG=1 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE=1 -D_MT=1 -D_REENTRANT=1 -I/mnt/sdata/Serious-Engine/Sources/. -I/mnt/sdata/Serious-Engine/Sources/External/SDL12  -m32 -g   -O0 -ggdb3 -pthread -pipe -fsigned-char -m32 -o CMakeFiles/ecc.dir/Ecc/Scanner.cpp.o -c /mnt/sdata/Serious-Engine/Sources/Ecc/Scanner.cpp
[  2%] Linking CXX executable ecc
/usr/bin/cmake -E cmake_link_script CMakeFiles/ecc.dir/link.txt --verbose=1
/usr/bin/c++    -m32 -g   CMakeFiles/ecc.dir/Ecc/Main.cpp.o CMakeFiles/ecc.dir/Ecc/Parser.cpp.o CMakeFiles/ecc.dir/Ecc/Scanner.cpp.o  -o ecc -rdynamic 
make[2]: Leaving directory '/mnt/sdata/Serious-Engine/Sources/cmake-build'
[  2%] Built target ecc

The problem is that add_definitions() is only intended for preprocessor flags and is not used for the final linking call, whereas CXXFLAGS are.

In case it's relevant, running Linux, not OSX:

$ uname -a
Linux DeepThought 4.4.6-1-ck #1 SMP PREEMPT Fri Mar 25 18:42:58 EDT 2016 x86_64 GNU/Linux

Side note: It seems that the macro add_parser_and_scanner doesn't work properly. It only executes the last custom_command and thus errors if the Parser.hpp is not already existent (ie. in a clean build):

make VERBOSE=1
/usr/bin/cmake -H/mnt/sdata/Serious-Engine/Sources -B/mnt/sdata/Serious-Engine/Sources/cmake-build --check-build-system CMakeFiles/Makefile.cmake 0
/usr/bin/cmake -E cmake_progress_start /mnt/sdata/Serious-Engine/Sources/cmake-build/CMakeFiles /mnt/sdata/Serious-Engine/Sources/cmake-build/CMakeFiles/progress.marks
make -f CMakeFiles/Makefile2 all
make[1]: Entering directory '/mnt/sdata/Serious-Engine/Sources/cmake-build'
make -f CMakeFiles/ecc.dir/build.make CMakeFiles/ecc.dir/depend
make[2]: Entering directory '/mnt/sdata/Serious-Engine/Sources/cmake-build'
[  0%] Generating ../Ecc/Parser.h
cd /mnt/sdata/Serious-Engine/Sources && /usr/bin/cmake -E copy Ecc/Parser.hpp Ecc/Parser.h
Error copying file "Ecc/Parser.hpp" to "Ecc/Parser.h".
CMakeFiles/ecc.dir/build.make:68: recipe for target '../Ecc/Parser.h' failed

[icculus]

With the CXX_FLAGS hack it runs successfully:

Ugh, weird. I have it do all sorts of nasty voodoo to build the parser code, so it might be something I'm doing wrong. Let me dig a little deeper on this one and get back to you, okay?

[ericwomer]

But from this page it says this about add_definitions and switches:

CMAKE_C_FLAGS 
the compiler flags for compiling C sources. Note you can also specify switches with ADD_DEFINITIONS().

[ericwomer]

You do know with build-linux.sh sets CMAKE_CXX_FLAGS and CMAKE_C_FLAGS is set with -D to add the -m32 and it works for me, maybe you should try whats in the script. But on a side note there are other ways to set compile/link options since cmake-2.8.12.

[icculus]

Anyhow, the real solution is going to be getting the game 64-bit clean.

It's not just the SoundMixer386.asm file, there's inline asm all over the place. Some of it I cleaned out (possibly incorrectly) when I tried to get this running on a PowerPC mac ages ago (I got it to run, sort of, but it never rendered anything correctly). It might be useful if someone turns off all the asm code they can on a 32-bit build and see what breaks. The non-assembly versions are all wrapped in USE_PORTABLE_C blocks. I don't think everything got ported in any case.

64-bitness aside, I'd be curious if modern compilers can beat that assembly code anyhow (and even if they can't, computers and GPUs are way faster now anyway, so having the C code in there is probably preferable from a maintenance and portability viewpoint anyhow and shouldn't be a showstopping performance hit).

After that non-trivial task, there were a few places where I vaguely recall seeing things that weren't 64-bit clean (maybe the scripting language interpreter?), but those should be pretty obvious (immediate crash at the point of the bug instead of strange results...probably).

[icculus]

(Also these guys are claiming they made a 64-bit clean Serious Engine 1, so I guess it's possible...

http://store.steampowered.com/app/227780

)

[ljrk0]

@salamanderrake Hm, (https://cmake.org/cmake/help/v3.0/command/add_definitions.html)[but it seems] that add_definitions() is just for compilation flags (actually even only supposed for preprocessor flags like -D_MYDEFINE. But as -m32 is both, a compilation and a linker flag, I think it should be put in CXX_FLAGS.

And build-linux.sh wasn't there when I first pulled. It could at least be regarded as a workaround, depending on @rcgordon 's opinion on that.

I've tried with some other people to make it build on Linux with CMake too, before. Most errors we ran into though were 64-bit unrelated and at least Ecc seemed to work definitely fine.

[icculus]

at least Ecc seemed to work definitely fine.

I just pushed a change for making ECC work with cross-compile builds (since it needs to run on the building machine and not the system that is being built for). ECC does appear to work correctly as a 64-bit binary, so you can see the example code I posted in the commit message about building just that piece:

https://github.com/rcgordon/Serious-Engine/commit/9581eeb02ae584426d96acd8f7c6dc68bdb8c59d

(Granted, this isn't the best thing ever, but if you are building a 32-bit Serious Sam on a system that doesn't have 32-bit support, that at least solves that problem, too.)

[icculus]

Heads up: the latest in this repo now (sort of) works with 64-bit builds. It has some obvious issues, but it can make it through the intro demos and stuff.

Build like this:

cmake -DCMAKE_BUILD_TYPE=Debug -DUSE_I386_ASM=FALSE ..

[icculus]

I'm going to close this PR now, since we're pretty close to a real 64-bit build and never needing -m32 again. :)