General DLL programming questions

AIAndy said:
Poor Koshling, always optimizing other people's inefficient code.

It would be better to think about how to do stuff in an efficient way before implementing, as afterwards it might not be that easy to implement some of the optimizations.
If that information about if there is orbital infrastructure is needed often, then it is better to store it on the city and player object similar to how it is done with some other information. Because going through all cities and then checking all building types for each is quite costly. If that is then used in an AI evaluation that is run for all buildings it can build, then there is a significant cost.
Click to expand...

Of course it would be better! But for me, I do the best I KNOW how to do and it may often take being shown a better way and how to apply it once I've already made my best effort. This comes as a result of learning C++ only as far as it has been so far necessary to mod. Coming at this from a modder angle, without previous programming experience, leads us into less optimal program designs, yes, but thank god for you guys who can guide us into better structures because for the modder, rather than the programmer, if it WORKS is the main thing. Not having ANY previous C++ programming knowledge or experience is going to mean we've got a pretty long learning curve to get to the kinds of understandings you and Koshling have.

And it doesn't help that most of the time I ask a question I get an answer that I find so thoroughly baffling that I say to myself "shouldn't have bothered asking... I should've known I wouldn't understand the answer."

I don't blame that on y'all though... I know how it is to understand something of vast complexity to the point that it becomes next to impossible to explain even a part of it to someone who understand only 5% of what I do - how can you know what I would grasp and what I wouldn't? To be honest, I'm just glad you guys aren't angry with us for being crap programmers messing with your code.
 
Koshling said:
Maybe we just bottle it up well ;)
Click to expand...

Well, I hope you don't blow up at me for asking for some more feedback. :p I've done the following things to implement the actual orbital commerces.

1. I've changed UpdateBuildingCommerces() to look like this.

PHP: 
void CvCity::updateBuildingCommerce()
{
	int iNewBuildingCommerce;
	int iI, iJ;

	for (iI = 0; iI < NUM_COMMERCE_TYPES; iI++)
	{
		iNewBuildingCommerce = 0;

		for (iJ = 0; iJ < GC.getNumBuildingInfos(); iJ++)
		{
			//ls612: Support for Orbital buildings
			if (GC.getBuildingInfo((BuildingTypes)iJ).isOrbital())
			{
				iNewBuildingCommerce += getBuildingCommerceByBuilding(((CommerceTypes)iI), ((BuildingTypes)iJ));;
			}
			else
			{
				iNewBuildingCommerce += getOrbitalBuildingCommerceByBuilding(((CommerceTypes)iI), ((BuildingTypes)iJ));
			}
		}

		if (getBuildingCommerce((CommerceTypes)iI) != iNewBuildingCommerce)
		{
			m_aiBuildingCommerce[iI] = iNewBuildingCommerce;

			setCommerceDirty((CommerceTypes)iI);
		}
	}
}

Thus bypassing the need for another array, as a building will never be orbital and non-orbital at the same time. The getOrbitalBuildingCommerceByBuilding method looks like this.

PHP: 
int CvCity::getOrbitalBuildingCommerceByBuilding(CommerceTypes eIndex, BuildingTypes eBuilding) const
{
	//ls612: Orbital Buildings have their commerce handled in a special manner
	int iCommerce = 0;
	int iNumOrbital = GET_PLAYER(getOwnerINLINE()).countNumCitiesWithOrbitalInfrastructure();
	

	FAssertMsg(eIndex >= 0, "eIndex expected to be >= 0");
	FAssertMsg(eIndex < NUM_COMMERCE_TYPES, "eIndex expected to be < NUM_COMMERCE_TYPES");
	FAssertMsg(eBuilding >= 0, "eBuilding expected to be >= 0");
	FAssertMsg(eBuilding < GC.getNumBuildingInfos(), "GC.getNumBuildingInfos expected to be >= 0");


	if (getNumBuilding(eBuilding) > 0)
	{
		CvBuildingInfo& kBuilding = GC.getBuildingInfo(eBuilding);	
		int iBaseCommerceChange = GC.getBuildingInfo(eBuilding).getCommerceChange(eIndex);

		if ( iBaseCommerceChange < 0 && eIndex == COMMERCE_GOLD && GC.getDefineINT("TREAT_NEGATIVE_GOLD_AS_MAINTENANCE") )
		{
			iBaseCommerceChange = 0;
		}
		
		if (iBaseCommerceChange != 0)
		{
			if (hasOrbitalInfrastructure())
			{
				iCommerce += std::min((iBaseCommerceChange * iNumOrbital), getPopulation());
			}

			if (!hasOrbitalInfrastructure())
			{
				iCommerce += std::min((iBaseCommerceChange * iNumOrbital), getPopulation());
				if (iCommerce != 0)
				{
					iCommerce /= 2;
				}
			}
		}

		return iCommerce;
	}

	return 0;
}

