Unofficial Patch for 3.19

[PieceOfMind]

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

 

[phungus420]

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.

 

[KaytieKat]

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

 

[denev]

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

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

 

[PieceOfMind]

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

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.

 

[EmperorFool]

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 with something queued after it when the tech is completed?

 

[Roland Johansen]

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

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

 

[PieceOfMind]

Does the same thing when building 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.

 

[deanej]

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

 

[jdog5000]

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.

 

[jdog5000]

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

 

[EmperorFool]

@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.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:

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

BBAI:

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?:

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

 

[EmperorFool]

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. 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()?

 

[EmperorFool]

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?

 

[jdog5000]

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.

 

[jdog5000]

I can't seem to find where this happens so I can fix it in BULL. 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 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/

 

[EmperorFool]

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

 

[EmperorFool]

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?

 

[EmperorFool]

This XML Event fix should be included.

 

[jdog5000]

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.