Version 1.6 discussion thread

[jdog5000]

Another set of fixes for BTS 3.19 ... Civ5 comes out in less than three months, will we find still more issues with Civ4?

A big to all the bug hunters who contributed to this release, the Unofficial Patch simply wouldn't be possible without all of you.


Version 1.6 changes:

- CvPlayerAI::AI_commerceWeight - Standardized culture slider thresholds for detecting when human player is going for cultural victory
- CvTeam::doTurn - Removed unnecessary code (thanks EF)
- CvCityAI::AI_yieldValue - Resolved issues with rounding off small commerce amounts leading to working suboptimal tiles/specialists in some circumstances (reported by Caboose)
- CvPlot::calculateImprovementYieldChange - Fixed AI valuation of improvements causing negative yields (for mods) (thanks Afforess)
- CvGameTextMgr::assignFontIds - Removes an erroneous extra increment command that was breaking GameFont.tga files when using exactly 49 or 74 resource types in a mod (thanks LunarMongoose)
- CvGameTextMgr::setPlotHelp - Can now handle plots with extra healing powers (thanks LunarMongoose)
- CvGameTextMgr::setFeatureHelp - Now displays feature damage/healing (thanks LunarMongoose)
- CvUnit::healTurns and CvUnit::canHeal - Now handles damage and healing from plot features (thanks LunarMongoose)
- Added CIV4GameText_UnofficialPatch.xml to mod files
- CvGame::doTurn - Fixed bug with simultaneous team turns when team 0 is dead (thanks snarko)
- CvDLLWidgetData:arseActionHelp - Now displays that improvement will be destroyed when removing feature (thanks EF)
- CvGameTextMgr::buildBuildingRequiresString - Adds text if building is required in city for wonder, like hospital for red cross (thanks EF)
- CvCity:opOrder - Fix bug with production decay with multiple queued units (thanks jesusin and EF)
- CvUnit::canLoad - Reinstated improved MongooseSDK fix to not show load button for unit already on transport if there is no other transport it could switch to (thanks LunarMongoose)
- CvUnit::canPillage - Fix bug where all terrain units could pillage resources when on a transport (thanks LunarMongoose)
- CvPlayerAI::AI_executiveValue - Fixed bug causing AI not to use executives it had produced in some circumstances (thanks denev)
- CvPlayerAI::AI_cultureVictoryTechValue - Fixed bug where AI ignored buildings with obsolete safe commerce (thanks Fuyu)
- CvInfo (8 places) - Added booleans to record if a building has a SpecialistYieldChange or BonusYieldModifier defined (for next change)
- CvGameTextMgr::setBuildingHelp - Using above, improved response time of city screen (thanks Afforess)
- CvPlayerAI::AI_bestTech - Fix bug where iTempValue never added to iValue for building maintenance
- CvPlayerAI::AI_bestTech - AI now also counts ObsoleteSafeCommerce culture for cultural buildings

 

[LunarMongoose]

Yeah, I was afraid you wouldn't fix the one thing I was worried you'd miss lol. Please don't misunderstand, I love your work JDog and am thrilled to be a part of it, but I'm also a perfectionist so I can't let little stuff like this rest... *ahem* Quoting myself from post 353 of the main thread:

You didn't use the code segment in my last post here exactly as-is, even though I posted the whole function for a reason. Sigh, you need to learn to trust me more, dood. You left the vanilla "if (healRate(pPlot) <= 0)" condition in at the end, however this is completely redundant with the new check of healTurns(pPlot) returning MAX_INT or not since healTurns calls healRate itself, and uses the same condition as part of its return value. So your version will cause healRate, which is a relatively expensive function, to get called twice most of the time, when it never needs to be.

Here's the deal. My condition, which you already added, is "if (healTurns(pPlot) == MAX_INT) return false". The subsequent vanilla one, which you need to remove but didn't, is "if (healRate(pPlot) <= 0) return false". If you look inside healTurns(), you'll find "iHeal = healRate(pPlot);" followed by:

	if (iHeal > 0)
	{
		iTurns = (getDamage() / iHeal);

		if ((getDamage() % iHeal) != 0)
		{
			iTurns++;
		}

		return iTurns;
	}
	else
	{
		return MAX_INT;
	}

Note that "else" on a "> 0" check is identical to the "<= 0" check up top. This wouldn't be a big deal, except that, as I said, healRate() is a relatively expensive function: it loops over the current tile as well as all adjacent tiles looking for the area healing effect a promotion can provide. Adding my bugfix check to CvUnit::canHeal() without removing the redundant vanilla one causes this to be called twice most of the time when it never needs to be.

Sorry for beating you over the head with this, but I wanted to be sure I got my point across this time.

Edit - Actually it's slightly worse than this, since the two conditions are not identical afterall (just reread it again), b/c of the extra code for feature effects in healTurns(). So if you had, say, a negative base heal rate for whatever reason, that was then overriden by a stronger feature healing effect, you should have a Heal button because your net healing would be positive, but the extra vanilla check that just looks at base heal rate would block that. Okay, I'm going to shut up now.

 

[jdog5000]

Ah, crap. Will be fixed in BBAI 1.01 when that's released very soon, will fix in UP svn to.

 

[Ritari]

I'll post this here as well:

The 1.6 version packed zip is missing the "ini" file. So anybody not having an earlier version, needs to download and install some previous version first and then copy 1.6 on top of the older version, until the 1.6 zip file is fixed.

 

[karadoc]

I've noticed a minor mistake in CvCityAI::AI_doHurry. Here it is:

// Only consider population hurry if that's actually what the city can do!!!
if( (iHurryPopulation > 0) && (getPopulation() > iHurryPopulation) )
{

I'm not sure if this code is original BTS code, or from the unofficial patch. The problem is that cities aren't allowed to use all of their population to hurry something. This if statement should be using maxHurryPopulation() rather than getPopulation(). But actually the test is redundant anyway because earlier in the function 'canHurry' is called, which includes a population check.