Python to SDK Data iteration conversion

OrionVeteran

Deity
Joined
Dec 25, 2003
Messages
2,443
Location
Newport News VA
I have a very elegant Python function which returns a numerical rating for a plot. The "for" commands use Python data iteration tables to evauate and rate the plot. It works perfectly in Python. However, I need to make a call to this function from SDK. I'd prefer to avoid making a Python call from the SDK. Instead, I'd like to convert the Python function to SDK, where it can process very fast. Problem is that I don't know how to convert the Python data iteration over to SDK and need an example of how to do it. Here is the Python function that I need to convert:


Code:
[SPOILER]
dTerrainData = {
		"TERRAIN_HILL": 3,
		"TERRAIN_GRASS": 3,
		"TERRAIN_PLAINS": 3,
		"TERRAIN_DESERT": 2,
		"TERRAIN_TUNDRA": 2,
		"TERRAIN_COAST": 4,
		"TERRAIN_SNOW": 1,
		"TERRAIN_OCEAN": 1
	}
	
	
dFeatureData = {
		"FEATURE_FOREST": 3,
		"FEATURE_JUNGLE": 2,
		"FEATURE_OASIS": 3,
		"FEATURE_FLOOD_PLAINS": 2,
		"FEATURE_FALLOUT": 1
	}
	
	
dImprovementData = {
		"IMPROVEMENT_FARM": 4,
		"IMPROVEMENT_MINE": 4,
		"IMPROVEMENT_ROAD": 2,
		"IMPROVEMENT_RAILROAD": 3,
		"IMPROVEMENT_GOODY_HUT": 3,
		"IMPROVEMENT_FISHING_BOATS": 4,
		"IMPROVEMENT_WHALING_BOATS": 3,
		"IMPROVEMENT_WELL": 4,
		"IMPROVEMENT_OFFSHORE_PLATFORM": 4,
	}



def getPlotRating(pPlot):
	# Orion's Minewarefare Mod
	# Purpose: Rate the plot
	# Returns a numerical rating for the plot 
	iPlotRating = 0
	ePlotType = pPlot.getPlotType()
	eTerrainType = pPlot.getTerrainType()
	eFeatureType = pPlot.getFeatureType()
	eImprovementType = pPlot.getImprovementType()
		
	# Terrain
	for szTerrainType, iTerrainValue in dTerrainData.iteritems():
		iTerrainType = gc.getInfoTypeForString(str(szTerrainType))
		if iTerrainType == eTerrainType:
			iPlotRating += iTerrainValue
	
	# Features
	for szFeatureType, iFeatureValue in dFeatureData.iteritems():
		iFeatureType = gc.getInfoTypeForString(str(szFeatureType))
		if iFeatureType == eFeatureType:
			iPlotRating += iFeatureValue
	
	# Improvements
	for szImprovementType, iImprovementValue in dImprovementData.iteritems():
		iImprovementType = gc.getInfoTypeForString(str(szImprovementType))
		if iImprovementType == eImprovementType:
			iPlotRating += iImprovementValue
			
	# Food
	for iFoodYield in range(1, 7):
		if pPlot.getYield(YieldTypes.YIELD_FOOD) == iFoodYield:
			iPlotRating += iFoodYield
			break
	
	# Production
	for iProductionYield in range(1, 7):
		if pPlot.getYield(YieldTypes.YIELD_PRODUCTION) == iProductionYield:
			iPlotRating += iProductionYield
			break
	
	# COMMERCE
	for iCommerceYield in range(1, 7):
		if pPlot.getYield(YieldTypes.YIELD_PRODUCTION) == iCommerceYield:
			iPlotRating += iCommerceYield
			break
	
	if hasBonusOnPlot(pPlot):
		iPlotRating += 4	
	if pPlot.isFreshWater():
		iPlotRating += 2
		
	return iPlotRating
[/SPOILER]

Any help would be greatly appreciated.
 
Let's start with the Array.

Is this how it's done?

Code:
UnitTypeWeightArray iTerrainTypes;

