Makefile2 improved

[billw2015]

I merged makefile 2.5 into our one, with some improvements that allow it to make parallel builds in all build configurations. I also updated the project file to use it, and updated the deps.exe.

To test it out:

1. Unzip the attached file into your Sources directory
2. Run deps.exe and extract direct into the Sources directory as well, it should create the deps folder with everything in it.
3. Then you should be able to build, run, debug, profile etc., from Visual Studio.

Cool features it adds:

• A profile specific build, this is like the release one we currently have
• An assert build that is almost as fast as release but has asserts enabled
• Parallel build all over
• No more final_release, it is just release. probably we would check in distribute profile or assert build instead of release one. I would suggest assert so they get fixed! That would mean we can also get rid of the .debug dll and the instructions to test against it.
• The deps.exe so the entire build is self contained.

/edit Updated to fix intellisense settings.

Mirror cos this website download sucks:

https://1drv.ms/u/s!Aj1eMcUFhShGhEmR72MyF5GCxTMl

 

[Thunderbrd]

That would mean we can also get rid of the .debug dll and the instructions to test against it.

Why would we want to do this? debug is still the main one to run to attach VS to isn't it?

 

[olduser]

Here are a few things i noticed.

• A *.pdb should be created for all configurations
• The Debug configuration doesn't need /DFP_PROFILE_ENABLE /DUSE_INTERNAL_PROFILER because profiling without compiler optimizations makes no sense.
• The Assert configuration shouldn't have /DFP_PROFILE_ENABLE /DUSE_INTERNAL_PROFILER because profiling with asserts enabled which take some cpu time makes no sense. The dll built with this configuration could be used by modders.
  
• The Profile configuration should have /DFP_PROFILE_ENABLE /DUSE_INTERNAL_PROFILER to use it for profiling with the internal profiler.
• The Release configuration should be shipped when a release is made and used for profiling with external tools or Visual Studio.

 

[olduser]

Why would we want to do this? debug is still the main one to run to attach VS to isn't it?

Yes but we added the debug dll to the svn to make it possible for xml or python modders to check their work with all assert checks enabled.

 

[billw2015]

A *.pdb should be created for all configurations

/edit I checked, they are but the flag is specified in two different places for some reason.

The Debug configuration doesn't need /DFP_PROFILE_ENABLE /DUSE_INTERNAL_PROFILER because profiling without compiler optimizations makes no sense.

