Access exception crash from code

[j_mie6]

can anybody we what is wrong with this code in that it causes a crash, consistently, the third time a plot is accessed (where the function is called on every plot in the game via a for loop on getPlotByIndex()) in which that plot has an improvement other than NO_IMPROVEMENT.

Here's the code:

void CvGame::doWatchtowerManage(CvPlot * pPlot)
{
	int iSearchRadius = GC.getDefineINT("MAXIMUM_EXTENDED_VISIBILITY_RADIUS");
	int iX = pPlot->getX_INLINE();
	int iY = pPlot->getY_INLINE();
	CvPlot * iterPlot;
	ImprovementTypes eImprovement;
	CvImprovementInfo pImprovement;
	int iRadius = 0;

	for (int x = iX - iSearchRadius; x <= iX + iSearchRadius; x++)
	{
		if (x < 0 || x > GC.getMapINLINE().getGridWidthINLINE()-1) continue;
		for (int y = iY + iSearchRadius; y >= iY - iSearchRadius; y--)
		{
			if (y < 0 || y > GC.getMapINLINE().getGridHeightINLINE()-1) continue;
			iterPlot = GC.getMapINLINE().plot(x, y);
			eImprovement = iterPlot->getImprovementType();
			if (eImprovement != NO_IMPROVEMENT)
			{
[B]				pImprovement = GC.getImprovementInfo(eImprovement);
				iRadius = pImprovement.getVisibilityRadius();
				if (iRadius == 0) continue;
				if (plotDistance(pPlot->getX_INLINE(), pPlot->getY_INLINE(), iterPlot->getX_INLINE(), iterPlot->getY_INLINE()) <= iRadius)
				{
					//manage visibility
					TeamTypes eTeam = iterPlot->getTeam();
					TeamTypes ePreviousTeam = iterPlot->getPreviousTeam();
					if (ePreviousTeam == eTeam) continue;
					else
					{
						if (eTeam != NO_TEAM && !pPlot->isVisible(eTeam, false)) { pPlot->changeVisibilityCount(eTeam, 1, NO_INVISIBLE, true); }
						if (ePreviousTeam != NO_TEAM) { pPlot->changeVisibilityCount(ePreviousTeam, -1, NO_INVISIBLE, true); }
					}
				}[/B]
			}
		}
	}
	return;
}

where the getVisibilityRadius is just an extra field in the improvement infos. Bold represents possible area of crash

thanks,
Jamie

 

[God-Emperor]

What happens if a visibility count goes negative? It might be doing that. Nowhere in the code you posted is the "previousTeam" setting for the plot ever changed. If it is not being changed anywhere else, every time you run this it will reduce the visibility count for them by 1, going negative at some point unless the changeVisibilityCount function doesn't let that happen. (I haven't looked at any of this stuff to see what the plot functions used in there do exactly, so...)

It also seems to me that you only need to do this sort of checking for individual plots rather than looping over every plot on the map. It needs to be done when a plot changes owner and has a watchtower on it, a watchtower is built, or a watchtower is destroyed. Also, instead of searching every plot to see if there is a watchtower in range start with plots that have a watchtower on them and spread the effect around the watchtower plot for the specified range. Much less looping over every plot multiple times that way.

 

[j_mie6]

aaaaah yes I forgot to assign the previous owner, but that should be a problem because the crash never reaches any owned tiles, (unless a barb city spawned... in which case nice catch) but I will check it out.

as for the looping, I know it's inefficient but I wanted to get the basics done first before optimising. (tbh I wasn't reallus rue how I should be doing the iteration, but now I know what I should do. where would a good place be to put the code where a plot changes hands, iirc there is no python event for such things and has to be done every turn.

 

[ls612]

What happens if a visibility count goes negative? It might be doing that. Nowhere in the code you posted is the "previousTeam" setting for the plot ever changed. If it is not being changed anywhere else, every time you run this it will reduce the visibility count for them by 1, going negative at some point unless the changeVisibilityCount function doesn't let that happen. (I haven't looked at any of this stuff to see what the plot functions used in there do exactly, so...)

It also seems to me that you only need to do this sort of checking for individual plots rather than looping over every plot on the map. It needs to be done when a plot changes owner and has a watchtower on it, a watchtower is built, or a watchtower is destroyed. Also, instead of searching every plot to see if there is a watchtower in range start with plots that have a watchtower on them and spread the effect around the watchtower plot for the specified range. Much less looping over every plot multiple times that way.

The changeVisibilityCount function has a safety guard built in which should prevent the count from remaining below zero (if it is below zero the function will adjust it to zero).