Adding trade routes to cities based on traits

[TC01]

For Final Frontier Plus, Iwant to set it up so that traits can increase the number of trade routes a city has. This is to mimic some Python code in Final Frontier (that is tied to a specific trait; hence why I want to make it doable for all traits).

I already added the tag, and then added the code below (the stuff that's in red) to CvCity::getTradeRoutes():

int CvCity::getTradeRoutes() const
{
	int iTradeRoutes;

	iTradeRoutes = GC.getGameINLINE().getTradeRoutes();
	iTradeRoutes += GET_PLAYER(getOwnerINLINE()).getTradeRoutes();
	if (isCoastal(GC.getMIN_WATER_SIZE_FOR_OCEAN()))
	{
		iTradeRoutes += GET_PLAYER(getOwnerINLINE()).getCoastalTradeRoutes();
	}
	iTradeRoutes += getExtraTradeRoutes();

	[COLOR="Red"]for (int iTrait = 0; iTrait < GC.getNumTraitInfos(); iTrait++)
	{
		if (GET_PLAYER(getOwner()).hasTrait((TraitTypes)iTrait))
		{
			CvTraitInfo kTraitInfo = GC.getTraitInfo((TraitTypes)iTrait);
			iTradeRoutes += kTraitInfo.getNumBonusTradeRoutes();
		}
	}[/COLOR]

	return std::min(iTradeRoutes, GC.getDefineINT("MAX_TRADE_ROUTES"));
}

Unfortunately this code causes a CTD when starting a game.

With a debug DLL and with VC++ attached to BTS, I see that an unhandled exception occuring on the highlighed line (in CvPlayer::findStartingPlot, see below for the entire function):

CvPlot* CvPlayer::findStartingPlot(bool bRandomize)
{
	PROFILE_FUNC();

	long result = -1;
	CyArgsList argsList;
	argsList.add(getID());		// pass in this players ID
	[COLOR="Red"]if (gDLL->getPythonIFace()->callFunction(gDLL->getPythonIFace()->getMapScriptModule(), "findStartingPlot", argsList.makeFunctionArgs(), &result))[/COLOR]
	{
		if (!gDLL->getPythonIFace()->pythonUsingDefaultImpl()) // Python override
		{
			CvPlot *pPlot = GC.getMapINLINE().plotByIndexINLINE(result);
			if (pPlot != NULL)
			{
				return pPlot;
			}
			else
			{
				FAssertMsg(false, "python findStartingPlot() returned an invalid plot index!");
			}
		}
	}

	CvPlot* pLoopPlot;
	bool bValid;
	int iBestArea = -1;
	int iValue;
	int iRange;
	int iI;

	bool bNew = false;
	if (getStartingPlot() != NULL)
	{
		iBestArea = getStartingPlot()->getArea();
		setStartingPlot(NULL, true);
		bNew = true;
	}

	AI_updateFoundValues(true);//this sets all plots found values to -1
	
	if (!bNew)
	{
		iBestArea = findStartingArea();
	}

	iRange = startingPlotRange();
	for(int iPass = 0; iPass < GC.getMapINLINE().maxPlotDistance(); iPass++)
	{
		CvPlot *pBestPlot = NULL;
		int iBestValue = 0;
		
		for (iI = 0; iI < GC.getMapINLINE().numPlotsINLINE(); iI++)
		{
			pLoopPlot = GC.getMapINLINE().plotByIndexINLINE(iI);

			if ((iBestArea == -1) || (pLoopPlot->getArea() == iBestArea))
			{
				//the distance factor is now done inside foundValue
				iValue = pLoopPlot->getFoundValue(getID());

				if (bRandomize && iValue > 0)
				{
					iValue += GC.getGameINLINE().getSorenRandNum(10000, "Randomize Starting Location");
				}

				if (iValue > iBestValue)
				{
					bValid = true;

					if (bValid)
					{
						iBestValue = iValue;
						pBestPlot = pLoopPlot;
					}
				}
			}
		}

		if (pBestPlot != NULL)
		{
			return pBestPlot;
		}

		FAssertMsg(iPass != 0, "CvPlayer::findStartingPlot - could not find starting plot in first pass.");
	}

	FAssertMsg(false, "Could not find starting plot.");
	return NULL;
}

I would guess the problem is related to the fact that the Final Frontier mapscripts, which overrides findStartingPlot, place a player's starting city. Looking at the Python logs, I see that the last thing that happens before the CTD is that the mapscript places a city for player 0). But I'm not sure why that fact, plus the change I made to getTradeRoutes(), would cause this.

Any ideas as to what is wrong here? If nothing can be done I could probably do this somewhere else in the SDK, or even in Python where it was before (but using my XML tag instead of a specific trait), but I would prefer to make it work here.

 

[Afforess]

Danger! You have a problem right here:

CvTraitInfo kTraitInfo = GC.getTraitInfo((TraitTypes)iTrait);

This compiles, but doesn't work like you would expect it to. You need change it to this:

CvTraitInfo &kTraitInfo = GC.getTraitInfo((TraitTypes)iTrait);

I'm sure someone else who actually has a CS degree can explain why...

 

[TC01]

Danger! You have a problem right here:

CvTraitInfo kTraitInfo = GC.getTraitInfo((TraitTypes)iTrait);

This compiles, but doesn't work like you would expect it to. You need change it to this:

CvTraitInfo &kTraitInfo = GC.getTraitInfo((TraitTypes)iTrait);

I'm sure someone else who actually has a CS degree can explain why...

Thanks, now it's working.

What happens without the & there?

 

[Afforess]

The ampersand signifies you are modifying a value in memory... or something fancy like that. I'm sure EF can explain it, in a few hours.

 

[phungus420]

No CS degree, but I can tell you why. The & means you are evaluating the object by reference, ie you are looking at the object itself in CvTraitInfo. & is a fancy C++ way of expressing a pointer, complete with automatic dereferencing. There may be some more to it then that, but I know to the compiler & is treated exactly like a pointer. If you don't evaluate this object as a reference, ie the object itself where it exists in memory, you will be working with what's known as a shallow copy of the object. This means that if you call a function on the object, the values the functions return wol't work, as you're no longer evaluating the actual class object, but instead a set of data that is copied and in effect only mimics the object, and it only will exist in the stack in the specific function where you created this copy of the object. In otherwords the attributes you can look at and work with (which wol't be much actually), aren't really something in CvTraitInfo itself; it would sort of be like working with a picture of a dog, instead of a dog itself, it wol't work if you try to feed it, or play with it or for much really.

 

[Afforess]

Thanks Phungus, that's the kind of explanation I was looking for.

 

[EmperorFool]

Some of that explanation was pretty close. First, yes C++ implements references using pointers under the hood, but they are not pointers in the traditional sense. When you assign the reference of x to y, you are telling the compiler that y is another name for x. Why is that different? Mostly it doesn't matter except that references cannot be NULL (a language constraint).

As Phungus explained, without the reference a shallow copy of the CvTrainInfo object is created and stored on the stack. This by itself is not a problem. The code in that function will work with this shallow copy just fine. The problem arises when kTraitInfo goes out of scope at the end of the if() block. Since the object lives on the stack it is destroyed along with anything it points to. Thus its description (a CvWString object) and text key and any arrays it contains are all deleted.

Since it's a shallow copy, both the original and the copy pointed to the same (now deleted) objects. The crash occurs the next time the original object tries to access any of those delete objects it references.