Fall Further 050 Bug Report Thread

[MootPoint]

Okay, I found a recursion. I *think* it's a barbarian unit trying to attack a city, and the mission to move into the city never sets the attack flag:

getGroup()->pushMission(MISSION_MOVE_TO, pBestPlot->getX_INLINE(), pBestPlot->getY_INLINE(), ((bFollow) ? MOVE_DIRECT_ATTACK : 0));

bFollow is a param that always defaults to false when one calls CvUnitAI::AI_cityAttack().

So, the barb goes through the motions of doing the attack, but is blocked from moving into the city square by this check in CvSelectionGroup::groupMove():

		if ((pLoopUnit->canMove() && ((bCombat && (!(pLoopUnit->isNoCapture()) || !(pPlot->isEnemyCity(*pLoopUnit)))) ? pLoopUnit->canMoveOrAttackInto(pPlot) : pLoopUnit->canMoveInto(pPlot))) || (pLoopUnit == pCombatUnit))
		{
            pLoopUnit->move(pPlot, true);
		}
/*************************************************************************************************/
/**	Alertness								END													**/
/*************************************************************************************************/
		else
		{
			pLoopUnit->joinGroup(NULL, true);
			pLoopUnit->ExecuteMove(((float)(GC.getMissionInfo(MISSION_MOVE_TO).getTime() * gDLL->getMillisecsPerTurn())) / 1000.0f, false);
		}

The code jumps into the else clause, pushes the same bloody Move mission right back into the queue, and it never gets resolved.

Is a barbarian unit supposed to be able to sack the city?

*edit* - okay, I found out what's causing the failure - the failure's in CvUnit.cpp:

	if (isBarbarian() && GC.getGameINLINE().getElapsedGameTurns() < (30 * GC.getGameSpeedInfo(GC.getGameINLINE().getGameSpeedType()).getGrowthPercent())/100)
	{
		if (pPlot->isCity() && atWar(pPlot->getTeam(), getTeam()))
		{
			if (!bAttack)
			{
				return false;
			}
		}
	}

So the canMoveInto() function fails because the elapsed game turn count check is less than the 30 * getGameSpeedInfo() check. If it was later in the game, this recursion wouldn't happen...

*edit 2*
Okay, proposed fix: the root of the problem is that when the AI city attack code tries to determine whether it can attack the plot, it calls generatePath(), which in turn calls canMoveOrAttackInto(), which calls canMoveInto() twice, with the attack flag off, then on. So the AI will always think that it can attack the city, but later when it tries to do the move, it's blocked. So I think it's actually a pretty simple fix - instead of:

	if (isBarbarian() && GC.getGameINLINE().getElapsedGameTurns() < (30 * GC.getGameSpeedInfo(GC.getGameINLINE().getGameSpeedType()).getGrowthPercent())/100)
	{
		if (pPlot->isCity() && atWar(pPlot->getTeam(), getTeam()))
		{
			//if (!bAttack)    <- delete this
			{
				return false;
			}
		}
	}

Just get rid of the if(!bAttack) check & return false immediately.. Then you'll get consistency - if you're a barbarian unit, and you're at war with the plot's city, you can't attack before turn x, period.

(Tested against the save game, and it now works...)

 

[MootPoint]

Side note: check out CvUnit.cpp, line 4341, when it's trying to determine whether animal teams can move into a plot:

			if (!plot()->isOwned())
			{
				if (pPlot->isOwned() && atWar(pPlot->getTeam(), getTeam()))
				{
					return false;
				}
			}

If the plot isn't owned, the inside check against pPlot->IsOwned() will never pass. Copy/paste bug.

 

[MootPoint]

Totally different issue: ranged attacks. I have a giant spider with Greater Venenum, but he can't use his ranged attack abilities (the button isn't available). Is this because he's still a hidden nationality unit?

 

[Vehem]

Totally different issue: ranged attacks. I have a giant spider with Greater Venenum, but he can't use his ranged attack abilities (the button isn't available). Is this because he's still a hidden nationality unit?

Not sure yet - I've been trying to work that one out myself too...

 

[Thunder_Gr]

I think this save would be very interesting. It is a save made in the middle of an infinite loop!
When you load it, you should enter "waiting for other civilizations" mode...
Please, tell me where it is stuck! I really want to know.

BtW, this is the first serious use of magic by the AI. The Sheaim have repeatedly used fireballs on my positions and they are using skeletons and mages in a way that makes sense!

 

