Report Questionable Behavior

[LunarMongoose]

Originally Posted by Fuyu

So I'm guessing you really didn't just do to create the vacuum? Because I would totally do that and screw any more complicated approach.

The problem with that is that processCivics() is a very expensive function. It performs a ton of value assignments AND loops over the whole BuildingClass list which is huge (among other loops). This will slow down the game significantly when you're calling processCivics() up to 10 times per AI_doCivics() call (to remove up to 5 existing civics then restore them once calculations are done), and you're probably calling AI_doCivics() once per AI player every turn.

The system I've set up modifies AI_getH/HWeight() directly to start with the health and happiness values for each city it's considering as if all civics were off: basically "int iCityHappy = pLoopCity->happyDifferenceWithoutCivics()". This function mostly just reads off stored values, and the ones it does compute on the fly are simple if/else things that don't loop over any long lists. It will run MUCH faster than what you were proposing.

The PROBLEM with it, which I just realized before I went to sleep last night, is that you can't update health/happiness on a per-civic-category basis as you select a civic in each category as you go... But I don't think your approach is a good idea either - it will simply be too slow, especially when you consider we're already slowing things down a bunch by making it look at all an AI's cities instead of just the first 7... Meh.

Originally Posted by Fuyu

As for the military happiness: count your units with UNITAI_CITY_DEFENCE to find out how much happiness you will gain. Or if there are more unitais that mostly stay in cities, add them too, total units is too much though, you can't count on them staying in cities.

Checking UNITAI_CITY_DEFENSE is a good idea... I was just going with isMilitaryHappiness() which is the XML value that defines whether a unit can provide Monarchy happiness (or just plain elimination of Fear For Safety effect regardless of civics). I was then gonna scale it by some number of to factor in the fact that not all units would be in their own cities at all times.

The problem with what you're suggesting Roland, is you're trying to come up with exact precise perfect values for the weight and that's not a good idea. The speed of this code is important, the stats of a civ empire change quickly as turns pass anyway, and a good estimate is all you need or want here I'm pretty sure. There are "good estimate" values all OVER both the vanilla and BBAI code... if you wanted to start turning them all into extremely-accurate numbers we'd be here discussing this all year heh.

Basically I need a formula of the form: some sort of unit count, times some sort of scale coefficient to factor in the logistical realities of managing an army, divided by some sort of city count.

Originally Posted by Fuyu

Not sure about step 7 yet. What should be the threshold?
single cathegory condition: If at least one civic value is 120%+ of the active civic's vacuum value, and sum new > sum old (non-vacuum) then revolt to new civics combination.
If that fails, check the sums' relative difference. if new * (100 + threshold) > old * 100, revolt.
threshold value = (3*5/total cathegories)% + (number of civic changes*5/total cathegories)%?
1/5: 4% (20% of average cathegory value)
2/5: 5% (12,5% per cath)
3/5: 6% (10% per cath)
This should not make the AI more likely to revolt than normally but it should make it easier to change more than one civic in one go.

Nice work Fuyu, that's exactly what I needed and didn't want to have to come up with myself lol. Doesn't matter til I decide what to do about the H/H by civic category issue though...

[LunarMongoose]

Originally Posted by Roland Johansen

Ok, thank you for the information. Averaging over the number of cities and then multiplying again with the number of cities. Right. We need to do something with those fast calculating computers.

But I do understand. It's a piece of general code outside of the specific calculation per civic. Thank you for the info.

It was actually done this way because the weight functions were only using the first 7 cities, so they had to return a density value (value per city) that would then be multiplied by the total number of cities to get total value. You couldn't do both parts of this externally b/c the calling functions don't technically know how many cities are being considered, and you don't want to do them both internally b/c there's an issue with the Republic effect where it's multiplied by num-cities-affected rather than the total number.

Originally Posted by Afforess

Instead of counting actual units why not take the smaller number of CvCityAI::AI_minDefender() and UNITAI_CITY_DEFENSE. That's likely to be more correct.

