Frustrating bug --- please help

[agnat86]

I'm currently trying to teach myself to mod. I've just learned how to edit and compile a DLL, and I never encounter any trouble when making simple modifications.

However, whenever I add new XML tags to XML files, things go wrong. The game crashes to desktop without a visible cause, and the only message I get is simply "Colonization has stopped working." This usually happens at the end of a turn, and it is reproducable.

I have added a new parameter (WaterExtraYield) to Civ4TraitInfos.XML, because I wanted to make a trait for natives (Fishermen) which increases the food production on water.

To do this, I looked at another parameter (ExtraYieldThresholds), which is the same kind of parameter (it also requires you to specify a yield and a number), searched for all occurences of this one in .cpp and .h files, and simply cloned all lines of code that contained "ExtraYieldThreshold" for "WaterExtraYield." I have based this more or less on this beginner's guide. I also made the necessary changes to CIV4CivilizationsSchema.xml.

But while my parameter works in exactly the same way as that other one (except obviously for the functional parts in CvPlot.cpp), it causes these mysterious crashes.

I have encountered this problem only once before, also when I tried to add an XML tag this way. Because the function of that one was completely different, it seems that this bug is caused not by an error in the code where I describe what it does, but merely by the act of adding an XML tag.

This is really annoying, as my new parameter works exactly the way I intended it to work, and the game sometimes progresses many turns in a row without any trouble, until it suddenly crashes. Does anyone know how this happens? Is there some other place where I should define the number of XML tags that a trait could have?

I would really appreciate your help on this, it would be hard to make new features if I were limited to the existing parameters in XML files.

Thanks in advance.

 

[Kailric]

I'm currently trying to teach myself to mod. I've just learned how to edit and compile a DLL, and I never encounter any trouble when making simple modifications.

However, whenever I add new XML tags to XML files, things go wrong. The game crashes to desktop without a visible cause, and the only message I get is simply "Colonization has stopped working." This usually happens at the end of a turn, and it is reproducable.

I have added a new parameter (WaterExtraYield) to Civ4TraitInfos.XML, because I wanted to make a trait for natives (Fishermen) which increases the food production on water.

To do this, I looked at another parameter (ExtraYieldThresholds), which is the same kind of parameter (it also requires you to specify a yield and a number), searched for all occurences of this one in .cpp and .h files, and simply cloned all lines of code that contained "ExtraYieldThreshold" for "WaterExtraYield." I have based this more or less on this beginner's guide. I also made the necessary changes to CIV4CivilizationsSchema.xml.

But while my parameter works in exactly the same way as that other one (except obviously for the functional parts in CvPlot.cpp), it causes these mysterious crashes.

I have encountered this problem only once before, also when I tried to add an XML tag this way. Because the function of that one was completely different, it seems that this bug is caused not by an error in the code where I describe what it does, but merely by the act of adding an XML tag.

This is really annoying, as my new parameter works exactly the way I intended it to work, and the game sometimes progresses many turns in a row without any trouble, until it suddenly crashes. Does anyone know how this happens? Is there some other place where I should define the number of XML tags that a trait could have?

I would really appreciate your help on this, it would be hard to make new features if I were limited to the existing parameters in XML files.

Thanks in advance.

As far as I know there is no set number of xml tags other than physical memory. I do know you can add all the xml tags you want and it will not effect the game at all unless you modify the code the do something with it so I would first assume there is a problem in your code rather than a problem in the xml tag. If the xml is wrong the game generally will not even start up or you'll have lots of xml errors as it loads. If you followed the example of the similar attributes in the Cvinfos and all is setup correctly then the problem could be in the execution of the new attribute. Post all relevent code in the C++ files so we can have a look at it.

Also, if you truly want to do some cool moding it is best to get a debug version up and running. You can use the free Mirosoft studio express 2008 edition if you don't have it that works with the Debug Makefile.

Read through that thread and you will learn a bit about it. With express 2008 you can actually attach the editor to the Colonization process. So that when the games crashes or you get a "failed assert" the game will pause and you can follow then follow the code line by line in real time as you play the game. This is indispensable in bug hunting.

 

[raystuttgart]

@Kailric:
I agree, with all you wrote.
But I suggest to choose Visual Studio 2010 C++ Express Edition.

@agnat86:
I you started with the "Beginner's Package" for DLL-Projects you find in my SVN, the setup for Debug-DLL is ready to go.
(The folder is called "Workspace". There is also an explanation called "Workspace Guide". If you follow those instructions your project is set up in 1min.)

 

[agnat86]

@ Kailric & raystuttgart

Thanks for the help! I will try to find out what goes wrong.

 

[Kailric]

@Kailric:
I agree, with all you wrote.
But I suggest to choose Visual Studio 2010 C++ Express Edition.

