Things we must solve before V29

strategyonly · Mar 25, 2013

OK now with the new SVN WE have DISAPPEARING units, in the city, all around the city, if you cant see them how can you fight them, and these are NORMAL units ie scouts, atlatl's etc) :mad:

Also what i posted in the Bug thread:

http://forums.civfanatics.com/showpost.php?p=12312203&postcount=878

http://forums.civfanatics.com/showpost.php?p=12312457&postcount=879

"Something strange in the neighborhood, and this isn't Ghostbusters!!"

Thunderbrd · Mar 25, 2013

AIAndy said:
The plot groups are Koshling's code so best would be if he looks at it.

Alright. I just hope I've given him a good head start and that my entire Sunday's worth of modding time wasn't just a total waste.

Some conversation with my wife after your post last night and she reminds me... open borders used to not be the prerequisite for trading with a nation anyhow. She's right... We never take open borders with other civs but we still get foreign trade routes - or at least that's how it used to be. So has this changed by design intent?

Hydromancerx said:
@Thunderbrd

The Bombard Bug is back.

http://forums.civfanatics.com/showpost.php?p=12312062&postcount=877

Wow... ok. Did NOT expect that but now that I've got a save I should be able to take a look at what's taking place there. Thanks!

strategyonly said:
OK now with the new SVN WE have DISAPPEARING units, in the city, all around the city, if you cant see them how can you fight them, and these are NORMAL units ie scouts, atlatl's etc)

Ah... my wife's reported bug. The really messed up thing with this one, in terms of trying to figure out what's happening, is that if a game is saved, closed out, then reopened, the bug vanishes for a bit. So running her save under the debugger, I couldn't get the bug to repeat. I had thought it might've been an SVN update that resolved her issue but if YOU are also getting it... it was a significant bug when she was experiencing it.

I think God himself must want our release delayed! :lol:

Koshling · Mar 25, 2013

Thunderbrd said:
Here is the savegame and some discussion.

Apparently my wife's game is NOT suffering from loss of trade routes. But this one is somehow. May have something to do with the fact that he only has a capital city. But really, he should be getting foreign trade routes. I can't see why he wouldn't be and his trade routes tally shows he should have 11 routes but no valid cities are being found somehow.

Just to be clear, its in his one and only city that he's not showing routes when he should be.

The diagnosis in that post is apparently completely off base though.

A number of other incorrect calculations are showing in the city screen's help hovers. This save also shows the defense value miscalculation I mentioned earlier as well.

UPDATE: VICTORY!!!!

Apparently changing this line in CvCity::updateTradeRoutes() and calculateExtraTradeRouteProfit(int iExtra, int* &aiTradeYields) const

Code:

if (pLoopCity->plotGroup(getOwnerINLINE()) == plotGroup(getOwnerINLINE()) || GC.getDefineINT("IGNORE_PLOT_GROUP_FOR_TRADE_ROUTES"))

to

Code:

if (isConnectedTo(pLoopCity) || GC.getDefineINT("IGNORE_PLOT_GROUP_FOR_TRADE_ROUTES"))

resolves the foreign trade bug.

I'll update with a dll with this fix later tonight.

Sorry, I'm afraid this is a complete mis-diagnosis. As AIAndy says, your change means that if A has the ability to trade with B, then B gets the ability to trade with A which is not the way it is supposed to be.

I reverted your change and then dug into the save in detail...

...after finding that there was nothing wrong with the plot groups anyway I started looking at the reasons the trade routes are not there. It turns out they are not there because they shouldn't be! Reasons as follows:

