Report Questionable Behavior

Alright, here's what I'm going with for now. Note that everything I mentioned above is a result of BBAI code so these issues would not be present in vanilla.

I cleaned up the "iCurValue += (iCurValue * iThreshold) / 100;" line, which for small values like 7 or 20 could have significant rounding errors. (I also changed the declaration/assignment + if/increment iThreshold code to an if-based double assignment, which may run ever-so-slightly faster depending on the compiler optimizations and other low-level details... or it may not have any effect at all heh. But at least it's easier to read! :p)

Lastly, I'm hesitant to set the base threshold to zero since that's what vanilla has and JDog probably added it to prevent ping-ponging back and forth between two equivalent civics. I don't see the harm in frequent tweaking of civics for a Spiritual civ however, so I'm using a slightly smaller number.

Spoiler :
Code:
/************************************************************************************************/
/* 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;
		//
		// Mongoose AICivicFix END
		//

		if ( paeBestCivic[iI] == NO_CIVIC || iBestValue < iCurValue )
		{
			paeBestCivic[iI] = getCivics((CivicOptionTypes)iI);
			iBestValue = iCurValue;
		}

		//
		// Mongoose AICivicFix BEGIN
		//
		//
		// 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                                                  */
/************************************************************************************************/

This still doesn't solve the problem with unrest specifically introduced by switching off a +happiness civic, or irreversible city shrinkage from switching off a +health civic. It will consider the value of extra health and happiness, but not realize harm in giving them up if they're being fully utilized.

One could argue that that goes with the territory, ie a +happiness civic is useless unless the extra happiness is actually being used, and if it's being used then removing it would always trigger anger/unrest, so that would be assumed and built into the +happiness effect... So really I guess the optimal solution would be to reduce the value of adopting such a civic if an AI player doesn't have a lot of cities near or at their happiness limits, rather than adding a penalty for switching off the civic if doing so would cause unrest. But there should probably be one or the other, and maybe both.

Edit - Which is exactly what CvPlayerAI::AI_getHappinessWeight() is doing. :) It's using some complicated logic and arithmetic and I don't have time (or desire lol) to analyze it right now, but it looks pretty good at first glance. If there are any further improvements to be made however, this is probably where they will come from.
 
Damn it, you too Afforess? >_<
Civic value should be analyzed in a vacuum so to speak, and have nothing to do with whether they are currently active or not.
That is the problem. There is no such vacuum.
The problem lies with AI_getHappinessWeight():

Let's say, Civic A gives additional happiness (and for simplicity purposes: no other civic in this cathegory does). Our first 7 cities are not so happy.

scenario 1: Civic A is not selected. In this scenario, because our cities aren't all that happy, AI_getHappinessWeight(iTempValue, 1) will correctly return a high value: "switch to civic A right now!"

scenario 2: Civic A is already selected. In this scenario, because our cities are quite happy because the civic we are already running is providing such a nice bonus, AI_getHappinessWeight(iTempValue, 1) will return a relatively low, and thus wrong, value, basically saying: "we don't really need this civic"
However, -AI_getHappinessWeight(-iTempValue, 1) (note the two minuses there) would return the same value as AI_getHappinessWeight(iTempValue, 1) for scenario 1 where the civic is not selected.

When there is more than one civic in a single cathegory that provides happiness, and one of them is active, this solution will not give the correct value to the other happiness civic. This is where a vacuum would be really nice:
  1. Remove all civic effects
  2. Calculate all civic values for all civics in all cathegories
  3. Find the cathegory where the absolute difference between the best civic and the second best is greatest, and select said best civic. Store the values of best civic and active civic.
  4. Recalculate civic values for the rest of the cathegories, accounting for the effects of the civic selected.
  5. Repeat the find cathegory -> select civic -> recalculate cycle until all cathegories have a selection.
  6. Calculate sum of all newly selected civic values, compare to sum of active civic values.
  7. Change all civics to the new selections if new minus old is big enough.


edit:
This still doesn't solve the problem with unrest specifically introduced by switching off a +happiness civic, or irreversible city shrinkage from switching off a +health civic. It will consider the value of extra health and happiness, but not realize harm in giving them up if they're being fully utilized
Exactly.
 
Lol read my edit dood, which I was apparently too slow in writing. :)

Grr, you're gonna make me study that function in detail now aren't you...
 
Didn't install it on this comp yet..

