Fun with the Apostolic Palace

Breaking this off from the 0.19.1 topic....

This post is about addressing the AP "hibernation" problems. Occasionally, the AP votes will stop coming for long stretches of time. One such occurrence is when a non-AP-owner Pope switches to Free Religion after a recent Pope vote. DanF did a good job explaining this situation in a post I shall be quoting below with some spoilerized code and reference blocks.

DanF5771 said:

Hmmm, after some tests with a modified CvVictoryScreen.py to show me the timer values for votes and Resident elections, I can't say I'm really much wiser.
There is just one odd thing:
When the VoteTimer reaches 0 it triggers a vote and the timer is then reset to iVoteInterval (9 turns for normal game speed). But if there are no valid resolutions the VoteTimer is set to 0 again immediately, hence the game tries for a new vote on the very next turn. The ResidentTimer starts with 4 (GlobalDefines DIPLO_VOTE_SECRETARY_GENERAL_INTERVAL) and decrements every time the VoteTimer reaches 0. If the ResidentTimer reaches 0 itself the next vote will be the election of a new Resident (this vote is always possible). But in case there are no valid resolutions the timers develop like this:

Spoiler :

Code:

[SIZE="3"]  VoteTimer  |  ResidentTimer
	9   |   4
	8   |   4
	7   |   4
	6   |   4
	5   |   4
	4   |   4
	3   |   4
	2   |   4
	1   |   4
	0   |   4   (no vote possible)
	0   |   3   (no vote possible)
	0   |   2   (no vote possible)
	0   |   1   (no vote possible)
	0   |   0   (Vote for new Resident)[/SIZE]

If there is no valid Resident (former Resident was not the builder of AP and changed Religion or switched to FR) the game skips the normal vote triggering procedure when the VoteTimer reaches 0. The VoteTimer is therefore reset to 9, the ResidentTimer decrements and the game goes on. Thus it takes much longer until a new Resident gets elected and until valid votes can be triggered again (up to almost 50 turns scaled for game speed). Maybe that was the reason why some thought the AP was in a state of hibernation. But it *will* come back to life eventually, so it's not really a bug that breaks the AP completely.

I would suggest to alter CvGame::clearSecretaryGeneral (the method that's called when the Resident loses his position due to FR or another Religion):

Spoiler :

Code:

[SIZE="3"]void CvGame::clearSecretaryGeneral(VoteSourceTypes eVoteSource)
{
	for (int j = 0; j < GC.getNumVoteInfos(); ++j)
	{
		CvVoteInfo& kVote = GC.getVoteInfo((VoteTypes)j);

		if (kVote.isVoteSourceType(eVoteSource))
		{
			if (kVote.isSecretaryGeneral())
			{
				VoteTriggeredData kData;
				kData.eVoteSource = eVoteSource;
				kData.kVoteOption.eVote = (VoteTypes)j;
				kData.kVoteOption.iCityId = -1;
				kData.kVoteOption.szText.empty();
				kData.kVoteOption.ePlayer = NO_PLAYER;
				setVoteOutcome(kData, NO_PLAYER_VOTE);

				[COLOR="RoyalBlue"][B]setSecretaryGeneralTimer(eVoteSource, 0);[/B][/COLOR]
			}
		}
	}
}[/SIZE]

The blue code sets the ResidentTimer to 0 so that a new Resident can be elected at the earliest possible voting date (VoteTimer = 0).
I don't have much time right now unfortunately to test this thoroughly (just tried with LegionSteve's save and it worked fine). I also hadn't encountered that problem personally so I don't have any other saves or testbeds to try it out...

Click to expand...

The original test saves for this posted by LegionSteve awhile ago weren't optimal; the first was a well-after-the-fact illustration and the second was pretty significantly WorldBuildered. I wanted something a little closer to true game conditions so I've attached a less-severly modified save based on the ungy-04 SG. In that game originally Wang Kon had built the AP and switched to Free Religion a few turns before teching Mass Media and obsoleting the thing. In this modification, the AP has been magically transported to Paris so when Wang goes FR he will become ineligible to be the Pope, causing CvGame::clearSecretaryGeneral() to be triggered. Additionally, the human player was given a tiny Hindu city so that we can get in on the voting and better see what's happening. With the original 3.17 logic, a pope vote is almost 40 turns away for the reasons Dan outlined above. With Dan's proposed change, the pope vote comes up after just 8 turns.

