Updating BULL to the latest Unofficial Patch

_alphaBeta_

_alphaBeta_

King
Joined
Dec 28, 2005
Messages
976
Location
NJ, USA
I began, and I think finished, the task of combining BULL build 111 with v1.4 of the Unofficial Patch for 3.19.

Two sticky points:

Sticky Point #1



CvGameTextMgr::getOtherRelationsString procedure originally looked like this in the official 3.19:

Code: 
void CvGameTextMgr::getOtherRelationsString(CvWStringBuffer& szString, PlayerTypes eThisPlayer, PlayerTypes eOtherPlayer)
{
	if (eThisPlayer == NO_PLAYER || eOtherPlayer == NO_PLAYER)
	{
		return;
	}
	CvPlayer& kThisPlayer = GET_PLAYER(eThisPlayer);
	CvPlayer& kOtherPlayer = GET_PLAYER(eOtherPlayer);

	for (int iTeam = 0; iTeam < MAX_CIV_TEAMS; ++iTeam)
	{
		CvTeamAI& kTeam = GET_TEAM((TeamTypes) iTeam);
		if (kTeam.isAlive() && !kTeam.isMinorCiv() && iTeam != kThisPlayer.getTeam() && iTeam != kOtherPlayer.getTeam())
		{
			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()));
				}

				if (!kTeam.isHuman() && kTeam.AI_getWorstEnemy() == kThisPlayer.getTeam())
				{
					szString.append(NEWLINE);
					szString.append(gDLL->getText(L"TXT_KEY_WORST_ENEMY_OF", kTeam.getName().GetCString()));
				}
			}
		}
	}
}
EF made this change as part of BULL. It was probably an earlier version of the unofficial patch that prompted the change. I'm not sure of the history.

Code: 
void CvGameTextMgr::getOtherRelationsString(CvWStringBuffer& szString, PlayerTypes eThisPlayer, PlayerTypes eOtherPlayer)
{
	if (eThisPlayer == NO_PLAYER || eOtherPlayer == NO_PLAYER)
	{
		return;
	}
	CvPlayer& kThisPlayer = GET_PLAYER(eThisPlayer);
	CvPlayer& kOtherPlayer = GET_PLAYER(eOtherPlayer);

	for (int iTeam = 0; iTeam < MAX_CIV_TEAMS; ++iTeam)
	{
		CvTeamAI& kTeam = GET_TEAM((TeamTypes) iTeam);
		if (kTeam.isAlive() && !kTeam.isMinorCiv() && iTeam != kThisPlayer.getTeam() && iTeam != kOtherPlayer.getTeam())
		{
[COLOR="Red"]// BUG - Unofficial Patch - start
			// EF: don't show enemies status of or wars with players that the active player hasn't met
			if (kTeam.isHasMet(kOtherPlayer.getTeam()) && kTeam.isHasMet(GC.getGameINLINE().getActiveTeam()))
// BUG - Unofficial Patch - end[/COLOR]			{
				if (::atWar((TeamTypes) iTeam, kThisPlayer.getTeam()))
				{
					szString.append(NEWLINE);
					szString.append(gDLL->getText(L"TXT_KEY_AT_WAR_WITH", kTeam.getName().GetCString()));
				}

				if (!kTeam.isHuman() && kTeam.AI_getWorstEnemy() == kThisPlayer.getTeam())
				{
					szString.append(NEWLINE);
					szString.append(gDLL->getText(L"TXT_KEY_WORST_ENEMY_OF", kTeam.getName().GetCString()));
				}
			}
		}
	}
}

Focusing on the change itself:
Code: 
if (kTeam.isHasMet(kOtherPlayer.getTeam()))
if (kTeam.isHasMet(kOtherPlayer.getTeam()) [COLOR="Red"]&& kTeam.isHasMet(GC.getGameINLINE().getActiveTeam()))[/COLOR]