1) The human player does not know 'Navigation' so they cannot link up trade routes across ocean tiles. Based on the map that means the ONLY cities you could POSSIBLY have trade routes with are the 4 on the same continent as you.
2) Persepolis cannot be traded with because you have no open borders agreement
3) The two Israeli cities on your continent cannot be traded with because Israel is using the civic 'Communalism' (don't ask me why!) which includes 'no foreign trade routes'

Hence there are NO foreign cities in the entire game you can legitimately trade with, and having no trade routes is therefore totally correct.

I see no evidence at all of any bug.

I will push the change to revert the previous 'fix' with my other fixes shortly

Hydromancerx · Mar 25, 2013

Hydromancerx said:
@Nimek

In addition to the metal stuff I have added Bridges and around 40 other new building such as a Bridal Shop, Funeral Pyre, Karaoke Club, Pancake House, Day Care Center and Dried Meat Maker. You can see the full list in the SVN logs. There are also 5 new Pests and a TON of tweaks to buildings. Oh and a few new wonders which include the Humphrey Metrodome and Golden Gate Bridge.

Ok so I was curious to see how many building I did this cycle. So here it is ...

Spoiler :

There may have been a few more but those are all the ones I can find.

ls612 · Mar 25, 2013

So if I'm understanding correctly the two main things that still need to get done for release are;

The mysterious case of disappearing units (which I have not experinced, and looks awfuly like some messed up engine-DLL interaction which will be hard to pin down).
The bombard bug that Hydro so kindly gave a save for.

Am I right in thinking that that is all we need to fix?

Dancing Hoskuld · Mar 25, 2013

As far as I know that is correct.

Koshling · Mar 25, 2013

ls612 said:
So if I'm understanding correctly the two main things that still need to get done for release are;

The mysterious case of disappearing units (which I have not experinced, and looks awfuly like some messed up engine-DLL interaction which will be hard to pin down).

The bombard bug that Hydro so kindly gave a save for.

Am I right in thinking that that is all we need to fix?

CTD just reported by Zeinul (which is what I am looking at now)

Edit - I'm not going to get this done tonight, it's proving elusive.

Is someone looking at Hydro's bombardment save, or the invisible units?

Thunderbrd · Mar 26, 2013

@Koshling: Ok, fair 'nuff on the trade evaluation. There are two things to that that I was unaware of with the 'rules' considered and although I'd taken a slim look at what our screens would allow me to see (I'm not sure how you figured out what all civics Israel was on as the civic list on other leaders cuts off and won't scroll over!).
The first error in my thinking was that I'm not sure when open borders ever became a prerequisite for trading (this never USED to be this way! In Vanilla at least... is this a new C2C rule then?)
The second would be that I was not aware that the OTHER civ's inability to foreign trade can deny your ability to foreign trade (though I'd wondered and my inability to figure out what civics the other leaders fully had was keeping me from seeing this - but I discounted it in consideration that I figured it shouldn't matter.)
And three... I'd assumed he was at a naval point where he'd have no difficulty with overseas trade so my inexperience with that stage of the game threw me there.

Anyhow, yeah, sorry for the inaccuracy. I figure this all adds up to perhaps some needed improvements in the displays somewhere that could help a player figure out why his city had no foreign trade in this situation. But alright then... not a bug. Thanks for fully clarifying. This one has been on my list for a long time and if nothing else I suppose its been good practice at 'trying'.

As for the issue in CvUnit with the combat evaluations... I really don't understand why those particular combat modifiers would allow a unit to evaluate a potential combat against itself when all other combat modifier segments don't??? Without those evaluations, invisible units are losing the ability for those combat modifiers (vs combat class, vs unit class and vs domain) to be in-effect! Thus, if you promote, say, an assassin with anti-melee, and it goes to attack a melee unit that is not aware it is there, it gains no anti-melee benefit. (And vice versa... if I have anti-melee and a rogue attacks me and I don't see it, MY anti-melee wouldn't have any effect either!)

While the unit's ability to evaluate a possible attack against itself is somewhat baffling to begin with, I can think of a way to eliminate that possibility there while maintaining those combat modifiers. But before I jump into anything further here, I'd like to hear what you have to say about how a unit can even begin to start evaluating an attack against itself and why THOSE combat modifiers (and not modifiers like terrain evaluations, being in a city, etc...) aren't needing to be equally as insolated against that.

The idea I have is to base the isolation of those modifier calculations on JUST whether the unit is evaluating the attack as if it were against itself rather than on if the attacker is invisible or not. (But I suspect this won't work either will it?)

Truth is, I probably understood the cause for this better when I WAS working on the combat mod as opposed to now, about a year after last working on anything in that maxCombatStrength function. A hazy memory does suggest that if the unit can't see the attacking enemy it will simply evaluate against itself so it can generate most of the combat modifiers. Perhaps, if that's the case, you may be able to see a way to resolve the real bug - inapplicability of combat modifiers that should still have an effect.

By the way, does this have anything to do with why units are suddenly going invisible when next to your units?

And the third thing is that while its my intention to look into the bombard bug, I must warn that I'm working 11 hr days right now so my time is extremely limited at being able to. I MIGHT be able to make some headway on it tonight and will give it a shot but if you've exhausted the other issues and I haven't been able to get to it, feel free.

I realize my ineffective debugging efforts must be a bit frustrating for you... it certainly has been for me! I'm getting better but I'm still rather imperfect at this. I recognize that and am learning as much by failures as by successes. Thanks again for your efforts. C2C could hardly function without ya buddy!

strategyonly · Mar 26, 2013

Koshling said:
Is someone looking at the invisible units?

I wonder IF the invisible forest also has SOMETHING to do with this?? :dunno:

Thunderbrd · Mar 26, 2013

@Koshling: I looked a bit further into the issue with CvUnit. This is my suggested change which, if I'm right, should address both the issue you noted about the self evaluation (which a little research above the initial tweak does reveal the cause for) and the loss of some combat modifiers.

Spoiler :

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;
	//TB SubCombat Mod Begin
	int iI;
	UnitCombatTypes eUnitCombatType;
	//TB SubCombat Mod End

	FAssertMsg((pPlot == NULL) || (pPlot->getTerrainType() != NO_TERRAIN), "(pPlot == NULL) || (pPlot->getTerrainType() is not expected to be equal with NO_TERRAIN)");
	
	// handle our new special case
	const	CvPlot*	pAttackedPlot = NULL;
	bool	bAttackingUnknownDefender = false;
	if (pAttacker == this)
	{
		bAttackingUnknownDefender = true;
		pAttackedPlot = pPlot;
		
		// reset these values, we will fiddle with them below
		pPlot = NULL;
		pAttacker = NULL;
	}
	// otherwise, attack plot is the plot of us (the defender)
	else if (pAttacker != NULL)
	{
		pAttackedPlot = plot();
	}

	CombatStrCacheEntry* pCacheEntry = NULL;
	const CvUnit* pOriginalAttacker = pAttacker;

	if (pCombatDetails != NULL)
	{
		pCombatDetails->iExtraCombatPercent = 0;
		pCombatDetails->iAnimalCombatModifierTA = 0;
		pCombatDetails->iAIAnimalCombatModifierTA = 0;
		pCombatDetails->iAnimalCombatModifierAA = 0;
		pCombatDetails->iAIAnimalCombatModifierAA = 0;
		pCombatDetails->iBarbarianCombatModifierTB = 0;
		pCombatDetails->iAIBarbarianCombatModifierTB = 0;
		pCombatDetails->iBarbarianCombatModifierAB = 0;
		pCombatDetails->iAIBarbarianCombatModifierAB = 0;
		pCombatDetails->iPlotDefenseModifier = 0;
		pCombatDetails->iFortifyModifier = 0;
		pCombatDetails->iCityDefenseModifier = 0;
		pCombatDetails->iHillsAttackModifier = 0;
		pCombatDetails->iHillsDefenseModifier = 0;
		pCombatDetails->iFeatureAttackModifier = 0;
		pCombatDetails->iFeatureDefenseModifier = 0;
		pCombatDetails->iTerrainAttackModifier = 0;
		pCombatDetails->iTerrainDefenseModifier = 0;
		pCombatDetails->iCityAttackModifier = 0;
		pCombatDetails->iDomainDefenseModifier = 0;
		pCombatDetails->iCityBarbarianDefenseModifier = 0;
		pCombatDetails->iClassDefenseModifier = 0;
		pCombatDetails->iClassAttackModifier = 0;
		pCombatDetails->iCombatModifierA = 0;
		pCombatDetails->iCombatModifierT = 0;
		pCombatDetails->iDomainModifierA = 0;
		pCombatDetails->iDomainModifierT = 0;
		pCombatDetails->iAnimalCombatModifierA = 0;
		pCombatDetails->iAnimalCombatModifierT = 0;
		pCombatDetails->iRiverAttackModifier = 0;
		pCombatDetails->iAmphibAttackModifier = 0;
		pCombatDetails->iKamikazeModifier = 0;
		pCombatDetails->iModifierTotal = 0;
		pCombatDetails->iBaseCombatStr = 0;
		pCombatDetails->iCombat = 0;
		pCombatDetails->iMaxCombatStr = 0;
		pCombatDetails->iCurrHitPoints = 0;
		pCombatDetails->iMaxHitPoints = 0;
		pCombatDetails->iCurrCombatStr = 0;
		pCombatDetails->eOwner = getOwnerINLINE();
		pCombatDetails->eVisualOwner = getVisualOwner();
		pCombatDetails->sUnitName = getName().GetCString();
	}
	else if (baseCombatStr() == 0)
	{
		return 0;
	}
	else if ( bSurroundedModifier )
	{
		PROFILE("maxCombatStr.Cachable");

		if ( CombatStrCacheInitializedTurn != GC.getGameINLINE().getGameTurn() )
		{
			memset(CombatStrCache, 0, sizeof(CombatStrCache));

			CombatStrCacheInitializedTurn = GC.getGameINLINE().getGameTurn();
		}

		int	iBestLRU = MAX_INT;

		for(iI = 0; iI < COMBATSTR_CACHE_SIZE; iI++)
		{
			CombatStrCacheEntry* pEntry = &CombatStrCache[iI];

			if ( pEntry->iLRUIndex == 0 )
			{
				pCacheEntry = pEntry;
				break;
			}
			else if ( pEntry->pPlot == pPlot && pEntry->pAttacker == pOriginalAttacker && pEntry->pForUnit == this )
			{
				//OutputDebugString("maxCombatStr.CachHit\n");
				PROFILE("maxCombatStr.CachHit");
				pEntry->iLRUIndex = iNextCombatCacheLRU++;
				return pEntry->iResult;
			}
			else if ( pEntry->iLRUIndex < iBestLRU )
			{
				iBestLRU = pEntry->iLRUIndex;
				pCacheEntry = pEntry;
			}
		}

		//char buffer[300];

		//sprintf(buffer,"maxCombatStr cache miss for unit %d, attacker %d @(%d,%d)\n", getID(), pOriginalAttacker == NULL ? -1 : pOriginalAttacker->getID(), pPlot == NULL ? -1 : pPlot->getX_INLINE(), pPlot == NULL ? -1 : pPlot->getY_INLINE());
		//OutputDebugString(buffer);
	}

	int iModifier = 0;
	int iExtraModifier;
	//TB Combat Mods Begin
	int iExtraModifier2;
	//TB Combat Mods End 

	iExtraModifier = getExtraCombatPercent();
	iModifier += iExtraModifier;
	if (pCombatDetails != NULL)
	{
		pCombatDetails->iExtraCombatPercent = iExtraModifier;
	}
	
	// do modifiers for animals and barbarians (leaving these out for bAttackingUnknownDefender case)
	if (pAttacker != NULL)
	{
		if (isAnimal())
		{
			if (pAttacker->isHuman())
			{
				iExtraModifier = GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getAnimalCombatModifier();
				iModifier += iExtraModifier;
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iAnimalCombatModifierTA = iExtraModifier;
				}
			}
			else
			{
				iExtraModifier = GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getAIAnimalCombatModifier();
				iModifier += iExtraModifier;
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iAIAnimalCombatModifierTA = iExtraModifier;
				}
			}
		}

		if (pAttacker->isAnimal())
		{
			if (isHuman())
			{
				iExtraModifier = -GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getAnimalCombatModifier();
				iModifier += iExtraModifier;
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iAnimalCombatModifierAA = iExtraModifier;
				}
			}
			else
			{
				iExtraModifier = -GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getAIAnimalCombatModifier();
				iModifier += iExtraModifier;
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iAIAnimalCombatModifierAA = iExtraModifier;
				}
			}
		}

		if (isBarbarian())
		{
			//TB Combat Mods Begin

			if (!isAnimal())
			{
				iExtraModifier2 = -pAttacker->vsBarbsModifier();
			}
			else
			{	
				iExtraModifier2 = 0;
			}
			//TB Combat Mods End (The lines that follow also include adjustments with iExtraModifier2)

			if (pAttacker->isHuman())
			{
				iExtraModifier = GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getBarbarianCombatModifier();
				iModifier += (iExtraModifier + iExtraModifier2);
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iBarbarianCombatModifierTB = (iExtraModifier + iExtraModifier2);
				}
			}
			else
			{
				iExtraModifier = GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getAIBarbarianCombatModifier();
				iModifier += (iExtraModifier + iExtraModifier2);
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iAIBarbarianCombatModifierTB = (iExtraModifier + iExtraModifier2);
				}
			}
		}

		if (pAttacker->isBarbarian())
		{
			//TB Combat Mods Begin
			if (!(pAttacker->isAnimal()))
			{
				iExtraModifier2 = vsBarbsModifier();
			}
			else
			{	
				iExtraModifier2 = 0;
			}
			//TB Combat Mods End (The following lines include references to iExtraModifier2 which is also a portion of this mod)

			if (isHuman())
			{
				iExtraModifier = -GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getBarbarianCombatModifier();
				iModifier += (iExtraModifier + iExtraModifier2);
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iBarbarianCombatModifierAB = (iExtraModifier + iExtraModifier2);
				}
			}
			else
			{
				iExtraModifier = -GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getAIBarbarianCombatModifier();
				iModifier += (iExtraModifier + iExtraModifier2);
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iAIBarbarianCombatModifierTB = (iExtraModifier + iExtraModifier2);
				}
			}
		}
	}
	
	// add defensive bonuses (leaving these out for bAttackingUnknownDefender case)
	if (pPlot != NULL)
	{
		if (!noDefensiveBonus())
		{
/************************************************************************************************/
/* BETTER_BTS_AI_MOD                      03/30/10                                jdog5000      */
/*                                                                                              */
/* General AI                                                                                   */
/************************************************************************************************/
			// When pAttacker is NULL but pPlot is not, this is a computation for this units defensive value
			// against an unknown attacker.  Always ignoring building defense in this case is a conservative estimate,
			// but causes AI to suicide against castle walls of low culture cities in early game.  Using this units
			// ignoreBuildingDefense does a little better ... in early game it corrects undervalue of castles.  One
			// downside is when medieval unit is defending a walled city against gunpowder.  Here, the over value
			// makes attacker a little more cautious, but with their tech lead it shouldn't matter too much.  Also
			// makes vulnerable units (ships, etc) feel safer in this case and potentially not leave, but ships
			// leave when ratio is pretty low anyway.

			//iExtraModifier = pPlot->defenseModifier(getTeam(), (pAttacker != NULL) ? pAttacker->ignoreBuildingDefense() : true);
			iExtraModifier = pPlot->defenseModifier(getTeam(), (pAttacker != NULL) ? pAttacker->ignoreBuildingDefense() : ignoreBuildingDefense());
/************************************************************************************************/
/* BETTER_BTS_AI_MOD                       END                                                  */
/************************************************************************************************/
			iModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iPlotDefenseModifier = iExtraModifier;
			}
		}
		//TB Combat Mods (fortification)
		int iFort = fortifyModifier();
		int iOverrun = ((pAttacker != NULL) ? pAttacker->overrunTotal() : 0);
		int iOverrunzero = ((iOverrun < 0) ? 0 : iOverrun);
		int iOverruntotal = ((iOverrunzero > 100) ? 100 : iOverrunzero);
		int iFortModTotal = ((iFort * (100 - iOverruntotal))/100);
		
		iExtraModifier = iFortModTotal;
		iModifier += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iFortifyModifier = iExtraModifier;
		}

		if (pPlot->isCity(true, getTeam()))
		{
			iExtraModifier = cityDefenseModifier();

/************************************************************************************************/
/* Afforess	                  Start		 05/22/10                                               */
/*                                                                                              */
/*                                                                                              */
/************************************************************************************************/
			if (pPlot->isCity(false))
			{
				iExtraModifier += 0;
				//TB SubCombat Mod Begin
				for (iI = 0; iI < GC.getNumUnitCombatInfos(); iI++)
				{
					if (hasCombatType((UnitCombatTypes)iI))
					{
						eUnitCombatType = ((UnitCombatTypes)iI);
						iExtraModifier += pPlot->getPlotCity()->getUnitCombatExtraStrength(eUnitCombatType);
					}
				}
				//TB SubCombat Mod End
			}
/************************************************************************************************/
/* Afforess	                     END                                                            */
/************************************************************************************************/
			iModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iCityDefenseModifier = iExtraModifier;
			}

		}
		
		if (pPlot->isHills())
		{
			iExtraModifier = hillsDefenseModifier();
			iModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iHillsDefenseModifier = iExtraModifier;
			}
		}

		if (pPlot->getFeatureType() != NO_FEATURE)
		{
			iExtraModifier = featureDefenseModifier(pPlot->getFeatureType());
			iModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iFeatureDefenseModifier = iExtraModifier;
			}
		}

		iExtraModifier = terrainDefenseModifier(pPlot->getTerrainType());
		iModifier += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iTerrainDefenseModifier = iExtraModifier;
		}
	}

	//TB note: this was further below, underneath the Attacker = this clause.  The next if line was necessary to isolate these still, but hopefully this can debug the invisibility issue without creating a further problem.
	int iTempModifier2 = 0;
	if (pAttacker != NULL && pAttackedPlot != NULL)
	{
		iExtraModifier = unitClassDefenseModifier(pAttacker->getUnitClassType());
		iTempModifier2 += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iClassDefenseModifier = iExtraModifier;
		}

		iExtraModifier = -pAttacker->unitClassAttackModifier(getUnitClassType());
		iTempModifier2 += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iClassAttackModifier = iExtraModifier;
		}

		iExtraModifier = 0;
		//TB SubCombat Mod Begin
		for (iI = 0; iI < GC.getNumUnitCombatInfos(); iI++)
		{
			if (pAttacker->hasCombatType((UnitCombatTypes)iI))
			{
				eUnitCombatType = ((UnitCombatTypes)iI);
				iExtraModifier += unitCombatModifier(eUnitCombatType);
			}
		}

		iTempModifier2 += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iCombatModifierA = iExtraModifier;
		}

		iExtraModifier = 0;
		//TB SubCombat Mod Begin
		for (iI = 0; iI < GC.getNumUnitCombatInfos(); iI++)
		{
			if (hasCombatType((UnitCombatTypes)iI))
			{
				eUnitCombatType = ((UnitCombatTypes)iI);
				iExtraModifier -= pAttacker->unitCombatModifier(eUnitCombatType);
			}
		}
		//TB SubCombat Mod End
		iTempModifier2 += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iCombatModifierT = iExtraModifier;
		}

		iExtraModifier = domainModifier(pAttacker->getDomainType());
		iTempModifier2 += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iDomainModifierA = iExtraModifier;
		}

		iExtraModifier = -pAttacker->domainModifier(getDomainType());
		iTempModifier2 += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iDomainModifierT = iExtraModifier;
		}

		if (pAttacker->isAnimal())
		{
			iExtraModifier = animalCombatModifier();
			iTempModifier2 += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iAnimalCombatModifierA = iExtraModifier;
			}
		}

		if (isAnimal())
		{
			iExtraModifier = -pAttacker->animalCombatModifier();
			iTempModifier2 += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iAnimalCombatModifierT = iExtraModifier;
			}
		}
	}
	// if we are attacking to an plot with an unknown defender, the calc the modifier in reverse
	if (bAttackingUnknownDefender)
	{
		pAttacker = this;
	}

	// calc attacker bonueses