This is how it was in the original makefile so I kept it that way. The cost of the profile code I guess is insignificant relatively speaking, and I guess you might have problems that relate to that code itself? I don't think they are a problem.
The idea that profiling in debug makes no sense has some validity, but only serves as a general rule, not a blanket statement. It depends on what the optimizer is doing, how extreme the performance issue is, if you already know the area you are interested in and can exclude misleading parts of the profile results. The original perf issue I found in this code base was found in debug profiling (I couldn't get the optimized profile results in VS at the time).

The Assert configuration shouldn't have /DFP_PROFILE_ENABLE /DUSE_INTERNAL_PROFILER because profiling with asserts enabled which take some cpu time makes no sense. The dll built with this configuration could be used by modders.

The internal profiler is made for modders isn't it? It is what gives python and C++ perf results *without* needing to know 3rd party tools. And again the perf cost is negligible, and was already present in the previous "Release" build that modders were using. It amounts to about 5% of the total time of end of turn. Still I will remove it if you don't agree with my previous suggestions about its use for modders, after all 5% is 5%.

The Profile configuration should have /DFP_PROFILE_ENABLE /DUSE_INTERNAL_PROFILER to use it for profiling with the internal profiler.

The Release configuration should be shipped when a release is made and used for profiling with external tools or Visual Studio.

Yeah this is fine if we don't want to differ other compiler flags between profile and release.
/edit I will test this change now

 

[olduser]

This is how it was in the original makefile so I kept it that way. The cost of the profile code I guess is insignificant relatively speaking, and I guess you might have problems that relate to that code itself? I don't think they are a problem.
The idea that profiling in debug makes no sense has some validity, but only serves as a general rule, not a blanket statement. It depends on what the optimizer is doing, how extreme the performance issue is, if you already know the area you are interested in and can exclude misleading parts of the profile results. The original perf issue I found in this code base was found in debug profiling (I couldn't get the optimized profile results in VS at the time).

Here's another reason try stepping through the code while debugging with the internal profiler enabled. You have step trough the profiler code as well and if you want to step backward into another function you can crash the profiler.

The internal profiler is made for modders isn't it? It is what gives python and C++ perf results *without* needing to know 3rd party tools. And again the perf cost is negligible, and was already present in the previous "Release" build that modders were using. It amounts to about 5% of the total time of end of turn. Still I will remove it if you don't agree with my previous suggestions about its use for modders, after all 5% is 5%.

The dll in the svn is the FinalRelease dll without the profiler.

At moment the Release dll which exists for profiling with the internal profiler doesn't have the asserts enabled. But Assert dll would that would make the Assert dll even slower. I once put the Release dll into the svn insted of the FinalRelease and the slowdown was instantly reported.

 

[billw2015]

Here's another reason try stepping through the code while debugging with the internal profiler enabled. You have step trough the profiler code as well and if you want to step backward into another function you can crash the profiler.

Yeah good reason, I will remove the profiler flags from debug.

At moment the Release dll which exists for profiling with the internal profiler doesn't have the asserts enabled. But Assert dll would that would make the Assert dll even slower. I once put the Release dll into the svn insted of the FinalRelease and the slowdown was instantly reported.

What should the dll(s) in the repo be when using this makefile? I would suggest based on what you said that it is only the assert dll, without profiling. This will lead to asserts getting fixed, and assert reports from users of the svn. I think people on svn are agreeing to help fix bugs right? Asserts should certainly count.
Alternatively it could be the release dll as the main one, and assert dll as the .debug one (maybe rename it to .assert for clarity)? That would match what is currently in there, but with better perf on the assert enabled one. However it would lead to asserts being ignored probably, and makes it more annoying to check them for modders.

 

[olduser]

What should the dll(s) in the repo be when using this makefile?

I would use the Assert dll without profiler.

This will lead to asserts getting fixed, and assert reports from users of the svn.

I would hope so but probably some will be just commented out because they disrupt the game play.

 

[Toffer90]

As a non-dll modder I only see the need for, on the SVN, the fastest possible default dll and one alternative that gives asserts while otherwice being as fast as possible.
If dll modders need other dll's for debugging then I think it's best they have them locally, not on the SVN.

 

[billw2015]

Assuming the assert dll is within a few % of the speed of the release one, why would it not be the main dll for modders?

 

[Toffer90]

Assuming the assert dll is within a few % of the speed of the release one, why would it not be the main dll for modders?

Because it's annoying to play the game with asserts popping up all the time, many more play the SVN than mod it, modders play it a lot too.
SO, the founder of C2C doesn't mod anymore, but he always use the latest SVN and I think he plays a lot, he would explode if he couldn't play the game without asserts ruining the flow.
And he would probably not like to have to rename dll files every time the dll has been updated on the SVN.

Very few non-dll modders use the current debug dll more than: "start game, end turn once, exit game." (this too is often skipped unless the xml changes are extensive or experimental) simply because it is so darned slow, but I'm sure an alternative assert dll that is only slightly slower than the default dll would get used a lot more by non-dll modders, even if we have to rename the dll's to switch to the assert dll.

 

[Thunderbrd]

Yes but we added the debug dll to the svn to make it possible for xml or python modders to check their work with all assert checks enabled.

I see, and we'd use a different version for this function in this case so it wouldn't be necessary to package it into the Assets folder, even though WE would still use it when we need to (we'd just build it on demand). Am I understanding this correctly?

Assuming the assert dll is within a few % of the speed of the release one, why would it not be the main dll for modders?

There are numerous asserts that have been firing for years that nobody knows a fix to. This might be something we could get away with if we had a clean code and didn't expect for there to be numerous asserts that get ignored every run. But that is not the case at the moment. Too many issues exist that have been beyond our ability to resolve since Koshling and AIAndy stopped being directly involved in modding. There are some I'm aware of that I believe I can eventually fix but are overwhelmingly complex to me and have intimidated me from working on them right away. Making an assert dll an SVN version DLL would be... horrific.

 

[billw2015]

There are numerous asserts that have been firing for years that nobody knows a fix to. This might be something we could get away with if we had a clean code and didn't expect for there to be numerous asserts that get ignored every run. But that is not the case at the moment. Too many issues exist that have been beyond our ability to resolve since Koshling and AIAndy stopped being directly involved in modding. There are some I'm aware of that I believe I can eventually fix but are overwhelmingly complex to me and have intimidated me from working on them right away. Making an assert dll an SVN version DLL would be... horrific.

And if it magically had 0 current asserts how you would feel about it being the default going forward?

 

[Toffer90]

And if it magically had 0 current asserts how you would feel about it being the default going forward?

That would make it ok by me, to use it as default if asserts are rare I mean.

 

[Thunderbrd]

And if it magically had 0 current asserts how you would feel about it being the default going forward?

Sure, once the code is cleaned of what we know is there.

 

[olduser]

Another thing that seems to be missing is running SubWCRev.exe to generate the version.h.

 

[billw2015]

Another thing that seems to be missing is running SubWCRev.exe to generate the version.h.

Good catch thanks, I will put that in, and double check again I didn't miss anything else.

 

[MattCA]

Was this forgetten or do we not need version.h?