Is this good? If it is, I'll push it to the SVN now, otherwise I can make any improvements you suggest. I apoligize for wasting your time with my projects though, I won't do so in the future if you don't want.
 
ls612 said:
Well, I hope you don't blow up at me for asking for some more feedback. :p I've done the following things to implement the actual orbital commerces.

1. I've changed UpdateBuildingCommerces() to look like this.

PHP: 
void CvCity::updateBuildingCommerce()
{
	int iNewBuildingCommerce;
	int iI, iJ;

	for (iI = 0; iI < NUM_COMMERCE_TYPES; iI++)
	{
		iNewBuildingCommerce = 0;

		for (iJ = 0; iJ < GC.getNumBuildingInfos(); iJ++)
		{
			//ls612: Support for Orbital buildings
			if (GC.getBuildingInfo((BuildingTypes)iJ).isOrbital())
			{
				iNewBuildingCommerce += getBuildingCommerceByBuilding(((CommerceTypes)iI), ((BuildingTypes)iJ));;
			}
			else
			{
				iNewBuildingCommerce += getOrbitalBuildingCommerceByBuilding(((CommerceTypes)iI), ((BuildingTypes)iJ));
			}
		}

		if (getBuildingCommerce((CommerceTypes)iI) != iNewBuildingCommerce)
		{
			m_aiBuildingCommerce[iI] = iNewBuildingCommerce;

			setCommerceDirty((CommerceTypes)iI);
		}
	}
}

Thus bypassing the need for another array, as a building will never be orbital and non-orbital at the same time. The getOrbitalBuildingCommerceByBuilding method looks like this.

PHP: 
int CvCity::getOrbitalBuildingCommerceByBuilding(CommerceTypes eIndex, BuildingTypes eBuilding) const
{
	//ls612: Orbital Buildings have their commerce handled in a special manner
	int iCommerce = 0;
	int iNumOrbital = GET_PLAYER(getOwnerINLINE()).countNumCitiesWithOrbitalInfrastructure();
	

	FAssertMsg(eIndex >= 0, "eIndex expected to be >= 0");
	FAssertMsg(eIndex < NUM_COMMERCE_TYPES, "eIndex expected to be < NUM_COMMERCE_TYPES");
	FAssertMsg(eBuilding >= 0, "eBuilding expected to be >= 0");
	FAssertMsg(eBuilding < GC.getNumBuildingInfos(), "GC.getNumBuildingInfos expected to be >= 0");


	if (getNumBuilding(eBuilding) > 0)
	{
		CvBuildingInfo& kBuilding = GC.getBuildingInfo(eBuilding);	
		int iBaseCommerceChange = GC.getBuildingInfo(eBuilding).getCommerceChange(eIndex);

		if ( iBaseCommerceChange < 0 && eIndex == COMMERCE_GOLD && GC.getDefineINT("TREAT_NEGATIVE_GOLD_AS_MAINTENANCE") )
		{
			iBaseCommerceChange = 0;
		}
		
		if (iBaseCommerceChange != 0)
		{
			if (hasOrbitalInfrastructure())
			{
				iCommerce += std::min((iBaseCommerceChange * iNumOrbital), getPopulation());
			}

			if (!hasOrbitalInfrastructure())
			{
				iCommerce += std::min((iBaseCommerceChange * iNumOrbital), getPopulation());
				if (iCommerce != 0)
				{
					iCommerce /= 2;
				}
			}
		}

		return iCommerce;
	}

	return 0;
}

Is this good? If it is, I'll push it to the SVN now, otherwise I can make any improvements you suggest. I apoligize for wasting your time with my projects though, I won't do so in the future if you don't want.
Click to expand...

One error on first glance:

You need to use getNumActiveBuilding() not getNumBuilding() or you'll be counting obsoleted buildings and those deactivated due to lack of vicinity bonuses/religions/civics etc.
 
OK, everything seems to be working alright with my new tags, so all that I need to do is add game text for them. How would I do that? Looking in the CvGameTextMgr file I can't figure head from tail, but my question is should I add a routine to an existing function or make a new function for it?

Also, I know that Thunderbrd broke the SVN with his changes, so I won't be committing this until later.
 
ls612 said:
OK, everything seems to be working alright with my new tags, so all that I need to do is add game text for them. How would I do that? Looking in the CvGameTextMgr file I can't figure head from tail, but my question is should I add a routine to an existing function or make a new function for it?

Also, I know that Thunderbrd broke the SVN with his changes, so I won't be committing this until later.
Click to expand...

I fixed it. You might want to take note of my error, ls, for your own benefit:
if (getCombatStuns() == 0 && iAttackerCombatRoll < iAttackerHitChance)
Click to expand...
The bit in red was ALL I forgot. I don't know how it managed to compile like that but be forewarned of this simple mistake.

