[SDK]Warlords and/or 161

Iustus

King
Joined
Jul 18, 2006
Messages
609
Location
Sunnyvale, CA
The intent of this thread is to list specific code bugs found in the code of the SDK, now in the 2.08 Warlords code. (The older bugs for 161 and 2.0 warlords included but listed as fixed).

Only specific references to specific lines of code from the SDK should be listed.

The list: updated Feb 20, 2007

  1. Assert in declareWar always fires when AI declares war on vassal of a human player (Warlords 2.08 only)
  2. FreeWinsVsBarbs from the handicap infos is not correctly used (actually causes the barbarians to win in some cases)
  3. getTradeString has a crash bug when generating a trade string for city trades
  4. AI_endWarVal does not count the power of masters of a vassal (but it does count power of the vassals of a master)
  5. getHurryCost has an off by one error, causing some pop rushes to not complete the next turn
  6. CvTeam::setVassal does not clear the pending warplans of the vassal
  7. CvTeam::addTeam and CvTeam::shareCounters (called by addTeam) incorrectly clear the warplan for active wars
  8. CvGameTextMgr:: parseCivicInfoand CvGameTextMgr::setBuildingHelp incorrectly display all buildings of the same class with BuildingHappinessChanges. (Visible once the XML bug is fixed so that Zulu specific barracks also gives +2 happy from Nationhood)

Fixed in Warlords 2.08:
  1. AI_bestTech misvalues some techs which enable improvements (fixed in 2.08)
  2. AI_foundValue - undervalue initial coastal locations (fixed in 2.08)
  3. (AI combat odds 161 only)
  4. AI_doDiplo - vassal accepted without proper war check (2.08 fixes another way)
  5. AI_doDiplo - vassal requested every turn (fixed in 2.08)
  6. doResearch - overflow research is not adjusted by calculateResearchModifier (fixed in 2.08)
  7. getCombatOdds - iAttackerOdds can be off by one in some cases (fixed in 2.08)
  8. doCombat - the defender is given a 1 in 1000 bonus chance to win each combat round (which is not accounted for in getCombatOdds) (fixed in 2.08)


-Iustus
 
AI_foundValue (in CVPlayerAI.cpp), lines 1600-1615 (the end of the function)

Code:
if (bStartingLoc)
{
	iDifferentAreaTile = 0;

	for (iI = 0; iI < NUM_CITY_PLOTS; iI++)
	{
		pLoopPlot = plotCity(iX, iY, iI);

		if ((pLoopPlot == NULL) || (pLoopPlot->area() != pArea))
		{
			iDifferentAreaTile++;
		}
	}

	iValue /= (max(0, (iDifferentAreaTile - ((NUM_CITY_PLOTS * 2) / 3))) + 2);
}

This code will degrade the value of coastal cities for every offshore tile.

One possible fix:
Code:
if (bStartingLoc)
{
	iDifferentAreaTile = 0;

	for (iI = 0; iI < NUM_CITY_PLOTS; iI++)
	{
		pLoopPlot = plotCity(iX, iY, iI);

		if ((pLoopPlot == NULL) || ([B]!(pLoopPlot->isWater() && pPlot->isCoastalLand()) &&[/B] (pLoopPlot->area() != pArea)))
		{
			iDifferentAreaTile++;
		}
	}

	iValue /= (max(0, (iDifferentAreaTile - ((NUM_CITY_PLOTS * 2) / 3))) + 2);
}

This would not count water tiles as 'different' for starting plots.

This does still not solve the problem that all water tiles are counted as having only 1 food, not the 2 you would get with a lighthouse that every human player uses to count all sea tiles.

That is a tougher problem to solve however. A call to something like getSeaPlotYield is indicated, but there is currently no way to make an adjustment for a city that does not yet exist with a hypothetical lighthouse improvement. Doing so while not making assumptions about the XML content would be tricky, but possible.

-Iustus
 
161 only (already fixed in Warlords):

The AI version (faster) to calculate combat odds uses the pre 152 patch formulas, so badly misestimates odds, as documented here

-Iustus
 
(Warlords only)

in CvPlayerAI.cpp, AI_doDiplo, lines 7415-7432:

