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

Specific Bug Reports

Discussion in 'Civ4 - Better AI' started by Iustus, Jan 25, 2007.

  1. Afforess

    Afforess The White Wizard

    Joined:
    Jul 31, 2007
    Messages:
    12,239
    Location:
    Austin, Texas
    Found a cause of an infinite loop, in CvSelectionGroup, from Better AI:

    Code:
    /************************************************************************************************/
    /* Afforess	                  Start		 07/27/10                                               */
    /*                                                                                              */
    /* Infinite Loop Fix                                                                            */
    /************************************************************************************************/
    /*
    				if( pLoopUnit->canMove() && pLoopUnit->canPillage(plot()) )
    */
    				if( pLoopUnit->canMove() && pLoopUnit->canPillage(pLoopUnit->plot()) )
    /************************************************************************************************/
    /* Afforess	                     END                                                            */
    /************************************************************************************************/
    
    Your code appears twice in CvSelectionGroup.cpp, so this fix will need to be applied twice.
     
  2. Fuyu

    Fuyu Emperor

    Joined:
    Nov 5, 2009
    Messages:
    1,225
    Location:
    Austria
    I could have never found this one. I always thought the units inside a SelectionGroup are all standing on the same plot so plot() and pLoopUnit->plot() should, without exception, return a pointer to the same plot.
    If you know why plot() alone is failing, please share the wisdom.
     
  3. Afforess

    Afforess The White Wizard

    Joined:
    Jul 31, 2007
    Messages:
    12,239
    Location:
    Austin, Texas
    plot() returns the plot of the head unit; not where the selectiongroup is. Of course, until a few days ago, I thought selectiongroups were always on the same tile too. In fact, I think they *should* always be on the same tile; I have no idea what lead to the circumstances that created a selectiongroup over two tiles (the tiles were diagonally adjacent, IIRC); but what happen was a unit was trying to pillage, but the head unit was on a tile that couldn't be pillaged, so the pillage() call was returning false, so the loop never ended.
     
  4. Fuyu

    Fuyu Emperor

    Joined:
    Nov 5, 2009
    Messages:
    1,225
    Location:
    Austria
    Until this very moment, I was 100% sure of it. So much for that. :rolleyes:
    Maybe some xUPT issue, like unit gets expelled from a tile but not removed from its selectiongroup?
    Which I thought had to be identical, but yes I saw the difference. And in any case, if we already have a unit pointer pLoopUnit it's faster/simpler to just use that, no need to look up the head unit again each time.
    It's a good change, I just couldn't imagine it woud ever make a difference. Even less a big one like that.

    gj :goodjob:
     
  5. Afforess

    Afforess The White Wizard

    Joined:
    Jul 31, 2007
    Messages:
    12,239
    Location:
    Austin, Texas
    I don't think it's an XUPT issue, I just call jumpToNearestValidPlot() for full cities...
     
  6. Cybah

    Cybah Emperor

    Joined:
    Jun 22, 2007
    Messages:
    1,480
    Do we need both lines or just

    PHP:
    if( pLoopUnit->canMove() && pLoopUnit->canPillage(pLoopUnit->plot()) )
    instead of the other one?
     
  7. Fuyu

    Fuyu Emperor

    Joined:
    Nov 5, 2009
    Messages:
    1,225
    Location:
    Austria
    I wasn't really suspecting it was, just very wildly guessing because I still hope selectiongroups spreading over multiple tiles is not part of stock BTS.

    @Cybah: except for that line, everything is commented out. So yes, that's all you need in the 2 places inside startMission.
     
  8. Afforess

    Afforess The White Wizard

    Joined:
    Jul 31, 2007
    Messages:
    12,239
    Location:
    Austin, Texas
    Fuyu, I'm adding this line here and hoping it's never triggered:

    Code:
    void CvSelectionGroup::doTurn()
    ...
    while (pUnitNode != NULL)
    		{
    			pLoopUnit = ::getUnit(pUnitNode->m_data);
    			pUnitNode = nextUnitNode(pUnitNode);
    /************************************************************************************************/
    /* Afforess	                  Start		 07/30/10                                               */
    /*                                                                                              */
    /*                                                                                              */
    /************************************************************************************************/
    			FAssertMsg((pLoopUnit->plot() == plot()), "Selectiongroup is not on the same tile!");
    /************************************************************************************************/
    /* Afforess	                     END                                                            */
    /************************************************************************************************/
    			pLoopUnit->doTurn();
    
    			if (pLoopUnit->isHurt())
    			{
    				bHurt = true;
    			}
    		}
    
     
  9. Fuyu

    Fuyu Emperor

    Joined:
    Nov 5, 2009
    Messages:
    1,225
    Location:
    Austria
    Perfect :) Adding it too.
     
  10. EmperorFool

    EmperorFool Deity

    Joined:
    Mar 2, 2007
    Messages:
    9,633
    Location:
    Mountain View, California
    I'm pretty sure looking at the SDK that a selection group must not span multiple plots. If it does, it's a bug. I've seen more than a few places that assume all units in the group are on the same plot.
     
  11. Afforess

    Afforess The White Wizard

    Joined:
    Jul 31, 2007
    Messages:
    12,239
    Location:
    Austin, Texas
    That's what I thought. My solution is logical though, and I added the assert so I can catch it next time I see it happening.
     
  12. sjodster

    sjodster Chieftain

    Joined:
    Mar 29, 2010
    Messages:
    96
    LunarMongoose is correct. I'm in the process of merging BetterAI into the Realism mod and the iRange check was already there. The order of the if statements was the modification and it looks to me like it was made solely to avoid the generatePath call. The original efficiency label was correct.

    I agree. Potential enemies are ultimately ignored due to the second conditional; and isVisibleEnemyUnit just checks for visible enemy units (could be non-military), whereas getNumVisibleEnemyDefenders checks for visible enemy military units. The entire first conditional is superfluous.

    AI_potentialEnemy takes the players that the AI has warplans for into consideration as enemies. Including the AI_potentialEnemy check essentially means that a unit calling this function could start a war.

    AI_leaveAttack is called from the following 4 methods:

    1. AI_collateralMove
    2. AI_reserveMove
    3. AI_cityDefenseMove
    4. AI_cityDefenseExtraMove

    It seems to me that a unit that is supposed to be defending a city shouldn't be starting wars, even if such a war is in the AI's plan. So potential defenders should be ignored in at least the last two circumstances.

    It is probably appropriate for units with the first two AIs to start wars, but they can start wars in their AI_anyAttack or AI_cityAttack calls. So my vote is to ignore potential enemies altogether in AI_leaveAttack.

    -Josh

    (P.S. Thank you both for all of your hard work!)
     
  13. sjodster

    sjodster Chieftain

    Joined:
    Mar 29, 2010
    Messages:
    96
    There is an unmarked modification in CvUnitAI.cpp near line 9877. The definition of CvUnitAI::AI_shadow has been changed in BetterAI. The change is correctly marked in the .h file but not the .cpp file.

    It should be something like this:

    Code:
    /************************************************************************************************/
    /* BETTER_BTS_AI_MOD                      04/01/10                                jdog5000      */
    /*                                                                                              */
    /* Unit AI                                                                                      */
    /************************************************************************************************/
    /* original bts code
    bool CvUnitAI::AI_shadow(UnitAITypes eUnitAI, int iMax, int iMaxRatio, bool bWithCargoOnly)
    */
    bool CvUnitAI::AI_shadow(UnitAITypes eUnitAI, int iMax, int iMaxRatio, bool bWithCargoOnly, bool bOutsideCityOnly, int iMaxPath)
    /************************************************************************************************/
    /* BETTER_BTS_AI_MOD                       END                                                  */
    /************************************************************************************************/
    
    -Josh
     
  14. denev

    denev Warlord

    Joined:
    Jul 5, 2008
    Messages:
    208
    Location:
    Japan
    m_iStrategyRand isn't saved at initial save file.
    I have added one line as below.
    Code:
    void CvPlayerAI::AI_init()
    {
    	AI_reset(false);
    
    	//--------------------------------
    	// Init other game data
    	if ((GC.getInitCore().getSlotStatus(getID()) == SS_TAKEN) || (GC.getInitCore().getSlotStatus(getID()) == SS_COMPUTER))
    	{
    		FAssert(getPersonalityType() != NO_LEADER);
    		AI_setPeaceWeight(GC.getLeaderHeadInfo(getPersonalityType()).getBasePeaceWeight() + GC.getGameINLINE().getSorenRandNum(GC.getLeaderHeadInfo(getPersonalityType()).getPeaceWeightRand(), "AI Peace Weight"));
    		AI_setEspionageWeight(GC.getLeaderHeadInfo(getPersonalityType()).getEspionageWeight());
    		//AI_setCivicTimer(((getMaxAnarchyTurns() == 0) ? (GC.getDefineINT("MIN_REVOLUTION_TURNS") * 2) : CIVIC_CHANGE_DELAY) / 2);
    		AI_setReligionTimer(1);
    		AI_setCivicTimer((getMaxAnarchyTurns() == 0) ? 1 : 2);
    	}
    
    	[COLOR="Blue"]AI_getStrategyRand();[/COLOR]
    }
    I don't assure that this is best method.
    But I hope that it helps someone.
     
  15. Fuyu

    Fuyu Emperor

    Joined:
    Nov 5, 2009
    Messages:
    1,225
    Location:
    Austria
    Without a new random seed on reload, the numbers will be the same anyway. No need to save them.
     
  16. denev

    denev Warlord

    Joined:
    Jul 5, 2008
    Messages:
    208
    Location:
    Japan
    Initial autosave file keeps 0 (initial value of m_iStrategyRand) as m_iStrategyRand of each player.
    These may be fixed after first player ends his first turn.
    This means that if first player changes his action, second (or below) player's m_iStrategyRand is changed too.

    I can't make an assertion that it is a bug, but I believe it is funny.
     
  17. LunarMongoose

    LunarMongoose King

    Joined:
    Jan 29, 2006
    Messages:
    731
    Gender:
    Male
    Location:
    Boston, MA, USA
    With the "New Random Seed on Reload" game option OFF, combat result chains are preserved across loads in single player universally, and they were also preserved in multiplayer UP TO A CERTAIN POINT which was years ago. Somewhere in the Warlords period, or maybe when BTS came out I don't remember, it stopped working in multiplayer and reloads always triggered new random results regardless of the game option setting. This was on my really-long-longterm list of stuff to look into and try to fix, but I wrote it off as an issue with the PRN generator code which I know zilch about and which makes my brain hurt when I try to read the mathematics involved. I know, I know... *embarassed*

    ANYWAY... If Denev's fix fixes this then I will hug Denev so hard he suffocates. I don't have time to test it yet - still playing Starcraft 2 every waking minute heh - but I should be back on here soon-ish-ly. Just checking in atm heh. And thanks for your comments, sjodster. :)
     
  18. sjodster

    sjodster Chieftain

    Joined:
    Mar 29, 2010
    Messages:
    96
    I found an unmarked change in CvUnitAI::AI_settlerSeaTransport() circa line 17119. The original BTS code is this:

    Code:
    	if (iAreaBestFoundValue > iOtherAreaBestFoundValue)
    	{
    		//let the settler walk.
    		unloadAll();
    		return true;
    	}
    

    In BetterAI the code was changed to this:

    Code:
    	if (iAreaBestFoundValue > iOtherAreaBestFoundValue)
    	{
    		//let the settler walk.
    		getGroup()->unloadAll();
    		getGroup()->pushMission(MISSION_SKIP);
    		return true;
    	}
    
    I don't know if this was an unofficial patch or BetterAI modification.
     
  19. sjodster

    sjodster Chieftain

    Joined:
    Mar 29, 2010
    Messages:
    96
    Found another unmarked change in CvUnitAI::AI_specialSeaTransportSpy(). The 02/23/10 efficiency modification needs to have the lower comment moved down so that it contains the entire modification.

    The original code was this:

    Code:
    				if (iValue > 0)
    				{
    					if (GET_PLAYER(getOwnerINLINE()).AI_plotTargetMissionAIs(pLoopPlot, MISSIONAI_ATTACK_SPY, getGroup(), 4) == 0)
    					{
    						if (generatePath(pLoopPlot, 0, true, &iPathTurns))
    						{
    							iValue *= 1000;
    
    							iValue /= (iPathTurns + 1);
    
    The current BetterAI code is this:

    Code:
    /************************************************************************************************/
    /* BETTER_BTS_AI_MOD                      02/23/10                                jdog5000      */
    /*                                                                                              */
    /* Efficiency                                                                                   */
    /************************************************************************************************/
    				iValue *= 1000;
    
    				if (iValue > iBestValue)
    				{
    /************************************************************************************************/
    /* BETTER_BTS_AI_MOD                       END                                                  */
    /************************************************************************************************/
    					if (GET_PLAYER(getOwnerINLINE()).AI_plotTargetMissionAIs(pLoopPlot, MISSIONAI_ATTACK_SPY, getGroup(), 4) == 0)
    					{
    						if (generatePath(pLoopPlot, 0, true, &iPathTurns))
    						{
    							iValue /= (iPathTurns + 1);
    
    This line was moved above the conditional:

    Code:
    iValue *= 1000;
    
    However, the deletion of the original line is unmarked. The comment should be moved down so that the code looks like this:

    Code:
    /************************************************************************************************/
    /* BETTER_BTS_AI_MOD                      02/23/10                                jdog5000      */
    /*                                                                                              */
    /* Efficiency                                                                                   */
    /************************************************************************************************/
    				iValue *= 1000;
    
    				if (iValue > iBestValue)
    				{
    					if (GET_PLAYER(getOwnerINLINE()).AI_plotTargetMissionAIs(pLoopPlot, MISSIONAI_ATTACK_SPY, getGroup(), 4) == 0)
    					{
    						if (generatePath(pLoopPlot, 0, true, &iPathTurns))
    						{
    /************************************************************************************************/
    /* BETTER_BTS_AI_MOD                       END                                                  */
    /************************************************************************************************/
    
     
  20. sjodster

    sjodster Chieftain

    Joined:
    Mar 29, 2010
    Messages:
    96
    There's an unmarked change in bool CvUnitAI::AI_pickup(UnitAITypes eUnitAI, bool bCountProduction, int iMaxPath) circa line 19838.

    The original BTS code is this:

    Code:
    	if ((pBestPlot != NULL) && (pBestPickupPlot != NULL))
    	{
    		FAssert(!atPlot(pBestPlot));
    		getGroup()->pushMission(MISSION_MOVE_TO, pBestPlot->getX_INLINE(), pBestPlot->getY_INLINE(), 0, false, false, MISSIONAI_PICKUP, pBestPickupPlot);
    		return true;
    	}
    
    The BetterAI code is this:

    Code:
    	if ((pBestPlot != NULL) && (pBestPickupPlot != NULL))
    	{
    		FAssert(!atPlot(pBestPlot));
    		getGroup()->pushMission(MISSION_MOVE_TO, pBestPlot->getX_INLINE(), pBestPlot->getY_INLINE(), MOVE_AVOID_ENEMY_WEIGHT_3, false, false, MISSIONAI_PICKUP, pBestPickupPlot);
    		return true;
    	}
    
    	return false;
    }
    
    A '0' was changed to 'MOVE_AVOID_ENEMY_WEIGHT_3'.
     

Share This Page