Specific Bug Reports

Hey Jdog.

First of all, I cant thank you enough for all the work you've done here!

Now i recently dled the latest version. I use BAI with the Full of Resources and XXL world mods, by simply replacing the dll with yours. In the previous version (0.84 i believe), this worked fine. However, with 1.00 (48 civs version), when i load it it comes up with 6 messages such as failed to load BBAI_Global_defines, BBAI_Tech_Diffusion and BBAI_Lead_from_Behind.

It still loads up, and i can play maps, but will this cause problems for me?

Thanks for all you're help and the work you've done!
 
Search for the XML files with the same name in the BBAI mod and copy them as well to the similar places in the mods you are coping the BBAI dll file into ... IIRC that is :p
 
In CvUnitAI::AI_leaveAttack()

Code: 
				if (AI_plotValid(pLoopPlot))
				{
					if (pLoopPlot->isVisibleEnemyUnit(this) || (pLoopPlot->isCity() && AI_potentialEnemy(pLoopPlot->getTeam(), pLoopPlot)))
					{
/************************************************************************************************/
/* BETTER_BTS_AI_MOD                      02/22/10                                jdog5000      */
/*                                                                                              */
/* Efficiency                                                                                   */
/************************************************************************************************/
						if (pLoopPlot->getNumVisibleEnemyDefenders(this) > 0)
						{
							if (!atPlot(pLoopPlot) && (generatePath(pLoopPlot, 0, true, &iPathTurns) && (iPathTurns <= iRange)))
							{
/************************************************************************************************/
/* BETTER_BTS_AI_MOD                       END                                                  */
/************************************************************************************************/
						
								iValue = getGroup()->AI_attackOdds(pLoopPlot, true);

The isVisibleEnemyUnit() and getNumVisibleEnemyDefenders() calls are redundant (in a segment labelled "Efficiency" heh), and more significantly the latter call invalidates the isCity() second half of the "or" condition before it, which may or may not be causing a problem (depending on whether the city-related conditions were really meant to allow this code to run, which I'm not sure about).

The same situation exists in CvUnitAI::AI_anyAttack() btw, although here the getNumVisibleEnemyDefenders() call is compared against the iMinStack argument with a >= condition, so if a zero were ever passed in it would still allow the city conditions to function (and it would allow the isVisibleEnemyUnit() check to function for that matter, since it could return a Worker or something that wouldn't show up as a defender).
 
Code: 
/************************************************************************************************/
/* BETTER_BTS_AI_MOD                      05/16/10                              jdog5000        */
/*                                                                                              */
/* War tactics AI                                                                               */
/************************************************************************************************/
void CvPlayerAI::AI_conquerCity(CvCity* pCity)
{
	bool bRaze = false;
	int iRazeValue;
	int iI;

	if (canRaze(pCity))
	{
		iRazeValue = 0;
		int [COLOR="Blue"]iCloseness[/COLOR] = [COLOR="Red"][B]pCity[/B][/COLOR]->AI_playerCloseness(getID());

As you can seen, AI_playerCloseness is only given one argument (if that's the right word?). That function requires two arguments however, and no default value is given to the second argument in the header file.

int AI_playerCloseness(PlayerTypes eIndex, int iMaxDistance);

If I understand correctly, this means playercloseness will be calculated in this case with some random large number as iMaxDistance. I assume that is not intended??
 
Maniac said:
As you can seen, AI_playerCloseness is only given one argument (if that's the right word?). That function requires two arguments however, and no default value is given to the second argument in the header file.
Click to expand...

Yes, "argument" is the correct term. From CvCity.h:

Code: 
	virtual int AI_playerCloseness(PlayerTypes eIndex, int iMaxDistance = 7) = 0;

Just because the Civ4 SDK doesn't make a lot of use of C++ as an object-oriented language, doesn't mean it never does. ;)
 
Also, keep in mind that in C++ a missing argument that doesn't have a default value is a compile-time error: the compiler will stop and issue an error. The only time you'll get seemingly random values is when you forget to initialize variables (and ignore the compiler warning and have the compiler set to not auto-initialize) or initialize allocated memory (e.g. arrays).

For function arguments, the compiler generates code that writes the values onto the stack (a data structure for passing values to functions). It always writes some known value. Now, you can certainly declare a variable without initializing it and pass that to a function, resulting in whatever happened to be in the memory location assigned to the variable at the time. :)
 
r580 CvGame.cpp Line 7335
Code: 
logBBAI("  Vote for assigning %S to %d (%S) passes", pCity->getName(), GET_PLAYER(kData.kVoteOption.eOtherPlayer).getTeam(), GET_PLAYER(kData.kVoteOption.eOtherPlayer).getCivilizationDescription(0) );

The second argument should be
Code: 
pCity->getName().GetCString()

Under some circumstances, this mistake will yield a CTD.

By the way, IMO, using %S to handle the conversion between chars and wchars, namely string and wstring, is not a good choice.
I prefer to use MultiByteToWideChar/WideCharToMultiByte.
 
dilandau said:
By the way, IMO, using %S to handle the conversion between chars and wchars, namely string and wstring, is not a good choice.
Click to expand...

It's not a question of choice... The game has always used %s everywhere and at every level (SDK/Python/XML), without any problems, and we are just working with what already exists so we're sticking with its protocols. Thanks for the bug report though!
 
LunarMongoose said:
It's not a question of choice... The game has always used %s everywhere and at every level (SDK/Python/XML), without any problems, and we are just working with what already exists so we're sticking with its protocols. Thanks for the bug report though!
Click to expand...

%S != %s

using %s to format output, yes, no problem.
But using %S to convert chars/wchars, that's another story.

For example, the following is the method Firaxis used.
Code: 
class CvString : public std::string
{
public:
	......
	explicit CvString(const std::wstring& s) { Copy(s.c_str()); }		// don't want accidental conversions down to narrow strings
	......
	void Copy(const wchar* w)
	{
		if (w)
		{
			int iLen = wcslen(w);
			if (iLen)
			{
				char *s = new char[iLen+1];
				sprintf(s, "%S", w);	// convert
				assign(s);
				delete [] s;
			}
		}
	}
	.....
}

But there is a better way to deal with such conversion.
Code: 
{
	......
	void Copy(const std::wstring& w)
	{
		if (w.c_str())
		{
			int iLen = WideCharToMultiByte(CP_ACP, 0, w.c_str(), -1, NULL, 0, NULL, NULL);
			if (iLen)
			{
				char *s = new char[iLen];
				WideCharToMultiByte(CP_ACP, 0, w.c_str(), -1, s, iLen, NULL, NULL);
				assign(s);
				delete [] s;
			}
		}
	}
	......
}

When wstring/string only contains ascii characters, sprintf/swprintf with %S can accomplish the conversion perfectly.
If wstring/string has some non-ascii characters, then sprintf/swprint might not format buf correctly.

For example, the following text was generated by logBBAI(" Vote for assigning %S to %d (%S) passes", pCity->getName().GetCString(), GET_PLAYER(kData.kVoteOption.eOtherPlayer).getTeam(), GET_PLAYER(kData.kVoteOption.eOtherPlayer).getCivilizationDescription(0) );
Code: 
[9545.654]   Vote for assigning  to 0 () passes
city name and civilization description didn't appear due to such incorrect conversion.
 
TheOnlyDJCat said:
Hi again,

I just started a new team multiplayer game after updating to BBAI 1.0 (and BUG mod 4.4) and around 1000BC CIV crashes to desktop with the "Microsoft Runtime crashes for civbts.exe" message. From now on this happened every 10 minutes.
I tried reboot, graphic driver reinstall, emptying civ cache, lower graphic settings, compatibility mode for XP (runnning Windows7 64). But the crashes continued to happen, when I or my friend are doing something in the GUI (lowering taxes, moving units etc.)

Anybody seen this crash with BBAI 1.0? I have used the several BBAI (and BUG) versions almost from the beginning of this fantastic mod and never got this bug :(
Click to expand...

I haven't heard any other reports of MP issues with BBAI ... have you figured anything out?
 
LunarMongoose said:
In CvUnitAI::AI_leaveAttack()

Code: 
				if (AI_plotValid(pLoopPlot))
				{
					if (pLoopPlot->isVisibleEnemyUnit(this) || (pLoopPlot->isCity() && AI_potentialEnemy(pLoopPlot->getTeam(), pLoopPlot)))
					{
/************************************************************************************************/
/* BETTER_BTS_AI_MOD                      02/22/10                                jdog5000      */
/*                                                                                              */
/* Efficiency                                                                                   */
/************************************************************************************************/
						if (pLoopPlot->getNumVisibleEnemyDefenders(this) > 0)
						{
							if (!atPlot(pLoopPlot) && (generatePath(pLoopPlot, 0, true, &iPathTurns) && (iPathTurns <= iRange)))
							{
/************************************************************************************************/
/* BETTER_BTS_AI_MOD                       END                                                  */
/************************************************************************************************/
						
								iValue = getGroup()->AI_attackOdds(pLoopPlot, true);

The isVisibleEnemyUnit() and getNumVisibleEnemyDefenders() calls are redundant (in a segment labelled "Efficiency" heh), and more significantly the latter call invalidates the isCity() second half of the "or" condition before it, which may or may not be causing a problem (depending on whether the city-related conditions were really meant to allow this code to run, which I'm not sure about).

The same situation exists in CvUnitAI::AI_anyAttack() btw, although here the getNumVisibleEnemyDefenders() call is compared against the iMinStack argument with a >= condition, so if a zero were ever passed in it would still allow the city conditions to function (and it would allow the isVisibleEnemyUnit() check to function for that matter, since it could return a Worker or something that wouldn't show up as a defender).
Click to expand...

That code is not my doing ... the efficiency label is wrong, the only change I made in that part of AI_leaveAttack was to enforce that path distances to the new target were in fact less than the max range. So it's a bug fix.

Firaxis uses that nested structure of defense tests in a number of places, so there's probably a reason to it.
 
jdog5000 said:
That code is not my doing ... the efficiency label is wrong, the only change I made in that part of AI_leaveAttack was to enforce that path distances to the new target were in fact less than the max range. So it's a bug fix.

Firaxis uses that nested structure of defense tests in a number of places, so there's probably a reason to it.
Click to expand...

If I remember correctly (too tired to check atm) you also inverted the nest order from vanilla, which is probably why you tagged it Efficiency. However my point stands; there is absolutely no Firaxian reason that could explain this code lol. VisibleEnemyDefenders is a more restrictive version of VisibleEnemyUnit which nulls the first half of the outer level condition outright, and b/c it's alone it also totally wastes the "|| (pLoopPlot->isCity() && AI_potentialEnemy(pLoopPlot->getTeam(), pLoopPlot)))" check, which can be true or false it doesn't matter; it can only get you through the first if.

With the original vanilla nest order it probably seemed like it was okay at a glance, but if you merge the two if's with an "and" and look at it logically you'll see what I mean. I also did search through the rest of the AI files before posting about this, and didn't find any other wasted condition checks like this. I'm sure they do use the nested structure a lot, but everywhere else the components work together properly.

I mean, the fix to improve speed and maintain current functionality is just to completely delete the outer if with both of its irrelevant conditions, but I was hoping you could figure out whether the "|| (isCity() && AI_potentialEnemy())" thing was supposed to be there or not, b/c if it was then it needs to be written back into the function.
 
argh! can anyone help me merging bonus commerce with bbai latest developer?

this code:

PHP: 
				if (!gDLL->getXMLIFace()->NextSibling(pXML->GetXML()))
				{
					break;
				}
			}

			// set the current xml node to it's parent node
			gDLL->getXMLIFace()->SetToParent(pXML->GetXML());
		}

		// set the current xml node to it's parent node
		gDLL->getXMLIFace()->SetToParent(pXML->GetXML());
	}
	
//BCM: Added 21.9.09
	
	pXML->Init2DIntList(&m_ppaiBonusCommerceModifier, GC.getNumBonusInfos(), NUM_COMMERCE_TYPES);
	if (gDLL->getXMLIFace()->SetToChildByTagName(pXML->GetXML(),"BonusCommerceModifiers"))
	{
		iNumChildren = gDLL->getXMLIFace()->GetNumChildren(pXML->GetXML());

		if (gDLL->getXMLIFace()->SetToChildByTagName(pXML->GetXML(),"BonusCommerceModifier"))
		{
			for(j=0;j<iNumChildren;j++)
			{
				pXML->GetChildXmlValByName(szTextVal, "BonusType");
				k = pXML->FindInInfoClass(szTextVal);
				if (k > -1)
				{
					// delete the array since it will be reallocated
					SAFE_DELETE_ARRAY(m_ppaiBonusCommerceModifier[k]);
					if (gDLL->getXMLIFace()->SetToChildByTagName(pXML->GetXML(),"CommerceModifiers"))
					{
						// call the function that sets the commerce change variable
						pXML->SetCommerce(&m_ppaiBonusCommerceModifier[k]);
						gDLL->getXMLIFace()->SetToParent(pXML->GetXML());
					}
					else
					{
						pXML->InitList(&m_ppaiBonusCommerceModifier[k], NUM_COMMERCE_TYPES);
					}

				}

				if (!gDLL->getXMLIFace()->NextSibling(pXML->GetXML()))
				{
					break;
				}
			}

			// set the current xml node to it's parent node
			gDLL->getXMLIFace()->SetToParent(pXML->GetXML());
		}

		// set the current xml node to it's parent node
		gDLL->getXMLIFace()->SetToParent(pXML->GetXML());

with this code

PHP: 
				if (!gDLL->getXMLIFace()->NextSibling(pXML->GetXML()))
				{
					break;
				}
			}

			for(int ii=0;(!m_bAnyBonusYieldModifier) && ii<GC.getNumBonusInfos(); ii++)
			{
				for(int ij=0; ij < NUM_YIELD_TYPES; ij++ )
				{
					if( m_ppaiBonusYieldModifier[ii][ij] != 0 )
					{
						m_bAnyBonusYieldModifier = true;
						break;
					}
				}
			}

			// set the current xml node to it's parent node
			gDLL->getXMLIFace()->SetToParent(pXML->GetXML());
		}

		// set the current xml node to it's parent node
		gDLL->getXMLIFace()->SetToParent(pXML->GetXML());



edit: like this maybe?

PHP: 
				if (!gDLL->getXMLIFace()->NextSibling(pXML->GetXML()))
				{
					break;
				}
			}

			for(int ii=0;(!m_bAnyBonusYieldModifier) && ii<GC.getNumBonusInfos(); ii++)
			{
				for(int ij=0; ij < NUM_YIELD_TYPES; ij++ )
				{
					if( m_ppaiBonusYieldModifier[ii][ij] != 0 )
					{
						m_bAnyBonusYieldModifier = true;
						break;
					}
				}
			}
			// set the current xml node to it's parent node
			gDLL->getXMLIFace()->SetToParent(pXML->GetXML());
		}

		// set the current xml node to it's parent node
		gDLL->getXMLIFace()->SetToParent(pXML->GetXML());
	}
	
//BCM: Added 21.9.09
	
	pXML->Init2DIntList(&m_ppaiBonusCommerceModifier, GC.getNumBonusInfos(), NUM_COMMERCE_TYPES);
	if (gDLL->getXMLIFace()->SetToChildByTagName(pXML->GetXML(),"BonusCommerceModifiers"))
	{
		iNumChildren = gDLL->getXMLIFace()->GetNumChildren(pXML->GetXML());

		if (gDLL->getXMLIFace()->SetToChildByTagName(pXML->GetXML(),"BonusCommerceModifier"))
		{
			for(j=0;j<iNumChildren;j++)
			{
				pXML->GetChildXmlValByName(szTextVal, "BonusType");
				k = pXML->FindInInfoClass(szTextVal);
				if (k > -1)
				{
					// delete the array since it will be reallocated
					SAFE_DELETE_ARRAY(m_ppaiBonusCommerceModifier[k]);
					if (gDLL->getXMLIFace()->SetToChildByTagName(pXML->GetXML(),"CommerceModifiers"))
					{
						// call the function that sets the commerce change variable
						pXML->SetCommerce(&m_ppaiBonusCommerceModifier[k]);
						gDLL->getXMLIFace()->SetToParent(pXML->GetXML());
					}
					else
					{
						pXML->InitList(&m_ppaiBonusCommerceModifier[k], NUM_COMMERCE_TYPES);
					}

				}

				if (!gDLL->getXMLIFace()->NextSibling(pXML->GetXML()))
				{
					break;
				}
			}

			// set the current xml node to it's parent node
			gDLL->getXMLIFace()->SetToParent(pXML->GetXML());
		}

		// set the current xml node to it's parent node
		gDLL->getXMLIFace()->SetToParent(pXML->GetXML());
	}
	
//BCM: End
/************************************************************************************************/
/* UNOFFICIAL_PATCH                        END                                                  */
/************************************************************************************************/
 
jdog5000 said:
Thanks dilandau, I didn't know the subtleties and was just mimicking Firaxis. Would making that change in Copy solve the issue?
Click to expand...

It shouldn't be regarded as an issue.
For western languages, sprintf/swprintf with %S brings us no problem, basically no problem.
Considering there are tons of format strings and lots of them contain %S, I thought it's unnecessary to adopt the new way.

Though, if I were the author of CvString.h, I would have chosen MultiByteToWideChar/WideCharToMultiByte to convert string/wstring.
 
jdog5000 said:
I haven't heard any other reports of MP issues with BBAI ... have you figured anything out?
Click to expand...

Hi JDog,

tested two times again after downgrading to BUG Mod 4.3 -> no crashes anymore, so I'm quite sure it is a BUG Mod problem.

Nevertheless not really a bug, but we could not win a game after upgrading to BBAI 1.0 on immortal. Really strong AIs now ;)
 
TheOnlyDJCat said:
but we could not win a game after upgrading to BBAI 1.0 on immortal. Really strong AIs now ;)
Click to expand...

When BBAI reaches version 2.0 the AIs will transcend Civ4 and start playing against you in real life; then you'll really feel the pressure. Just lettin ya know. :p
 
LunarMongoose said:
Heck a city gets at least 4 points for each resource in its radius... (...) Actually that minimum value of 4 per resource is ridiculous, I'm changing it to 2 right now lol, then I need to look at the values AI_bonusVal() returns and see if those need work too.
Click to expand...
I should really stop trusting you blindly :p

iValue += std::min(4,std::max(1, AI_bonusVal(bla)/10));

This will add at least 1 point, and at most 4 points for a single resource. Which makes a lot more sense.
Spoiler :
My current values, since I had to do some changing after finding out I read the "std::min(4" part wrong:
Code: 
	if (pCity->isCoastal(GC.getMIN_WATER_SIZE_FOR_OCEAN()))
	{
		iValue += 2;
	}

	iValue += 4*pCity->getNumActiveWorldWonders();

	for (iI = 0; iI < GC.getNumReligionInfos(); iI++)
	{
		if (pCity->isHolyCity((ReligionTypes)iI))
		{
			iValue += std::max(2,((GC.getGameINLINE().calculateReligionPercent((ReligionTypes)iI)) / 5));
			iValue += ((AI_getFlavorValue(/* AI_FLAVOR_RELIGION */ (FlavorTypes)1) +2) / 6);

			if (GET_PLAYER(pCity->getOwnerINLINE()).getStateReligion() == iI)
			{
				iValue += 2;
			}
			if (getStateReligion() == iI)
			{
				iValue += 8;
			}
			if (GC.getLeaderHeadInfo(getLeaderType()).getFavoriteReligion() == iI)
			{
				iValue += 1;
			}
		}
 
You must log in or register to reply here.

Similar threads

Laurana Kanan
  • Sticky
Guidelines for Reporting Bugs at Civfanatics
Replies
0
Views
2K
Laurana Kanan
Laurana Kanan
kcd_swede
[BTS] BOTM 293: Willem, Monarch - Final spoiler (game submitted)
Replies
0
Views
121
kcd_swede
kcd_swede
kcd_swede
[BTS] BOTM 292: Boudica, Immortal; Final Spoiler (game submitted or abandoned)
Replies
7
Views
389
Mitchum
M
Leoreth
City Names Feedback Thread
2
Replies
20
Views
1K
Lilna
L
ATARi144kHz
Highly demanding Civ VII w/o DLC save is needed for Intel/AMD processors reviews
Replies
0
Views
230
ATARi144kHz
ATARi144kHz
Back
Top Bottom