/************************************************************************************************/
/* UNOFFICIAL_PATCH                       09/20/09                                jdog5000      */
/*                                                                                              */
/* Bugfix                                                                                       */
/************************************************************************************************/
/* original code
	if (pAttacker != NULL)
*/
	if (pAttacker != NULL && pAttackedPlot != NULL)
/************************************************************************************************/
/* UNOFFICIAL_PATCH                        END                                                  */
/************************************************************************************************/
	{
		int iTempModifier = 0;		

		//TB note: part of above debug effort
		iTempModifier += iTempModifier2;
		if (pAttackedPlot->isCity(true, getTeam()))
		{
			iExtraModifier = -pAttacker->cityAttackModifier();
			iTempModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iCityAttackModifier = iExtraModifier;
			}

			if (pAttacker->isBarbarian())
			{
				iExtraModifier = GC.getDefineINT("CITY_BARBARIAN_DEFENSE_MODIFIER");
				iTempModifier += iExtraModifier;
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iCityBarbarianDefenseModifier = iExtraModifier;
				}
			}
		}

		if (pAttacker->attackCombatModifierTotal() > 0)
		{
			iExtraModifier = -pAttacker->attackCombatModifierTotal();
			iTempModifier += iExtraModifier;
		}

		if (defenseCombatModifierTotal() > 0)
		{
			iExtraModifier = defenseCombatModifierTotal();
			iTempModifier += iExtraModifier;
		}

		if (pAttackedPlot->isHills())
		{
			iExtraModifier = -pAttacker->hillsAttackModifier();
			iTempModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iHillsAttackModifier = iExtraModifier;
			}
		}
		
		if (pAttackedPlot->getFeatureType() != NO_FEATURE)
		{
			iExtraModifier = -pAttacker->featureAttackModifier(pAttackedPlot->getFeatureType());
			iTempModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iFeatureAttackModifier = iExtraModifier;
			}
		}
		else
		{
			iExtraModifier = -pAttacker->terrainAttackModifier(pAttackedPlot->getTerrainType());
			/*** Dexy - Others' bug fixes START ****/
			iTempModifier += iExtraModifier;
			// OLD CODE
			// iModifier += iExtraModifier;
			/*** Dexy - Others' bug fixes  END  ****/
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iTerrainAttackModifier = iExtraModifier;
			}
		}

		// only compute comparisions if we are the defender with a known attacker
		// TB Debug: this exclusion appears to be fairly unnecessary and causes trouble for Assassins and such.  The intention of it seems to be subverted in C2C in general.
		// Thus we'll test disabling this exclusion entirely.
		//if (!bAttackingUnknownDefender)
		//{
		//	FAssertMsg(pAttacker != this, "pAttacker is not expected to be equal with this");
		//  Attempting to move the following above the Attacker = this clause.
			//iExtraModifier = unitClassDefenseModifier(pAttacker->getUnitClassType());
			//iTempModifier += iExtraModifier;
			//if (pCombatDetails != NULL)
			//{
			//	pCombatDetails->iClassDefenseModifier = iExtraModifier;
			//}

			//iExtraModifier = -pAttacker->unitClassAttackModifier(getUnitClassType());
			//iTempModifier += iExtraModifier;
			//if (pCombatDetails != NULL)
			//{
			//	pCombatDetails->iClassAttackModifier = iExtraModifier;
			//}

			//iExtraModifier = 0;
			////TB SubCombat Mod Begin
			//for (iI = 0; iI < GC.getNumUnitCombatInfos(); iI++)
			//{
			//	if (pAttacker->hasCombatType((UnitCombatTypes)iI))
			//	{
			//		eUnitCombatType = ((UnitCombatTypes)iI);
			//		iExtraModifier += unitCombatModifier(eUnitCombatType);
			//	}
			//}

			//iTempModifier += iExtraModifier;
			//if (pCombatDetails != NULL)
			//{
			//	pCombatDetails->iCombatModifierA = iExtraModifier;
			//}

			//iExtraModifier = 0;
			////TB SubCombat Mod Begin
			//for (iI = 0; iI < GC.getNumUnitCombatInfos(); iI++)
			//{
			//	if (hasCombatType((UnitCombatTypes)iI))
			//	{
			//		eUnitCombatType = ((UnitCombatTypes)iI);
			//		iExtraModifier -= pAttacker->unitCombatModifier(eUnitCombatType);
			//	}
			//}
			////TB SubCombat Mod End
			//iTempModifier += iExtraModifier;
			//if (pCombatDetails != NULL)
			//{
			//	pCombatDetails->iCombatModifierT = iExtraModifier;
			//}

			//iExtraModifier = domainModifier(pAttacker->getDomainType());
			//iTempModifier += iExtraModifier;
			//if (pCombatDetails != NULL)
			//{
			//	pCombatDetails->iDomainModifierA = iExtraModifier;
			//}

			//iExtraModifier = -pAttacker->domainModifier(getDomainType());
			//iTempModifier += iExtraModifier;
			//if (pCombatDetails != NULL)
			//{
			//	pCombatDetails->iDomainModifierT = iExtraModifier;
			//}

			//if (pAttacker->isAnimal())
			//{
			//	iExtraModifier = animalCombatModifier();
			//	iTempModifier += iExtraModifier;
			//	if (pCombatDetails != NULL)
			//	{
			//		pCombatDetails->iAnimalCombatModifierA = iExtraModifier;
			//	}
			//}

			//if (isAnimal())
			//{
			//	iExtraModifier = -pAttacker->animalCombatModifier();
			//	iTempModifier += iExtraModifier;
			//	if (pCombatDetails != NULL)
			//	{
			//		pCombatDetails->iAnimalCombatModifierT = iExtraModifier;
			//	}
			//}
		//}

		if (!(pAttacker->isRiver()))
		{
			if (pAttacker->plot()->isRiverCrossing(directionXY(pAttacker->plot(), pAttackedPlot)))
			{
				iExtraModifier = -GC.getRIVER_ATTACK_MODIFIER();
				iTempModifier += iExtraModifier;
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iRiverAttackModifier = iExtraModifier;
				}
			}
		}

		if (!(pAttacker->isAmphib()))
		{
			if (!(pAttackedPlot->isWater()) && pAttacker->plot()->isWater())
			{
				iExtraModifier = -GC.getAMPHIB_ATTACK_MODIFIER();
				iTempModifier += iExtraModifier;
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iAmphibAttackModifier = iExtraModifier;
				}
			}
		}
		
