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. PieceOfMind

    PieceOfMind Drill IV Defender Retired Moderator

    Joined:
    Jan 15, 2006
    Messages:
    9,319
    Location:
    Australia
    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...
     
  2. phungus420

    phungus420 Deity

    Joined:
    Mar 1, 2003
    Messages:
    6,296
    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.
     
  3. KaytieKat

    KaytieKat King

    Joined:
    Jul 29, 2007
    Messages:
    999
    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
     
  4. denev

    denev Warlord

    Joined:
    Jul 5, 2008
    Messages:
    208
    Location:
    Japan
    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.
     

    Attached Files:

  5. PieceOfMind

    PieceOfMind Drill IV Defender Retired Moderator

    Joined:
    Jan 15, 2006
    Messages:
    9,319
    Location:
    Australia
    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
     
  6. EmperorFool

    EmperorFool Deity

    Joined:
    Mar 2, 2007
    Messages:
    9,633
    Location:
    Mountain View, California
    Does the same thing when building :science: with something queued after it when the tech is completed?
     
  7. Roland Johansen

    Roland Johansen Deity

    Joined:
    Apr 29, 2003
    Messages:
    4,292
    Location:
    the Netherlands
    This is also true in BTS 3.17 (not that that makes it ok).
     
  8. PieceOfMind

    PieceOfMind Drill IV Defender Retired Moderator

    Joined:
    Jan 15, 2006
    Messages:
    9,319
    Location:
    Australia
    I'm away from my normal computer for a few days so someone else will need to check. I'd prefer not to speculate.
     
  9. deanej

    deanej Deity

    Joined:
    Apr 8, 2006
    Messages:
    4,859
    Location:
    New York State
    Found a bug in 3.19 that only shows up when you mod it:
    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).
     
  10. jdog5000

    jdog5000 Revolutionary

    Joined:
    Nov 25, 2003
    Messages:
    2,601
    Location:
    California
    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.
     
  11. jdog5000

    jdog5000 Revolutionary

    Joined:
    Nov 25, 2003
    Messages:
    2,601
    Location:
    California
    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:
     
  12. EmperorFool

    EmperorFool Deity

    Joined:
    Mar 2, 2007
    Messages:
    9,633
    Location:
    Mountain View, California
    @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:

    Code:
    /*************************************************************************************************/
    /** 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.getNumCultureLevelInfos() - 1)).getSpeedThreshold(GC.getGameINLINE().getGameSpeedType()))
    			{
    				iWeight /= 50;
    			}
    			// UNOFFICIAL_PATCH 
    			else if (AI_isDoStrategy(AI_STRATEGY_CULTURE3) [B]|| getCommercePercent(COMMERCE_CULTURE) > 80[/B] )
    ...
    
    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:

    Code:
    if (getCivicPercentAnger(getCivics((CivicOptionTypes)(GC.getCivicInfo(eCivic).getCivicOptionType())),true) > getCivicPercentAnger(eCivic))
    
    BBAI:

    Code:
    if ( getCivicPercentAnger(eCivic) < getCivicPercentAnger(getCivics((CivicOptionTypes)(GC.getCivicInfo(eCivic).getCivicOptionType())),true) )
    
    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?:

    Code:
    if (NO_BONUS != pLoopPlot->getNonObsoleteBonusType(getTeam()))
    {
    	iValue += std::max([B]1[/B], GET_PLAYER(eTargetPlayer).AI_bonusVal(pLoopPlot->getBonusType()) - 10);
    }
    
     
  13. EmperorFool

    EmperorFool Deity

    Joined:
    Mar 2, 2007
    Messages:
    9,633
    Location:
    Mountain View, California
    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()?
     
  14. EmperorFool

    EmperorFool Deity

    Joined:
    Mar 2, 2007
    Messages:
    9,633
    Location:
    Mountain View, California
    Also, I didn't see Alexman's proposed fix for the colony spawn bug introduced in 3.19 in BBAI 0.78. Is this in the next version?
     
  15. jdog5000

    jdog5000 Revolutionary

    Joined:
    Nov 25, 2003
    Messages:
    2,601
    Location:
    California
    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:

    Code:
    /*************************************************************************************************/
    /** 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                                                  */
    /*************************************************************************************************/
    
    You're quite right, removed from future versions. Thanks for the careful checking!

    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.

    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.
     
  16. jdog5000

    jdog5000 Revolutionary

    Joined:
    Nov 25, 2003
    Messages:
    2,601
    Location:
    California
    Yes, that's where it is. Here's have I've got:

    Code:
    /*************************************************************************************************/
    /** 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                                                  */
    /*************************************************************************************************/
    
    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/
     
  17. EmperorFool

    EmperorFool Deity

    Joined:
    Mar 2, 2007
    Messages:
    9,633
    Location:
    Mountain View, California
    Cool, my fix for that differs only by variable name. ;)
     
  18. EmperorFool

    EmperorFool Deity

    Joined:
    Mar 2, 2007
    Messages:
    9,633
    Location:
    Mountain View, California
    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

    Code:
    if ( kLoopUnit.getMoves() > 1 )
    
    compared to the new code

    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?
     
  19. EmperorFool

    EmperorFool Deity

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

    jdog5000 Revolutionary

    Joined:
    Nov 25, 2003
    Messages:
    2,601
    Location:
    California
    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.
     

Share This Page