And this leads us into problem #2. If the pope vote actually fails (e.g. if the only eligible candidate doesn't have enough votes on his own and everyone else abstains) we are pushed back into the same situation. In the second attached save, after our 8 turn waiting period, the pope vote has come up but Napolean doesn't have enough votes to win. (I WBed away Hinduism from all but one of his and Tokugawa's cities and abstained on our vote too.) However, the Pope timer gets reset back to 4 and so we're once again forced into a long waiting period; this one approximately 50 turns.

In this case, I think we need to look at CvGame::doVoteResults(), check for a secgen/pope vote that had no winner, and reset the secgen/pope timer if necessary which should give us the standard 9-turn delay. Testing on that will happen in the next couple days.

But wait, there's more! There are also other reports of AP weirdness such as the following quote from r_rolo1 in a BUG topic where this issue came up.

r_rolo1 said:

The problem is that if a certain vote is called ( like the inocuous OB agreement ) and it happens something that changes the AI mind about it before the AI vote on that ( like for a example, a war between 2 AP members ( this already happened to me )), the vote sometimes gets stuck and no vote result ( and by extension no other AP votes ) is posted ,sometimes during that whole session, sometimes during a smaller period ( reloading or simply save/load normally solves it ).

I'm pretty sure that I have a save in those conditions ( I discovered that it is highly reproducible anyway: get a AP vote, DOW on a AP member on the same turn, see the vote disapear ) buried in my saves folder.... gimme a day or two

Click to expand...

Hopefully he'll come up with some saves so we can see what's happening there as this sounds like a somewhat different issue.

And finally there's the old issue of the disappearing hammer bonus that I am trying very hard to ignore until somebody forces a save upon me.

Invalid Attachment specified.

Click to expand...

Sorry Dresden, the links to your 2 saves seem to be broken ...

Thanks. Fixed.

If I remember correctly a vote can also die if new player (like a colony) appears between voting and the display of the results. This was supposed to have been fixed or something but some said it was still around. Any one have any more concrete information on this?

I'm definitely glad you're looking into these things!

Interesting. Hopefully someone who has seen such a problem can provide a save. I did some limited testing with both spawning a colony after a religious victory vote and declaring war after an open borders vote. In both cases, the vote was properly canceled (and a message about the cancellation was displayed) and the next vote triggered as scheduled.

We're not done with the AP. We've now got a lead on at least one hammer bonus problem. If you defy a resolution (losing the bonus), capture a city that has the bonus intact, and then support a resolution to regain the bonus, the captured city gets double-bonuses. See AP giving too much hammers for details.

I've only tested the behavior so far and have just started looking at the code. CvPlayer::acquireCity() is supposed to handle this and it does do *something* with the production bonuses, but it's obviously not the right thing. At a first glance, it looks like it undoes the AP hammer bonus on the old city, then it reverses itself and readds it before transitioning to the new city:


But I don't understand everything that's going on yet so I don't want to simply remove that second clause until I'm sure that's the issue.

A tough nut, but I hope I could / we can crack it:
You were following the right lead, Dresden. The reason for the bonus hammer problems of the AP seems to be that the BuildingYieldChange (BYC) vector of the old city overwrites any prior BYCs already introduced for the new city, because in CvPlayer::acquireCity the set-method is used instead of the change-method to push the old city's BYC vector into the new city (pNewCity->setBuildingYieldChange). Such a prior change occurs when the new city gets the religions of the old city. After the initialisation of the new city pNewCity->setHasReligion calls pNewCity->processVoteSourceBonus which in turn calls pNewCity->changeBuildingYieldChange which is supposed to give the valid bonuses to the conquering player but those get overwritten later. So would it be possible to lose hammers? Not with the current implementation, since the BYC vector only stores iChanges != 0 (no overwrite with 0). The situations where people complained about missing hammer bonuses in certain cities were in fact (probably) the other way around -- they were getting invalid bonuses in the other cities (conquered from loyal members) while being a disloyal member themselves.

In Magma_Dragoon's example he (the disloyal member) captures a city from Hatty (the loyal member) so that the following will happen during the execution of CvPlayer::acquireCity:
1. remove AP bonus for old city
2. re-add AP bonus for old city
3. build HasReligion array for old city
4. build BYC vector for old city
5. kill old city
6. init new city
7. set religion for new city (calls processVoteSourceBonus but does nothing as new owner is disloyal)
8. set BYC for new city --> gives the re-added hammer bonus of the old city to the new city