/************************************************************************************************/
/* Afforess	                  Start		 02/05/10                                               */
/*                                                                                              */
/*                                                                                              */
/************************************************************************************************/
		if (GET_PLAYER(pAttacker->getOwnerINLINE()).isDarkAge())
		{
			iTempModifier += GC.getDefineINT("DARK_AGE_ATTACK_STRENGTH_PERCENT_DECREASE");
		}
/************************************************************************************************/
/* Afforess	                     END                                                            */
/************************************************************************************************/

		if (pAttacker->getKamikazePercent() != 0)
		{
/************************************************************************************************/
/* Afforess	                  Start		 02/05/10                                    Dexy       */
/*                                                                                              */
/*       Unofficial Patch                                                                       */
/************************************************************************************************/
			iExtraModifier = -pAttacker->getKamikazePercent();
/************************************************************************************************/
/* Afforess	                     END                                                            */
/************************************************************************************************/
			iTempModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iKamikazeModifier = iExtraModifier;
			}
		}
		
/************************************************************************************************/
/* Afforess	                  Start		 02/05/10                                               */
/*                                                                                              */
/*                                                                                              */
/************************************************************************************************/
		if (bSurroundedModifier)
		{
			// the stronger the surroundings -> decrease the iModifier more
			//TB Combat Mods (S&D promos) begin
			int iSurround = pAttacker->surroundedDefenseModifier(pAttackedPlot, bAttackingUnknownDefender ? NULL : this);
			int iDynamicDefense = dynamicDefenseTotal();
			int iDynamicDefenseModifier = ((iSurround * iDynamicDefense) / 100);
			int iSurroundTotalstepone = iSurround - iDynamicDefenseModifier;
			int iSurroundTotal = (iSurroundTotalstepone < 0 ? 0 : iSurroundTotalstepone);
			iExtraModifier = -iSurroundTotal;
			iTempModifier += iExtraModifier;
			//TB Combat Mods (S&D promos) end
		}
