Modmodding Q&A Thread

I'm trying to make it so immigration to the new world has a chance of spawning a worker, but I keep getting these errors:

Code:
Traceback (most recent call last):
  File "BugEventManager", line 400, in _handleDefaultEvent
  File "CvRFCEventHandler", line 663, in onBeginGameTurn
  File "UniquePowers", line 63, in checkTurn
  File "UniquePowers", line 368, in checkImmigration
  File "UniquePowers", line 458, in doImmigration
  File "RFCUtils", line 184, in makeUnit
TypeError: unindexable object

The code I added is in UniquePowers.py in the function def doImmigration(self) between the code for spreading religion and the one for notifying the player, and looks like so:

Code:
            # chance to spawn a worker
            if (gc.getGame().getGameTurn() <= getTurnForYear(1950)):
                iRandom = gc.getGame().getSorenRandNum(1, 'random Worker spawn')
                if iRandom == 0:
                    utils.makeUnit(iWorker, iTargetPlayer, targetPlot, 1)

The last line is what the game seems to have an issue with. Am I not using that function correctly? Both iTargetPlayer and targetPlot are defined a bit above in the function already:
Code:
            targetPlot = gc.getMap().plot(x, y)
            iTargetPlayer = targetCity.getOwner()
 
The tPlot argument of utils.makeUnit() should be a tuple containing the x and y coordinates. You have a plot object as an argument. You could use the code below.

Code:
utils.makeUnit(iWorker, iTargetPlayer, (x, y), 1)
 
Thanks, worked like a charm!

Onward to my next indexing issue!

I'm trying to add a conquest event for Persia against Babylonia and Egypt but keep getting the error "IndexError: list index out of range".

I pretty much copied and pasted the code for Alexander's conquests in AIWars.py and added the two new events to the end of lConquests:

Code:
iPersiaYear = -530
tPersiaMesopotamiaTL = (74, 37)
tPersiaMesopotamiaBR = (78, 45)
tPersiaEgyptTL = (65, 32)
tPersiaEgyptBR = (73, 38)

tConquestPersiaMesopotamia = (13, iPersia, iBabylonia, tPersiaMesopotamiaTL, tPersiaMesopotamiaBR, 2, iPersiaYear, 20)
tConquestPersiaEgypt = (14, iPersia, iEgypt, tPersiaEgyptTL, tPersiaEgyptBR, 2, iPersiaYear, 20)

lConquests = [tConquestRomeCarthage, tConquestRomeGreece, tConquestRomeAnatolia, tConquestRomeCelts, tConquestRomeEgypt, tConquestGreeceMesopotamia, tConquestGreeceEgypt, tConquestGreecePersia, tConquestCholaSumatra, tConquestSpainMoors, tConquestTurksPersia, tConquestTurksAnatolia, tConquestMongolsPersia, tConquestPersiaMesopotamia, tConquestPersiaEgypt]

I also updated iNumConquests in Consts.py from 13 to 15.

Is there something else I overlooked that needs to be edited or did I do something wrong?
 
Can you see which line of code causes the error?

Could it be the line below?
Code:
        if data.lConquest[iID]: return
Did you code the new conqueror events and loaded a Persian savegame to test? (to skip the autoplay) If this is the case, I think the error is caused because data.lConquest has still length 13. iNumConquests determines the length of the list during initialization of a game. If you change that value after initialization it will not change the length of the list of savegames. Starting a new game will solve this.
 
I got the scrolling scoreboard kind of working, but it has some issues with spacing, especially when BUG's advanced scoreboard is enabled. I don't suppose anyone could check and let me know what I did wrong? I honestly have no clue how this UI stuff works.
 

Attachments

I realized I should have included screenshots and the code I suspect is where a fix can be made.

upload_2019-12-29_21-25-31.png


This is with BUG scoreboard off, it's mostly working, just has an issue where the scoreboard is populated from the bottom rather than the top.