Anyhow, as for CvGameTextMgr, you wouldn't add a new function but rather a new routine in an existing function: setBuildingHelpActual

You'll find there this example:
Code: 
	if (kBuilding.isAutoBuild())
	{
		szBuffer.append(NEWLINE);
		szBuffer.append(gDLL->getText("TXT_KEY_BUILDING_AUTO_BUILD"));
	}

I'd duplicate this example in particular as I think its a good one. You can find examples with integers as well but this is a good one for a boolean. Any questions?
 
Thunderbrd said:
I fixed it. You might want to take note of my error, ls, for your own benefit:
The bit in red was ALL I forgot. I don't know how it managed to compile like that but be forewarned of this simple mistake.

Anyhow, as for CvGameTextMgr, you wouldn't add a new function but rather a new routine in an existing function: setBuildingHelpActual

You'll find there this example:
Code: 
	if (kBuilding.isAutoBuild())
	{
		szBuffer.append(NEWLINE);
		szBuffer.append(gDLL->getText("TXT_KEY_BUILDING_AUTO_BUILD"));
	}

I'd duplicate this example in particular as I think its a good one. You can find examples with integers as well but this is a good one for a boolean. Any questions?
Click to expand...

Functions are legitimate variables (or type pointer-to-function) and since NULL (the pointer) is just the value 0, the compiler was treating that as check that the pointer to the function was null. That's a legitimate (but pointless really for a method) test.

This is why (most) modern languages are much more strongly typed than C++. You need to bear in mind we're using a 25 year old language to develop in.
 
Koshling said:
Functions are legitimate variables (or type pointer-to-function) and since NULL (the pointer) is just the value 0, the compiler was treating that as check that the pointer to the function was null. That's a legitimate (but pointless really for a method) test.

This is why (most) modern languages are much more strongly typed than C++. You need to bear in mind we're using a 25 year old language to develop in.
Click to expand...

And that language is still the most-used language for compiled programs, so they must have done something right. Anyhow, thanks for the help with that, it should be hitting the SVN today.
 
ls612 said:
Well, I hope you don't blow up at me for asking for some more feedback. :p I've done the following things to implement the actual orbital commerces.

1. I've changed UpdateBuildingCommerces() to look like this.

PHP: 
void CvCity::updateBuildingCommerce()
{
	int iNewBuildingCommerce;
	int iI, iJ;

	for (iI = 0; iI < NUM_COMMERCE_TYPES; iI++)
	{
		iNewBuildingCommerce = 0;

		for (iJ = 0; iJ < GC.getNumBuildingInfos(); iJ++)
		{
			//ls612: Support for Orbital buildings
			if (GC.getBuildingInfo((BuildingTypes)iJ).isOrbital())
			{
				iNewBuildingCommerce += getBuildingCommerceByBuilding(((CommerceTypes)iI), ((BuildingTypes)iJ));;
			}
			else
			{
				iNewBuildingCommerce += getOrbitalBuildingCommerceByBuilding(((CommerceTypes)iI), ((BuildingTypes)iJ));
			}
		}

		if (getBuildingCommerce((CommerceTypes)iI) != iNewBuildingCommerce)
		{
			m_aiBuildingCommerce[iI] = iNewBuildingCommerce;

			setCommerceDirty((CommerceTypes)iI);
		}
	}
}
Click to expand...

I didn't really pay attention to your explanation of what you are doing, but isn't the "if" inside the 2nd "for" backwards? It is saying "if the building is orbital then use the non-orbital getBuildingCommerceByBuilding, otherwise (i.e. when it is not orbital) use the orbital version: getOrbitalBuildingCommerceByBuilding. Shouldn't the orbital buildings use the orbital version of the function, and the non-orbital buildings use the non-orbital version of the function?

Koshling said:
This is why (most) modern languages are much more strongly typed than C++. You need to bear in mind we're using a 25 year old language to develop in.
Click to expand...

Most older languages too. Almost all of them, like Fortran for example (it's a pity more software isn't written in Fortran, it would have fewer bugs; on the other hand C++ has the highest bug rate of any language in common use so using almost anything else would result in fewer bugs). The more "modern" languages (most stuff since sometime in the 80s, really) tend to be weakly typed. Like Python - you can use any variable to store any data of any type and change it at whim just by assigning it a value of a different type.
 
God-Emperor said:
I didn't really pay attention to your explanation of what you are doing, but isn't the "if" inside the 2nd "for" backwards? It is saying "if the building is orbital then use the non-orbital getBuildingCommerceByBuilding, otherwise (i.e. when it is not orbital) use the orbital version: getOrbitalBuildingCommerceByBuilding. Shouldn't the orbital buildings use the orbital version of the function, and the non-orbital buildings use the non-orbital version of the function?
Click to expand...

Duh. :hammer2: Thanks for seeing that.
 
God-Emperor said:
Most older languages too. Almost all of them, like Fortran for example (it's a pity more software isn't written in Fortran, it would have fewer bugs; on the other hand C++ has the highest bug rate of any language in common use so using almost anything else would result in fewer bugs). The more "modern" languages (most stuff since sometime in the 80s, really) tend to be weakly typed. Like Python - you can use any variable to store any data of any type and change it at whim just by assigning it a value of a different type.
Click to expand...

That's rather an over-generalization. There was a spurt in weakly types languages in the 90s, but recently things have gone back to strong typing but with object oriented polymorphism, and generics (Java, C#). C# would be my language of choice these days, but partly that's familiarity, but also that it's way harder to do stupid things in it like you can (in different ways) in both C++ (mostly screwing up memory management) and Python (bad indenting, inappropriate use of types, plain old syntax errors that it won't notice until the statement in question actually needs to be executed)
 
@God-Emperor,

Would you please :please: ( with sugar on top) consider becoming part of C2C's Team?

I feel you would be a natural fit. Even part time help would be great. (Not trying to over step SO authority but I think he would agree).

JosEPh :)
 
ls612 said:
What function calculates the displayed score in the Scoreboard? Or is that done in python?
Click to expand...
I've been wondering that myself actually... I think the calculated score itself is probably done in the dll, then called and displayed via python, but I don't know what file... CvPlayer perhaps?


Ok, in the SVN thread I alluded to having some questions regarding the read/write wrappers in CvUnit. I need to know if the conversion from this:
Spoiler :
Code: 
	//	Backward compatibility - read array format if present
	for(iI = 0; iI < GC.getNumPromotionInfos(); iI++)
	{
		g_paiTempAfflictOnAttackCount[iI] = 0;
		g_paiTempCureAfflictionCount[iI] = 0;
		g_paiTempAfflictionTurnCount[iI] = 0;
		g_paiTempAfflictionHitCount[iI] = 0;
		g_paiTempAfflictionTolerance[iI] = 0;
		g_paiTempFortitudeModifierTypeAmount[iI] = 0;
	}
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONS, GC.getNumPromotionInfos(), g_paiTempAfflictOnAttackCount, "m_paiAfflictOnAttackCount");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONS, GC.getNumPromotionInfos(), g_paiTempCureAfflictionCount, "m_paiCureAfflictionCount");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONS, GC.getNumPromotionInfos(), g_paiTempAfflictionTurnCount, "m_paiAfflictionTurnCount");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONS, GC.getNumPromotionInfos(), g_paiTempAfflictionHitCount, "m_paiAfflictionHitCount");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONS, GC.getNumPromotionInfos(), g_paiTempAfflictionTolerance, "m_paiAfflictionTolerance");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONS, GC.getNumPromotionInfos(), g_paiTempFortitudeModifierTypeAmount, "m_paiFortitudeModifierTypeAmount");

	//	Read new format compressed data if present
	do
	{
		iI= -1;
		WRAPPER_READ_DECORATED(wrapper, "CvUnit", &iI, "hasAfflicationInfo");
		if ( iI != -1 )
		{
			int iNewIndex = wrapper.getNewClassEnumValue(REMAPPED_CLASS_TYPE_PROMOTIONS, iI, true);

			if ( iNewIndex != NO_PROMOTION )
			{
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempAfflictOnAttackCount[iNewIndex], "afflictOnAttack");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempCureAfflictionCount[iNewIndex], "cureAffliction");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempAfflictionTurnCount[iNewIndex], "afflictionTurn");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempAfflictionHitCount[iNewIndex], "afflictionHit");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempAfflictionTolerance[iNewIndex], "afflictionTolerance");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempFortitudeModifierTypeAmount[iNewIndex], "fortitudeModifierType");
			}
		}
	} while(iI != -1);
	
	for(iI = 0; iI < GC.getNumPromotionInfos(); iI++)
	{
		bool	bNonDefaultValue =
					(g_paiTempAfflictOnAttackCount[iI] != 0 ||
					 g_paiTempCureAfflictionCount[iI] != 0 ||
					 g_paiTempAfflictionTurnCount[iI] != 0 ||
					 g_paiTempAfflictionHitCount[iI] != 0 ||
					 g_paiTempAfflictionTolerance[iI] != 0 ||
					 g_paiTempFortitudeModifierTypeAmount[iI] != 0);

		if ( bNonDefaultValue )
		{
			PromotionKeyedInfo* info = findOrCreatePromotionKeyedInfo((PromotionTypes)iI);

			info->m_iAfflictOnAttackCount = g_paiTempAfflictOnAttackCount[iI];
			info->m_iCureAfflictionCount = g_paiTempCureAfflictionCount[iI];
			info->m_iAfflictionTurnCount = g_paiTempAfflictionTurnCount[iI];
			info->m_iAfflictionHitCount = g_paiTempAfflictionHitCount[iI];
			info->m_iAfflictionTolerance = g_paiTempAfflictionTolerance[iI];
			info->m_iFortitudeModifierTypeAmount = g_paiTempFortitudeModifierTypeAmount[iI];
		}
	}

to this:
Spoiler :
Code: 
	//	Backward compatibility - read array format if present
	for(iI = 0; iI < GC.getNumPromotionLineInfos(); iI++)
	{
		g_paiTempCureAfflictionCount[iI] = 0;
		g_paiTempAfflictionTurnCount[iI] = 0;
		g_paiTempAfflictionLineCount[iI] = 0;
		g_paiTempAfflictionTolerance[iI] = 0;
		g_paiTempFortitudeModifierTypeAmount[iI] = 0;
	}
	WRAPPER_SKIP_ELEMENT(wrapper, "CvUnit", g_paiTempAfflictOnAttackCount, SAVE_VALUE_ANY);
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONLINES, GC.getNumPromotionLineInfos(), g_paiTempCureAfflictionCount, "m_paiCureAfflictionCount");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONLINES, GC.getNumPromotionLineInfos(), g_paiTempAfflictionTurnCount, "m_paiAfflictionTurnCount");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONLINES, GC.getNumPromotionLineInfos(), g_paiTempAfflictionLineCount, "m_paiAfflictionLineCount");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONLINES, GC.getNumPromotionLineInfos(), g_paiTempAfflictionTolerance, "m_paiAfflictionTolerance");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONLINES, GC.getNumPromotionLineInfos(), g_paiTempFortitudeModifierTypeAmount, "m_paiFortitudeModifierTypeAmount");

	//	Read new format compressed data if present
	do
	{
		iI= -1;
		WRAPPER_READ_DECORATED(wrapper, "CvUnit", &iI, "hasAfflicationInfo");
		if ( iI != -1 )
		{
			int iNewIndex = wrapper.getNewClassEnumValue(REMAPPED_CLASS_TYPE_PROMOTIONLINES, iI, true);

			if ( iNewIndex != NO_PROMOTIONLINE )
			{
				WRAPPER_SKIP_ELEMENT(wrapper, "CvUnit", &g_paiTempAfflictOnAttackCount[iNewIndex], SAVE_VALUE_ANY);
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempCureAfflictionCount[iNewIndex], "cureAffliction");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempAfflictionTurnCount[iNewIndex], "afflictionTurns);
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempAfflictionLineCount[iNewIndex], "hasAffliction");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempAfflictionTolerance[iNewIndex], "afflictionTolerance");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempFortitudeModifierTypeAmount[iNewIndex], "fortitudeModifierType");
			}
		}
	} while(iI != -1);
	
	for(iI = 0; iI < GC.getNumPromotionLineInfos(); iI++)
	{
		bool	bNonDefaultValue =
					(g_paiTempCureAfflictionCount[iI] != 0 ||
					 g_paiTempAfflictionTurnCount[iI] != 0 ||
					 g_paiTempAfflictionLineCount[iI] != 0 ||
					 g_paiTempAfflictionTolerance[iI] != 0 ||
					 g_paiTempFortitudeModifierTypeAmount[iI] != 0);

		if ( bNonDefaultValue )
		{
			PromotionLineKeyedInfo* info = findOrCreatePromotionLineKeyedInfo((PromotionLineTypes)iI);

			info->m_iCureAfflictionCount = g_paiTempCureAfflictionCount[iI];
			info->m_iAfflictionTurnCount = g_paiTempAfflictionTurnCount[iI];
			info->m_iAfflictionLineCount = g_paiTempAfflictionLineCount[iI];
			info->m_iAfflictionTolerance = g_paiTempAfflictionTolerance[iI];
			info->m_iFortitudeModifierTypeAmount = g_paiTempFortitudeModifierTypeAmount[iI];
		}
	}