/************************************************************************************************/
/* Afforess	                     END                                                            */
/************************************************************************************************/
	
		//TB Combat Mods (StrAdjperAtt) begin
		if (pAttacker->currentStrAdjperAttTotal() != 0)
		{
			iExtraModifier = -(pAttacker->currentStrAdjperAttTotal());
			iTempModifier += iExtraModifier;
		}

		if (currentStrAdjperDefTotal() != 0)
		{
			iExtraModifier = currentStrAdjperDefTotal();
			iTempModifier += iExtraModifier;
		}
		//TB Combat Mods (StrAdjperAtt) end
		// if we are attacking an unknown defender, then use the reverse of the modifier
		if (bAttackingUnknownDefender)
		{
			iModifier -= iTempModifier;
		}
		else
		{
			iModifier += iTempModifier;
		}
	}

	if (pCombatDetails != NULL)
	{
		pCombatDetails->iModifierTotal = iModifier;
		pCombatDetails->iBaseCombatStr = baseCombatStr();
	}

	if (iModifier > 0)
	{
		iCombat = (baseCombatStr() * (iModifier + 100));
	}
	else
	{
		iCombat = ((baseCombatStr() * 10000) / (100 - iModifier));
	}

	if (pCombatDetails != NULL)
	{
		pCombatDetails->iCombat = iCombat;
		pCombatDetails->iMaxCombatStr = std::max(1, iCombat);
		pCombatDetails->iCurrHitPoints = currHitPoints();
		pCombatDetails->iMaxHitPoints = maxHitPoints();
		pCombatDetails->iCurrCombatStr = ((pCombatDetails->iMaxCombatStr * pCombatDetails->iCurrHitPoints) / pCombatDetails->iMaxHitPoints);
	}

	if ( pCacheEntry != NULL )
	{
		pCacheEntry->iLRUIndex = iNextCombatCacheLRU++;
		pCacheEntry->iResult = std::max(1, iCombat);
		pCacheEntry->pPlot = pPlot;
		pCacheEntry->pAttacker = pOriginalAttacker;
		pCacheEntry->pForUnit = this;

		//char buffer[300];

		//sprintf(buffer,"maxCombatStr cache result (%d) for unit %d, attacker %d @(%d,%d)\n", pCacheEntry->iResult, getID(), pOriginalAttacker == NULL ? -1 : pOriginalAttacker->getID(), pPlot == NULL ? -1 : pPlot->getX_INLINE(), pPlot == NULL ? -1 : pPlot->getY_INLINE());
		//OutputDebugString(buffer);
	}

	return std::max(1, iCombat);
}

You haven't updated anything in CvUnit yet so I didn't want to foul things up with an SVN update here.

I don't believe I have a test save for replicating the combat modifier loss where the attacker is invisible but theoretically I believe this solution solves things. The changes in the above begins with:

//TB note: this was further below, underneath the Attacker = this clause. The next if line was necessary to isolate these still, but hopefully this can debug the invisibility issue without creating a further problem.

Oh, and I think the Bombard Bug is resolved.

AIAndy · Mar 26, 2013

strategyonly said:
I wonder IF the invisible forest also has SOMETHING to do with this??

Invisible forests means that something is still generating them as secondary features which should not happen.

Dancing Hoskuld · Mar 26, 2013

As far as I know the only things that generate forest are growth and the terraform improvements.

MaXimillionZero · Mar 26, 2013

Thunderbrd said:
The first error in my thinking was that I'm not sure when open borders ever became a prerequisite for trading (this never USED to be this way! In Vanilla at least... is this a new C2C rule then?)

Open borders are a pre-requisite of having trade routes, but not of trading resources, at least in BtS. Although if you're playing C2C with advanced diplomacy you should only need right of passage or whatever it's called, not fully open borders.

strategyonly · Mar 26, 2013

Dancing Hoskuld said:
As far as I know the only things that generate forest are growth and the terraform improvements.

When it popped-up it did say, A new forest had been formed (or words related to that).

Koshling · Mar 26, 2013

Thunderbrd said:

@Koshling: I looked a bit further into the issue with CvUnit. This is my suggested change which, if I'm right, should address both the issue you noted about the self evaluation (which a little research above the initial tweak does reveal the cause for) and the loss of some combat modifiers.

