RevolutionDCM for BTS

[phungus420]

No surprise, but no dice, Assert never trips, and crash still occurs. I tried adding this bit of code:

CivilizationTypes CvInitCore::getCiv(PlayerTypes eID) const
{
	if (!m_aeCiv)
	{
		FAssertMsg(false, "something odd is going on");

		return NO_CIVILIZATION;
	}

	FASSERT_BOUNDS(0, MAX_PLAYERS, eID, "CvInitCore::getCiv");
	if ( checkBounds(eID, 0, MAX_PLAYERS) )
	{
		[B]FASSERT_BOUNDS(-1, GC.getNumCivilizationInfos(), m_aeCiv[eID], "CvInitCore::getCiv, CivilizationType out of bounds");
		if ( checkBounds(m_aeCiv[eID], -1, GC.getNumCivilizationInfos()) )
		{
			return m_aeCiv[eID];
		}
		else
		{
			return NO_CIVILIZATION;
		}[/B]
	}
	else
	{
		return NO_CIVILIZATION;
	}
}

Doing so causes an access violation error crash in "Vector" (making the min bound 0 also screws things up horribly, as I forgot that -1 would be used for all the players that hadn't been created yet). Trying to backtrack this function, you find it's only really used once in CvPlayer:

CivilizationTypes CvPlayer::getCivilizationType() const														
{
	return GC.getInitCore().getCiv(getID());
}

But the function CvPlayer::getCivilizationType() is called too many times to count to try to backtrack it further.

Any ideas why that code I added would cause it to crash in vector, and never trip the assert message?

 

[Fimbulvetr]

I honestly don't get it. If m_aeCiv[eID] causes the access violation .. why?

 

[Afforess]

What if the civilization type is being accessed before the array is constructed?

 

[Fimbulvetr]

How do you check against that? I'm guessing what I suggested only works for pointers...
Would m_aeCiv[eID] simply being undefined (m_aeCiv exists and has size MAX_PLAYERS but the value being accessed has never been assigned a value) also cause this access violation?

 

[Afforess]

If it was undefined, there would be a value, and you could access it w/o crashing, but it would be garbage. If it wasn't constructed the pointer would equal NULL. At least - that's how I understand it. Correct me if I'm wrong.

 

[Fimbulvetr]

I agree but if that's how it works, then (!m_aeCiv) should be true if the array isn't constructed. And if it is constructed, which it is from the beginning until you quit the game, trying to access a value within the bounds of the array shouldn't cause any trouble either.
But couldn't it be that debug dlls work differently and access to the undefined m_aeCiv[eID] causes this error instead of returning garbage? idk ..

 

[phungus420]

If it were null (!m_aeCiv) would catch it, it does not. What I don't understand is why it still crashes with the above code that checks the value inside the array at the specified point (to ensure that the returned civilization type is valid). As far as I know this check shouldn't crash, unless m_aeCiv were null, which it is checked for already; and if there is garbage in there, it should just do what what the code does when garbage is returned, fire an assert and return -1. I don't get what could be crashing, it doesn't make any sense.

 

[Fimbulvetr]

But if that (debug dlls not handling garbage) was true, then CvGame::addPlayer should already cause this error on

	CivilizationTypes eOldCiv = GET_PLAYER(eNewPlayer).getCivilizationType();

and to fix it you'd need to add a

GC.getInitCore().resetPlayer(eNewPlayer)

at the beginning. But as soon as

	GC.getInitCore().setCiv(eNewPlayer, eCiv);

is reached, m_aeCiv[eID] should have a real value, not just garbage, so I probably just proved that my idea was wrong. again.

 

[phungus420]

Well, I'm not sure where and when the call to CvInitCore::getCiv is being made that causes the crash. This function is called 1000s of times a turn, it's impractical to set a breakpoint and watch it (everytime CvPlayer::getCivilizationType() is called, so is this function).

OK, here is what we know.

1)m_aeCiv[playerID] is being called in a way that causes an access violation error (when the integer math is performed on the pointer to get the place in memory where m_aeCiv(playerID) is, the memory address is outside of where the memory was allocated for the array)
2)the pointer *m_aeCiv is not set to NULL
3)The playerID is a valid ID, so it's not jumping outside of the array.