[xienwolf]

Side note: check out CvUnit.cpp, line 4341, when it's trying to determine whether animal teams can move into a plot:

			if (!plot()->isOwned())
			{
				if (pPlot->isOwned() && atWar(pPlot->getTeam(), getTeam()))
				{
					return false;
				}
			}

If the plot isn't owned, the inside check against pPlot->IsOwned() will never pass. Copy/paste bug.

Look at it a bit closer. Outer IF uses plot(), inner IF uses pPlot. It says that if they are in unowned territory, they cannot enter hostile owned territory.


Pity about needing to ditch the !bAttack IF statement on CanMoveOrAttackInto call. Now it'll be a bit more obvious to players that there is a "safe period" enacted. The original code, if allowed to work, would have allowed them to kill your defenders, but not to take the city.

 

[Thunder_Gr]

Tried to debug my game, but it itterates mostly in the .exe .
The only indication of the .dll my debugger gives me, is a stored address in the stack of a CVGameCore.Dll@getPlayerAITurn()(or something similar, I forgot to take a notice of it...)...But there is just a function that returns a variable....
Could anyone know of a good free debugger that I would be of any help for these situations?

 

[Thunder_Gr]

Btw, sorry for the initial lack of information. My save game is a FF050j game. I play Amurites. Monarch difficulty, Standart time, Large map.
I have named the file test, because I did not really believe I could save when it is not, yet, my turn... I was hoping I might have been able to load past the deadlock, but I got this, instead...

 

[MootPoint]

Look at it a bit closer. Outer IF uses plot(), inner IF uses pPlot. It says that if they are in unowned territory, they cannot enter hostile owned territory.
.

Whoops, didn't see that. But... if the outer loop is a constructor, it'll always default to unowned, no? In which case, the outer check is totally superfluous... I'll step into it to verify.

Pity about needing to ditch the !bAttack IF statement on CanMoveOrAttackInto call. Now it'll be a bit more obvious to players that there is a "safe period" enacted. The original code, if allowed to work, would have allowed them to kill your defenders, but not to take the city.

Hmm... yeah, it would suck to lose that. I think I have an idea on how we can rework this:

if ((pLoopUnit->canMove() && ((bCombat && (!(pLoopUnit->isNoCapture()) || !(pPlot->isEnemyCity(*pLoopUnit)))) ? pLoopUnit->canMoveOrAttackInto(pPlot) : pLoopUnit->canMoveInto(pPlot))) || (pLoopUnit == pCombatUnit))
{
	pLoopUnit->move(pPlot, true);
}

How about this instead?

