Big problem with new Promotion Tag

[OrionVeteran]

I am trying to create a new tag in CIV4PromotionInfos.xml for a prerequisite project.
I added all the XML and SDK code below and compiled the DLL successfully.
Note: I had to use a read pass for the game to load properly.
What have I left out or done wrong?



CIV4PromotionInfos.xml

Spoiler :

For all promotions:
<ProjectPrereq>NONE<ProjectPrereq>

Changed two "nuclear" promotions to require:
<ProjectPrereq>PROJECT_MANHATTAN_PROJECT<ProjectPrereq>

CIV4UnitSchema.xml

Spoiler :

<ElementType name="PromojectPrereq" content="textOnly"/>

<element type="PromojectPrereq"/>

SevoPediaPromotion.py

Spoiler :

eProject = gc.getPromotionInfo(self.iPromotion).getProjectPrereq()
CyInterface().addImmediateMessage(str(eProject), "")
if (eProject > -1):
screen.attachImageButton(panelName, "", gc.getProjectInfo(eProject).getButton(), GenericButtonSizes.BUTTON_SIZE_CUSTOM, WidgetTypes.WIDGET_PEDIA_JUMP_TO_PROJECT, eProject, 1, False)

The message comes back with a -1 for any promotion including the two I changed to PROJECT_MANHATTAN_PROJECT.

CvInfos.h

Spoiler :

int getProjectPrereq() const;

int m_iProjectPrereq;

CvInfos.cpp

Spoiler :

m_iProjectPrereq(NO_PROJECT),

int CvPromotionInfo::getProjectPrereq() const
{
return m_iProjectPrereq;
}


stream->Read(&m_iProjectPrereq);

stream->Write(m_iProjectPrereq);


CvPromotionInfo::ReadPass2:

pXML->GetChildXmlValByName(szTextVal, "ProjectPrereq");
m_iProjectPrereq = GC.getInfoTypeForString(szTextVal);

CyInfoInterface1.cpp

Spoiler :

.def("getProjectPrereq", &CvPromotionInfo::getProjectPrereq, "int ()")

CvUnit.cpp

Spoiler :

Under canAcquirePromotion:

ProjectTypes eProject = (ProjectTypes)GC.getPromotionInfo(ePromotion).getProjectPrereq();

if (eProject != NO_PROJECT)
{
if (GET_TEAM(getTeam()).getProjectCount((ProjectTypes)eProject) < 1)
{
return false;
}
}

 

[Nightinggale]

The problem is in CvXMLLoadUtility::LoadPreMenuGlobals() in CvXMLLoadUtilitySet.cpp

Most of the XML files are loaded in this function. Pay attention to the following lines: (line numbers are vanilla)

603 LoadGlobalClassInfo(GC.getPromotionInfo()
626 LoadGlobalClassInfo(GC.getProjectInfo()

This means promotions are read before projects and the type strings are unavailable. The solution for this type of problem are readPass2 and readPass3.

When a file is done being read, it is looped and readPass2 is called for each element. This is useful if the file refers to itself because then the order inside the file doesn't matter. However it has no influence regarding reading strings from other files.

readPass3 is not called automatically. Instead you mod CvXMLLoadUtility::LoadPreMenuGlobals() to call it once you have the needed strings.

This means after reading projects, add code like this:

    for (int i=0; i < GC.getNumPromotionInfos(); ++i)
   {
       GC.getPromotionInfo((PromotionTypes)i).readPass3();
   }

Also write in readPass3 what you tried in readPass2 and you should get the result you want.

 

[OrionVeteran]

The problem is in CvXMLLoadUtility::LoadPreMenuGlobals() in CvXMLLoadUtilitySet.cpp

Most of the XML files are loaded in this function. Pay attention to the following lines: (line numbers are vanilla)

603 LoadGlobalClassInfo(GC.getPromotionInfo()
626 LoadGlobalClassInfo(GC.getProjectInfo()

This means promotions are read before projects and the type strings are unavailable. The solution for this type of problem are readPass2 and readPass3.

When a file is done being read, it is looped and readPass2 is called for each element. This is useful if the file refers to itself because then the order inside the file doesn't matter. However it has no influence regarding reading strings from other files.

readPass3 is not called automatically. Instead you mod CvXMLLoadUtility::LoadPreMenuGlobals() to call it once you have the needed strings.

This means after reading projects, add code like this:

    for (int i=0; i < GC.getNumPromotionInfos(); ++i)
   {
       GC.getPromotionInfo((PromotionTypes)i).readPass3();
   }

Also write in readPass3 what you tried in readPass2 and you should get the result you want.

I'll give it a shot and see what happens.

 

[OrionVeteran]

Also write in readPass3 what you tried in readPass2 and you should get the result you want.

I ended up adding two lines to readPass2:

pXML->GetChildXmlValByName(szTextVal, "ProjectPrereq");
 m_aszExtraXMLforPass3.push_back(szTextVal);

Then I created readPass3:

bool CvPromotionInfo::readPass3()
{
 if (m_aszExtraXMLforPass3.size() < 1)
 {
  FAssert(false);
  return false;
 }
 
 m_iProjectPrereq = GC.getInfoTypeForString(m_aszExtraXMLforPass3[0]);
 m_aszExtraXMLforPass3.clear();

 return true;
}

It all works!

Thank you very much.

 

[Nightinggale]

I ended up adding two lines to readPass2:

pXML->GetChildXmlValByName(szTextVal, "ProjectPrereq");
 m_aszExtraXMLforPass3.push_back(szTextVal);

Personally I would set this in read() rather than readPass2. In fact I'm not that happy with readPass2. The code calling it looks a bit fragile to me. What it does is assume the order in XML is the same as the order in the info array. Proper XML files will agree with this, but it's not always the case. Imagine two modders both add PROMOTION_TEST. The first one is read into the info array just fine. The next one is detected to have the same type, which means it overwrites the first one instead of appending. The result is that the assumption that the vector and the XML file use the same order is no longer valid and you will get bugs.

The assumption also dies if you use modules. Say you have 30 promotions and you read a module with one promotion. You then have 31 promotions in the info array, but the XML file only has one and it assumes that one to be the first in the array while it's actually the last.

It will only call readPass2 for as many elements in the info array as there are elements in the XML file, meaning in the case of a module with one element, only the first element will call readPass2. This means it's not even enough to ignore the ability to read from XML in readPass2, you simply can't be sure it will be called at all.

readPass3 on the other hand will loop the info array as it is after reading both the XML file and the modules. This means it ignores overwritten modules and all those issues and it will call everything in the array.

Summary:
readPass2 can fail to work depending on XML setup
readPass3 will always work if used as written in #2

The XML loading code is of a code structure, which is no longer recommended (using a real database in recommended instead, like in civ6). Loading strings and looking up where the string points to will often be a mess and civ4 is no exception. It's doable to write code, which often works, but hard to write code, which always works.

I spent ages on this problem in M:C. It now loads the basic info for all XML files (including type strings). After that all files are loaded again, but this time with full info. This means that all strings from all files are available for all files and the readPass2+3 code structure is no longer needed. I managed to move it to FTTW, but I have yet to figure out a way to make a standard movable version of the code.
Also it asserts if an XML file use a type string, which is already in use as the new code design considers this an XML bug.

 

[OrionVeteran]

Personally I would set this in read() rather than readPass2. In fact I'm not that happy with readPass2. The code calling it looks a bit fragile to me. What it does is assume the order in XML is the same as the order in the info array. Proper XML files will agree with this, but it's not always the case. Imagine two modders both add PROMOTION_TEST. The first one is read into the info array just fine. The next one is detected to have the same type, which means it overwrites the first one instead of appending. The result is that the assumption that the vector and the XML file use the same order is no longer valid and you will get bugs.

The assumption also dies if you use modules. Say you have 30 promotions and you read a module with one promotion. You then have 31 promotions in the info array, but the XML file only has one and it assumes that one to be the first in the array while it's actually the last.

It will only call readPass2 for as many elements in the info array as there are elements in the XML file, meaning in the case of a module with one element, only the first element will call readPass2. This means it's not even enough to ignore the ability to read from XML in readPass2, you simply can't be sure it will be called at all.

readPass3 on the other hand will loop the info array as it is after reading both the XML file and the modules. This means it ignores overwritten modules and all those issues and it will call everything in the array.

Summary:
readPass2 can fail to work depending on XML setup
readPass3 will always work if used as written in #2

The XML loading code is of a code structure, which is no longer recommended (using a real database in recommended instead, like in civ6). Loading strings and looking up where the string points to will often be a mess and civ4 is no exception. It's doable to write code, which often works, but hard to write code, which always works.

I spent ages on this problem in M:C. It now loads the basic info for all XML files (including type strings). After that all files are loaded again, but this time with full info. This means that all strings from all files are available for all files and the readPass2+3 code structure is no longer needed. I managed to move it to FTTW, but I have yet to figure out a way to make a standard movable version of the code.
Also it asserts if an XML file use a type string, which is already in use as the new code design considers this an XML bug.

OK I have moved the two lines from ReadPass2 and placed them in Read. It works.

pXML->GetChildXmlValByName(szTextVal, "ProjectPrereq");
 m_aszExtraXMLforPass3.push_back(szTextVal);

But I have a couple of questions: If readPass2 can be such a problem, then why leave the other 6 lines in readPass2? Can these lines be moved as well?

pXML->GetChildXmlValByName(szTextVal, "PromotionPrereq");
 m_iPrereqPromotion = GC.getInfoTypeForString(szTextVal);
 pXML->GetChildXmlValByName(szTextVal, "PromotionPrereqOr1");
 m_iPrereqOrPromotion1 = GC.getInfoTypeForString(szTextVal);
 pXML->GetChildXmlValByName(szTextVal, "PromotionPrereqOr2");
 m_iPrereqOrPromotion2 = GC.getInfoTypeForString(szTextVal);

If these 6 lines are moved out of readPass2, then I suspect there may be a problem with m_aszExtraXMLforPass3.

Note: I stopped using XML modules long ago because of the heavy performance hit placed on the mod when using multiple modules. XML Modules make for nice coding, but performance suffered greatly.

 

[Nightinggale]

If you write those in read(), it will work if the order of the file is correct. That is any prerequisites for a promotion are written before the promotion in question. Alternatively you move them to readPass3, in which case the order no longer matters.

I'm actually a bit puzzled that readPass2 seems to have issues like that. It has been in use for years but I haven't seen anybody complaining, but I can't read the code in any way other than it looks dangerous. Perhaps the real solution would be to look into the code calling readPass2. If it instead of assuming the order will read the tag from XML, get the index and then call readPass2 on that index, then it should work as intended. Now that I know of this issue, I might actually try to make this fix. It should be fairly easy to implement in all mods since it's just one block of a few lines of code in a place I don't think anybody has modded.

 

[OrionVeteran]

If you write those in read(), it will work if the order of the file is correct. That is any prerequisites for a promotion are written before the promotion in question. Alternatively you move them to readPass3, in which case the order no longer matters.

I'm actually a bit puzzled that readPass2 seems to have issues like that. It has been in use for years but I haven't seen anybody complaining, but I can't read the code in any way other than it looks dangerous. Perhaps the real solution would be to look into the code calling readPass2. If it instead of assuming the order will read the tag from XML, get the index and then call readPass2 on that index, then it should work as intended. Now that I know of this issue, I might actually try to make this fix. It should be fairly easy to implement in all mods since it's just one block of a few lines of code in a place I don't think anybody has modded.

You have my interest. I look forward to your pending fix.

Thanks again,

OV

 

[OrionVeteran]

Now that I know of this issue, I might actually try to make this fix. It should be fairly easy to implement in all mods since it's just one block of a few lines of code in a place I don't think anybody has modded.

Did you get a chance to write this fix?

 

[Nightinggale]

Oops, I got sidetracked and then I completely forgot about this
I should probably try to look into this shortly. After all I just spent time rereading the whole thread to figure out what I was planning to do.

 

[OrionVeteran]

Oops, I got sidetracked and then I completely forgot about this
I should probably try to look into this shortly. After all I just spent time rereading the whole thread to figure out what I was planning to do.

No worries, as I am a patient man.