Array sometimes becomes corrupt

[Red Key]

I'm having a CTD issue after adding some tags and a new array in the SDK. Hopefully, someone out there has had a similar problem and/or knows the solution.

The array is supposed to hold terrain types but it sometimes returns values that are out of range of terrain types. Somehow it becomes corrupt but I haven't find any consistent way to reproduce the bug. Even if I reload a save from right before the bug happened I haven't seen it happen on the same turn again. Also I haven't seen the bug happen except after loading a saved game (at least it didn't occur when I let AIAutoPlay play for a few hundred turns).

More details: Below are some of the tags I am adding to the FFH2 spell schema. They are meant for terraforming spells. <TerrainType> is a terrain that the spell can be used on. <TerrainConversion> is what to change that terrain into. Many terraforming spells change more than one terrain type so this supports listing multiple <TerrainConversion>. I have done a similar thing for features as well.

Spoiler :

	<ElementType name="TerrainType" content="textOnly"/>
	<ElementType name="TerrainConvert" content="textOnly"/>
	<ElementType name="TerrainConversion" content="eltOnly">
		<element type="TerrainType"/>
		<element type="TerrainConvert"/>
	</ElementType>
	<ElementType name="TerrainConversions" content="eltOnly">
		<element type="TerrainConversion" minOccurs="0" maxOccurs="*"/>
	</ElementType>

In CvInfos.cpp here is some of the code I have added. Here are the default/initial values in the constructor and also the stuff I added to the destructor for the arrays:

Spoiler :

m_iCommandType(NO_COMMAND),
// MNAI begin
m_piTerrainConvert(NULL),
m_piFeatureConvert(NULL),
m_pbFeatureInvalid(NULL)
// MNAI end
{
}

//------------------------------------------------------------------------------------------------------
//
//  FUNCTION:   ~CvSpellInfo()
//
//  PURPOSE :   Default destructor
//
//------------------------------------------------------------------------------------------------------
CvSpellInfo::~CvSpellInfo()
{
	// MNAI begin
	SAFE_DELETE_ARRAY(m_piTerrainConvert);
	SAFE_DELETE_ARRAY(m_piFeatureConvert);
	SAFE_DELETE_ARRAY(m_pbFeatureInvalid);
	// MNAI end
}

Here is the new function I added for accessing the terrain convert array:

Spoiler :

int CvSpellInfo::getTerrainConvert(int i) const		
{
	FAssertMsg(i < GC.getNumTerrainInfos(), "Index out of bounds");
	FAssertMsg(i > -1, "Index out of bounds");
	return m_piTerrainConvert ? m_piTerrainConvert[i] : -1;
}

Code from the read() function:

Spoiler :

	stream->Read(&m_iCommandType);
	// MNAI begin
	SAFE_DELETE_ARRAY(m_piTerrainConvert);
	m_piTerrainConvert = new int[GC.getNumTerrainInfos()];
	stream->Read(GC.getNumTerrainInfos(), m_piTerrainConvert);

	SAFE_DELETE_ARRAY(m_piFeatureConvert);
	m_piFeatureConvert = new int[GC.getNumFeatureInfos()];
	stream->Read(GC.getNumFeatureInfos(), m_piFeatureConvert);

	SAFE_DELETE_ARRAY(m_pbFeatureInvalid);
	m_pbFeatureInvalid = new bool[GC.getNumFeatureInfos()];
	stream->Read(GC.getNumFeatureInfos(), m_pbFeatureInvalid);
	// MNAI end
}

Code from the write() function:

Spoiler :

	stream->Write(m_iCommandType);
	// MNAI begin
	stream->Write(GC.getNumTerrainInfos(), m_piTerrainConvert);
	stream->Write(GC.getNumFeatureInfos(), m_piFeatureConvert);
	stream->Write(GC.getNumFeatureInfos(), m_pbFeatureInvalid);
	// MNAI end
}

Code from the other read() function for the XML. I am copying the way TerrainPassableTech and FeaturePassableTech did it:

Spoiler :

pXML->GetChildXmlValByName(m_szSound, "Sound");

	// MNAI begin
	CvString* pszTemp = NULL;
	pXML->SetVariableListTagPair(&pszTemp, "TerrainConversions", sizeof(GC.getTerrainInfo((TerrainTypes)0)), GC.getNumTerrainInfos());
	m_piTerrainConvert = new int[GC.getNumTerrainInfos()];
	for (int i = 0; i < GC.getNumTerrainInfos(); ++i)
	{
		m_piTerrainConvert[i] = pszTemp[i].IsEmpty() ? NO_TERRAIN : pXML->FindInInfoClass(pszTemp[i]);
	}
	SAFE_DELETE_ARRAY(pszTemp);

	pXML->SetVariableListTagPair(&pszTemp, "FeatureConversions", sizeof(GC.getFeatureInfo((FeatureTypes)0)), GC.getNumFeatureInfos());
	m_piFeatureConvert = new int[GC.getNumFeatureInfos()];
	for (int i = 0; i < GC.getNumFeatureInfos(); ++i)
	{
		m_piFeatureConvert[i] = pszTemp[i].IsEmpty() ? i : pXML->FindInInfoClass(pszTemp[i], true);
	}
	SAFE_DELETE_ARRAY(pszTemp);

	pXML->SetVariableListTagPair(&m_pbFeatureInvalid, "FeatureInvalids", sizeof(GC.getFeatureInfo((FeatureTypes)0)), GC.getNumFeatureInfos());
	// MNAI end

	return true;
}

Here is the code where the bug occurs. The setTerrainType() function is sometimes given values that are out of range of all terrain types by getTerrainConvert(). I have never seen getFeatureConvert() give setFeatureType() invalid values even though it comes first, so the problem seems to only be with the terrain conversions.

Spoiler :

void CvUnit::castTerraform(int spell)
{
	CvSpellInfo& kSpellInfo = GC.getSpellInfo((SpellTypes) spell);
	CvPlot* pPlot = plot();
	int iRange = kSpellInfo.getRange();

	for (int iDX = -iRange; iDX <= iRange; ++iDX)
	{
		for (int iDY = -iRange; iDY <= iRange; ++iDY)
		{
			CvPlot* pLoopPlot = plotXY(pPlot->getX_INLINE(), pPlot->getY_INLINE(), iDX, iDY);
			if (pLoopPlot != NULL)
			{
				FeatureTypes eFeature = pLoopPlot->getFeatureType();
				if (eFeature != NO_FEATURE)
				{
					if (kSpellInfo.isFeatureInvalid(eFeature))
					{
						continue;
					}
					if (kSpellInfo.getFeatureConvert(eFeature) != eFeature)
					{
						pLoopPlot->setFeatureType((FeatureTypes) kSpellInfo.getFeatureConvert(eFeature));
					}
				}
				if (kSpellInfo.getTerrainConvert(pLoopPlot->getTerrainType()) != NO_TERRAIN)
				{
					[U][B]pLoopPlot->setTerrainType((TerrainTypes) kSpellInfo.getTerrainConvert(pLoopPlot->getTerrainType()));[/B][/U]
					if (kSpellInfo.isRemoveInvalidFeature() && pLoopPlot->getFeatureType() != NO_FEATURE)
					{
						if (!GC.getFeatureInfo(pLoopPlot->getFeatureType()).isTerrain(pLoopPlot->getTerrainType()))
						{
							pLoopPlot->setFeatureType(NO_FEATURE);
						}
					}
				}
			}
		}
	}