Spoiler :

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;
	//TB SubCombat Mod Begin
	int iI;
	UnitCombatTypes eUnitCombatType;
	//TB SubCombat Mod End

	FAssertMsg((pPlot == NULL) || (pPlot->getTerrainType() != NO_TERRAIN), "(pPlot == NULL) || (pPlot->getTerrainType() is not expected to be equal with NO_TERRAIN)");
	
	// handle our new special case
	const	CvPlot*	pAttackedPlot = NULL;
	bool	bAttackingUnknownDefender = false;
	if (pAttacker == this)
	{
		bAttackingUnknownDefender = true;
		pAttackedPlot = pPlot;
		
		// reset these values, we will fiddle with them below
		pPlot = NULL;
		pAttacker = NULL;
	}
	// otherwise, attack plot is the plot of us (the defender)
	else if (pAttacker != NULL)
	{
		pAttackedPlot = plot();
	}

	CombatStrCacheEntry* pCacheEntry = NULL;
	const CvUnit* pOriginalAttacker = pAttacker;

	if (pCombatDetails != NULL)
	{
		pCombatDetails->iExtraCombatPercent = 0;
		pCombatDetails->iAnimalCombatModifierTA = 0;
		pCombatDetails->iAIAnimalCombatModifierTA = 0;
		pCombatDetails->iAnimalCombatModifierAA = 0;
		pCombatDetails->iAIAnimalCombatModifierAA = 0;
		pCombatDetails->iBarbarianCombatModifierTB = 0;
		pCombatDetails->iAIBarbarianCombatModifierTB = 0;
		pCombatDetails->iBarbarianCombatModifierAB = 0;
		pCombatDetails->iAIBarbarianCombatModifierAB = 0;
		pCombatDetails->iPlotDefenseModifier = 0;
		pCombatDetails->iFortifyModifier = 0;
		pCombatDetails->iCityDefenseModifier = 0;
		pCombatDetails->iHillsAttackModifier = 0;
		pCombatDetails->iHillsDefenseModifier = 0;
		pCombatDetails->iFeatureAttackModifier = 0;
		pCombatDetails->iFeatureDefenseModifier = 0;
		pCombatDetails->iTerrainAttackModifier = 0;
		pCombatDetails->iTerrainDefenseModifier = 0;
		pCombatDetails->iCityAttackModifier = 0;
		pCombatDetails->iDomainDefenseModifier = 0;
		pCombatDetails->iCityBarbarianDefenseModifier = 0;
		pCombatDetails->iClassDefenseModifier = 0;
		pCombatDetails->iClassAttackModifier = 0;
		pCombatDetails->iCombatModifierA = 0;
		pCombatDetails->iCombatModifierT = 0;
		pCombatDetails->iDomainModifierA = 0;
		pCombatDetails->iDomainModifierT = 0;
		pCombatDetails->iAnimalCombatModifierA = 0;
		pCombatDetails->iAnimalCombatModifierT = 0;
		pCombatDetails->iRiverAttackModifier = 0;
		pCombatDetails->iAmphibAttackModifier = 0;
		pCombatDetails->iKamikazeModifier = 0;
		pCombatDetails->iModifierTotal = 0;
		pCombatDetails->iBaseCombatStr = 0;
		pCombatDetails->iCombat = 0;
		pCombatDetails->iMaxCombatStr = 0;
		pCombatDetails->iCurrHitPoints = 0;
		pCombatDetails->iMaxHitPoints = 0;
		pCombatDetails->iCurrCombatStr = 0;
		pCombatDetails->eOwner = getOwnerINLINE();
		pCombatDetails->eVisualOwner = getVisualOwner();
		pCombatDetails->sUnitName = getName().GetCString();
	}
	else if (baseCombatStr() == 0)
	{
		return 0;
	}
	else if ( bSurroundedModifier )
	{
		PROFILE("maxCombatStr.Cachable");

		if ( CombatStrCacheInitializedTurn != GC.getGameINLINE().getGameTurn() )
		{
			memset(CombatStrCache, 0, sizeof(CombatStrCache));

			CombatStrCacheInitializedTurn = GC.getGameINLINE().getGameTurn();
		}

		int	iBestLRU = MAX_INT;

		for(iI = 0; iI < COMBATSTR_CACHE_SIZE; iI++)
		{
			CombatStrCacheEntry* pEntry = &CombatStrCache[iI];

			if ( pEntry->iLRUIndex == 0 )
			{
				pCacheEntry = pEntry;
				break;
			}
			else if ( pEntry->pPlot == pPlot && pEntry->pAttacker == pOriginalAttacker && pEntry->pForUnit == this )
			{
				//OutputDebugString("maxCombatStr.CachHit\n");
				PROFILE("maxCombatStr.CachHit");
				pEntry->iLRUIndex = iNextCombatCacheLRU++;
				return pEntry->iResult;
			}
			else if ( pEntry->iLRUIndex < iBestLRU )
			{
				iBestLRU = pEntry->iLRUIndex;
				pCacheEntry = pEntry;
			}
		}

		//char buffer[300];

		//sprintf(buffer,"maxCombatStr cache miss for unit %d, attacker %d @(%d,%d)\n", getID(), pOriginalAttacker == NULL ? -1 : pOriginalAttacker->getID(), pPlot == NULL ? -1 : pPlot->getX_INLINE(), pPlot == NULL ? -1 : pPlot->getY_INLINE());
		//OutputDebugString(buffer);
	}

	int iModifier = 0;
	int iExtraModifier;
	//TB Combat Mods Begin
	int iExtraModifier2;
	//TB Combat Mods End 

	iExtraModifier = getExtraCombatPercent();
	iModifier += iExtraModifier;
	if (pCombatDetails != NULL)
	{
		pCombatDetails->iExtraCombatPercent = iExtraModifier;
	}
	
	// do modifiers for animals and barbarians (leaving these out for bAttackingUnknownDefender case)
	if (pAttacker != NULL)
	{
		if (isAnimal())
		{
			if (pAttacker->isHuman())
			{
				iExtraModifier = GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getAnimalCombatModifier();
				iModifier += iExtraModifier;
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iAnimalCombatModifierTA = iExtraModifier;
				}
			}
			else
			{
				iExtraModifier = GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getAIAnimalCombatModifier();
				iModifier += iExtraModifier;
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iAIAnimalCombatModifierTA = iExtraModifier;
				}
			}
		}

		if (pAttacker->isAnimal())
		{
			if (isHuman())
			{
				iExtraModifier = -GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getAnimalCombatModifier();
				iModifier += iExtraModifier;
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iAnimalCombatModifierAA = iExtraModifier;
				}
			}
			else
			{
				iExtraModifier = -GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getAIAnimalCombatModifier();
				iModifier += iExtraModifier;
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iAIAnimalCombatModifierAA = iExtraModifier;
				}
			}
		}

		if (isBarbarian())
		{
			//TB Combat Mods Begin

			if (!isAnimal())
			{
				iExtraModifier2 = -pAttacker->vsBarbsModifier();
			}
			else
			{	
				iExtraModifier2 = 0;
			}
			//TB Combat Mods End (The lines that follow also include adjustments with iExtraModifier2)

			if (pAttacker->isHuman())
			{
				iExtraModifier = GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getBarbarianCombatModifier();
				iModifier += (iExtraModifier + iExtraModifier2);
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iBarbarianCombatModifierTB = (iExtraModifier + iExtraModifier2);
				}
			}
			else
			{
				iExtraModifier = GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getAIBarbarianCombatModifier();
				iModifier += (iExtraModifier + iExtraModifier2);
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iAIBarbarianCombatModifierTB = (iExtraModifier + iExtraModifier2);
				}
			}
		}

		if (pAttacker->isBarbarian())
		{
			//TB Combat Mods Begin
			if (!(pAttacker->isAnimal()))
			{
				iExtraModifier2 = vsBarbsModifier();
			}
			else
			{	
				iExtraModifier2 = 0;
			}
			//TB Combat Mods End (The following lines include references to iExtraModifier2 which is also a portion of this mod)

			if (isHuman())
			{
				iExtraModifier = -GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getBarbarianCombatModifier();
				iModifier += (iExtraModifier + iExtraModifier2);
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iBarbarianCombatModifierAB = (iExtraModifier + iExtraModifier2);
				}
			}
			else
			{
				iExtraModifier = -GC.getHandicapInfo(GC.getGameINLINE().getHandicapType()).getAIBarbarianCombatModifier();
				iModifier += (iExtraModifier + iExtraModifier2);
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iAIBarbarianCombatModifierTB = (iExtraModifier + iExtraModifier2);
				}
			}
		}
	}
	
	// add defensive bonuses (leaving these out for bAttackingUnknownDefender case)
	if (pPlot != NULL)
	{
		if (!noDefensiveBonus())
		{
/************************************************************************************************/
/* BETTER_BTS_AI_MOD                      03/30/10                                jdog5000      */
/*                                                                                              */
/* General AI                                                                                   */
/************************************************************************************************/
			// When pAttacker is NULL but pPlot is not, this is a computation for this units defensive value
			// against an unknown attacker.  Always ignoring building defense in this case is a conservative estimate,
			// but causes AI to suicide against castle walls of low culture cities in early game.  Using this units
			// ignoreBuildingDefense does a little better ... in early game it corrects undervalue of castles.  One
			// downside is when medieval unit is defending a walled city against gunpowder.  Here, the over value
			// makes attacker a little more cautious, but with their tech lead it shouldn't matter too much.  Also
			// makes vulnerable units (ships, etc) feel safer in this case and potentially not leave, but ships
			// leave when ratio is pretty low anyway.

			//iExtraModifier = pPlot->defenseModifier(getTeam(), (pAttacker != NULL) ? pAttacker->ignoreBuildingDefense() : true);
			iExtraModifier = pPlot->defenseModifier(getTeam(), (pAttacker != NULL) ? pAttacker->ignoreBuildingDefense() : ignoreBuildingDefense());
/************************************************************************************************/
/* BETTER_BTS_AI_MOD                       END                                                  */
/************************************************************************************************/
			iModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iPlotDefenseModifier = iExtraModifier;
			}
		}
		//TB Combat Mods (fortification)
		int iFort = fortifyModifier();
		int iOverrun = ((pAttacker != NULL) ? pAttacker->overrunTotal() : 0);
		int iOverrunzero = ((iOverrun < 0) ? 0 : iOverrun);
		int iOverruntotal = ((iOverrunzero > 100) ? 100 : iOverrunzero);
		int iFortModTotal = ((iFort * (100 - iOverruntotal))/100);
		
		iExtraModifier = iFortModTotal;
		iModifier += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iFortifyModifier = iExtraModifier;
		}

		if (pPlot->isCity(true, getTeam()))
		{
			iExtraModifier = cityDefenseModifier();

/************************************************************************************************/
/* Afforess	                  Start		 05/22/10                                               */
/*                                                                                              */
/*                                                                                              */
/************************************************************************************************/
			if (pPlot->isCity(false))
			{
				iExtraModifier += 0;
				//TB SubCombat Mod Begin
				for (iI = 0; iI < GC.getNumUnitCombatInfos(); iI++)
				{
					if (hasCombatType((UnitCombatTypes)iI))
					{
						eUnitCombatType = ((UnitCombatTypes)iI);
						iExtraModifier += pPlot->getPlotCity()->getUnitCombatExtraStrength(eUnitCombatType);
					}
				}
				//TB SubCombat Mod End
			}
/************************************************************************************************/
/* Afforess	                     END                                                            */
/************************************************************************************************/
			iModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iCityDefenseModifier = iExtraModifier;
			}

		}
		
		if (pPlot->isHills())
		{
			iExtraModifier = hillsDefenseModifier();
			iModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iHillsDefenseModifier = iExtraModifier;
			}
		}

		if (pPlot->getFeatureType() != NO_FEATURE)
		{
			iExtraModifier = featureDefenseModifier(pPlot->getFeatureType());
			iModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iFeatureDefenseModifier = iExtraModifier;
			}
		}

		iExtraModifier = terrainDefenseModifier(pPlot->getTerrainType());
		iModifier += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iTerrainDefenseModifier = iExtraModifier;
		}
	}

	//TB note: this was further below, underneath the Attacker = this clause.  The next if line was necessary to isolate these still, but hopefully this can debug the invisibility issue without creating a further problem.
	int iTempModifier2 = 0;
	if (pAttacker != NULL && pAttackedPlot != NULL)
	{
		iExtraModifier = unitClassDefenseModifier(pAttacker->getUnitClassType());
		iTempModifier2 += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iClassDefenseModifier = iExtraModifier;
		}

		iExtraModifier = -pAttacker->unitClassAttackModifier(getUnitClassType());
		iTempModifier2 += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iClassAttackModifier = iExtraModifier;
		}

		iExtraModifier = 0;
		//TB SubCombat Mod Begin
		for (iI = 0; iI < GC.getNumUnitCombatInfos(); iI++)
		{
			if (pAttacker->hasCombatType((UnitCombatTypes)iI))
			{
				eUnitCombatType = ((UnitCombatTypes)iI);
				iExtraModifier += unitCombatModifier(eUnitCombatType);
			}
		}

		iTempModifier2 += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iCombatModifierA = iExtraModifier;
		}

		iExtraModifier = 0;
		//TB SubCombat Mod Begin
		for (iI = 0; iI < GC.getNumUnitCombatInfos(); iI++)
		{
			if (hasCombatType((UnitCombatTypes)iI))
			{
				eUnitCombatType = ((UnitCombatTypes)iI);
				iExtraModifier -= pAttacker->unitCombatModifier(eUnitCombatType);
			}
		}
		//TB SubCombat Mod End
		iTempModifier2 += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iCombatModifierT = iExtraModifier;
		}

		iExtraModifier = domainModifier(pAttacker->getDomainType());
		iTempModifier2 += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iDomainModifierA = iExtraModifier;
		}

		iExtraModifier = -pAttacker->domainModifier(getDomainType());
		iTempModifier2 += iExtraModifier;
		if (pCombatDetails != NULL)
		{
			pCombatDetails->iDomainModifierT = iExtraModifier;
		}

		if (pAttacker->isAnimal())
		{
			iExtraModifier = animalCombatModifier();
			iTempModifier2 += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iAnimalCombatModifierA = iExtraModifier;
			}
		}

		if (isAnimal())
		{
			iExtraModifier = -pAttacker->animalCombatModifier();
			iTempModifier2 += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iAnimalCombatModifierT = iExtraModifier;
			}
		}
	}
	// if we are attacking to an plot with an unknown defender, the calc the modifier in reverse
	if (bAttackingUnknownDefender)
	{
		pAttacker = this;
	}

	// calc attacker bonueses