@agnat86:
I you started with the "Beginner's Package" for DLL-Projects you find in my SVN, the setup for Debug-DLL is ready to go.
(The folder is called "Workspace". There is also an explanation called "Workspace Guide". If you follow those instructions your project is set up in 1min.)

Agree with 2010 but I didn't know if that makefile would work for it. Nightinggale has made a new and improved makefile just recently found here http://forums.civfanatics.com/showthread.php?p=12549883.

I am not sure if that one works for Civ4Col but I know he made one cause we use it for the M:C project. One thing to note about the 2010 version when I first installed it it was missing some of the features like "Attach to Process". I figured out I could access it with the short cut ctrl-alt-P but soon I figured out that I needed to switch on "Expert" mode under Tools/Settings then select Expert. I have no idea why they would do that unless they truly intended you to be an expert that already knew the expert option was there. I merely found it by accident

 

[Nightinggale]

But I suggest to choose Visual Studio 2010 C++ Express Edition.

That's the one I use as well. Highly recommended.

@agnat86:
I you started with the "Beginner's Package" for DLL-Projects you find in my SVN, the setup for Debug-DLL is ready to go.
(The folder is called "Workspace". There is also an explanation called "Workspace Guide". If you follow those instructions your project is set up in 1min.)

I added project files to Religion and Revolution Extended and Medieval Conquest as well. Both should compile out of the box assuming you did step 1 in the modiki. While that guide mentions a makefile, I would recommend my own, which is also part of RaRE and M:C.
Pay special attention to "running the debugger" in that guide as this is how the computer can tell you why the game crashes. Do run in window mode when using the debugger. The debugger freezes the game when crashing and alt+tab will not work. I think control+alt+delete will allow you to move to the debugger, but that's a bit extreme for avoiding the window mode.

 

[agnat86]

I've tried running the game using a Debug-DLL, and at the point where it crashed, I got this message:

"Assert Failed

File: c:\users\frank\desktop\workspace\cvgamecoredll\CvPlayerAI.h
Line: 24
Expression: ePlayer >= 0
Message: Player is not assigned a valid value"

I do not understand what this means, I'm afraid.

This is the piece of code I added to CvPlot.cpp, containing three new variables that allow traits to increase yields on land, water and hills tiles respectively (my additions in bold:

Spoiler :

iYield += GC.getGameINLINE().getPlotExtraYield(m_iX, m_iY, eYield);

if (ePlayer != NO_PLAYER)
{
if (GET_PLAYER(ePlayer).getExtraYieldThreshold(eYield) > 0)
{
if (iYield >= GET_PLAYER(ePlayer).getExtraYieldThreshold(eYield))
{
iYield += GC.getDefineINT("EXTRA_YIELD");
}
}
}

if (!isWater())
{
if (!isImpassable())
{
CvCity* pWorkingCity = getWorkingCity();
if (NULL != pWorkingCity)
{
if (!bDisplay || pWorkingCity->isRevealed(eTeam, false))
{
iYield += GET_PLAYER(ePlayer).getLandExtraYield(eYield);
}
}
}
}

if (isWater())
{
if (!isImpassable())
{
CvCity* pWorkingCity = getWorkingCity();
if (NULL != pWorkingCity)
{
if (!bDisplay || pWorkingCity->isRevealed(eTeam, false))
{
iYield += GET_PLAYER(ePlayer).getWaterExtraYield(eYield);
}
}
}
}

if (isHills())
{
if (!isImpassable())
{
CvCity* pWorkingCity = getWorkingCity();
if (NULL != pWorkingCity)
{
if (!bDisplay || pWorkingCity->isRevealed(eTeam, false))
{
iYield += GET_PLAYER(ePlayer).getHillsExtraYield(eYield);
}
}
}
}

and a second part:

Spoiler :

int iExtraYieldThreshold = 0;
for (int iTrait = 0; iTrait < GC.getNumTraitInfos(); iTrait++)
{
CvTraitInfo& trait = GC.getTraitInfo((TraitTypes)iTrait);
iExtraYieldThreshold = std::max(trait.getExtraYieldThreshold(eYield), iExtraYieldThreshold);
}
if (iExtraYieldThreshold > 0 && iMaxYield > iExtraYieldThreshold)
{
iMaxYield += GC.getDefineINT("EXTRA_YIELD");
}

if (!isWater() && !isImpassable())
{
int iLandExtraYield = 0;
for (int iTrait = 0; iTrait < GC.getNumTraitInfos(); iTrait++)
{
CvTraitInfo& trait = GC.getTraitInfo((TraitTypes)iTrait);
iLandExtraYield = std::max(trait.getLandExtraYield(eYield), iLandExtraYield);
}
iMaxYield += iLandExtraYield;
}

if (isWater() && !isImpassable())
{
int iWaterExtraYield = 0;
for (int iTrait = 0; iTrait < GC.getNumTraitInfos(); iTrait++)
{
CvTraitInfo& trait = GC.getTraitInfo((TraitTypes)iTrait);
iWaterExtraYield = std::max(trait.getWaterExtraYield(eYield), iWaterExtraYield);
}
iMaxYield += iWaterExtraYield;
}

if (isHills() && !isImpassable())
{
int iHillsExtraYield = 0;
for (int iTrait = 0; iTrait < GC.getNumTraitInfos(); iTrait++)
{
CvTraitInfo& trait = GC.getTraitInfo((TraitTypes)iTrait);
iHillsExtraYield = std::max(trait.getHillsExtraYield(eYield), iHillsExtraYield);
}
iMaxYield += iHillsExtraYield;
}

return iMaxYield;
}

 

