Compiling NetHelper with MSVC toolset 1.40 (2015)

[Vagabond]

I took a stab at updating NetHelper to compile against the 2015 C++ toolset (ver 1.40). This included the two project dependencies: libnatpmp and miniupnpc.

I have both dependencies compiling on their own.

I've setup the project to compile statically into a dll.

When compiling NetHelper itself in win32 debug or win32 release, I get 14 errors. 13 LNK2019 and the corresponding LNK1120.


Error   LNK2019   unresolved external symbol __imp__UPNP_GetValidIGD referenced in function "public: bool __thiscall PortForwarder::Initialize(bool,bool)" (?Initialize@PortForwarder@@QAE_N_N0@Z)   NetHelper   \NetHelper\src\PortForward.obj   1

Error   LNK2019   unresolved external symbol __imp__closenatpmp referenced in function "public: __thiscall PortForwarder::~PortForwarder(void)" (??1PortForwarder@@QAE@XZ)   NetHelper   \NetHelper\src\PortForward.obj   1

Error   LNK2019   unresolved external symbol __imp__freeUPNPDevlist referenced in function "public: bool __thiscall PortForwarder::Initialize(bool,bool)" (?Initialize@PortForwarder@@QAE_N_N0@Z)   NetHelper   \NetHelper\src\PortForward.obj   1

Error   LNK2019   unresolved external symbol __imp__FreeUPNPUrls referenced in function "public: __thiscall PortForwarder::~PortForwarder(void)" (??1PortForwarder@@QAE@XZ)   NetHelper   \NetHelper\src\PortForward.obj   1

Error   LNK2019   unresolved external symbol __imp__getnatpmprequesttimeout referenced in function "int __cdecl ListenForPmpResponse(struct natpmp_t &,struct natpmpresp_t *,int)" (?ListenForPmpResponse@@YAHAAUnatpmp_t@@PAUnatpmpresp_t@@H@Z)   NetHelper   \NetHelper\src\PortForward.obj   1

Error   LNK2019   unresolved external symbol __imp__initnatpmp referenced in function "public: bool __thiscall PortForwarder::Initialize(bool,bool)" (?Initialize@PortForwarder@@QAE_N_N0@Z)   NetHelper   \NetHelper\src\PortForward.obj   1

Error   LNK2019   unresolved external symbol __imp__readnatpmpresponseorretry referenced in function "int __cdecl ListenForPmpResponse(struct natpmp_t &,struct natpmpresp_t *,int)" (?ListenForPmpResponse@@YAHAAUnatpmp_t@@PAUnatpmpresp_t@@H@Z)   NetHelper   \NetHelper\src\PortForward.obj   1

Error   LNK2019   unresolved external symbol __imp__sendnewportmappingrequest referenced in function "public: bool __thiscall PortForwarder::Forward(bool,int,int,char *,char *,int)" (?Forward@PortForwarder@@QAE_N_NHHPAD1H@Z)   NetHelper   \NetHelper\src\PortForward.obj   1

Error   LNK2019   unresolved external symbol __imp__sendpublicaddressrequest referenced in function "public: bool __thiscall PortForwarder::Initialize(bool,bool)" (?Initialize@PortForwarder@@QAE_N_N0@Z)   NetHelper   \NetHelper\src\PortForward.obj   1

Error   LNK2019   unresolved external symbol __imp__upnpDiscover referenced in function "public: bool __thiscall PortForwarder::Initialize(bool,bool)" (?Initialize@PortForwarder@@QAE_N_N0@Z)   NetHelper   C:\Users\Brett\Documents\OutpostUniverseGitHub\NetHelper\src\PortForward.obj   1   
Error   LNK2019   unresolved external symbol __imp__UPNP_AddPortMapping referenced in function "public: bool __thiscall PortForwarder::Forward(bool,int,int,char *,char *,int)" (?Forward@PortForwarder@@QAE_N_NHHPAD1H@Z)   NetHelper   \NetHelper\src\PortForward.obj   1