So when Magma_Dragoon votes for a resolution that passes he becomes a loyal member again and gains the doubled hammer bonus in that city.
Maybe there was a problem with losing hammers at some point so that the devs "fixed" this by introducing the re-add step 2. But we should definitely remove this again and either change the set-BYC call to a change-BYC call or move the setHasReligion block (plus the setHasCorp block (?)) further down so that they can change the set BYCs.

Ok, sorry for the delay dan, but I had this one buried deep in my saves folder.....

This one is from MC III game. It is 1400 AD and there was a AP vote to open borders with the members in this turn ( I said yes ). I dowed GK and HC in that turn and the AP results of this election never came , freezing the AP for a while ( IIRC until the war was over , but it was a while ago ). I thought it was wierd and replayed ... the results were the same at that time as well

I decided to post the last save before it, but I'm not even sure if HC already had made the AP

Thanks rolo! A very quick test showed nothing suspicious .
The AP hasn't been build yet in the 1310 AD save (HC built it in 1355 AD in Machu Picchu). In 1400 AD the timers are 5 till next vote and 4 votes till next election. Combined with epic speed this constellation might lead to the long "hibernation" period described above, should HC somehow lose his residency (due to being dead as a result of your DoW?). For my test I had changed the iVoteInterval for the AP to 2 in the xml (-> 3 on epic). I then closed borders with HC so that he would put up the vote for open borders again - I said yes and DoW'd immediately. The vote got cancelled (announced by a message). I conquered the AP city but when the vote timer reached 0 again HC put up the vote to end the war... So the AP showed its regular (displeasing) behaviour.
Gonna take a closer look tomorrow, thanks again.

Yep, thanks rolo. It sounds just like a standard canceled vote (the message is darned easy to miss, especially if you are watching the side of the screen expecting the results popup) which basically nullifies this vote but the next scheduled vote should still happen. We'll poke around though, just to be sure.

As you can imagine dan, the AP changed hands in 3 turns after 1400 AD HC survived until the end of the game... in fact everyone was alive ( ofc that most of them were badly stripped down and properly vassaled )

Well, probably missed the message

P.S as dan is with hands in code, there is still a question of the "Is there a logic in AI DOW?" thread that is still unawnsered there ( and AFAIK everywhere ) : do AI have diplo in account when voting to declare war or peace via diplo ( or via events ) ?

Been looking at this again and I think I now understand why the code reads as it does. In the VoteSourceInfos.xml, there are three possible loyalty bonuses: Building yields, Building commerce, and a Free Specialist. So I think the reason the code currently does this...


...is to handle the case of the free specialist. The loyalty bonus is disabled, then the specialists are processed, then it's reenabled to make sure nothing else is unduly affected. Maybe that was the only original bonus and the building yields/commerce came later in development and they forgot to special-case those too. But since the building commerce and yield vectors do give potential loyalty bonuses and are not stuck in this section, any bonuses they have get mistakenly carried over. The choices for this part of the fix therefore are to move the building commerce/yield vector assignment into that area or just disable the second processVoteSourceBonus() as previously discussed. In this case, I think disabling the second processVoteSourceBonus() is probably still the way to go since I just don't see any benefit to reinstating the loyalty bonus at this point and this overly-cautious approach is what caused the bug to begin with. This also makes things a bit more bulletproof for modification if someone makes processVoteSourceBonus affect other stuff later.

As for the second part of the fix -- what to do to the new city after it is initialized -- I've gone over various possibilities and I think moving the setHasReligion block is the best way to go. That way we can pretty much guarantee that processVoteSourceBonus() will not be called until after everything has been setup and we can leave the setBuilding... calls alone for now. That seems unlikely to cause unforseen consequences and adds similar modding flexibility like the first part of the fix. I see no reason to move the corporation block since corps don't directly affect any of the things that get set afterwards.

That's a good explanation. I agree about the second/re-enabling processVoteSourceBonus() causing nothing but trouble, so my opinion to remove this has not changed. I also agree moving the setHasReligion block further down. Actually I agree with everything you said

(I was thinking about hypothetical mods which might give analogous bonuses to cities with a certain corporation and worried about situations where a player with state property conquered such a city, leading to similar problems like the current troubles with the AP hammers. So I thought to generally set first and change (via setHasReligion/setHasCorporation) later, but no such things are possible without major changes to the SDK, so let's just ignore that.)