I actually added another tracking value to the player structure (at this point I figured why not heh, I've already broken savegames), that mirrors getNumMilitaryUnits() except it checks MilitaryHappiness instead, to which I would also add whether it has UNITAI_CITY_DEFENSE, so I wouldn't be counting units. I didn't know you could get a count from UNITAI_CITY_DEFENSE directly though, how does that work?

[Afforess]

GET_PLAYER(getOwnerINLINE()).AI_getNumAIUnits(UNIT AI_CITY_DEFENSE)

[LunarMongoose]

Originally Posted by Afforess

GET_PLAYER(getOwnerINLINE()).AI_getNumAIUnits(UNIT AI_CITY_DEFENSE)

Dood, that is, like, so totally better than what I did!

Gandalf-dood: I knooooow!

Mwa ha ha...

[Roland Johansen]

Originally Posted by LunarMongoose

The problem with what you're suggesting Roland, is you're trying to come up with exact precise perfect values for the weight and that's not a good idea. The speed of this code is important, ...

Basically I need a formula of the form: some sort of unit count, times some sort of scale coefficient to factor in the logistical realities of managing an army, divided by some sort of city count.

I agree speed is important. Just don't forget that some factor should be used to take into account that the happiness effect is typically used in the cities where it is most effective when you're using hereditary rule. The big cities, close to their happiness cap typically get a higher number of units. Just use an arbitrary bonus to represent this if you don't want to approximate the effect with a formula because of game speed issues.

[Afforess]

Yeah, I have a really good memory; I tend to remember all the code I read. Anyway, that's tracked all throughout the game, so it should work very well for your purposes.

[Afforess]

Originally Posted by Roland Johansen

I agree speed is important. Just don't forget that some factor should be used to take into account that the happiness effect is typically used in the cities where it is most effective when you're using hereditary rule. The big cities, close to their happiness cap typically get a higher number of units. Just use an arbitrary bonus to represent this if you don't want to approximate the effect with a formula because of game speed issues.

It wouldn't be hard to get a rough estimate, which is infinitely better than a random number.

int iLoop;
	int iNumDefenders = AI_getNumAIUnits(UNITAI_CITY_DEFENSE);
	for (CvCity* pLoopCity = firstCity(&iLoop); pLoopCity != NULL; pLoopCity = nextCity(&iLoop))
	{
		int iNewHappy = std::min(iNumDefenders / iNumCities, pLoopCity->AI_neededDefenders())
		//if iNumCities == 0, the loop will never run, so no division by zero.
		//this is a conservative estimate, change std::min to std::max to have a more liberal estimate
	}

I didn't add any values or anything, but you could get an estimate of how much happiness will get added to each city.

[Roland Johansen]

Originally Posted by Afforess

It wouldn't be hard to get a rough estimate, which is infinitely better than a random number.

int iLoop;
	int iNumDefenders = AI_getNumAIUnits(UNITAI_CITY_DEFENSE);
	for (CvCity* pLoopCity = firstCity(&iLoop); pLoopCity != NULL; pLoopCity = nextCity(&iLoop))
	{
		int iNewHappy = std::min(iNumDefenders / iNumCities, pLoopCity->AI_neededDefenders())
		//if iNumCities == 0, the loop will never run, so no division by zero.
		//this is a conservative estimate, change std::min to std::max to have a more liberal estimate
	}

I didn't add any values or anything, but you could get an estimate of how much happiness will get added to each city.

I'm not really a coder. I don't know what std::min does for instance. Likely something with a minimum, but I don't know exactly.

I just commented that the happiness effect of hereditary rule is a bit better than the average number of defenders in a city because more units are typically placed in the cities that need more happiness. I can discuss formulas, but I don't understand all lines of code easily as I've spent very little time of my life reading code. As a mathematician, I do like to think about formulas to approximate stuff.

Maybe the happiness bonus can be based on the number of defenders the AI wants to place in each city. This is also calculated somewhere to determine AI's defensive tactics. It's likely a decent approximation of how many units the AI will place in the city. Or is the AI intelligent enough to place more units in unhappy cities when using hereditary rule?

[Afforess]

Originally Posted by Roland Johansen

I'm not really a coder. I don't know what std::min does for instance. Likely something with a minimum, but I don't know exactly.

Returns the lower number of the two given numbers.

Originally Posted by Roland Johansen

Maybe the happiness bonus can be based on the number of defenders the AI wants to place in each city. This is also calculated somewhere to determine AI's defensive tactics. It's likely a decent approximation of how many units the AI will place in the city. Or is the AI intelligent enough to place more units in unhappy cities when using hereditary rule?

That's exactly what this does:

int iLoop;
	int iNumDefenders = AI_getNumAIUnits(UNITAI_CITY_DEFENSE);
	for (CvCity* pLoopCity = firstCity(&iLoop); pLoopCity != NULL; pLoopCity = nextCity(&iLoop))
	{
		int iNewHappy = std::min(iNumDefenders / iNumCities, pLoopCity->AI_neededDefenders())
		//if iNumCities == 0, the loop will never run, so no division by zero.
		//this is a conservative estimate, change std::min to std::max to have a more liberal estimate
	}

[LunarMongoose]

Originally Posted by Roland Johansen

Or is the AI intelligent enough to place more units in unhappy cities when using hereditary rule?

I was just going to ask that heh. Turns out nothing in CvCityAI.cpp checks getHappyPerMilitaryUnit(), so it is almost certainly the case that AIs simply get the Monarchy happiness they get from the same unit allocations to cities that they would've made anyway without the civic, and make no special effort to leverage the effect any further.

This could undoubtedly use some improvement as well. Gee, I sure wish JDOG!!!, were here...

... Though I can probably do it myself. Sigh. Why are SO many things broken in the vanilla code? This is getting ridiculous heh.

[LunarMongoose]

Alright, some code submissions at last heh. I've made one extremely slight improvement to CvPlayerAI::AI_doCivics() since I posted it two days ago - removing an unnecessary iBestValue assignment - so rather than update that I'm just going to repost it here:

Spoiler:

/************************************************************************************************/
/* BETTER_BTS_AI_MOD                      07/19/09                                jdog5000      */
/*                                                                                              */
/* Civic AI                                                                                     */
/************************************************************************************************/
void CvPlayerAI::AI_doCivics()
{
	CivicTypes* paeBestCivic;

	//
	// Mongoose AICivicFix BEGIN
	//
	//
	// Mongoose AICivicFix END
	//

	int iI;

	FAssertMsg(!isHuman(), "isHuman did not return false as expected");

/************************************************************************************************/
/* BETTER_BTS_AI_MOD                      07/20/09                                jdog5000      */
/*                                                                                              */
/* Barbarian AI, efficiency                                                                     */
/************************************************************************************************/
	if( isBarbarian() )
	{
		return;
	}
/************************************************************************************************/
/* BETTER_BTS_AI_MOD                       END                                                  */
/************************************************************************************************/

	if (AI_getCivicTimer() > 0)
	{
		AI_changeCivicTimer(-1);
		return;
	}

	if (!canRevolution(NULL))
	{
		return;
	}

	FAssertMsg(AI_getCivicTimer() == 0, "AI Civic timer is expected to be 0");

	paeBestCivic = new CivicTypes[GC.getNumCivicOptionInfos()];

	// Threshold to make AI hold off on civics changes, threshold is percentage to add to
	// value of current civics

	//
	// Mongoose AICivicFix BEGIN
	//
	int iThreshold;
	if ((getAnarchyTurns() == 0) || isGoldenAge())
	{
		iThreshold = 105;
	}
	else
	{
		iThreshold = 120;
	}
	//
	// Mongoose AICivicFix END
	//

	int iCurValue;
	int iBestValue;
	for (iI = 0; iI < GC.getNumCivicOptionInfos(); iI++)
	{
		paeBestCivic[iI] = AI_bestCivic((CivicOptionTypes)iI, &iBestValue);
		iCurValue = AI_civicValue( getCivics((CivicOptionTypes)iI) );

		//
		// Mongoose AICivicFix BEGIN
		//
		iCurValue = (iCurValue * iThreshold) / 100;
		if (paeBestCivic[iI] == NO_CIVIC || iBestValue < iCurValue)
		{
			paeBestCivic[iI] = getCivics((CivicOptionTypes)iI);
		}
		//
		// Mongoose AICivicFix END
		//
	}


	// XXX AI skips revolution???
	if (canRevolution(paeBestCivic))
	{
		revolution(paeBestCivic);
		AI_setCivicTimer((getMaxAnarchyTurns() == 0) ? (GC.getDefineINT("MIN_REVOLUTION_TURNS") * 2) : CIVIC_CHANGE_DELAY);
	}

	SAFE_DELETE_ARRAY(paeBestCivic);
}
/************************************************************************************************/
/* BETTER_BTS_AI_MOD                       END                                                  */
/************************************************************************************************/

Next, the thing I just added to the bottom of CvCityAI::AI_neededDefenders():

Spoiler:

		if( isCapital() && GET_PLAYER(getOwnerINLINE()).AI_isDoVictoryStrategy(AI_VICTORY_SPACE4) )
		{
			iDefenders += 6;
		}
	}
	
	iDefenders = std::max(iDefenders, AI_minDefenders());

	//
	// Mongoose AICivicFix BEGIN
	//
	int iHappyPerMilitaryUnit = GET_PLAYER(getOwnerINLINE()).getHappyPerMilitaryUnit();
	if (iHappyPerMilitaryUnit > 0)
	{
		int iHappinessNeeded = std::max(0, (unhappyLevel(1) - happyLevel()) + 1);
		int iMilitaryUnitsNeeded = iHappinessNeeded / iHappyPerMilitaryUnit;
		if (iHappinessNeeded % iHappyPerMilitaryUnit > 0)
		{
			iMilitaryUnitsNeeded++;
		}
		iDefenders = std::max(iDefenders, getMilitaryHappinessUnits() + iMilitaryUnitsNeeded);
	}
	//
	// Mongoose AICivicFix END
	//

	return iDefenders;
}
/************************************************************************************************/
/* BETTER_BTS_AI_MOD                       END                                                  */
/************************************************************************************************/