anyway


f(x) = 20 * (5 - x)
F(x) = 20 * (5x - x²/2) = 100x - 10x²
The definite integral of f(x) between iHappyNow and iHappyThen is (100 * iHappyThen - 10 * iHappyThen * iHappyThen) - (100 * iHappyNow - 10 * iHappyNow * iHappyNow) - which is what is used in that function to calculate the happyWeight for a single city.
iHappyNow is the current total happiness of the city (happy-unhappy), iHappyThen is iHappyNow plus the first argument of the function.

Obvious oversight, if you look at f(x):
Code:
[COLOR="Gray"]		int iHappyNow = iCityHappy;
		int iHappyThen = iCityHappy + iHappy;[/COLOR]
		[COLOR="Green"]//these lines are missing:[/COLOR]
[B]		iHappyNow = std::min(5, iHappyNow)
		iHappyThen = std::min(5, iHappyThen)[/B]
otherwise positive happiness change can give negative value (and the other way round) if the city is really really happy right now.

The return value is just the average of the first 7 player cities.

edit: now with picture ;)
 

Attachments

  • AI_getHappinessWeight2.png
    AI_getHappinessWeight2.png
    13.5 KB · Views: 329
Well I did preface the whole thing by saying I was worried I'd be proven at least partially wrong again. :p

Btw, nice kludge/hack solution to the problem a few posts back, even though it doesn't cover all possible scenarios. And sorry for not figuring out what you were doing with that ahead of time... but at least I'm finally up to speed now. :) Meh... yeah that was definitely figure-out-able. I'm gonna go hibernate in a nice dark cave now...

*ahem* The code I posted is still an improvement though. But yeah having thought it fully through now, I agree the procedure you outlined is the only way to REALLY solve this. Soooo... I'm currently attempting to write the code to do exactly that. Wish me luck. ;)

Edit - And nice math work heh... I stand by my characterization of calculus as "complicated" when I haven't seen an integral in 12-15 years, but hey at least I knew enough to recognize what'd probably be involved and didn't try to claim I understood the function right away. ;) Thank you for doing that, especially since it yielded an additional code improvement.

Edit 2 - The average of the player's first 7 cities? Grr, stupid... Maybe if you picked 7 cities at random from his whole empire that'd be okay as a statistical average, but the FIRST 7 are likely to be the most developed and, more importantly, have the highest population, at least for a while before populations reach their limits and stabilize around the world. I assume you agree that, despite the speed cost, it'd be better to check all of them?
 
I agree the procedure you outlined is the only way to REALLY solve this. Soooo... I'm currently attempting to write the code to do exactly that. Wish me luck. ;)
Mind sharing it when complete? Removing civic effects, temporary adding effects of best civics, and restoring the old civic effects are the parts I have doubts I could figure out all that quickly. Maybe I could actually switch civics during those processes?

Thank you for doing that, especially since it yielded an additional code improvement.
If you mean the 2 missing lines: Beware of my lack of concentration, I was using std::max when it should have been min.

Edit 2 - The average of the player's first 7 cities? Grr, stupid... Maybe if you picked 7 cities at random from his whole empire that'd be okay as a statistical average, but the FIRST 7 are likely to be the most developed and, more importantly, have the highest population, at least for a while before populations reach their limits and stabilize around the world. I assume you agree that, despite the speed cost, it'd be better to check all of them?
I was thinking it is actually smart to only factor in the cities that matter most but it should probably be done in a different way: Use all cities but weight every city's happyWeight by its population. So if you have one city with 20 pop and one with 1 pop, the first city will profit from happy and the other won't, and the return value would be (20*first city happyWeight + 1*second city happyWeight / totalpop)
 
Mind sharing it when complete? Removing civic effects, temporary adding effects of best civics, and restoring the old civic effects are the parts I have doubts I could figure out all that quickly. Maybe I could actually switch civics during those processes?)

My policy currently is to share fixes and not share mods, so yeah I would. :) As to actually switching civics as part of the process... No not really, since if nothing else you would trigger the minimum turn delay lockout, and there would probably be other consequences (eg with Favorite Civics in diplomacy) that, even if reversable, would still cause more processing overhead than necessary.

If you mean the 2 missing lines: Beware of my lack of concentration, I was using std::max when it should have been min.

Hey, I did that in this forum a few weeks ago too! *high-five*