Error   LNK2019   unresolved external symbol __imp__UPNP_DeletePortMapping referenced in function "public: bool __thiscall PortForwarder::Forward(bool,int,int,char *,char *,int)" (?Forward@PortForwarder@@QAE_N_NHHPAD1H@Z)   NetHelper   \NetHelper\src\PortForward.obj   1

Error   LNK2019   unresolved external symbol __imp__UPNP_GetExternalIPAddress referenced in function "public: bool __thiscall PortForwarder::Initialize(bool,bool)" (?Initialize@PortForwarder@@QAE_N_N0@Z)   NetHelper   \NetHelper\src\PortForward.obj   1


It appears all the errors come from PortForward.obj. All the unresolved external symbols start with __imp__. I think this stands for import. I suspect part of the code base is set to export the functions for access across different DLLs while part of the code base is set to link against the functions as if they are part of the same module.

Inside Visual Studio, When I click 'goto definition' on the PortForwarder class calls to the functions in question, Visual Studio is smart enough to pull up the function definitions.

It may have something to do with __declspec(dllimport) or __declspec(dllexport) calls.

There is a flag inside both dependent projects to mark them as static builds. Both flags seem to be set.

Not sure how to troubleshoot this. Maybe someone wants to review the project files? I could email or post them if interested.

Thanks,
Brett

[Arklon]

Sounds like miniupnp/libnatpmp headers are using __declspec(dllimport) (i.e. for dynamic linking) because you don't have the #defines for static link mode set: MINIUPNP_STATICLIB in miniupnpc_declspec.h, and NATPMP_STATICLIB in declspec.h.

Also, I'd recommend VC 2017 now, since 15.7 it's gotten past the first year of being unstable and they've fixed the code gen bugs now, and better yet it has full C++17 support. The IDE itself still seems a bit less stable than 2015 though, but the compiler is fine. Regardless of what toolset you use, you should statically link the CRT (which also requires configuring the miniupnp/libnatpmp builds to do so as well), even though it creates a bigger DLL it's worth it to avoid VC runtime version dependency hell.

[Vagabond]

Good news. Thanks to Arklon's suggestion of adding the two defines directly to the codebase, I got NetHelper to compile. I ran it with Outpost 2 using a DEBUG build and was able to see NetHelper's DLL be stepped into for initialization and destruction.

When I click on Internet TCP/IP for multiplayer, Outpost 2 appears to be showing me my external IP address. In the past I remember seeing the internal IP address. Still need to actually test a scenario with someone else.

Also, NetHelper destruction is averaging about 7 seconds for me now, way improved over earlier results. Although it would probably be better to see that number reduced further.

Can someone please post the name of the DLL that is reported as missing when Outpost 2 is started with the current distributed version of Outpost 2? (It is probably already on the forum somewhere but I missed it)

The new build of NetHelper is attached with the .pdb file bundled. It is a release build, so the symbols will jump around a bit when step debugging. Could someone who is getting the missing DLL message download it and try it and make sure I actually fixed that problem?

Thanks,
Brett

[Hooman]

Thank you two very much for working on this!

Previously, I was seeing error messages about:

C:\windows\system32\msvcr100.dll
C:\windows\system32\msvcp100.dll

With the replacement DLL, there are no longer any error message popups.

Both before and after tests were run on Linux with Wine, under OllyDbg.


I checked the multiplayer Internet TCP/IP menu, though I saw an internal IP address there. I'm in a cafe though, so that might not mean much.

[Vagabond]

Thanks Hooman.

I think we can call the missing dependencies as solved.

Not sure about the internet cafe not showing the external IP address. I'm not good enough at networks to say if that is normal behavior.

Since I had to make edits to the dependency projects, I probably need to fork both of them, edit, then add the forks as submodules on Git. I will just name them miniupnp-OP2 unless there are better suggestions.

Then Arklon can review / edit.

Brett

[Hooman]

Would it be useful to contribute anything back to the original project?

Was the edit just to compile them with the additional macro defined? And was the macro added at the command line with the /D option (likely set as additional defines in the preprocessor section of the project configuration)? It may make sense to add a static library configuration to the project with that macro defined.


I'd like to see the binary changes incorporated into trunk.

[Vagabond]

