OOS problems and how to fix them

If something is async or not depends on where you call it from. So you pass it to the functions you call starting from the uppermost level. If that uppermost level is an async UI display function (like the function for odds display), then you pass true, otherwise you pass false.

That can mean adding bAsync to a lot of function signatures though so I am fine with your solution which should work for now as humans can only check odds for their own units which means that at least one human unit is involved.

 

TB, I can try your change in my current game as this part of the dll is the same in AND and C2C; I'll let you know. Thank you.

 

Cool! I also added:

Code:

	AI_setPredictedHitPoints(-1);
	pDefender->AI_setPredictedHitPoints(-1);

to resolveCombat. I did that here:

Code:

void CvUnit::resolveCombat(CvUnit* pDefender, CvPlot* pPlot, CvBattleDefinition& kBattle)
{
	PROFILE_FUNC();
	MEMORY_TRACK();

	CombatDetails cdAttackerDetails;
	CombatDetails cdDefenderDetails;

	AI_setPredictedHitPoints(-1);
	pDefender->AI_setPredictedHitPoints(-1);
	int iAttackerStrength = currCombatStr(NULL, NULL, &cdAttackerDetails);

To protect against the first suspect region AIAndy pointed out.

 

Yeah, I just wasn't seeing where those bAsync function calls were passing true either. Maybe I just didn't look hard enough

Anyhow... I'm glad this solution, simple as it is, should work. It would be a huge relief!

 

Oh, and move the check out from the flush cache thing to the bSurroundedModifier if. The cache should be neither changed nor used in this case (flushing it is a change).

 

Ok

 

So the code should be (besides some small non significant changes between AND and C2C)

Code:

int CvUnit::maxCombatStr(const CvPlot* pPlot, const CvUnit* pAttacker, CombatDetails* pCombatDetails, bool bSurroundedModifier) const
// OLD CODE
// int CvUnit::maxCombatStr(const CvPlot* pPlot, const CvUnit* pAttacker, CombatDetails* pCombatDetails) const
/*** Dexy - Surround and Destroy  END  ****/
{
	PROFILE_FUNC();

	int iCombat;
	int iI;
[COLOR="Red"]	bool bAsync = (GET_PLAYER(getOwner()).isHuman() || GET_PLAYER(pAttacker->getOwner()).isHuman());[/COLOR]
	FAssertMsg((pPlot == NULL) || (pPlot->getTerrainType() != NO_TERRAIN), "(pPlot == NULL) || (pPlot->getTerrainType() is not expected to be equal with NO_TERRAIN)");

and then

Code:

	else if ( bSurroundedModifier ||[COLOR="Red"] bAsync[/COLOR] )

correct?
Problem is that it's CTD. I get an error on the line

Code:

 bool bAsync = (GET_PLAYER(getOwner()).isHuman() || GET_PLAYER(pAttacker->getOwner()).isHuman());

as soon as I hit EoT, both in multiplayer and singleplayer, both on my current MP game or in a freshly started SP game. Actually in MP it crashes before finishing loading the savegame.

 

Yeah, been working on debugging that spot... took me a bit to realize what was going on as my compiler wasn't so kind as to point me in that direction and I didn't suspect that change was the problem.


OK... just sorted it out. Instead of

Code:

bool bAsync = (GET_PLAYER(getOwner()).isHuman() || GET_PLAYER(pAttacker->getOwner()).isHuman());

it should be:

Code:

	bool bInvolvesHuman = false;
	if (pAttacker != NULL)
	{
		bInvolvesHuman = (GET_PLAYER(getOwner()).isHuman() || GET_PLAYER(pAttacker->getOwner()).isHuman());
	}
	else
	{
		bInvolvesHuman = GET_PLAYER(getOwner()).isHuman();
	}

I'd forgotten how this code can be called so often without a defined attacker and what consequences that would have on the establishment of that bool value.

Sorry.

Also fyi: I've also sorted out a problem with my recent Commerce Audit. The Audit caught some problems itself - then caused a few. So my next update should have it right. Some of those original problems were likely present in original coding so I'd urge you to update those commerce compilation adjustments once I've got them on the SVN.

 

Code:

else if ( bSurroundedModifier && !bInvolvesHuman)

 

You're great guys!

 

One other thing I noticed... this fixes Surround and Destroy in general! I've been noticing that moving in units into surround positions didn't seem to be improving my odds anymore and wondering what that was all about... this change brings me back to being able to see the improved odds after such a move.

 

Well, no other OOS so far except for that damned OOS where random log is identical but position of a single unit is different in the OOS logs. Mostly is a caravel of my opponent, sometimes is another of her caravels, a single time happened with my caravel and another single time with a worker of mine. No idea what could be causing it. I've seen it happening in different situations. Inside/outside borders, with or without units embarked on the caravel, with or without AI pathing option, with and without routing the unit to move for more turns consecutively. I don't know what to think and where to look besides pathing system (which sadly is a big place to look into.

Edit: sometime OOS was cleared moving back the unit of 1 tile on its path, sometimes it didn't help.

 

When you have both players running and you can figure out which unit is 'out of position' from what the other system is showing you'll be able to see which plot it stopped registering further movement - sometimes yes, you can get it back to where it stopped but of course sometimes it's too far back (not just one space but sometimes many.)

I swear I thought Koshling and/or AIAndy had fixed that though... I recall having the problem then not having the problem and it's weird we're now having it again.

 

Can you point me out which revision should have solved the problem? Or at least approximately when it was supposed to be solved, then I can check the code myself. I'm almost sure I've imported all Koshling's code and probably all AiAndy's, but at least I'd know where to start looking.

 

It was quite a long time ago so I could look but it'd take a week of scouring the lists to find it given how slow my internet connection would enable me to scan back that far. Even then I could miss it. I suppose my ultimate point would be... pm Koshling and see if he might be able to point out the way based on his own memory.

 

I guess you were talking about this one
http://forums.civfanatics.com/showpost.php?p=12497887&postcount=30
There was an OOS fix by Koshling in rev5588 a few days later, but I guess it was on other multithreading issues, and I'm almost sure I've imported all the relevant code from that rev into AND dll. I'll double check again.

Also, there's probably some OOS issue in the void CvPlayerAI::AI_doDiplo(), because I got some OOS errors where AI was taking different decisions, like

Code:

59183	268	Global	AI Diplo Demand Tribute	1000	787

against

Code:

59183	268	Global	AI Diplo Open Borders	20	15

 

I have compared CvPathGenerator of AND and C2C and they are identical except for a bad change in the C2C one which only effected the multithreaded version of the path generator. I have undone that one now.

Then I searched the C2C log for OOS (you can filter the log) and checked that the relevant code is in AND and it seems to be.

 

I know, I've just done the same. So this OOS problem is probably both in AND and C2C. It's the most frequent I got until now, but what I don't understand is why it doesn't always happen. I mean, come on, it's just moving a caravel. I can do it while keeping all others parameters fixed. But sometimes it goes OOS while sometimes it doesn't. I've taken into consideration AI pathing, cargo, promotions, proximity to borders or to barbarian units, multithreading, go-to function. With or without these parameters, I randomly get an OOS or I don't. One thing I've thought about is maybe visibility? Since those units are exploring what's constantly changing is what they see. It doesn't explain my worker's OOS caused by different positions, but I don't know what else to think.

 

Visibility is possible. That is an OOS problem we had earlier. It happens when not all plots are properly considered for visiibility processing.
And AND currently does have a different calculation there.

 

There is/was (not sure) a bug where terrain, usually ocean, would be revealed without any unit near it. This was at most times associated with caravels or any ship which can get the visibility upgrades. Is this maybe a remnant or cause of the OOS? If one ship through a bug reveals territory which is then not synced?