I was thinking it is actually smart to only factor in the cities that matter most but it should probably be done in a different way: Use all cities but weight every city's happyWeight by its population. So if you have one city with 20 pop and one with 1 pop, the first city will profit from happy and the other won't, and the return value would be (20*first city happyWeight + 1*second city happyWeight / totalpop)

My concern with that would be, that I thought population was already being more-or-less taken into account by the existing AI_getHappinessWeight() code: if a city is small it will automatically be nowhere near its happiness limit. I suppose that's not always true though, since maybe it's not connected to the rest of your empire yet and not getting the 10 luxury types the rest of your cities have, or maybe it's being blockaded currently. However these may be longterm problems for that player, in which case you actually might want them factored in when considering a +happiness civic that would relieve the pressure in such places. I personally would probably consider a size 20 city being able to slowly grow to 22 no more valuable than a size 2 city suddenly being able to quickly grow to 4 from a civic, so my initial inclination would be to stick with even weights. I could easily be wrong on this though.
 
My concern with that would be, that I thought population was already being more-or-less taken into account by the existing AI_getHappinessWeight() code: if a city is small it will automatically be nowhere near its happiness limit.
Yes, and therefore the value for this city will be low (or zero, or even below if you don't cap the happyNow/Then values to 5), and that will greatly reduce the average, which is what is returned.
Simple example: 1 big city near happy cap -> AI_getHappinessWeight() returns high values. Player then founds a second city which is very far for happy cap because it is so small (1pop) -> AI_getHappinessWeight() returns the average of the values for both cities, which is one high value, and one very low value or even zero. Thus, AI_getHappinessWeight() return value got (almost) halved by founding a new city. This doesn't look wrong to you?

To me it barely matters if a happy bonus provides no profit for newly founded or small dessert cities, it matters a lot more if the important cities profit. Of course, size might not always be the best measure for importance but it's good enough for me, so weighting by population is what I'd go for.

I personally would probably consider a size 20 city being able to slowly grow to 22 no more valuable than a size 2 city suddenly being able to quickly grow to 4 from a civic, so my initial inclination would be to stick with even weights.
Well the assumption is 1) that the big city might grow slower but also has a lot of buildings that make the additional pop more valueable, 2) that the small cities will rarely have happiness prolems anyway and if they do, in general doing something about it is easier for small cities than it is for bigger ones.
I certainly prefer a happy bonus for a big city over the same happy bonus for a small one, even if the small one has reached its happy cap too.
 
My policy currently is to share fixes and not share mods, so yeah I would. :)

Are you afraid someone's gonna violate your "copyright"? :confused:

Sorry for the off-topic question, but I really don't understand what goes through the minds of for instance you and Cybah.
 
I can't comprehend it either, but unless you want to unleash a most likely pointsless discussion (for which you should open a new thread in the general Creation & Customization area) you should probably simply accept that different people have different ways of thinking. Not everything that makes sense to you must make sense to someone else, and vice-versa.
If I had ever created something I was proud of I might want to hide it too, it just never happened so I wouldn't know ;)

BASE, MongooseMod and this CCV thing are the only closed-source mods I can think of though, so as far as I can see it's a relatively limited phenomenon. :)
[offtopic]
 
My policy currently is to share fixes and not share mods, so yeah I would.

I understand this sort of mentality (unconsciously, I know I do it.). The reason I understand it is because there are a lot of what I like to refer to as "Compilation" mods out there, that just use a bunch of existing modcomps, through them together, and spice it up with a tiny bit of original XML, but are all basically the same. They are boring, unpopular mods, but they outnumber the interesting mods, 5 to 1. If you release your cooler features (like I did with Advanced Automations), suddenly is crops up in 5 different mods, and it makes my mod less Unique, when I spent all the effort and countless hours writing and testings it, forging into un-traversed modding territory. The other side of the coin is that steal others work was how I learned C++. I began by just doing small SDK tweaks to the "Mountains: Back To Service" modcomp. Nowadays, I steal very little work (apart from the mods mine is built off of already), and write new features myself. However, a lot of the modders who make the "Compilation" mods don't spend the time to learn how the modcomps they are merging actually work, so it doesn't make them any smarter; which is why I choose not to have any real opinion on the subject. Anyway, worst case Scenario, I just ask LunarMongoose for a piece of code or a part of his mod, and I doubt he'll refuse me.;)