Hooman, I had to define the macros directly in the project source code to get it to work. I'm changing very little in both projects so it is frustrating to be forced into forking.

I'm bad at macros. Perhaps once the code is forked, Arklon or Leeor can review and fix in a way that allows the fork to be removed.

There are other project settings involved as well which I don't know how the author feels about changing.

I did find one legitimate mistake in the project settings. I plan to submit a PR for it and test the waters on working with the author for code changes.

At a minimum there are some complier warnings that should be reviewed before pushing the binary. Also Arklon mentioned he had some changes of his own which if close to being ready we should wait on?

Several of the warnings are me redefining the macros.

Brett

[Vagabond]

Well, we have a fork of miniupnp on Outpost Universe's Github page now. There is also a small merge request for updating the MSVC project to match a renamed file. Zero introduction, just forked and did a merge request.

https://github.com/miniupnp/miniupnp/pull/331

I just left the repo name the same as the original.

It appears that natpmp is going to be more complicated. The version on GitHub seems to be at least one version behind the one available for download on the miniupnp site. I am currently using the official one from their site and not the copy on GitHub. So I cannot just fork the repo with only one or two changes. Many files appear changed since it looks like a lot of the code files contain a version string at the top that has been updated.

I think I will fork natpmp, then apply the changes to bring it up to date with their official downloadable version. This sort of gives them credit for the work while allowing us to use the newest stuff I guess. Not really clean, but I can't think of a better solution.

-Brett

----

EDIT: The natpmp repository is newer than the posted version on the official website. I just read it incorrectly.

[Vagabond]

I managed to test the previously posted NetHelper dll. I was able to host a TCP/IP game without fiddling with any network settings and have someone else join it. I was able to join their hosted TCP/IP game as well without fiddling.

Using NetFix, they were able to join a game I hosted. For some reason, I was unable to see their game using NetFix. We didn't dig into any logging or network settings to try to resolve.

The good news is I've never been able to host or join Outpost 2 games without using Hamachi or spending at least on hour having someone fiddle with network settings on my behalf. I also switched routers about a month ago, so that might have something to do with it...


In regards to natpmp, my previous post was incorrect. The GitHub code is newer than the downloadable zip file on the official website. I went ahead and integrated with the GitHub code this go around. A new compile issue was found, but was simple to get around.

At some later version of natpmp, the typedef ssize_t was introduced in the Windows code. This causes a compile time error. It appears ssize_t is a POSIX definition that is not fully supported on Windows. Oddly enough SSIZE_T is available on Windows.

I had to add the following macro to natpmp.c to get natpmp to compile on VS2017.

#define ssize_t SSIZE_T

We could try to push this as a change to natpmp's code base. Open to suggestions if there is a more proper way to handle before creating the pull request. It is contained in a #if win32 section so would not affect Linux builds.


The miniupnp pull request fixing the project error was accepted by miniupnp's author. I opened a second pull request on libnatpmp (same author) to improve the gitignore file when dealing with msvc user setting files and compilation artifacts.

All the code is now loaded onto GitHub with the submodules plumbed in properly. The post build event was updated. It expects a folder called Outpost2 to be placed in the root repository directory with the game files.

I uploaded 3 issues that include some compile time warnings that we may want to clean up and other suggestions. There are 2 other warnings that I ran out of time to deal with or write issues against. Will get to later.

The libnatpmp and miniupnpc projects in our repo do not have issue tracking enabled right now. They are more meant as a bridge to allow requesting changes with the original library and to allow for needed minor project setting variations. I haven't approached the author about aligning his MSVC project settings with what we want (static builds with VS2017). Maybe a bit presumptuous. He seems to really only work natively on Linux and the MSVC code follows along when needed by someone.

I haven't tested the latest compilation yet, although everything seemed fine.

Let me know thoughts on all of this...

-Brett

[Hooman]

Wow, good job. You're really moving forward with this.


I noticed in the diff for your commit on master of the fork of miniupnp:

Code: [Select]