Code:
bool bAccepted = true;
TeamTypes eMasterTeam = GET_PLAYER((PlayerTypes)iI).getTeam();
for (int iTeam = 0; iTeam < MAX_CIV_TEAMS; iTeam++)
{
	if (iTeam != getTeam() && iTeam != eMasterTeam && atWar(getTeam(), (TeamTypes)iTeam) && !atWar(eMasterTeam, (TeamTypes)iTeam))
	{
		if (GET_TEAM(eMasterTeam).AI_declareWarTrade((TeamTypes)iTeam, getTeam(), false) != NO_DENIAL)
		{
			bAccepted = false;
			break;
		}
	}
}

if (bAccepted)
{
	GC.getGameINLINE().implementDeal(getID(), ((PlayerTypes)iI), &ourList, &theirList);
}

the false passed to AI_declareWarTrade means power is not compared. The simple fix is

Code:
if (GET_TEAM(eMasterTeam).AI_declareWarTrade((TeamTypes)iTeam, getTeam(), [B]true[/B]) != NO_DENIAL)

but this would not take into account multiple wars. If you want to make a proper calculation of the total power of everyone the potential vassal is at war with, you could do it the following way:

Code:
bool bAccepted = true;
[B]int iSumDefensivePower = 0;[/B]
TeamTypes eMasterTeam = GET_PLAYER((PlayerTypes)iI).getTeam();
for (int iTeam = 0; iTeam < MAX_CIV_TEAMS; iTeam++)
{
	if (iTeam != getTeam() && iTeam != eMasterTeam && atWar(getTeam(), (TeamTypes)iTeam) && !atWar(eMasterTeam, (TeamTypes)iTeam))
	{
		if (GET_TEAM(eMasterTeam).AI_declareWarTrade((TeamTypes)iTeam, getTeam(), false) != NO_DENIAL)
		{
			bAccepted = false;
			break;
		}
		[B]else
		{
			bool bLandTarget = AI_isLandTarget((TeamTypes)iTeam);
			
			// do not use getDefensivePower, that would count vassals twice
			// could add an assert here to confirm that every vassal of iTeam is also at war with us
			iSumDefensivePower += (GET_TEAM((TeamTypes)iTeam).getPower(false) / ((bLandTarget) ? 2 : 1));
		}[/B]
	}
}
[B]if (bAccepted)
{
	// could modify iSumDefensivePower by a factor from leaderhead if desired
	if (iSumDefensivePower > getPower(true) + GET_TEAM(eMasterTeam).getPower(true))
		bAccepted = false;
}[/B]

if (bAccepted)
{
	GC.getGameINLINE().implementDeal(getID(), ((PlayerTypes)iI), &ourList, &theirList);
}

-Iustus
 
(Warlords only)
a second issue with AI_doDiplo, in CvPlayerAI.cpp, lines 7348-7394:

Code:
if (AI_getContactTimer(((PlayerTypes)iI), CONTACT_PERMANENT_ALLIANCE) == 0)
{
	bool bOffered = false;
	if (GC.getGameINLINE().getSorenRandNum(GC.getLeaderHeadInfo(getPersonalityType()).getContactRand(CONTACT_PERMANENT_ALLIANCE), "AI Diplo Alliance") == 0)
	{
	[I]...[/I]
	}

	if (!bOffered)
	{
		setTradeItem(&item, TRADE_VASSAL);

		if (canTradeItem((PlayerTypes)iI, item, true))
		{
			ourList.clear();
			theirList.clear();

There is a random check, to see if we want to offer a permenant alliance, however if we do not, there is neither a random nor a evaluation made whether an offer should be made to become a vassal. As soon as is possible, and then every turn after that, every AI offers to make itself a vassal to everyone until it finally becomes a vassal or can no longer offer (because it owns a vassal, etc).

Note, that PEACE_TREATY_LENGTH*2 turns later, it will evaluate the deal, perhaps canceling it, using AI_considerOffer with a human master and getTradeDenial with an AI master.

It is entirely possible I am missing something, as I have not stepped through the code as it is running, but I cannot for the life of me see where any check is made.

As far as I can tell, the CONTACT_PERMANENT_ALLIANCE timer, (in fact all timers), starts at zero. Since this seems to be the only place that timer is changed, I cannot see how it can be anything but zero until the first time an AI offers itself as a vassal, and for AIs, the value is never set, so they will offer every turn.

The fix is a bit more complicated, as I am not sure what the intended behavior should be. Is the desired result that every AI settle into a vassal/master state as soon as possible? Or should it only happen when there is a large power differential? Or should it happen when an AI is afraid of someone?

I think at a minimum, AIs should not voluntarily ask to be vassals of players who have a lower power value. (Just getPower(false) for each team and compare). Even better would be for an AI to evaluate each team and then pick the best match to be a vassal to, looking at things like distance from us (is it better to be on the same continent as our master, so they can protect us?), power of the vassal, etc.

Perhaps something about how desperate we are for a master compared to the power of the master. This would cause civs that are losing a war to look for weaker and weaker masters as they continue to lose.

-Iustus
 
Warlords and 161

the calculated "turns remaining" science rate does not agree with actual turns.

calculateResearchModifier() should be called to modify getOverflowResearch()

documented here

(do_Research, line 11338 in CvPlayers.cpp, Warlords version)

-Iustus
 
Warlords and 161
CvGameCoreUtils.cpp, line 575, in getCombatOdds:

Code:
iAttackerOdds = ((GC.getDefineINT("COMBAT_DIE_SIDES") * iAttackerStrength) / (iAttackerStrength + iDefenderStrength));

this can actually be off by one. In order for the combinatorial math to work out correctly, iDefenderOdds + iAttackerOdds must equal (GC.getDefineINT("COMBAT_DIE_SIDES") (which is 1000 by default). In fact, because of integer math, this can round down, making iAttackerOdds one short of the real number.

the fix:
Code:
iAttackerOdds = (GC.getDefineINT("COMBAT_DIE_SIDES") - iDefenderOdds;

If you do not see why this is so, take a look at the combat code, CvUnit.cpp, updateCombat:

line 883:
Code:
iDefenderOdds = ((GC.getDefineINT("COMBAT_DIE_SIDES") * iDefenderStrength) / (iAttackerStrength + iDefenderStrength));

line 921
Code:
if (GC.getGameINLINE().getSorenRandNum(GC.getDefineINT("COMBAT_DIE_SIDES"), "Combat") <= iDefenderOdds)

line 955
Code:
else

clearly, the defender odds are used, and if the roll fails, then the round off goes to the attacker. (however, there is another off by one bug here, see next post).

-Iustus
 
Warlords and 161
the combat code, CvUnit.cpp, updateCombat:

line 883:
Code:
iDefenderOdds = ((GC.getDefineINT("COMBAT_DIE_SIDES") * iDefenderStrength) / (iAttackerStrength + iDefenderStrength));

line 921
Code:
if (GC.getGameINLINE().getSorenRandNum(GC.getDefineINT("COMBAT_DIE_SIDES"), "Combat") [B]<=[/B] iDefenderOdds)

this has an off by one error. getSorenRandNum(1000) returns a random number from 0-999. So that check should be just less than, not less than or equal to.

the fix:
line 921
Code:
if (GC.getGameINLINE().getSorenRandNum(GC.getDefineINT("COMBAT_DIE_SIDES"), "Combat") [B]<[/B] iDefenderOdds)

Note, if the intent is to give the defender a 1 in 1000 extra edge (well 1 in COMBAT_DIE_SIDES actually), then the getCombatOdds function (see above post), needs to be changed to add one to iDefenderOdds. However, I do not think that is the intent, this seems like a true bug in updateCombat to me.

-Iustus
 
in CvTeam.cpp, CvTeam::declareWar, line 941

Code:
FAssertMsg((isHuman() || isMinorCiv() || GET_TEAM(eTeam).isMinorCiv() || isBarbarian() || GET_TEAM(eTeam).isBarbarian() || AI_isSneakAttackReady(eTeam) || (GET_TEAM(eTeam).getAtWarCount(true) > 0) || !(GC.getGameINLINE().isFinalInitialized()) || gDLL->GetWorldBuilderMode() || getVassalCount() > 0  || isAVassal() || getDefensivePactCount() > 0), "Possibly accidental AI war!!!");

the bug is that it was changed so that you declare war on the vassels and masters of the person you declaring war on, not the other way around.

So, you need to add those two checks

Code:
FAssertMsg((isHuman() || isMinorCiv() || GET_TEAM(eTeam).isMinorCiv() || isBarbarian() || GET_TEAM(eTeam).isBarbarian() || AI_isSneakAttackReady(eTeam) || (GET_TEAM(eTeam).getAtWarCount(true) > 0) || !(GC.getGameINLINE().isFinalInitialized()) || gDLL->GetWorldBuilderMode() || getVassalCount() > 0  || isAVassal() || [B]GET_TEAM(eTeam).getVassalCount() > 0  || GET_TEAM(eTeam).isAVassal() [/B]|| getDefensivePactCount() > 0), "Possibly accidental AI war!!!");

This is the relavent code that was already changed in 2.08 which caused the need for this fix (lines 1201-1217):
Code:
for (iI = 0; iI < MAX_TEAMS; iI++)
{
	if (iI != getID() && iI != eTeam)
	{
		if (GET_TEAM((TeamTypes)iI).isAlive())
		{
			if (GET_TEAM((TeamTypes)iI).isVassal(eTeam) || GET_TEAM(eTeam).isVassal((TeamTypes)iI))
			{
				[B]declareWar((TeamTypes)iI, bNewDiplo);[/B]
			}
			else if (GET_TEAM((TeamTypes)iI).isVassal(getID()) || isVassal((TeamTypes)iI))
			{
				GET_TEAM((TeamTypes)iI).declareWar(eTeam, bNewDiplo);
			}
		}
	}
}

-Iustus
 
CvUnit.cpp, updateCombat, lines 903-916:

Code:
if (pDefender->isBarbarian())
{
	if (GET_PLAYER(getOwnerINLINE()).getWinsVsBarbs() < GC.getHandicapInfo(GET_PLAYER(getOwnerINLINE()).getHandicapType()).getFreeWinsVsBarbs())
	{
		[B]iDefenderOdds = min(10, iDefenderOdds);[/B]
	}
}
if (isBarbarian())
{
	if (GET_PLAYER(pDefender->getOwnerINLINE()).getWinsVsBarbs() < GC.getHandicapInfo(GET_PLAYER(pDefender->getOwnerINLINE()).getHandicapType()).getFreeWinsVsBarbs())
	{
		[B]iDefenderOdds = max(90, iDefenderOdds);[/B]
	}
}

At some point, pre 1.61, the combat rolls were changed from 1-100 to 1-1000, but this code was not updated.

This is a pretty nasty bug, in that it will cause human players at easy difficulty settings to automatically lose a few defensive battles they were supposed to automatically win.

This is the fix:

Code:
if (pDefender->isBarbarian())
{
	if (GET_PLAYER(getOwnerINLINE()).getWinsVsBarbs() < GC.getHandicapInfo(GET_PLAYER(getOwnerINLINE()).getHandicapType()).getFreeWinsVsBarbs())
	{
		[B]iDefenderOdds = min((10 * GC.getDefineINT("COMBAT_DIE_SIDES")) / 100, iDefenderOdds);[/B]
	}
}
if (isBarbarian())
{
	if (GET_PLAYER(pDefender->getOwnerINLINE()).getWinsVsBarbs() < GC.getHandicapInfo(GET_PLAYER(pDefender->getOwnerINLINE()).getHandicapType()).getFreeWinsVsBarbs())
	{
		[B]iDefenderOdds = max((90 * GC.getDefineINT("COMBAT_DIE_SIDES")) / 100, iDefenderOdds);[/B]
	}
}

-Iustus
 
CvGameTextMgr.cpp, in CvGameTextMgr::getTradeString, line 7882:

Code:
szBuffer.Format(L"%s", GET_PLAYER(ePlayer1).getCity(tradeData.m_iData)->getName());

This causes a potential crash (it crashed on me once). The problem is that
CvCity::getName returns a CvWString, not a CString, the fix is easy:

Code:
szBuffer.Format(L"%s", GET_PLAYER(ePlayer1).getCity(tradeData.m_iData)->getName()[B].GetCString()[/B]);

-Iustus
 
CvTeamAI.cpp, in AI_endWarVal, lines 896-897:

Code:
iValue *= (GET_TEAM(eTeam).[B]getPower[/B](true) + 10);
iValue /= max(1, (getPower(true) + GET_TEAM(eTeam).[B]getPower[/B](true) + 10));

this does not take into account the power of masters of a vassal (we do not need to do this calc on ourselves, its not really relavant)

instead, getDefensivePower should be used:

Code:
iValue *= (GET_TEAM(eTeam).[B]getDefensivePower[/B]() + 10);
iValue /= max(1, (getPower(true) + GET_TEAM(eTeam).[B]getDefensivePower[/B]() + 10));

you could also clean it up so that it doesnt call the function twice:

Code:
int iOurPower = getPower(true);
int iTheirPower = GET_TEAM(eTeam).getDefensivePower();

iValue *= iTheirPower + 10;
iValue /= max(1, iOurPower + iTheirPower + 10);

-Iustus
 
This is a tricky one to explain, the bug is in CvCity.cpp, getHurryCost, lines 4301-4320:

Code:
int CvCity::getHurryCost(bool bExtra, int iProductionLeft, int iHurryModifier, int iModifier) const
{
	[B]int iProduction = (iProductionLeft * iHurryModifier) / 100;[/B]

	if (bExtra)
	{
		int iExtraProduction = getExtraProductionDifference(iProduction, iModifier);
		if (iExtraProduction > 0)
		{
			int iAdjustedProd = iProduction * iProduction;
			iProduction = iAdjustedProd / iExtraProduction;
			if (iAdjustedProd % iExtraProduction > 0)  // round up
			{
				++iProduction;
			}
		}
	}

	return max(0, iProduction);
}

The problem is a rounding issue. In short, the fix is:
Code:
	int iProduction = [B]([/B](iProductionLeft * iHurryModifier) [B]+ (100 - 1))[/B] / 100;

Without this fix, the following assert will fire under real conditions, hurryProduction, lines 4385, 4400:

Code:
int CvCity::hurryProduction(HurryTypes eHurry) const
{
	int iProduction;

	if (GC.getHurryInfo(eHurry).getProductionPerPopulation() > 0)
	{
		iProduction = (100 * getExtraProductionDifference(hurryPopulation(eHurry) * GC.getGameINLINE().getProductionPerPopulation(eHurry))) / max(1, getHurryCostModifier());
		[B]FAssert(iProduction >= productionLeft());[/B]
	}
	else
	{
		iProduction = productionLeft();
	}

	return iProduction;
}

This happened in the following example:
iProduction = 335
productionLeft() = 336

More details:
getHurryCostModifier = 134 (epic speed)
getProductionPerPopulation = 45 (epic speed)
getExtraProductionDifference(x) = 2*x (200% production boost in city)
hurryPopulation() = 5

this gives:
iProduction = (100 * (2 * (5 * 45))) / 134
iProduction = (100 * (2 * (225)) / 134
iProduction = (100 * 450) / 134
iProduction = 335 (which is one short of 336 production left)

If you look at the code for getHurryCost, you can see how this happens:
Code:
iProduction = (iProductionLeft * iHurryModifier) / 100;
iProduction = (336 * 134) / 100
iProduction = 450

Well that is the problem. That 450 is going to result in 335 when it is transformed back. We need a 451 result in order for the integer math to transform correctly. (It will result in one more population cost, resulting in 6 * 45 * 2 / 134 = 402 production for 6 population, rather than 335 production for 5 population).

Note, another fix would be to give an extra production when the rounding is right, but the whole idea was to always be consistent about how many pop was how much production, to eliminate the 'best times' to pop rush. (If you wanted to do that, you could change hurryProduction to round up.)

-Iustus
 
in CvTeam.cpp, CvTeam::setVassal, line 3282-3328

This is not so much an error in the code that is there, but rather code that is missing. If an AI has a warplan, but they have not declared war yet, the warplan still is active when they become a vassal. It can cause some strange behavior when they try to declare war (but it will fail now).

There is a similar bug in permenant alliances, see next post.

Here is the existing code:
Spoiler :

Code:
// Declare war on teams you should be at war with
for (int iI = 0; iI < MAX_TEAMS; iI++)
{
	if ((iI != getID()) && (iI != eIndex))
	{
		if (GET_TEAM((TeamTypes)iI).isAlive())
		{
			if (GET_TEAM(eIndex).isHasMet((TeamTypes)iI))
			{
				meet((TeamTypes)iI, true);
			}

			if (isHasMet((TeamTypes)iI))
			{
				GET_TEAM(eIndex).meet((TeamTypes)iI, true);
			}

			if (GET_TEAM(eIndex).isAtWar((TeamTypes)iI))
			{
				declareWar(((TeamTypes)iI), false);
			}
			else if (isAtWar((TeamTypes)iI))
			{
				if (bCapitulated)
				{
					makePeace((TeamTypes)iI);
				}
				else
				{
					GET_TEAM(eIndex).declareWar((TeamTypes)iI, false);
				}
			}
		}
	}
}

// All our vassals now become free
for (int iI = 0; iI < MAX_TEAMS; iI++)
{
	if ((iI != getID()) && (iI != eIndex))
	{
		CvTeam& kLoopTeam = GET_TEAM((TeamTypes)iI);
		if (kLoopTeam.isAlive() && kLoopTeam.isVassal(getID()))
		{
			freeVassal((TeamTypes)iI);
		}
	}


And this is the code you want to add to fix the issue:
Code:
[B]// clear warplans for wars that are not already started
for (int iI = 0; iI < MAX_TEAMS; iI++)
{
	if (!isAtWar((TeamTypes)iI))
	{
		AI_setWarPlan((TeamTypes) iI, NO_WARPLAN);
		GET_TEAM((TeamTypes) iI).AI_setWarPlan(getID(), NO_WARPLAN);
	}
}[/B]

here is the code in place:
Spoiler :

Code:
// Declare war on teams you should be at war with
for (int iI = 0; iI < MAX_TEAMS; iI++)
{
	if ((iI != getID()) && (iI != eIndex))
	{
		if (GET_TEAM((TeamTypes)iI).isAlive())
		{
			if (GET_TEAM(eIndex).isHasMet((TeamTypes)iI))
			{
				meet((TeamTypes)iI, true);
			}

			if (isHasMet((TeamTypes)iI))
			{
				GET_TEAM(eIndex).meet((TeamTypes)iI, true);
			}

			if (GET_TEAM(eIndex).isAtWar((TeamTypes)iI))
			{
				declareWar(((TeamTypes)iI), false);
			}
			else if (isAtWar((TeamTypes)iI))
			{
				if (bCapitulated)
				{
					makePeace((TeamTypes)iI);
				}
				else
				{
					GET_TEAM(eIndex).declareWar((TeamTypes)iI, false);
				}
			}
		}
	}
}

[B]// clear warplans for wars that are not already started
for (int iI = 0; iI < MAX_TEAMS; iI++)
{
	if (!isAtWar((TeamTypes)iI))
	{
		AI_setWarPlan((TeamTypes) iI, NO_WARPLAN);
		GET_TEAM((TeamTypes) iI).AI_setWarPlan(getID(), NO_WARPLAN);
	}
}[/B]

// All our vassals now become free
for (int iI = 0; iI < MAX_TEAMS; iI++)
{
	if ((iI != getID()) && (iI != eIndex))
	{
		CvTeam& kLoopTeam = GET_TEAM((TeamTypes)iI);
		if (kLoopTeam.isAlive() && kLoopTeam.isVassal(getID()))
		{
			freeVassal((TeamTypes)iI);
		}
	}
}
 
this bug is in two places in CvTeam.cpp. It causes warplans to be set to NO_WARPLAN even though a war is active.

first place:
CvTeam::addTeam, lines 545-549:
Code:
if (GET_TEAM((TeamTypes)iI).isAlive())
{
	GET_TEAM((TeamTypes)iI).AI_setWarPlan(getID(), NO_WARPLAN);
	GET_TEAM((TeamTypes)iI).AI_setWarPlan(eTeam, NO_WARPLAN);
}

the fix, add atWar check:

Code:
if (GET_TEAM((TeamTypes)iI).isAlive())
{
	[B]if (!GET_TEAM((TeamTypes)iI).isAtWar(getID()))
	{[/B]
		GET_TEAM((TeamTypes)iI).AI_setWarPlan(getID(), NO_WARPLAN);
	[B]}

	if (!GET_TEAM((TeamTypes)iI).isAtWar(eTeam))
	{[/B]
		GET_TEAM((TeamTypes)iI).AI_setWarPlan(eTeam, NO_WARPLAN);
	[B]}[/B]
}


second place (called from addTeam):
CvTeam::shareCounters, line 677:

Code:
GET_TEAM(eTeam).AI_setWarPlan(((TeamTypes)iI), NO_WARPLAN);

To fix, add a atWar check:

Code:
[B]if (!GET_TEAM((TeamTypes)eTeam).isAtWar((TeamTypes)iI))
{[/B]
	GET_TEAM(eTeam).AI_setWarPlan(((TeamTypes)iI), NO_WARPLAN);
[B]}[/B]

Note, both of these fixes occur after setAtWar is called appropriately for both teams. Also note, the setAtWar could be changed to correctly keep track of which side declared war, rather than switching the 'declared war side' to the side with the PA in the case of the 2nd team being in a defensive war. (Just check AI_isChosenWar to decide which side declares).

-Iustus
 
this is repeated from this separate thread
I was making a MOD with new civics and I used theBuildinghappinesschange options like in the standard civic 'nationhood' where barracks get +2 :) . I noticed that this is only possible for standard buildings and not for the unique buildings. In the standard CIV IV Warlords the Ikhanda (= zulu barracks) doesn't get the +2 :) under nationhood.

In the same way, I can't give extra : to unique building, unless you code the standard and the unique building, so it appears in the civic screen.

Example :
give walls +1 :)
=> celtic dun doesn't get this.

Give walls and celtic dun +1:
=> in civic screen +1 :) for walls and dun appears for all civs.

Is this a bug?
If it is, will Firaxis do something about it?
If it isn't, how can I code this more ellegantly?

This is a real bug. I checked the SDK code, and found the error there. Currently, Nationhood does not give its bonus to Zulu Ikhanda.

There are two ways to fix this:
(1) Change the defintion so that in XML, a building class, not a building type is specified. This would be a huge change, would invalidate a lot of existing mods, and requires significant XML, Python and XML changes.
(2) Change the XML to specify Ikhanda as well as Barracks and change the SDK to display the correct thing. I will specify the code changes needed to make this fix, as it is much less disruptive (albeit harder for a mod coder to get right).

First, change Civ4CivicInfos.xml, lines 575-584 to the following, by adding the Ikhanda:
Code:
<BuildingHappinessChanges>
	<BuildingHappinessChange>
		<BuildingType>BUILDING_BARRACKS</BuildingType>
		<iHappinessChange>2</iHappinessChange>
	</BuildingHappinessChange>
	[B]<BuildingHappinessChange>
		<BuildingType>BUILDING_ZULU_IKHANDA</BuildingType>
		<iHappinessChange>2</iHappinessChange>
	</BuildingHappinessChange>[/B]
</BuildingHappinessChanges>

This will cause the game to behave properly, although it will have the following display 'bug':


and this as well:


Note, there is also the same bug in the SDK with buildings that have BuildingHappinessChanges. (There are no such buildings in the default XML, but if you add some, then the same display bug will occur). In the following picture, you can see the bug where Monuments are changed so that they give +1 happy if you build a forge (or mint). (Note, BuildingHappinessChanges for buildings are global, so building one affects every city, this makes this example not very realistic, but it would make sense if someone created civilization specific wonders...)
Spoiler example changes, this is not a fix, Civ4Buildinginfos.xml replace line 5404 with: :
Code:
<BuildingHappinessChanges>
	<BuildingHappinessChange>
		<BuildingType>BUILDING_FORGE</BuildingType>
		<iHappinessChange>1</iHappinessChange>
	</BuildingHappinessChange>
	<BuildingHappinessChange>
		<BuildingType>BUILDING_MALI_MINT</BuildingType>
		<iHappinessChange>1</iHappinessChange>
	</BuildingHappinessChange>
</BuildingHappinessChanges>



You need to SDK changes to fix the bugs:

in CvGameTextMgr:: parseCivicInfo, CvGameTextMgr.cpp, lines 3336-3343:
Spoiler old code :
Code:
//	Building Happiness
for (iI = 0; iI < GC.getNumBuildingInfos(); iI++)
{
	if (GC.getCivicInfo(eCivic).getBuildingHappinessChanges(iI) != 0)
	{
		szHelpText += NEWLINE + gDLL->getText("TXT_KEY_CIVIC_BUILDING_HAPPINESS", GC.getCivicInfo(eCivic).getBuildingHappinessChanges(iI), ((GC.getCivicInfo(eCivic).getBuildingHappinessChanges(iI) > 0) ? gDLL->getSymbolID(HAPPY_CHAR) : gDLL->getSymbolID(UNHAPPY_CHAR)), GC.getBuildingInfo((BuildingTypes)iI).getTextKeyWide());
	}
}
fix:
Code:
//	Building Happiness
for (iI = 0; iI < GC.getNumBuildingInfos(); iI++)
{
	if (GC.getCivicInfo(eCivic).getBuildingHappinessChanges(iI) != 0)
	{
		[B]// make sure this building is valid for this player (so we do not list class buildings twice)
		PlayerTypes ePlayer = bPlayerContext ? GC.getGameINLINE().getActivePlayer() : NO_PLAYER;
		BuildingClassTypes eLoopBuildingClass = (BuildingClassTypes)GC.getBuildingInfo((BuildingTypes)iI).getBuildingClassType();
		if (ePlayer == NO_PLAYER || iI == GC.getCivilizationInfo(GET_PLAYER(ePlayer).getCivilizationType()).getCivilizationBuildings(eLoopBuildingClass))
		{[/B]
			szHelpText += NEWLINE + gDLL->getText("TXT_KEY_CIVIC_BUILDING_HAPPINESS", GC.getCivicInfo(eCivic).getBuildingHappinessChanges(iI), ((GC.getCivicInfo(eCivic).getBuildingHappinessChanges(iI) > 0) ? gDLL->getSymbolID(HAPPY_CHAR) : gDLL->getSymbolID(UNHAPPY_CHAR)), GC.getBuildingInfo((BuildingTypes)iI).getTextKeyWide());
		}
	}
}

in CvGameTextMgr:: parseCivicInfo, CvGameTextMgr.cpp, lines 5197-5208:
Spoiler old code :
Code:
for (iI = 0; iI < GC.getNumBuildingInfos(); iI++)
{
	if (GC.getBuildingInfo(eBuilding).getBuildingHappinessChanges(iI) != 0)
	{
		szTempBuffer.Format(L"%s%s", NEWLINE, gDLL->getText("TXT_KEY_BUILDING_HAPPINESS_CHANGE", GC.getBuildingInfo(eBuilding).getBuildingHappinessChanges(iI),
			((GC.getBuildingInfo(eBuilding).getBuildingHappinessChanges(iI) > 0) ? gDLL->getSymbolID(HAPPY_CHAR) : gDLL->getSymbolID(UNHAPPY_CHAR))).c_str());
		CvWString szBuilding;
		szBuilding.Format(L"<link=literal>%s</link>", GC.getBuildingInfo((BuildingTypes)iI).getDescription());
		setListHelp(szBuffer, szTempBuffer, szBuilding, L", ", (GC.getBuildingInfo(eBuilding).getBuildingHappinessChanges(iI) != iLast));
		iLast = GC.getBuildingInfo(eBuilding).getBuildingHappinessChanges(iI);
	}
}
fix:
Code:
for (iI = 0; iI < GC.getNumBuildingInfos(); iI++)
{
	if (GC.getBuildingInfo(eBuilding).getBuildingHappinessChanges(iI) != 0)
	{
		[B]// make sure this building is valid for this player (so we do not list class buildings twice)
		BuildingClassTypes eLoopBuildingClass = (BuildingClassTypes)GC.getBuildingInfo((BuildingTypes)iI).getBuildingClassType();
		if (ePlayer == NO_PLAYER || iI == GC.getCivilizationInfo(GET_PLAYER(ePlayer).getCivilizationType()).getCivilizationBuildings(eLoopBuildingClass))
		{[/B]
			szTempBuffer.Format(L"%s%s", NEWLINE, gDLL->getText("TXT_KEY_BUILDING_HAPPINESS_CHANGE", GC.getBuildingInfo(eBuilding).getBuildingHappinessChanges(iI),
				((GC.getBuildingInfo(eBuilding).getBuildingHappinessChanges(iI) > 0) ? gDLL->getSymbolID(HAPPY_CHAR) : gDLL->getSymbolID(UNHAPPY_CHAR))).c_str());
			CvWString szBuilding;
			szBuilding.Format(L"<link=literal>%s</link>", GC.getBuildingInfo((BuildingTypes)iI).getDescription());
			setListHelp(szBuffer, szTempBuffer, szBuilding, L", ", (GC.getBuildingInfo(eBuilding).getBuildingHappinessChanges(iI) != iLast));
			iLast = GC.getBuildingInfo(eBuilding).getBuildingHappinessChanges(iI);
		}
	}
}

and the result is (from the perspective of the UB user):

and


-Iustus
 
Top Bottom