Code:
line 994 of CvMainInterfacePLE under interfaceScreen
#screen.setText( szName, "Background", u"", CvUtil.FONT_RIGHT_JUSTIFY, 996, 622, -0.3, FontTypes.SMALL_FONT, WidgetTypes.WIDGET_CONTACT_CIV, i, -1 )
screen.setTextAt( szName, "ScoreBackground", u"", CvUtil.FONT_RIGHT_JUSTIFY, 0, 0, -0.3, FontTypes.SMALL_FONT, WidgetTypes.WIDGET_CONTACT_CIV, i, -1 )

line 4965 under updateInfoPaneStrings
#screen.setText( szName, "Background", szBuffer, CvUtil.FONT_RIGHT_JUSTIFY, xResolution - 12, yCoord - (iCount * iBtnHeight), -0.3, FontTypes.SMALL_FONT, iWidgetType, iPlayer, -1 )
screen.setTextAt( szName, "ScoreBackground", szBuffer, CvUtil.FONT_RIGHT_JUSTIFY, 0, 0 - (iBtnHeight* iCount) + yResolution, -0.3, FontTypes.SMALL_FONT, iWidgetType, ePlayer, -1 )

line 1072 of CvMainInterface under interfaceScreen
#screen.setText( szName, "Background", u"", CvUtil.FONT_RIGHT_JUSTIFY, 996, 622, -0.3, FontTypes.SMALL_FONT, WidgetTypes.WIDGET_CONTACT_CIV, i, -1 )
screen.setTextAt( szName, "ScoreBackground", u"", CvUtil.FONT_RIGHT_JUSTIFY, 0, 0, -0.3, FontTypes.SMALL_FONT, WidgetTypes.WIDGET_CONTACT_CIV, i, -1 )

line 5359 under updateScoreStrings
#screen.setText( szName, "Background", szBuffer, CvUtil.FONT_RIGHT_JUSTIFY, xResolution - 12, yCoord - (iCount * iBtnHeight), -0.3, FontTypes.SMALL_FONT, iWidgetType, iPlayer, -1 )
screen.setTextAt( szName, "ScoreBackground", szBuffer, CvUtil.FONT_RIGHT_JUSTIFY, 0, 0 - (iBtnHeight* iCount) + yResolution, -0.3, FontTypes.SMALL_FONT, iWidgetType, ePlayer, -1 )
upload_2019-12-29_21-33-29.png


meanwhile bug doesn't seem to replace the default scoreboard

upload_2019-12-29_21-42-12.png


