Troubles with Arrays and Arrays of Arrays...

Remind me again why you need the bool? Suppose you had just the tech integer. Could you test if that is -1, instead of having a second array?
 
phungus420 said:
Yeah, I searched all through the XML, and couldn't find any examples. Would it be possible to code the parser to read 3 children, or is this in the Exe / is too difficult to do? I can of curse set up two tags, but it just doesn't look as clean, and it also runs into logic coding problems, if someone sets a building in one tag, but forgets to in another, etc. Ultimately I'll probably settle on that aproach though, but figure I should ask if this is at least feasible.
Click to expand...

BonusTypeStructs in Civ4ImprovementInfos is an example. Quite a bit of code for it though IIRC.

Code: 
            <BonusTypeStructs>
                <BonusTypeStruct>
                    <BonusType>BONUS_MARBLE</BonusType>
                    <bBonusMakesValid>1</bBonusMakesValid>
                    <bBonusTrade>1</bBonusTrade>
                    <iDiscoverRand>0</iDiscoverRand>
                    <YieldChanges>
                        <iYieldChange>0</iYieldChange>
                        <iYieldChange>1</iYieldChange>
                        <iYieldChange>2</iYieldChange>
                    </YieldChanges>
                </BonusTypeStruct>
            </BonusTypeStructs>
 
Excellent pointer. There is a special purpose class, CvImprovementBonusInfo just for this, and a special purpose xml parser function, SetImprovementBonuses just for this. If you can do without the bool array, the solution is very simple. You could model new code after CvImprovementBonusInfo, but that is a much larger task.
 
I don't see how it's possible to go without the bool. Without a bool there is no way to test if a building is required for a unit, but also the requirement isn't eventually overridden by a tech. Setting up a unique class is a work around I'd only want to go to as a last resort; this if for RevDCM, so anything that's written for it should be useable in the long term for other modders; quick fixes doesn't fit the purpose.
 
The bool is there so you can turn off the building class requirement without removing the whole element itself. I wouldn't have done it this way--the presence of a building class should be sufficient to say, "this building class is required," and its absence the opposite. But that's how Firaxis did it, and there's no use fighting it now. :)

As for making generic code here, you could copy SetVariableListTagPair() to create SetVariableListTagTriple() that accepts a second int array pointer. This second array will only accept ints that are looked up via FindInInfoClass(). Note that FindInInfoClass() doesn't care what actual type it is, so all these functions will happily accept a TechTypes where a BuildingClassTypes should go. Avoiding that is left up to the modder.

There are seven SetVariableListTagPair() functions: four (bool, int, float, and CvString) that take iInfoBaseSize and iInfoBaseLength and three (same minus float) that take m_paszTagList and iTagListLength. In all cases iInfoBaseSize and m_paszTagList are unused and the lengths are used to cap the number of entries.

The ones with info base use FindInfoTypeNum() while the others use GC.getTypesEnum(). The two functions seem to do the same thing so you'll have to drill down to learn more.

In any case, you may want to simply copy all seven functions to create your new functions. Add the int **ppiOtherList and iDefaultOtherListVal parameters, call IniitList() on the other list, and in the loop read the third value:

Code: 
void CvXMLLoadUtility::SetVariableListTagPair(
        bool **ppbList, [B][COLOR="Red"]int **ppiOtherList,[/COLOR][/B] const TCHAR* szRootTagName,
        int iInfoBaseSize, int iInfoBaseLength, bool bDefaultListVal[B][COLOR="Red"], int iDefaultOtherListVal[/COLOR][/B])
{
    ...
    InitStringList(ppbList, iInfoBaseLength, bDefaultListVal);
    [B][COLOR="Red"]InitStringList(ppiOtherList, iInfoBaseLength, iDefaultOtherListVal);[/COLOR][/B]
    ...
    iIndexVal = FindInInfoClass(szTextVal);
    if (iIndexVal != -1)
    {
        [B][COLOR="Red"]if (GetNextXmlVal(szTextVal))
        {
            (*ppbList)[iIndexVal] = FindInInfoClass(szTextVal);
        }[/COLOR][/B]
    }
    
    gDLL->getXMLIFace()->SetToParent(m_pFXml);
    ...
}

Note that probably use GC.getTypesEnum() in the three that use it to match.

Now, if you want to get really crazy, you could add the two parameters onto the end with defaults (NULL and 0 or whatever makes sense for the default value). Then check the other array for NULL in the code. If it's not NULL, call InitList() on it and inside the loop read the value.

Code: 
void CvXMLLoadUtility::SetVariableListTagPair(
        bool **ppbList, const TCHAR* szRootTagName,
        int iInfoBaseSize, int iInfoBaseLength, bool bDefaultListVal,
        [B][COLOR="Red"]int **ppiOtherList = NULL, int iDefaultOtherListVal = 0[/COLOR][/B])
{
    ...
    InitStringList(ppbList, iInfoBaseLength, bDefaultListVal);
    [B][COLOR="Red"]if (ppiOtherList != NULL)
    {
        InitStringList(ppiOtherList, iInfoBaseLength, iDefaultOtherListVal);
    }[/COLOR][/B]
    ...
    iIndexVal = FindInInfoClass(szTextVal);
    if (iIndexVal != -1)
    {
        GetNextXmlVal(ppbList[iIndexVal]);
        [B][COLOR="Red"]if (ppiOtherList != NULL)
        {
            if (GetNextXmlVal(szTextVal))
            {
                (*ppbList)[iIndexVal] = FindInInfoClass(szTextVal);
            }
        }[/COLOR][/B]
    }
    
    gDLL->getXMLIFace()->SetToParent(m_pFXml);
    ...
}

Now you don't need extra functions. This assumes these functions are not exported from the DLL. If they are marked with DllExport you cannot change the parameter list--even if you add defaults since the default values must be compiled into the calling code (EXE).

In the end, not too bad.
 
Rather then setting up a tripple XML tag reading function, I think it would be best to just overload the function (only a pair makes sense for a single array anyway, you can't read and add a third value to a 1 dimensional array, at least I'm not seeing how) and have the overloaded function search for a specified tag, rather then grab the value of the second item in the XML (currently this function has the coder specify the first tag to be read, then it reads the value of the tag immediatly below it). I've tried looking at the SDK and reading the above post, but I started nodding off; so I don't think I can be too productive right now. I'll come back to this in a few hours. If anyone has any insights or advise about overloading the function SetVariableListTagPair so that the modder can specify the two specific tags they want read; please feel free to let me know.
 
Ok, there are a couple of approaches which you can use here, ranging from "simple to implement now" to "pain to do now, but provides useful tools for later" including in the middle "OMG IT HURTS, WHY?!? But I learned something!"


The really simple to implement solution is already partially mentioned by others offering assistance. Ditch the Boolean.

Code: 
<PrereqBuildingClasses>
	<PrereqBuildingClass>
		<BuildingClassType>BUILDINGCLASS_FORGE</BuildingClassType>
		<bPrereq>1</bPrereq>
	</PrereqBuildingClass>
<PrereqBuildingClasses>

Instead becomes
Code: 
<PrereqBuildingClasses>
	<PrereqBuildingClass>
		<BuildingClassType>BUILDINGCLASS_FORGE</BuildingClassType>
		<TechTypeOverride>NONE</TechTypeOverride>
	</PrereqBuildingClass>
	<PrereqBuildingClass>
		<BuildingClassType>BUILDINGCLASS_BARRACKS</BuildingClassType>
		<TechTypeOverride>TECH_MACHINERY</TechTypeOverride>
	</PrereqBuildingClass>
<PrereqBuildingClasses>


To do this, you just modify the Schema to use TechTypeOverride (defined to be a text entry, not bool or int), and in the DLL you continue to use SetVariableListTagPair, but you add an extra parameter to the end of your arguement of -2:

pXML->SetVariableListTagPair(&m_pbPrereqBuildingClass, "PrereqBuildingClasses", sizeof(GC.getBuildingClassInfo((BuildingClassTypes)0)), GC.getNumBuildingClassInfos(), -2);

Now, the final change is that your current code to check if there is a prereq needs to check against a -2 meaning no prereq, a -1 meaning a prereq with no Tech Override, and any non-negative value meaning there is a prereq, and it has a tech override corresponding to the integer value listed.

Code: 
bool CvUnitInfo::getPrereqBuildingClass(int iType)
{
    return m_bpPrereqBuldingClass[iType];
}

And a sample check would be:

Code: 
bool CvCity::hasBuildingClassesRequired(UnitTypes eUnit) const
{
	for ( int i = 0; i < GC.getNumBuildingClassInfos(); i++)  
	{
		if (GC.getUnitInfo(eUnit).getPrereqBuildingClass(i) == -1)
		{
			if (!isHasBuildingClass(i))
			{
				return false;
			}
		}
		else if (GC.getUnitInfo(eUnit).getPrereqBuildingClass(i) > -1)
		{
			if (!GET_TEAM(getTeam()).isHasTech((TechTypes)(GC.getUnitInfo(eUnit).getPrereqBuildingClass(i))))
			{
				if (!isHasBuildingClass(i))
				{
					return false;
				}
			}
		}
	}
	return true;
}

So, if the value was -2, then the buildingclass wasn't listed in the XML, so isn't checked for presense. If the value was -1, then NONE was listed for the Tech Override and the buildingclass is checked regardless of techs. If the value was a valid tech enum, then the tech is checked, and if lacking, the building is looked for.

Failure to find the building when looked for results in a refusal of permission, success means you keep checking the rest of the classes. If you make it through the list, the unit is allowed.


The other approaches would be more painful as they require writing special rules for reading the XML, which is quite possible, I believe I may have some examples in the Fall Further code, but can't think of any off the top of my head as I haven't used the code in quite some time now :( And there is the example listed of Bonus Structs. You will HAVE to use this method if TechTypes aren't loaded before UnitTypes, but in the base Firaxis code it appears that they are, so you are probably safe.

Anyway, if you want to go the other route, or this doesn't work as well as I am thinking it should, you may need to nudge me by PM to get further responses, but I'll try to check back here for a few days. Though that'd be at best once a day, so for a prompt response, toss me a PM saying to look here again.
 
Since this is for RevDCM, which is used as a modding core for multiple mods, going for a quick implementation is out. Currently I think the best aproach would be to overload the function SetVariableListTagPair in the CvXMLLoadUtility class; have it so the coder can specify the initial tag to be read to set the position in the array (like it is now), and all them to specify the tag to pull the value from that goes in this position (rather then how it currently simply pulls the value from the tag immediatly below the specified tag). I must get some sleep and then I will look at this once I'm no longer nodding off on the computer. I think it'd be best to move further discussion to the thread in the Python/SDK forums, as I think this topic is a bit advanced for this thread, and there are others involved in the discussion. If you have any further insights, especially with regarding how to write the implemtation code for reading the XML, please let me know; I know absolutely nothing about parsing XML data from C++. And thanks for taking the time to look at this.
 
Even if you go this route you need to modify SetVariableListTagPair() to read in a second string and then look up its info type number. The current function only does applies this to the first string read to determine the array index. You must write new code or modify SetVariableListTagPair() AFAICT.
 
One thing I must note here, is that Xienwolf's suggestion uses the least amount of memory of any of the current suggestions.
 
Sure, but you're talking about 4 * NUM_TECHS * NUM_BUILDING_CLASSES bytes. I try to avoid overloading the value of enums when I can. The whole point of an enum is that you can depend on it taking on only a prescribed set of values. Adding new values to it shouldn't be done lightly. Complicating the accessor code to save 40k (100 techs and 100 building classes) doesn't seem worth it to me here.
 
EmperorFool said:
Sure, but you're talking about 4 * NUM_TECHS * NUM_BUILDING_CLASSES bytes. I try to avoid overloading the value of enums when I can. The whole point of an enum is that you can depend on it taking on only a prescribed set of values. Adding new values to it shouldn't be done lightly. Complicating the accessor code to save 40k (100 techs and 100 building classes) doesn't seem worth it to me here.
Click to expand...

Not quite. It's a 40k saving, PER UNIT. Not to be underestimated.

As for messing up the enums, create a couple helper functions. The user code can still work fine if you just add a few of those.
 
No, I had a typo. The saving is 4 * NUM_BUILDING_CLASSES per unit. Each 4 represents the single TechTypes value stored in each array slot, one slot per building class, one array per unit. So it's still ~40k total.
 
Afforess said:
As for messing up the enums, create a couple helper functions. The user code can still work fine if you just add a few of those.
Click to expand...

What should isBuildingClassRequired(int i) return when the array slot holds a valid tech? The CvUnitInfo won't have access to the player to check for the tech. I agree it's totally doable; I just have a gut feel the other solution will be easier and robust enough.
 
EmperorFool said:
What should isBuildingClassRequired(int i) return when the array slot holds a valid tech? The CvUnitInfo won't have access to the player to check for the tech. I agree it's totally doable; I just have a gut feel the other solution will be easier and robust enough.
Click to expand...

Overload it. One return int for the override tech, (-1 for no tech), and one bool for whether there is even a requirement. Create some helper's in CvTeam to check if you need the building or not to train the unit.
 
Given the choice between a) use 40k of RAM and b) change an API used in many places and risk modders using it incorrectly and introducing subtle errors (tech != NO_TEAM but it certainly isn't a valid tech so GC.getTechInfo(-2) is going to be nasty to track down), I'd choose the former.
 
I'm not the one coding this, so I'm not invested in what Phungus chooses, but...
EmperorFool said:
so GC.getTechInfo(-2) is going to be nasty to track down
Click to expand...

I disagree, -2 will trigger a FAssert call right before the CTD. It would be easy to debug.
 
As I said, parsing XML is something I just don't have any experience with. Here is the default function, currently used:

Code: 
void CvXMLLoadUtility::SetVariableListTagPair(int **ppiList, const TCHAR* szRootTagName,
	int iInfoBaseSize, int iInfoBaseLength, int iDefaultListVal)
{
Spoiler :
Code: 
	int i;
	int iIndexVal;
	int iNumSibs;
	TCHAR szTextVal[256];
	int* piList;

	if (0 > iInfoBaseLength)
	{
		char	szMessage[1024];
		sprintf( szMessage, "Allocating zero or less memory in CvXMLLoadUtility::SetVariableListTagPair \n Current XML file is: %s", GC.getCurrentXMLFile().GetCString());
		gDLL->MessageBox(szMessage, "XML Error");
	}
	InitList(ppiList, iInfoBaseLength, iDefaultListVal);
	if (gDLL->getXMLIFace()->SetToChildByTagName(m_pFXml,szRootTagName))
	{
		if (SkipToNextVal())
		{
			iNumSibs = gDLL->getXMLIFace()->GetNumChildren(m_pFXml);
			piList = *ppiList;
			if (0 < iNumSibs)
			{
				if(!(iNumSibs <= iInfoBaseLength))
				{
					char	szMessage[1024];
					sprintf( szMessage, "There are more siblings than memory allocated for them in CvXMLLoadUtility::SetVariableListTagPair \n Current XML file is: %s", GC.getCurrentXMLFile().GetCString());
					gDLL->MessageBox(szMessage, "XML Error");
				}
				if (gDLL->getXMLIFace()->SetToChild(m_pFXml))
				{
					for (i=0;i<iNumSibs;i++)
					{
						if (GetChildXmlVal(szTextVal))
						{
							iIndexVal = FindInInfoClass(szTextVal);

							if (iIndexVal != -1)
							{
								GetNextXmlVal(&piList[iIndexVal]);
							}

							gDLL->getXMLIFace()->SetToParent(m_pFXml);
						}

						if (!gDLL->getXMLIFace()->NextSibling(m_pFXml))
						{
							break;
						}
					}

					gDLL->getXMLIFace()->SetToParent(m_pFXml);
				}
			}
		}

		gDLL->getXMLIFace()->SetToParent(m_pFXml);
	}
}

Now I'd like to overload it like so:

Code: 
void SetVariableListTagPair(int **ppiList, const TCHAR* szRootTagName,
	int iInfoBaseSize, int iInfoBaseLength, int iDefaultListVal = 0,
	const TCHAR* szValueTagName, int iValueInfoBaseLength)
Where szValueTagName is the name of the next tag to look for, and iValueInfoBaseLength is the maximum value that should be found in that tag (otherwise it would be outside the array size for that info class).

I'll step through the code and see if I am reading it right:

Spoiler :
Code: 
//Variables we need, and initializes the list so that all parts of the array hold the value defined in the default value:
	int i;
	int iIndexVal;
	int iNumSibs;
	TCHAR szTextVal[256];
	int* piList;

	if (0 > iInfoBaseLength)
	{
		char	szMessage[1024];
		sprintf( szMessage, "Allocating zero or less memory in CvXMLLoadUtility::SetVariableListTagPair \n Current XML file is: %s", GC.getCurrentXMLFile().GetCString());
		gDLL->MessageBox(szMessage, "XML Error");
	}
	InitList(ppiList, iInfoBaseLength, iDefaultListVal);
Spoiler :
Code: 
//defines the parent tag (for instance "BuildingHappinessChanges") defined in the CvInfos, then drops to the tag below this to begin reading
	if (gDLL->getXMLIFace()->SetToChildByTagName(m_pFXml,szRootTagName))
	{
		if (SkipToNextVal())
Spoiler :
Code: 
//Reads the value of that tag to set the position in the array it will assign the value to; throws error if it reads an index outside the accepted array range
			iNumSibs = gDLL->getXMLIFace()->GetNumChildren(m_pFXml);
			piList = *ppiList;
			if (0 < iNumSibs)
			{
				if(!(iNumSibs <= iInfoBaseLength))
				{
					char	szMessage[1024];
					sprintf( szMessage, "There are more siblings than memory allocated for them in CvXMLLoadUtility::SetVariableListTagPair \n Current XML file is: %s", GC.getCurrentXMLFile().GetCString());
					gDLL->MessageBox(szMessage, "XML Error");
				}
And here is where I'm having problems (assuming my understanding of the above is correct).
Spoiler :
Code: 
//drop to the next tag and assign the value in this tag to the position in the array set above.
//The question is how do I tell it to drop to the specified tag passed into the function in szValueTagName and check to make sure it is in the accepted range?
				if (gDLL->getXMLIFace()->SetToChild(m_pFXml))
				{
					for (i=0;i<iNumSibs;i++)
					{
						if (GetChildXmlVal(szTextVal))
						{
							iIndexVal = FindInInfoClass(szTextVal);

							if (iIndexVal != -1)
							{
								GetNextXmlVal(&piList[iIndexVal]);
							}

							gDLL->getXMLIFace()->SetToParent(m_pFXml);
						}

						if (!gDLL->getXMLIFace()->NextSibling(m_pFXml))
						{
							break;
						}
					}
 
OK, here is what I came up with. It compiles, but that means nothing now, as I know enough to make it compile, but not enough to ensure it is doing what I need it to do; and as I say I don't understand the XML parsing functions, this is purely based on guesswork:

Code: 
void CvXMLLoadUtility::SetVariableListTagPair(int **ppiList, const TCHAR* szRootTagName,
		int iInfoBaseSize, int iInfoBaseLength,
		const TCHAR* szValueTagName, int iValueInfoBaseLength, int iDefaultListVal)
{
	int i;
	int iIndexVal;
	int iNumSibs;
	TCHAR szTextVal[256];
	int* piList;

	if (0 > iInfoBaseLength)
	{
		char	szMessage[1024];
		sprintf( szMessage, "Allocating zero or less memory in CvXMLLoadUtility::SetVariableListTagPair \n Current XML file is: %s", GC.getCurrentXMLFile().GetCString());
		gDLL->MessageBox(szMessage, "XML Error");
	}
	InitList(ppiList, iInfoBaseLength, iDefaultListVal);
	if (gDLL->getXMLIFace()->SetToChildByTagName(m_pFXml,szRootTagName))
	{
		if (SkipToNextVal())
		{
			iNumSibs = gDLL->getXMLIFace()->GetNumChildren(m_pFXml);
			piList = *ppiList;
			if (0 < iNumSibs)
			{
				if(!(iNumSibs <= iInfoBaseLength))
				{
					char	szMessage[1024];
					sprintf( szMessage, "There are more siblings than memory allocated for them in CvXMLLoadUtility::SetVariableListTagPair \n Current XML file is: %s", GC.getCurrentXMLFile().GetCString());
					gDLL->MessageBox(szMessage, "XML Error");
				}
				if (gDLL->getXMLIFace()->SetToChildByTagName(m_pFXml,szValueTagName))
				{
					for (i=0;i<iNumSibs;i++)
					{
						if (GetChildXmlVal(szTextVal))
						{
							iIndexVal = FindInInfoClass(szTextVal);

							if (iIndexVal != -1)
							{
								GetNextXmlVal(&piList[iIndexVal]);
							}
							if( (iIndexVal > iValueInfoBaseLength) || (iIndexVal < -1) )
							{
								char	szMessage[1024];
								sprintf( szMessage, "A defined value for an array is outside of the accepted size of the infoclass!\n Current XML file is: %s", GC.getCurrentXMLFile().GetCString());
								gDLL->MessageBox(szMessage, "XML Error");
							}

							gDLL->getXMLIFace()->SetToParent(m_pFXml);
						}

						if (!gDLL->getXMLIFace()->NextSibling(m_pFXml))
						{
							break;
						}
					}

					gDLL->getXMLIFace()->SetToParent(m_pFXml);
				}
			}
		}

		gDLL->getXMLIFace()->SetToParent(m_pFXml);
	}
}

Please let me know if you see any problems with it.
 
Hrm.. I am really not sure what EF and Aff are arguing about regarding my posted solution. Nowhere in the code sample I posted would you wind up with a getTechInfo(-2) call. Maybe you are theorizing about someone else trying to check against this value through future work? In that case, you would have a safeguard built in with something like:

Code: 
bool CvUnitInfo::isBuildingClassPrereqOverridden(int iType) const
{
    return m_bpPrereqBuldingClass[iType] > -1;
}

or you could do a boolean check if the buildingclass is EVER required:
Code: 
bool CvUnitInfo::isBuildingClassPrereq(int iType) const
{
    return m_bpPrereqBuldingClass[iType] != -2;
}

Then as long as this is queried before trying to do any getTechInfo calls, you never call against -2. But this is such a niche function that I wouldn't anticipate it being reused by other code. Though there is much to be said for being safe from the start if your code is used by many others. Personally I'd like to think just leaving a comment in the code warning about proper handling of a -2 is sufficient.



@Phungus: I advise not overloading the function, as the name no longer fits what the new function actually does. The entire point of writing a new chunk of code like this one is so it is available for future use. So since it is a new function ANYWAY, give it a new name which is more descriptive of the new task, and will thus be likely to be used by future modders.


Since we want the function to be robust, think of any cases where you may want to use this, or something similar in nature, again. That determines how flexible we try to make it.

Also, if you JUST modify the current setVariableTagListPair to work for you, then you will have an order dependent end result. Personally, I am NOT a fan of that, especially since learning how to make the XML Schema no longer care about the order of tags either (SDK only cares about order in a few limited cases, mostly Yields/Commerces, and these silly pair lists).

The value of an order dependent result is that you don't care what name each tag holds. The value of order independent result is that there are fewer ways to mess things up for the XML only modders who won't understand a new format being tossed at them.

The parts where you think you understand the code are basically correct, though the wording is vague enough that there could still be some misconceptions lingering in the background.


Ok, so now for the part where you aren't sure about things, it gets a tad more confusing. This is because you are dealing with the order dependence issue.

Base code hops to a child, checks the first value in the lookup tables to get an INT, then snags the next value to store. Then it returns to the next layer up in the XML, and checks if there is more data to read or not.


So, if you want a solution for just your case specifically, and don't care about order dependence, I would suggest having it read 2 more values, first a boolean to see if there is an override at all, then a lookup value to see what the override should be if it exists.

Now, you could just have the return value be a double array which is 3 by X (Buildingclasses in this case), but then you are using a byte instead of a bit in the two boolean cases. So a Struct would be better from a strict memory conservation sense.


The issue is, deciding how you want to be able to use these functions in the future. If you don't care about flexibility, it is really easy. But if you do want to be flexible so it can be utilized for future projects, then you have to decide which areas you want to be flexible in.

Will post on your attempt in a new post.
 
You must log in or register to reply here.

Similar threads

keldath
ai civic code review
Replies
0
Views
328
keldath
keldath
Leoreth
Optimizing CvPlot data
Replies
15
Views
2K
Leoreth
Leoreth
keldath
[Dev] Civic dependency Tree (Parent)
Replies
2
Views
932
keldath
keldath
LPlate2
Array of Integers
Replies
6
Views
979
LPlate2
LPlate2
gavenkoa
Reasons for AI not trading, like WDWTSTTTJY / WFYABTA & the ways to avoid trade blockade.
Replies
3
Views
592
gavenkoa
gavenkoa
Back
Top Bottom