Wow that was a long way of saying it's not a big deal. :p
 
Alright Fuyu you convinced me, I'll make the happiness effect weighted by population, though it'll be offset-based, ie 10+population, rather than just using the population number by itself so the small cities aren't almost-completely ignored. (I do value +happiness some in new cities because, even if it's not useful for them right now, it will be later when they've had time to grow, basically.)

As to the closed-source thing, yeah basically what Afforess said. It's particularly important to me right now b/c MongooseMod just launched and I haven't had time to establish a big fan-base yet... I will consider releasing the full source later once I don't need every advantage I can get to attract mindless slaves, err I mean enthralled supporters. ;)

Update on what I'm working on, the AI civic logic... There are six possible sources of happiness from civics and four possible sources of health from civics, unless you're in a mod that mods civics to have additional capabilities, which I can't cover/support with this project obviously.

* LargestCityHappiness and MilitaryUnitHappiness can be used as-is.
* StateReligionHappiness and NonStateReligionHappiness can be computed fairly effortlessly (have to subtract off the initial value globals basically).
* FeatureHappiness is combined with ImprovementHappiness (ie Forest Preserves) in the vanilla code, and so, while it could be computed, it will be significantly faster to store the value separately.
* ExtraBuildingHappiness is contributed to by multiple sources, particularly events, so computing it would be a nightmare and this must be stored separately.

* NoUnhealthyBuildings and NoUnhealthyPopulation can be computed fairly harmlessly (have to ignore them on a case-by-case basis if the effects are already present from buildings like the Globe Theater, basically).
* ExtraHealth and ExtraBuildingHealth are contributed to by multiple sources, particularly events, so computing them would be a nightmare and these must be stored separately.

The result of this is that, in order to do this properly ie so that it runs as fast as possible, a couple new data fields need to be added to the player and/or city structures, which will break existing save games. Sorry, no way around it, and I suspect JDog will go ahead and support me on this if he ever gets back but I guess we'll see. :)

Oh, and it also means this "fix" is a carpload of work that's going to involve changes in a dozen or so places in the code. But it WILL result in AIs evaluating civics completely correctly, since happiness and health are the only effects where changing them changes the value of adding more of them... Everything else is strictly cumulative and linear I believe. Please let me know if I'm wrong about that though heh.
 