but more importantly the spacing is off and all the icons are in the top left corner. This is of a much higher priority than the others, and is also the one that stumps me the most. I have a feeling I may have missed something I need to change somewhere, or perhaps that I made an incorrect assumption in assuming the only two setText(name, "Background", etc in MainInterface correlate to the only two in Scoreboard.

Code:
line 428 of BUG/scoreboard under draw column.type = FIXED
#screen.setText( name, "Background", value, CvUtil.FONT_RIGHT_JUSTIFY, x, y - p * height, Z_DEPTH, FontTypes.SMALL_FONT, *widget )
screen.setTextAt( name, "ScoreBackground", value, CvUtil.FONT_RIGHT_JUSTIFY, 0, 0, Z_DEPTH, FontTypes.SMALL_FONT, *widget )

line 480 under draw column.type = DYNAMIC
#screen.setText( name, "Background", value, align, x - adjustX, y - p * height, Z_DEPTH, FontTypes.SMALL_FONT, *widget )
screen.setTextAt( name, "ScoreBackground", value, align, 0, 0 - (p * height) + y, Z_DEPTH, FontTypes.SMALL_FONT, *widget )
 

Attachments

  • upload_2019-12-29_21-42-10.png
    upload_2019-12-29_21-42-10.png
    438.5 KB · Views: 240
A question about some theory about programming. I stumbled on this function in CvCity.cpp. As you can see, there are for-loops that loop over all buildinginfos. One for changing health and one for changing happiness. Obviously both could be merged so the loop over buildinginfos only need to be executed once. This requires some additional integers to be declared. (iGoodValue needs to be split up into iGoodHealthValue and iGoodHappinessValue.)

I assume that the benefits of merging both loops outweights the overhead of the declaring additional integers and therefore it is good coding practice to merge loops like this. Is this correct though?
I will make a small PR is this is indeed beneficial. I'm pretty sure there are more examples of this in the code, but I will not actively search for them. If I accidentally stumble upon them (like this case) I will make a small PR.

Code:
void CvCity::processBonus(BonusTypes eBonus, int iChange)
{
    int iI;

    changePowerCount((getBonusPower(eBonus, true) * iChange), true);
    changePowerCount((getBonusPower(eBonus, false) * iChange), false);

    for (iI = 0; iI < NUM_YIELD_TYPES; iI++)
    {
        changeBonusYieldRateModifier(((YieldTypes)iI), (getBonusYieldRateModifier(((YieldTypes)iI), eBonus) * iChange));
    }

    for (iI = 0; iI < NUM_COMMERCE_TYPES; iI++)
    {
        changeBonusCommerceRateModifier(((CommerceTypes)iI), (getBonusCommerceRateModifier(((CommerceTypes)iI), eBonus) * iChange));
    }
   
    int iValue;
    int iGoodValue;
    int iBadValue;
   
    iGoodValue = 0;
    iBadValue = 0;
   
    for (iI = 0; iI < GC.getNumBuildingInfos(); iI++)
    {
        iValue = GC.getBuildingInfo((BuildingTypes) iI).getBonusHealthChanges(eBonus) * getNumActiveBuilding((BuildingTypes)iI);

        if (iValue >= 0)
        {
            iGoodValue += iValue;
        }
        else
        {
            iBadValue += iValue;
        }
    }

    changeBonusGoodHealth(iGoodValue * iChange);
    changeBonusBadHealth(iBadValue * iChange);
   
   
    iGoodValue = 0;
    iBadValue = 0;

    for (iI = 0; iI < GC.getNumBuildingInfos(); iI++)
    {
        iValue = getNumActiveBuilding((BuildingTypes)iI) * GC.getBuildingInfo((BuildingTypes) iI).getBonusHappinessChanges(eBonus);

        if (iValue >= 0)
        {
            iGoodValue += iValue;
        }
        else
        {
            iBadValue += iValue;
        }
    }

    changeBonusGoodHappiness(iGoodValue * iChange);
    changeBonusBadHappiness(iBadValue * iChange);
}


The new code.
Spoiler :

Code:
void CvCity::processBonus(BonusTypes eBonus, int iChange)
{
    int iI;

    changePowerCount((getBonusPower(eBonus, true) * iChange), true);
    changePowerCount((getBonusPower(eBonus, false) * iChange), false);

    for (iI = 0; iI < NUM_YIELD_TYPES; iI++)
    {
        changeBonusYieldRateModifier(((YieldTypes)iI), (getBonusYieldRateModifier(((YieldTypes)iI), eBonus) * iChange));
    }

    for (iI = 0; iI < NUM_COMMERCE_TYPES; iI++)
    {
        changeBonusCommerceRateModifier(((CommerceTypes)iI), (getBonusCommerceRateModifier(((CommerceTypes)iI), eBonus) * iChange));
    }
   
    int iHealth;
    int iGoodHealth = 0;
    int iBadHealth = 0;
    int iHappiness;
    int iGoodHappiness = 0;
    int iBadHappiness = 0;
   
    for (iI = 0; iI < GC.getNumBuildingInfos(); iI++)
    {
        iHealth = GC.getBuildingInfo((BuildingTypes) iI).getBonusHealthChanges(eBonus) * getNumActiveBuilding((BuildingTypes)iI);

        if (iHealth >= 0)
        {
            iGoodHealth += iHealth;
        }
        else
        {
            iBadHealth += iHealth;
        }
       
        iHappiness = getNumActiveBuilding((BuildingTypes)iI) * GC.getBuildingInfo((BuildingTypes) iI).getBonusHappinessChanges(eBonus);

        if (iHappiness >= 0)
        {
            iGoodHappiness += iHappiness;
        }
        else
        {
            iBadHappiness += iHappiness;
        }
    }
    changeBonusGoodHealth(iGoodHealth * iChange);
    changeBonusBadHealth(iBadHealth * iChange);

    changeBonusGoodHappiness(iGoodHappiness * iChange);
    changeBonusBadHappiness(iBadHappiness * iChange);
}
 
Last edited:
Generally, those small differences are not worth thinking about. Obviously, 2*n and n*2 is the same complexity, so it doesn't matter directly in which order the loops are executed. So you're down to the overhead costs of additional variables versus additional loops. I don't know which actually wins here, but it's so miniscule that the better option will not be significant.

On the other hand, another aspect to keep in mind here is clean code. It's also important that code remains legible and elegant if performance allows for it. And from that perspective I prefer the current version because it is more concise.
 
I'm currently trying to remove all hardcoded stuff from CvRhyes.cpp in RFGW (which is based on vanilla RFC), and I stumbled upon this:
Code:
int turnPlayed[33] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
CvPlayer::doTurn() reads:
Code:
//Rhye - start
if (turnPlayed[getID()] == 1)
  return;
//Rhye - end
turnPlayed[getID()] is set to 1 at the end of the function. Apparently it is changed back to 0 in CvGame::doTurn():
Code:
//Rhye - start
for (iI = 0; iI < MAX_PLAYERS; iI++)
  turnPlayed[iI] = 0;
//Rhye - end
So I checked the code in DoC, and it looks like this array has been replaced with a boolean variable in CvPlayer, which otherwise functions identically. I guess this was made to ensure that doTurn isn't called twice in a turn...
But my question is: why is this necessary at all? This is the most confusing thing out of all of Rhye's code I've seen so far.
 
Good question. I think I tried to remove it when doing something similar to what you're working on, but must have run into some kind of issue, otherwise I wouldn't have kept it. Unfortunately I do not remember what the problem was.

I think a lot of the weird unexplainable code in those core functions is there to facilitate the RFC autoplay, which unfortunately is the part of the mod I understand least (which may be related to its haphazard and poorly documented implementation). I am pretty sure there is an easier or at least more succinct way to get autoplay to work like RFC needs it, and I want to explore that soon. I hope I become more knowledgeable about autoplay by that time.
 
I think a lot of the weird unexplainable code in those core functions is there to facilitate the RFC autoplay, which unfortunately is the part of the mod I understand least (which may be related to its haphazard and poorly documented implementation). I am pretty sure there is an easier or at least more succinct way to get autoplay to work like RFC needs it, and I want to explore that soon. I hope I become more knowledgeable about autoplay by that time.
I don't think the whole catapult thing is needed, either. This is what I found after further investigation:
Turns out the game disables autoplay every turn if it's enabled in CvGame::doTurn():
Code:
if (getAIAutoPlay() > 0)
{
  changeAIAutoPlay(-1);
  if (getAIAutoPlay() == 0)
  {
    reviveActivePlayer();
  }
}
This automatically spawns unit no. 0 at CvGame::reviveActivePlayer(), and thus respawns the player:
Code:
GET_PLAYER(getActivePlayer()).initUnit(((UnitTypes)0), 0, 0);
And here's where it gets weird: instead of simply commenting out the changeAIAutoPlay(-1) part, Rhye decided to create... well... this:
Spoiler :

Code:
//Rhye - start
void CvGame::setAIAutoPlayCatapult(int iNewValue)
{
   int iOldValue;

   iOldValue = getAIAutoPlay();

   if (iOldValue != iNewValue)
   {
       m_iAIAutoPlay = std::max(0, iNewValue);

       if ((iOldValue == 0) && (getAIAutoPlay() > 0))
       {
           CvPlot* pPlot = GC.getMapINLINE().plotINLINE(0, 0);
           if (pPlot->isUnit()) {
               GC.getMapINLINE().plotINLINE(0, 0)->getUnitByIndex(0)->kill(false);
               for (int iI = 0; iI < MAX_PLAYERS; iI++)
               {
                   if (GET_PLAYER((PlayerTypes)iI).isHuman())
                   {
                       GC.getMapINLINE().plotINLINE(0, 0)->setRevealed(GET_PLAYER((PlayerTypes)iI).getTeam(), false, false, NO_TEAM, true);
                       GC.getMapINLINE().plotINLINE(0, 1)->setRevealed(GET_PLAYER((PlayerTypes)iI).getTeam(), false, false, NO_TEAM, true);
                       GC.getMapINLINE().plotINLINE(1, 0)->setRevealed(GET_PLAYER((PlayerTypes)iI).getTeam(), false, false, NO_TEAM, true);
                       GC.getMapINLINE().plotINLINE(1, 1)->setRevealed(GET_PLAYER((PlayerTypes)iI).getTeam(), false, false, NO_TEAM, true);
                       GC.getMapINLINE().plotINLINE(GC.getMapINLINE().getGridWidthINLINE()-1, 0)->setRevealed(GET_PLAYER((PlayerTypes)iI).getTeam(), false, false, NO_TEAM, true);
                       GC.getMapINLINE().plotINLINE(GC.getMapINLINE().getGridWidthINLINE()-1, 1)->setRevealed(GET_PLAYER((PlayerTypes)iI).getTeam(), false, false, NO_TEAM, true);
                       GC.getMapINLINE().plotINLINE(2, 0)->setRevealed(GET_PLAYER((PlayerTypes)iI).getTeam(), false, false, NO_TEAM, true);
                       GC.getMapINLINE().plotINLINE(2, 1)->setRevealed(GET_PLAYER((PlayerTypes)iI).getTeam(), false, false, NO_TEAM, true);
                       GC.getMapINLINE().plotINLINE(2, 2)->setRevealed(GET_PLAYER((PlayerTypes)iI).getTeam(), false, false, NO_TEAM, true);
                       GC.getMapINLINE().plotINLINE(1, 2)->setRevealed(GET_PLAYER((PlayerTypes)iI).getTeam(), false, false, NO_TEAM, true);
                       GC.getMapINLINE().plotINLINE(0, 2)->setRevealed(GET_PLAYER((PlayerTypes)iI).getTeam(), false, false, NO_TEAM, true);
                       GC.getMapINLINE().plotINLINE(GC.getMapINLINE().getGridWidthINLINE()-1, 2)->setRevealed(GET_PLAYER((PlayerTypes)iI).getTeam(), false, false, NO_TEAM, true);
                       GC.getMapINLINE().plotINLINE(GC.getMapINLINE().getGridWidthINLINE()-2, 2)->setRevealed(GET_PLAYER((PlayerTypes)iI).getTeam(), false, false, NO_TEAM, true);
                       GC.getMapINLINE().plotINLINE(GC.getMapINLINE().getGridWidthINLINE()-2, 1)->setRevealed(GET_PLAYER((PlayerTypes)iI).getTeam(), false, false, NO_TEAM, true);
                       GC.getMapINLINE().plotINLINE(GC.getMapINLINE().getGridWidthINLINE()-2, 0)->setRevealed(GET_PLAYER((PlayerTypes)iI).getTeam(), false, false, NO_TEAM, true);
                   }
               }
           }
           else {
               logMsg("NO UNIT IN 0,0!!!"); //Rhye
           }
       }
   }
}


setAIAutoPlayCatapult(1) is called at CvGame::update() if the player's starting turn is greater than the current turn, except if the game is at turn zero, in which case setAIAutoPlay(1) is called. This of course doesn't happen if the player's starting turn is 0.

So if I understand the whole thing correctly, here's what happens during autoplay:
1. m_iAIAutoPlay is set to 1 at turn 0, starting the whole process (except if the human player starts at turn 0)
2. m_iAIAutoPlay is set to 0 because m_iAIAutoPlay's value is greater than 0
3. m_iAIAutoPlay is set to 0 again, just to be sure. Also, all units/cities of the player are destroyed
4. The human player gets a unit at 0,0
5. m_iAIAutoPlay is set to 1 (the first and only argument of setAIAutoPlayCatapult is always 1, thus m_iAIAutoPlay's value is changed to 1)
6. The unit at 0,0 is killed
7. The game hides all plots revealed by this unit
8. Jump to step 2 next turn if the player hasn't spawned yet

I threw together a simple DLL just to try, and indeed, steps 2 to 8 don't seem to be necessary at all. I simply removed the first block of code I mentioned, then called setAIAutoPlay(1) at the beginning of the game and this every turn (after updateTurnTimer() at CvGame::update()):
Code:
void CvRiseFall::checkTurn() {
   CvGame& game = GC.getGameINLINE();
   for (int i = 0; i<GC.getMAX_PLAYERS(); i++) {
       CvPlayer& player = GET_PLAYER((PlayerTypes) i);
       if(!player.isAlive() && !player.getSpawned()) {
           if(player.getStartingYear() <= game.getGameTurnYear()) {
               player.initUnit(((UnitTypes)0), 10, 10);
               if(player.isHuman()) {
                   game.setAIAutoPlay(0);
               }
               player.setSpawned(true);
           }
       }
   }
}
Looks like it works, just have to make sure at least two civs are alive at the beginning to avoid conquest victory (which is true in RFC as well, I think). And of course this is just a proof of concept so it still needs a lot of work (exclude minor civs, spawn correct units, etc).
So in the end I don't think this really has anything to do with the code I mentioned in my previous post. I'm considering replacing the RFC DLL with a new one anyways, possibly starting with BBAI or K-Mod. That might actually be easier than cleaning up all of Rhye's weird code (and the bugs stemming from it, especially since I couldn't get a debugger working with wine).
 
Thanks, that already helps for when I plan to clean this up. Good luck on a DLL merge, I would recommend Advanced Civ instead of K-Mod though, it's basically the successor project and its changes are very well documented in case you want to pick and choose.
 
Question: How do I merge the Python exception stack trace dialog into one dialog box only? It's really annoying that whenever a Python exception occurs, the stack trace is displayed one line per dialog box. It makes debugging a lot more hassle.
 
Question: How do I merge the Python exception stack trace dialog into one dialog box only? It's really annoying that whenever a Python exception occurs, the stack trace is displayed one line per dialog box. It makes debugging a lot more hassle.
I don't think there's a way to do that, but you can always check Documents/My Games/Beyond the Sword/Logs/PythonErr.log if you need the full stack trace.
 
I've asked around, nobody seems to know a way to do that.
 
Hey, random question, does the current version of the mod allow the human player to collapse to core each time, and if not, what would I need to change to allow it? Thanks!
 
Hey, random question, does the current version of the mod allow the human player to collapse to core each time, and if not, what would I need to change to allow it? Thanks!
Assets/Python/Stability.py
Code:
# help overexpanding AI: collapse to core, unless fall date
if utils.getHumanID() != iPlayer:
if gc.getGame().getGameTurnYear() < tFall[iPlayer]:
if len(utils.getOwnedCoreCities(iPlayer)) < len(utils.getCityList(iPlayer)):
collapseToCore(iPlayer)
return

scheduleCollapse(iPlayer)
def triggerCollapse(iPlayer):

replace the line
Code:
if utils.getHumanID() != iPlayer:
with
Code:
if True:
 
I'm trying to find the courage to do my first modding attempt (aside from drawing maps in civ II a long long time ago)

If I'd want to change the UP of the Inca (from every worked mountaintile giving two food and one production into giving two food and one free citizen). What parts of the code would I have to change for that?
And what programs would I need to acces that code? (/find it?)
 
Back
Top Bottom