is problematic in any way. Note these changes:
  • Changing the basic loop data through PromotionInfo to PromotionLineInfo (note I HAVE duplicated everything I can find in regards to establishing the KeyedInfo structures already so that shouldn't be an issue in and of itself.)

  • Changing the 'naming' of these entries.

  • Removing one entry entirely (and moving some others to a new loop at the bottom of the wrapper read lists) - biggest concern here is that the WRAPPER_READ_CLASS_ARRAY_DECORATED replaced with WRAPPER_SKIP_ELEMENT has a different number of parameter calls so I'm worried it wouldn't work right in that spot or that I may have included the wrong data in its parameters perhaps.

  • In the act of ending up with one less entry, I'm hoping it doesn't mess up any of the lists there that as a result have one less line in total (I suspect those are safe to have removed the no longer applicable entry?)

On the Wrapper Write side I only have listed what's applicable here and thus it ends up being 1 less line on each process list - that should be ok shouldn't it?


In doing my darnedest to analyze this entire method I figured out I'd really screwed the pooch on all those new tags at the end of the current wrapper lists - they should've been all part of one loop since they were all UnitCombat based infos. Oops... sorry for that. When I noticed the problem I didn't want to rush to fix it in fear that it would create a savegame incompatibility issue. I'm glad you mentioned you had a means to do so.

But at some point you stated I'd done something SO wrong that the data was reading incorrectly - what was that? I noticed one naming tag was incorrect on a recent change - was that all or is there anything else I'm missing here?
 
Thunderbrd said:
I've been wondering that myself actually... I think the calculated score itself is probably done in the dll, then called and displayed via python, but I don't know what file... CvPlayer perhaps?


Ok, in the SVN thread I alluded to having some questions regarding the read/write wrappers in CvUnit. I need to know if the conversion from this:
Spoiler :
Code: 
	//	Backward compatibility - read array format if present
	for(iI = 0; iI < GC.getNumPromotionInfos(); iI++)
	{
		g_paiTempAfflictOnAttackCount[iI] = 0;
		g_paiTempCureAfflictionCount[iI] = 0;
		g_paiTempAfflictionTurnCount[iI] = 0;
		g_paiTempAfflictionHitCount[iI] = 0;
		g_paiTempAfflictionTolerance[iI] = 0;
		g_paiTempFortitudeModifierTypeAmount[iI] = 0;
	}
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONS, GC.getNumPromotionInfos(), g_paiTempAfflictOnAttackCount, "m_paiAfflictOnAttackCount");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONS, GC.getNumPromotionInfos(), g_paiTempCureAfflictionCount, "m_paiCureAfflictionCount");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONS, GC.getNumPromotionInfos(), g_paiTempAfflictionTurnCount, "m_paiAfflictionTurnCount");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONS, GC.getNumPromotionInfos(), g_paiTempAfflictionHitCount, "m_paiAfflictionHitCount");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONS, GC.getNumPromotionInfos(), g_paiTempAfflictionTolerance, "m_paiAfflictionTolerance");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONS, GC.getNumPromotionInfos(), g_paiTempFortitudeModifierTypeAmount, "m_paiFortitudeModifierTypeAmount");

	//	Read new format compressed data if present
	do
	{
		iI= -1;
		WRAPPER_READ_DECORATED(wrapper, "CvUnit", &iI, "hasAfflicationInfo");
		if ( iI != -1 )
		{
			int iNewIndex = wrapper.getNewClassEnumValue(REMAPPED_CLASS_TYPE_PROMOTIONS, iI, true);

			if ( iNewIndex != NO_PROMOTION )
			{
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempAfflictOnAttackCount[iNewIndex], "afflictOnAttack");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempCureAfflictionCount[iNewIndex], "cureAffliction");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempAfflictionTurnCount[iNewIndex], "afflictionTurn");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempAfflictionHitCount[iNewIndex], "afflictionHit");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempAfflictionTolerance[iNewIndex], "afflictionTolerance");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempFortitudeModifierTypeAmount[iNewIndex], "fortitudeModifierType");
			}
		}
	} while(iI != -1);
	
	for(iI = 0; iI < GC.getNumPromotionInfos(); iI++)
	{
		bool	bNonDefaultValue =
					(g_paiTempAfflictOnAttackCount[iI] != 0 ||
					 g_paiTempCureAfflictionCount[iI] != 0 ||
					 g_paiTempAfflictionTurnCount[iI] != 0 ||
					 g_paiTempAfflictionHitCount[iI] != 0 ||
					 g_paiTempAfflictionTolerance[iI] != 0 ||
					 g_paiTempFortitudeModifierTypeAmount[iI] != 0);

		if ( bNonDefaultValue )
		{
			PromotionKeyedInfo* info = findOrCreatePromotionKeyedInfo((PromotionTypes)iI);

			info->m_iAfflictOnAttackCount = g_paiTempAfflictOnAttackCount[iI];
			info->m_iCureAfflictionCount = g_paiTempCureAfflictionCount[iI];
			info->m_iAfflictionTurnCount = g_paiTempAfflictionTurnCount[iI];
			info->m_iAfflictionHitCount = g_paiTempAfflictionHitCount[iI];
			info->m_iAfflictionTolerance = g_paiTempAfflictionTolerance[iI];
			info->m_iFortitudeModifierTypeAmount = g_paiTempFortitudeModifierTypeAmount[iI];
		}
	}

to this:
Spoiler :
Code: 
	//	Backward compatibility - read array format if present
	for(iI = 0; iI < GC.getNumPromotionLineInfos(); iI++)
	{
		g_paiTempCureAfflictionCount[iI] = 0;
		g_paiTempAfflictionTurnCount[iI] = 0;
		g_paiTempAfflictionLineCount[iI] = 0;
		g_paiTempAfflictionTolerance[iI] = 0;
		g_paiTempFortitudeModifierTypeAmount[iI] = 0;
	}
	WRAPPER_SKIP_ELEMENT(wrapper, "CvUnit", g_paiTempAfflictOnAttackCount, SAVE_VALUE_ANY);
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONLINES, GC.getNumPromotionLineInfos(), g_paiTempCureAfflictionCount, "m_paiCureAfflictionCount");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONLINES, GC.getNumPromotionLineInfos(), g_paiTempAfflictionTurnCount, "m_paiAfflictionTurnCount");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONLINES, GC.getNumPromotionLineInfos(), g_paiTempAfflictionLineCount, "m_paiAfflictionLineCount");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONLINES, GC.getNumPromotionLineInfos(), g_paiTempAfflictionTolerance, "m_paiAfflictionTolerance");
	WRAPPER_READ_CLASS_ARRAY_DECORATED(wrapper, "CvUnit", REMAPPED_CLASS_TYPE_PROMOTIONLINES, GC.getNumPromotionLineInfos(), g_paiTempFortitudeModifierTypeAmount, "m_paiFortitudeModifierTypeAmount");

	//	Read new format compressed data if present
	do
	{
		iI= -1;
		WRAPPER_READ_DECORATED(wrapper, "CvUnit", &iI, "hasAfflicationInfo");
		if ( iI != -1 )
		{
			int iNewIndex = wrapper.getNewClassEnumValue(REMAPPED_CLASS_TYPE_PROMOTIONLINES, iI, true);

			if ( iNewIndex != NO_PROMOTIONLINE )
			{
				WRAPPER_SKIP_ELEMENT(wrapper, "CvUnit", &g_paiTempAfflictOnAttackCount[iNewIndex], SAVE_VALUE_ANY);
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempCureAfflictionCount[iNewIndex], "cureAffliction");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempAfflictionTurnCount[iNewIndex], "afflictionTurns);
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempAfflictionLineCount[iNewIndex], "hasAffliction");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempAfflictionTolerance[iNewIndex], "afflictionTolerance");
				WRAPPER_READ_DECORATED(wrapper, "CvUnit", &g_paiTempFortitudeModifierTypeAmount[iNewIndex], "fortitudeModifierType");
			}
		}
	} while(iI != -1);
	
	for(iI = 0; iI < GC.getNumPromotionLineInfos(); iI++)
	{
		bool	bNonDefaultValue =
					(g_paiTempCureAfflictionCount[iI] != 0 ||
					 g_paiTempAfflictionTurnCount[iI] != 0 ||
					 g_paiTempAfflictionLineCount[iI] != 0 ||
					 g_paiTempAfflictionTolerance[iI] != 0 ||
					 g_paiTempFortitudeModifierTypeAmount[iI] != 0);

		if ( bNonDefaultValue )
		{
			PromotionLineKeyedInfo* info = findOrCreatePromotionLineKeyedInfo((PromotionLineTypes)iI);

			info->m_iCureAfflictionCount = g_paiTempCureAfflictionCount[iI];
			info->m_iAfflictionTurnCount = g_paiTempAfflictionTurnCount[iI];
			info->m_iAfflictionLineCount = g_paiTempAfflictionLineCount[iI];
			info->m_iAfflictionTolerance = g_paiTempAfflictionTolerance[iI];
			info->m_iFortitudeModifierTypeAmount = g_paiTempFortitudeModifierTypeAmount[iI];
		}
	}
