Fixing broken Asserts

[Afforess]

My source files could be pretty well described as convulted. Especially since I'm not the original owner, and it has a bunch of mod's and modcomps in it. (I believe it has RevDCM [RevDCM includes Better AI, UP, and IDW], Civic Flavors, WOC, a heavily modified Mountains, back to service, Route restrictor, Realistic Diplomacy, CAR, Multiple Production Mod, Event Images, Food threshold modifiers, and some leftovers from the previous owner too.) Well, the trouble arises when I try to use a debug DLL. It's nearly impossible for me to sort out what Asserts are real problems and which have been broken over time in the code and are no longer necessary. So I would appreciate anyone's help, telling me which of these need to stay and actually are giving me a real problem, and which are broken and need to be fixed or removed entirely.

Upon loading XML, Civ4 with my Debug DLL, I immediately receive a

Assert Failed

File: CvGlobals.cpp
Line: 3688
Expression: strcmp(szType, "NONE")==0 || strcmp(szType, "")==0
Message: type FLAVOR_MIILTARY not found

I'm not sure what this is trying to tell me, as I thought all flavor values were in the XML.
Fixed. Stupid Typos.

After selecting "Ignore Always", I get my next friendly error:

Assert Failed

File: CvGlobals.cpp
Line: 4059
Expression: iExisting==-1 || iExisting==idx || strcmp(szType, "ERROR")==0
Message: xml info type entry RELIGION_HELLENISM already exists

I'm not sure what this one is about either.

