search

wowserhq / blizzardry

JavaScript library for mastering the wizardry that is Blizzard's game files
MIT License
54 stars 15 forks source link

Windows support #51

Closed petersandor closed 9 years ago

petersandor commented 9 years ago

Hi, I love this module but I noticed a couple of issues when I tried to use it on Windows. In storm-lib.js:308, GetLastError() is exported from StormLib but as you may know, the function is not present in Windows builds of StormLib. So I would like to ask how to properly handle this? It appears that the function isn't even used anywhere in the module, so I just commented it out.

timkurvers commented 9 years ago

Hi Peter,

It's used in case of error scenarios, for example, when trying to open a non-MPQ file.

For node-ffi to interface with shared libraries like StormLib, the function names in it need to be unmangled. On non-Windows platforms, StormLib does this by wrapping extern C around the declaration for GetLastError/SetLastError.

How did you compile/install StormLib on Windows? I might be able to have a look in a virtual machine.

petersandor commented 9 years ago

Hi, nice work btw with the module and the wowser project :+1:

I just compiled the lib in VS as-is. Everything seems to work (checked with Dependency Walker, all function names are clear, unmangled).

Other issue that I have on Windows is with BLP lib, there is some access violation when blp_processFile is called (something with fseek()).

timkurvers commented 9 years ago

Hi again. Am running into the same issue. It seems GetLastError and friends are declared in Kernel32.dll, which could be the issue. There might be a way to include them in the resulting DLL, as opposed to opening up Kernel32.dll only for Windows platforms.

timkurvers commented 9 years ago

Managed to get it to work, by using a forwarding declaration:

#ifdef PLATFORM_WINDOWS
    #pragma comment(linker, "/export:GetLastError=Kernel32.GetLastError")
    #pragma comment(linker, "/export:SetLastError=Kernel32.SetLastError")
#endif

Could you give this a shot @petersandor? I'll have a look at the BLPConverter issues later this week.

petersandor commented 9 years ago

Hi Tim, thanks for looking into this. I can't test this thoroughly (I'm at work, will test later when I arrive home). But from what I've tried everything looks good.

stormlib_dw

petersandor commented 9 years ago

Closing this as StormLib maintainer included the fix (https://github.com/ladislav-zezula/StormLib/issues/48) and all related DLL issues have been resolved.

Yay for Windows support! :sparkles: