1. We have added a Gift Upgrades feature that allows you to gift an account upgrade to another member, just in time for the holiday season. You can see the gift option when going to the Account Upgrades screen, or on any user profile screen.
    Dismiss Notice

OOS problems and how to fix them

Discussion in 'Bugs and Crashes' started by 45°38'N-13°47'E, Jan 20, 2014.

  1. AIAndy

    AIAndy Chieftain

    Joined:
    Jun 8, 2011
    Messages:
    3,403
    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.
     
  2. 45°38'N-13°47'E

    45°38'N-13°47'E Chieftain

    Joined:
    Jun 7, 2008
    Messages:
    5,834
    Location:
    Just wonder...
    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.
     
  3. Thunderbrd

    Thunderbrd C2C War Dog

    Joined:
    Jan 2, 2010
    Messages:
    25,884
    Gender:
    Male
    Location:
    Las Vegas
    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.
     
  4. Thunderbrd

    Thunderbrd C2C War Dog

    Joined:
    Jan 2, 2010
    Messages:
    25,884
    Gender:
    Male
    Location:
    Las Vegas
    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!
     
  5. AIAndy

    AIAndy Chieftain

    Joined:
    Jun 8, 2011
    Messages:
    3,403
    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).
     
  6. Thunderbrd

    Thunderbrd C2C War Dog

    Joined:
    Jan 2, 2010
    Messages:
    25,884
    Gender:
    Male
    Location:
    Las Vegas
  7. 45°38'N-13°47'E

    45°38'N-13°47'E Chieftain

    Joined:
    Jun 7, 2008
    Messages:
    5,834
    Location:
    Just wonder...
    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.
     
  8. Thunderbrd

    Thunderbrd C2C War Dog

    Joined:
    Jan 2, 2010
    Messages:
    25,884
    Gender:
    Male
    Location:
    Las Vegas
    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.
     
  9. AIAndy

    AIAndy Chieftain

    Joined:
    Jun 8, 2011
    Messages:
    3,403
    Code:
    else if ( bSurroundedModifier && !bInvolvesHuman)
    
     
  10. 45°38'N-13°47'E

    45°38'N-13°47'E Chieftain

    Joined:
    Jun 7, 2008
    Messages:
    5,834
    Location:
    Just wonder...
    You're great guys! :goodjob:
     
  11. Thunderbrd

    Thunderbrd C2C War Dog

    Joined:
    Jan 2, 2010
    Messages:
    25,884
    Gender:
    Male
    Location:
    Las Vegas
    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.
     
  12. 45°38'N-13°47'E

    45°38'N-13°47'E Chieftain

    Joined:
    Jun 7, 2008
    Messages:
    5,834
    Location:
    Just wonder...
    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.
     
  13. Thunderbrd

    Thunderbrd C2C War Dog

    Joined:
    Jan 2, 2010
    Messages:
    25,884
    Gender:
    Male
    Location:
    Las Vegas
    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.
     
  14. 45°38'N-13°47'E

    45°38'N-13°47'E Chieftain

    Joined:
    Jun 7, 2008
    Messages:
    5,834
    Location:
    Just wonder...
    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.
     
  15. Thunderbrd

    Thunderbrd C2C War Dog

    Joined:
    Jan 2, 2010
    Messages:
    25,884
    Gender:
    Male
    Location:
    Las Vegas
    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.
     
  16. 45°38'N-13°47'E

    45°38'N-13°47'E Chieftain

    Joined:
    Jun 7, 2008
    Messages:
    5,834
    Location:
    Just wonder...
    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
     
  17. AIAndy

    AIAndy Chieftain

    Joined:
    Jun 8, 2011
    Messages:
    3,403
    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.
     
  18. 45°38'N-13°47'E

    45°38'N-13°47'E Chieftain

    Joined:
    Jun 7, 2008
    Messages:
    5,834
    Location:
    Just wonder...
    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.
     
  19. AIAndy

    AIAndy Chieftain

    Joined:
    Jun 8, 2011
    Messages:
    3,403
    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.
     
  20. Stormwind

    Stormwind Chieftain

    Joined:
    Dec 7, 2008
    Messages:
    697
    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?
     

Share This Page