Because of 3 and 2, the only thing I know that can cause this is if the pointer is dangling, but how do you check for a dangling pointer, and how is it possible for this pointer to be dangling in the first place?

 

[mamba]

OK, here is what we know.

1)m_aeCiv[playerID] is being called in a way that causes an access violation error (when the integer math is performed on the pointer to get the place in memory where m_aeCiv(playerID) is, the memory address is outside of where the memory was allocated for the array)
2)the pointer *m_aeCiv is not set to NULL
3)The playerID is a valid ID, so it's not jumping outside of the array.

Because of 3 and 2, the only thing I know that can cause this is if the pointer is dangling, but how do you check for a dangling pointer, and how is it possible for this pointer to be dangling in the first place?

It has been a while since I had to bother with C and I never worked on the civ dll, so take this with a grain of salt, but doesn't 2) and 3) ensure that 1) does not happen ?

Does the access to m_aeCiv[playerID] cause the violation ? if so 2) or 3) has to be false from my understanding (or the types are simply not compatible, causing the calculation to be off). Could it be that *m_aeCiv[playerID] causes the violation, in which case the array may not be initialized correctly ?

 

[Fimbulvetr]

Yes mamba, it should not happen, we all agree. Too bad it still does.

@phungus: You could 7zip me all RevDCM assets&source files that are different from what's currently on svn, and I can try to create this error on my own system, maybe my debugger reveals more.. yes, doubtful, but that's all I can think of.

 

[phungus420]

Well in terms of just general computer science, you must look at the array as a pointer that points to a memory address. An array is just a shorthand way for the computer to find memory with integer math performed on the address. So in a conceptual way say *m_aeCiv is pointing to 02412 as the memory adress, then m_aeCiv[0] points to this location, maeCiv[1] will point to 02413, and maeCiv[10] points to 02423, etc (it's a little more complicated, since the type of memory will also define the size of the memory needed, thus the integer math may mean the memory block jumps wol't be straight up integers, but rather the needed data for the bits times the size of the data type of the array, but conceptually this is how it works). Now when you set up an array the computer sets asside the block of memory the size it needs in order to access any address up to the size of the array, so if you declare maeCiv[99] as the size of your array when it is created, then all the memory cells from 02412 to 02512 will be set as reserved for this array. Now if a function asks for the memory outside this block of reserved memory for the array, the program will instantly terminate as an access violation error. I'm not sure why it does this rather then returning garbage, it's probably a safety feature to ensure you don't start screwing with vital data on random locations on the hard drive. Now when a pointer is set to NULL then it points to nothing, which is why !maeCiv would catch it, as the pointer is set to nothing. A dangling pointer however, or a pointer that points to a non specific adress on the hard drive, but is still pointing somewhere is still pointing to something so !thePointer call wol't return true, but trying to access memory from it using integer math on the pointer (how an array works) will just cause an instant crash; this is the only thing I can figure out could be going on. My CS knowledge isn't very advanced though, so I could be missing something else, but this is the only thing that makes sense to me, since this specific function checks to see if the adress is within the bounds of the memory allocated for the array, and with Fuyu's code also ensures that the pointer isn't NULL.

 

[Fimbulvetr]

Start as Minors, Limited Religions <- I'm about to start a test game with these settings on, the rest off. Any changes required?
I'm using last revision from svn.

edit: My computer science skills obviously aren't that much more advanced since I simply agree with everything you said and can't really add anything either.

 

[phungus420]

Give me a few moments, I'm in the middle of working on things and updating the SVN, also I have a big post with some info and a critical bug I've found in BBAI that you should probably look at first.

 

[Fimbulvetr]

Ok np, I'll wait then.

 

[phungus420]

@Fuyu