bool cityRaid = (pPlot->isEnemyCity(*pLoopUnit) && (!pLoopUnit->isNoCapture() ? true :  pPlot->getNumDefenders(pPlot->getOwner()) > 0);
bool canAttackPlot = bCombat || cityRaid;
bool canEnterPlot = canAttackPlot ? pLoopUnit->canMoveOrAttackInto(pPlot) : pLoopUnit->canMoveInto(pPlot);
		
if ((pLoopUnit->canMove() && canEnterPlot) || pLoopUnit == pCombatUnit)
{
	pLoopUnit->move(pPlot, true);
}

The idea being, we pay attention to the isNoCapture() status of the unit. In the logic that blocks barbarian attacks for x turns, we turn off the barbarian unit's capture ability until the time expires. So this code above won't allow a noCapture unit to attack unless there's some defenders left in the plot.

(Haven't tried this out yet, but I'll give it a whirl shortly...)

 

[MrPopov]

I am having problems conquering cities

I am playing a game as Faeryl of Svartalfar and I am running Council of Esus as religion.
I am in a war started by Tasunke and I killed all the guys in one of his cities but my hunters and swordsmen would enter the city but not conquer it. Even adepts could enter the city. I have the CoE shrine and I have tried using units without the shrous (or whatever it is that lets them be visible in your land) and of course these units are not masked (ie they are not HN)

THe only unit that actually triggered the city capture was Gibbon Goetia but only after he summoned a mistform.

I guess I am either confused about some kind of game mechanism that I am unaware of, or it's a bug.


Also I am curious regarding forest stealth. Can it be removed such that the unit can defend if an enemy unit enters its tile

I found out what was going on. I was attacking from a forest tile and my units had forest stealth. I have been a bit more careful lately when attacking cities, and have resolved this problem when I attack from a non-forest tile.

There should probably be a way to remove forest stealth when you need to.

 

[MootPoint]

Tried to debug my game, but it itterates mostly in the .exe .
The only indication of the .dll my debugger gives me, is a stored address in the stack of a CVGameCore.Dll@getPlayerAITurn()(or something similar, I forgot to take a notice of it...)...But there is just a function that returns a variable....
Could anyone know of a good free debugger that I would be of any help for these situations?

Visual Studio Express is a free download (last I checked), and should do just fine. Mind you, I don't think it's your debugger that's necessarily the issue - I think most people keep building Final_Release, which is hard to debug with, as the compiler will make optimizations, so when you're stepping with the debugger, the lines won't always match up with your code, and your variable evaluator will sometimes show bogus values. You should try the make file I posted a few pages back & build the Debug target instead - you'll probably have better luck with that.

Also, once you've got that going, try putting a breakpoint on CvGame::update(). That's the carotid artery, so to speak, of the game loop... look for the line that says DoTurn() in there - I'm sure it's never being executed, since that was the same symptom I was seeing with DragonLord's save game.

Cheers

 

[Thunder_Gr]

DEFINITELY name a Great Engineer AND a Great Sage after Moot...

Assume I rightly there'll be a new patch coming?

Edit: BTW, has anyone solved the hang I posted yet?

Hmm... What about this, in CvUnitAI.cpp, in AI_update():

/*************************************************************************************************/
/**	Xienwolf Tweak							01/04/09											**/
/**					Clearing Asserts and helping the AI stop looping so much					**/
/**Near as I can tell, this is refusing to allow the unit to do ANYTHING if it winds up isolated**/
/**	I personally don't care for that, but it makes sense to keep the AI from trying to move the	**/
/**	Unit, so need to re-apply this once I figure out how to check for possible missions/spells	**/
/**				before canceling the unit's ability to make any movements						**/
/*************************************************************************************************/
/**								---- Start Original Code ----									**
//FfH: Added by Kael 12/22/2007
    CvPlot* pPlot = plot();
    CvPlot* pLoopPlot;
    bool bValid = false;
	for (int iDX = -1; iDX <= 1; iDX++)
	{
		for (int iDY = -1; iDY <= 1; iDY++)
		{
			pLoopPlot = plotXY(pPlot->getX_INLINE(), pPlot->getY_INLINE(), iDX, iDY);
			if (pLoopPlot != NULL)
			{
			    if (canMoveOrAttackInto(pLoopPlot))
			    {
			        bValid = true;
			    }
			}
		}
	}
    if (bValid == false)
    {
		getGroup()->pushMission(MISSION_SKIP);
        return false;
    }
//FfH: End Add

Stubbing this out could potentially explain why these units aren't getting the MISSION_SKIP option - maybe this code chunk needs to be reinstated at the end of the function, if no other valid AI moves are applicable?

Once more, MootPoint was right. I was able to get out of my infinite loop by adding

getGroup()->pushMission(MISSION_SKIP);

just before the method returns.
It should fix Dragonlord's hang as well.

I did test, putting the entire block there, the results are the same. It does not seem to affect AI play, it just avoids the deadlock.
I did tests using the original dll, one with just that line and one with the whole block.
The AI seemed uneffected. The original .dll locks down, though.

Here are the two versions I tried, for anyone too impatient to wait for the new patch.

The .dlls also include this:

if (isBarbarian() && GC.getGameINLINE().getElapsedGameTurns() < (30 * GC.getGameSpeedInfo(GC.getGameINLINE().getGameSpeedType()).getGrowthPercent())/100)
	{
		if (pPlot->isCity() && atWar(pPlot->getTeam(), getTeam()))
		{
			//if (!bAttack)    <- delete this
			{
				return false;
			}
		}
	}

fix proposed by MootPoint. This means, I did delete the if block

 

[Thunder_Gr]

Visual Studio Express is a free download (last I checked), and should do just fine. Mind you, I don't think it's your debugger that's necessarily the issue - I think most people keep building Final_Release, which is hard to debug with, as the compiler will make optimizations, so when you're stepping with the debugger, the lines won't always match up with your code, and your variable evaluator will sometimes show bogus values. You should try the make file I posted a few pages back & build the Debug target instead - you'll probably have better luck with that.

Also, once you've got that going, try putting a breakpoint on CvGame::update(). That's the carotid artery, so to speak, of the game loop... look for the line that says DoTurn() in there - I'm sure it's never being executed, since that was the same symptom I was seeing with DragonLord's save game.

Cheers

Unfortunatelly, I didn't see any offline installation download available, and my computer is not the computer that has access to the Internet...So, no luck for me.
And, yes, I wanted to build a debug library, but codeblocks cannot do that.

 

[MootPoint]

Once more, MootPoint was right. I was able to get out of my infinite loop by adding

getGroup()->pushMission(MISSION_SKIP);

just before the method returns.
It should fix Dragonlord's hang as well.

I did test, putting the entire block there, the results are the same. It does not seem to affect AI play, it just avoids the deadlock.
I did tests using the original dll, one with just that line and one with the whole block.
The AI seemed uneffected. The original .dll locks down, though.

So, while it might fix the deadlock, I'm not sure I'm advocating that it's the right fix. The consequence of doing this is that if the group can't figure out what to do, it's just going to stupidly sit there. At the very least, if we're going to default to a SKIP action at the end of this function, there should be an assert that fires to let us know that we're defaulting to a poor choice, so that others debugging a similar scenario can at least catch the problem, back up & figure out why no other AI option was available/chosen.

I'll also point out that I don't know this code base very well. I've got debugger skills , but I can't predict all the subtleties of how things are supposed to work, which is why I'm posting what I'm discovering for feedback in case I go off in the weeds.

 

[Thunder_Gr]

So, while it might fix the deadlock, I'm not sure I'm advocating that it's the right fix. The consequence of doing this is that if the group can't figure out what to do, it's just going to stupidly sit there. At the very least, if we're going to default to a SKIP action at the end of this function, there should be an assert that fires to let us know that we're defaulting to a poor choice, so that others debugging a similar scenario can at least catch the problem, back up & figure out why no other AI option was available/chosen.

I'll also point out that I don't know this code base very well. I've got debugger skills , but I can't predict all the subtleties of how things are supposed to work, which is why I'm posting what I'm discovering for feedback in case I go off in the weeds.

I was a bit worried, too. But I played 6 turns from the same savepoint(turn 337 IIRC), and the AI played the exact same moves with all 3 .dlls. It deviated in a more aggressive path with the "one line" version the 3rd time I checked(yes, I did 6 turns x3 Dlls x 3 times). This means that it does not affect the AI when there are valid moves.
In addition the "push" indicates a stack, so, it could be added in the end of a stack of actions, which is fine, at least if that stack is cleared after each turn.
In anycase It can be used as a short-termed fix. Or as a last resort to get over the deadlock and then revert to the original dll without any ill effects, as well.

As always, I am sure Xienwolf will implement the proper way to avoid the deadlocks in the next patch.

 

[vdex34]

Unfortunatelly, I didn't see any offline installation download available, and my computer is not the computer that has access to the Internet...So, no luck for me.
And, yes, I wanted to build a debug library, but codeblocks cannot do that.

VC++ 2005 express edition offline install available here

The offline install is an .iso image though (CD). The 2008 edition is available on the same website, but for some reason M$ made it so 2008 can only be burned to a DVD. (Its a CD size image both versions.

 

[xienwolf]

pUnit->plot() tells you which plot the unit is currently standing on. I'm not sure why they broke from standard convention and avoided using pUnit->getPlot(), it does sometimes get confusing, but it is not a constructor.

As for the rest, I'll take some time to look it over later tonight when I take a longer break from work.

 

[MootPoint]

Xien, forget the changes I proposed to CvSelectionGroup.cpp - they're not needed. Here's a better fix to CvUnit::canMoveInto() that fixes the recursion bug & also allows barbarians to attack if there are defenders left:

	if (isBarbarian() && GC.getGameINLINE().getElapsedGameTurns() < (30 * GC.getGameSpeedInfo(GC.getGameINLINE().getGameSpeedType()).getGrowthPercent())/100)
	{
		if (pPlot->isCity() && atWar(pPlot->getTeam(), getTeam()))
		{
			return (pPlot->getNumDefenders(pPlot->getOwner()) > 0);
		}
	}

 

[MootPoint]

@Thunder_Gr: could you post that save game that was deadlocking you? I'm curious whether my latest change fixes it, or whether the SKIP was still required... Thx!

 

[xienwolf]

That looks like about what I'd like for function behavior