That should make the AIs add more units to cities that currently need happiness if they're operating the Monarchy civic. Now then, moving back to basic functionality updates to the PlayerAI stuff that will have to do for now til we can get a full system working b/c I need to post an update to my mod and this is taking forever lol:

Spoiler:

//This returns a positive number equal approximately to the sum
//of the percentage values of each unit (there is no need to scale the output by iHappy)
//100 * iHappy means a high value.
int CvPlayerAI::AI_getHappinessWeight(int iHappy, int iExtraPop) const
{
	int iWorstHappy = 0;
	int iBestHappy = 0;
	int iTotalUnhappy = 0;
	int iTotalHappy = 0;
	int iLoop;
	CvCity* pLoopCity;
	int iCount = 0;

	if (0 == iHappy)
	{
		iHappy = 1;
	}
	int iValue = 0;
	for (pLoopCity = firstCity(&iLoop); pLoopCity != NULL; pLoopCity = nextCity(&iLoop))
	{
		int iCityHappy = pLoopCity->happyLevel() - pLoopCity->unhappyLevel(iExtraPop);
		
		iCityHappy -= std::max(0, pLoopCity->getCommerceHappiness());

		//
		// Mongoose AICivicFix BEGIN
		//
		// -- thanks to Fuyu for the min(5) requirements
		//
		//int iCityHappy = pLoopCity->happyDifferenceWithoutCivics(iExtraPop);
		int iHappyNow  = std::min(5, iCityHappy);
		int iHappyThen = std::min(5, iCityHappy + iHappy);
		//
		// Mongoose AICivicFix END
		//

		//Integration
		int iTempValue = (((100 * iHappyThen - 10 * iHappyThen * iHappyThen)) - (100 * iHappyNow - 10 * iHappyNow * iHappyNow));
		if (iHappy > 0)
		{
			//
			// Mongoose AICivicFix BEGIN
			//
			// -- thanks to Fuyu for persuading me to weight this based on population
			//
			iValue += std::max(0, iTempValue) * (pLoopCity->getPopulation() + 5);
			//
			// Mongoose AICivicFix END
			//
		}
		else
		{
/************************************************************************************************/
/* UNOFFICIAL_PATCH                       10/21/09                                jdog5000      */
/*                                                                                              */
/* Bugfix                                                                                       */
/************************************************************************************************/
/* orginal bts code
			iValue += std::max(0, -iTempValue);

			// Negative happy changes should produce a negative value, not the same value as positive
			iValue += std::min(0, iTempValue);
*/
			//
			// Mongoose AICivicFix BEGIN
			//
			// -- thanks to Fuyu for persuading me to weight this based on population
			//
			iValue += std::min(0, iTempValue) * (pLoopCity->getPopulation() + 5);
			//
			// Mongoose AICivicFix END
			//
/************************************************************************************************/
/* UNOFFICIAL_PATCH                        END                                                  */
/************************************************************************************************/
		}

		//
		// Mongoose AICivicFix BEGIN
		//
		iCount += pLoopCity->getPopulation() + 5;
		//
		// Mongoose AICivicFix END
		//
	}

	return (0 == iCount) ? 50 * iHappy : iValue / iCount;
}