is problematic in any way. Note these changes:
  • Changing the basic loop data through PromotionInfo to PromotionLineInfo (note I HAVE duplicated everything I can find in regards to establishing the KeyedInfo structures already so that shouldn't be an issue in and of itself.)

  • Changing the 'naming' of these entries.

  • Removing one entry entirely (and moving some others to a new loop at the bottom of the wrapper read lists) - biggest concern here is that the WRAPPER_READ_CLASS_ARRAY_DECORATED replaced with WRAPPER_SKIP_ELEMENT has a different number of parameter calls so I'm worried it wouldn't work right in that spot or that I may have included the wrong data in its parameters perhaps.

  • In the act of ending up with one less entry, I'm hoping it doesn't mess up any of the lists there that as a result have one less line in total (I suspect those are safe to have removed the no longer applicable entry?)

On the Wrapper Write side I only have listed what's applicable here and thus it ends up being 1 less line on each process list - that should be ok shouldn't it?


In doing my darnedest to analyze this entire method I figured out I'd really screwed the pooch on all those new tags at the end of the current wrapper lists - they should've been all part of one loop since they were all UnitCombat based infos. Oops... sorry for that. When I noticed the problem I didn't want to rush to fix it in fear that it would create a savegame incompatibility issue. I'm glad you mentioned you had a means to do so.

But at some point you stated I'd done something SO wrong that the data was reading incorrectly - what was that? I noticed one naming tag was incorrect on a recent change - was that all or is there anything else I'm missing here?
Click to expand...

Well, it's probably self consistent, but it won't be backward compatible if any existing save had any values recorded against the promotions, as opposed to the promotion lines now. The basic rule is that any tag that is present in a save has to be consumed. If the one you try to read is ot the next one in the save stream, it assumes that the one you are trying to read is new since the save was made, and therefore that it isn't there, so leaves it at it's default value and continues with the next read (not updating the position in the stream that it's reading). This allows newer saves to add new values, but does not allow values that used to be there to be removed without explicit skips. Since the old saves will have the values recorded under promotions (if any actually ever did) and they a not handled by the new read routine, the effect would be to stall the read stream at that point continue on through the read routine, but never finding a match. Eventually it would reach the sect end read call at the end of the read routine, at which point it would declare it an incompatible save.
 
I don't believe any units in any game would currently have had any values in those slots so I presume then that we're fine?

BTW, what you stated suggests that within the bounds of one of these sections I CAN add new wrapper tags without having to create a new section at the end of the entire wrapper segment. Is that true?
 
Thunderbrd said:
I don't believe any units in any game would currently have had any values in those slots so I presume then that we're fine?

BTW, what you stated suggests that within the bounds of one of these sections I CAN add new wrapper tags without having to create a new section at the end of the entire wrapper segment. Is that true?
Click to expand...

Yes. In fact it's better NOT to put the new tags at the end of an object section really as that can make debugging it harder due to the special processing that happens when the stream reads an object end tag.
 
I'd like to increase the complexity here a little to make the ai select these promotions more intelligently and selectively.

Code: 
	for (iI = 0; iI < GC.getPromotionInfo(ePromotion).getNumCureAfflictionChangeTypes(); ++iI)
	{
		if (kPromotion.getCureAfflictionChangeType(iI) > 0)
		{
			iValue += 75;
		}
	}
I want the evaluation to be something along the lines of:
if the promotion grants us this ability then,
iTemp = 10;

then count how many units the AI has that is afflicted with what this promotion allows the unit to cure and call that iAfflictionCount.
Then count how many units the AI has on the same plot that have this affliction and call that count iPlotAfflictedCount.

iPlotAfflictedCount *= 10;

iAfflictionCount += ((iAfflictionCount * iPlotAfflictedCount)/100);

iTemp *= iAfflictionCount;

iValue += iTemp;

Its what's in red italics that I'm not sure how to reference effectively. How would y'all code that kind of reference call?
 
Thunderbrd said:
count how many units the AI has that is afflicted with what this promotion allows the unit to cure and call that iAfflictionCount.
Click to expand...
Something like:
Code: 
CvPlayer::countAfflictedUnits(PromotionTypes eAffliction)
{
    int iResult = 0;
    int iLoop;

     for (CvUnit* pLoopUnit = firstUnit(&iLoop); pLoopUnit != NULL; pLoopUnit = nextUnit(&iLoop))
    {
        if (pLoopUnit->isHasPromotion(eAfflication) )
        {
            iResult++;
        }
    }

    return iResult;
}
If you're going to wind up calling it a lot, you may want to cache it (once per turn say) for performance reasons.
Thunderbrd said:
count how many units the AI has on the same plot that have this affliction and call that count iPlotAfflictedCount.
Click to expand...

Code: 
int CvPlot::getNumAfflictedUnits(PlayerTypes eOwner, PromotionTypes eAfflication) const
{
    return plotCount(PUF_isAfflicted, eAffliction, -1, ePlayer);
}
together with a new Plot Unit Function (PUF, which you'll find a palette of defined in CvGameCoreUtils.cpp):
Code: 
bool PUF_isAfflicted(const CvUnit* pUnit, int iData1, int iData2)
{
    return pUnit->isHasPromotion((PromotionTypes)iData1);
}
 
You must log in or register to reply here.

Similar threads

D
Another scientific victory
Replies
4
Views
815
damerell
D
A
  • Locked
New Version – 4.21.1 (May 3, 2025)
2 3 4
Replies
66
Views
15K
Recursive
Recursive
S
[MOD] Just a list of what I think C2C going stand-alone MAY pay at least some attention to.
Replies
15
Views
2K
Osk21
Osk21
Taefin
Taefin's More AI Cheats
Replies
2
Views
802
Taefin
Taefin
A
General/Various questions
Replies
0
Views
457
AntiChrist7
A
Back
Top Bottom