Yes, and therefore the value for this city will be low (or zero, or even below if you don't cap the happyNow/Then values to 5), and that will greatly reduce the average, which is what is returned.

Hmm, isn't using the average happiness increase as a basis for evaluating the power of a civic a major error in the original code? It's very hard to get a good comparison between the effects of civics when you immediately start using averages. If you divide by the number of cities (take the average effect) in every civic evaluation, then it has no effect in the evaluation of the civics. If you only use this average in the evaluation of some civics, then the comparison between civics becomes very uninsightful and vague.

If I as a human player want to exactly compare the power of 2 civics, one which gives + 1 :happiness: in every city and another that gives +20 gold in the capital (and if I have the patience of a computer to exactly count stuff), then I'll just count the effect in every city and add it together.

You could do something simple like:
value of 1 happiness is:
2 if city is unhappy
1.5 if city is at the happiness cap
1 if the city is 1 below the happiness cap
0.5 if the city is 2 below the happiness cap
0.2 otherwise

Take the sum over all cities

value of +20 gold in the capital is:
20

And of course, one of the main elements which was discussed here, switching between civics with different levels of happiness boost should be taken into account. So you'd have to count from a basic zero-level civic where no happiness effect is present.
 
Hmm, isn't using the average happiness increase as a basis for evaluating the power of a civic a major error in the original code? It's very hard to get a good comparison between the effects of civics when you immediately start using averages. If you divide by the number of cities (take the average effect) in every civic evaluation, then it has no effect in the evaluation of the civics. If you only use this average in the evaluation of some civics, then the comparison between civics becomes very uninsightful and vague.

Edit for Great Justice - The averages are part of the AI_getHappinessWeight() and AI_getHealthWeight() functions, but they are multiplied by getNumCities() in the AI_civicValue() code that started all this 30 posts ago, turning them back into full, unbounded values. See? I toldja this was complicated lol.

And of course, one of the main elements which was discussed here, switching between civics with different levels of happiness boost should be taken into account. So you'd have to count from a basic zero-level civic where no happiness effect is present.

I'm about half done writing the code to perform that, at the moment. It involves a significant amount of work, primarily due to the way the vanilla code bundles effects from civics with effects from other sources into the same variables. Implementing Fuyu's evaluation loop correctly is also not trivial, and the whole thing is riddled with opportunities to screw stuff up and/or implement stuff sub-optimally.
 
Okay, I am now done with the code that calculates health and happiness values for cities without any civic effects. I have double- and triple-checked it and it should be bug-free. Already it's a lot of code, but it should run very nearly as fast as before thanks to some efficiency improvements that should cancel out the slight extra overhead of maintaining and using a few extra state variables.

I am now working on the code that uses that, which is in AI_civicValue(), AI_H/HWeight() and... gulp... AI_doCivics(). There's at least one additional improvement to make here btw: BBAI is unchanged from vanilla, and assumes 3 military units will be present in general slash on average when calculating the value of the Monarchy happiness effect. I'm going to change that to something along the lines of the civ's land-unit army size divided by their number of cities... Please suggest a scale coefficient for that if you want one. :)
 
So I'm guessing you really didn't just do
Code:
[COLOR="Green"]//Civic vacuum[/COLOR]
for (iI = 0; iI < GC.getNumCivicOptionInfos(); iI++)
{	eOldCivic = getCivics((CivicOptionTypes)iI);
	if (eOldCivic != NO_CIVIC)
	{
		processCivics(eOldCivic, [B]-1[/B]);
	}
}
to create the vacuum? Because I would totally do that and screw any more complicated approach.
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.

AI_doCivics(): My approach would have been:
find out which cathegories only have a single valid option, and ignore those from this point on (except for the total cathegories number)
find out which civics could need their values to be recalculated (those that give a bonus where the value calculation uses AI_H/HWeight())
create vacuum
AI_bestCivic((CivicOptionTypes)iI, &iBestValue, &iSecondBestValue); <- we need the difference, at least in absolute numbers, but maybe relative too when we decide whether to revolution or not, which we can both get when we have the value of that second best option. (Vacuum values of active civics are stored seperately for decision making, in all other cases vacuum values are of no interest and can be overwritten by recalculated values.)
After selecting the civic (processCivics(eNewCivic, 1)) of the cathegory with the highest absolute difference, only recalculate if the civic is one of those that could need recalulation AND if eNewCivic from before actually did change happiness/health in some way.

When we have all values we need for the decision, we have to revert the effect of all new civics (processCivics(eNewCivic, -1)) and the vacuum is no longer needed:
Code:
for (iI = 0; iI < GC.getNumCivicOptionInfos(); iI++)
{	eOldCivic = getCivics((CivicOptionTypes)iI);
	if (eOldCivic != NO_CIVIC)
	{
		processCivics(eOldCivic, [B]1[/B]);
	}
}
[COLOR="Green"]//Civic vacuum END[/COLOR]

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.
 
Edit for Great Justice - The averages are part of the AI_getHappinessWeight() and AI_getHealthWeight() functions, but they are multiplied by getNumCities() in the AI_civicValue() code that started all this 30 posts ago, turning them back into full, unbounded values. See? I toldja this was complicated lol.

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. :crazyeye: ;)

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

I am now working on the code that uses that, which is in AI_civicValue(), AI_H/HWeight() and... gulp... AI_doCivics(). There's at least one additional improvement to make here btw: BBAI is unchanged from vanilla, and assumes 3 military units will be present in general slash on average when calculating the value of the Monarchy happiness effect. I'm going to change that to something along the lines of the civ's land-unit army size divided by their number of cities... Please suggest a scale coefficient for that if you want one. :)

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.

I do agree with Fuyu that not all units will be providing happiness.

On the other hand, those units that do provide happiness will be placed exactly in those places where they're needed the most. And if you calculate the advantage by averaging them over the cities, then the calculated value will also be lower than the one that you'd get when you specifically look at the cities which do get the units.

You could also take into account that some additional units might be created to combat unhappiness. I don't know whether the AI actually does that.

So I'd take the number of units suggested by Fuyu, but add a greater number of units to the larger cities. Maybe again a scaling of (5+ pop) of the units added to the various cities. (In an empire of city A, B and C, city A gets a number of units equal to (5+ popA)/(5+popA+5+popB+5+popC) * number of defensive units).
 
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.
 
Top Bottom