Moments later (were still "Init XML" folks) I recieve:
(Unmodded RevDCM's sources have this one too)

Assert Failed

File: CvXMLLoadUtilitySet.cpp
Line: 1529
Expression: bSuccess
Message: OWN TYPE - dependency not found: (null), in file: "NONE"

This one has me baffled as well. No ideas on what causes it.

Ignoring that, I finally get to the shiny spinning globe of earth. I select single player, the default map (aboria), Standard Size, Normal Speed, and try to generate a map.
Now the key word is try. I get another Asset immediately.
(Unmodded RevDCM's sources have this one too)

Assert Failed

File: c:\program files (x86)\firaxis games\sid meier's civilization 4\beyond the sword\cvgamecoredll\CvInfos.h
Line: 93
Expression: false
Message: Override this

I'm assuming "Override This" means ignore it. But why have an assert that you ignore? I thought the point was to tell you something was wrong.

Moments later, after ignoring that assert, I recieve this one:

Assert Failed

File: CvGlobals.cpp
Line: 4027
Expression: strcmp(szType, "NONE")==0 || strcmp(szType, "")==0
Message: info type IMPROVEMENT_SANITIZE_WATER not found, Current XML file is: xml\GameInfo/CIV4EspionageMissionInfo.xml

Improvement Sanitize_Water? I've never heard of it. Now the funny thing is with this assert is that if I select "ignore once", I just get again, meaning there are more than one error like this. I have to select "ignore always" to get past this assert.
Fixed. Python error.

And two asserts later, we're at the map. I select my settler and build my capital. Then, I get an assert.

Assert Failed

File: CvCity.cpp
Line: 11001
Expression: eIndex >= 0
Message: eIndex expected to be >= 0

This one baffles me almost as much as the "Override this" assert. If the eIndex is >= 0, but is EXPECTED to be >=0, why is there an assert firing? Or am I just crazy?

Ignoring that, my city settles. I tell my city to build a warrior, and pick Animal Husbandry to research. I play for 2 turns to ensure no errors. Then I save. Then, I reload my save. Then, I get an assert.

Assert Failed

File: CvTeam.cpp
Line: 3580
Expression: getAliveCount() >= 0
Message:

This assert has no message. Real helpful Firaxis. The expression says that the getAliveCount is greater than or equal to zero. I would hope so, as I'm assuming this mean the number of players that are alive are greater than 0. So why is this an assert?

Ignoring that, I immediately get another assert.

Assert Failed

File: CvGlobals.cpp
Line: 1458
Expression: eCivilizationNum > -1
Message:

Brilliant, another messageless assert. This one is telling me that eCivilizationNum is greater than -1. I should hope so. I'm assuming this means there are more than -1 civilizations, which would make sense, so why does this assert fire?

Then, after ignoring that assert, I get the best message of them all.

This program has encountered a problem and needs to be closed. Do you wish to save a diagnostic file?

Yes, Yes I do.

Edit:
After more playing around with a debug dll, I got a few more Asserts:

Assert Failed

File: CvGlobals.cpp
Line: 4027
Expression: strcmp(szType, "NONE")==0 || strcmp(szType, "")==0
Message: info type BUILDING_BOULEVARD_CIRCLE not found, Current XML file is: xml\GameInfo/CIV4EspionageMissionInfo.xml

Assert Failed

File: CvCity.cpp
Line: 5918
Expression: getBaseGreatPeopleRate() >= 0
Message:

Assert Failed

File: CvCity.cpp
Line: 7579
Expression: getBuildingDefense() >= 0
Message:

Assert Failed

File: CvPlayerAI.cpp
Line: 5588
Expression: ePlayer != getID()
Message: shouldn't call this function on ourselves

Assert Failed

File: CvCity.cpp
Line: 5432
Expression: eIndex != NO_BUILDING
Message: BuildingType eIndex is expected to not be NO_BUILDING

So, can anyone help me detirmine which asserts are telling me actual problems and which are false alarms?

 

[EmperorFool]

Let me start with a little background on why we have assertions in the first place and how to use them. Whenever some code is about to run in your program, there are often some input values that will cause the program to fail or work incorrectly.

• If you are going to access the eIndex'th element in an array, eIndex must be in the range [0, number of elements - 1].
• If you are going to spread eReligion to a city, eReligion must be valid: >= 0, < GC.getNumReligionInfos().
• When you look up a unit type from an XML key, it must exist.

In an ideal world, all input values could be assumed to be valid. Performing validity checks on input values can take CPU time, and if all values are valid, this is truly wasted time. Assertions allow you to put those checks in the code in such a way that they can be turned off easily with a compiler flag. You leave them on during development and testing, but you turn them off for releases after you've done enough testing to ensure they all pass.

Each assertion has a test that must evaluate to true and an optional message that tells the developer/tester what was expected. When you see that the "getAliveCount() >= 0)" assertion failed, it means this expression was false. You are interpreting them backwards. All those expressions are failing rather than being true.

One thing is very important: the message and expression are only related in that they occur near each other. It's perfectly valid to have a message that contradicts the test expression. Code context is very important. You need to look at the code where the assertion occurs and see if the test it is performing is still valid. From a cursory glance of those messages, my guess is that they are all still valid. But without seeing the code, I can only go off of the expressions and messages.

The "override this" assert is actually a function defined in a base class such as CvInfoBase that you are expected to override in your subclass, for example CvTechInfo. Find the assertion, see what function it is, and make sure every class that extends CvInfoBase defines its own function with the same signature.

 

[Afforess]

Each assertion has a test that must evaluate to true and an optional message that tells the developer/tester what was expected. When you see that the "getAliveCount() >= 0)" assertion failed, it means this expression was false. You are interpreting them backwards. All those expressions are failing rather than being true.

Oh. That explains my confusion. So the problem was that the AliveCount was < 0. That is bad.

One thing is very important: the message and expression are only related in that they occur near each other. It's perfectly valid to have a message that contradicts the test expression. Code context is very important. You need to look at the code where the assertion occurs and see if the test it is performing is still valid. From a cursory glance of those messages, my guess is that they are all still valid. But without seeing the code, I can only go off of the expressions and messages.

If they are all valid, then that means I have at least 8 major problems. Great. How would you advise that I begin fixing these?

The "override this" assert is actually a function defined in a base class such as CvInfoBase that you are expected to override in your subclass, for example CvTechInfo. Find the assertion, see what function it is, and make sure every class that extends CvInfoBase defines its own function with the same signature.

There is no CvInfosBase, unless you mean CvInfoCore, or some section of another file.

 

[EmperorFool]

If they are all valid, then that means I have at least 8 major problems. Great. How would you advise that I begin fixing these?

They appear valid with limited information. Without the code context I can only go off of the expressions and messages. For each, go to the place where they occur in the code and compare them against the code that follows.

Sometimes the assertion is no longer valid because the code the assumptions about the inputs have changed. Sometimes the function has been changed to handle a "bad" input without crashing.

FAssertMsg(pCity != NULL, "pCity must not be NULL");

[B]if (pCity == NULL)
{
    return 0;
}[/B]
... code that dereferences pCity and will crash if it is NULL ...

Here a non-assertion test was added to allow the function to work fine if pCity is NULL, but the assertion that says NULL is illegal remains. The fix here is to remove the old invalid assertion.

For assertions that test an input read from an XML file, make sure it's testing the correct thing. FLAVOR_MILITARY exists in the core BTS assets, right? The assertion expression is actually enforcing that the input value is "NONE" or "" (empty string). Now, this could be a final test to say, "if the code got to this point, the flavor string must be NONE or empty. If it's not, it must be an invalid flavor key."

There is no CvInfosBase, unless you mean CvInfoCore, or some section of another file.

I'm talking about the class CvInfoBase from CvInfos.h/cpp. Check out these two functions:

virtual bool readPass2(CvXMLLoadUtility* pXML) { pXML; FAssertMsg(false, "Override this"); return false; }
virtual bool readPass3() { FAssertMsg(false, "Override this"); return false; }

This is a common coding pattern to enforce that a subclass must override a function if they are going to call it. It is called "subclass responsibility". Look for a CvXXXInfo class that doesn't define readPass2() or readPass3() but does specify two-pass reading in XMLLoadUtilitySet.cpp. For example, in the unmodded SDK CvImprovementInfo is read using two passes:

LoadGlobalClassInfo(
    GC.getImprovementInfo(), 
    "CIV4ImprovementInfos", 
    "Terrain", 
    "Civ4ImprovementInfos/ImprovementInfos/ImprovementInfo", 
    [B]true[/B], 
    &CvDLLUtilityIFaceBase::createImprovementInfoCacheObject
);

Find this area in XMLLoadUtilitySet.cpp and check CvInfos.cpp for every class that passes "true" for "bTwoPass". Make sure they all define readPass2(). You're using WoC here, and it may define more functions that the subclass must override in certain cases.

 

[The_J]

To the first one:
Look, if FLAVOR_MIILTARY is mentioned in the XML\GlobalTypes.xml. If not, add it -> fixed.

 

[Afforess]

To the first one:
Look, if FLAVOR_MIILTARY is mentioned in the XML\GlobalTypes.xml. If not, add it -> fixed.

Pulled right from the XML file:

    <FlavorTypes>
        <FlavorType>FLAVOR_MILITARY</FlavorType>
        <FlavorType>FLAVOR_RELIGION</FlavorType>
        <FlavorType>FLAVOR_PRODUCTION</FlavorType>
        <FlavorType>FLAVOR_GOLD</FlavorType>
        <FlavorType>FLAVOR_SCIENCE</FlavorType>
        <FlavorType>FLAVOR_CULTURE</FlavorType>
        <FlavorType>FLAVOR_GROWTH</FlavorType>
        <FlavorType>FLAVOR_ESPIONAGE</FlavorType>
        <FlavorType>FLAVOR_LIBERTARIAN</FlavorType>
        <FlavorType>FLAVOR_AUTHORITARIAN</FlavorType>
        <FlavorType>FLAVOR_CENTRISM</FlavorType>
        <FlavorType>FLAVOR_RIGHT</FlavorType>
        <FlavorType>FLAVOR_LEFT</FlavorType>
    </FlavorTypes>

You can see some have been added, but Military is right there at the top.

 

[The_J]

Ah, a typo.
In the exception it's MIILTARY, not MILITARY, so there's somewhere an error in one xml.

 

[Afforess]

Ah, a typo.
In the exception it's MIILTARY, not MILITARY, so there's somewhere an error in one xml.

Wow, that pretty bad. I'm going to check through the XML and see if I can't find it.

 

[The_J]

I guess, it's better to ask Zappara .

 

[Afforess]

I guess, it's better to ask Zappara .

I reported it to him. I found the error in the xml. One tech had a misspelling. I guess he got tired one night. Completely understandable.

One Assert down, 7 to go.

 

[The_J]

mmmhh...okay, for IMPROVEMENT_SANITIZE_WATER:
Xienwolf has mentioned it somewhere, that everything, which is in a python file, but not more mentioned in the XML file, will show up as an error in CIV4EspionageMissionInfo.xml.
For example, in CvAdvisorUtils are the popups for first melee unit, first archer unit, etc. handled. If you remove this unitcombat, but don't remove it in the python file, you'll get an error about the unitcombat in CIV4EspionageMissionInfo.xml.
-> happy python searching .

 

[Afforess]

mmmhh...okay, for IMPROVEMENT_SANITIZE_WATER:
Xienwolf has mentioned it somewhere, that everything, which is in a python file, but not more mentioned in the XML file, will show up as an error in CIV4EspionageMissionInfo.xml.
For example, in CvAdvisorUtils are the popups for first melee unit, first archer unit, etc. handled. If you remove this unitcombat, but don't remove it in the python file, you'll get an error about the unitcombat in CIV4EspionageMissionInfo.xml.
-> happy python searching .

I wonder if it's this:

    #########################
    # Rise of Mankind start #
    #########################
    if (not gc.getPlayer(pCity.getOwner()).isFeatAccomplished(FeatTypes.FEAT_UNITCOMBAT_NAVAL)):

        if (pUnit.getUnitCombatType() == gc.getInfoTypeForString("UNITCOMBAT_WOODEN_SHIPS")):
        
            gc.getPlayer(pCity.getOwner()).setFeatAccomplished(FeatTypes.FEAT_UNITCOMBAT_NAVAL, True)
            
            if (featPopup(pCity.getOwner()) and (gc.getGame().getStartYear() == gc.getDefineINT("START_YEAR"))):
                popupInfo = CyPopupInfo()
                popupInfo.setButtonPopupType(ButtonPopupTypes.BUTTONPOPUP_PYTHON)
                popupInfo.setData1(FeatTypes.FEAT_UNITCOMBAT_NAVAL)
                popupInfo.setData2(pCity.getID())
                popupInfo.setText(localText.getText("TXT_KEY_FEAT_UNITCOMBAT_NAVAL", (pUnit.getNameKey(), pCity.getNameKey(), )))
                popupInfo.setOnClickedPythonCallback("featAccomplishedOnClickedCallback")
                popupInfo.setOnFocusPythonCallback("featAccomplishedOnFocusCallback")
                popupInfo.addPythonButton(localText.getText("TXT_KEY_FEAT_ACCOMPLISHED_OK", ()), "")
                popupInfo.addPythonButton(localText.getText("TXT_KEY_FEAT_ACCOMPLISHED_MORE", ()), "")
                popupInfo.addPopup(pCity.getOwner())
    #########################
    # Rise of Mankind end   #
    #########################

Because RoM has no UNITCOMBAT_NAVAL. I checked, they have been broken into wooden ships, Early Iron ships, Modern, and future ships.

EDIT: Upon second examination, the code doesn't seem to have any problems. Zappara just never renamed the functions but changed the getInfoClassForStrings. I'm not sure why there is a problem now...

 

[EmperorFool]

There are many tools that allow you to search a hierarchy of files with one command by you. Myself, I use Eclipse for the Python/XML work and Visual Studio for the C++. Both tools have a "Find in Files" command. I would bet Notepad++ has one as well.

 

[Afforess]

I would bet Notepad++ has one as well.

I use Notepad++ for everything, XML, Python and C++, (oh and for my Java HW.)
Yeah it does, I've used it before. It's called "Find in Files."

 

[EmperorFool]

No, this looks like it was modified so that the first wooden ship you built would trigger the feat accomplishment. Look more closely and you will see that it doesn't look for a CombatTypes called UNITCOMBAT_NAVAL. Instead if expects a FeatTypes called FEAT_UNITCOMBAT_NAVAL which it probably left unchanged in the SDK since there was no need to change it.

 

[Afforess]

No, this looks like it was modified so that the first wooden ship you built would trigger the feat accomplishment. Look more closely and you will see that it doesn't look for a CombatTypes called UNITCOMBAT_NAVAL. Instead if expects a FeatTypes called FEAT_UNITCOMBAT_NAVAL which it probably left unchanged in the SDK since there was no need to change it.

EDIT: Upon second examination, the code doesn't seem to have any problems. Zappara just never renamed the functions but changed the getInfoClassForStrings. I'm not sure why there is a problem now...

I noticed.

 

[EmperorFool]

What did you find when you searched all files for "IMPROVEMENT_SANITIZE_WATER"?

 

[Afforess]

What did you find when you searched all files for "IMPROVEMENT_SANITIZE_WATER"?

I search RoM's entire assets folder, it took a while, but I got exactly one result.

###AdvancedNukesbegin###
        pPlot = CyMap().plot(iX,iY)
        if(iImprovement==gc.getInfoTypeForString('IMPROVEMENT_SANITIZE_WATER')):
            pPlot.setTerrainType(gc.getInfoTypeForString( "TERRAIN_COAST" ), 1, 1)
            pPlot.setImprovementType(-1)
###AdvancedNukesend###

This is from Civ Fuehrer's modmod for RoM that adds more nukes. I've never taken a real close look at the code before.

 

[EmperorFool]

This is in Python code which doesn't run before the XML files are parsed, and I think you said above that you get this assert while the XML is being parsed.

Assert Failed

File: CvGlobals.cpp
Line: 4027
Expression: strcmp(szType, "NONE")==0 || strcmp(szType, "")==0
Message: info type IMPROVEMENT_SANITIZE_WATER not found, Current XML file is: xml\GameInfo/CIV4EspionageMissionInfo.xml

Are you sure you searched all of the XML files?

BTW, this assertion is raised inside getInfoTypeForString(), so it could be caused from Python under other circumstances.

 

[Afforess]

This is in Python code which doesn't run before the XML files are parsed, and I think you said above that you get this assert while the XML is being parsed.

Are you sure you searched all of the XML files?

BTW, this assertion is raised inside getInfoTypeForString(), so it could be caused from Python under other circumstances.

As I said earlier, I search all the files in RoM. I think it might be a file that RoM never changed, and in the BTS directory. I'm searching now.