int CvPlayerAI::AI_getHealthWeight(int iHealth, int iExtraPop) const
{
	int iWorstHealth = 0;
	int iBestHealth = 0;
	int iTotalUnhappy = 0;
	int iTotalHealth = 0;
	int iLoop;
	CvCity* pLoopCity;
	int iCount = 0;
	
	if (0 == iHealth)
	{
		iHealth = 1;
	}
	int iValue = 0;
	for (pLoopCity = firstCity(&iLoop); pLoopCity != NULL; pLoopCity = nextCity(&iLoop))
	{
		int iCityHealth = pLoopCity->goodHealth() - pLoopCity->badHealth(false, iExtraPop);

		//
		// Mongoose AICivicFix BEGIN
		//
		// -- thanks to Fuyu for the min(5) requirements
		//
		//int iCityHealth = pLoopCity->healthDifferenceWithoutCivics(iExtraPop);
		int iHealthNow  = std::min(5, iCityHealth);
		int iHealthThen = std::min(5, iCityHealth + iHealth);
		//
		// Mongoose AICivicFix END
		//

		//Integration
		int iTempValue = (((100 * iHealthThen - 6 * iHealthThen * iHealthThen)) - (100 * iHealthNow - 6 * iHealthNow * iHealthNow));
		if (iHealth > 0)
		{
			//
			// Mongoose AICivicFix BEGIN
			//
			// -- thanks to Fuyu for persuading me to weight this based on population
			//
			iValue += std::max(0, iTempValue) * (pLoopCity->getPopulation() + 5);
			//
			// Mongoose AICivicFix END
			//
		}
		else
		{
/************************************************************************************************/
/* UNOFFICIAL_PATCH                       10/21/09                                jdog5000      */
/*                                                                                              */
/* Bugfix                                                                                       */
/************************************************************************************************/
/* orginal bts code
			iValue += std::max(0, -iTempValue);

			// Negative health changes should produce a negative value, not the same value as positive
			iValue += std::min(0, iTempValue);
*/
			//
			// Mongoose AICivicFix BEGIN
			//
			// -- thanks to Fuyu for persuading me to weight this based on population
			//
			iValue += std::min(0, iTempValue) * (pLoopCity->getPopulation() + 5);
			//
			// Mongoose AICivicFix END
			//
/************************************************************************************************/
/* UNOFFICIAL_PATCH                        END                                                  */
/************************************************************************************************/
		}

		//
		// Mongoose AICivicFix BEGIN
		//
		iCount += pLoopCity->getPopulation() + 5;
		//
		// Mongoose AICivicFix END
		//
	}
	
/************************************************************************************************/
/* UNOFFICIAL_PATCH                       10/21/09                                jdog5000      */
/*                                                                                              */
/* Bugfix                                                                                       */
/************************************************************************************************/
/* orginal bts code
	return (0 == iCount) ? 50 : iValue / iCount;
*/
	// Mirror happiness valuation code
	return (0 == iCount) ? 50*iHealth : iValue / iCount;
/************************************************************************************************/
/* UNOFFICIAL_PATCH                        END                                                  */
/************************************************************************************************/
}

