NagyD / SDLPoP

An open-source port of Prince of Persia, based on the disassembly of the DOS version.
GNU General Public License v3.0
1.1k stars 140 forks source link

fixes compilation error for MSVC on Windows #305

Closed pratikone closed 1 year ago

pratikone commented 1 year ago

Current SDLPop doesn't compile on Windows using MSVC/Visual Studio compiler. It throws 3 errors when trying to build on windows.

  1. SetProcessDPIAware syntax issue
  2. __imp_CommandLineToArgvW definition not found during linker because shell32.dll was missing
  3. wstat not found, replaced with _wstat.

It compiles and links correctly and produces a pop exe which plays.

Bonus : adds missing build instructions to README

NagyD commented 1 year ago

Dev-C++ gave me warnings, so I changed two things in bf97796df594688fae6f7b68fbcd57f20a7beb13.

Please verify that this does not break compilation on MSVC!

  1. SetProcessDPIAware syntax issue

    I got this warning:

    [Warning] parameter names (without types) in function declaration

    I don't know why is SetProcessDPIAware in the parameter list of the typedef, so I changed it to void to reflect that this function type takes no parameters.

  2. wstat not found, replaced with _wstat.

    Unfortunately, on Dev-C++, _wstat expects the wrong type.

    [Warning] passing argument 2 of '_wstat' from incompatible pointer type
    [Note] expected 'struct _stat *' but argument is of type 'struct stat *'

    So I added an #if to use _wstat on MSVC and wstat otherwise.

dstarosta commented 1 year ago

The latest change (#1) produces a pointer type warning when compiling in 64-bit mode with the -Wextra flag. I believe the type definition for SetProcessDpiAware should be:

#if defined WIN64 || _WIN64
    typedef long long (WINAPI * dpiaware)(void);
#else
    typedef BOOL (WINAPI * dpiaware)(void);
#endif
pratikone commented 1 year ago

Thanks for merging it in

NagyD commented 1 year ago

The latest change (#1) produces a pointer type warning when compiling in 64-bit mode with the -Wextra flag.

Do you mean the change in bf97796df594688fae6f7b68fbcd57f20a7beb13? Do you get no warning if you change void back to SetProcessDPIAware? What does the warning say and which line gets the warning?

dstarosta commented 1 year ago

The error is cast between incompatible function types from 'FARPROC' {aka 'long long int (*)()'} to 'BOOL (*)()' {aka 'int (*)()'} [-Wcast-function-type] __SetProcessDPIAware = (BOOL(WINAPI *)(void))GetProcAddress(__user32, "SetProcessDPIAware").

It is happening here in Windows x64 builds: https://github.com/NagyD/SDLPoP/blob/bf97796df594688fae6f7b68fbcd57f20a7beb13/src/seg009.c#L2611

NagyD commented 1 year ago

The latest change (#1) produces a pointer type warning when compiling in 64-bit mode with the -Wextra flag. I believe the type definition for SetProcessDpiAware should be:

#if defined WIN64 || _WIN64
    typedef long long (WINAPI * dpiaware)(void);
#else
    typedef BOOL (WINAPI * dpiaware)(void);
#endif

The documentation says that SetProcessDPIAware returns BOOL, regardless of the bits of the platform. I think this means that the typedef should always use BOOL.

The error is cast between incompatible function types from 'FARPROC' {aka 'long long int (*)()'} to 'BOOL (*)()' {aka 'int (*)()'} [-Wcast-function-type] __SetProcessDPIAware = (BOOL(WINAPI *)(void))GetProcAddress(__user32, "SetProcessDPIAware").

GetProcAddress returns FARPROC, which is defined differently on 32-bit and 64-bit platforms. It is declared as returning a 32-bit integer on 32-bit systems, a 64-bit integer on 64-bit systems. But that's just a "general" function type. The return value of GetProcAddress is meant to be cast into a different type.

A quick search shows that this warning should be ignored: