Fall Further 050 Bug Report Thread

[xienwolf]

The proper fix would be:

				CvCity* pCity = GC.getMapINLINE().findCity(getX_INLINE(), getY_INLINE(), NO_PLAYER, eTeam, false, false, NO_TEAM, NO_DIRECTION, 0, true);
				if (pCity == NULL)
				{
					return false;
				}
				if (plotDistance(getX_INLINE(), getY_INLINE(), pCity->getX(), pCity->getY()) < 10)

I swear that was in there once upon a time, but quite possibly it wasn't. That section of the code was an afterthought to be honest.

What it is doing is keeping the AI from thinking that explorable lairs are Goody Tiles (and thus aiming to walk to them) if they meet the conditions which block the AI from exploration in Python (basically the lair is too close to their undefended cities to be safe to explore). If they don't have ANY cities ANYWHERE, you want to tell them to ignore the lair and move elsewhere (it ignores settlements because it doesn't matter much if you lose those to Barbarian lair spawns)


Nice, second page with the other crash analyzed. Still not quite the ideal solution, mainly due to the comment for the import being somewhat misleading. (UnitTypes)iI does not even come close to matching up nicely with (UnitClass)iI. But this entire function is in horrible need of a re-write from the ground up (just look at how horribly it would select an ideal city defender and the lack of Archers defending cities becomes alarmingly obvious). But the other fix is pretty much spot-on (doesn't actually matter what you divide by since if you get a 0 in that denominator it means you had a 0 in the numerator as well)

 

[Dragonlord]

DEFINITELY name a Great Engineer AND a Great Sage after Moot...

Assume I rightly there'll be a new patch coming?

Edit: BTW, has anyone solved the hang I posted yet?

 

[notque]

Excellent stuff Moot

 

[xienwolf]

Not sure when a new patch will be along. Doing some things with the AI right now which need serious testing, and not quite enough time to do everything we want to at present. Depends on how the weekend goes in the end I fear. Haven't had a chance to play with your WoC myself, but I thought someone had posted some progress on checking it out, I might be imagining things.

 

[notque]

DEFINITELY name a Great Engineer AND a Great Sage after Moot...

Assume I rightly there'll be a new patch coming?

Edit: BTW, has anyone solved the hang I posted yet?

I haven't solved it yet, I found out it was barbarian related, did a ton more tests, didn't track it down and got very busy.

I hope to look through it some more later tonight.

 

[vivictius]

Getting a CTD at the end of turn, no python error.

-------------------------------------------------------

Seems to be related to the error MootPoint was getting, looks like daracaat was just finishing researching Corruption of Spirit and that caused the crash (i.e. doesnt crash if I delete him or just give him the tech).

.... on a side note, I've noticed Daracaat seems to tech rather well for a Barbarian trait leader, is there a difference in his AI?

 

[xienwolf]

Holy Cat in the Hat! That is a LOT of Civs and a HUGE map. Amazed my laptop even loaded the thing, let alone finished the turn.

Anyway, it asked me to choose what the Overcouncil votes on next, no crash at all (also went through the next turn without crashing, autoplayed it instead of just forcing the turn to end). Had you done anything special during the turn after the point of the save, or can you load this save, hit END TURN and insta-crash?

 

[vivictius]

Lol, actually, its is Nikis-Knights map with the extra FF civs added in.

 

[Thunder_Gr]

I think your debugger's lying to you

I've tracked the problem down to CvPlot.cpp, line 3874: the code assumes that the FindCity() function will come back with a valid pointer. Here's my patch:

I am sure you are right . I cannot debug with codeblocks in the source window, so, I was sure the results had a high chance to be unreliable. I should have given more notice to the fact that it was itterating through the ..plot.. function before it came down to the crash, though...

And, thank you for the exellent debug work you are doing here!

There's two crashes here:

First one's in CvPlayerAI::AI_calculateUnitAIViability()

Here's my patch -

	for (int iI = 0; iI < GC.getNumUnitClassInfos(); iI++)
	{
		UnitTypes eLoopUnit = (UnitTypes)GC.getUnitClassInfo((UnitClassTypes)iI).getDefaultUnitIndex();
/*************************************************************************************************/
/** Better AI                      03/18/09               Written: jdog5000  Imported: Notque   **/
/**                                                                                             **/
/** Seems like the call before this one should be checking for our UUs instead of the default	**/
/** Then of course we need to not do any of this at all if we don't have any unit for this class**/
/**                                                                                             **/
/**				More Units from cities with mil prod bonuses								    **/
/*************************************************************************************************/
/**								---- Start Original Code ----									**
		CvUnitInfo& kUnitInfo = GC.getUnitInfo((UnitTypes)iI);
/**								----  End Original Code  ----									**/
		FAssert(eLoopUnit > NO_UNIT);
		// Mootpoint 03/26/09 - fall back to a safe value if the default unit index is invalid
		if (eLoopUnit == NO_UNIT)
		{
			eLoopUnit = (UnitTypes)iI;
		}
		CvUnitInfo& kUnitInfo = GC.getUnitInfo(eLoopUnit);

Crash number two is in CvCityAI.cpp, line 7927 (this one's the div by zero). Proposed patch:

iSlaveryValue *= iConsumtionPerPop * 2;
iSlaveryValue /= std::max(1, iConsumtionPerPop * 2 + std::max(0, iAdjustedFoodDifference));

Nice, second page with the other crash analyzed. Still not quite the ideal solution, mainly due to the comment for the import being somewhat misleading. (UnitTypes)iI does not even come close to matching up nicely with (UnitClass)iI. But this entire function is in horrible need of a re-write from the ground up (just look at how horribly it would select an ideal city defender and the lack of Archers defending cities becomes alarmingly obvious).

I assume, however, that it does a temporary workaround until the method is rewritten, right?

But the other fix is pretty much spot-on (doesn't actually matter what you divide by since if you get a 0 in that denominator it means you had a 0 in the numerator as well)

Does this mean that

if(iSlaveryValue!=0) iSlaveryValue /= iConsumtionPerPop * 2 + std::max(0, iAdjustedFoodDifference);

would also do the trick?

Not sure when a new patch will be along. Doing some things with the AI right now which need serious testing, and not quite enough time to do everything we want to at present. Depends on how the weekend goes in the end I fear. Haven't had a chance to play with your WoC myself, but I thought someone had posted some progress on checking it out, I might be imagining things.

I hope you don't mind me compiling the fixes and uploading them, until the next patch, then?

I include 2 dlls. The first is the one containing the fix for my crash(added the Xienwolf code for the NULL check) and the fixes for the other crash as mootpoint posted them.
No other change.

The second, contain the above fixes and the new CvCity.cpp, as posted by mootpoint in another thread.

 

[Valkrionn]

Hmm.. Why is Herne not included in FF? His art popup is, I'm assuming it's just an oversight?

 

[Guybrush!]

As the Scions, if you build the Blasted Garden (maybe other buildings that give food) your city (Not in city viewer) will look like it is growing due to food, though the bar doesn't actually fill.

 

[MootPoint]

Edit: BTW, has anyone solved the hang I posted yet?

I'm looking at it now. This one's really bizarre. I'm able to get your save game working by hacking in the debugger.

It's kind of complicated to explain, but basically, in CvGame.cpp's update() call, there's this check:

		if (getNumGameTurnActive() == 0)
		{
			if (!isPbem() || !getPbemTurnSent())
			{
				doTurn();
			}
		}

the getNumGameTurnActive() flips between 1 & 0, and basically what controls this is setTurnActive(). The code loops through each player, sets each one active, and in the code below, in updateMoves() call in CvGame.cpp, the code that calls setAutoMoves(false) is what resets setTurnActive() to false.

		if (player.isAlive())
		{
			if (player.isTurnActive())
			{
				if (!(player.isAutoMoves()))
				{
					player.AI_unitUpdate();

					if (!(player.isHuman()))
					{
						if (!(player.hasBusyUnit()) && !(player.hasReadyUnit(true)))
						{
							player.setAutoMoves(true);
						}
					}
				}

				if (player.isAutoMoves())
				{
					for(pLoopSelectionGroup = player.firstSelectionGroup(&iLoop); pLoopSelectionGroup; pLoopSelectionGroup = player.nextSelectionGroup(&iLoop))
					{
						pLoopSelectionGroup->autoMission();
					}

					if (!(player.hasBusyUnit()))
					{
						player.setAutoMoves(false);
					}
				}

The problem in your save file is that the last player (id 50) thinks that it has ready units, so unlike for all the other AI players, it doesn't flip on & off the setAutoMoves(), so the main update() loop is left in a weird state & never calls doTurn().

So I'm suspicious of the code block above. I find this wierd:

if (!(player.hasBusyUnit()) && !(player.hasReadyUnit(true)))
{
	player.setAutoMoves(true);
}

And yet,

if (!(player.hasBusyUnit()))
{
	player.setAutoMoves(false);
}

Why isn't there parity here on the use of hasReadyUnit()? I suspect that's a bug. Unfortunately, this is code from the original BTS SDK, so I'm not sure what to make of it.

 

[Thunder_Gr]

I had occurrances of enemy units withdrawing from my fireballs' attacks...I suppose it is a bug?

 

[Iceciro]

My group using the new DLL file above, with CvGameCoreDLL_withCvCity.rar has an issue where in MP, the players that aren't the host are unable to load savegames (Made with that DLL tonight, not a different one.) with the error. States the mod is invalid, we tried to do it multiple times with multiple saves . May be unrelated to the DLL and something we've done, but it sure doesn't seem like it.

 

[Thunder_Gr]

My group using the new DLL file above, with CvGameCoreDLL_withCvCity.rar has an issue where in MP, the players that aren't the host are unable to load savegames (Made with that DLL tonight, not a different one.) with the error. States the mod is invalid, we tried to do it multiple times with multiple saves . May be unrelated to the DLL and something we've done, but it sure doesn't seem like it.

Of course they have all the same DLL, right? I was ready to post a warning about waiting for an official patch for multiplayer use, and use the DLLs provided only for single player..but I was too late, it seems.

Perhaps it turns out for the better though...We could see that as "advanced MP debugging". There is no debug info in these DLLs, just for your information.(Linked with /debug but propably ignored since it was compiled full optimizations, NO_DEBUG...etc and the other linker switches were for max optimizations. I may remove the /debug switch in the next compilation, I just left it there because it was preplaced)

Of course, I would really like it if someone of the FF team could post a list of the compiler and linker switches they use.
I cannot use /Zi, with codeblocks, because it hits a: "compiler limit reached: Debug module size exceeded" before compilation finishes. But this, of course, is not a Final_Release issue, since final_release is never compiled with /Zi.

 

[xienwolf]

I assume, however, that it does a temporary workaround until the method is rewritten, right?

Sorry, thought that I had posted the preferred method for that function as well. The preference would be instead of setting eLoopUnit = (UnitTypes)iI to just place everything after assignment of eLoopUnit in a "If (eLoopUnit != NO_UNIT)" block. Then it won't evaluate the NULL unit for combat strength and attempt to replace the currently selected "Best Unit" for the job.

Does this mean that

if(iSlaveryValue!=0) iSlaveryValue /= iConsumtionPerPop * 2 + std::max(0, iAdjustedFoodDifference);

would also do the trick?

Aye, that would work just fine. Well, until someone comes along with a strange case where they have negative food consumption per population or something.

I hope you don't mind me compiling the fixes and uploading them, until the next patch, then?

Meh, feel free. Kinda wierd to have unofficial patches for a Modmod, but quite interesting all the same. Not sure what the issue is with the MP players posted later, and I always just used the default compiler options when I had been in Codeblocks, and haven't moved far from the defaults in VS now that I use it. To be honest, I haven't a clue what /Zi actually does for you. And the error you get is what we had seen for some of the python files when attempting to build a debug DLL (it is why CyMapInterface was split into 2 files)

So I'm suspicious of the code block above. I find this wierd:

if (!(player.hasBusyUnit()) && !(player.hasReadyUnit(true)))
{
	player.setAutoMoves(true);
}

And yet,

if (!(player.hasBusyUnit()))
{
	player.setAutoMoves(false);
}

Why isn't there parity here on the use of hasReadyUnit()? I suspect that's a bug. Unfortunately, this is code from the original BTS SDK, so I'm not sure what to make of it.

I'd need longer to look things over, and maybe to step through it with a debugger. But I would imagine that the main issue is units not being passed a valid mission assignment during their doTurn pass. Thus the ideal fix might be to install a second loop over all units to re-assign them missions, or isolating why one unit wasn't assigned a SKIP mission after failing to be assigned anything else.

 

[Thunder_Gr]

Sorry, thought that I had posted the preferred method for that function as well. The preference would be instead of setting eLoopUnit = (UnitTypes)iI to just place everything after assignment of eLoopUnit in a "If (eLoopUnit != NO_UNIT)" block. Then it won't evaluate the NULL unit for combat strength and attempt to replace the currently selected "Best Unit" for the job.

Nice, On my way to apply it.

Aye, that would work just fine. Well, until someone comes along with a strange case where they have negative food consumption per population or something.

Not really possible since iConsumtionPerPop * 2 will always be 0 or positive.

Meh, feel free. Kinda wierd to have unofficial patches for a Modmod, but quite interesting all the same.

If you people manage to post the code fixes, I will be able to allow the others continue with their saved games until the new patch can be released with new features.

Not sure what the issue is with the MP players posted later, and I always just used the default compiler options when I had been in Codeblocks, and haven't moved far from the defaults in VS now that I use it. To be honest, I haven't a clue what /Zi actually does for you. And the error you get is what we had seen for some of the python files when attempting to build a debug DLL (it is why CyMapInterface was split into 2 files)

/Zi is the command line switch for "include debug information". Yes, I did have the problem when trying to compile a Debug Version, but I can't get it done. Could it be I can do it if I exclude some files from the compilation?

 

[xienwolf]

iCon ^2 would always be 0 or pos. iCon * 2 can easily be negative, but will always be even.

Far as I know, codeblocks is just not capable of doing a Debug build, ever. Only ones I have heard of being created are all with VS

 

[Dragonlord]

I'm looking at it now. This one's really bizarre.

I take a certain perverse pride in that!

Thus the ideal fix might be to install a second loop over all units to re-assign them missions, or isolating why one unit wasn't assigned a SKIP mission after failing to be assigned anything else.

You might start by looking at those Hill Giants. As I mentioned in the original post, I saw one of those Hill Giants poised to attack my city fidgeting as if waiting for orders, while the globe was spinning. I think that's the one causing the hang, then.

Thanks for all the effort you guys are putting into this, much appreciated!

 

[Thunder_Gr]

iCon ^2 would always be 0 or pos. iCon * 2 can easily be negative, but will always be even.

Far as I know, codeblocks is just not capable of doing a Debug build, ever. Only ones I have heard of being created are all with VS

Oh, yes, You are right! But even so, with iCon<0 you do not get a Division by Zero, right?