That makes the weight functions consider all cities, weight them by population (without an offset - changed my mind again and completely agree with Fuyu lol), and use Fuyu's min(5) bound checks for the integral math so very high levels of happiness don't mess it up.

Then we have in CvPlayerAI::AI_civicValue():

Spoiler:

		int iElapsedTurns = GC.getGame().getElapsedGameTurns();

		if (iElapsedTurns > iTargetGameTurn)
		{
			iValue += (std::min(iTargetGameTurn, iElapsedTurns - iTargetGameTurn) * (iNumOtherCities * kCivic.getCivicPercentAnger())) / (15 * iTargetGameTurn);
		}
	}

	//
	// Mongoose AICivicFix BEGIN
	//
	iTempValue = kCivic.getExtraHealth();
	if (iTempValue != 0)
	//
	// Mongoose AICivicFix END
	//
	{
/************************************************************************************************/
/* UNOFFICIAL_PATCH                       10/21/09                                jdog5000      */
/*                                                                                              */
/* Bugfix                                                                                       */
/************************************************************************************************/
/* orginal bts code
		iValue += (getNumCities() * 6 * AI_getHealthWeight(isCivic(eCivic) ? -kCivic.getExtraHealth() : kCivic.getExtraHealth(), 1)) / 100;
		iValue += (getNumCities() * 6 * AI_getHealthWeight(kCivic.getExtraHealth(), 1)) / 100;
*/
		//
		// Mongoose AICivicFix BEGIN
		//
		// -- thanks to Fuyu for this code, which is an imperfect kludge/hack but still an improvement
		//
		iValue += (getNumCities() * 6 * (isCivic(eCivic)? -AI_getHealthWeight(-iTempValue, 1) : AI_getHealthWeight(iTempValue, 1))) / 100;
		//
		// Mongoose AICivicFix END
		//
/************************************************************************************************/
/* UNOFFICIAL_PATCH                        END                                                  */
/************************************************************************************************/
	}

	//
	// Mongoose AICivicFix BEGIN
	//
	// -- thanks to Afforess for mentioning I could get a UNITAI_CITY_DEFENSE count directly
	//
	iTempValue = (kCivic.getHappyPerMilitaryUnit() * AI_getNumAIUnits(UNITAI_CITY_DEFENSE)) / std::max(1, getNumCities());
	//
	// Mongoose AICivicFix END
	//

	if (iTempValue != 0)
	{
/************************************************************************************************/
/* UNOFFICIAL_PATCH                       10/21/09                                jdog5000      */
/*                                                                                              */
/* Bugfix                                                                                       */
/************************************************************************************************/
/* orginal bts code
		iValue += (getNumCities() * 9 * AI_getHappinessWeight(isCivic(eCivic) ? -iTempValue : iTempValue, 1)) / 100;
		iValue += (getNumCities() * 9 * AI_getHappinessWeight(iTempValue, 1)) / 100;
*/
		//
		// Mongoose AICivicFix BEGIN
		//
		// -- thanks to Fuyu for this code, which is an imperfect kludge/hack but still an improvement
		//
		iValue += (getNumCities() * 9 * (isCivic(eCivic)? -AI_getHappinessWeight(-iTempValue, 1) : AI_getHappinessWeight(iTempValue, 1))) / 100;
		//
		// Mongoose AICivicFix END
		//
/************************************************************************************************/
/* UNOFFICIAL_PATCH                        END                                                  */
/************************************************************************************************/
	}
		
	iTempValue = kCivic.getLargestCityHappiness();
	if (iTempValue != 0)
	{
/************************************************************************************************/
/* UNOFFICIAL_PATCH                       10/21/09                                jdog5000      */
/*                                                                                              */
/* Bugfix                                                                                       */
/************************************************************************************************/
/* orginal bts code
		iValue += (12 * std::min(getNumCities(), GC.getWorldInfo(GC.getMapINLINE().getWorldSize()).getTargetNumCities()) * AI_getHappinessWeight(isCivic(eCivic) ? -iTempValue : iTempValue, 1)) / 100;
		iValue += (12 * std::min(getNumCities(), GC.getWorldInfo(GC.getMapINLINE().getWorldSize()).getTargetNumCities()) * AI_getHappinessWeight(iTempValue, 1)) / 100;
*/
		//
		// Mongoose AICivicFix BEGIN
		//
		// -- thanks to Fuyu for this code, which is an imperfect kludge/hack but still an improvement
		//
		iValue += (12 * std::min(getNumCities(), GC.getWorldInfo(GC.getMapINLINE().getWorldSize()).getTargetNumCities()) * (isCivic(eCivic)? -AI_getHappinessWeight(-iTempValue, 1) : AI_getHappinessWeight(iTempValue, 1))) / 100;
		//
		// Mongoose AICivicFix END
		//
/************************************************************************************************/
/* UNOFFICIAL_PATCH                        END                                                  */
/************************************************************************************************/
	}
	
	if (kCivic.getWarWearinessModifier() != 0)
	{
		int iAngerPercent = getWarWearinessPercentAnger();
		int iPopulation = 3 + (getTotalPopulation() / std::max(1, getNumCities()));

		int iTempValue = (-kCivic.getWarWearinessModifier() * iAngerPercent * iPopulation) / (GC.getPERCENT_ANGER_DIVISOR() * 100);
		if (iTempValue != 0)
		{
/************************************************************************************************/
/* UNOFFICIAL_PATCH                       10/21/09                                jdog5000      */
/*                                                                                              */
/* Bugfix                                                                                       */
/************************************************************************************************/
/* orginal bts code
			iValue += (11 * getNumCities() * AI_getHappinessWeight(isCivic(eCivic) ? -iTempValue : iTempValue, 1)) / 100;
			iValue += (11 * getNumCities() * AI_getHappinessWeight(iTempValue, 1)) / 100;
*/
			//
			// Mongoose AICivicFix BEGIN
			//
			// -- thanks to Fuyu for this code, which is an imperfect kludge/hack but still an improvement
			//
			iValue += (11 * getNumCities() * (isCivic(eCivic)? -AI_getHappinessWeight(-iTempValue, 1) : AI_getHappinessWeight(iTempValue, 1))) / 100;
			//
			// Mongoose AICivicFix END
			//
/************************************************************************************************/
/* UNOFFICIAL_PATCH                        END                                                  */
/************************************************************************************************/
		}
	}
	
	iValue += (kCivic.getNonStateReligionHappiness() * (iTotalReligonCount - iHighestReligionCount) * 5);

	if (kCivic.isStateReligion())
	{

This uses Fuyu's kludge in all 4 places (he just posted one originally ), and switches the Monarchy effect's value from assuming a flat 3 units will be present on average to using UNITAI_CITY_DEFENSE over getNumCities(). I feel this is still a good approximation since all existing defense units will still be accounted for, just not necessarily in the right places... it makes a near-optimal estimation when most of the player's cities are near or at their happiness limits... And I'd really prefer to avoid adding a city loop here for speed if possible.

[Afforess]

Watch out, your getNumCities call isn't checked for nonzero values. It's going to cause CTD's.

[LunarMongoose]

Originally Posted by Afforess

Watch out, your getNumCities call isn't checked for nonzero values. It's going to cause CTD's.

No, no it's not, and umm... yes, yes it is. *ahem* Fixed.

Anyway, there's a self-proclaimed semi-official update that makes a lot of improvements without breaking savegames, and shouldn't have any bugs left in it. I'm going to stop for a little bit to work on my mod's patch and info threads, but we do need to finish a better health/happiness evaluation system here.

Btw Afforess, the problem with going through and adding up each city's AI_neededDefenders(), aside from the fact that AI_neededDefenders() now depends on the civic you'd be considering so once you adopt it it might return a different value which would be messy... is that AI_neededDefenders() is just a target value that the AI is trying to reach, not the number of troops currently/actually available, and the civics will be re-evaluated every turn anyway I assume. Besides, you may be losing a lot of units to enemies you're fighting at the moment, which would make the target values farther off from being reached, but the civic choice should be based on what you can do with what you currently have.

Which is the same reason I ultimately went with no offset to the population weighting - it doesn't matter if the happiness will be really valuable to the small cities later once they've (quickly) grown some; it just matters if it's valuable to them now, b/c the civics will be re-evaluated later anyway - although now I'm wondering if a smaller offset like 5 wouldn't still be a good idea heh.

Edit - And in the interests of erring on the side of caution... and the fact that JDog uses offsets a lot... and the fact that I'm a little nervous completely marginalizing the effect of size 1-2 cities when I don't have to... I went ahead and put an offset of 5 in there, which is still a lot smaller than the 10 I was originally planning to use heh. Anyway, sorry for the confusion; I'm done updating the update now.

[Afforess]

Originally Posted by LunarMongoose

Btw Afforess, the problem with going through and adding up each city's AI_neededDefenders(), aside from the fact that AI_neededDefenders() now depends on the civic you'd be considering so once you adopt it it might return a different value which would be messy... is that AI_neededDefenders() is just a target value that the AI is trying to reach, not the number of troops currently/actually available

Yes, I know that, which is why I suggested using a combination of the number of the UNITAI_CITY_DEFENSE AND AI_neededDefenders!

[LunarMongoose]

Originally Posted by Afforess

Yes, I know that, which is why I suggested using a combination of the number of the UNITAI_CITY_DEFENSE AND AI_neededDefenders!

Well right, except that 1) at that point it's not worth the cost of having a city loop in the code, 2) if you're using min() then you might occasionally have a value lower than the UNITAI_CITY_DEFENSE level and I don't see why you'd ever want to base it off less than the number of troops you currently have, and 3) if you're using max() then you're back to my original point where the target troop level values don't really matter for this since you don't actually have them yet and the civics will be re-evaluated on subsequent turns when they might be part of your actual troops anyway.

