1. We have added a Gift Upgrades feature that allows you to gift an account upgrade to another member, just in time for the holiday season. You can see the gift option when going to the Account Upgrades screen, or on any user profile screen.
    Dismiss Notice

Unofficial Patch for 3.19

Discussion in 'Civ4 - Unofficial Patches' started by jdog5000, Jun 12, 2009.

  1. EmperorFool

    EmperorFool Deity

    Joined:
    Mar 2, 2007
    Messages:
    9,633
    Location:
    Mountain View, California
    I found two bugs in Python/pyHelper/Popup.py:

    1. Typo: X used instead of iX.

    Code:
    	def addPythonButtonXY( self, strFunctionName, strButtonText, strHelpText, strArtPointer = "Art\Interface\Popups\PopupRadioButton.kfm", iData1 = -1, iData2 = -1, bOption = True, iX = -1, iY = -1 ):
    		"adds a python button at XY"
    		self.popup.addPythonButtonXY( strFunctionName, strButtonText, strHelpText, strArtPointer, iData1, iData2, bOption, [B][COLOR="Red"]X[/COLOR][/B], iY )	
    
    should be

    Code:
    	def addPythonButtonXY( self, strFunctionName, strButtonText, strHelpText, strArtPointer = "Art\Interface\Popups\PopupRadioButton.kfm", iData1 = -1, iData2 = -1, bOption = True, iX = -1, iY = -1 ):
    		"adds a python button at XY"
    		self.popup.addPythonButtonXY( strFunctionName, strButtonText, strHelpText, strArtPointer, iData1, iData2, bOption, [B][COLOR="Red"]iX[/COLOR][/B], iY )	
    
    2. Unset variable "loopTitle" accessed in addTitleData().

    I have no idea how to fix it since it isn't called by any vanilla code. The comments don't match the code, and I see no parameter that could be holding the image's extra size information. That also makes it pretty darn unimportant in my book. ;)
     
  2. jdog5000

    jdog5000 Revolutionary

    Joined:
    Nov 25, 2003
    Messages:
    2,601
    Location:
    California
    Version 1.5 has been released! You can download it here at CFC or check out the first post for more info and options. There will also be a discussion thread.

    There are 24 fixes and tweaks in this version, special thanks to NotSoGood, EmperorFool, LunarMongoose, Sephi, Afforess, denev, jsbrigo, and God-Emperor for their bug hunting!
     
  3. EmperorFool

    EmperorFool Deity

    Joined:
    Mar 2, 2007
    Messages:
    9,633
    Location:
    Mountain View, California
    Can you please tag the 1.5 release in the repository?

    Edit: I'm merging the trunk into BULL now and wasn't sure if there were any changes since the release, but I prefer to go off of tags as it's easier to track diffs and put it into the changelog.
     
  4. EmperorFool

    EmperorFool Deity

    Joined:
    Mar 2, 2007
    Messages:
    9,633
    Location:
    Mountain View, California
    There is a fix in CvPlayerAI.cpp to make decisions based on culture strategy also consider high culture slider values. There are two if() cases--one with a city and one without--and each has three if() cases based on the strategy or culture slider level, but the case with a city doesn't add the slider detection to the second culture level check.

    Here is my proposed change which copies the code from the NULL city branch on line 1585 to line 1561:

    Code:
    else if (AI_isDoStrategy(AI_STRATEGY_CULTURE2) [B][COLOR="Red"]|| getCommercePercent(COMMERCE_CULTURE) > 70[/COLOR][/B])
    
    Also the culture percentages are different between the two cases (80/nothing/50 vs. 90/70/50). Is that intentional?
     
  5. EmperorFool

    EmperorFool Deity

    Joined:
    Mar 2, 2007
    Messages:
    9,633
    Location:
    Mountain View, California
    In CvTeam::doTurn(), line 891 of CvTeam.cpp, there is a fix to check isBarbarian(). However the loop is over MAX_CIV_TEAMS. Doesn't this enum exclude barbarian civs making the fix unnecessary?
     
  6. jdog5000

    jdog5000 Revolutionary

    Joined:
    Nov 25, 2003
    Messages:
    2,601
    Location:
    California
    Done.

    Good catch. Perhaps 80/60/40 for all, since it's a > check this means setting the culture slider at 50 qualifies, that seems like a good starting point for special culture treatment.

    Yes, you are correct.
     
  7. Fuyu

    Fuyu Emperor

    Joined:
    Nov 5, 2009
    Messages:
    1,225
    Location:
    Austria
  8. EmperorFool

    EmperorFool Deity

    Joined:
    Mar 2, 2007
    Messages:
    9,633
    Location:
    Mountain View, California
    Thanks.

    Some mods allow setting the slider in 5% increments, but I don't know if that affects AIs or not. Regardless, use >= 50 if you want 50 to be included but not 45 or > 40 if you want 45 to be included. I typically prefer to use >= when specifying a cutoff to make it clear at exactly what point the effect should kick in.

    By "all" are you saying to use the same values for the city vs. non-city case, or are you talking about the non-city case specifically?
     
  9. auldian

    auldian King

    Joined:
    Nov 5, 2004
    Messages:
    648
    Location:
    Alaska, USA
    Should a player use Better BTS AI or Unofficial Patch for 3.19?
     
  10. Afforess

    Afforess The White Wizard

    Joined:
    Jul 31, 2007
    Messages:
    12,239
    Location:
    Austin, Texas
    Better BTS AI includes the Unofficial Patch, so it's your choice...
     
  11. auldian

    auldian King

    Joined:
    Nov 5, 2004
    Messages:
    648
    Location:
    Alaska, USA
    Thank you! :)
     
  12. denev

    denev Warlord

    Joined:
    Jul 5, 2008
    Messages:
    208
    Location:
    Japan
    CvPlayerAI.cpp line 4116
    Code:
    	if (kLoopBuilding.getCommerceChange(COMMERCE_CULTURE) > 0)
    	{
    		bIsCultureBuilding = true;
    	}
    I believe this should be
    Code:
    	if (kLoopBuilding.getCommerceChange(COMMERCE_CULTURE) > 0[COLOR="RoyalBlue"] || kLoopBuilding.getObsoleteSafeCommerceChange(COMMERCE_CULTURE) > 0[/COLOR])
    	{
    		bIsCultureBuilding = true;
    	}
    Current BtS doesn't consider a monument as a cultural building.
     
  13. Fuyu

    Fuyu Emperor

    Joined:
    Nov 5, 2009
    Messages:
    1,225
    Location:
    Austria
    There more ways a building could provide extra culture:
    • CommerceModifiers
    • GlobalCommerceModifiers
    • SpecialistExtraCommerces
    • StateReligionCommerces
    • FreeSpecialistCount (when the specialist produces culture)
    • FreeBuilding (when the civilizationBuilding of this buildingClass is a culturebuilding)
    Not sure what CommerceFlexibles are supposed to be as that is not used in stock BTS.
    Anyway you are right, just checking for commerceChange is a bit too simple there.

    There are also some late wonders that don't change culture production, so those would be meaningless for cultural victories.
    Code:
    									if (AI_isDoVictoryStrategy(AI_VICTORY_CULTURE1) [COLOR="Blue"]&& bIsCultureBuilding[/COLOR])
    									{
    										iValue += 400;
    									}
     
  14. jdog5000

    jdog5000 Revolutionary

    Joined:
    Nov 25, 2003
    Messages:
    2,601
    Location:
    California
    Good point denev and Fuyu ... I think given what the code seems to be doing here it should read:

    Code:
    	if (!isLimitedWonderClass((BuildingClassTypes)iJ))
    	{
    		if (kLoopBuilding.getCommerceChange(COMMERCE_CULTURE) > 0 || kLoopBuilding.getObsoleteSafeCommerceChange(COMMERCE_CULTURE) > 0)
    		{
    
    There are two places this is needed in that function.

    Then, if running for culture victory:

    Code:
    if (AI_isDoVictoryStrategy(AI_VICTORY_CULTURE1))
    {
    	if (kLoopBuilding.getCommerceChange(COMMERCE_CULTURE) > 2 || 
    		kLoopBuilding.getObsoleteSafeCommerceChange(COMMERCE_CULTURE) > 2 ||
    		kLoopBuilding.getCommerceModifier(COMMERCE_CULTURE) > 10)
    	{
    		iValue += 400;
    	}
    }
    
    How does that look?
     
  15. Fuyu

    Fuyu Emperor

    Joined:
    Nov 5, 2009
    Messages:
    1,225
    Location:
    Austria
    Looks good.
    Whereever it says
    Code:
    (kLoopBuilding.getCommerceChange(COMMERCE_CULTURE) > 0)
    it should say
    Code:
    (kLoopBuilding.getCommerceChange(COMMERCE_CULTURE) > 0 || kLoopBuilding.getObsoleteSafeCommerceChange(COMMERCE_CULTURE) > 0)
    Just one more place, replacing "kLoopBuilding.getCommerceChange(COMMERCE_CULTURE)" with "(kLoopBuilding.getCommerceChange(COMMERCE_CULTURE) + kLoopBuilding.getObsoleteSafeCommerceChange(COMMERCE_CULTURE))"
    Code:
                    if (AI_isDoVictoryStrategy(AI_VICTORY_CULTURE2))
                    {
                        int iMultiplier = (isLimitedWonderClass((BuildingClassTypes)iJ) ? 1 : 3);
                        iBuildingValue += (150 * [COLOR="Blue"][B]([/B][/COLOR]kLoopBuilding.getCommerceChange(COMMERCE_CULTURE) [COLOR="Blue"][B]+ kLoopBuilding.getObsoleteSafeCommerceChange(COMMERCE_CULTURE))[/B][/COLOR]) * iMultiplier;
                        iBuildingValue += kLoopBuilding.getCommerceModifier(COMMERCE_CULTURE) * 4 * iMultiplier ;
                    }
    In CvPlayerAI::AI_cultureVictoryTechValue() there is a similar line
    Code:
    				iValue += (150 * [COLOR="Blue"][B]([/B][/COLOR]kLoopBuilding.getCommerceChange(COMMERCE_CULTURE) [COLOR="Blue"][B]+ kLoopBuilding.getObsoleteSafeCommerceChange(COMMERCE_CULTURE))[/B][/COLOR]) * 20;
    ---

    You are catching all wonders of the base game at least, because both Eiffel Tower and Sistine Chapel produce base culture too.
    I'd use >= as comparison operator since you aren't comparing to anything special but only arbitrary numbers. At least 3 culture, or at least 11%, is that what you mean? 3 is good but I'd include 10% too.
    Code:
    if (AI_isDoVictoryStrategy(AI_VICTORY_CULTURE1))
    {
    	if (kLoopBuilding.getCommerceChange(COMMERCE_CULTURE) >[COLOR="Blue"]= 3[/COLOR] || 
    		kLoopBuilding.getObsoleteSafeCommerceChange(COMMERCE_CULTURE) >[COLOR="Blue"]= 3[/COLOR] ||
    		kLoopBuilding.getCommerceModifier(COMMERCE_CULTURE) >[COLOR="Blue"]=[/COLOR] 10)
    	{
    		iValue += 400;
    	}
    }
     
  16. jdog5000

    jdog5000 Revolutionary

    Joined:
    Nov 25, 2003
    Messages:
    2,601
    Location:
    California
    Looks good Fuyu, thanks.

    One other question ... does that line you pointed out in CvPlayerAI::AI_cultureVictoryTechValue look bugged to you?

    It's original form was:

    Code:
    				iValue += (150 * kLoopBuilding.getCommerceChange(COMMERCE_CULTURE)) * 20;
    
    Which is ludicrously out of proportion with the surrounding values ... a building granting 1 culture/turn give a value of +3000! Compared to a building boosting culture by 50% giving +100. That's nuts.

    I'm thinking it was supposed to be / 20 at the end there.
     
  17. Fuyu

    Fuyu Emperor

    Joined:
    Nov 5, 2009
    Messages:
    1,225
    Location:
    Austria
    Well it's either *3000 or *7,5.

    Comparing
    Code:
    iValue += (150 * kLoopBuilding.getCommerceChange(COMMERCE_CULTURE)) * 20;
    iValue += kLoopBuilding.getCommerceModifier(COMMERCE_CULTURE) * [B]2[/B];
    to
    Code:
    if (AI_isDoVictoryStrategy(AI_VICTORY_CULTURE2))
    {
       int iMultiplier = (isLimitedWonderClass((BuildingClassTypes)iJ) ? 1 : 3);
       iBuildingValue += (150 * (kLoopBuilding.getCommerceChange(COMMERCE_CULTURE) + kLoopBuilding.getObsoleteSafeCommerceChange(COMMERCE_CULTURE))) * iMultiplier;
       iBuildingValue += kLoopBuilding.getCommerceModifier(COMMERCE_CULTURE) * [B]4[/B] * iMultiplier ;
    }
    I can only assume they meant *75 because in the other case it's
    150 * x * iMultiplier - for commerceChange, and
    x * 4 * iMultiplier - for commerceModifier

    and in this case it's
    x * 2 - for commerceModifier

    I have no idea how a /2 became a *20 though.

    edit: The only idea that comes to mind is that the 150* part was never meant to be there, just the *20 one. At least if I had to select between the multipliers 7,5 , 20 , 75 and 3000, 20 would make even more sense than 75 since in that function it's specifically for cultural victory, where commerceModifiers should have a lot more weight.

    edit2: You're probably right about the /20. Cities that go legendary need a lot of base culture, that easily makes the commerceModifiers worth 10 times more than normally.
     
  18. jesusin

    jesusin Ant GOTM Staff

    Joined:
    Aug 26, 2005
    Messages:
    4,124
    Location:
    Madrid
  19. EmperorFool

    EmperorFool Deity

    Joined:
    Mar 2, 2007
    Messages:
    9,633
    Location:
    Mountain View, California
    I looked into the code and it seems you are correct.

    1. Add two Warriors to the queue.
    2. Put at least 1:hammers: into the first.
    3. Insert anything else into the queue before the Warriors, e.g. Barracks, Warrior, Warrior.
    4. Let the Barracks complete (must take more than ten turns on Normal; if not insert something else).
    5. With BULL, hover over the Warrior and you'll see that it will decay if you don't put :hammers: into it this turn.
    6. Let the first Warrior complete. Make sure the second Warrior is immediately after it in the queue.
    7. With BULL, hover over the second Warrior (now first in queue) and you'll see that it is still subject to decay. The timer has not been reset.
    Warning: The above scenario is based on my reading of the code. I'll set up a test to verify the behavior some time this weekend and report back. I didn't see any other place setting the counter to zero, so I'm pretty confident the bug exists. Good catch, jesusin!

    Each turn decay is applied after production. But doDecay() will only clear a unit's decay timer if it isn't first in the queue.

    Code:
    for each unit:
        if unit is not first in queue:
            if unit has at least 1 hammer:
                increment counter
                if counter > game-speed-limit
                    decay
            else:
                reset counter to zero
    
    As you can see the idea is that decay should only affect items not first in the queue, but the counter should be reset when a unit is completed. The fixes go into popOrder() and are trivial.

    Code:
    void CvCity::popOrder(int iNum, bool bFinish, bool bChoose)
    {
    	...
    	
    	case ORDER_TRAIN:
    		eTrainUnit = ((UnitTypes)(pOrderNode->m_data.iData1));
    		...
    		if (bFinish)
    		{
    			...
    			setUnitProduction(eTrainUnit, 0);
    			[B][COLOR="Red"]setUnitProductionTime(eTrainUnit, 0);[/COLOR][/B]
    
    	...
    	
    	case ORDER_CONSTRUCT:
    		eConstructBuilding = ((BuildingTypes)(pOrderNode->m_data.iData1));
    		...
    		if (bFinish)
    		{
    			...
    			setBuildingProduction(eConstructBuilding, 0);
    			[B][COLOR="Red"]setBuildingProductionTime(eConstructBuilding, 0);[/COLOR][/B]
    
     
  20. EmperorFool

    EmperorFool Deity

    Joined:
    Mar 2, 2007
    Messages:
    9,633
    Location:
    Mountain View, California
    I noticed that once I had built the six Hospitals that the Red Cross requires I still couldn't build it in one of my cities. Hovering over the Red Cross gave no indication that it couldn't be built. However, hovering over the Hospital showed that it was a prereq of the Red Cross.

    The fix is easy. The code checks if the building has a minimum number of other buildings that must be built by the player (e.g. six Hospitals), and if it does the general prereq building class check is skipped. Just remove the "else" and you're done.

    Code:
    void CvGameTextMgr::buildBuildingRequiresString(CvWStringBuffer& szBuffer, BuildingTypes eBuilding, bool bCivilopediaText, bool bTechChooserText, const CvCity* pCity)
    {
    	...
    
    	if (NULL == pCity || !pCity->canConstruct(eBuilding))
    	{
    		...
    
    		for (int iI = 0; iI < GC.getNumBuildingClassInfos(); ++iI)
    		{
    			if (ePlayer == NO_PLAYER && kBuilding.getPrereqNumOfBuildingClass((BuildingClassTypes)iI) > 0)
    			{
    				...
    			}
    			else if (ePlayer != NO_PLAYER && GET_PLAYER(ePlayer).getBuildingClassPrereqBuilding(eBuilding, ((BuildingClassTypes)iI)) > 0)
    			{
    				...
    			}
    // BUG - Unofficial Patch - start
    			[s][B][COLOR="Red"]else[/COLOR][/B][/s] if (kBuilding.isBuildingClassNeededInCity(iI))
    // BUG - Unofficial Patch - end
    			{
    				...
    			}
    		}
    
    		...
    	}
    }
    
    Edit: Naw, that doesn't look very nice. We want to hide the second requirement line if we've already added the x/y requirement. The following does just that:

    Code:
    		for (int iI = 0; iI < GC.getNumBuildingClassInfos(); ++iI)
    		{
    // BUG - Unofficial Patch - start
    			[B][COLOR="Red"]// EF: show "Requires Hospital" if "Requires Hospital (x/5)" requirement has been met[/COLOR][/B]
    			[B][COLOR="Red"]bool bShowedPrereq = false;[/COLOR][/B]
    
    			if (ePlayer == NO_PLAYER && kBuilding.getPrereqNumOfBuildingClass((BuildingClassTypes)iI) > 0)
    			{
    				eLoopBuilding = (BuildingTypes)GC.getBuildingClassInfo((BuildingClassTypes)iI).getDefaultBuildingIndex();
    				szTempBuffer.Format(L"%s%s", NEWLINE, gDLL->getText("TXT_KEY_BUILDING_REQUIRES_NUM_SPECIAL_BUILDINGS_NO_CITY", GC.getBuildingInfo(eLoopBuilding).getTextKeyWide(), kBuilding.getPrereqNumOfBuildingClass((BuildingClassTypes)iI)).c_str());
    
    				szBuffer.append(szTempBuffer);
    				[B][COLOR="Red"]bShowedPrereq = true;[/COLOR][/B]
    			}
    			else if (ePlayer != NO_PLAYER && GET_PLAYER(ePlayer).getBuildingClassPrereqBuilding(eBuilding, ((BuildingClassTypes)iI)) > 0)
    			{
    				if ((pCity == NULL) || (GET_PLAYER(ePlayer).getBuildingClassCount((BuildingClassTypes)iI) < GET_PLAYER(ePlayer).getBuildingClassPrereqBuilding(eBuilding, ((BuildingClassTypes)iI))))
    				{
    					eLoopBuilding = ((BuildingTypes)(GC.getCivilizationInfo(GET_PLAYER(ePlayer).getCivilizationType()).getCivilizationBuildings(iI)));
    
    					if (eLoopBuilding != NO_BUILDING)
    					{
    						if (pCity != NULL)
    						{
    							szTempBuffer.Format(L"%s%s", NEWLINE, gDLL->getText("TXT_KEY_BUILDING_REQUIRES_NUM_SPECIAL_BUILDINGS", GC.getBuildingInfo(eLoopBuilding).getTextKeyWide(), GET_PLAYER(ePlayer).getBuildingClassCount((BuildingClassTypes)iI), GET_PLAYER(ePlayer).getBuildingClassPrereqBuilding(eBuilding, ((BuildingClassTypes)iI))).c_str());
    						}
    						else
    						{
    							szTempBuffer.Format(L"%s%s", NEWLINE, gDLL->getText("TXT_KEY_BUILDING_REQUIRES_NUM_SPECIAL_BUILDINGS_NO_CITY", GC.getBuildingInfo(eLoopBuilding).getTextKeyWide(), GET_PLAYER(ePlayer).getBuildingClassPrereqBuilding(eBuilding, ((BuildingClassTypes)iI))).c_str());
    						}
    
    						szBuffer.append(szTempBuffer);
    						[B][COLOR="Red"]bShowedPrereq = true;[/COLOR][/B]
    					}
    				}
    			}
    			[s][B][COLOR="Red"]else[/COLOR][/B][/s] if ([B][COLOR="Red"]!bShowedPrereq && [/COLOR][/B]kBuilding.isBuildingClassNeededInCity(iI))
    // BUG - Unofficial Patch - end
    
    Edit2: Related to the above, here are two XML <Text> fixes so that the "Requires Hospital (x/y)" lines make the building into a link in the Civilopedia:

    Code:
    	<!-- General BTS Fixes -->
    	
    	<TEXT>
    		<Tag>TXT_KEY_BUILDING_REQUIRES_NUM_SPECIAL_BUILDINGS</Tag>
    		<!-- %s1_Name = Can be Buildings, Units, etc -->
    		<English>[COLOR_WARNING_TEXT]Requires [B][COLOR="Red"][LINK=literal][/COLOR][/B]%s1_Name[B][COLOR="Red"][\LINK][/COLOR][/B] (%d2_NumCurr/%d3_NumTotal Total)[COLOR_REVERT]</English>
    		<French>[COLOR_WARNING_TEXT]Requiert : [LINK=literal]%s1_Name[\LINK] (%d2_NumCurr/%d3_NumTotal au total)[COLOR_REVERT]</French>
    		<German>[COLOR_WARNING_TEXT]Erfordert: [LINK=literal]%s1_Name[\LINK] (%d2_NumCurr/%d3_NumTotal gesamt)[COLOR_REVERT]</German>
    		<Italian>[COLOR_WARNING_TEXT]Richiede [LINK=literal]%s1_Name[\LINK] (%d2_NumCurr/%d3_NumTotal totali)[COLOR_REVERT]</Italian>
    		<Spanish>[COLOR_WARNING_TEXT]Requiere [LINK=literal]%s1_Name[\LINK] (%d2_NumCurr de un total de %d3_NumTotal)[COLOR_REVERT]</Spanish>
    	</TEXT>
    	<TEXT>
    		<Tag>TXT_KEY_BUILDING_REQUIRES_NUM_SPECIAL_BUILDINGS_NO_CITY</Tag>
    		<!-- %s1_Name = Can be Buildings, Units, etc -->
    		<English>[COLOR_WARNING_TEXT]Requires [LINK=literal]%s1_Name[\LINK] (%d2_NumTotal Total)[COLOR_REVERT]</English>
    		<French>[COLOR_WARNING_TEXT]Requiert : [LINK=literal]%s1_Name[\LINK] (%d2_NumTotal au total)[COLOR_REVERT]</French>
    		<German>[COLOR_WARNING_TEXT]Erfordert: [LINK=literal]%s1_Name[\LINK] (%d2_NumTotal gesamt)[COLOR_REVERT]</German>
    		<Italian>[COLOR_WARNING_TEXT]Richiede [LINK=literal]%s1_Name[\LINK] (%d2_NumTotal totali)[COLOR_REVERT]</Italian>
    		<Spanish>[COLOR_WARNING_TEXT]Requiere [LINK=literal]%s1_Name[\LINK] (%d2_NumTotal en total)[COLOR_REVERT]</Spanish>
    	</TEXT>
    
     

Share This Page