v1.4 of the unofficial patch takes this line even further:
Code: 
if (kTeam.isHasMet(kOtherPlayer.getTeam()) && kTeam.isHasMet(kThisPlayer.getTeam()) && kTeam.isHasMet(GC.getGameINLINE().getActiveTeam()))
There's also numerous other changes in the v1.4 citing 'BETTER_BTS_AI' (pretty much everything below this line is different, same structure, but different). From reading the comments and staring at the code I think all this is doing is cleaning up how "worst enemy" and "at war with" statements are displayed.
Code: 
void CvGameTextMgr::getOtherRelationsString(CvWStringBuffer& szString, PlayerTypes eThisPlayer, PlayerTypes eOtherPlayer)
{
	if (eThisPlayer == NO_PLAYER || eOtherPlayer == NO_PLAYER)
	{
		return;
	}
	CvPlayer& kThisPlayer = GET_PLAYER(eThisPlayer);
	CvPlayer& kOtherPlayer = GET_PLAYER(eOtherPlayer);

/************************************************************************************************/
/* BETTER_BTS_AI                         11/08/08                                jdog5000       */
/*                                                                                              */
/*                                                                                              */
/************************************************************************************************/
/* original code
	for (int iTeam = 0; iTeam < MAX_CIV_TEAMS; ++iTeam)
	{
		CvTeamAI& kTeam = GET_TEAM((TeamTypes) iTeam);
		if (kTeam.isAlive() && !kTeam.isMinorCiv() && iTeam != kThisPlayer.getTeam() && iTeam != kOtherPlayer.getTeam())
		{
			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()));
				}

				if (!kTeam.isHuman() && kTeam.AI_getWorstEnemy() == kThisPlayer.getTeam())
				{
					szString.append(NEWLINE);
					szString.append(gDLL->getText(L"TXT_KEY_WORST_ENEMY_OF", kTeam.getName().GetCString()));
				}
			}
		}
	}
*/
	// Put all war, worst enemy strings on one line
	CvWStringBuffer szWarWithString;
	CvWStringBuffer szWorstEnemyString;
	bool bFirst = true;
	bool bFirst2 = true;
	for (int iTeam = 0; iTeam < MAX_CIV_TEAMS; ++iTeam)
	{
		CvTeamAI& kTeam = GET_TEAM((TeamTypes) iTeam);
		if (kTeam.isAlive() && !kTeam.isMinorCiv() && iTeam != kThisPlayer.getTeam() && iTeam != kOtherPlayer.getTeam())
		{
/************************************************************************************************/
/* UNOFFICIAL_PATCH                       09/28/09                 Emperor Fool & jdog5000      */
/*                                                                                              */
/*                                                                                              */
/************************************************************************************************/
/* original bts code
			if (kTeam.isHasMet(kOtherPlayer.getTeam()))
*/
[COLOR="Red"]			if (kTeam.isHasMet(kOtherPlayer.getTeam()) && kTeam.isHasMet(kThisPlayer.getTeam()) && kTeam.isHasMet(GC.getGameINLINE().getActiveTeam()))[/COLOR]
/************************************************************************************************/
/* UNOFFICIAL_PATCH                        END                                                  */
/************************************************************************************************/
			{
				if (::atWar((TeamTypes) iTeam, kThisPlayer.getTeam()))
				{
					setListHelp(szWarWithString, L"", kTeam.getName().GetCString(), L", ", bFirst);
					bFirst = false;
				}

				if ( !kTeam.isHuman() && kTeam.AI_getWorstEnemy() == kThisPlayer.getTeam() )
				{
					setListHelp(szWorstEnemyString, L"", kTeam.getName().GetCString(), L", ", bFirst2);
					bFirst2 = false;
				}
			}
		}
	}

	if( !szWorstEnemyString.isEmpty() )
	{
		CvWString szTempBuffer;

		szTempBuffer.assign(gDLL->getText(L"TXT_KEY_WORST_ENEMY_OF", szWorstEnemyString));

		szString.append(NEWLINE);
		szString.append(szTempBuffer);
	}
	if( !szWarWithString.isEmpty() )
	{
		CvWString szTempBuffer;

		szTempBuffer.assign(gDLL->getText(L"TXT_KEY_AT_WAR_WITH", szWarWithString));

		szString.append(NEWLINE);
		szString.append(szTempBuffer);
	}

/************************************************************************************************/
/* BETTER_BTS_AI                          END                                                   */
/************************************************************************************************/
}
What do you BUG guys make of this?