[Afforess]

Originally Posted by LunarMongoose

Well right, except that 1) at that point it's not worth the cost of having a city loop in the code, 2) if you're using min() then you might occasionally have a value lower than the UNITAI_CITY_DEFENSE level and I don't see why you'd ever want to base it off less than the number of troops you currently have, and 3) if you're using max() then you're back to my original point where the target troop level values don't really matter for this since you don't actually have them yet and the civics will be re-evaluated on subsequent turns when they might be part of your actual troops anyway.

1.) You can loop over 200 times in less than .01 seconds. Heck, checking all the :canConstruct calls, a fairly complex function, for all the buildings takes less than 10ms. You could even loop over EVERY UNIT you own in less than 10ms. It's recursive functions that are problematic.

2.) If you are at war, some city defenders may be moving in-between cities, in which case, a conservative estimate has more false negatives than false positives, and false negatives, IMHO are better than false positives.

3.) See 2.)

[LunarMongoose]

Originally Posted by Afforess

1.) You can loop over 200 times in less than .01 seconds. Heck, checking all the :canConstruct calls, a fairly complex function, for all the buildings takes less than 10ms. You could even loop over EVERY UNIT you own in less than 10ms. It's recursive functions that are problematic.

Well then maybe Fuyu's full processCivics() approach is the way to go for the whole thing, it's just that that much wasted/redundant processing makes me cringe... Hmm, maybe there's a way to use a reduced version of processCivics()...