iTerrainTypes.push_back(std::make_pair(TERRAIN_HILL, 3));
iTerrainTypes.push_back(std::make_pair(TERRAIN_GRASS, 3));
iTerrainTypes.push_back(std::make_pair(TERRAIN_PLAINS, 3));
iTerrainTypes.push_back(std::make_pair(TERRAIN_DESERT, 2));
iTerrainTypes.push_back(std::make_pair(TERRAIN_TUNDRA, 2));
iTerrainTypes.push_back(std::make_pair(TERRAIN_COAST, 4));
iTerrainTypes.push_back(std::make_pair(TERRAIN_SNOW, 1));
iTerrainTypes.push_back(std::make_pair(TERRAIN_OCEAN, 1));
iTerrainTypes.push_back(std::make_pair(TERRAIN_PEAK, 0));
 
You could probably do it that way, but it would be easier to just make an array of ints that has a number of elements equal to the number of terrain types, particularly if you are assigning a value to most or all of them. The index into the array is the terrain type, the value of that array element is the rating. Then you can just have a series of "aiTerrainValues[TERRAIN_foo] = x;"
to initialize it in a way that is clear (assuming all the TERRAIN_foo values are defined somewhere - they are not defined anywhere in the regular parts of the DLL, except NO_TERRAIN, since they are defined in the XML so the DLL doesn't know what they are ahead of time).

By the way...

While I don't want to rain on your parade (and keeping in mind that any code that produces the result it is supposed to is vastly superior to code that does not), your "very elegant Python function" is quite inefficient. This:
Code:
	# Food
	for iFoodYield in range(1, 7):
		if pPlot.getYield(YieldTypes.YIELD_FOOD) == iFoodYield:
			iPlotRating += iFoodYield
			break
is pretty much identical in what it does to this much shorter and quicker piece of code:
Code:
	iPlotRating += pPlot.getYield(YieldTypes.YIELD_FOOD)
assuming no plot ever gives more than 7 food. If one does, then it is adding 0 in your loop method instead of the amount of food it produces. A negative number for a plot yield should be impossible, I think, so that end of the range is not an issue. Instead of looping over integers to find out what number it is then adding that number, just add that number since you knew what it was all along.

Ditto for the other loops after that.

For the 3 loops before that, they could at the very least use a "break" statement since no plot has more than one of any of those things. There are other things that could be done to make all that faster too, via direct lookups using the value you are interested in instead of loops to find a match.

That may not seem like it is helpful for porting to C++, but it is. Since the Python does these things, you'd probably have the C++ do it the same way.
 
You could probably do it that way, but it would be easier to just make an array of ints that has a number of elements equal to the number of terrain types, particularly if you are assigning a value to most or all of them. The index into the array is the terrain type, the value of that array element is the rating. Then you can just have a series of "aiTerrainValues[TERRAIN_foo] = x;"
to initialize it in a way that is clear (assuming all the TERRAIN_foo values are defined somewhere - they are not defined anywhere in the regular parts of the DLL, except NO_TERRAIN, since they are defined in the XML so the DLL doesn't know what they are ahead of time).

By the way...

While I don't want to rain on your parade (and keeping in mind that any code that produces the result it is supposed to is vastly superior to code that does not), your "very elegant Python function" is quite inefficient. This:
Code:
	# Food
	for iFoodYield in range(1, 7):
		if pPlot.getYield(YieldTypes.YIELD_FOOD) == iFoodYield:
			iPlotRating += iFoodYield
			break
is pretty much identical in what it does to this much shorter and quicker piece of code:
Code:
	iPlotRating += pPlot.getYield(YieldTypes.YIELD_FOOD)
assuming no plot ever gives more than 7 food. If one does, then it is adding 0 in your loop method instead of the amount of food it produces. A negative number for a plot yield should be impossible, I think, so that end of the range is not an issue. Instead of looping over integers to find out what number it is then adding that number, just add that number since you knew what it was all along.

Ditto for the other loops after that.

For the 3 loops before that, they could at the very least use a "break" statement since no plot has more than one of any of those things. There are other things that could be done to make all that faster too, via direct lookups using the value you are interested in instead of loops to find a match.

That may not seem like it is helpful for porting to C++, but it is. Since the Python does these things, you'd probably have the C++ do it the same way.

Here is what I have so far: My concern is the Array. I'm sure it is not quite right.

Spoiler :

Code:
int CvPlot::getPlotRating() const
{
	int iI; 
	int iPlotRating = 0;
	int aiTerrainValues[TERRAIN_GRASS] = 3;
	int aiTerrainValues[TERRAIN_PLAINS] = 3;
	int aiTerrainValues[TERRAIN_DESERT] = 2;
	int aiTerrainValues[TERRAIN_TUNDRA] = 2;
	int aiTerrainValues[TERRAIN_SNOW] = 1;
	int aiTerrainValues[TERRAIN_COAST] = 4;
	int aiTerrainValues[TERRAIN_OCEAN] = 1;
	int aiTerrainValues[TERRAIN_PEAK] = 0;
	int aiTerrainValues[TERRAIN_HILL] = 3;
	
	//Food
	iPlotRating += getYield(YieldTypes.YIELD_FOOD);
	//Production
	iPlotRating += getYield(YieldTypes.YIELD_PRODUCTION);
	//COMMERCE
	iPlotRating += getYield(YieldTypes.YIELD_COMMERCE);

	for (iI = 0; iI < GC.getNumTerrainInfos(); iI++)
	{
		if (getTerrainType() != NO_TERRAIN)
		{
			iPlotRating += aiTerrainValues[iI];
			break;
		}
	}
	
	return iplotRating;
}
 
The array is definitely not correct like that.
But tbh it is not a good solution to implement it like that in C++ anyway.

I would suggest adding a new field to the terrain, feature and improvement XML that contains the rating and then you can retrieve it in this function and just add it.
The advantage is that it works if a mod adds more terrains, features or improvements.

If you insist on having that information in the code itself, best use a switch statement.
 
The array is definitely not correct like that.
But tbh it is not a good solution to implement it like that in C++ anyway.

I would suggest adding a new field to the terrain, feature and improvement XML that contains the rating and then you can retrieve it in this function and just add it.
The advantage is that it works if a mod adds more terrains, features or improvements.

If you insist on having that information in the code itself, best use a switch statement.

:hmm: Hmmm... I'm compelled to agree about your stated advantage of adding new XML tags for terrain, feature and improvement. The new tags would indeed be more elegant. Always one more thing...

:thanx:
 
I agree that new tags are better than trying to hardcode arrays in the DLL.

Anything that looks like a property for something should be done as a property of that thing. It means you don't have to rebuild the DLL any time you want to add or remove one of those things, or just change the value for one of them.
 
I agree that new tags are better than trying to hardcode arrays in the DLL.

Anything that looks like a property for something should be done as a property of that thing. It means you don't have to rebuild the DLL any time you want to add or remove one of those things, or just change the value for one of them.

I am currently working on the new XML tags now. When finished, I will post the new SDK function that utilizes the new tags and confirm that it actually works. As always, I appreciate the excellent advice from you and AIAndy.

:thanx:
 
I have finally finished the new tags and made the required changes to the function. It works! :dance:

Here is the the end result:

Spoiler :

Code:
int CvPlot::getPlotRating() const
{
	int iPlotRating = 0;
	
	if (getTerrainType() != NO_TERRAIN)
	{
		//Plot Terrain Rating
		iPlotRating += GC.getTerrainInfo(getTerrainType()).getTerrainRating();
		//Food
		iPlotRating += GC.getTerrainInfo(getTerrainType()).getYield((YieldTypes)YIELD_FOOD);
		//Production
		iPlotRating += GC.getTerrainInfo(getTerrainType()).getYield((YieldTypes)YIELD_PRODUCTION); 
		//COMMERCE
		iPlotRating += GC.getTerrainInfo(getTerrainType()).getYield((YieldTypes)YIELD_COMMERCE);
	}
	
	if (getFeatureType() != NO_FEATURE)
	{
		//Plot Feature Rating
		iPlotRating += GC.getFeatureInfo((FeatureTypes)getFeatureType()).getFeatureRating();
	}
	
	if (getImprovementType() != NO_IMPROVEMENT)
	{
		//Plot Improvement Rating
		iPlotRating += GC.getImprovementInfo((ImprovementTypes)getImprovementType()).getImprovementRating();
	}
	
	if (isFreshWater())
	{
		iPlotRating += 2;
	}
	
	if (getBonusType() != NO_BONUS)
	{
		iPlotRating += 4;
	}
	
	return iPlotRating;
}
 
Top Bottom