I was doing my testing with LoR, since they are virtually the same code wise, just added art and units in LoR that make things more interesting to watch when doing AIAutoplay testing. I loaded up straight RevDCM and ran it through AIAutoplay waiting for this bug to rear it's head, then realized that my only save with this bug occuring was an Autosave. It is now lost... However when trying to load up a previous save from LoR to replicate this bug (figured it would happen in a couple of turns, wasn't sure how far back it was), I saw the save I had was from a different game, so it seems totally lost. For some reason though I just ran that save forward with AIAutoplay and ran into another crash. With this crash I am 100% sure it is an issue with BBAI, as the crash occurs in bool CvUnit::canMoveInto(const CvPlot* pPlot, bool bAttack, bool bDeclareWar, bool bIgnoreLoad) const
Looking at the crash logs it apears to be an issue with on of Hamurabi's settlers he is trying to load on a boat, at least it appears this way, though I'm sure it's an issue in this function and is being caused by one of Hamurabi's settlers. Normally with a crash like this I'd upload the save game with the crash in the BBAI forums and jdog would download LoR's SVN and squash the bug, as it's definately BBAI and not some added content in LoR that is causing it. Unfortunately jdog has announced he is no longer working on BBAI

I am uploading the save here for you. The save requires the latest version of LoR which you can find on the SVN here:

LoR the mod itself: https://civ4lor.svn.sourceforge.net/svnroot/civ4lor/Trunk/LoR/
LoR source code: https://civ4lor.svn.sourceforge.net/svnroot/civ4lor/Trunk/LoRSource/CvGameCoreDLL/

[edit]This crash may require a debug dll, I didn't test it with a release dll, to reproduce just go into AI autoplay and go forward 1 turn, and you may be able to replicate it with just pressing enter, not sure, but always got it by going into AIAuotplay[/edit]

For now I will update the current RevDCM SVN with the new fixes from here and have a go with some AIAutoPlay in straight up RevDCM and see if I can't reproduce the m_aeCiv access violation error bug, at which time I'll upload the save and post here.

 

[phungus420]

OK, I have updated the current SVN of RevDCM. I'll go ahead and run trough some games using AIAutoplay and see if I can't reproduce the afformentioned crash with the getCiv function and upload a save if I can. For now though Fuyu, please try to figure out the issue with CvUnit::canMoveInto, this is probably a rare bug as BetterBugAI hasn't gotten many crash reports, or it could be a crash intrinsic to debug dlls.

 

[Fimbulvetr]

I've run hundreds of turns with AIAutoplay using a debug dll of Better BUG AI, and I fixed all the asserts I got from Civ4lerts and another one from BUG scoreboard smilies. CanMoveInto is either very rare or not a better ai issue.

 

[Fimbulvetr]

Seems like minor civs declare war even on dead teams and when they settle into full civs, they also try to make peace with dead teams . And when a team dies, it makes peace with all players too, including the dead ..
My debugger says the settleMinorCiv fails because of AI logging in makePeace(), checking for isAlive() should help:

/************************************************************************************************/
/* BETTER_BTS_AI_MOD                      05/21/10                                jdog5000      */
/*                                                                                              */
/* AI logging                                                                                   */
/************************************************************************************************/
		if( gTeamLogLevel >= 1 [B]&& isAlive() && && GET_TEAM(eTeam).isAlive()[/B])
		{
			logBBAI("      Team %d (%S) and team %d (%S) make peace", getID(), GET_PLAYER(getLeaderID()).getCivilizationDescription(0), eTeam, [COLOR="Red"]GET_PLAYER(GET_TEAM(eTeam).getLeaderID()).getCivilizationDescription(0)[/COLOR]);
		}
/************************************************************************************************/
/* BETTER_BTS_AI_MOD                       END                                                  */
/************************************************************************************************/

(those are the results of playing rev545 - or 546 since i could fix that myself)

 

[phungus420]

Nice find Fuyu, that looks to be the cause of the team strangeness issues.

In regards to the crash in CvUnit::canMoveInto, I've ran across these about a dozen times before with testing, and after uploading them in the BBAI forums they were always due to BBAI code. I'm like 99% positive this is an issue with trying to load that settler onto a boat. Crashes like these were always exceedingly rare (otherwise jdog would catch them when first implementing the code). I'd probably need to run a few dozen full games on my computer before coming across it again, which on this old rig would litterally take a month of 24-7 AIAutoplay. Please take a look at that save and see if you can't figure out what's going on with that settler and why it is causing this function to crash.