Originally Posted by Afforess

2.) If you are at war, some city defenders may be moving in-between cities, in which case, a conservative estimate has more false negatives than false positives, and false negatives, IMHO are better than false positives.

Not necessarily, b/c false negatives can cause the civic to not be adopted in the first place by a lower-than-actual value-score for it, but false-positives can be corrected once it's adopted by making more troops or, more importantly, deploying existing troops more intelligently now that my code will actually make them do that. (This was argued quite a bit above, that we were underestimating the value of this civic ability b/c of redistribution to boost specific cities vs taking an empire-wide average effect.)

[Afforess]

Originally Posted by LunarMongoose

Well then maybe Fuyu's full processCivics() approach is the way to go for the whole thing, it's just that that much wasted/redundant processing makes me cringe... Hmm, maybe there's a way to use a reduced version of processCivics()...

Why? CvPlayer::processCivics is a fast function call. I'd bet you a $5 that it takes less time to call for all the civics, twice, than it takes for the game to run all the code to open a city screen.

Originally Posted by LunarMongoose

Not necessarily, b/c false negatives can cause the civic to not be adopted in the first place by a lower-than-actual value-score for it, but false-positives can be corrected once it's adopted by making more troops or, more importantly, deploying existing troops more intelligently now that my code will actually make them do that. (This was argued quite a bit above, that we were underestimating the value of this civic ability b/c of redistribution to boost specific cities vs taking an empire-wide average effect.)

IDK, I'm not convinced, but I do see your point.

[Fuyu]

Originally Posted by Afforess

CvPlayer::processCivics is a fast function call. I'd bet you a $5 that it takes less time to call for all the civics, twice, than it takes for the game to run all the code to open a city screen.