-      <PreprocessorDefinitions>_CRT_SECURE_NO_WARNINGS;MINIUPNP_STATICLIB;DEBUG;%(PreprocessorDefinitions)</PreprocessorDefinitions>
+      <PreprocessorDefinitions>_CRT_SECURE_NO_WARNINGS;DEBUG;%(PreprocessorDefinitions);</PreprocessorDefinitions>

It seems the MINIUPNP_STATICLIB define was removed from the pre-processor definitions, and then added to the source file.

This is confusing, since I would expect that to not be defined for a DLL build.

I checked back to the prior commit.

Checking the file: miniupnpc/msvc/miniupnpc.vcxproj, I noticed there were 4 project configurations: Debug Dll, Debug, Release Dll, and Release. As expected, the Debug and Release configurations have their ConfigurationType set to StaticLibrary, and have a PreprocessorDefinitions list including MINIUPNP_STATICLIB.

It looks like the project already has configurations for both DLL and static library builds.

Perhaps the original problem was simply a matter of changing which configuration got built?
(And of course the toolset upgrade)


The project file changes for libnatpmp look more extensive. Looked like the solution file was for an older version, and the upgrade amounted to basically rewriting the whole file, and adding all new project files. Curiously, I noticed the project file was split, with DLL builds in one project file, and static library builds in the other file, rather than having all 4 configurations in one project file.

It looks like the static configuration project file already has the necessary NATPMP_STATICLIB preprocessor macro defined. I don't think it needs to be added to the source file as well.

Looks like the new project files would be useful to contribute back to the original author. I might suggest combining all 4 configurations into one project file though.


The #define ssize_t SSIZE_T looks relevant, and may be worth contributing back. I don't know enough about it to really comment on it.

I'd be curious to know how the #define interacts with older versions of the Windows kits.

From my cursory reading, it sounds like ssize_t is a signed version of size_t, used when a negative return value is used as a sentinel value, for what would otherwise be a positive size_t type value. It's part of Posix, but not part of the C/C++ standards.

[Vagabond]

I couldn't seem to get the static built projects to work differently than the non-static built miniupnp and libnatpmp so I went with the non-static builds. I will try to revisit this in the future and see if I can get it working.

I deleted the MINIUPNP_STATICLIB and NATPMP_STATICLIB defines from the project settings because they didn't seem to be working and when I defined them in the source code directly, I would get redefine errors. I seemed to have to define them directly in the source code to keep the linker from marking functions as __imp__ (import I think?). Definitely something that should be sorted out in the long run. Although if we want to leave alone for now, I think we are safe to release NetHelper in this setup.

libnatpmp uses Visual Studio 2008. I think in 2010, VS switched the format of the solution and project files. This forces creation of the new project files. I think 2010-2017 are all compatible formats that do not require creation of different project files. You just tweak settings like the toolset you target. That is what causes all the churn in the project settings.

It might be nice to push the project upgrades of libnatpmp back into the owner's repo. Not sure if I will get that far though.

Hooman, thanks for carefully reading all the changes and providing feedback. Esp on the warnings I posted as issues in the NetHelper GitHub section.

My current plan is to put some time into cleaning up the different warnings. Then probably do a bit more testing as best I can.

-Brett

[Hooman]

I deleted the MINIUPNP_STATICLIB and NATPMP_STATICLIB defines from the project settings because they didn't seem to be working and when I defined them in the source code directly, I would get redefine errors. I seemed to have to define them directly in the source code to keep the linker from marking functions as __imp__ (import I think?). Definitely something that should be sorted out in the long run. Although if we want to leave alone for now, I think we are safe to release NetHelper in this setup.

Can you confirm which project configuration you were compiling during your earlier testing?

[Vagabond]

I spent more time troubleshooting the static library defines for libnatpmp and miniupnp. Turns out, if I define the macros in both libnatpmp/miniupnp and NetHelper project settings, it compiles properly without adding the define directly in the source code. I went ahead and committed these changes.

I tried compiling the code with the static libnatpmp and miniupnp projects. No dice. I get a lot of  MSVC LNK2001 errors (unresolved external symobl). I just stuck with modifying the non-static labeled projects after this.

