DEBUG: The neutral tile improvement pillage bug

[Martin Gühmann]

I located the neutral tile pillage bug in gaiacontroller.cpp or at least I can fix it there:


STDEHANDLER(GaiaController_CutImprovements)
{
sint32 owner;
sint32 type;
MapPoint pos;

if(!args->GetPos(0, pos))
return GEV_HD_Continue;

TerrainImprovement ti;
Cell *cell = g_theWorld->GetCell(pos);
owner = cell->GetOwner();

//Added by Martin Gühmann to make shure that
//g_player[-1] does not happen.
//Not a problem for the array but for the
//GetGaiaController()
if (owner < 0)
return GEV_HD_Continue;

Player *owner_player = g_player[owner];
if (owner_player == NULL ||
owner_player->GetGaiaController() == NULL)
return GEV_HD_Continue;

sint32 num = cell->GetNumDBImprovements();
for (sint16 i = 0; i < num; i++)
{
type = cell->GetDBImprovement(i);

if (GaiaController::sm_endgameImprovements & ((uint64)0x1 << (uint64)type))
{
owner_player->GetGaiaController()->
HandleTerrImprovementChange(type,pos, -1);
}
}

return GEV_HD_Continue;
}


The problem is the GetGaiaController() function. If it is used on a player with index -1 the game crashs. I guess is that there is no player with index -1 and therefore this function tries to access an object that doesn't exist.

In player.cpp I tried this:


GaiaController *Player::GetGaiaController()
{
if(this->m_owner < 0) return NULL; //Added
return m_gaiaController;
}


But without any success maybe the game crashs when the function is called and not when its code is executed. So what do you think about it? I will add it to the altered source files thread once I got your comments or some time passed.

-Martin

[MrBaggins]

Just curious, but is gaiacontroller the place to handle this? how did you come by that information.

I just assumed it would be void World::CutImprovements(const MapPoint &point) in WrlEnv.cpp or something.

[Solver]

Forgive my ignorance, what is the meaning of this bug?

[MrBaggins]

Game crashes if a unit pillages a tile which doesn't have an owner.

[Martin Gühmann]

Originally posted by MrBaggins
Just curious, but is gaiacontroller the place to handle this? how did you come by that information.

I used the debugger of VC++. It directed me to the function in player.cpp I posted above. On the first glance this function looked OK and therefore I decided to find the places where it is called and it is called in the gaiacontroller.cpp in the according handler section. So I added the two lines and it does work now. And I think it shouldn't be a problem as player -1 is non existent and therefore doesn't have a Gaia Controller to check for.

At first I thought there might be a more elegant way to do it but I didn't feel like to bother with it for hours just to come to the same sollution in the end.

-Martin

[Gilgamensch]

Martin,

might sound stupid,

but why not making it slightly different:

If the owner is < 0, change owner to 0 (babarian, or?)

might be easier, or?

[The Big Mc]

there’s no difference in game play or the amount of code you have to Wright :D

[Gilgamensch]

But keeping it in the same area, makes other problem resolution easier;)

[Martin Gühmann]

Originally posted by Gilgamensch
But keeping it in the same area, makes other problem resolution easier;)

I don't know what you mean Gilgamensch, the problems lies here or more precise GetGaiaController function, but I wasn't able to fix it in the GetGaiaController, although I prefer a fix there.

-Martin

[Gilgamensch]

That was also my point, trying to fix in the procedure where it is broken and not somewhere else.

But did you read my other suggestion?

[Martin Gühmann]

I think your other sollution is more complicated, and needs also be done at the place where I did the fix. The version of the GetGaiaController function I posted above should do something along this but it crashes. Possibly the index in that function is nor not -1 but NULL. Maybe I should do anther trial or someone else.

-Martin

[Gilgamensch]

I haven't got any compiler, so sorry if I can't assist you with this.

But if I see it right,

you just return to GEV_HD_continue, but you don't 'change' the owner.

My idea was more to change the owner to like babarian and get away with it, this way.

[Martin Gühmann]

The HandleTerrImprovementChange is used to determine whether a player can lunch the Gaia Controller, player -1 doesn't exists and therefore it can't lunch the GaiaController and therefore it is just some waste of time to execute the code further. The player is invalid and if the player is invalid then the code should stop here.

-Martin

[J Bytheway]

Will this cause any problems if a player has gaia controller tile imps which enter or leave his borders after being constructed?

[Gilgamensch]

*beating the table with my head*

yep, that would be right.

BUT only as a thought, maybe there is some other check afterwards, while it was run through there. So maybe you still might want to try to change the owner there before returning........(hard to tell without having checked everything........)

[Martin Gühmann]

Originally posted by J Bytheway
Will this cause any problems if a player has gaia controller tile imps which enter or leave his borders after being constructed?

Difficuilt question we need to check where the HandleTerrImprovementChange is used and then we can tell.

Gilgamensch changing the owner is I feel something that shouldn't be done the belongs noone thefore the tile improvement shouldn't belong to anybody either, well this I think another bug.

-Martin

[Gilgamensch]

OK, then I see two ways:

If you don't like to change it to babarian (which IIRC is player 0)

then

we would need to remove all TI's if the border changes......


But, I don't see where your problem would be, to change it to babarian? (espacially for pillaging?)

[Martin Gühmann]

Well I tried to move the bugfix into the Player::GetGaiaController but as I disabled all the code I added here, nothing happened no crash.
I just added the fix for the buildingutil.cpp by NelsonAndBronte. So I have no idea why it doesn't crash now.

-Martin

[Gilgamensch]

sounds a little bit like a reversed Murphy's Law.............

But if it doesn't chrash anymore, would be good (just had one of those games.............)