/************************************************************************************************/
/* UNOFFICIAL_PATCH                       09/20/09                                jdog5000      */
/*                                                                                              */
/* Bugfix                                                                                       */
/************************************************************************************************/
/* original code
	if (pAttacker != NULL)
*/
	if (pAttacker != NULL && pAttackedPlot != NULL)
/************************************************************************************************/
/* UNOFFICIAL_PATCH                        END                                                  */
/************************************************************************************************/
	{
		int iTempModifier = 0;		

		//TB note: part of above debug effort
		iTempModifier += iTempModifier2;
		if (pAttackedPlot->isCity(true, getTeam()))
		{
			iExtraModifier = -pAttacker->cityAttackModifier();
			iTempModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iCityAttackModifier = iExtraModifier;
			}

			if (pAttacker->isBarbarian())
			{
				iExtraModifier = GC.getDefineINT("CITY_BARBARIAN_DEFENSE_MODIFIER");
				iTempModifier += iExtraModifier;
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iCityBarbarianDefenseModifier = iExtraModifier;
				}
			}
		}

		if (pAttacker->attackCombatModifierTotal() > 0)
		{
			iExtraModifier = -pAttacker->attackCombatModifierTotal();
			iTempModifier += iExtraModifier;
		}

		if (defenseCombatModifierTotal() > 0)
		{
			iExtraModifier = defenseCombatModifierTotal();
			iTempModifier += iExtraModifier;
		}

		if (pAttackedPlot->isHills())
		{
			iExtraModifier = -pAttacker->hillsAttackModifier();
			iTempModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iHillsAttackModifier = iExtraModifier;
			}
		}
		
		if (pAttackedPlot->getFeatureType() != NO_FEATURE)
		{
			iExtraModifier = -pAttacker->featureAttackModifier(pAttackedPlot->getFeatureType());
			iTempModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iFeatureAttackModifier = iExtraModifier;
			}
		}
		else
		{
			iExtraModifier = -pAttacker->terrainAttackModifier(pAttackedPlot->getTerrainType());
			/*** Dexy - Others' bug fixes START ****/
			iTempModifier += iExtraModifier;
			// OLD CODE
			// iModifier += iExtraModifier;
			/*** Dexy - Others' bug fixes  END  ****/
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iTerrainAttackModifier = iExtraModifier;
			}
		}

		// only compute comparisions if we are the defender with a known attacker
		// TB Debug: this exclusion appears to be fairly unnecessary and causes trouble for Assassins and such.  The intention of it seems to be subverted in C2C in general.
		// Thus we'll test disabling this exclusion entirely.
		//if (!bAttackingUnknownDefender)
		//{
		//	FAssertMsg(pAttacker != this, "pAttacker is not expected to be equal with this");
		//  Attempting to move the following above the Attacker = this clause.
			//iExtraModifier = unitClassDefenseModifier(pAttacker->getUnitClassType());
			//iTempModifier += iExtraModifier;
			//if (pCombatDetails != NULL)
			//{
			//	pCombatDetails->iClassDefenseModifier = iExtraModifier;
			//}

			//iExtraModifier = -pAttacker->unitClassAttackModifier(getUnitClassType());
			//iTempModifier += iExtraModifier;
			//if (pCombatDetails != NULL)
			//{
			//	pCombatDetails->iClassAttackModifier = iExtraModifier;
			//}

			//iExtraModifier = 0;
			////TB SubCombat Mod Begin
			//for (iI = 0; iI < GC.getNumUnitCombatInfos(); iI++)
			//{
			//	if (pAttacker->hasCombatType((UnitCombatTypes)iI))
			//	{
			//		eUnitCombatType = ((UnitCombatTypes)iI);
			//		iExtraModifier += unitCombatModifier(eUnitCombatType);
			//	}
			//}

			//iTempModifier += iExtraModifier;
			//if (pCombatDetails != NULL)
			//{
			//	pCombatDetails->iCombatModifierA = iExtraModifier;
			//}

			//iExtraModifier = 0;
			////TB SubCombat Mod Begin
			//for (iI = 0; iI < GC.getNumUnitCombatInfos(); iI++)
			//{
			//	if (hasCombatType((UnitCombatTypes)iI))
			//	{
			//		eUnitCombatType = ((UnitCombatTypes)iI);
			//		iExtraModifier -= pAttacker->unitCombatModifier(eUnitCombatType);
			//	}
			//}
			////TB SubCombat Mod End
			//iTempModifier += iExtraModifier;
			//if (pCombatDetails != NULL)
			//{
			//	pCombatDetails->iCombatModifierT = iExtraModifier;
			//}

			//iExtraModifier = domainModifier(pAttacker->getDomainType());
			//iTempModifier += iExtraModifier;
			//if (pCombatDetails != NULL)
			//{
			//	pCombatDetails->iDomainModifierA = iExtraModifier;
			//}

			//iExtraModifier = -pAttacker->domainModifier(getDomainType());
			//iTempModifier += iExtraModifier;
			//if (pCombatDetails != NULL)
			//{
			//	pCombatDetails->iDomainModifierT = iExtraModifier;
			//}

			//if (pAttacker->isAnimal())
			//{
			//	iExtraModifier = animalCombatModifier();
			//	iTempModifier += iExtraModifier;
			//	if (pCombatDetails != NULL)
			//	{
			//		pCombatDetails->iAnimalCombatModifierA = iExtraModifier;
			//	}
			//}

			//if (isAnimal())
			//{
			//	iExtraModifier = -pAttacker->animalCombatModifier();
			//	iTempModifier += iExtraModifier;
			//	if (pCombatDetails != NULL)
			//	{
			//		pCombatDetails->iAnimalCombatModifierT = iExtraModifier;
			//	}
			//}
		//}

		if (!(pAttacker->isRiver()))
		{
			if (pAttacker->plot()->isRiverCrossing(directionXY(pAttacker->plot(), pAttackedPlot)))
			{
				iExtraModifier = -GC.getRIVER_ATTACK_MODIFIER();
				iTempModifier += iExtraModifier;
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iRiverAttackModifier = iExtraModifier;
				}
			}
		}

		if (!(pAttacker->isAmphib()))
		{
			if (!(pAttackedPlot->isWater()) && pAttacker->plot()->isWater())
			{
				iExtraModifier = -GC.getAMPHIB_ATTACK_MODIFIER();
				iTempModifier += iExtraModifier;
				if (pCombatDetails != NULL)
				{
					pCombatDetails->iAmphibAttackModifier = iExtraModifier;
				}
			}
		}
		
/************************************************************************************************/
/* Afforess	                  Start		 02/05/10                                               */
/*                                                                                              */
/*                                                                                              */
/************************************************************************************************/
		if (GET_PLAYER(pAttacker->getOwnerINLINE()).isDarkAge())
		{
			iTempModifier += GC.getDefineINT("DARK_AGE_ATTACK_STRENGTH_PERCENT_DECREASE");
		}
