Unofficial Patch for 3.19

EmperorFool said:

It's a bug in that you cannot see the odds for the unit you'll end up fighting, and it won't ask you if you'd like to DoW on the friendly unit if there's an unfriendly unit there. In that sense, it's a bug in my eyes.

Click to expand...

That is the bug I was looking for, but it always asks me for a DoW when I try to recreate the situation using the world builder. I could swear I've seen the bug you speak of, but I was starting to doubt myself before your last message.

Since it's not going to ask if you want to declare war on the friendly units, I would argue that it should not show combat odds against those friendly units. I could understand changing the functionality for something like BULL, but for an UP fix, I think the original intentions should be honored. Keep the effects of dragging, right clicking, and holding ALT consistent, and just don't show combat odds against friendly units when attacking that plot won't ask you to declare war.

I could probably find the cause of that bug if I could get a save showing the bug.

Hmm, I did add a feature to BULL to keep it from asking you to DoW when you move onto a tile with friendly units, so maybe I only saw this behavior after adding that feature. I would try your test with an AI with which you have a peace treaty. In that case you cannot declare war. If it shows the odds for that friendly unit, it's a bug. If not, it's merely suboptimal behavior IMHO.

CvPlayerAI::AI_executiveValue()

I believe it should be

denev said:

CvPlayerAI::AI_executiveValue()

Click to expand...

Wow, if I'm reading the code correctly that's actually huge; it looks like it was preventing AIs from ever spreading corporations outside of their own empire, since the assignment "condition" would always return true and cause an abort. Gotta love simple little things like this lol. Nice find dood.

You are probably not reading it right , since AI spread corps outside of their empire ( I seen that in my last game in my empire ) Unless there is some other code that makes the AI spread corps ...

That's definitely a bug as it is setting the value to 0 instead of checking if it is zero. However, the result of an assignment is the value assigned which is zero in this case, and zero evaluates to false so this condition is never achieved. it's bad, but not as horrible as you'd expect. I assume this is why I often see AIs with 6 execs camped in their cities.

EmperorFool said:

However, the result of an assignment is the value assigned

Click to expand...

Oh. That would make more sense, but for some reason I thought the result of an assignment was always "true". *sheepish*

I also thought the return value of a successful assignment was true, which is obviously wrong as a little googling confirmed. Makes me wonder where I got that idea..

Version 1.6 has been posted, a big thanks to all of you who contributed!

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

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.

The game should create an ini file for you when you first launch the mod. There's nothing that would be changed in the ini file from the default.

Near the bottom of CvPlayer::getUnitLayerColors()


I'm pretty sure it's supposed to be


Otherwise you're relying on the initial value for .g and .b to be zero (bad practice), and getting a black final color instead of a red one (which is what's usually intended for enemy-related stuff).

CvCityAI.cpp::AI_bestPlotBuild(...)
iValue is FINAL RETURN VALUE of this function.
Therefore, it is sacred one and Firaxis (nor all living thigs in the world) can not use it as temporary variable.
I believe that it should be replaced with proper named variable. (Every names are sacred things. Variable's names are more sacred than any other names.)

And iValue needs a suitable initialization.
It is used before being initialized when bValid is false.
This means that AI_bestPlotBuild sometimes returns a funny value.

denev said:

iValue is FINAL RETURN VALUE of this function.
Therefore, it is sacred one and Firaxis (nor all living thigs in the world) can not use it as temporary variable.
I believe that it should be replaced with proper named variable. (Every names are sacred things. Variable's names are more sacred than any other names.)

And iValue needs a suitable initialization.
It is used before being initialized when bValid is false.
This means that AI_bestPlotBuild sometimes returns a funny value.

Click to expand...

Umm... I'm pretty sure Firaxis can use variables however they want to, dood.

There is nothing wrong with iValue's name, and even if there were it's named and used the same way in hundreds of other places in the code. You'd also have to rename iBestValue and iTempValue btw, since they are related.

It is already suitably initialized everywhere that is necessary; I checked. It is most definitely NOT being used before being initialized when bValid is false, and no, AI_bestPlotBuild() will not sometimes return a funny value. This is because iValue is NOT the final return value of the function. Technically the function has no return value btw, but it fakes one by assigning pointer data so okay. What happens is iValue IS used as a temporary variable because that is in fact its purpose. iBestValue stores the function's "return value", but is assigned in tandem with eBestBuild which prevents invalid responses at the end.

Oops... I have read a BBAI code. (AI_getImprovementValue())
You are right, LunarMongoose. Original BtS code doesn't have any problem.
iValue is a temporary variable at this function certainly.

AI_getImprovementValue() is some old left-over code that can barely do what it's supposed to anyway, as it's old and was never updated when AI_bestPlotBuild() was changed.

Fuyu said:

AI_getImprovementValue() is some old left-over code that can barely do what it's supposed to anyway, as it's old and was never updated when AI_bestPlotBuild() was changed.

Click to expand...

Oh I see. Interesting... In that case, sorry for snapping at you Denev; I may have overreacted a little heh.

In CvUnitAI::AI_exploreMove() and CvUnitAI::AI_exploreSeaMove(), the lines:


need to be changed to


And similarly, in CvUnitAI::AI_heal(), the line:


needs to be


Otherwise the AIs won't use a now-otherwise-fully-supported healing Feature to actually stop and heal.

denev said:

CvPlayerAI::AI_executiveValue()

Code:

	if (iOurCitiesHave >= iOurCitiesCount)
	{
		iSpreadInternalValue = 0;
		if (iSpreadExternalValue [COLOR="Red"]=[/COLOR] 0)
		{
			return 0;
		}
	}

Click to expand...

This code snippet doesn't make much sense to me. Why bother having that IF statement there at all? iSpreadInternalValue will always be 0 in this case because it's being set one line beforehand!

Tholal said:

This code snippet doesn't make much sense to me. Why bother having that IF statement there at all? iSpreadInternalValue will always be 0 in this case because it's being set one line beforehand!

Click to expand...

It's checking iSpreadExternalValue though (or well, supposed to be...).