Unofficial Patch for BTS 3.19:

Change list:

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::parseActionHelp - 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::popOrder - 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


Version 1.50 changes:

- CvCityAI::AI_buildingValueThreshold - Fixed bug causing AI cities to over value buildings which unlock specialists in many circumstances when picking what to build
- CyInfoInterface2 - Fixed issue with "getHappiness" for improvements (thanks NotSoGood)
- CvPlayer::canFound - Changed handling of founding on water tiles so that Python callback has final say if it is turned on
- CvPlayer::removeBuildingClass - Fixed issue with removing buildings when building class was maxed out for some mods (thanks EF)
- CvPlot::doImprovement - Minor efficiency improvement for existing game speed scaling fix
- CvPlot::doImprovment & CvPlot::doFeature - Now scale accoding to getVictoryDelayPercent, which is more appropriate
- CvUnitAI::AI_specialSeaTransportMissionary - Fixed minor bug in valuation for executives
- CvPlayerAI::AI_targetGold - Fixed bug causing AI to not bother with budgeting for expenses for first 40 turns of scenarios and advanced start games
- CvCity::init - Reverted to standard BTS code where floodplains are removed when a city is founded
- CvCity::kill - Replace floodplain after city is destroyed
- CvTeam::doTurn - Added barbarian passive tech fix from Mongoose SDK
- CvPlayerAI::AI_changePeacetimeTradeValue and CvPlayerAI::AI_changePeacetimeGrantValue - AIs you haven't met yet will no longer get angry at you for trading with their worst enemy (thanks Sephi)
- CvPlayerAI::AI_getStrategyHash - Fixed several bugs where player ID was used where team ID was intended
- CvPlayerAI::AI_targetCityValue - Fixed issues with valuation of cities with inactive world wonders, multi-holy-cities
- CvCity - Added function getNumActiveWorldWonders()
- CvPlayerAI::AI_commerceWeight - Improved valuation of generating culture in human player cities when culture bar is > 50% (ie, player probably going for cultural victory)
- CyInfoInterface1 - Added missing CvUnitInfo::isOnlyDefensive (thanks Afforess)
- CvCityAI::AI_calculateCulturePressure and CvCityAI::AI_playerCloseness - Fixed incorrect usage of getID() (thanks denev)
- CvPlayerAI::AI_foundValue - Added missing division for iClaimThrehsold (thanks denev)
- CvTeam::shareCounters - Fixed bug causing inappropriate overflow research and effectively free techs when forming a Permanent Alliance
- CIV4GameText_Events_BTS.xml - Fixed Italian translation for TXT_KEY_EVENTTRIGGER_MOTOR_OIL and usage of Amun-Re (thanks jsbrigo)
- Popup.py - Fixed X versus iX in addPythonButtonXY(), and (maybe) fixed reference to non-existent variable in addTitleData() (thanks EF)
- CvCity::getProductionModifier (three versions of this function) - Changed to allow mods to create negative production modifiers (thanks NotSoGood)
- CvUnit::rangeStrike - Fixed bug with determining if can range strike, and potential OOS from range strikes (thanks God-Emperor)
- CvUnit::canRangeStrikeAt - Added check for whether target plot is visible (thanks God-Emperor)


Version 1.40 changes:

- CvGameTextMgr::setBuildingHelp - Fixed issue in mods with display of exposing spies text for buildings which lower espionage defense (thanks Afforess)
- CvPlayer and CvDLLWidgetData - Fixed bug where you could acquire unlimited free techs from Oracle or Liberalism (thanks Emperor Fool)
- CvCity - Fixed extra overflow production bug when stopping culture process after border pop (thanks denev)
- CvUnit::canLoadUnit - Rolled back change blocking switching cargo among transports
- CIV4BuildingInfos.xml - Fixed incorrect sound for building supermarket, added missing sound for building coal plant (thanks Arian)


Version 1.30 changes:

- CvPlayerAI::AI_bestTech - Fixed potential rare crash bug in mods where units with no transport capacity can upgrade to units with transport capacity (thanks Afforess)
- CvGameTextMgr::getOtherRelationsString - Fixed potential issue revealing names of civs you haven't met yet (thanks Emperor Fool)
- CvCity::popOrder - Fixed issue introduced by prior attempt to fix handling of buildings with player limits. Building classes which set iMaxPlayerInstances should now work correctly regardless of what iExtraPlayerInstances is set to. (thanks ztjal)
- CvPlayerAI::AI_civicValue - Fixed crash bug in multi-player simultaneous turns games created by necessary re-timing of AI_doSplit (thanks to TheOnlyDJCat for debugging help)
- CvPlayer::canDoCivics - Same as above
- CvUnit::maxCombatStr - Handled very rare case where pointer might not be properly set when loading simultaneous turns game in hotseat
- CvPlayer::canFound - Fixed several issues for mods using the python callback to allow founding cities on water tiles (thanks Emperor Fool)
- CvPlayerAI::AI_getHealthWeight - Fixed bug for mods where civics with negative iExtraHealth were evaluated incorrectly (thanks phungus420)
- CvPlayerAI::AI_getHappinessWeight - Fixed bug for mods where civics with negative happiness effects from troops, largest cities, or war weariness were evaluated incorrectly
- CvPlayerAI::AI_civicValue - Changes so that civics valuation works properly with above changes
- CvCity::setBuildingHealthChange - Fixed several copy/paste and logic bugs affecting bonus building health from events, especially after city conquest (thanks Emperor Fool)
- CvCity::setBuildingHappyChange - Fixed several logic bugs affecting bonus building health from events, especially after city conquest
- CvPlot::doFeature - Plot feature (Forest/jungle) appearance now scales by game speed
- CvPlot::doImprovement - Bonus appearance in mines now scales by game speed
- CvAdvisorUtils.py - Blocked city liberation popup if you haven't met liberation player (reported by r_rolo1)
- CvPlayer::canTradeItem - You can now ask AI members of your own team to change religion or civics (thanks denev)
- CIV4EventTriggerInfos.xml - EVENTTRIGGER_SPY_DISCOVERED now requires you have a spy, fixes issue where event would fire with no espionage enabled and your only choice was war
- CvCity::init - Building a city on floodplains no longer removes floodplains, they'll still be there if city is destroyed (from Mongoose SDK)
- CvUnit::convert - Fixed potential issue in mods where units with transport capacity might upgrade to units without (from Mongoose SDK)
- CvUnit::canLoadUnit - Block units which are on another transport from taking on cargo of their own (from Mongoose SDK)
- CvUnit::shouldLoadOnMove - Fixed issue with all terrain land units moving onto water tiles with transports in them (from Mongoose SDK)
- CvUnit::canAirDefend - Land units which are on transports can no longer defend against air attacks (from Mongoose SDK)


Version 1.20 changes:

- CvUnit::canSpread - Moved Python cannot spread callback to end of function where it belongs, will speed up those mods which use this callback a little
- CvPlayerAI::AI_missionaryValue - Fixed copy and past bug causing overvaluation of missionaries for AIs going for cultural victory early in the game.
- CvGameTextMgr (many places) - Fixed issues where unhappiness and unhealthiness from civics or buildings would incorrectly show up as -(unhappy face) instead of +(unhappy face) in several circumstances. (Thanks EmporerFool, Grave, Afforess)
- CvGameTextMgr - Game will now properly display info for buildings which generate unhappiness in an area or globally, or produce state religion unhappiness (should these ever come up in mods)
- CvPlayerAI::AI_doTurnPost and CvPlayerAI::AI_doTurnUnitsPost - Added in accidentally skipped fix for AI_doSplit issues as suggested by alexman
- CvTeamAI::AI_calculateAreaAIType - Fixed incorrect index usage (thanks cephalo)
- CvCity::popOrder - Fixed issue with improper use of iExtra causing national wonders to max out early in some mods (thanks davidlallen)
- CvTeam::addTeam - Fixed bug where, if civs A and B join in a permanent alliance, they get the max of A and B's espionage points against C but C just keeps its point against A and loses its points to B if that's higher
- CvUnit::canMoveInto - Removed strange behavior where setting a unit to be unable to enter a terrain type would be overridden by features (forrest, fallout) (thanks TC01)
- CvCityAI::AI_chooseProduction - Fixed bug reducing AI production of workers, and a similar issue for barb players producing too many


Version 1.10 fixes:

- CvGame::addPlayer - Restored 3.17 UP changes forcing clearing of old player names, civ descriptions, etc when placing a new player in a previously occupied slot.

- CvCity::setCultureLevel - Now behavior where a city building culture would cancel order when reaching next level only applies to human player. It was completely unnecessary for AI and caused a small bug with extra overflow production.

- CvUnit::canMoveInto - Changed to allow paratroopers to capture workers and undefended cities after paradropping. Also allows units with multiple moves to capture multiple tiles of non-combat units like workers.

- CvCity::kill - When a player loses a city, that city now clears all of its trade route claims so that they can be taken by other cities owned by the player. Previously, these trade routes were permanently lost. (Thanks RedFury and DanF5771)

- CvUnitAI::AI_spreadReligionAirlift and CvUnitAI::AI_spreadCorporationAirlift - AI will no longer airlift multiple of the same kind of missionary or executive to the same target spread city on the same turn

- CvAdvisorUtils.py - Advisor will no longer bug for city liberation to player you're at war with.

- CvDomesticAdvisor.py - City liberation to player you're at war with no longer offered.

- CvDLLButtonPopup::launchFreeColonyPopup - Removed cities whose liberation player you are at war with from popup.

- CIV4EventInfos.xml - In EVENT_OVERWHELM_DONE_1, removed references to non-existent Python functions which blocked this option. (thanks Pep)

- CvSelectionGroup::continueMission - Fixed issue causing units with multiple orders to forget their later orders under certain circumstances. (thanks Pep)

- CvCityAI::AI_doHurry - Fixed several bugs where AI would incorrectly think it was getting a great deal on a pop/gold rush when it was actually doing the other kind of rush. (thanks Pep)


Version 1.0 fixes:

1) CvUnitAI::AI_paradrop - fixed bug with valuation of terrain bonuses causing paradrops to avoid bonuses in some circumstances when intention is clearly to encourage landing on bonuses and pillaging

2) CvUnitAI::AI_settleMove - fixed bug when settler cannot reach a city site in an area (blocked by mountains, other player). Caused settler to wait infinitely in city instead of loading into transport.

3) CvTeamAI::AI_doWar - use bFinancesProLimitedWar for limited war calc instead of max war version

4) CvPlayerAI::AI_calculateUnitAIViability - Fixed incorrect integer usage which caused function return to be meaningless, blocked AI logic for building privateers

5) CvPlayerAI::AI_isFinancialTrouble, CvCityAI::AI_updateBestBuild, and CvPlayerAI::AI_getMinFoundValue - Fixed bug in calculating expenses when AI has negative gold per turn

6) CvPlayerAI::AI_getTotalFloatingDefendersNeeded - Fixed poor decision by AI if it has captured one or two cities on someone else's continent, it would minimally defend its new cities

7) CvPlayerAI::AI_getStrategyHash - Fixed incorrect counting of destroyers as mobile anti-air

8) CvPlayerAI::AI_doDiplo - Fixed issue where team is sneak attack ready but hasn't declared war, AI would still demand tribute. If other team accepted, it blocked war declaration for 10 turns but AI still launched invasion and was then bounced when it could eventually declare.

9) CvPlayerAI::AI_bestPlotEspionage - relevant weights are 0, +- 50, +- 100, so comparison for Agg AI should be < 51 instead of < 50

10) CvPlayerAI::AI_commerceWeight - Governors now do smarter things when human player sets culture slider to 100%

11) isPotentialEnemy in CvGameCoreUtils - Fixed bug leading to AI launching invasions when unable to declare war, troops eventually got bounced when war was declared

12) CvGame::addPlayer - No longer invalidate color choice for added civ if it's taken by this player slot

13) CvCityAI::AI_cityThreat - Fixed bug when AI is running crush strategy, wrong int was divided

14) CvCity::popOrder - Kept overflow fixes from 3.17 unofficial patch

15) CvCityAI::AI_neededDefenders - Improved efficiency and minor tweaks

16) CvPlayerAI::AI_isFinancialTrouble, CvCityAI::AI_updateBestBuild, and CvPlayerAI::AI_getMinFoundValue - Fixed bug where spending gpt for resources reduced calculation of expenses (thanks DanF5771)

17) CvPlayerAI::AI_conquerCity - Fixed bug where cityAcquiredAndKept event reported wrong player in some circumstances (thanks Maniac)

18) CvPlayerAI::AI_unitValue - Fixed potential crash bug in looking up AI_unitValue for UNITAI_MISSIONARY units without passing a valid CvArea*

19) CvUnit::isIntruding - Kept 3.17 unofficial patch feature that vassal spies are never caught in master's territory

20) CvUnit::collateralCombat - Kept 3.17 unofficial patch feature which allows mods to enables barrage promotions for collateral damage units

21) CvUnitAI::AI_assaultSeaMove - Fixed bug where unit type was used when unit AI type was intended

22) CvUnit::canMoveInto - Capturing an undefended city is now considered an attack action, like capturing a worker. Mainly affects paratroopers, who now cannot capture undefended cities right after paradrop. Previously, paratroopers could capture undefended cities but not cities with only a worder or ship in them.

23) CvTeam::shareCounters - Fixed bug in permanent alliances when No Tech Brokering is turned on, where a civ would often become unable to trade a tech they had researched after entering a permanent alliance.

24) CvNetDoCommand::Execute - Fixed bug causing "Upgrade All" action to sometimes only do a fraction of the available units.

25) CvPlayerAI::AI_unitValue - Stealth boats do not make good escorts since they often don't defend, so AI is now very unlikely to use them as escorts for transports.

26) CIV4UnitInfos.xml - Removed UNITAI_ESCORT_SEA from Stealth Destroyer (stealth doesn't defend first, so not a good escort). Set iPower weighting for Missile Cruiser to much more appropriate 42 from 14 (iPower ratings for boats were increased by Firaxis a while back, but they missed this one).

27) CvCityAI::AI_yieldValue - Fixed issue causing city governor and AI to heavily weight food when building gold or any other form of commerce. Produced unexpected and poor results for human player, did not help AI either.

28) CvCity::setCultureLevel - Removed behavior where a city building culture would cancel order when reaching next level. Completely unnecessary for AI, human player is unlikely to be expecting this. Also appears to cause a bug with extra overflow production.

29) CIV4UnitClassInfos.xml - Fixed description entry for ship of the line (thanks ripple01)

30) CIV4ArtDefines_Unit.xml - Fixed many discrepancies in TrainSound between unique and non-unique units (thanks ripple01)

31) CvRandomEventInterface.py - Fixed function name from getHelpThGoths1 to getHelpTheGoths1 (thanks Chrill)

32) CvMapGeneratorUtil.py - Fixed function getLatitudeAtPlot in both FeatureGenerator and TerrainGenerator so that it properly returns 1.0 for both north and south poles and correctly locates equator (thanks Temudjin)

33) CvPlot::getLatitude - Fixed so that it properly returns highest latitude value for both north and south poles and correctly locates equator, improved integer rounding errors which caused slight skew in terrain types between northern and southern hemisphere (thanks Temudjin)


Downloads:

Download links for the current version:
CivFanatics (http://forums.civfanatics.com/downloads.php?do=file&id=12819)
Sourceforge (https://sourceforge.net/projects/civ4patch/files/)


Download links for prior versions:
Sourceforge (https://sourceforge.net/projects/civ4patch/files/)


Links for sources and other development files:
SDK sources at CivFanatics (http://forums.civfanatics.com/downloads.php?do=file&id=12820)
SDK and other file repository at sourceforge (https://sourceforge.net/projects/civ4patch/develop)


Install Instructions:

This UP requires Civ4: Beyond the Sword patched to 3.19. There are two ways to use it:

1 As a mod. Place the Unofficial 3.19 Patch folder which contains the Assets folder in "C:\Program Files\Firaxis Games\Sid Meier's Civilization 4\Beyond the Sword\Mods\". You can then load Unofficial 3.19 Patch as a mod in the normal way or set BTS to autoload it in "\My Games\Beyond the Sword\CivilizationIV.ini". This method maintains compatibility for online play but the changes in the UP will only apply when the mod is running (not in other mods, Firaxis scenarios). Recommended if you want to easily go back to standard BTS or play online.

2 Replace default files. The contents of the Assets folder are intended to replace the files located in the same locations in "C:\Program Files\Firaxis Games\Sid Meier's Civilization 4\Beyond the Sword\Assets\". It is recommended that you backup your current dll and other files which will be replaced (renaming them will work fine) before installing this version. When the game is not running, simply paste the contents the UP's Assets folder into the same locations in BTS's Assets folder. This method will make the Unofficial Patch changes always apply (unless you load a mod with its own DLL or copies of other files ...). For online play everyone must have installed the Unofficial 3.19 Patch in this way or things may not work correctly. Recommended if you always want to use the Unofficial 3.19 Patch or continue a current save game from plain BTS.


Merging Instructions:

The full list of changes, including which functions and files were touched, is included in the download. Check it out.

The SDK sources can be found in a zip here at CFC (http://forums.civfanatics.com/downloads.php?do=file&id=12820), or checked out from sourceforge using SVN (https://sourceforge.net/projects/civ4patch/develop). If your mod already has a DLL, you will need to merge in the UP source code and recompile. All sourcecode changes are marked with UNOFFICIAL_PATCH, I also highly recommend using WinMerge (http://winmerge.org/) to help make merging easier (it's free too).

The UP also includes changes to some Python and XML files from BTS. You will also need to copy or merge these into your mod, again WinMerge is your friend.

There were two changes from the UP missing from 3.19 that I noticed tonight.


Collateral damage for non-collateral units allowed to work if modded back in
Vassal's spies not caught in masters' territory

Number 1 is clearly out given that the "official" stance is that tanks cannot receive the Barrage line of promotions. Tough petunias, boys.

There may have been others. I did the merge last night pretty quick, and I stopped paying as close attention to the actual changes and more to getting it to compile after a while. Then of course there's the overflow gold bug. It makes sense to use the original UP code; that's what I did for BULL.

Collateral damage for non-collateral units allowed to work if modded back in

clearly out given that the "official" stance is that tanks cannot receive the Barrage line of promotions. Tough petunias, boys.

That was a 2 part fix. Another part of the UP removed the barrage promotion from tanks. The collateral fix just means that if you have a mod that puts the ability to buy barrage on a non-collateral unit back, it will work.

So including it would have no effect on unmodded gameplay.

That was a 2 part fix. Another part of the UP removed the barrage promotion from tanks. The collateral fix just means that if you have a mod that puts the ability to buy barrage on a non-collateral unit back, it will work.

True enough, but didn't it only half work? I remember something from Solver that he had enabled it for those who mod the promotion but that it had a subpar effect. As you say, it doesn't hurt to keep it in there.

I don't remember the final state, but there were several changes to "non-native collateral damage" to try to tweak it back to something like pre 3.17 functionality.

As long as tanks don't have access to barrage (which they don't), it doesn't effect the standard game at all.

I don't remember the final state, but there were several changes to "non-native collateral damage" to try to tweak it back to something like pre 3.17 functionality.

I think the only change that didn't make it into 3.19 regarding collateral was a check for native collateral damage was extended to also check for extra CD. Regarding the thread discussion, I think that the actual damage from collateral was based heavily on native CD, but it's all based on fuzzy memory.

As long as tanks don't have access to barrage (which they don't), it doesn't effect the standard game at all.

Correct. It does make sense to put it back in then.

Hey, since alexman clearly stated their intentions regarding the gold overflow, does that mean we should call this POP: the Partially Official Patch? :mischief:

To much fun with acronyms is bad for your health :p

but oh so hilarious :lol:

Partially Official Omissions Patch?

I couldn't think of a second O. Brilliant!

...yeah, I was hoping no one would manage to fill in the other O. ;)

I totally can't wait for the BULL release announcement.

BULL 1.0 . . . Now with more POOP!

I guess that merged build should be called BULL-POOP.

Okay, I'm done now. :D

Fixes are marked with UNOFFICIAL_PATCH in the Better BTS AI sourceforge repository (https://sourceforge.net/projects/civ4betterai/).

Are these bugs fixed in Better BTS AI 0.77?

Are these bugs fixed in Better BTS AI 0.77?

Yes, many have been in for months.

Another one for the pile:

While merging Planetfall and 319 code, I was reminded of a probable bug:

In CvPlayerAI::AI_conquerCity it says at the end:

CvEventReporter::getInstance().cityAcquiredAndKept (GC.getGameINLINE().getActivePlayer(), pCity)

I believe that should be:

CvEventReporter::getInstance().cityAcquiredAndKept (getID(), pCity)

Not a bug, but would be a good feature:

Which brings me to one thing that bugs me on OCC - if I take a city, I should have the opportunity to liberate it to it's original owner. Instead, it gets razed to the ground without question. So, when France takes and English city and then I take it back a few turns later, I piss off the English... sigh...

How about a third option of being able to keep it and raze your current city? :mischief:

Traveling tour of destruction?

How about a third option of being able to keep it and raze your current city? :mischief:

Probably shouldn't be in the unofficial patch.

But jdog, please add this to RevDCM :) It would just be a cool option, and wol't hurt anything.

Can someone have a quick look at this and let me know if I'm just being fick. It looks to me as if when a colony is formed some AI Civs are getting multiple free turns:

Playing as Justinian, the Mongol Pikeman highlighted near Moscow should not be able to attack Adrianople(?), but upon pressing end of turn Lizzy spawns Washington and they do attack, and also a Korean SoD from some distance away ends up in the forest next to Adrianople.

Screenshots and saves attached.

Thanks.

Could the knights in the stack have attacked, and then the remaining musketmen and pikeman been killed in a counter attack? This is all possible during the inter-turn.

here is a bug report for v319 ... http://forums.civfanatics.com/showthread.php?p=8173407#post8173407

It involves PBEM and the use of the altroot command line.

Could the knights in the stack have attacked, and then the remaining musketmen and pikeman been killed in a counter attack? This is all possible during the inter-turn.

According to the event log they died attacking my grenadier (screenshots attached) they also appear to attack from the forest but that could just be screwy graphics? Not sure what you mean by counter attack(?)...

Regarding the Korean SoD, did they perhaps lose Open Borders with Russia and get kicked out . . . next to your city?

I gave a somewhat detailed look to GravityWave save and IMHO there is definitely a bug in here....

First things first: This is a game with 18 civs, all of them alive ATM besides Babylon, that was player #6 ( you can check this easily ). The human player is Bizantium, that is player #4. The players before #4 already played ( including England, the master to be of America )....

When the human finishes its turn in 1714, the game goes to player #5 ( Russia ), skips #6 ( Babylon ) because it is dead, passes to #7 ( Egypt ) and so on ( I had checked friendly and enemy moves to keep track of all ).... all is normal until the ( BUG ) warning that England founded Dover and the diplo window of Lincoln ( that gets to be player #6 instead of Hammurabi of Babylon ), followed by the Rifling tech window....

After that the game shows Egypt, ... , moving the units again and it is in this phase that the flying pikeman attacks the city and the korean Speedy Gonzalez gets to that forest. This can be checked by the Event log, that shows the mongol pike attacking in 1716, before the human move , a thing that shouldn't happen because Mongolia is suposed to play after Bizantium by the player order.

Seeing this I decided to mark some units of various civs to check for double movement. All of the tested civs showed it, except Russia ( player #5 )....

The conclusion I can take from this ( that will require obviously code check ) is that the game started the year 1716 playing with the newlyfounded colony ( player #6 ) instead of starting with player #0 ( England ) and that continued the order until player #17, resuming to #0,1... until the player #4 , the human, giving a effective 2 turn period in the IBT to all the AI except for poor Russia :D

Regarding the Korean SoD, did they perhaps lose Open Borders with Russia and get kicked out . . . next to your city?

According to the diplo screen they have Open Borders - I guess it is possible for Peter to of cancelled the agreement and then Wang Kong resigned it, and done the same with Genghis which kicked out all the units in to the same square, enabling the Mongols to attack Adrianople.... I have no idea how to check/test this though...

Just read r_rolo1's post, and I'm sure that's the problem. Thanks for looking in to it... at least it's an improvement over the living dead civs mucking up any chance of a diplo victory... though I can see a lot of times when this could be a game ruining, if not breaking bug.

It was not that what happened, because in that case the mongols would had attacked in 1714 and not in 1716 as stated by the events log.

So it sounds like there's a problem in 3.19's fix for spawning new colonies in games that have all the AI slots filled?

I would definitely say so ......

GravityWave, maybe it would be better to post something in the bugs forum. The firaxians are scanning that forum regularly ( even now alexman is seeing it )

Did 1716 happen twice for all the AIs #6 through #18 or did #0 through #5 get skipped in 1717? My guess is that when spawning the colony there is some code that allows the new colony to move in that turn that assumes they will get the last slot. When they take an existing slot, everyone after them gets another turn.

I haven't tested what happened to all the civs .... But I think that I seen a portuguese unit ( portugal plays before bizantium ) moving.... I'll test it as soon as possible.

P.S That can't be... if that had happened, the mongols would had attacked in 1714 and not in 1716, I think....

I rechecked and indeed it looks that the players #0-3 ( England, Netherlands, Portugal and rome, by that order ) have only played 1 turn in the IBT as expected.....

So a chronology:

-1714, end of turn for player #4 . Players 0-3 already played.....
-1714, IBT, usual moves for the alive AI from #5 to 17
-1714, Colony is created and it has slot #6 assigned
-1716 - Colony plays once ( queues confirm that .... btw didn't knew that colonies also received the initial 10 hammers as the other AI in the start :p )
-1716 - AI from #7 to 17 play, then the ball passes to #0 and from there to #3. In this period the Mongols attack Adrianopole and the koreans get to that forest. This makes the AI that ocuppy the slots between 7 and 17 to have played two turns in the human IBT. Players #0-3 and 5 only played once ( players 0-3 probably already in 1716 and 5 in 1714 )

So does the year advance twice, skipping players 0-4? Or do players 6-17 get two moves in 1716? Towards the end of your chronology, you stop showing the year so it's unclear.

Also, what player # is England who spawns the colony? If they aren't #17 (last player), do the AIs between England and the barbs get two turns or do they get skipped during the first normal set of turns?

Finally, do the barbs get two rounds during this hiccup? They are player #18 here, right?

Ok.....

England is player 0 and already played in 1714 when the human finishes it's turn. Player 6 ( the colony ) only plays once ( not sure if in 1714 or 1716 ( turns 752 and 753, respectively ... this is marathon ) ). Players 7-17 play once in 1714 ( after the human ) and another in 1716, this one before the human turn ( that is player #4, remember ) instead of being after, making them to play 2 turns in the human IBT . Players 0-3 play one turn in the human IBT ( as they should ), as well as player 5 ( also as it should be ). Haven't checked barbs....

Between 1716 and 1718 ( the next turns ), everyone plays only one turn in the human IBT, btw.... I would say that the colony creation messes up the play order...

I might do some more testing later, but there is a unrelated detail that stroke me: the colony has the artstyle and the flag of the dead civ ( babylon ), but not the color, that is the regular american blue. Wierd.....

Wait, so, if you keep playing out the game from there, in the end everyone will have taken the same total number of turns- only the turn order was rearranged?

I would say that it looks that way....

Since England is player #0, the colony had to be created in 1716--not 1714. It sounds like when England split the colony, the "current player" jumped from #0 to #5--skipping #1-#5.

After players #6-#17 get their turn in 1716--the second turns for them during the single IBT for the human, it goes to 1718 where players #0-#3 play and then it's the human's (#4) turn.

It sounds like players #1-#5 got skipped in 1716, so not everyone gets the same number of turns. I wonder if this happens when the new colony isn't taking a previously-played slot.

The players 0-5 are not skipped in 1716. Players 0-3 already played in 1714 and they have a extra played turn in the IBT ( in 1716, most surely ). Player 5 plays ( in 1714 ) after the human and only plays once in the IBT and has one turn played in the 1716-1718 IBT ( meaning that it played in 1716 ). The Human ( player #4 ) obviously play in both turns.

The players 0-5 are not skipped in 1716.

Player #0 (England) surely plays in 1716 because they spawned the colony (#5), right?

Players 0-3 already played in 1714 and they have a extra played turn in the IBT ( in 1716, most surely ).

They are supposed to play in 1716, so that's not extra. They're turns during 1714 are before the human's--not in the IBT.

Player 5 plays ( in 1714 ) after the human and only plays once in the IBT.

Since player #5 is created by player #0 in 1716, surely it plays its turn in 1716 as well.

The Human ( player #4 ) obviously play in both turns.

Which turns do you mean by "both" given that there seem to be three turns involved here: 1714, 1716, and 1718.

This is what I'm thinking to be the timeline, but it's getting quite confusing.


1714 starts
Players 0-3 play
Player 4 plays -- HUMAN TURN
Dead player 5 skipped
Players 6-17 play
Player 18 plays -- BARB TURN
1716 starts
Player 0 frees a colony; player 5 created anew
Players 1-3 play -- DOES THIS HAPPEN?
Player 4 skipped -- HUMAN SKIPPED
Players 5-17 play
Player 18 plays -- BARB TURN
1718 starts
Players 0-3 play
Player 4 plays -- HUMAN TURN

The bold parts are the ones in question, and the second one (player skipped) is the problem. Is this timeline accurate?

Player #0 (England) surely plays in 1716 because they spawned the colony (#5), right?
Colony is #6 . Besides that it is correct ( England even enters in anarchy )
They are supposed to play in 1716, so that's not extra. They're turns during 1714 are before the human's--not in the IBT.
I never said they were extra turns ( in fact , I'm getting increasingly convinced that technically there are no extra turns played by anyone ) And technically the IBT means "in between (human) turns" :D
Since player #5 is created by player #0 in 1716, surely it plays its turn in 1716 as well.
Like I said above the colony is player #6. Player #5 is Russia
Which turns do you mean by "both" given that there seem to be three turns involved here: 1714, 1716, and 1718.
I meant 1714 and 1716 in this particular regard. Sorry for the confusion. Human plays all the turns involved.
This is what I'm thinking to be the timeline, but it's getting quite confusing.


1714 starts
Players 0-3 play
Player 4 plays -- HUMAN TURN ..... here is where we are in the posted save
Player #5 ( Russia ) makes his move
Dead player 6 skipped
Players 7-17 play
Player 18 plays -- BARB TURN
1716 starts
Player 0 frees a colony; player 6 created anew Not sure.... Player 6 has 1 turn of prod in queues before the human play in this turn
Players 1-3 play -- DOES THIS HAPPEN? YES, but not necessary in this point. We have no way of accurately knowing if they play before or after player 17 or not without going to debug ( a thing I should already had done )
Player 4 skipped -- HUMAN SKIPPED NO . Human plays in 1716, after players 6-17
Players 6-17 play
Here or after the next point, the human ( player #4 ) and player #5 ( Russia ) play
Player 18 plays -- BARB TURN
1718 starts
Players 0-3 play Also players 6-17
Player 4 plays -- HUMAN TURN

The bold parts are the ones in question, and the second one (player skipped) is the problem. Is this timeline accurate?
Red text is my comments/corrections

# 1718 starts
# Players 0-3 play Also players 6-17
# Player 4 plays -- HUMAN TURN

So it seems that the human player #4 gets permanently moved to the end of the queue. I really don't see how this could be given the game mechanics of looping over players, but I'm not very familiar with this area of the code.

No, the human is not moved to the end. Player #5 is ;)

Better said, between points 13 and 17, all of the AI play one turn as far as I could detect it ( this mainly by queue analysis, but also by troop movement ). This strongly points to the turns starting with 6 ( the colony ) to 17 and then 0 to 5.

P.S This really needs debug analysis, but i'll not not do that at 1:30 AM ;)

I modified CvEventManager to report on player/game turns and saw some interesting results. I loaded the save and hit end turn. Everything looks great for turn 752.


End player 4 turn 752
End player 5 turn 752
End player 7 turn 752
End player 8 turn 752
End player 9 turn 752
End player 10 turn 752
End player 11 turn 752
End player 12 turn 752
End player 13 turn 752
Player 14 Chinese attacks Player 4 a bunch of times
End player 14 turn 752
End player 15 turn 752
End player 16 turn 752
Player 17 Korean attacks Player 4 a few times
End player 17 turn 752
End player 18 turn 752
End game turn 752


On the next turn, England (0) frees their colony to create player 6, and here's where things go off the rails. Play first jumps from player 0 to player 6, skipping 1-5.


Player 6's alive status set to: 1
Team 6 becomes a Vassal State of Team 0
End player 0 turn 753
End player 6 turn 753
End player 7 turn 753
End player 8 turn 753
End player 9 turn 753
End player 10 turn 753
End player 11 turn 753


But after player 11 goes the game really gets screwy and jumps back to player 1, but it doesn't continue up to 5. No no, take a look.


End player 1 turn 753
End player 2 turn 753
End player 12 turn 753
Player 13 Civilization Persian Empire Unit Knight was killed by Player 4
End player 3 turn 753
End player 13 turn 753


Notice that somehow player 13 attacked player 4 during player 3's turn. :confused: Play continues from 13 up through 18 before it is now the human's turn again. Notice that turn 753 has not ended yet and player 5 hasn't had a turn yet either.


End player 14 turn 753
End player 15 turn 753
Player 16 Mongolian attacks Player 4 a couple of times
End player 16 turn 753
End player 17 turn 753
End player 18 turn 753


Here I simply end the turn for the human to let the madness continue. Players 4 and 5 get their turn 753, but then players 6-18 get another crack at turn 753 for good measure.

At this point we return to the normal universe: turn 753 ends, players 0-3 take their turn 754, and it's now player 4's turn 754.


End player 4 turn 753
End player 5 turn 753
End player 6 turn 753
End player 7 turn 753
End player 8 turn 753
End player 9 turn 753
End player 10 turn 753
End player 11 turn 753
End player 12 turn 753
End player 13 turn 753
End player 14 turn 753
End player 15 turn 753
End player 16 turn 753
End player 17 turn 753
End player 18 turn 753
End game turn 753
End player 0 turn 754
End player 1 turn 754
End player 2 turn 754
Player 3 Egyptian attacks Player 7 once
End player 3 turn 754


I ended the turn to see what happens, but the next round went exactly as above.

The only part that is messed up is turn 753 from the point where England splits off the colony. Some players get two turns, but everyone eventually gets a turn. Once the game turn ends, however, everything goes back to normal. :goodjob:

Was this game started under 3.17? Can someone that still has 3.17 try it and see what happens? If you can, copy CvEventManager to your CustomAssets/Python folder and add these two bold lines:


def onEndGameTurn(self, argsList):
'Called at the end of the end of each turn'
iGameTurn = argsList[0]
CvUtil.pyPrint("End game turn %d" % iGameTurn)

def onEndPlayerTurn(self, argsList):
'Called at the end of a players turn'
iGameTurn, iPlayer = argsList
CvUtil.pyPrint("End player %d turn %d" % (iPlayer, iGameTurn))

Yes, I have seen this happen before ... civs fill in dead player slots a lot in my Revolution mod. This multiple turn thing seems crazy, but its true!

The root of the problem before came from the fact that CvPlayer::init() calls CvPlayer::setAlive(true). Inside this function there's the following snippet (broken on to multiple lines for convenience):


if (GC.getGameINLINE().isMPOption(MPOPTION_SIMULTANEO US_TURNS)
|| (GC.getGameINLINE().getNumGameTurnActive() == 0)
|| (GC.getGameINLINE().isSimultaneousTeamTurns() && GET_TEAM(getTeam()).isTurnActive()))
{
setTurnActive(true);
}


Last time I ran into this, I was creating a new rebel civ in between player turns and so no one's turn was active. This caused the new civ's turn to be set active based on the code above. Then, once that happened the game let all the civs with higher player numbers have a second play for the turn.

It's a weird bug.

So, are colonies created while the motherland civ's turn is still active? It seems so from EF's log above, so we may have a different cause of the same symptoms here ... not sure.

But why the bouncing around? And how do we correct this?

So this isn't caused by 3.19? It worked before because new colonies could only appear at the end of the list of players?

To fix this bug, I would suggest simply moving the call to AI_doSplit() from CvPlayerAI::AI_doTurnPost to the very end of CvPlayerAI::AI_doTurnUnitsPost.

honestly - why fix it! This is a delightful bug that we should all enjoy!

We're being watched :mischief: ... thanks alexman!

What is the current best approach?

Play the BTS version 3.17 and the .21 unofficial patch? at least for now? which is what I have,

and wait a bit until BTS version 3.19 gets a new unofficial patch?

or play BTS 3.19 with no unofficial patch applied is better?

What is the current best approach?

Play the BTS version 3.17 and the .21 unofficial patch? at least for now? which is what I have,

and wait a bit until BTS version 3.19 gets a new unofficial patch?

or play BTS 3.19 with no unofficial patch applied is better?

I would go BTS 3.19 + BBAI 0.77

I would recommend BTS 3.19 and BBAI 0.78 ... which will be posted in the 12 hours.

Another old one which may not have been fixed in 3.19, it's in BBAI and is now marked with UNOFFICIAL_PATCH:

http://forums.civfanatics.com/showthread.php?t=281541

Hope this stuff makes it into RevolutionDCM soon.

Hi

I'm not sure if this is an issue or not. It could just be me BUT I think one of the UP's did a lil tweak to make the forest heavy starts less ridiculous and now with 3.19 it seems like starts of like ALL but 1 or 2 tiles being forrrested are back.

Kaytie

I would recommend BTS 3.19 and BBAI 0.78 ... which will be posted in the 12 hours.

I might take a look at that. Right now I play 3.17 + BAT 1.2 (Graphics but includes BUG) + UP 0.21 + Blue Marble.

I guess I am thinking that to evolve from this go to 3.19 with a (needed?) new UP and that other mods are compatible with 3.19. Is anybody looking at or working on a 3.19 UP? Or is it obsolete at this point? Does not seem to be completely clear yet.

:stupid:
We should PM the mods and get them to switch stickys so the most current version is up top. As the format is right now, it probably is confusing.

Thanks again for looking in to the colony problem, but whilst I'm on a roll...

Has anyone else not always been getting a "The enemy has been spotted near..." message? Seems to happen for some SoDs stationed next to one of my cities, but in an other civs culture, and always for horseman and knights (perhaps it's problem with the mounted units?) which still remain unspotted even if they then cross in to my culture. Save and screenshot attached.

Thanks.

Another bug I have found in 3.19: Mass upgrade isn't working properly. Even when I have enough money to upgrade all units in the map, pressing "ALT + upgrade" only upgrades a few ones. Has anyone experienced this?

(Edited) I have attached a savegame. I'm trying to upgrade ALL riflemen (10) to infantry and I have enough money.

When I do ALT + upgrade selecting the rifleman in Amsterdam, only 5 riflemen are upgraded
When I do ALT + upgrade selecting the rifleman in Lagos, 9 riflemen are upgraded


This behaviour is very odd...

I suggest you provide a save. No way to track the bug otherwise.

Another bug I have found in 3.19: Mass upgrade isn't working properly. Even when I have enough money to upgrade all units in the map, pressing "ALT + upgrade" only upgrades a few ones. Has anyone experienced this?

Note that you cannot upgrade units outside of your culture and units which haven't got movement points left. Another reason that you cannot upgrade is because the area where you're trying to upgrade isn't connected to the right resources.

If all of this isn't the case, then upload a savegame so that the issue can be tracked down.

Note that you cannot upgrade units outside of your culture and units which haven't got movement points left. Another reason that you cannot upgrade is because the area where you're trying to upgrade isn't connected to the right resources.

If all of this isn't the case, then upload a savegame so that the issue can be tracked down.

No, it's not the case. All the units to upgrade have movement points left, they are inside my borders, I have enough money and I'm trying to upgrade riflemen to infantry so resources are not an issue.

I have posted a savegame on my previous post.

I can reproduce that bug - not all riflemen are upgraded when you hit alt+upgrade. I'm curious though, is ALT+upgrade meant to do all units globally or just all of that type on the selected tile? To be honest, I never use ALT+upgrade, if you couldn't already tell...

It used to upgrade all units that were possible to upgrade (were in your territory, still had a move left, and had access to alll necessary resources), been that way since vanilla. This must have been accidently messed up in the patch.

Hi

I can confirm that mass upgrade not working like it should too. There always seems to be a few units somewhere that SHOULD have been upgraded and somehow just dont now. It has happened in multiples games I have played since the patch.

Kaytie

When building a culture by hammer, if cultural border is expanded, hammer is used to not only culture, but also next queue item.:eek:

I hope that this bug is to be fixed by unofficial patch, please.

When building a culture by hammer, if cultural border is expanded, hammer is used to not only culture, but also next queue item.:eek:

I hope to fix this bug by unofficial patch, please.

lol that's bizarre. I can confirm I can reproduce that on my machine. Building culture, it seems if you queue something after the build culture, when the borders pop the culture is removed from the queue but the overflow is incorrectly applied to the next build. It might have made sense if it used as many hammers to get the border pop then the rest used for the unit (should be against the rules IMO) but at the moment it's double dipping. :D

Building culture, it seems if you queue something after the build culture, when the borders pop the culture is removed from the queue but the overflow is incorrectly applied to the next build.

Does the same thing when building :science: with something queued after it when the tech is completed?

When building a culture by hammer, if cultural border is expanded, hammer is used to not only culture, but also next queue item.:eek:

I hope that this bug is to be fixed by unofficial patch, please.

This is also true in BTS 3.17 (not that that makes it ok).

Does the same thing when building :science: with something queued after it when the tech is completed?

I'm away from my normal computer for a few days so someone else will need to check. I'd prefer not to speculate.

Found a bug in 3.19 that only shows up when you mod it:
Found an interesting bug/exploit/(feature?) in The Eugenics Wars, as the Aegis. This may be an intended feature, but no one's mentioned it so far, so:

The transport sequence unit is an invisible transport. It can move into enemy territory and over enemy units.

However, if I load a visible unit into the invisible transport, and, say, move it into an enemy city, the game tries to put a (loaded) visible unit and visible enemy units into the same plot at the same time, fails, and expells all the enemy units to a neighboring plot.

I then move my 15-speed unit out of the city, unload the unit, and take the city.

If you have enough units and Transport Sequences, you can do this to every city of a civ in one turn, defeat your enemy, and wipe out all their expelled forces, because they no longer control any cities.

Used this to (in one game) wipe out all three Eugenics civs in a couple of turns, (in another) to destroy a major civ in a turn.
It's my understanding that loaded units are not supposed to be visible and thus should not be bumping enemy units (even if not, I need to fix this).

This is also true in BTS 3.17 (not that that makes it ok).

Did you try in vanilla 3.17 or with the UP?


Everyone -

Thanks for all the reports of bugs or strange things! I'll be fixing what I can figure out for BBAI and have already marked over 20 fixes with UNOFFICIAL_PATCH in those sources. So, it shouldn't be much extra work for me to put out a new unofficial patch.

Don't know when it'll happen exactly, shouldn't be more than a week or two.

Another bug I have found in 3.19: Mass upgrade isn't working properly. Even when I have enough money to upgrade all units in the map, pressing "ALT + upgrade" only upgrades a few ones. Has anyone experienced this?

(Edited) I have attached a savegame. I'm trying to upgrade ALL riflemen (10) to infantry and I have enough money.

When I do ALT + upgrade selecting the rifleman in Amsterdam, only 5 riflemen are upgraded
When I do ALT + upgrade selecting the rifleman in Lagos, 9 riflemen are upgraded


This behaviour is very odd...

Well, that was a fun bug ... a classic example of the dangers of using pointers. The loop upgrading all units had a check for whether the loop unit was of the same type as the original unit, but used a pointer to the original unit. At some iteration of the loop, the original unit would be upgraded and what the pointer pointed to would change, short-circuiting the upgrade. Saving the unit type to upgrade before the loop was all it took to fix.

That bug's probably been there since day one ... good find :goodjob:

@jdog5000 - Not sure if this is the best place to discuss this stuff, but I'm pulling UP fixes from BBAI and plugging the ones I'm missing into BULL and had some questions.

CvPlayerAI.cpp

In AI_commerceWeight() this one is tagged only before the line that changes, but other lines are changed as it's also in a larger BBAI fix scope:


/************************************************** ***********************************************/
/** BETTER_BTS_AI_MOD 04/29/09 jdog5000 */
/** */
/** Bugfix, Cultural Victory AI */
/************************************************** ***********************************************/
// Adjustments for human player going for cultural victory (who won't have AI strategy set)
// so that governors do smart things
if (pCity != NULL)
{
if (pCity->getCultureTimes100(getID()) >= 100 * GC.getCultureLevelInfo((CultureLevelTypes)(GC.getN umCultureLevelInfos() - 1)).getSpeedThreshold(GC.getGameINLINE().getGameSp eedType()))
{
iWeight /= 50;
}
// UNOFFICIAL_PATCH
else if (AI_isDoStrategy(AI_STRATEGY_CULTURE3) || getCommercePercent(COMMERCE_CULTURE) > 80 )
...


The bold part is what's different, but there are other differences further down. Could you explain a) which lines are part of the UP and b) what is the fix here?

The fix in AI_civicTrade() seems to already be in 3.19. The "original code" in BBAI is not in my 3.19. My 3.19 has the BBAI UP fix (though the if() test is reversed).

3.19:


if (getCivicPercentAnger(getCivics((CivicOptionTypes) (GC.getCivicInfo(eCivic).getCivicOptionType())),tr ue) > getCivicPercentAnger(eCivic))


BBAI:


if ( getCivicPercentAnger(eCivic) < getCivicPercentAnger(getCivics((CivicOptionTypes)( GC.getCivicInfo(eCivic).getCivicOptionType())),tru e) )


It looks like the UP fix here can be removed in favor of the logically identical 3.19 code.

The fix in AI_getTotalFloatingDefendersNeeded() seems more like an AI improvement than a fix. Is there really a bug there? Yes, the original code doesn't consider the situation of intercontinental invasions nor being attacked on that island, but is one part a fix and another part AI improvement?

CvUnitAI.cpp

In paradrop(), a fix makes sure that iValue isn't decremented if the value of the resource is < 10. Should it use 0 instead of 1 as the minimum value. If the AI truly devalues the resource so much, why increase the value of the plot at all?:


if (NO_BONUS != pLoopPlot->getNonObsoleteBonusType(getTeam()))
{
iValue += std::max(1, GET_PLAYER(eTargetPlayer).AI_bonusVal(pLoopPlot->getBonusType()) - 10);
}

The loop upgrading all units had a check for whether the loop unit was of the same type as the original unit . . .

I can't seem to find where this happens so I can fix it in BULL. :confused: Could you please post the file/line or some other pointer, please? I looked for all references to CvUnit::upgrade(), but none seemed to relate.

Ah, is this handled by CvNetDoCommand::Execute()?

Also, I didn't see Alexman's proposed fix (http://forums.civfanatics.com/showpost.php?p=8177706&postcount=45) for the colony spawn bug introduced in 3.19 in BBAI 0.78. Is this in the next version?

CvPlayerAI.cpp

In AI_commerceWeight() this one is tagged only before the line that changes, but other lines are changed as it's also in a larger BBAI fix scope:

The bold part is what's different, but there are other differences further down. Could you explain a) which lines are part of the UP and b) what is the fix here?

This is to make life easier for a human player going for cultural victory, at 100% culture on the slider a city governor would previously value commerce lower than at say 90%, so pushing up the slider could actually slow down cultural victory a little.

New version of code to make it clearer:


/************************************************** ***********************************************/
/** UNOFFICIAL_PATCH 04/29/09 jdog5000 */
/** */
/** Poor behavior */
/************************************************** ***********************************************/
/* original bts code
else if (AI_isDoStrategy(AI_STRATEGY_CULTURE3))
*/
// Slider check works for detection of whether human player is going for cultural victory
else if (AI_isDoStrategy(AI_STRATEGY_CULTURE3) || getCommercePercent(COMMERCE_CULTURE) > 80 )
/************************************************** ***********************************************/
/** UNOFFICIAL_PATCH END */
/************************************************** ***********************************************/



The fix in AI_civicTrade() seems to already be in 3.19. The "original code" in BBAI is not in my 3.19. My 3.19 has the BBAI UP fix (though the if() test is reversed).


You're quite right, removed from future versions. Thanks for the careful checking!


The fix in AI_getTotalFloatingDefendersNeeded() seems more like an AI improvement than a fix. Is there really a bug there? Yes, the original code doesn't consider the situation of intercontinental invasions nor being attacked on that island, but is one part a fix and another part AI improvement?


I consider that to be an AI bug ... the intention was clearly to reduce defensive requirements on tiny islands, but there a many other scenarios which get detrimentally affected by the prior logic.

This kind of poorly constructed AI logic is perhaps kind of a gray area between bug fixing and AI improvement, but this particular one definitely crossed my personal threshold for what's a bug.


CvUnitAI.cpp

In paradrop(), a fix makes sure that iValue isn't decremented if the value of the resource is < 10. Should it use 0 instead of 1 as the minimum value. If the AI truly devalues the resource so much, why increase the value of the plot at all?:


For the UP, you're probably right, it should be 0. Firaxis decided to subtract 10, the value given to most happy or health resources an AI does not have access to, the value is lower (usually 2) if the AI does have the resource already. This shows they wanted paratroopers to only bother with important strategic resources like oil which have a much higher value.

For BBAI, I think 1 makes more sense. A plot with a non-obsolete bonus is a better target than a random plot in my mind, since, all other things being equal, denying happiness resources to your enemy is a useful operation.

I can't seem to find where this happens so I can fix it in BULL. :confused: Could you please post the file/line or some other pointer, please? I looked for all references to CvUnit::upgrade(), but none seemed to relate.

Ah, is this handled by CvNetDoCommand::Execute()?

Yes, that's where it is. Here's have I've got:


/************************************************** ***********************************************/
/** UNOFFICIAL_PATCH 07/08/09 jdog5000 */
/** */
/** Bugfix */
/************************************************** ***********************************************/
/* orginal bts code

for (CvUnit* pLoopUnit = GET_PLAYER(m_ePlayer).firstUnit(&iLoop); pLoopUnit != NULL; pLoopUnit = GET_PLAYER(m_ePlayer).nextUnit(&iLoop))
{
if (pLoopUnit->getUnitType() == pUnit->getUnitType())
*/
// Have to save type ahead of time, pointer can change
UnitTypes eUpgradeType = pUnit->getUnitType();
for (CvUnit* pLoopUnit = GET_PLAYER(m_ePlayer).firstUnit(&iLoop); pLoopUnit != NULL; pLoopUnit = GET_PLAYER(m_ePlayer).nextUnit(&iLoop))
{
if (pLoopUnit->getUnitType() == eUpgradeType)
/************************************************** ***********************************************/
/** UNOFFICIAL_PATCH END */
/************************************************** ***********************************************/


Also, I didn't see Alexman's proposed fix (http://forums.civfanatics.com/showpost.php?p=8177706&postcount=45) for the colony spawn bug introduced in 3.19 in BBAI 0.78. Is this in the next version?

It's in my working copy in the BBAI svn repository, which you might want to check out ... there are several other things in there, I cleaned up some of the ambiguous parts as well which I was going to get to before release of the new UP (maybe this weekend).

http://sourceforge.net/projects/civ4betterai/

Cool, my fix for that differs only by variable name. ;)

The fix in AI_getStrategyHash() says in the readme that it counts Destroyers as mobile anti-air, yet the code says that it only counts land units now. The old code


if ( kLoopUnit.getMoves() > 1 )


compared to the new code


if ( kLoopUnit.getDomainType() == DOMAIN_LAND && kLoopUnit.getMoves() > 1 )


agrees with the comment but disagrees with the readme. Which was the intention, and is a change needed?

This XML Event fix (http://forums.civfanatics.com/showthread.php?t=325832) should be included.

The fix in AI_getStrategyHash() says in the readme that it counts Destroyers as mobile anti-air, yet the code says that it only counts land units now. The old code


if ( kLoopUnit.getMoves() > 1 )


compared to the new code


if ( kLoopUnit.getDomainType() == DOMAIN_LAND && kLoopUnit.getMoves() > 1 )


agrees with the comment but disagrees with the readme. Which was the intention, and is a change needed?

The new code does what was intended. Counting Destroyers as mobile anti-air is inappropriate since they cannot defend attacking stacks against air attacks. The readme was probably supposed to say that it fixed the bug where destroyers were previously counted as mobile anti-air.

.. well at least it somewhat changed them :crazyeye:. And yes, just about all of them too :scan:.

I just had a closer look at the way latitudes are generated by map scripts. There is a method CvPlot::getLatitude(), which generates a value between 0..90 and there are several problems with it. .. well I found two ;):

1) plot.getLatitude() divides by GridHeight instead of (GridHeight-1), which results into the absence of the northpole. .. or at least the highest row of plots won't have the latitude 90 as would be expected. To illustrate this, here is an example for a map with height 6 (yes it's a very small map):
For heights (0,1,2,3,4,5) plot.getLatitude() generates something like (90,60,30,0,30,60) with a south pole @ 0 and an equator @ 3, but no north pole. What should be generated is (90,54,18,18,54,90) with the equator between 2 and 3 and poles at both ends.

2) Actually what plot.getLatitude() will generate is (90,62,31,0,28,59), because of rounding errors introduced by using only int and multiplying/dividing by 100.

Here is the Python pseudo-code of how it is now
# get latitude 0..90 for given plot - corrects error in build-in plot.getLatitude()
# Precision really should be 10000 or greater!
def getLatitude( plot ):
if CyMap().isWrapX() or (not CyMap().isWrapY()):
iLat = plot.getY() * 100 / CyMap().getGridHeight()
else:
iLat = plot.getX() * 100 / CyMap().getGridWidth()
iLat = iLat * ( CyMap().getTopLatitude() - CyMap().getBottomLatitude() ) / 100
return abs( iLat + CyMap().getBottomLatitude() )

Here is the Python pseudo-code of how it should be
# get latitude 0..90 for given plot - corrects error in build-in plot.getLatitude()
# using float - this gives the best results
def floatGetLatitude( plot ):
if CyMap().isWrapX() or (not CyMap().isWrapY()):
iLat = float(plot.getY()) / (CyMap().getGridHeight() - 1)
else:
iLat = float(plot.getX()) / (CyMap().getGridWidth() - 1)
iLat = iLat * ( CyMap().getTopLatitude() - CyMap().getBottomLatitude() )
return abs( int( round( iLat + CyMap().getBottomLatitude() ) ) )

I'm not quite done here as the designers were pretty consistent in the way latitudes are calculated. In CvMapGeneratorUtil.py both, the TerrainGenerator and the FeatureGenerator make the same error of using (getGridWidth()) instead of (getGridWidth()-1). Only two lines need changing though.

Note that unless the mod provides it's own CvMapGeneratorUtil.py, the map generator uses the one in ..civilization 4\warlords\assets\python if installed or ..civilization 4\assets\python - BtS doesn't provide it's own.
Also note that TerrainGenerator deliberately returns a fuzzy result.

Changes in CvMapGeneratorUtil.py
CvMapGeneratorUtil.TerrainGenerator.getLatitudeAtP lot():
# lat = abs((self.iHeight / 2) - iY)/float(self.iHeight/2) # 0.0 = equator, 1.0 = pole
lat = abs( ( (self.iHeight-1)/2.0 ) - iY ) / ( (self.iHeight-1)/2.0 ) # 0.0 = equator, 1.0 = pole

CvMapGeneratorUtil.FeatureGenerator.getLatitudeAtP lot():
# return abs((self.iGridH/2) - iY)/float(self.iGridH/2) # 0.0 = equator, 1.0 = pole
return abs( ( (self.iGridH-1)/2.0 ) - iY ) / ( (self.iGridH-1)/2.0 ) # 0.0 = equator, 1.0 = pole

At last a modified fractal.py map script for you to play with (that's what we do, don't we). Start a game and look into '...\My Documents\My Games\Beyond the Sword\Logs\PythonDbg.log' to see the results of different latitude generators.

jdog5000,

Two XML errors in 3.19 that should be fixed:

1) In Civ4UnitClassInfos.xml, the Ship Of The Line entry reads as follows:

<UnitClassInfo>
<Type>UNITCLASS_SHIP_OF_THE_LINE</Type>
<Description>TXT_KEY_SHIP_OF_THE_LINE</Description>
<iMaxGlobalInstances>-1</iMaxGlobalInstances>
<iMaxTeamInstances>-1</iMaxTeamInstances>
<iMaxPlayerInstances>-1</iMaxPlayerInstances>
<iInstanceCostModifier>0</iInstanceCostModifier>
<DefaultUnit>UNIT_SHIP_OF_THE_LINE</DefaultUnit>
</UnitClassInfo>

The Description entry should read TXT_KEY_UNIT_SHIP_OF_THE_LINE. To my knowledge this bug only affects modded games.

2) In Civ4ArtDefines_Unit.xml, most (all?) of the unique units added in BtS have an incorrect <TrainSound> entry. It should read AS2D_UNIT_BUILD_UNIQUE_UNIT to match the unique units from Vanilla and Warlords?

Cheers,
ripple01

I found a small bug when scrolling through the various barbarian events in the cvrandominterface.py file .

in cvrandominterface.py

def getHelpThGoths1(argsList):


should of course be (at least according to civ4eventinfos) (</3 typo errors):

def getHelpTheGoths1(argsList):

Or you could put the typo into the XML file since the patch already requires a new XML file.

Update: Unofficial Path for 3.19 version 1.0 will be up on Wed or Thurs, including fixes for the three issues just above this post.

All the SDK changes for the patch have been checked in to the BBAI sourceforge repository and tested, I just need to split the new UP into its own project and give it one final pass of testing as a standalone mod.

Version 1.0 has been released! The download links will always be in the FP of this thread, for convenience:

Download (http://forums.civfanatics.com/downloads.php?do=file&id=12819)

SDK files if you're merging mods (http://forums.civfanatics.com/downloads.php?do=file&id=12820)

Also, here is the full change list for 1.0:

Unofficial Patch v 1.0 fixes:

1) CvUnitAI::AI_paradrop - fixed bug with valuation of terrain bonuses causing paradrops to avoid bonuses in some circumstances when intention is clearly to encourage landing on bonuses and pillaging

2) CvUnitAI::AI_settleMove - fixed bug when settler cannot reach a city site in an area (blocked by mountains, other player). Caused settler to wait infinitely in city instead of loading into transport.

3) CvTeamAI::AI_doWar - use bFinancesProLimitedWar for limited war calc instead of max war version

4) CvPlayerAI::AI_calculateUnitAIViability - Fixed incorrect integer usage which caused function return to be meaningless, blocked AI logic for building privateers

5) CvPlayerAI::AI_isFinancialTrouble, CvCityAI::AI_updateBestBuild, and CvPlayerAI::AI_getMinFoundValue - Fixed bug in calculating expenses when AI has negative gold per turn

6) CvPlayerAI::AI_getTotalFloatingDefendersNeeded - Fixed poor decision by AI if it has captured one or two cities on someone else's continent, it would minimally defend its new cities

7) CvPlayerAI::AI_getStrategyHash - Fixed incorrect counting of destroyers as mobile anti-air

8) CvPlayerAI::AI_doDiplo - Fixed issue where team is sneak attack ready but hasn't declared war, AI would still demand tribute. If other team accepted, it blocked war declaration for 10 turns but AI still launched invasion and was then bounced when it could eventually declare.

9) CvPlayerAI::AI_bestPlotEspionage - relevant weights are 0, +- 50, +- 100, so comparison for Agg AI should be < 51 instead of < 50

10) CvPlayerAI::AI_commerceWeight - Governors now do smarter things when human player sets culture slider to 100%

11) isPotentialEnemy in CvGameCoreUtils - Fixed bug leading to AI launching invasions when unable to declare war, troops eventually got bounced when war was declared

12) CvGame::addPlayer - No longer invalidate color choice for added civ if it's taken by this player slot

13) CvCityAI::AI_cityThreat - Fixed bug when AI is running crush strategy, wrong int was divided

14) CvCity::popOrder - Kept overflow fixes from 3.17 unofficial patch

15) CvCityAI::AI_neededDefenders - Improved efficiency and minor tweaks

16) CvPlayerAI::AI_isFinancialTrouble, CvCityAI::AI_updateBestBuild, and CvPlayerAI::AI_getMinFoundValue - Fixed bug where spending gpt for resources reduced calculation of expenses (thanks DanF5771)

17) CvPlayerAI::AI_conquerCity - Fixed bug where cityAcquiredAndKept event reported wrong player in some circumstances (thanks Maniac)

18) CvPlayerAI::AI_unitValue - Fixed potential crash bug in looking up AI_unitValue for UNITAI_MISSIONARY units without passing a valid CvArea*

19) CvUnit::isIntruding - Kept 3.17 unofficial patch feature that vassal spies are never caught in master's territory

20) CvUnit::collateralCombat - Kept 3.17 unofficial patch feature which allows mods to enables barrage promotions for collateral damage units

21) CvUnitAI::AI_assaultSeaMove - Fixed bug where unit type was used when unit AI type was intended

22) CvUnit::canMoveInto - Capturing an undefended city is now considered an attack action, like capturing a worker. Mainly affects paratroopers, who now cannot capture undefended cities right after paradrop. Previously, paratroopers could capture undefended cities but not cities with only a worder or ship in them.

23) CvTeam::shareCounters - Fixed bug in permanent alliances when No Tech Brokering is turned on, where a civ would often become unable to trade a tech they had researched after entering a permanent alliance.

24) CvNetDoCommand::Execute - Fixed bug causing "Upgrade All" action to sometimes only do a fraction of the available units.

25) CvPlayerAI::AI_unitValue - Stealth boats do not make good escorts since they often don't defend, so AI is now very unlikely to use them as escorts for transports.

26) CIV4UnitInfos.xml - Removed UNITAI_ESCORT_SEA from Stealth Destroyer (stealth doesn't defend first, so not a good escort). Set iPower weighting for Missile Cruiser to much more appropriate 42 from 14 (iPower ratings for boats were increased by Firaxis a while back, but they missed this one).

27) CvCityAI::AI_yieldValue - Fixed issue causing city governor and AI to heavily weight food when building gold or any other form of commerce. Produced unexpected and poor results for human player, did not help AI either.

28) CvCity::setCultureLevel - Removed behavior where a city building culture would cancel order when reaching next level. Completely unnecessary for AI, human player is unlikely to be expecting this. Also appears to cause a bug with extra overflow production.

29) CIV4UnitClassInfos.xml - Fixed description entry for ship of the line (thanks ripple01)

30) CIV4ArtDefines_Unit.xml - Fixed many discrepancies in TrainSound between unique and non-unique units (thanks ripple01)

31) CvRandomEventInterface.py - Fixed function name from getHelpThGoths1 to getHelpTheGoths1 (thanks Chrill)

32) CvMapGeneratorUtil.py - Fixed function getLatitudeAtPlot in both FeatureGenerator and TerrainGenerator so that it properly returns 1.0 for both north and south poles and correctly locates equator (thanks Temudjin)

33) CvPlot::getLatitude - Fixed so that it properly returns highest latitude value for both north and south poles and correctly locates equator, improved integer rounding errors which caused slight skew in terrain types between northern and southern hemisphere (thanks Temudjin)

27) CvCityAI::AI_yieldValue - Fixed issue causing city governor and AI to heavily weight food when building gold or any other form of commerce. Produced unexpected and poor results for human player, did not help AI either.TMIT will be pleased.

On the whole, great work jdog5000! :goodjob:
I'll be merging this into any mods I have soon.

Firstly, thanks for the patch! :worship:

The only small quibble is the "removed behavior where a city building culture would cancel order when reaching next level." I personally use this a lot, usually after capturing a city (I queue build culture followed by culture buildings etc.), but also sometimes for cities bordering other civs. Maybe it's just me who does this(?), but I prefer the old behaviour (I know I'll miss a border pop having to rely on the event log, and having to manually switch production on all the cities I capture will be a pain). Perhaps I'm just being lazy or have bad strategy..., but otherwise great work.

Great job jdog.

I was wondering if BTS 3.19 had copied the fix to the starting area enhancement/equaliser algorithm that was present in unofficial patches 0.21 and 0.19.1 of BTS 3.17. Without this fix to the starting area enhancement, the algorithm would add forest to the starting area and then tried to add food resources and such to the starting area. Because of the already present forests, the food resources couldn't be added anymore. The unofficial patch changed something in this algorithm so that there would still be room for the food resources that were supposed to be added to equalise the starting areas.

The only small quibble is the "removed behavior where a city building culture would cancel order when reaching next level." I personally use this a lot, usually after capturing a city (I queue build culture followed by culture buildings etc.), but also sometimes for cities bordering other civs. Maybe it's just me who does this(?), but I prefer the old behaviour (I know I'll miss a border pop having to rely on the event log, and having to manually switch production on all the cities I capture will be a pain). Perhaps I'm just being lazy or have bad strategy..., but otherwise great work.I use this too - a valuable addition that stops you building culture for nothing (ie after first border pop).

I use this too - a valuable addition that stops you building culture for nothing (ie after first border pop).

Alright, I'll see if I can find a way to fix the extra production overflow you've been getting by doing this ... didn't know you'd been cheating, did you? :p

That's okay, the AIs been cheating too (even though it doesn't need this feature at all).

1st post needs to include a copy of the changes noted in the readme.

Great job jdog.

I was wondering if BTS 3.19 had copied the fix to the starting area enhancement/equaliser algorithm that was present in unofficial patches 0.21 and 0.19.1 of BTS 3.17. Without this fix to the starting area enhancement, the algorithm would add forest to the starting area and then tried to add food resources and such to the starting area. Because of the already present forests, the food resources couldn't be added anymore. The unofficial patch changed something in this algorithm so that there would still be room for the food resources that were supposed to be added to equalise the starting areas.

Yes, the starting location changes came over to 3.19.

3.19 had nearly every change from UP 0.21. Unfortunately I didn't keep a log of the ones that didn't make it as I looked them over. :(

Yes, the starting location changes came over to 3.19.

Thanks. One of the changes mentioned in the 3.19 change log looked as if it could be about the starting changes but it only mentioned sea-starts so I wasn't sure.

3.19 had nearly every change from UP 0.21. Unfortunately I didn't keep a log of the ones that didn't make it as I looked them over. :(

I was just checking for this small change as it wasn't clearly mentioned in the 3.19 change log. The problem with nearly every change is in the crucial word nearly. It causes you guys extra work to get things back to the UP standard in various mods. With the new UP things will hopefully become standardised again among various mods.

Hey, I am a noob with a question. I want to play a mod that requires patch 3.19. I cant downlaod the patch b/c I have the digital version. Will this unofficial patch allow me to play the mod?

Also, how do I install this up properly?

Hey, I am a noob with a question. I want to play a mod that requires patch 3.19. I cant downlaod the patch b/c I have the digital version. Will this unofficial patch allow me to play the mod?

Also, how do I install this up properly?

No, sorry. This mod requires 3.19 to be installed already, then makes a few tweaks and fixes on 3.19.

Thanks for the updates - is there any way we can get installation instructions? Sorry if I missed them.

Best wishes,

Breunor

In this thread http://forums.civfanatics.com/showthread.php?t=328325 The_J asks for help in debugging the Mars Now mod and I told him to build a a debug dll because it has been invalueble to me for degugging and in my response I told him the degug dll has been very good at directing me toward issues. But I realized there is a cause of critical crashes that gives no such information. If you screw up and leave a comma in the button path when there is no atlas or leave the comma out if there is an atlas you just get a crash with no assert. Would it be possible to add in a failed assert message for bad button paths? I realize this would only help modders but it seems like something that should throw an assert as it causes a critical bug. Anyway great job as always jdog, course I get all the benifits from the UP by using RevDCM :)

I would bet that's handled entirely in the EXE. :(

The loading of the XML is handled in the SDK. I wonder if it would be possible to add something there.

The loading of the XML is handled in the SDK. I wonder if it would be possible to add something there.

That's a good point. What exactly does it mean to have an atlas? Keep in mind that I am not an artist, but I am a little familiar with graphics files and formats. Is there a tutorial that covers the format those comma-filled strings take and what each piece means?

The atlas files contain a ton of buttons all in one file. This is what Firaxis uses; since Warlords they haven't even made stand-alone buttons. For stand-alone buttons, the Button entry looks line this:

<Button>Art\path to file\Stand Alone Button.dds</Button>

When the atlas files are used:

<Button>,Art\path to file\Stand Alone Button (not used for anything).dds,Art\path to file\Atlas File.dds,x location in atlas,y location in atlas</Button>

I was wondering why some had a leading , like that. So if there's a leading , the first file is ignored. Very odd, but understandable.

So the problem is when someone uses

Art/Blah/Blah/Blah.dds,

with that trailing comma? Or does the crash happen when the atlas is specified correctly but doesn't exist or has the wrong path/name?

Something might happen then, but more common would be leading comma. This is where new modders can be tripped up, as it looks like you can just delete the atlas stuff at the end and not worry about the starting comma. This is compounded by the fact that some art entries will work like that and never cause a crash. In general, if you can build it in a city, it will care about the leading comma.

Another two bugs and their fixes:

Event "Doctrine Overwhelm" option 1 not working. Related thread: http://forums.civfanatics.com/showthread.php?t=325832
Queued orders skipped after a "move to" order with no movement points remaining. Related thread: http://forums.civfanatics.com/showthread.php?t=326158


The first one an be easily added to the unofficial patch if you want.
The second one is present in Civ4 from vanilla version. I found the problem and coded a quick fix, but I think it can be recoded in a easier way.

A behaviour that IMO could be tweaked is the liberation of cities. I started this thread and uploaded a savegame: http://forums.civfanatics.com/showthread.php?t=328189

It's related to the relocation of an AI capital when its former capital has been conquered. Sometimes, the new capital selected is the largest AI city, even if it is clearly better to select a smaller city placed on the continent where the largest number of AI cities are.

Thanks for the links Pep, will take a look.

I think I found a typo in CvUnitAI.cpp:

In line 2139 IIRC:

pOldGroup->AI_seperateNonAI(UNITAI_ATTACK_CITY);

should be:

pOldGroup->AI_separateNonAI(UNITAI_ATTACK_CITY);

Wow, Arian... this may actually be a source of some CTD's I've had. Going through BBAI 0.75, there are 20 instances of seperate, and 30 instances of separate.

Great find! :)

pOldGroup->AI_seperateNonAI(UNITAI_ATTACK_CITY);

should be:

pOldGroup->AI_separateNonAI(UNITAI_ATTACK_CITY);

C++, Python, and most programming languages don't care what you name your variables and functions. If you misspell a word, it only matters that you misspell it when referring to the same function.

You can name a function AI_efhsehfuksekusefknuskfnu() for all the compiler cares. If you refer to it in some place as AI_efhsehfuksejusefknuskfnu(), the compiler will tell you that this second function doesn't exist and not produce a DLL.

Going through BBAI 0.75, there are 20 instances of seperate, and 30 instances of separate.

This would only matter if both of the above spellings were defined as functions in the same class. In that case, the compiler would use whichever one you named. However, I doubt this is the case.

There are four main functions dealing with separation, as I see it. They are:

void CvSelectionGroupAI::AI_separate()
void CvSelectionGroupAI::AI_seperateNonAI(UnitAITypes eUnitAI)
void CvSelectionGroupAI::AI_seperateAI(UnitAITypes eUnitAI)
void CvSelectionGroupAI::AI_seperateEmptyTransports()

They are used differently, of course, but some of code has Firaxis comments that "pointers could become invalid". This is besides the spelling issue, actually (which I no longer think is the culprit of the ctds I've had). How do invalid pointers affect program execution? And would there be a difference in behaviour from XP to Vista? I've had some strange crashes involving unit_ai's that have been extremely difficult to reproduce, but I do have older saves that reproduce ctds. The game setup is extremely simple: Duel maps, two civs not at war. Two AI units in a stack (horseman and catapult), and the game crashes if the catapult has UNITAI_ATTACK_CITY at the end of the AI's turn. Change the AI or remove the catapult, the crash doesn't happen...

Sorry for the OT, this probably really doesn't belong in the UP thread. :blush:

Say you have code like:


void CvUnitAI::AI_someFunction()
{
CvSelectionGroupAI* pGroup = getGroup();
pGroup->AI_seperateNonAI(UNITAI_ATTACK_CITY);

a bunch of other stuff
}


After the call to AI_seperateNonAI, the group pointed to be pGroup may no longer be a valid group ... the group may have split, the leader could have been kicked out, etc, so the place that pGroup points to may no longer be a group at all. If your code used pGroup again in this function after the call to AI_seperateNonAI, you'd at best get occasional weird behavior and quite possibly a CTD.

There are quite a few examples of this, where functions create new objects and invalidate previous ones. CvPlayer::acquireCity( CvCity* pCity, ...) is one which springs to mind.

The patch looks great, but I have no idea how to install it. Also, I'm using a mod, so does that make a difference for how I install this?

Two options: Like a mod, or by replacing the default files. You can check out the BBAI directions (http://forums.civfanatics.com/downloads.php?do=file&id=9819), works the same with slightly different names.

If you're using a mod, then things get more complicated. If the mod has a custom DLL, then there's nothing you can do (short of compiling a merged DLL yourself ...). If it doesn't have its own DLL, then you can follow the replacing BTS instructions and probably get most of the changes.

What you really need it to convince the mod maker to merge in the UP.

Team score is simple sum of the member player's score.
Player score consists of populations, lands, wonders and techs.

But team tech is shared by all members of the same team.
Therefore, team score counts only one tech more than one times.

A team which includes more than one player has a big advantage in time victory.
Is this a bug?

That's a good point denev, it does produce a bias towards teams with more players on them. That said, research costs do scale up based on how many players are on the team

Unfortunately, looking through the code I don't see a robust and easy way to fix this. The score calculation for each player is done in Python so that it's easily moddable. In the SDK function GvGame::updateScore where team scores are calculated, it simply isn't possible to get an accurate tech score number for each player as there are a number of adjustments on tweaks to CvPlayer::getTechScore in Python.

The only robust thing I can see to do is in CvPlayer::getTechScore divide the raw tech score by the number of players on the team. This will in turn cause the individual player scores for any team members to go down because their tech is no longer their own. However, the team score would then still be the sum of the player scores, which makes sense.

Anyone else want to weigh in on this?

Once I tried to fix this problem at FfH2 because Basium gived a lot amount of score bonus to host civilization.
This mini mod calls Python function from DLL.
I guess it may not be smart way, but I hope it serves as a reference.

I found a bug in city production automation that can reduce your gold nearly to 0 to gold-rush a building:

http://forums.civfanatics.com/showthread.php?p=8338414#post8338414

It seems this bug also applies to AI Civs.

Another two bugs and their fixes:

Queued orders skipped after a "move to" order with no movement points remaining. Related thread: http://forums.civfanatics.com/showthread.php?t=326158


The first one an be easily added to the unofficial patch if you want.
The second one is present in Civ4 from vanilla version. I found the problem and coded a quick fix, but I think it can be recoded in a easier way.
I reported the queued order bug over two years ago. It was introduced with Warlords, and did not exist in vanilla up to patch 1.61. I believe the bug was back-ported to vanilla with a later patch, but don't recall exactly.

I haven't played Civ4 in a while, and found this post while reading up on the latest UP for the latest OP. Kind of amusing that such a prominent bug is still there after all this time.

Version 1.10 has been posted!

There are a number of tweaks and fixes in this version, including two very important bugs. The first bug permanently blocked access to any trade routes after a city was lost, resulting in lower trade yield for civs that had lost cities in war or set free a colony. The second set of bugs affected the AI's handling of population and gold rushing, and was a significant drain on AIs running Universal Suffrage. Thanks to RedFury, DanF5771, and Pep for their bug hunting!

Also, two prior changes were tweaked by popular demand. With this version, Paratroopers are always able to capture defenseless cities after dropping and a city set to build culture will ask for new orders after cultural borders expand.

Version 1.10 fixes:

- CvGame::addPlayer - Restored 3.17 UP changes forcing clearing of old player names, civ descriptions, etc when placing a new player in a previously occupied slot.

- CvCity::setCultureLevel - Now behavior where a city building culture would cancel order when reaching next level only applies to human player. It was completely unnecessary for AI and caused a small bug with extra overflow production.

- CvUnit::canMoveInto - Changed to allow paratroopers to capture workers and undefended cities after paradropping. Also allows units with multiple moves to capture multiple tiles of non-combat units like workers.

- CvCity::kill - When a player loses a city, that city now clears all of its trade route claims so that they can be taken by other cities owned by the player. Previously, these trade routes were permanently lost. (Thanks RedFury and DanF5771)

- CvUnitAI::AI_spreadReligionAirlift and CvUnitAI::AI_spreadCorporationAirlift - AI will no longer airlift multiple of the same kind of missionary or executive to the same target spread city on the same turn

- CvAdvisorUtils.py - Advisor will no longer bug for city liberation to player you're at war with.

- CvDomesticAdvisor.py - City liberation to player you're at war with no longer offered.

- CvDLLButtonPopup::launchFreeColonyPopup - Removed cities whose liberation player you are at war with from popup.

- CIV4EventInfos.xml - In EVENT_OVERWHELM_DONE_1, removed references to non-existent Python functions which blocked this option. (thanks Pep)

- CvSelectionGroup::continueMission - Fixed issue causing units with multiple orders to forget their later orders under certain circumstances. (thanks Pep)

- CvCityAI::AI_doHurry - Fixed several bugs where AI would incorrectly think it was getting a great deal on a pop/gold rush when it was actually doing the other kind of rush. (thanks Pep)

For those of you keeping score at home ...

There's also a bug with Permanent Alliances and espionage points. If China and Greece merge, they will always keep the greater of their individual espionage point totals towards you, but what you get depends on whether China is merged into Greece or Greece into China.

Thanks to StMikael and phungus420 for the report.

Version 1.10 has been posted!

:worship:

Just had an AI spawn a colony using a dead civs slot with no problems - great work, much appreciated.

:thanx:

// XXX marble and stone???

// Unofficial Patch Start
// * Fixed bug where AI uses player's ID instead of attitude when evaluating deals. [DanF5571]
eAttitude = GET_PLAYER(getID()).AI_getAttitude(ePlayer);
// Unofficial Patch End

In Unofficial 3.17 patch, CvPlayerAI.cpp 6930 line has been changed like above.

But this Unofficial 3.19 patch has changed nothing it (6940 line).
Is this intentional?

// XXX marble and stone???

// Unofficial Patch Start
// * Fixed bug where AI uses player's ID instead of attitude when evaluating deals. [DanF5571]
eAttitude = GET_PLAYER(getID()).AI_getAttitude(ePlayer);
// Unofficial Patch End

In Unofficial 3.17 patch, CvPlayerAI.cpp 6930 line has been changed like above.

But this Unofficial 3.19 patch has changed nothing it (6940 line).
Is this intentional?

When I was looking at the 3.17 UP 0.21 and saw this comment, I compared it to the original line which I'm guessing was this:


eAttitude = AI_getAttitude(ePlayer);


That looks just fine to me. I suspect that when Firaxis was building the 3.19 patch they made the same decision and kept the old (working) code. They took almost every fix from UP 0.21, so I assume they purposely skipped this one.

That looks just fine to me. I suspect that when Firaxis was building the 3.19 patch they made the same decision and kept the old (working) code. They took almost every fix from UP 0.21, so I assume they purposely skipped this one.

Thanks EmperorFool, I understood well;)

While trying to track down a Stack Attack crash in BULL I came across this bug in 3.19's CvGameCoreUtils.cpp on line 2005:


void getActivityTypeString(CvWString& szString, ActivityTypes eActivityType)
{
switch (eActivityType)
{
...
case ACTIVITY_INTERCEPT: szString = L"ACTIVITY_SENTRY"; break;
...
}
}


That should be L"ACTIVITY_INTERCEPT", but I have no idea what ramifications fixing it has. I do notice that I fixed it in BUFFY's version of BULL, and it hasn't seemed to cause any problems yet.

Note: getActivityTypeString() isn't exposed to Python, nor is it called within the SDK. Perhaps the EXE uses it for something.

Pretty sure these functions are just intended for on screen debug output, several of them are also then simply not used in the relevant debug printing code ... seems to be the case with this one.

Will add it anyway.

I have a small request for the next version of the Unofficial Patch.

When putting a negative value for State Religion happiness, it creates a funky looking result in the Civics screen. However, it works just fine in game.

Example:

In CIV4CivicsInfos.xml:

<iNonStateReligionHappiness>-1</iNonStateReligionHappiness>


Looks like this in the Civics Screen:
http://forums.civfanatics.com/showpost.php?p=8390698&postcount=3222

However, in the game, by doing this you do infact will have +1 Unhappy citizens per Non-State Religion per city.


It's just a cosmetic error, but just thought I'd mention it.

It's a mismatch between the "TXT_KEY_CIVIC_NON_STATE_REL_HAPPINESS_NO_STATE" <Text> element and the code. When the civic has a state religion, the code doesn't pass in the # and type of happy/unhappy the civic produces, but the <Text> expects them. The fix is trivial:


if (GC.getCivicInfo(eCivic).getNonStateReligionHappin ess() != 0)
{
if (GC.getCivicInfo(eCivic).isStateReligion())
{
szHelpText.append(NEWLINE);
szHelpText.append(gDLL->getText("TXT_KEY_CIVIC_NON_STATE_REL_HAPPINESS_NO_STATE", abs(GC.getCivicInfo(eCivic).getNonStateReligionHap piness()), ((GC.getCivicInfo(eCivic).getNonStateReligionHappi ness() > 0) ? gDLL->getSymbolID(HAPPY_CHAR) : gDLL->getSymbolID(UNHAPPY_CHAR))));
}
else
{
szHelpText.append(NEWLINE);
szHelpText.append(gDLL->getText("TXT_KEY_CIVIC_NON_STATE_REL_HAPPINESS_WITH_STATE", abs(GC.getCivicInfo(eCivic).getNonStateReligionHap piness()), ((GC.getCivicInfo(eCivic).getNonStateReligionHappi ness() > 0) ? gDLL->getSymbolID(HAPPY_CHAR) : gDLL->getSymbolID(UNHAPPY_CHAR))));
}
}


Looking at the two <Text> entries, I think they are backwards. It always bothered me that Free Religion says, "per Non-State :religion:," given that all religions will be non-state religions since you cannot have a state religion. Now I see why. The other <Text> doesn't say Non-State, but it makes more sense for that case.

The names of the <Text> entries match what they mean in my eyes, so the code needs to swap which it uses when. The easiest is to just add a not (!) to the isStateReligion() test:


if (!GC.getCivicInfo(eCivic).isStateReligion())

Cool, thanks for the solution EF!

Having had some time in the last couple of months, I returned to the game and learned to elementary Mod my old trusty 161 Civ4 vanilla version.

Reaching a bit of proficiency after hours of silently trawling this site, I was enthused to buy BtS and I got a complete CiV4 pack version 313 BtS, Warlords and vanilla in one pack.

Following advise from this forum I then Uploaded the Official patch 319. First tried through the game but after 3 unsuccesful uploads, I got a copy from CFC which only worked when I removed all the old games and re-loaded BtS played 1 turn and then OP 319 worked.

Then having read about the bugs and the valiant efforts of the people in this sub-forum to fix patch 319 with the Unofficial 319 Patch, I down loaded your version 1.1 and installed that as well.

I now have some interesting results the whole GUI interface dissapeared, :crazyeye: after I installed the UP 319 v1.1. I now have a workable map with animated units but nothing else!! This happens whether it is a new game or an old saved one under the OP319. The HUD I believe it is called dissapeared.

Can anybody help please?

PS I used your recomendation to change the Firexis original files. I did that by renaming the old ones and I can and have reverted back to them whence the game functions normally.

One point that I noticed is that the OP-319 I downloaded from CFC although it says it is version 319 within the game, works off a 3.1.7 Dll whilst your UP-319 dll is a 3.1.9 version.:confused:

Thank you if you can answer my querry.

The 3.1.7 vs 3.1.9 DLL thing is not a problem, Firaxis just forgot to update the field in the resource file which fills in that property.

When the interface disappears that means something has gone wrong in the Python portion of you setup. If you went the renaming route, you have to have renamed the individual Python files, not whole folders. Same is true for the XML parts. Perhaps double check that you have renamed only the four Python files in the UP and placed the four new versions in their place, and similarly for XML.

The most sure approach to getting back to functionality is to reinstall ... annoying, but guaranteed to work if it comes to that.

You can also

Thanks for the prompt reply.
I in fact renamed the Python folder to "Original Python", etc. for the rest before I copied your folders in their place.

It was easy to retrieve functionality I renamed your folders to "UP319 Python" etc. and erased the word Original from the old ones. Voila, I was back in business shortly.

But that is not the point. The point is that I think you went to an extraordinary amount of trouble for us to be able to use a good upgrade and I would gratefully like to do so.

Anyway I reinstalled as I wanted to include some art work that was not functioning correctly and Noticed when Uninstalling that the OP 319 sits independent of my CIV4 Complete. Is it possible that this where my problem lies? That I have been working with the CIV4 Complete files and not the patch? If so how do I integrate them? And do I need to? or should I make a simple "Shortcuts Menu" and only use the older material for art and defines copy-overs?

Thank you again for being prompt. Regards.

EDIT:
Thank you for the idea of renaming each file and manually installing it in place instead of the whole folder that I did previously. It worked.

I in fact renamed the Python folder to "Original Python", etc. for the rest before I copied your folders in their place.

If you mean that you renamed the folder Beyond the Sword/Assets/Python, this was the problem. This is where the core game interface files reside. The UP only replaces a couple of those files, so swapping the entire folder removes most of the game!

Out of curiosity is it possible to create more promotion levels for units? Sometimes when I have a GG attached to a Cataphract with Blitz I run out of levels so can't get the Drill promotions that come after upgrading to Gunship :(.

And if it is possible would it be balanced/worthwhile sticking it in the unofficial patch?

Thanks.

Likely needs a mod. Fall From Heaven adventurer units accumulate one point per turn, up to 100, for example. It would seriously alter gameplay, so maybe shouldn't be in the patch.

Out of curiosity is it possible to create more promotion levels for units? Sometimes when I have a GG attached to a Cataphract with Blitz I run out of levels so can't get the Drill promotions that come after upgrading to Gunship :(.

And if it is possible would it be balanced/worthwhile sticking it in the unofficial patch?

Thanks.

I've played this game so much, I'm surprised I haven't seen this before, and I really like making attacking Warlords as well. Are you sure there is a unit level cap? Do you have a save with a unit hitting this cap? What level is the cap at?

I wonder if he realizes that he loses almost all of his XP with the upgrade (the amount for the new level stays the same, though, so upgrading a highly promoted unit basically means that it won't ever get promoted again).

I wonder if he realizes that he loses almost all of his XP with the upgrade (the amount for the new level stays the same.

He's talking about GG-lead units which don't lost their XP when they are upgraded (for free even).

I wonder if he realizes that he loses almost all of his XP with the upgrade (the amount for the new level stays the same, though, so upgrading a highly promoted unit basically means that it won't ever get promoted again).

He's talking about a unit with a great general attached. Those don't lose experience on upgrading.

EDIT: I shouldn't be posting in multiple threads at once. I'm too slow everywhere.

I have a request...

I'd like to see units be able to be traded for gold in the diplomacy screen. Perhaps the value of the unit could be determined by the cost to upgrade that unit? You would of course have to have the required PreReqTechs for the unit you wish to purchase.

Example: Say I want to buy a Longbowman. It would cost me the amount of gold it would take to upgrade a standard Archer to a Longbowman in order to purchase that Longbowman.


Just a rough idea....

That's beyond the scope of an unofficial patch. Interesting idea for a mod though.

I've played this game so much, I'm surprised I haven't seen this before, and I really like making attacking Warlords as well. Are you sure there is a unit level cap? Do you have a save with a unit hitting this cap? What level is the cap at?

Yes, I am sure there is a cap. At around 4xx (iirc) instead of the experience indicator saying how many points you need to get the next promotions it stays the same - i.e instead of saying 11 / 17 it would say 11 / 10 so you can never get another promo. I'm not surprised you ain't reached the cap as most people probably don't DOW on everyone and have mounted units with GG, Blitz, morale, & mobility (Justinian is my stress relief).

Unfortunately I don't have a save at moment, but next time I play as Byzantium I'll try to remember to post one.

Jdog, I would like to bring to your attention, a somewhat minor bug (http://forums.civfanatics.com/showthread.php?p=8455349#post8455349) in BTS.

Okay, thanks.

Version 1.2 has been posted here at CFC!

Version 1.20 changes:

- CvUnit::canSpread - Moved Python cannot spread callback to end of function where it belongs, will speed up those mods which use this callback a little
- CvPlayerAI::AI_missionaryValue - Fixed copy and past bug causing overvaluation of missionaries for AIs going for cultural victory early in the game.
- CvGameTextMgr (many places) - Fixed issues where unhappiness and unhealthiness from civics or buildings would incorrectly show up as -(unhappy face) instead of +(unhappy face) in several circumstances. (Thanks EmporerFool, Grave, Afforess)
- CvGameTextMgr - Game will now properly display info for buildings which generate unhappiness in an area or globally, or produce state religion unhappiness (should these ever come up in mods)
- CvPlayerAI::AI_doTurnPost and CvPlayerAI::AI_doTurnUnitsPost - Added in accidentally skipped fix for AI_doSplit issues as suggested by alexman
- CvTeamAI::AI_calculateAreaAIType - Fixed incorrect index usage (thanks cephalo)
- CvCity::popOrder - Fixed issue with improper use of iExtra causing national wonders to max out early in some mods (thanks davidlallen)
- CvTeam::addTeam - Fixed bug where, if civs A and B join in a permanent alliance, they get the max of A and B's espionage points against C but C just keeps its point against A and loses its points to B if that's higher
- CvUnit::canMoveInto - Removed strange behavior where setting a unit to be unable to enter a terrain type would be overridden by features (forrest, fallout) (thanks TC01)
- CvCityAI::AI_chooseProduction - Fixed bug reducing AI production of workers, and a similar issue for barb players producing too many

Version 1.2 has been posted here at CFC!

Stupid question ... I've just patched to 3.19 and installed Better AI 0.81. Do I need to get this unofficial patch or is everything I need already in the Better AI mod? Cheers!

Better AI includes the unofficial patch, so you're all set.

Better AI includes the unofficial patch, so you're all set.

Appreciate it, thanks.

Jdog5000, I've found the cause of a rare-ish CTD, and a possible fix for it. In CvPlayerAI.cpp, in the function "TechTypes CvPlayerAI::AI_bestTech(int iMaxPathLength, bool bIgnoreCost, bool bAsync, TechTypes eIgnoreTech, AdvisorTypes eIgnoreAdvisor) const" There is a section where in rare instances, the formula divides by 0.

The code in question is this section:

int iNewCapacity = kLoopUnit.getMoves() * kLoopUnit.getCargoSpace();
int iOldCapacity = GC.getUnitInfo(eExistingUnit).getMoves() * GC.getUnitInfo(eExistingUnit).getCargoSpace();
iAssaultValue += (800 * (iNewCapacity - iOldCapacity)) / iOldCapacity;

In Vanilla BTS, the iOldCapacity is never 0, but in Rise of Mankind (and possibly other mods), there is a promotion that adds an extra cargo space to ships, even to ships without preexisting cargo space. If the AI gave this promotion to a ship without cargo space, the iOldCapacity would be 0, and the function would divide by 0. I fixed this by changing that segment of code into this:

int iNewCapacity = kLoopUnit.getMoves() * kLoopUnit.getCargoSpace();
int iOldCapacity = GC.getUnitInfo(eExistingUnit).getMoves() * GC.getUnitInfo(eExistingUnit).getCargoSpace();
if (iOldCapacity <= 0)
{
iAssaultValue += (600 * iNewCapacity);
}
else
{
iAssaultValue += (800 * (iNewCapacity - iOldCapacity)) / iOldCapacity;
}

That fixed the CTD and allow me to go to the next turn. I would appreciate it if this fix made it into the next UP.

Man, I love when people find hidden problems like this AND propose a good solution.

Thanks Afforess!

In looking in the SDK for where exactly the cityAcquiredAndKept event is fired, I found another place where the active player is passed instead of the new owner of the city. That got me thinking: how exactly are modders using the ePlayer parameter passed to the event?

The cityAcquired event takes a player called eOldOwner. That's nice and descriptive. The AndKept event takes a player called iPlayer. Lovely!

CvPlayerAI::AI_conquerCity() was changed at the end to pass in getID() instead of the active player:


if (!bRaze)
{
// BUG - Unofficial Patch - start
// EF: was passing getActivePlayer()
CvEventReporter::getInstance().cityAcquiredAndKept (getID(), pCity);
// BUG - Unofficial Patch - end
}


I don't remember if this is in the UP or just BULL, but here's the other place I found it using the active player: when the popup asking if you want to keep or raze the city appears, sometimes there is the option to liberate it. In that case, CvButtonPopup handles firing the event (lame). Here's the fix:


else if (pPopupReturn->getButtonClicked() == 2)
{
CvCity* pCity = GET_PLAYER(GC.getGameINLINE().getActivePlayer()).g etCity(info.getData1());
if (NULL != pCity)
{
// BUG - Unofficial Patch - start
// EF: was passing getActivePlayer()
CvEventReporter::getInstance().cityAcquiredAndKept ((PlayerTypes)info.getData2(), pCity);
// BUG - Unofficial Patch - end
}

CvMessageControl::getInstance().sendDoTask(info.ge tData1(), TASK_GIFT, info.getData2(), -1, false, false, false, false);
}


I've checked all other calls to CvEventReporter::cityAcquiredAndKept(), and they seem to be passing the appropriate player.

When you hover over a player you see whom they are the worst enemy of. This fix hides those haters if you haven't met them.


void CvGameTextMgr::getOtherRelationsString(CvWStringBu ffer& szString, PlayerTypes eThisPlayer, PlayerTypes eOtherPlayer)
{
...
if (kTeam.isHasMet(kOtherPlayer.getTeam()))
{
if (::atWar((TeamTypes) iTeam, kThisPlayer.getTeam()))
{
szString.append(NEWLINE);
szString.append(gDLL->getText(L"TXT_KEY_AT_WAR_WITH", kTeam.getName().GetCString()));
}

// BUG - Unofficial Patch - start
// EF: don't show enemies that active player hasn't met
if (!kTeam.isHuman() && kTeam.AI_getWorstEnemy() == kThisPlayer.getTeam() && kTeam.isHasMet(GC.getGameINLINE().getActiveTeam()) )
// BUG - Unofficial Patch - end
{
szString.append(NEWLINE);
szString.append(gDLL->getText(L"TXT_KEY_WORST_ENEMY_OF", kTeam.getName().GetCString()));
}
}


I just wrote this but haven't tested it yet. I'll post again once I've had a chance to test.

I don't remember if this is in the UP or just BULL, but here's the other place I found it using the active player: when the popup asking if you want to keep or raze the city appears, sometimes there is the option to liberate it. In that case, CvButtonPopup handles firing the event (lame). Here's the fix:


else if (pPopupReturn->getButtonClicked() == 2)
{
CvCity* pCity = GET_PLAYER(GC.getGameINLINE().getActivePlayer()).g etCity(info.getData1());
if (NULL != pCity)
{
// BUG - Unofficial Patch - start
// EF: was passing getActivePlayer()
CvEventReporter::getInstance().cityAcquiredAndKept ((PlayerTypes)info.getData2(), pCity);
// BUG - Unofficial Patch - end
}

CvMessageControl::getInstance().sendDoTask(info.ge tData1(), TASK_GIFT, info.getData2(), -1, false, false, false, false);
}


I've checked all other calls to CvEventReporter::cityAcquiredAndKept(), and they seem to be passing the appropriate player.

Hmmm ... I'm not sure about this one. Here's the current chain of events:

1) Human player captures a city
2) Event fires saying human acquired city
3) Human player selects to liberate city to player X
4) Event fires saying human acquired and kept city
5) Player X is given city in trade from human player
6) Event fires saying that Player X acquired city (not acquired and kept since they have no option to raze it)

That seems right to me.

Yeah, I'm looking into this further. There are six places where the cityAcquiredAndKept event is fired. See this post (http://forums.civfanatics.com/showpost.php?p=8501026&postcount=43) for the list. I'll verify that the events are fired again after the liberation. If so, what you say makes sense.

So the CityAcquired and CityAcquiredAndKept events are fired twice each during liberation-via-conquest. I retract my latest fix request.

On the latest UP code, in the SVN (The RevDCM SVN, but it might be in others too), you made a slight error. In CvGameTextMgr.cpp

this:


/************************************************** ***********************************************/
/** UNOFFICIAL_PATCH 08/28/09 jdog5000 */
/** */
/** Bugfix */
/************************************************** ***********************************************/
/* original bts code
szHelpText.append(gDLL->getText("TXT_KEY_CIVIC_UNIT_HAPPINESS", GC.getCivicInfo(eCivic).getHappyPerMilitaryUnit(), ((GC.getCivicInfo(eCivic).getHappyPerMilitaryUnit( ) > 0) ? gDLL->getSymbolID(HAPPY_CHAR) : gDLL->getSymbolID(UNHAPPY_CHAR))));

*/
// Use absolute value with unhappy face
szHelpText.append(gDLL->getText("TXT_KEY_CIVIC_UNIT_HAPPINESS", abs(GC.getCivicInfo(eCivic).getHappyPerMilitaryUni t()), ((GC.getCivicInfo(eCivic).getHappyPerMilitaryUnit( ) > 0) ? gDLL->getSymbolID(HAPPY_CHAR) : gDLL->getSymbolID(UNHAPPY_CHAR))));
/************************************************** ***********************************************/
/** UNOFFICIAL_PATCH END */
/************************************************** ***********************************************/

should look like this:


// Happiness per military unit
if (GC.getCivicInfo(eCivic).getHappyPerMilitaryUnit() != 0)
{
szHelpText.append(NEWLINE);
/************************************************** ***********************************************/
/** UNOFFICIAL_PATCH 08/28/09 jdog5000 */
/** */
/** Bugfix */
/************************************************** ***********************************************/
/* original bts code
szHelpText.append(gDLL->getText("TXT_KEY_CIVIC_UNIT_HAPPINESS", GC.getCivicInfo(eCivic).getHappyPerMilitaryUnit(), ((GC.getCivicInfo(eCivic).getHappyPerMilitaryUnit( ) > 0) ? gDLL->getSymbolID(HAPPY_CHAR) : gDLL->getSymbolID(UNHAPPY_CHAR))));

*/
// Use absolute value with unhappy face

szHelpText.append(gDLL->getText("TXT_KEY_CIVIC_UNIT_HAPPINESS", abs(GC.getCivicInfo(eCivic).getHappyPerMilitaryUni t()), ((GC.getCivicInfo(eCivic).getHappyPerMilitaryUnit( ) > 0) ? gDLL->getSymbolID(HAPPY_CHAR) : gDLL->getSymbolID(UNHAPPY_CHAR))));

/************************************************** ***********************************************/
/** UNOFFICIAL_PATCH END */
/************************************************** ***********************************************/
}

Otherwise, every civic on the civic screen shows 0 unhappiness for each military troop. It's a simple error. You accidentally clipped off the condition.

Looks like that was only an issue in RevDCM ... thanks!

@jdog:

I see there's a small change in CvPlayer.cpp which isn't mentioned in the changelog:

UP1.1 (line 1045) reads:

PROFILE_FUNC();

while UP1.2 reads:

//PROFILE_FUNC();

What does this change do?

Commending out the call means the function won't be profiled (timed) during Debug builds. It doesn't affect gameplay, only speed when debugging. It may have been disabled because the function runs too quickly to matter, but I haven't looked at the code.

Yeah, what EF said. I try to keep the debug profiling/timing setup consistent between this and BBAI so that they're easier to compare, I've commented out the tracking of a couple functions and added new ones to better understand where computation time is being spent.

No gameplay effect what so ever, in a Final_Release build PROFILE_FUNC() is completely ignored.

OK thanks :)

It looks like there's a bug if you enable the canFoundCitiesOnWater Python callback. The function checks first for various things that would allow you to found on a plot. At the end it checks if the callback is enabled. However, it checks the result outside of the test to see if the callback is enabled, using the result from the cannotFound callback above. Also, it ignores whether or not the plot is water if the callback says, "Yes, you can found on water." This means you can found on non-water invalid plots if your mod allows founding on water.

This part of the code is really, really messed up. Here's the fixed code:


if (!bValid)
{
if (GC.getTerrainInfo(pPlot->getTerrainType()).isFoundFreshWater())
{
if (pPlot->isFreshWater())
{
bValid = true;
}
}
}

// BUG - Unofficial Patch - start
// EF: canFoundCitiesOnWater callback handling was incorrect and ignored isWater() if it returned true
if (!bValid && pPlot->isWater())
{
if(GC.getUSE_CAN_FOUND_CITIES_ON_WATER_CALLBACK())
{
CyArgsList argsList2;
argsList2.add(iX);
argsList2.add(iY);
lResult=0;
gDLL->getPythonIFace()->callFunction(PYGameModule, "canFoundCitiesOnWater", argsList2.makeFunctionArgs(), &lResult);

if (lResult == 1)
{
bValid = true;
}
}
}
// BUG - Unofficial Patch - start

if (!bValid)
{
return false;
}


If the initial checks before isWater() determine that the plot is valid, this code will not override that. I assume the checks above only pass if the plot is not water. However, if that's not the case it can be rewritten slightly:


// BUG - Unofficial Patch - start
// EF: canFoundCitiesOnWater callback handling was incorrect and ignored isWater() if it returned true
if (pPlot->isWater())
{
bValid = false;

if(GC.getUSE_CAN_FOUND_CITIES_ON_WATER_CALLBACK())
{
CyArgsList argsList2;
argsList2.add(iX);
argsList2.add(iY);
lResult=0;
gDLL->getPythonIFace()->callFunction(PYGameModule, "canFoundCitiesOnWater", argsList2.makeFunctionArgs(), &lResult);

if (lResult == 1)
{
bValid = true;
}
}
}
// BUG - Unofficial Patch - start


This is probably safer.

Thanks EF, that looks much more reasonable.

CvPlot::doFeature()
CvPlot::doImprovement

It seems that forest growing probability doesn't be affected by game speed (At least, I can't find such codes).
Discovering new resource is too.

How do you think about this?

CvCity.cpp



/************************************************** ***********************************************/
/** UNOFFICIAL_PATCH 09/17/09 davidlallen & jdog5000 */
/** */
/** Bugfix */
/************************************************** ***********************************************/
/* original bts code
if (GET_PLAYER(getOwnerINLINE()).isBuildingClassMaxed Out(((BuildingClassTypes)(GC.getBuildingInfo(eCons tructBuilding).getBuildingClassType())), 1))
*/
if (GET_PLAYER(getOwnerINLINE()).isBuildingClassMaxed Out(((BuildingClassTypes)(GC.getBuildingInfo(eCons tructBuilding).getBuildingClassType())), 0))
/************************************************** ***********************************************/
/** UNOFFICIAL_PATCH END */
/************************************************** ***********************************************/




That change is a mistake,like build "palace",it can't remove original capital's "palace"。

Thanks for the report, you're right that it causes an issue with removal of palaces. I posted my new solution in the original thread:

http://forums.civfanatics.com/showthread.php?p=8455349

CvPlot::doFeature()
CvPlot::doImprovement

It seems that forest growing probability doesn't be affected by game speed (At least, I can't find such codes).
Discovering new resource is too.

How do you think about this?

You are correct, there is no game speed modification for forest/jungle growth. But what modifier would make sense? Using any of the existing ones in GameSpeedInfo would be using something "off label" (not what it was intended for), so is it worth creating a new tag for this?

I see two that stand out to me:


getFeatureProductionPercent() - Forest chop :hammers:?
getImprovementPercent() - Improvement build speed (amount of work to build it)?

To determine whether or not a new percent value is required, one option is to equalize the :hammers:/time where time of course varies with game speed.

I play Epic exclusively, but let's pull some numbers out of the air. Marathon has three times as many turns as Normal. And a chop on Marathon produces 60 :hammers: compared to Normal's 20 :hammers:. Assuming a Forest growth chance of 1% per turn for both speeds, that gives

20 :hammers: x 1% x 3 Normal-turns/Marathon-turn = 0.6 :hammers:/Marathon-turn

and

60 :hammers: x 1% = 0.6 :hammers:/Marathon-turn

Ah, so decreasing the growth chance for slower game speeds would result in fewer :hammers:/turn at those speeds. This of course doesn't factor in Jungles, and it means that Forests are seen as "larger" at slower speeds. They require the same number of normalized-turns to chop yet yield more :hammers.

As well, since the growth chance is checked each turn the growth rate will actually be higher (once normalized) at slower speeds. The chance that a forest doesn't grow in a 3 turn period (equivalent to 1 turn at Normal) is 99% x 99% x 99% which is less than 99%, the chance a Forest doesn't grow at Normal speed.

I would almost prefer to have seen faster regrowth with the same :hammers: output at slower speeds, but that would be far to aggressive a change at this stage.

Again, all my numbers are from memory or imagination. If they differ from reality, please correct my calculations.

That sounds like reasonable logic to me, equalizing the hammer opportunity between game speeds basically. So the odds of spread would scale as:

X out of (10000 * FeatureProductionPercent)/100

Works for me.

If you're going to take a look at forest growth and game speed, I think it would be a good idea to revisit the AI's random decisions and gamespeed as well. Because of the AI's use of the RNG for deciding if it goes to war, and when to bother the human for requests, the diplomacy of different game speeds is drastically different. These RNG based decisions should also scale with gamespeed for consistency.

I think my equations were backwards. The higher number of turns at slower speeds combines with the higher :hammers: output at those same speeds to significantly increase the :hammers:/year at slower speeds. This assumes that at Marathon you get more :hammers:/chop.

To equalize the :hammers:/year you'd need to decrease the growth rate at slower speeds. Now that I reread your post, jdog5000, it looks like that's what your equation does.

But what modifier would make sense? Using any of the existing ones in GameSpeedInfo would be using something "off label" (not what it was intended for), so is it worth creating a new tag for this?

Hmmmm.
I had not reached that difficult problem when posting.

I believe that growing probability at 1 normal-turn should be as same as 3 Marathon-turns.
(3 is Marathon-FeatureProductionPercent / 100)

Given that 1 Marathon-turn growing probability is a, 1 Marathon-turn NOT growing probability is (1-a).
Then 3 Marathon-turns NOT growing probability is (1-a)^3.
If my belief is true, 1 normal-turn NOT growing probability should be (1-a)^3 too.

For example, given 1 normal-turn growing probability is 20%, 1 Marathon-turn growing probability is...

1 - 0.2 = 0.8

(1-a)^3 = 0.8
(1-a) = 0.8 ^ (1/3)
(1-a) = 0.92831776672255577848201527018396...
a = 1 - 0.92831776672255577848201527018396...
a = 0.071682233277444221517984729816042...

About 7.168%.
Is this true probability?
I fell into confusion.:sad:

EDIT: This logic doesn't work when normal-growing probability is 100%, sorry.

The AI evaluates Civic penalties to health (and I assume happiness as well) as bonuses. To test change the <iExtraHealth> tag of a civic, load up a game and in debug mode press ctrl to see the AI's weight for that civic. You'll see that the AI evaluates a 2 the same as a -2, and thus making a civic produce unhealthiness causes an AI preference to switch to that civic.

This bug was found in RevolutionDCM, but I assume this is a BtS issue, and thus requires a UP fix.

While trying to help Afforess with some building happiness/health features I noticed a couple bugs in CvCity. The first half of the function is fine; all you need to notice is the name of the function:


void CvCity::setBuildingHealthChange(BuildingClassTypes eBuildingClass, int iChange)
{
for (BuildingChangeArray::iterator it = m_aBuildingHealthChange.begin(); it != m_aBuildingHealthChange.end(); ++it)
{
if ((*it).first == eBuildingClass)
{
if ((*it).second != iChange)
{
if ((*it).second > 0)
{
changeBuildingGoodHealth(-(*it).second);
}
else if ((*it).second < 0)
{
changeBuildingBadHealth((*it).second);
}

if (iChange == 0)
{
m_aBuildingHealthChange.erase(it);
}
else
{
(*it).second = iChange;
}

if (iChange > 0)
{
changeBuildingGoodHealth(iChange);
}
else if (iChange < 0)
{
changeBuildingBadHealth(-iChange);
}
}

return;
}
}


But if the building isn't found in the list yet, it is appended to it. That's fine, but check out the two function calls after that's done:


if (0 != iChange)
{
m_aBuildingHealthChange.push_back(std::make_pair(e BuildingClass, iChange));

if (iChange > 0)
{
changeBuildingGoodHappiness(iChange);
}
else if (iChange < 0)
{
changeBuildingGoodHappiness(-iChange);
}
}
}


Um, WTF? :lol: I believe these should be changed to match the code just above:


if (iChange > 0)
{
changeBuildingGoodHealth(iChange);
}
else if (iChange < 0)
{
changeBuildingBadHealth(-iChange);
}


Now that I take in the full function, I think the first part is broken, too, along with the happiness version. This function sets the extra health for a given building class, but it calls changeBuildingGood/BadHealth() without first checking if the city even has the building class.

As it's only called in applyEvent() and acquireCity(), it works unless the building that received the extra health from an event is destroyed during conquest.

delete

It's there any way you guys can make the AI not building so many caravels...?

Is there a thread for the latest patch so we can discuss it without cluttering up this one?

It's there any way you guys can make the AI not building so many caravels...?

Unless this behavior is caused by a bug, I think a fix like this is more appropriate for Better BTS AI.

Is there a thread for the latest patch so we can discuss it without cluttering up this one?

I'll make a thread for the next version, this thread is fine for now.

That sounds like reasonable logic to me, equalizing the hammer opportunity between game speeds basically. So the odds of spread would scale as:

X out of (10000 * FeatureProductionPercent)/100

Works for me.

A similar adjustment can be made for the appearance of resources on mined hills. Presently, you'll get a lot more resources from mined tiles on marathon speed than on normal speed during the length of a game.

Hugely more important is a similar adjustment to the chances that some random diplomatic effects occur. The most important one is the chance to declare war. Presently, the chance to declare war each turn is exactly the same at marathon speed as it is on normal speed. Because a certain period of the game (say medieval times) typically takes 3 times as many turns on marathon speed, this means that the chance to declare war during that period is far larger at marathon speed. Dividing the odds to declare war each turn by 3 on marathon game speed would balance things (approximately).

For those who like a mathematical approach:
If p are the odds to declare war each turn, then the odds to declare war during a marathon speed game turn would become p/3 if the above made adjustment is made.
I want to show that the odds that an AI declares war during three marathon turns is close to the odds that an AI declares war during a single normal game turn if the above suggested adjustment is made.
The odds that an AI declare war during three marathon turns is 1 - (1-p/3)^3 and the odds that an AI declares war during a single normal game turn is p.

1-(1-p/3)^3 = p - (p^2)/3 + (p^3)/9 which converges to p for small p as
lim p->0 [p - (p^2)/3 + (p^3)/9]/p =1

So it would balance things as long as the odds to declare war per turn are small which is mostly true as far a I know.

Example for odds of war declaration of 1% per turn or p=0.01:

Odds per turn at normal speed: 1%
Odds at non-adjusted marathon speed odds per 3 turns: 2.97%
Odds at adjusted marathon speed odds per 3 turns: 0.997%

The failure of Firaxis to balance the war declarations of the various game speeds is the reason that marathon players often complain that the AI is predictable in its war declarations and that it is too aggressive. It makes sense that an AI that has a small chance to declare war each turn will eventually declare war when enough turns have passed. This might not happen during a normal game speed game, but it will happen almost surely during a marathon game.

There are other elements of the AI diplomacy which haven't been scaled to game speed. I'm not sure which ones, but all of them should be scaled. This will reduce the predictable result that at marathon game speed an AI will eventually do something even though the odds per turn are small.

I myself play at epic game speed which suffers far less from these non-scaled game elements, but still has a slightly different balance than normal game speed. I would like it if the marathon game players can also enjoy a more balanced game with less weird war declarations.

It seems another problem with the code I posted above for extra building health/happiness is that it calls changeBuildingBadHealth/Happiness() as if the value it stores should be positive. Looking at the other code in CvCity, these values are negative, the lower the number the more :yuck: there is from buildings.

CvPlayer::canTradeItem()

case TRADE_CIVIC:
if (!(GET_TEAM(getTeam()).isHuman()))
{
if (GET_PLAYER(eWhoTo).isCivic((CivicTypes)(item.m_iD ata)))
{
if (canDoCivics((CivicTypes)(item.m_iData)) && !isCivic((CivicTypes)(item.m_iData)))
{
if (canRevolution(NULL))
{
return true;
}
}
}
}
break;

case TRADE_RELIGION:
if (!(GET_TEAM(getTeam()).isHuman()))
{
if (GET_PLAYER(eWhoTo).getStateReligion() == ((ReligionTypes)(item.m_iData)))
{
if (canConvert((ReligionTypes)(item.m_iData)))
{
return true;
}
}
}
break;

This means that human player can not request teammate AI to convert religion or civic.
Because GET_TEAM(getTeam()).isHuman() returns true when the team includes a human player even if the human is eWhoTo player self.
I wonder this behavior and believe that it should be
if (!(GET_TEAM(getTeam()).isHuman()) || (getTeam() == GET_PLAYER(eWhoTo).getTeam())
.

How do you think?

This would mean you could bribe your own AI teammates but still be unable to bribe other human players' teammates. I agree that this makes sense. The question is whether or not this is deemed a bug fix or new feature.

Certainly, it might not be able to assert this is a bug...

The failure of Firaxis to balance the war declarations of the various game speeds is the reason that marathon players often complain that the AI is predictable in its war declarations and that it is too aggressive. It makes sense that an AI that has a small chance to declare war each turn will eventually declare war when enough turns have passed. This might not happen during a normal game speed game, but it will happen almost surely during a marathon game.

Hold on, with the changes that you suggest, wouldn't you then see the same amount of wars in a marathon as in a normal speed game? Or am I misunderstanding you?

Currently I am playing marathon, and I like the fact that by the time you hit mid-game there have been several different wars. The marathon game is longer, and I think it's good that that is reflected in the amount that goes on in a game. Marathon games would be dull if you had on average as many ways as in a normal speed game.

Hold on, with the changes that you suggest, wouldn't you then see the same amount of wars in a marathon as in a normal speed game? Or am I misunderstanding you?

No that's what I'm getting at. Marathon are stretched out normal games with more decision points and relatively faster movement due to the greater amount of turns.

In marathon, you'd get a similar number of wars, but they would be of greater length each.

The main problem with the current setup is that war will almost always happen even between civilisations that like eachother (pleased neighbours who almost never declare war at pleased). Even a small chance of war will inevitably lead to a war declaration after the dice have been rolled enough times. And due to the far greater amount of turns, the dice are rolled far more often.
I've seen multiple complaints about the predictability of a war declaration in marathon games.

By the way, the aggressive AI option is a good one if you want more wars in your games. That option was created to get more wars and aggression during your games.

Ok I see what you mean, but I prefer it as it is, and would hesitate to describe the current situation as in any way a bug. There are too few wars in normal games, and I like marathon for increasing the brutality. There is more time to mull over grievances between players and more wars in general. I don't think having the same amount of wars, but having them of increased duration, is a good solution. With movement speed at marathon implemented as it is, you can accomplish as much in n turns of fighting as you could on normal speed.

By the way, the aggressive AI option is a good one if you want more wars in your games. That option was created to get more wars and aggression during your games.

Again I disagree. All I have heard about aggressive AI suggests that the AI increased unit building just makes it easier to out-tech them, if you survive the first few turns. That's not what I want, I like normal AI behaviour but a longer game with more bitter rivalries and wars of revenge. Unlike normal speed, marathon wars seem to be more realistic in that at least before the mid-late game, a war is concluded with the loss of one or two cities, which is a blow but recoverable from, while on normal if the tide turns against you you are probably out of the game.

I think that Roland ( very unlike what he usually does ) did not explained well what he meant. Besides the fact that there are a lot of other things that should scale with speed and they don't, especially events, but not only , the fact is that warfare in epic, marathon and quick is neither independent or fully regulated by the speed you are using. Yes, like he said, war checks ( and demands ) are turn based , but the war prep period ( the "hands full" status ) is speed scaled. So we have a very anomalous situation where in slower speeds the Ai is more eager in average to think on warring, but will take the same ( in proportion ) time to prepare themselfes to war ( same to quick speed, just the other way around ). And for what I assume on reading your text, you are not oposed to scale the war checks with speed, you are just oposed to make it by Normal speed standarts ;)

Ok I see what you mean, but I prefer it as it is, and would hesitate to describe the current situation as in any way a bug. There are too few wars in normal games, and I like marathon for increasing the brutality. There is more time to mull over grievances between players and more wars in general. I don't think having the same amount of wars, but having them of increased duration, is a good solution. With movement speed at marathon implemented as it is, you can accomplish as much in n turns of fighting as you could on normal speed.



Again I disagree. All I have heard about aggressive AI suggests that the AI increased unit building just makes it easier to out-tech them, if you survive the first few turns. That's not what I want, I like normal AI behaviour but a longer game with more bitter rivalries and wars of revenge. Unlike normal speed, marathon wars seem to be more realistic in that at least before the mid-late game, a war is concluded with the loss of one or two cities, which is a blow but recoverable from, while on normal if the tide turns against you you are probably out of the game.

The negative comments which you've read about aggressive AI are from the days that Beyond the Sword was just released and was very buggy. I personally have more trouble with the aggressive AI on immortal level on huge maps on epic speed than the normal AI under similar settings. They won't tech slowly under such settings. The aggressive AI option might hurt the AI's at low difficulty levels, but the AI can easily support the bigger army at the higher levels without really hurting its research speed.

I do understand your preference for a more warlike game, but I've also read many comments about players who really hate the predictability with which the AI will eventually declare war on anyone they don't love at marathon speed. It reduces the value of relations to good or not-good. The finer differences between the various levels of not good aren't that important anymore for war declarations as they will eventually occur after enough rolls of the dice.

But I won't argue any further as I'm not arguing this point for my own games as I don't play on marathon speed. You may argue with other players who really dislike this consequence of the marathon setting. It's not a personal issue of mine.

Suggestion for a small audio improvement for supermarkets and coal plants:

Replace AS2D_BUILD_GROCER by AS2D_BUILD_SUPERMARKET
Add tag AS2D_BUILD_PLANTCOAL to coal plants

The sounds and xml are already done by Firaxis but somehow they forgot to change/add it for these city improvements :lol:

In Civ4BuildingInfos.xml there's a reference for the Hanging Gardens: AS2D_BUILD_HANGGARDENS, also properly defined in Audio2DScripts.xml and AudioDefines.xml but the wav seems to be missing (or am I looking in all the wrong places :confused:)

Version 1.3 has been posted! See the discussion thread for more.

I found a new bug. When a building gives a negative defense against spies, it still says "Helps thwart enemy spies." I got a screenshot for it.

232980

This is an issue with CvGameTextMgr. Here's the fix:

/************************************************** ***********************************************/
/** Afforess Unofficial Patch 31/10/09 */
/** */
/** */
/************************************************** ***********************************************/
if (kBuilding.getEspionageDefenseModifier() != 0)
{
if (kBuilding.getEspionageDefenseModifier() > 0)
{
szBuffer.append(NEWLINE);
szBuffer.append(gDLL->getText("TXT_KEY_BUILDING_ESPIONAGE_DEFENSE_MOD", kBuilding.getEspionageDefenseModifier()));

szBuffer.append(NEWLINE);
szBuffer.append(gDLL->getText("TXT_KEY_UNIT_EXPOSE_SPIES"));
}
if (kBuilding.getEspionageDefenseModifier() < 0)
{
szBuffer.append(NEWLINE);
szBuffer.append(gDLL->getText("TXT_KEY_BUILDING_ESPIONAGE_DEFENSE_MOD", kBuilding.getEspionageDefenseModifier()));
}
}
/************************************************** ***********************************************/
/** Afforess */
/************************************************** ***********************************************/

Too bad this didn't make the release...

Had a bug reported in LoR by PieceOfMind. When capturing a city he got contact with a civ that he could not see a unit for. A user reported this was default BtS behavior, where when capturing a city for a brief moment you have vision over some of the cultural view of the city (the inner ring). Tested this report, and it, indeed is a bug in BtS. Reports can be found here:

PieceOfMind's original bug report (http://forums.civfanatics.com/showpost.php?p=8603455&postcount=1395) -picture in post to visually represent the bug
User expenation of bug's behavior (http://forums.civfanatics.com/showpost.php?p=8603523&postcount=1398)

Attaching a save where the bug is reproduced (simply capture the city to the south of the chariot). Save is for default BtS 3.19, no modifications.

Interesting ... fun one.

Interesting ... fun one.

Well I assume this is from Vanilla. And given that it hasn't been reported or complained about in the 5 years of the game's existence, and it doesn't really cause any issues, I think it would be fine to ignore. But since it was reported to me, and I could reproduce it, I figured I'd just forward the report and let you or EF decide if you really want to bother with fixing it. I'm sure no one will complain either way.

I'm certainly not complaining. ;)

Still, as happens with some bugs, is it possible that investigating further could reveal another similar and perhaps more serious bug?

Seeing PieceOfMind makes me wonder: would you consider the rounding of trade routes which makes some multiplier buildings completely useless in some circumstances as a bug ? If so, worthy to be corrected here ?

Seeing PieceOfMind makes me wonder: would you consider the rounding of trade routes which makes some multiplier buildings completely useless in some circumstances as a bug ? If so, worthy to be corrected here ?

Well, based on some of the discussion EmperorFool provided in the PIG Mod thread, I'd say it's not any more unusual than rounded off hammers or food not being counted. For example, on large maps the food corps can take several food resources that provide no benefit (because of rounding) yet still subtract their fees from your city.

25% hammer bonuses in the early game like forges or Organised Religion only work to their fullest when your base hammers (including overflow and chopping) are a multiple of 4.

Fractional trade routes having a small rounding problem is no bigger a problem than the above IMO. It just looks more confusing on the interface.

As my opinion goes on other issues that are similar to this, the issue is with the game's documentation.

Having said that, I think it's pretty inconsistent and buglike gameplay that a forge in a city with 3 base hammers will do nothing, yet work at nearly the full 25% bonus in a city that has only 1 hammer and uses whipping almost exclusively for its production.

I'm not sure it's within the scope of the Unofficial Patch project, to be honest.

I'd say there is a problem. Because yes, as you say, there are lots of other rounding stuffs in the game. However, while the base value of science, gold, hammer... can take high values and make the rounding almost non-perceptible, trade routes almost always have a very low value (1 commerce at the beginning) which makes rounding a big lose a good chunk of the total commerce. If at least all trade routes were added before applying modifiers I would not complain, but the way the game is now people will think they get some benefit from a harbour while in fact they get none.

I'd say there is a problem. Because yes, as you say, there are lots of other rounding stuffs in the game. However, while the base value of science, gold, hammer... can take high values and make the rounding almost non-perceptible, trade routes almost always have a very low value (1 commerce at the beginning) which makes rounding a big lose a good chunk of the total commerce. If at least all trade routes were added before applying modifiers I would not complain, but the way the game is now people will think they get some benefit from a harbour while in fact they get none.

If my understanding is correct, the trade routes are in fact added up before they are rounded. Since there are no other other ways for base commerce to get rounded except for Bureacracy capitals, it's only really a big loss when you have 1 trade route at like _.75:commerce:.

The other reason it's really noticeable is that it will happen straight away with your first trade route. At least with hammer rounding and food rounding it doesn't start happening until you start obtaining modifiers through your own actions.

Trade route calculations work like this:


for each trade city
x = max( base trade rate times 100 with trade city, 100 )
x = x * (trade rate modifier)
x = x / 10000


So, rounding is done after multiplication by trade modifiers, however the modifiers are applied on a route by route basis and not on the sum. Given the way trade route profits are shown in game, this is to be expected.

A building which gives +50% trade modifier gives no benefit until the city it's built in has a trade route with a base rate times 100 of 134.

Here's how base trade rate is computed (it returns the base rate times 100):


int CvCity::getBaseTradeProfit(CvCity* pCity) const
{
int iProfit = std::min(pCity->getPopulation() * GC.getDefineINT("THEIR_POPULATION_TRADE_PERCENT"), plotDistance(getX_INLINE(), getY_INLINE(), pCity->getX_INLINE(), pCity->getY_INLINE()) * GC.getWorldInfo(GC.getMapINLINE().getWorldSize()). getTradeProfitPercent());

iProfit *= GC.getDefineINT("TRADE_PROFIT_PERCENT");
iProfit /= 100;

iProfit = std::max(100, iProfit);

return iProfit;
}


The value is XML driven and is the minimum of a population based rate and a distance based rate.

jdog, is that the same for EmperorFool's Fractional Trade Routes mod? I maybe didn't make it clear but my comments above were targeted only at that mod. I'm aware that in the unmodded game the trade routes are valued exactly as they are displayed in the trade routes box.

http://forums.civfanatics.com/showpost.php?p=8442961&postcount=175
Some background first perhaps:


All arithmetic is done using integers. When non-integral values are needed (e.g. commerce), the value is stored "times 100" to allow for exactly two decimals. This allows faster calculations while allowing for an exact fractional amount.
All yields (:food:, :hammers:, and :commerce:) are stored and added up as whole numbers. The only place a fractional yield occurs is in this new feature, and it's only fractional while adding up the trade routes for a single city. The total for a city is rounded as soon as it gets converted into actual :commerce: (and :food: and :hammers:, but in BTS the trade route yield modifiers for those yields are both 0%).

The storage and calculation of fractional trade routes aren't the problem; I've completely handled them with no issues. The only issue arrises is when displaying the values to the user not aggregated city-by-city. When displaying the total trade with a single rival, I must add up a subset of trade route yields across cities. If I do the rounding for each city individually as I would when summing up all trade routes, the lost :commerce: will be wrong, producing incorrect totals.

There's absolutely no way to produce correct display totals.

So, what to do given that I have to display slightly incorrect trade totals? My suggestion is to show the unrounded total to indicate that it's approximate, just as the unrounded value is shown when hovering over a single trade route in a city.

If my understanding is correct, the trade routes are in fact added up before they are rounded. Since there are no other other ways for base commerce to get rounded except for Bureacracy capitals, it's only really a big loss when you have 1 trade route at like _.75:commerce:.

I just checked in-game, and can confirm you what Jdog's code says: trade routes modifiers are applied on a per trade route basis, not on the sum. So with a city having 4 trade routes with a base value of 1, adding a harbour does not change anything (except health) since it changes 4*round(1) by 4*round(1.5). It this is not a bug, it is at least counter-intuitive, especially when the trade route panel displays 2 because you have an oversea trade route (base value similar, but additional modifier).

I also just noticed that the to compute their final output in terms of science/gold/... trade routes in a bureaucracy capital are multiplied 3 times: 1 time for things like a harbour, 1 time for bureaucracy, and 1 time for multipliers such a library or market :crazyeye:

I think you're all saying the same thing but not realizing it.


In the normal game, trade route yields are rounded per route causing Harbors to be nearly useless.
In BULL with Fractional Trade Routes enabled at compile-time, trade routes are rounded after adding all the routes for a single city.

I wouldn't call this a bug as it works exactly as designed; but it is suboptimal.

Note that while other multiplier buildings such as the Forge get rounded off sometimes, the rounding happens after all multipliers are applied. While a 7 :hammers: city with a Forge gains only +1 :hammers:, adding Organized Religion provides +3 :hammers: in total. If the rounding happened for each multiplier, the total would be +2 :hammers:.

Trade routes are doubly-shafted because you lose to rounding for each route in a city whereas rounding on other values only bites you once per city.

I just checked in-game, and can confirm you what Jdog's code says: trade routes modifiers are applied on a per trade route basis, not on the sum. So with a city having 4 trade routes with a base value of 1, adding a harbour does not change anything (except health) since it changes 4*round(1) by 4*round(1.5). It this is not a bug, it is at least counter-intuitive, especially when the trade route panel displays 2 because you have an oversea trade route (base value similar, but additional modifier).

I also just noticed that the to compute their final output in terms of science/gold/... trade routes in a bureaucracy capital are multiplied 3 times: 1 time for things like a harbour, 1 time for bureaucracy, and 1 time for multipliers such a library or market :crazyeye:

Did you read my other post, directly before that one? Sorry I didn't make it clear before but I was talking only about the Fractional Trade Routes component in BULL, as EF repeated also.

Did you read my other post, directly before that one? Sorry I didn't make it clear before but I was talking only about the Fractional Trade Routes component in BULL, as EF repeated also.

Sorry if I wasn't clear either. I just wanted to make it clear what is counter-intuitive with trade routes now.

EF > not sure what you wanted to say with the forge/OR example; trade routes also are rounded after all multipliers are applied. Fortunately :) But, as you said, the thing is that you lose commerce for each trade route.

Anyway, yes, it works as designed. I'm just not sure this design is good :( But if the change is not implemented, I'll perhaps just steal PIG's code and compile it for myself ;)

Sorry if I wasn't clear either. I just wanted to make it clear what is counter-intuitive with trade routes now.

EF > not sure what you wanted to say with the forge/OR example; trade routes also are rounded after all multipliers are applied. Fortunately :) But, as you said, the thing is that you lose commerce for each trade route.

Anyway, yes, it works as designed. I'm just not sure this design is good :( But if the change is not implemented, I'll perhaps just steal PIG's code and compile it for myself ;)

Hmm, it's not PIG's code but BULL's (search for fractional trade routes). It's not released for PIG yet anyway.

My point about the Forge and OR was more to what PoM said that those lose due to rounding as well. I'm saying that trade routes lose more.

Say you build a Harbor that gives you +0.5:commerce: for each 1:commerce: route. If you have 1 route, you lost 0.5:commerce: once. This is akin to losing a hammer that the Forge could give you. If you have two routes, you lost 1:commerce: total, but if you compare it (albeit a bit strained) to having a Forge and OR together, you would lose less from the latter combination because the modifiers add before being rounded.

All I'm saying is that the design is more flawed than just rounding at the city level. Trade routes work as :hammers: would if each individual building/civic's multiplier were rounded individually. In other words, trade routes are doubly-screwed. :p

The only question is whether or not Fractional Trade Routes should be included in the UP. While I think the loss due to rounding of trade routes per route is bad design, I have a hard time considering it a bug that needs patching in the UP.

Ah, right, got your analogy now.

Yeah, I know it's not a bug per se. But this is an unofficial patch after all, not just a squash-bug compilation ;) The same way, the paratrooper behaviour has been changed, and I know I would gladly include the change to the cottage/workshop swapping behaviour; but that's not the question, let's not start on this it's only an example :)

I have fixed the bug that allows you to acquire unlimited free techs from Oracle/Liberalism in BULL. It involves some changes in CvPlayer.h/cpp and CvDLLWidgetData.cpp and is in the BULL repository (https://civ4bull.svn.sourceforge.net/svnroot/civ4bull/trunk/). I believe my solution will work in multiplayer games, too.

Didn't the bug also have something extra to do with reloading a game? That sounds familiar but I couldn't find anything related to that.

BTW, has 1.3 been released? I only see up to 1.2 on the first page.

Cool, thanks EF! I'll check it out from BULL.

I apparently forgot to update the change log in this thread, fixed. Thanks for pointing that out.

EF. I noticed in BULL your new code for CvPlot::getLatitude() that you refer to hard numbers like 60 a few times instead of things like GC.getMapINLINE().getTopLatitude().

Is this going to be a problem? To merge this change in from BULL I'd need to overwrite the function getLatitude that was changed for the UP.

Oh, was getLatitude() changed in the UP? I'm merging the latest UP into BULL today but haven't gotten started yet--ACO took forever! :p I'll go check it out now.

The 60 is "number of minutes per degree" and not map-dependent. I use the map's top and bottom latitudes appropriately. I do, however, use -180 and 180 as the full range of longitude values because CvMap doesn't have anything for longitudes. The game doesn't have longitudes in it, so this won't matter. I included them only because it looks cooler than just latitude in the display.

ACO took forever!

Ugh, ACO basically lays waste to the CvGameTextMgr... Merging anything else with the CvGameTextMgr after you've got ACO is such a pain...

Ugh, ACO basically lays waste to the CvGameTextMgr... Merging anything else with the CvGameTextMgr after you've got ACO is such a pain...

It's true. :lol: It's the price one pays for lots of detail and options. At least ACO is completely contained in setCombatPlotHelp(), right? It pretty much claims that function for its own.

If you're using winmerge though, it has trouble identifying it as one big change because of all the common little snippets. This definitely makes merging a pain if you already have other code in setCombatPlotHelp().

Oh, was getLatitude() changed in the UP? I'm merging the latest UP into BULL today but haven't gotten started yet--ACO took forever! :p I'll go check it out now.

The 60 is "number of minutes per degree" and not map-dependent. I use the map's top and bottom latitudes appropriately. I do, however, use -180 and 180 as the full range of longitude values because CvMap doesn't have anything for longitudes. The game doesn't have longitudes in it, so this won't matter. I included them only because it looks cooler than just latitude in the display.

Oh, right. My mistake.

For now I've replaced the UP code with yours - I can't see any problem yet with doing that.

I reworked the lat/long functions a little. They now detect when the range should go to the full extend (-90 and 90) based on wrapping. I had to touch the same three files. Note that I renamed the getReal___Minutes() functions by removing the Real part since it's unnecessary since they have Minutes on the end.

BTW, the time sink of merging ACO for me involved converting to using getBugOptionXXX() instead of getDefineXXX() and creating all the options (there were far more options this time around) in BUG. This meant I had to write the XML, comie up with hover text, and place them on the options screen. I also had to extend BUG's option core a little to make it so I could use the same list of choices for the 13 similar view options so the translators wouldn't be annoyed. ;)

Yes, the code is long and formatted in a . . . let's say creative manner (though better this time), it really shouldn't take other modders much time at all to merge since they can do a straight copy-paste of the entire function.

This was probably just overlooked, but it should be fixed. (http://forums.civfanatics.com/showpost.php?p=8599855&postcount=194)

Version 1.4 has been released! Check out the thread for more.

I suggest to include this (http://forums.civfanatics.com/showthread.php?t=335966) in a future version.
Since it has its own Dll, you cannot use both the unofficial Patch and this one together.

Just as its name implies, the Show Hidden Attitude Mod shows hidden game information--and not just the stuff you can look up in the XML files and see for each leader (assuming you haven't selected Random Personalities for the game) but also the random die roll applied to each player at the start of the game.

It does not belong in the UP.

But its so damn useful :D .
Ok, then i have to try to mod it into the UP myself.

Jdog, I found a rare but potentially serious issue. When improvements give unhappiness (I don't believe any improvements do in Base BTS, but the code is there), it's checked in CvCity::updateFeatureHappiness(). This check loops through all the cities plots, but doesn't actually check that the city owns them. Improvements owned by nearby competitor cities are checked too, if they are in the BFC. This can cause improvements in other players countries to harm your cities too. The fix is simple, in CvCity::updateFeatureHappiness() just check that the pLoopPlot->getOwner() matches the cities owner right after the NULL check. I tested it out and it seems to work.

So my AI neighbors that are building Forest Preserves are providing my bordering cities with happiness? Woot! Of course, I never let an AI neighbor that is taking my BFC tiles survive that long, but it's good to know.

So my AI neighbors that are building Forest Preserves are providing my bordering cities with happiness? Woot! Of course, I never let an AI neighbor that is taking my BFC tiles survive that long, but it's good to know.

Only if those tiles are in the cities BFC. This isn't much of a problem in BTS, but in mods where there is a third radius for cities.... :mischief:

This is a known mechanic from the first days of civilization IV. It works the same for health from forests, unhealth from jungles, flood plains and fallout and happiness from forest preserves.

You could argue that all of these effects should only affect the city who controls the tile. And you could also argue that the effects are there because the improvement/overlay is in the neighbourhood (BFC) of the city. It's not a principle for me as in my view, it's just what works better for game balance in the game. However, you'll have a hard time arguing that it's an error in the game design and should be fixed by the unofficial patch.

I can imagine that the normal game rule (tile improvements/overlay effects every city in which BFC the tile is located) doesn't work well for game balance in mods that change the size of the BFC. Changing the size of the BFC has all kinds of effects on game balance and effects city output vs city maintenance, health and happiness caps, city placement patterns and lots of other things I haven't thought about. It's such a fundamental change of the game rules that it would be weird if it wouldn't conflict with lots of standard game rules. Perfectly balancing the game after such a fundamental change will be hard. Limiting the effects of tile improvements or overlays to the city that controls the tile will probably make balancing such a mod easier, however I could also see it work with the tile affecting multiple cities. However, it might be hard to teach the AI to exploit such a rule as well as a human can.

This is a known mechanic from the first days of civilization IV. It works the same for health from forests, unhealth from jungles, flood plains and fallout and happiness from forest preserves.

You could argue that all of these effects should only affect the city who controls the tile. And you could also argue that the effects are there because the improvement/overlay is in the neighbourhood (BFC) of the city. It's not a principle for me as in my view, it's just what works better for game balance in the game. However, you'll have a hard time arguing that it's an error in the game design and should be fixed by the unofficial patch.

I can imagine that the normal game rule (tile improvements/overlay effects every city in which BFC the tile is located) doesn't work well for game balance in mods that change the size of the BFC. Changing the size of the BFC has all kinds of effects on game balance and effects city output vs city maintenance, health and happiness caps, city placement patterns and lots of other things I haven't thought about. It's such a fundamental change of the game rules that it would be weird if it wouldn't conflict with lots of standard game rules. Perfectly balancing the game after such a fundamental change will be hard. Limiting the effects of tile improvements or overlays to the city that controls the tile will probably make balancing such a mod easier, however I could also see it work with the tile affecting multiple cities. However, it might be hard to teach the AI to exploit such a rule as well as a human can.

Sure, but that doesn't mean that it still isn't an issue. And I didn't limit it to the city that controls it (I am not entirely sure that is possible, if two cities overlapped, which get it?), I limited it to the PLAYER. So multiple cities can be affected, but only from the same player. Jungles that you own won't affect a culturally backwards neighbor's city. I'm not sure how this is anything but an exploit.

I'm not sure if this has been fixed already. I'm not even sure if i posted this in the right place. I tried looking at the first post but couldn't notice it.
I have posted it here (http://forums.civfanatics.com/showpost.php?p=8794715&postcount=11). It's about air bombing unowned tiles. You get a green "plot picker" but can't actually air bomb them. I'm pretty sure that's not intended.

Sure, but that doesn't mean that it still isn't an issue. And I didn't limit it to the city that controls it (I am not entirely sure that is possible, if two cities overlapped, which get it?), I limited it to the PLAYER. So multiple cities can be affected, but only from the same player. Jungles that you own won't affect a culturally backwards neighbor's city. I'm not sure how this is anything but an exploit.

Oh, ok. I just misunderstood.

I agree then.

From a realism angle, a tile should affect all cities that contain it in their BFC. Malaria doesn't respect national boundaries. ;)

From a game mechanics angle, I agree, but I think I err on RJ's side of being cautious with the UP. The unmodded game has worked this way forever. The only time it becomes an issues is in modded games.

Yes, you can exploit it by settling a jungle city, leaving the jungle, and coaxing rivals into settling cities leaving 2 tiles between them and your junk city. They will suffer :yuck: forever. And you should be able to do that. I think you'd be much better off cottaging that land anyway, but the option should be available.

In the case of the forest preserve, however, from a realism perspective it should not produce happiness to rivals if you have closed borders. With open borders, citizens of neighboring cities should be able to travel to it and be happy. That's obviously too much hard-coding to be good, but it points out how this issue isn't black and white. You could argue neighbors could be happy just knowing the forest is being preserved. :D

From a realism angle, a tile should affect all cities that contain it in their BFC. Malaria doesn't respect national boundaries. ;)

From a game mechanics angle, I agree, but I think I err on RJ's side of being cautious with the UP. The unmodded game has worked this way forever. The only time it becomes an issues is in modded games.

Yes, you can exploit it by settling a jungle city, leaving the jungle, and coaxing rivals into settling cities leaving 2 tiles between them and your junk city. They will suffer :yuck: forever. And you should be able to do that. I think you'd be much better off cottaging that land anyway, but the option should be available.

In the case of the forest preserve, however, from a realism perspective it should not produce happiness to rivals if you have closed borders. With open borders, citizens of neighboring cities should be able to travel to it and be happy. That's obviously too much hard-coding to be good, but it points out how this issue isn't black and white. You could argue neighbors could be happy just knowing the forest is being preserved. :D

Well actually, I didn't alter the FeatureHealth function at all. Only the Happiness one. They are separate functions in CvCity. So in this case, it does exactly what you want. :p

Yes, and for your mod that's how I would probably do it. I'm just saying that we should err on the side of caution when changing the unmodded game.

Malaria doesn't respect national boundaries. ;)

You could argue neighbors could be happy just knowing the forest is being preserved. :DI agree with the first, the second is a bit of a strech. Realism-wise, :health: and :yuck: should always spread, (un)happiness only to same player, same team or over open boarders.
But that's still not a bug.

edit: I guess we all agree anyway.

Jdog, I found a rare but potentially serious issue. When improvements give unhappiness (I don't believe any improvements do in Base BTS, but the code is there), it's checked in CvCity::updateFeatureHappiness(). This check loops through all the cities plots, but doesn't actually check that the city owns them. Improvements owned by nearby competitor cities are checked too, if they are in the BFC. This can cause improvements in other players countries to harm your cities too. The fix is simple, in CvCity::updateFeatureHappiness() just check that the pLoopPlot->getOwner() matches the cities owner right after the NULL check. I tested it out and it seems to work.

Hmm interesting. Since this is the same mechanic as for forest preserves, it does have an interesting implication:

If there were a tile improvement that was only negative in effect (e.g. gamma radiation tower, acid fountain), and you had recently captured one of my cities but my borders still were largely overlapping your new BFC, those tiles would be useless to me as long as I didn't still have another city nearby. Then, I can fill them with gamma radiation towers until you regret ever capturing it from me! :scan:

And I didn't limit it to the city that controls it (I am not entirely sure that is possible, if two cities overlapped, which get it?)

Maybe I misunderstand but they both get it, unless that can/has been modded somehow, right? E.g. a forest gives healthiness to all cities with it in the BFC.

Hmm interesting. Since this is the same mechanic as for forest preserves, it does have an interesting implication:

If there were a tile improvement that was only negative in effect (e.g. gamma radiation tower, acid fountain), and you had recently captured one of my cities but my borders still were largely overlapping your new BFC, those tiles would be useless to me as long as I didn't still have another city nearby. Then, I can fill them with gamma radiation towers until you regret ever capturing it from me! :scan:

There are negative improvements in RoM (shaft mine causes unhappiness), which is how I caught this bug. I had an unhappy city despite having no shaft mines. My nearby neighbor had 4 though. :mad:



Maybe I misunderstand but they both get it, unless that can/has been modded somehow, right? E.g. a forest gives healthiness to all cities with it in the BFC.

Exactly.

For your example I would expect the Shaft Mine to affect only the city working the plot. You can test this in the SDK with isWorking() or getWorkingCity() or something like that. Each plot has a single "working" city. This doesn't mean the city is using the plot, only that it can use it. A plot must be owned by your culture and be worked by the city to assign a citizen to actually get the yields from it.

I could see the usefulness of adding an XML tag or two to CvImprovementInfo. Normally I'd use an enumeration to control which cities are affected, but perhaps two boolean tags would suffice: bAffectsForeignCities and bAffectsNeighboringCities. The first implies the second, but the second is useful for Forest Preserves that should affect all domestic cities in their range.

Obviously this is not for the UP.

Just as its name implies, the Show Hidden Attitude Mod shows hidden game information--and not just the stuff you can look up in the XML files and see for each leader (assuming you haven't selected Random Personalities for the game) but also the random die roll applied to each player at the start of the game.

It does not belong in the UP.

Would you accept it if I removed the dice roll? Complete accuracy for human-AI relations and +/-2 error in AI-AI relations is better than nothing.

Would you accept it if I removed the dice roll? Complete accuracy for human-AI relations and +/-2 error in AI-AI relations is better than nothing.

In this post (http://forums.civfanatics.com/showthread.php?p=7727420#post7727420) I categorized all of the modifiers by what kind of data they required (5 types). I'm fine with exposing the first two (General and Leader Specific) as they are based on fixed game-rules and XML data. I disagree with exposing the other three (Score Ranking, Hidden, and Random).

Score Ranking: Can expose information about players you haven't met, though it should be fine once you've met all rivals.

Hidden: War success is totally hidden. You can keep track of your own war success with a lot of bookkeeping, but you cannot know the values for AIs that aren't fighting only you.

Random: This is rolled at the beginning of the game and cannot be figured out.

What I'd love to see is to create five preprocessor flags: _MOD_SHOWATTITUDE_XXX where XXX is some word describing the category of information (GENERAL, LEADER, HIDDEN, RANK, RANDOM). Then use #ifdefs to guard each individual modifier:


// Worse Rank
#ifdef _MOD_SHOWATTITUDE_LEADER
#ifdef _MOD_SHOWATTITUDE_RANK
// add modifer text
#endif
#endif


This would allow BULL to disable all modifiers requiring those three categories of information flagged above while allowing you to create a build allowing all modifiers. This would be very easy to do and control. I can help if this description wasn't enough. The beauty is that other modders could show all modifiers without changing any code, and you wouldn't need to maintain multiple source versions.

It sounds like I could reduce it to a single flag (HIDDEN), with an optional second flag to toggle the entire mod on and off.

RANK could be replaced by a test for all contacts.

Yes, a single SHOW_HIDDEN_ATTITUDES flag would be fine.

For BULL I'll need to hook into its way of checking runtime options for the overall enabled flag, so you can omit this for your mod. Anyone merging in your source code directly clearly wants it to show.

CvPlot::isVisibleEnemyDefender
CvPlot::getNumVisibleEnemyDefenders
CvPlot::getNumVisiblePotentialEnemyDefenders
CvPlot::isVisibleEnemyUnit

These functions say "pUnit->isAlwaysHostile(this)", but "this" plot is not a position of "pUnit", but a opponent unit.
bool CvPlot::isVisibleEnemyUnit(const CvUnit* pUnit) const
{
return (plotCheck(PUF_isEnemy, pUnit->getOwnerINLINE(), pUnit->isAlwaysHostile(this), NO_PLAYER, NO_TEAM, PUF_isVisible, pUnit->getOwnerINLINE()) != NULL);
}

Therefore, Privateer(= pUnit) with a command "Sentry" can not find neutral unit within a city, even if Privateer is "always hostile".
Because,

Privateer's "always hostile" ability is disabled in a city.
"this" plot is a city.
Then, Privateer assumes that it loses "always hostile" even if it doesn't stay in a city.
This is my approach for solution. New blockade action for naval units (http://forums.civfanatics.com/showthread.php?t=350289)
How do you think?

While debugging a CTD in RoM with AND I ran into the following problem:

http://forums.civfanatics.com/showpost.php?p=8819748&postcount=285

These (and possibly other) mods added units with combat value and having NO_UNITCOMBAT (-1) combat type (or no CombatType tag in the xml). This causes a silent heap corruption in CvPlayerAI.cpp at

for (iI = 0; iI < GC.getNumUnitClassInfos(); iI++)
{
if (m_aiUnitClassWeights[iI] > 0)
{
UnitTypes eUnit = (UnitTypes)GC.getUnitClassInfo((UnitClassTypes)iI) .getDefaultUnitIndex();
-> m_aiUnitCombatWeights[GC.getUnitInfo(eUnit).getUnitCombatType()] += m_aiUnitClassWeights[iI];
}
}


I recommend guarding this against a buffer under/overrun, for example with


int ctype = 0;
for (iI = 0; iI < GC.getNumUnitClassInfos(); iI++)
{
if (m_aiUnitClassWeights[iI] > 0)
{
UnitTypes eUnit = (UnitTypes)GC.getUnitClassInfo((UnitClassTypes)iI) .getDefaultUnitIndex();
ctype = GC.getUnitInfo(eUnit).getUnitCombatType();
// FAssert(ctype >= 0 && ctype < GC.getNumUnitCombatInfos());
if (ctype >= 0 && ctype < GC.getNumUnitCombatInfos())
m_aiUnitCombatWeights[ctype] += m_aiUnitClassWeights[iI];
}
}


as it was creating load/save issues and random CTDs later which are quite hard to trace down. I understand that the bug was probably introduced by xml modders, who were not aware of the fact that careless use of NO_UNITCOMBAT may cause crashes, but as it has serious consequences silently, I think the dll should be protected against this kind of abuse. Probably uncomment the assertion if you want the xml modders warned.

The other point which concerns vanilla BTS is in CyGlobalContext.cpp, where

CvCommerceInfo* CyGlobalContext::getCommerceInfo(int i) const
{
return (i>=0 && i<NUM_COMMERCE_TYPES) ? &GC.getCommerceInfo((CommerceTypes) i) : NULL;
}


is used to have a call on GC's zero length vector at start-up in these mods. It evaluates correctly in the end, but I would add

CvCommerceInfo* CyGlobalContext::getCommerceInfo(int i) const
{
if (GC.getCommerceInfo().size() == 0) return NULL;
return (i>=0 && i<NUM_COMMERCE_TYPES) ? &GC.getCommerceInfo((CommerceTypes) i) : NULL;
}


just to stop the debugger complaining. Probably another problem introduced by xml modding, but again, the dll should be aware that it may happen.

Version 1.30 changes:
- CvPlot::doFeature - Plot feature (Forest/jungle) appearance now scales by game speed
- CvPlot::doImprovement - Bonus appearance in mines now scales by game speed
Nuclear plants blowing up still doesn't.

CvCity::doMeltdown() changes:
/************************************************** **********************************************/
/* UNOFFICIAL_PATCH jdog5000 */
/* */
/* Gamespeed scaling */
/************************************************** **********************************************/
/* original bts code
if (GC.getBuildingInfo((BuildingTypes)iI).getNukeExpl osionRand() != 0)
{
if (GC.getGameINLINE().getSorenRandNum(GC.getBuilding Info((BuildingTypes)iI).getNukeExplosionRand(), "Meltdown!!!") == 0)
{
*/
int iOdds = GC.getBuildingInfo((BuildingTypes)iI).getNukeExplo sionRand();
iOdds *= GC.getGameSpeedInfo(GC.getGameINLINE().getGameSpee dType()).getResearchPercent();
iOdds /= 100;

if( iOdds > 0 )
{
if( GC.getGameINLINE().getSorenRandNum(iOdds, "Meltdown!!!") == 0)
{
/************************************************** **********************************************/
/* UNOFFICIAL_PATCH END */
/************************************************** **********************************************/
though the chance is so low to begin with (1 in 2000) I doubt anyone will notice

I'm (finally) updating the UP in BULL to 1.4 and I noticed what may be a bug, but I'm not certain.

One change in CvCity is in popOrder() for buildings. The code calls CvPlayer::isBuildingClassMaxedOut(); if so it calls CvPlayer::removeBuildingClass(). The UP change is to pass CvBuildingClassInfo::getExtraPlayerInstances() instead of 1 for iExtra to isBBuildingClassMaxedOut(). My question isn't about that change, however.

In removeBuildingClass() the cities are scanned for the first one with at least 1 real matching building, but it removes all of them. Now, for vanilla that's fine since you can only have one of each type of building, but some mods allow multiples. Should this be changed to remove just one of the buildings since only one is being added elsewhere?

Here's the change if you think so:


void CvPlayer::removeBuildingClass(BuildingClassTypes eBuildingClass)
{
CvCity* pLoopCity;
BuildingTypes eBuilding;
int iLoop;

eBuilding = ((BuildingTypes)(GC.getCivilizationInfo(getCiviliz ationType()).getCivilizationBuildings(eBuildingCla ss)));

if (eBuilding != NO_BUILDING)
{
for (pLoopCity = firstCity(&iLoop); pLoopCity != NULL; pLoopCity = nextCity(&iLoop))
{
if (pLoopCity->getNumRealBuilding(eBuilding) > 0)
{
pLoopCity->setNumRealBuilding(eBuilding, pLoopCity->getNumRealBuilding(eBuilding) - 1);
break;
}
}
}
}


Now, what is up with extra player instances? The Palace is 1 but all the national wonders are 0 (all of those have 1 max). What is this field used for? It seems that by passing in extra player instances to isBuildingClassMaxedOut() which adds extra and max for comparison, it's a wash.

Ah, does this control allowing a building with a max to be replaced by building it elsewhere? canConstruct() passes in the default 0 for iExtra which allows the Palace to be built in another city but not a national wonder. So extraPlayerInstances seems to say how many you can add to city queues once you've maxed out the class.

I submitted a fix for the canFoundCitiesOnWater() callback, but I see that the code is different from what I submitted. As it is now, if the checks above make the plot valid for a city, the plot being water will no longer make it invalid as it did in the original code. This will happen if the callback is disabled (as it is by default) or if it returns False.

The bug in the original code was that it ignored the result of the callback, but the fix in 1.4 actually makes water plots valid locations for cities by default. Is this intended?

Here's the start of my patch:


if (pPlot->isWater())
{
bValid = false;
...


Here is 1.4:


if (!bValid && pPlot->isWater())
{
...


And for comparison, here is the original code:


// lResult is 0 here
if(GC.getUSE_CAN_FOUND_CITIES_ON_WATER_CALLBACK())
{
... // and gets set to 1 if the callback says it's okay
}

if (lResult == 1)
{
bValid = true;
}
else
{
// if the callback is disabled or returns False, cannot found on water
if (pPlot->isWater())
{
return false;
}
}


So while the original code ignores isWater() if the callback returns True, it correctly blocks founding on water otherwise.

Given the checks above the code it probably won't come up in practice, but it might affect mods that allow founding on water or add more rules above this check.

Edit: Meh, I'm not really sure which way it should go. The three checks above are of the style "if it's not okay but if the terrain makes it okay, it's okay." As soon as one check says it's okay, the other checks are ignored. Leaving the !bValid check in as 1.4 has it matches that form. But 1.4 definitely differs from vanilla 3.19. That there's a definite bug in 3.19 makes it harder to pick a winner.

Minor nitpick: In the fix at the start of CvPlayer::canDoCivics() there's a check and early return.


// Circumvents second crash bug in simultaneous turns MP games
if( eCivic == NO_CIVIC )
{
return 1;
}


The function returns bool, though, so I recommend putting true there instead of 1. Yes, it's the same, but I did say it was a nitpick. ;)

As long as I'm nit-picking, my name doesn't have a space in it. :mischief:

In CvPlayerAI::AI_civicValue() there's a fix that looks wrong given what's around it, but I am very unfamiliar with the AI code.


/************************************************** **********************************************/
/* UNOFFICIAL_PATCH 10/21/09 jdog5000 */
/* */
/* Bugfix */
/************************************************** **********************************************/
/* orginal bts code
iValue += (getNumCities() * 6 * AI_getHealthWeight(isCivic(eCivic) ? -kCivic.getExtraHealth() : kCivic.getExtraHealth(), 1)) / 100;
*/
iValue += (getNumCities() * 6 * AI_getHealthWeight(kCivic.getExtraHealth(), 1)) / 100;
/************************************************** **********************************************/
/* UNOFFICIAL_PATCH END */
/************************************************** **********************************************/


The change makes it so the value is always positive instead of being negative if the AI is already running the civic. Many of the surrounding checks keep the isCivic() check, swapping their value negative if so. Why is this one changed yet military police, war weariness, etc. are left as-is?

Edit: Never mind. Pays to complete the merge of a function before asking questions! I just saw that the next three checks that do this have been changed.

I'm pretty sure already I talked to someone (jdog?) about the canFoundCitiesOnWater() callback, don't remember where though so I have to rely on my brain's memory and re-reading the code..

!bValid should always be true because bValid is false by default and only 3 checks can turn it to true before the callback: isFound(), isFoundCoast() with isCoastalLand(), and isFoundFreshWater() with isFreshWater(). A Water tile can never be coastal land or with fresh water (both isCoastalLand() and isFreshWater() check if the plot is water and immediately return false), and if isFound() returned true for a water tile (<bFound>1</bFound> for that terrain in CIV4TerrainInfos.xml), then the callback is simply not needed anymore and can be skipped .. iirc.
Does that make sense?