[raystuttgart]

You are missing security checks.

e.g. checks like this:

if (ePlayer != NO_PLAYER)

 

[Nightinggale]

"Assert Failed

File: c:\users\frank\desktop\workspace\cvgamecoredll\CvPlayerAI.h
Line: 24
Expression: ePlayer >= 0
Message: Player is not assigned a valid value"

I do not understand what this means, I'm afraid.

It mean a function is called with player as an argument where it is supposed to be a real player and instead it's NO_PLAYER (which is -1)

The original code does

if (ePlayer != NO_PLAYER)
	{
		if (GET_PLAYER(ePlayer)

You addition uses GET_PLAYER() without checking if ePlayer is different from NO_PLAYER. Change your code to only execute if ePlayer != NO_PLAYER and it should work. Well it should fix THIS bug.

 

[agnat86]

Great, I've added three security checks to the first part, and now it works fine. Thanks very much!

 

[Kailric]

Great, I've added three security checks to the first part, and now it works fine. Thanks very much!

Cool, I hope what we have here is an awesome up and coming Colonization modder!!

I had all kinds of questions like these when I first started. Fans of the game on these forums are all about helping, well as long as you don't have to drive anywhere .

Your problem was and still is one of my major pit falls. If its a variable in the function that means there is a chance that it will be NULL or -1 so always remember to do security checks.

I have a question too while we are here. Why in the code is there a getOwner() and a getOwnerINLINE() ?

When all getOwner() does is...

PlayerTypes CvCity::getOwner() const
{
	return getOwnerINLINE();
}

I think I messed up several times by doing a GET_PLAYER(getOwner()) as doing a GET_PLAYER(getOwnerINLINE()) would actually skip a step and would seem to be faster.

 

[raystuttgart]

Why in the code is there a getOwner() and a getOwnerINLINE() ?

getOwnerINLINE() is simply an inline function (thus supposed to be faster) that is available at certain objects (Unit, City, Plot, ...).

There are other inline functions as well, e.g. getGameINLINE().

I have tried to read and understand tutorials about inlined functions a while ago.
It is not necessarily that clear.

It is a non-standard construct that some compilers support and interpret to optimize the generated code.
It is supposed to eliminate time overhead of the call.

Actually it is still not that clear to me, when to explizitly inline functions or not.
Since when you do not explicitly tell the compiler, the compiler might try to optimize (and inline) by itself.

However, when inline functions already exist, I always call these instead of non-inline functions if those are available as well.

Edit:

If you have a wrapping function around an inline function, it is usually only done to give less experience programmers a method with type checking.
(Since inline functions do not have type checking and thus it is more likely to make errors when using them.)

That is probably the answer to your question why both methods exist.

 

[Nightinggale]

Actually it is still not that clear to me, when to inline functions or not.

It's a very tricky issue, but there is one case where it is always beneficial.

#ifdef _USRDLL
	inline PlayerTypes getOwnerINLINE() const
	{
		return m_eOwner;
	}
#endif

Return a variable is always faster when it is inline and the dll will be smaller as that inlined return statement will generate less code than a function call.

You should NOT use inline if the function produces too much code. The problem is that the CPU cache is limited and the bigger the dll, the more likely it is that the next part of the code isn't in the cache and the CPU will have to wait for the RAM.

While we are at the mysteries: #define _USRDLL looks odd as it is always defined. As it is defined in the makefile VC++ couldn't figure it out until I modified the project file to tell which flags the makefile defines (hence a reason to download the project files I uploaded). Red lines for undefined flags in compilable code isn't a good thing. In fact it was rather annoying.
I think it's a "leftover" from when the game was compiled and never removed just to annoy modders.

I think I messed up several times by doing a GET_PLAYER(getOwner()) as doing a GET_PLAYER(getOwnerINLINE()) would actually skip a step and would seem to be faster.

It would be a bit faster, but I don't think you will be able to tell the difference. It's not the place where the big speed boost is hidden.

There is one potential problem with the code you wrote. Do you know for sure that getOwnerINLINE() will not return NO_PLAYER?