Sticky Point #2


Some undocumented changes that the unofficial patch guys did:

Code: 
int CvTeamAI::AI_calculateAdjacentLandPlots(TeamTypes eTeam) const
{
	[COLOR="Red"]PROFILE_FUNC();[/COLOR]


bool CvUnit::canMoveInto(const CvPlot* pPlot, bool bAttack, bool bDeclareWar, bool bIgnoreLoad) const
{
	[COLOR="red"]PROFILE_FUNC();[/COLOR]
Are these necessary?
 
I left out that extra check because two teams cannot be at war if they haven't met, and AFAIK you cannot be the worst enemy of a team you haven't met either. It doesn't hurt to leave it in, though.

As for the rewriting, I only skimmed it but it looks like it just reorganizes it so that there is only one statement for each thing. Instead of

At war with Bob
Worst enemy of Bob
At war with Frank
Worst enemy of Sally​

you'd see

At war with Bob, Frank
Worst enemy of Bob, Sally​

I'm not attached to either presentation. In large games the list can get rather unruly, but I think the only real advantage to the UP's method is to shorten the text when it gets very long. Anyone else have an opinion?
 
Understood. That's what I made out of it. I just wasn't sure about this line.
Code: 
if (kTeam.isHasMet(kOtherPlayer.getTeam()) && kTeam.isHasMet(kThisPlayer.getTeam()) && kTeam.isHasMet(GC.getGameINLINE().getActiveTeam()))
Guess it doesn't much matter.

Any thoughts on my second sticky point about PROFILE_FUNC();?
 
kThisPlayer is the player who has the attitude, kOtherPlayer is the player about whom the attitude is being checked, and active team is the active player's team who is the person sitting at the computer. This line just checks that the team being checked for WE/war status has met all three teams above.

The PROFILE_FUNC() calls are for timing code execution. The UP should not be adding any calls to these functions, but it won't cause harm. They probably just leaked out of BBAI which is where jdog does his primary coding. I would leave them out personally.
 
EmperorFool said:
I left out that extra check because two teams cannot be at war if they haven't met, and AFAIK you cannot be the worst enemy of a team you haven't met either. It doesn't hurt to leave it in, though.

As for the rewriting, I only skimmed it but it looks like it just reorganizes it so that there is only one statement for each thing. Instead of
At war with Bob
Worst enemy of Bob
At war with Frank
Worst enemy of Sally​
you'd see
At war with Bob, Frank
Worst enemy of Bob, Sally​
I'm not attached to either presentation. In large games the list can get rather unruly, but I think the only real advantage to the UP's method is to shorten the text when it gets very long. Anyone else have an opinion?
Click to expand...

Shorter is better IMHO.
 
Attached is a PATCH file for updating BULL 112 to the newest Unofficial Patch v1.4 (build 24). May come in handy someday if you guys want to update BULL. I haven't put it through the paces yet, but it seems stable from preliminary testing.

Also included in an update to BUG (build 2092) to go along with Unofficial Patch v1.4.
 

Attachments

  • update.zip
    91.9 KB · Views: 70
Mainly for EmperorFool:

I started to merge BULL rev 137 into my own BULL which tracked the unofficial patch as well (so they should be relatively the same). I noticed the following:

Did you miss this unofficial patch entry regarding floodplanes?

Code: 
Left file: BULL_SVN 137\CvCity.cpp
Right file: My BTS Changes\CvGameCoreDLL_SVN\CvCity.cpp
                                                       <> 220 /************************************************************************************************/
                                                          221 /* UNOFFICIAL_PATCH                       10/30/09                     Mongoose & jdog5000      */
                                                          222 /*                                                                                              */
                                                          223 /* Bugfix                                                                                       */
                                                          224 /************************************************************************************************/
                                                          225 /* original bts code
220         if (pPlot->getFeatureType() != NO_FEATURE)    226         if (pPlot->getFeatureType() != NO_FEATURE)
                                                          227 */
                                                          228         // From Mongoose SDK
                                                          229         // Don't remove floodplains from tiles when founding city
                                                          230         if ((pPlot->getFeatureType() != NO_FEATURE) && (pPlot->getFeatureType() != (FeatureTypes)GC.getInfoTypeForString("FEATURE_FLOOD_PLAINS")))
                                                          231 /************************************************************************************************/
                                                          232 /* UNOFFICIAL_PATCH                        END                                                  */
                                                          233 /************************************************************************************************/

How about this one? Shouldn't iTempValue be used?
Code: 
Left file: BULL_SVN 137\CvCityAI.cpp
Right file: My BTS Changes\CvGameCoreDLL_SVN\CvCityAI.cpp
                                                                  <> 3143 /************************************************************************************************/
                                                                     3144 /* UNOFFICIAL_PATCH                       01/09/10                                jdog5000      */
                                                                     3145 /*                                                                                              */
                                                                     3146 /* Bugfix                                                                                       */
                                                                     3147 /************************************************************************************************/
                                                                     3148 /* original bts code
                                                                     3149                                 if (iFoodDifference < 2)
3142                                                                 3150                                 {
                                                                     3151                                     iValue /= 4;
                                                                     3152                                 }
                                                                     3153                                 if (iRunnable > 0)
                                                                     3154                                 {
                                                                     3155                                     iValue /= 1 + iRunnable;
                                                                     3156                                 }
                                                                     3157 */
------------------------------------------------------------------------
3143                                 if (iFoodDifference < 2)     =  3158                                 if (iFoodDifference < 2)
3144                                 {                               3159                                 {
------------------------------------------------------------------------
3145                                     iValue /= 4;             <> 3160                                     iTempValue /= 4;
------------------------------------------------------------------------
3146                                 }                            =  3161                                 }
3147                                 if (iRunnable > 0)              3162                                 if (iRunnable > 0)
3148                                 {                               3163                                 {
------------------------------------------------------------------------
3149                                     iValue /= 1 + iRunnable; <> 3164                                     iTempValue /= 1 + iRunnable;
------------------------------------------------------------------------
3150                                 }                            =  3165                                 }
------------------------------------------------------------------------
                                                                  <> 3166 /************************************************************************************************/
                                                                     3167 /* UNOFFICIAL_PATCH                        END                                                  */
                                                                     3168 /************************************************************************************************/


Mainly housekeeping, but this block of code has no comments. Looks like you added it at rev 126.

Code: 
void CvPlayerAI::AI_changeMemoryCount(PlayerTypes eIndex1, MemoryTypes eIndex2, int iChange)
{
	FAssertMsg(eIndex1 >= 0, "eIndex1 is expected to be non-negative (invalid Index)");
	FAssertMsg(eIndex1 < MAX_PLAYERS, "eIndex1 is expected to be within maximum bounds (invalid Index)");
	FAssertMsg(eIndex2 >= 0, "eIndex2 is expected to be non-negative (invalid Index)");
	FAssertMsg(eIndex2 < NUM_MEMORY_TYPES, "eIndex2 is expected to be within maximum bounds (invalid Index)");
	m_aaiMemoryCount[eIndex1][eIndex2] += iChange;
[COLOR="Red"]	if (eIndex1 == GC.getGameINLINE().getActivePlayer())
	{
		gDLL->getInterfaceIFace()->setDirty(Score_DIRTY_BIT, true);
	}[/COLOR]
	FAssert(AI_getMemoryCount(eIndex1, eIndex2) >= 0);
}

More housekeeping, your OOS fix is not commented:

Code: 
void CvSelectionGroup::checkLastPathPlot(CvPlot* pPlot)
{
	m_bLastPathPlotChecked = true;
	if (pPlot != NULL)
	{
[COLOR="Red"]		m_bLastPlotVisible = pPlot->isVisible(getTeam(), false);
		m_bLastPlotRevealed = pPlot->isRevealed(getTeam(), false);[/COLOR]	}
	else
	{
		m_bLastPlotVisible = false;
		m_bLastPlotRevealed = false;
	}
}

Remove calls to PROFILE_FUNC():

Code: 
int CvTeamAI::AI_calculateAdjacentLandPlots(TeamTypes eTeam) const
{
[COLOR="Red"]	PROFILE_FUNC();[/COLOR]
	CvPlot* pLoopPlot;
	int iCount;
	int iI;

	FAssertMsg(eTeam != getID(), "shouldn't call this function on ourselves");

	iCount = 0;

	for (iI = 0; iI < GC.getMapINLINE().numPlotsINLINE(); iI++)
	{
		pLoopPlot = GC.getMapINLINE().plotByIndexINLINE(iI);

		if (!(pLoopPlot->isWater()))
		{
			if ((pLoopPlot->getTeam() == eTeam) && pLoopPlot->isAdjacentTeam(getID(), true))
			{
				iCount++;
			}
		}
	}

	return iCount;
}

Code: 
bool CvUnit::canMoveInto(const CvPlot* pPlot, bool bAttack, bool bDeclareWar, bool bIgnoreLoad) const
{
[COLOR="Red"]	PROFILE_FUNC();[/COLOR]

	FAssertMsg(pPlot != NULL, "Plot is not assigned a valid value");

	if (atPlot(pPlot))
	{
		return false;
	}
[snip]

isCargo fix not commented:
Code: 
Left file: \BULL_SVN 137\CvUnit.cpp
Right file: \My BTS Changes\CvGameCoreDLL_SVN\CvUnit.cpp
                             <> 3159 /************************************************************************************************/
                                3160 /* UNOFFICIAL_PATCH                       11/06/09                     Mongoose & jdog5000      */
                                3161 /*                                                                                              */
                                3162 /* Bugfix                                                                                       */
                                3163 /************************************************************************************************/
                                3164     // From Mongoose SDK
                                3165     /*if (isCargo())
                                3166     {
                                3167         return false;
                                3168     }*/
                                3169 /************************************************************************************************/
                                3170 /* UNOFFICIAL_PATCH                        END                                                  */
                                3171 /************************************************************************************************/
                                3172
------------------------------------------------------------------------
3161     if (getCargo() > 0) =  3173     if (getCargo() > 0)
3162     {                      3174     {
3163         return false;      3175         return false;
3164     }                      3176     }
------------------------------------------------------------------------
------------------------------------------------------------------------
 
Oh right, the uncommented dirty bit block from r126, I accidentally removed that once for my mod while merging in a new BBAI without paying enough attention. Good thing I noticed right afterwards but comments would have helped.
 
First, thanks guys for helping out. I always intend to include comments, so if you spot changed code without comments it's definitely an error on my part. Please ping me as you have so I can add them.

@_alphaBeta_ - I totally forgot you had posted that update for BULL and 1.4. :( I spent a few hours last night merging in 1.4 to BULL. Oh well, nothing better to do when I'm sick I suppose. One product of the merge is that I swapped all of my fixes and comments for jdog's except in a couple cases.

1. That flood plains "fix" is no such thing. It changes a game rule that doesn't need changing. I have seen no official message from Firaxis saying they meant to remove only jungles and forests, so it's not a bug in my book. I agree they shouldn't be removed from a reality/gameplay perspective, but the UP is not the place to make changes like that. I'll leave those for mods like PIG.

The other place was the canFoundCitiesOnWater callback. jdog changed the fix I posted to the thread, so I kept my version because I am 39% sure that my fix is more correct. I'm 43% sure I don't know which is correct. I posted to the UP thread.

_alphaBeta_ said:
How about this one? Shouldn't iTempValue be used?
Click to expand...

Did I miss that in 1.4, or are you looking at the latest UP?

_alphaBeta_ said:
More housekeeping, your OOS fix is not commented:
Click to expand...

I don't comment changes to my own code as it would get very messy really quick. Does this make it harder to merge incrementally?

_alphaBeta_ said:
Remove calls to PROFILE_FUNC():
Click to expand...

I noticed that jdog added several calls to PROFILE_FUNC() in the UP, and I merged them right along in to make it easier for others to merge BULL. Are you saying I should not do this, or are these additions/deletions from a later UP?

_alphaBeta_ said:
isCargo fix not commented:
Click to expand...

I left out the fix because the fix itself is commented out--it has zero effect. I'll put it back in, though, just to be consistent.
 
EmperorFool said:
@_alphaBeta_ - I totally forgot you had posted that update for BULL and 1.4. :( I spent a few hours last night merging in 1.4 to BULL. Oh well, nothing better to do when I'm sick I suppose. One product of the merge is that I swapped all of my fixes and comments for jdog's except in a couple cases.
Click to expand...
Feel better. :)
I was pleased to see you adopt the comment style of the UP. Made things easy.

EmperorFool said:
1. That flood plains "fix" is no such thing. It changes a game rule that doesn't need changing. I have seen no official message from Firaxis saying they meant to remove only jungles and forests, so it's not a bug in my book. I agree they shouldn't be removed from a reality/gameplay perspective, but the UP is not the place to make changes like that. I'll leave those for mods like PIG.

The other place was the canFoundCitiesOnWater callback. jdog changed the fix I posted to the thread, so I kept my version because I am 39% sure that my fix is more correct. I'm 43% sure I don't know which is correct. I posted to the UP thread.
Click to expand...
Understood.

EmperorFool said:
Did I miss that in 1.4, or are you looking at the latest UP?
Click to expand...
Looks like you got me there. This change is from rev 28. Official UP 1.4 release was from rev 24. :blush:


EmperorFool said:
I don't comment changes to my own code as it would get very messy really quick. Does this make it harder to merge incrementally?
Click to expand...
I forgot that this is code from you originally. Nevermind.


EmperorFool said:
I noticed that jdog added several calls to PROFILE_FUNC() in the UP, and I merged them right along in to make it easier for others to merge BULL. Are you saying I should not do this, or are these additions/deletions from a later UP?
Click to expand...
Those calls have been there for some time according to the UP SVN logs. We discussed this earlier when I asked you what they were. Your response was that they were for timing and didn't really matter. I suppose they should be removed from the UP, but they were there for the 1.4 release, so I guess it makes sense to leave them in.

EmperorFool said:
I left out the fix because the fix itself is commented out--it has zero effect. I'll put it back in, though, just to be consistent.
Click to expand...
I had the changes reversed, and didn't realize the 3.19 code was left standing. It looks like the UP had those lines active in rev18, but they were commented out in rev24 (which is the 1.4 release). That whole comment block should probably be removed from the UP since it talks about a fix that was removed. The comment block was technically there for the 1.4 release though - but rather misleading.

Conclusion: You successfully refuted all my challenges. Sorry to waste your time. :blush:
 
[Edit: Looks like I've been ninja'd!]

I'm going off of the 1.4 tag, and the change isn't there. You're looking at the latest SVN version. I try to pull from released versions to make it easier on modders, users, and me.

Maybe it's time for a 1.5 UP release? ;)
 
_alphaBeta_ said:
Feel better. :)
Click to expand...

Thanks, staying up til 4am doesn't do wonders for that, but that's my fault and it feels really good to get BULL up to snuff with the latest UP.

_alphaBeta_ said:
Conclusion: You successfully refuted all my challenges. Sorry to waste your time. :blush:
Click to expand...

:lol: It's never a waste of time. I would rather spend a little extra time making sure BULL is top quality. :)
 
EmperorFool said:
:lol: It's never a waste of time. I would rather spend a little extra time making sure BULL is top quality. :)
Click to expand...

Slightly OT, but since I agree with that statement, and BULL introduces some Asserts in the city screen, perhaps you could look at how Jdog fixed your asserts in RevDCM? ;)

Just want BULL to be bug-free... well, not literally,... you get my point... :lol:
 
Do you happen to have the assertion that fails handy? File and line #? Does it happen when the City Screen opens? Every time?
 
You must log in or register to reply here.

Similar threads

Maniac
Local specialist yield changes
Replies
9
Views
2K
Maniac
Maniac
Kael
Case Study: Adding new XML attributes and using them in the SDK
4 5 6
Replies
101
Views
33K
MCG13
M
Top Bottom