/************************************************************************************************/
/* Afforess	                     END                                                            */
/************************************************************************************************/

		if (pAttacker->getKamikazePercent() != 0)
		{
/************************************************************************************************/
/* Afforess	                  Start		 02/05/10                                    Dexy       */
/*                                                                                              */
/*       Unofficial Patch                                                                       */
/************************************************************************************************/
			iExtraModifier = -pAttacker->getKamikazePercent();
/************************************************************************************************/
/* Afforess	                     END                                                            */
/************************************************************************************************/
			iTempModifier += iExtraModifier;
			if (pCombatDetails != NULL)
			{
				pCombatDetails->iKamikazeModifier = iExtraModifier;
			}
		}
		
/************************************************************************************************/
/* Afforess	                  Start		 02/05/10                                               */
/*                                                                                              */
/*                                                                                              */
/************************************************************************************************/
		if (bSurroundedModifier)
		{
			// the stronger the surroundings -> decrease the iModifier more
			//TB Combat Mods (S&D promos) begin
			int iSurround = pAttacker->surroundedDefenseModifier(pAttackedPlot, bAttackingUnknownDefender ? NULL : this);
			int iDynamicDefense = dynamicDefenseTotal();
			int iDynamicDefenseModifier = ((iSurround * iDynamicDefense) / 100);
			int iSurroundTotalstepone = iSurround - iDynamicDefenseModifier;
			int iSurroundTotal = (iSurroundTotalstepone < 0 ? 0 : iSurroundTotalstepone);
			iExtraModifier = -iSurroundTotal;
			iTempModifier += iExtraModifier;
			//TB Combat Mods (S&D promos) end
		}
/************************************************************************************************/
/* Afforess	                     END                                                            */
/************************************************************************************************/
	
		//TB Combat Mods (StrAdjperAtt) begin
		if (pAttacker->currentStrAdjperAttTotal() != 0)
		{
			iExtraModifier = -(pAttacker->currentStrAdjperAttTotal());
			iTempModifier += iExtraModifier;
		}

		if (currentStrAdjperDefTotal() != 0)
		{
			iExtraModifier = currentStrAdjperDefTotal();
			iTempModifier += iExtraModifier;
		}
		//TB Combat Mods (StrAdjperAtt) end
		// if we are attacking an unknown defender, then use the reverse of the modifier
		if (bAttackingUnknownDefender)
		{
			iModifier -= iTempModifier;
		}
		else
		{
			iModifier += iTempModifier;
		}
	}

	if (pCombatDetails != NULL)
	{
		pCombatDetails->iModifierTotal = iModifier;
		pCombatDetails->iBaseCombatStr = baseCombatStr();
	}

	if (iModifier > 0)
	{
		iCombat = (baseCombatStr() * (iModifier + 100));
	}
	else
	{
		iCombat = ((baseCombatStr() * 10000) / (100 - iModifier));
	}

	if (pCombatDetails != NULL)
	{
		pCombatDetails->iCombat = iCombat;
		pCombatDetails->iMaxCombatStr = std::max(1, iCombat);
		pCombatDetails->iCurrHitPoints = currHitPoints();
		pCombatDetails->iMaxHitPoints = maxHitPoints();
		pCombatDetails->iCurrCombatStr = ((pCombatDetails->iMaxCombatStr * pCombatDetails->iCurrHitPoints) / pCombatDetails->iMaxHitPoints);
	}

	if ( pCacheEntry != NULL )
	{
		pCacheEntry->iLRUIndex = iNextCombatCacheLRU++;
		pCacheEntry->iResult = std::max(1, iCombat);
		pCacheEntry->pPlot = pPlot;
		pCacheEntry->pAttacker = pOriginalAttacker;
		pCacheEntry->pForUnit = this;

		//char buffer[300];

		//sprintf(buffer,"maxCombatStr cache result (%d) for unit %d, attacker %d @(%d,%d)\n", pCacheEntry->iResult, getID(), pOriginalAttacker == NULL ? -1 : pOriginalAttacker->getID(), pPlot == NULL ? -1 : pPlot->getX_INLINE(), pPlot == NULL ? -1 : pPlot->getY_INLINE());
		//OutputDebugString(buffer);
	}

	return std::max(1, iCombat);
}

You haven't updated anything in CvUnit yet so I didn't want to foul things up with an SVN update here.

I don't believe I have a test save for replicating the combat modifier loss where the attacker is invisible but theoretically I believe this solution solves things. The changes in the above begins with:

Oh, and I think the Bombard Bug is resolved.

I was planning to PM you when I checked my changes in (but I'm still debugging something so I haven't got that far yet). I don't think you can safely move that clause (that method is a mess because it is so overloaded due to the 4 different sets of semantics it tries to implement depending on the pattern of its parameters [as documented in the function header comment]). Really this should b refactored into independent methods, rather than have one trying to do so many things, but that a BBAI-inherited thing which I've never tried to deal with. Once I've checked my change in could you go through whatever test case you have that demonstrates a problem and see if it still happens. If it does then post the test case with a detailed description and we can both be thinking about it.

JosEPh_II · Mar 26, 2013

AIAndy said:
Invisible forests means that something is still generating them as secondary features which should not happen.

On a related note, the invisible forests if on a grass, plains, or lush terrain (have not checked muddy yet) you can build farms on and the forest is Not removed.

After the farm is built you can then come back and "chop" the invisible forest for Hammers to nearest city without bothering the new farm improvement. So the hammers to city is delayed till you come back and chop, not when Farm is completed (automatic).

JosEPh

strategyonly · Mar 26, 2013

How about "sea grass" invisible also??

Thunderbrd · Mar 26, 2013

Koshling said:
I was planning to PM you when I checked my changes in (but I'm still debugging something so I haven't got that far yet). I don't think you can safely move that clause (that method is a mess because it is so overloaded due to the 4 different sets of semantics it tries to implement depending on the pattern of its parameters [as documented in the function header comment]). Really this should b refactored into independent methods, rather than have one trying to do so many things, but that a BBAI-inherited thing which I've never tried to deal with. Once I've checked my change in could you go through whatever test case you have that demonstrates a problem and see if it still happens. If it does then post the test case with a detailed description and we can both be thinking about it.

I'd love to but I can't seem to find among my bugfix downloads the right file for that. Which means I'm going to have to look through all the bug reports over the course of the version development (I THINK it might've been in a separate bug thread...) to find where it was reported and see if there's a file there. I'll let you know. I've also got my wife playing some today with a dll that tests the solution I posted there. I have some suspicions yes... but I want to see what goes wrong exactly. Could help me to figure out what they were thinking when setting this up.

I figure we could generate some good test results. THIS bug should be fairly replicable since denial of such combat bonuses for invisibles is pretty much an absolute rule the way the code is written as it is now.

Koshling · Mar 26, 2013

Thunderbrd said:
I'd love to but I can't seem to find among my bugfix downloads the right file for that. Which means I'm going to have to look through all the bug reports over the course of the version development (I THINK it might've been in a separate bug thread...) to find where it was reported and see if there's a file there. I'll let you know. I've also got my wife playing some today with a dll that tests the solution I posted there. I have some suspicions yes... but I want to see what goes wrong exactly. Could help me to figure out what they were thinking when setting this up.

I figure we could generate some good test results. THIS bug should be fairly replicable since denial of such combat bonuses for invisibles is pretty much an absolute rule the way the code is written as it is now.

I'm about to push my changes. With the change to prevent the combat mod unitclass bonuses being calculated against itself (for a unit in some modes of the call to maxCombatStr()) as far as I can see it is applying the bonuses when it should be.

Can you clarify the relationship with invisible units? I don't see why that would be relevant.

Thunderbrd · Mar 27, 2013

Tell you what. We can leave it be for now. But that's purely because we don't have a good example save to work with to analyze the potential problem, which may have been related to something else resolved anyhow. We should be watching, however, for a repeat of the bug as reported (that vs combat class bonuses aren't being granted to units that are invisible and can't be seen by their enemy.)

I'm not entirely sure that section was at fault for this in the first place the more I evaluate it myself. This was a hasty and clumsy attempt at a 'fix' to a poorly understood problem. Sorry for that in the first place.

Things we must solve before V29

C2C Supreme Commander

C2C War Dog

Vorlon

C2C Modder

Deity

Deity

Vorlon

C2C War Dog

C2C Supreme Commander

C2C War Dog

Deity

Deity

Warlord

C2C Supreme Commander

Vorlon

TBS WarLord

C2C Supreme Commander

C2C War Dog

Vorlon

C2C War Dog

Similar threads