I went ahead and worked on the project settings. The project file is now called NetHelper.vcxproj instead of OP2Script. I renamed the ReleaseMinSize build as just Release. Then I went in and tried to set optimization settings to default. Details are in the commit log on Git. I left the base address settings and other Outpost 2 specific quirks caused by its age alone. Likely, the file will be a bit bigger but the code will run a bit faster. Not that either speed or size matters for this project.

I spent some time this week trying to solve the deprecation warnings and the casting from a larger source to a smaller source warning. My attempts appear to be unfruitful. I don't have a great knowledge of WIN32 programming. I think my plan is to just leave the warnings as is for now.

I'm planning to test NetHelper one more time by playing through a multiplayer game without using Hamachi. Not sure when I'll have the time. Assuming that goes well, believe my work on the project is coming to a close.

If Arklon wants to spend some time playing/improving with the code, we can probably work other stuff in the meantime for the next Outpost 2 release. Otherwise, after another good test or two, we should probably push this DLL into the SVN repository for the next release.

Once we are ready to push this into the subversion REPO for use in game release, I'll compile a final copy and place the symbol file (PDB) in the release section of the NetHelper Git Repository to facilitate future debugging if needed.

For the NetHelper change log:
 * Updated to newer versions of libnatpmp and miniupnp
   * Newer dependecy versions cut down cleanup/destruction on application closing from minutes to about 8 seconds.
 * Changed dependency references from precompiled library files to  to source code via Git submodules
 * Removed dependency on
 * Set release compilation optimization properties closer to MSVC default settings (favoring speed over size)

Hooman, let me know if this didn't answer your question.

-Brett

[Hooman]

Turns out, if I define the macros in both libnatpmp/miniupnp and NetHelper project settings, it compiles properly without adding the define directly in the source code.

Ahh, that makes sense. The header files need to be processed both in the context of the library, while the library is being compiled, and in the context of the main program using the library, when the main program is being compiled.


For a DLL, the #defines used for declarations in the header files need to be different between the library and the main project. For the library itself, it needs to export symbols from the header files. For the main project, it needs to import symbols from the header files. That's typically controlled by having a default of import, so the project can use the header files without special configuration, but within the library itself, they compile with an extra define in the project settings to switch to export instead of import.

In libnatpmp, this is controlled by NATPMP_LIBSPEC, which is defined in natpmp_declspec.h:
(In miniupnp, the structure is equivalent, but using MINIUPNP_LIBSPEC instead, which is defined in miniupnpc_declspec.h).

Code: cpp [Select]

#if defined(_WIN32) && !defined(NATPMP_STATICLIB)
	/* for windows dll */
	#ifdef NATPMP_EXPORTS
		#define NATPMP_LIBSPEC __declspec(dllexport)
	#else
		#define NATPMP_LIBSPEC __declspec(dllimport)
	#endif
#else
	#if defined(__GNUC__) && __GNUC__ >= 4
		/* fix dynlib for OS X 10.9.2 and Apple LLVM version 5.0 */
		#define NATPMP_LIBSPEC __attribute__ ((visibility ("default")))
	#else
		#define NATPMP_LIBSPEC
	#endif
#endif

For a DLL build, when NATPMP_EXPORTS is defined, the headers will export the symbols, otherwise it defaults to importing the symbols. NATPMP_EXPORTS seems to be defined in build.bat for gcc builds, but curiously I don't see any references to it any any MSVC project files.

For static builds, when NATPMP_STATICLIB is defined, it instead builds a static library (for the library project), or tries to link to a static library (from the main project). There is no difference between import and export for a static library.

If you did not have NATPMP_STATICLIB defined in the main project, it would treat the header files as defining DLL imports. Hence the link errors when you're trying to link to a static version of the library.

Well, I'm glad you managed to figure that out.

[Vagabond]

I didn't see NATPMP_EXPORTS in MSVC either.

Thanks for explaining Hooman. It makes more sense what was going on to me now. It is hard working with the MSVC projects since they are an afterthought in the dependencies and I don't know if their settings are wrong or I am messed up.

Thanks for the assist on GitHub with the win32 programming. I did some merging and left some notes for you to look at with the WIN32 programming. We will get this done eventually.

-Brett