STRUCTURES Forbidden Terrain fix

[n0b0dy007]

While experimenting with changes to the STRUCTURES data (to add variety to the monotonous region builds), noticed the game was not picking up changes to ForbiddenTerrainCats[] entries.
After some digging, found and fixed a defect in StructureTemplate.bsf/StructureTpl_ListforbiddenTerrainCats(). Revised code below:

Code: Select all

FUNCTION StructureTpl_ListForbiddenTerrainCats(strucTplID)
{	
	int handle;	// dynamic array reference
	int count;	// array length / number of items
	int idx;	// loop iteration index

	// {pre: global contains valid DA handle}
	if (gHStructuresTplForbiddenTerrainCats[strucTplID] == 0)	// Create the DA for further use
	{
		gHStructuresTplForbiddenTerrainCats[strucTplID] = CreateDynamicArray();
	}
	handle = gHStructuresTplForbiddenTerrainCats[strucTplID];

	// {invariant: DA content duplicates loaded StructureTemplateArray}
	count = GetStructureTemplateArrayLength(strucTplID, $STRUCTURE_FORBIDDENTERRAINCATS);
	if (count != GetDynamicArrayLength(handle))	// size mismatch implies DA has stale data
	{
		ClearDynamicArray(handle);
		ResizeDynamicArray(handle, count);
		for (idx = 0; idx < count; idx++)
		{
			SetDynamicArray(handle, idx, GetStructureTemplateInt(strucTplID, $STRUCTURE_FORBIDDENTERRAINCATS, idx));	
		}
		// assume no need to update DA further ...
	}
	{post: global contains valid handle to DA containing duplicate of data loaded to StructureTemplateArray}
	return handle;
}

Also corrected some inefficiency in how the function is called, from Faction.bsf/Faction_Construct_StructureIsAllowed_Internal(), putting the less-expensive tests earlier in series.
The relevant portion is:

Code: Select all

	// Terrain tests -
	// Must be Adjacent to a river?
	value = StructureTpl_MustBeAdjRiver(strucTplID);
	if ((value  > 0) && (Region_IsAdjacentToRiver_Strait(regionID, TRUE, FALSE) == FALSE))
	{
		gStructureConstructErrorCode = STRUCTURE_CONSTRUCT_ERROR_WRONG_TERRAIN;
		return FALSE;
	}
	// Required Area
	value = StructureTpl_RequiredArea(strucTplID);
	if ((value > 0) && (Region_GetGeoArea(regionID) != value))	// Area_HasRegion(value, regionID) == FALSE
	{
		gStructureConstructErrorCode = STRUCTURE_CONSTRUCT_ERROR_WRONG_AREA;
		return FALSE;
	}
	// Required terrain
	value = StructureTpl_RequiredTerrainCat(strucTplID);
	if ((value > 0) && (Region_GetTerrain(regionID) != value))
	{
		gStructureConstructErrorCode = STRUCTURE_CONSTRUCT_ERROR_WRONG_TERRAIN;
		return FALSE;
	}
	// Forbidden terrain - More expensive test, multiple forbidden terrain cats are possible
	daHandle = StructureTpl_ListForbiddenTerrainCats(strucTplID);		// @TODO: this really wants to be a bitset
	count = GetDynamicArrayLength(daHandle);
	for (idx = 0; idx < count; idx++)
	{
		value = GetDynamicArray(daHandle, idx);	// value is one of TERRAIN_CAT_ values
		if ((value  > 0) && (Terrain_IsThisTerrainCat(terrainID, value, Region_FetchWeather(regionID)) == TRUE))
		{
			gStructureConstructErrorCode = STRUCTURE_CONSTRUCT_ERROR_WRONG_TERRAIN;
			return FALSE;
		}
	}

[Pocus]

I'll take a look at the base code, thanks.

[n0b0dy007]

The code in Faction.bsf/Faction_Construct_StructureIsAllowed_Internal() is kinda crufty, and I see the notes to remind yourself to clean it up.
A couple of other checks to include:

• if Structure_IsBeingDisassembled() for required and/or precursor/upgrade structures

• after the (successful? checked?) call to CreateDynamicArray(), all function exit paths should ensure a corresponding call to DestroyDynamicArray()

[Pocus]

Usually not enough time to make the perfect code. Also revising an existing code demands re-testing, as even a simple change can introduce a bug, from experience. These time, with too few testers, the risks are bigger than the rewards.

[Surt]

The code in Faction.bsf/Faction_Construct_StructureIsAllowed_Internal() is kinda crufty, and I see the notes to remind yourself to clean it up.
A couple of other checks to include:

• if Structure_IsBeingDisassembled() for required and/or precursor/upgrade structures

• after the (successful? checked?) call to CreateDynamicArray(), all function exit paths should ensure a corresponding call to DestroyDynamicArray()

Consider making a mini-mod to test it out?