Troubles when compiling OpenTomb with MSVC

[carlo-bramini]

I tried to compile OpenTomb with MSVC 2017 free Community Edition but I got some issues. Here there is a report of my results:

• Included FreeType cannot compile one into source code. It fails here: ftgzip.c(48): fatal error C1083: 'zlib.h': No such file or directory This is a bit strange, because zlib.h is found and used by other sources, but it fails just in FreeType. I have not idea of the reason. I bypassed this issue by using FreeType provided by vcpkg rather than the one included into OpenTomb. For doing this, I added an option to CMakeList.txt for selecting between system provided FreeType and the other one. By default, if no options are specified, it uses the one included in the main sources, like now.

• Although I bypassed the compilation error of included FreeType, VisualC++ generated lot of errors when compiling other sources, because the system FreeType and the included one were conflicting. This happened because gl_font.c tries to include FreeType without using the macros defined into ft2build.h. Replacing:

#include <freetype.h>
#include <ftglyph.h>
#include <ftmodapi.h>

with:

#include FT_FREETYPE_H
#include FT_GLYPH_H
#include FT_MODULE_H

Has fixed the problem on my side.

• I'm getting errors in several sources and include files because __attribute__() is used for setting alignment. I do not know if this is required or made just for performance gain.

• sys/time.h, clock_gettime() and CLOCK_REALTIME do not exist. It wouldn't be a big problem to emulate them on MSVC, but perhaps, since SDL seems to be heavy used, it would be worth to use its timing functions instead, for improving the portability.

• It is assumed that "pthread_t" is a scalar type. Actually, this could be a portability problem, because the type itself is an opaque object. In my opinion, it could be easily fixed by doing something like this:

s->is_thread_valid = pthread_create(...) != 0;

and use this item every time it is required to test if a thread handle is valid.

• I'm getting hundred of warning like: warning C4996: This function or variable may be unsafe. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.

every time strcpy(), sprintf(), etc, are used. This is just a very minor thing, but it can be simply cleaned by adding this:

# Disable warnings if unsafe functions are used (for MSVC)
if(WIN32)
    add_definitions(-D_CRT_SECURE_NO_WARNINGS)
endif()

to CMakeList.txt

The compiler returned also other messages, but in my opinion these are the most important ones. I hope that you will find it useful. Sincerely.

[vvs-]

Thanks for the report.

There were at least two previous attempts to port OT to MSVC by @stohrendorf (in old master) and recently by @Gh0stBlade. But both were just a one time efforts without providing routine support. Predictably their results were lost as time goes by. It just happens that any port is just as robust as CI on that platform and we don't have many. Care to provide AppVeyor project file and make a PR?

[Gh0stBlade]

The patches I made were definitely integrated into the current master. These issues are likely caused by changes after that point.

[vvs-]

@Gh0stBlade That just stresses the point of a working CI for any supported platform. The person making the port using such platform is naturally suited for providing tests. Otherwise it won't last long stretching resources thin. That was my initial argument against @stohrendorf port.

BTW, your time simulation patch on MSVC wasn't integrated at all, which is a pity.

[Gh0stBlade]

Appveyor will likely be frustrating to implement due to many libraries being used last time I checked... probably better to look into a package manager like hunter maybe... dunno.

[carlo-bramini]

Hello, I have some news about the MSVC. It works, as you can see from attached screenshot, it runs and it can be debugged.

The current sources in the master branch have much of the original issues resolved, at the moment the missing points are:

• Errors because GNU __attribute__() is used.
• sys/time.h, clock_gettime() and CLOCK_REALTIME do not exist.

Here I resolved these remaining issues in my local build and I also added to CMakeList.txt the chance to use system provided LUA 5.3 and BULLET, since they are provided by VCPKG. However, I have noticed something strange with BULLET. After I launch the engine, I'm getting an assert fault. This happens with both external library and the one included in the sources. The point is here:

https://github.com/opentomb/OpenTomb/blob/dd8af2eace701f51e3922ddeae9cb236b77ef4f2/extern/bullet/BulletCollision/CollisionShapes/btTriangleMeshShape.cpp#L184

If I comment that assert, the engine starts and it works perfectly. It happens only when the engine has been just launched and then it is not called anymore. I have not idea of the reason... Perhaps there is something not properly initialized at start and this should be checked somehow.

[vvs-]

AFAIK, system provided Bullet was never supported by OT. There is a version mismatch between most distributions and with API that OT expects. YMMV, depending on which OS and its particular version you are using.

[carlo-bramini]

As I have written, it happens with both system provided BULLET and also with the copy provided by OT.

I know that OT cannot use system provided BULLET, it is a change that I did in my CMakeList.txt. However, it should not be too difficult to test also the library version at configure time... I did this test with LUA detection, which accept an external library only if it is 5.3 or newer (see issue #508 ), so the same thing may be done BULLET, if the minimum required version is known.

[vvs-]

OT always relied on unreleased development version of Bullet. Its compatibility was never properly tested with officially provided Bullet on different combinations of OS versions and compilers. I wouldn't enable system Bullet by default as this could lead to many unexpected failures or performance issues which nobody is prepared to address.

[carlo-bramini]

It is what I have done and this is what I did already with the selection of system-provided FreeType, by default the library included with OT is always the default choice if no options are specified by the user.

[carlo-bramini]

I confirm that from 4d4d17c , now Visual Studio can compile the included copy of FreeType without errors.