Unfortunately I'd call processCivics() about (4*civic cathogories*players) times, so I think it is ok to be a bit concerned about speed. But the conclusion I draw is: I should just make that function faster by stealing your CvBuildingInfo::isAnySpecialistYieldChange idea.
With some CvCivicInfo::isAny(ImprovementYieldChange |BuildingHappinessChange |BuildingHealthChange |FeatureHappinessChange), and isCanChangeValueOfOtherCivics and isConstantValue, I think I can make doCivics as a whole fast enough. The real slowdown, that would be the loop over all cities for the happy/health weights instead of only the first 7 cities, but there is no way to optimize that.

Creating the vacuum and working with it is still better than the "imperfect kludge/hack" even if it might look expensive in comparison. The first thought I had there was to add a new argument "bLimited = false" to CvPlayer::processCivics, and if (bLimited) then let the function skip everything that doesn't affect the values of other civics, most importantly the parts that would cause triggering updateTradeRoutes() because that would really be expensive in advanced stages of a game and there woud be no gain at all.

@LM: I don't know what your CvCityAI::AI_neededDefenders() does but as far as I can see, in BBAI it is not where you should try to handle military happiness, it will cause the city to build UNITAI_ATTACK units that won't stay in the city, it might call units from other continents but they won't be fortifying in the city either. If you use AI_minDefenders() then the city will build city defenders before it tries to build happiness buildings but at least the effects are what you expect: building and calling for more city defenders. Something like AI_minDefenders(bool bIncludeUnitsForMilitaryHappiness = false) could work without having that sideeffect if you make sure to call the function with true whereever it makes sense.
CityAI:

	//20 means 5g or ~2 happiness...
	if (AI_chooseBuilding(iEconomyFlags, 15, 20))
	{
		if( gCityLogLevel >= 2 ) logBBAI("      City %S uses choose iEconomyFlags 2", getName().GetCString());
		return;
	}

	//minimal defense - military happiness.
	if (iPlotCityDefenderCount < (AI_minDefenders(true) + iPlotSettlerCount))
	{
		if (AI_chooseUnit(UNITAI_CITY_DEFENSE))
		{
			if( gCityLogLevel >= 2 ) logBBAI("      City %S uses choose min defender for military happiness", getName().GetCString());
			return;
		}
	}

	if (!bLandWar)
	{
		if (AI_chooseBuilding(iEconomyFlags, 40, 8))
		{
			if( gCityLogLevel >= 2 ) logBBAI("      City %S uses choose iEconomyFlags 3", getName().GetCString());
			return;
		}

Some limitations to civic modding I came across when looking at the current AI_civicValue function:
Civic bonus groups

• corporations: don't mix inside a single civic, and keep them in a single cathegory: iCorporationMaintenanceModifier, bNoCorporations, bNoForeignCorporations
• trade routes: don't mix, and keep them in a single cathegory: iTradeRoutes, bNoForeignTrade. TradeYieldModifiers is constant and doesn't care for trade route numbers or restrictions (bad?).
• happiness: can be in different cathegories, but mixing iHappyPerMilitaryUnit, iLargestCityHappiness and iWarWearinessModifier will cause overvaluation of the civic because all of those 3 use AI_getHappinessWeight. Values for other sources of happiness (building, (non)statereligion) are constant (bad!) and can be mixed.
• health: iExtraHealth is the only property to use AI_getHealthWeight, all mixing possible.

religion is interesting too, if a civic has any bonus tied to state religion is needs bStateReligion true. Which is perfectly fine as long as everything religion is in a single cathegory. If that is no longer the case, you could end up with free religion while running a state religion, or a civic could end up not getting the value it deserves. Which is what's happening to Theocracy's StateReligionFreeExperience bonus in MongooseMod btw.
fix:

	if (kCivic.isStateReligion() || isStateReligion())
	{
		if (iHighestReligionCount > 0)
		{
			if (kCivic.isStateReligion())
			{
				iValue += iHighestReligionCount;
			}

			iValue += ((kCivic.isNoNonStateReligionSpread()) ? ((getNumCities() - iHighestReligionCount) * 2) : 0);
			iValue += (kCivic.getStateReligionHappiness() * iHighestReligionCount * 4);
			iValue += ((kCivic.getStateReligionGreatPeopleRateModifier() * iHighestReligionCount) / 20);
			iValue += (kCivic.getStateReligionGreatPeopleRateModifier() / 4);
			iValue += ((kCivic.getStateReligionUnitProductionModifier() * iHighestReligionCount) / 4);
			iValue += ((kCivic.getStateReligionBuildingProductionModifier() * iHighestReligionCount) / 3);
			iValue += (kCivic.getStateReligionFreeExperience() * iHighestReligionCount * ((bWarPlan) ? 6 : 2));

I don't plan to do anything to corporations and trade routes but I want all happy and health boni to be evaluated with AI_getH/HWeight, and I'll somehow have to do that in one go, so that mixing would not lead to wrong numbers.
So much fun waiting for me

[Afforess]

I'm highly interested in the results though. Please do post them.