v40.1 patch

[billw2015]

Project here: https://github.com/caveman2cosmos/Caveman2Cosmos/projects/12

This should be fixes and perf improvements only, I don't think we want to put new gameplay features in a patch. Not sure about balance changes, I would guess not though.

I made a branch on git for this patch. Ideally we should put fixes directly into it and merge them back to master.
But we can also "cherry-pick" any commits in master into that branch as well (like a merge but it only includes changes from one commit, not all commits that haven't been merged yet). Regardless I can do that if other people are unsure about it.

If you have specific things you want in it then note them here, or add them to the project itself. Especially if you already put them into master and want me to add them to the patch then note it here so I know about it!

We should probably have some sort of cut off point where we release what fixes we have so far. I would suggest maybe 2 months from now would be a good target. It should be enough time for existing crashes and bugs to shake out, and for us to fix them. Of course if there is something serious then we should release it sooner.

 

[billw2015]

All further text fixes and graphics icon improvements should go in.

 

[JosEPh_II]

I posted this elsewhere but will mention it here as well. When you make commits Please make sure your Voice Capture Option is turned Off Before you commit.

As it is now, for those that do Not use Voice Capture and have the Option Off, you have to go thru 3 popups about Voice Capture and it not finding a proper device. Next Time I load up the mod I will get a screenshot.

We had this problem in the past when a modder would have that Option On and make a commit. Then everyone that does Not use the Voice Capture has to clear out at least 3 popups at game start. Extremely annoying.

 

[raxo2222]

This is vanilla bug, where vanilla BTS ini file gets reset.

I know, that ini file may get reset, when you close game when its loading.

 

[JosEPh_II]

This is vanilla bug, where vanilla BTS ini file gets reset.

I know, that ini file may get reset, when you close game when its loading.

I don't close the game when it's loading. This came with the v40 Release.

 

[raxo2222]

I don't close the game when it's loading. This came with the v40 Release.

Well your vanilla settings reset. This happens randomly.

Change this in vanilla BTS ini:
; Enable voice over IP capture and playback
EnableVoice = 1

to
EnableVoice = 0

If aliens or spirits exist, then they possessed your computer(s)

You can check exactly what was changed, if you click on TortoiseSVN -> Show Logs (for SVN) or just by looking at Github Desktop or Git Kraken (for github)

Also didn't you reinstall windows recently? Maybe you forgot to copy vanilla INI file?

 

[Thunderbrd]

I posted this elsewhere but will mention it here as well. When you make commits Please make sure your Voice Capture Option is turned Off Before you commit.

As it is now, for those that do Not use Voice Capture and have the Option Off, you have to go thru 3 popups about Voice Capture and it not finding a proper device. Next Time I load up the mod I will get a screenshot.

We had this problem in the past when a modder would have that Option On and make a commit. Then everyone that does Not use the Voice Capture has to clear out at least 3 popups at game start. Extremely annoying.

It really does have nothing to do with the mod. As I've explained previously, some systems are particularly bad at automatically reverting the .config settings to defaults, as Raxo is trying to explain happened.

 

[billw2015]

Thanks for spamming up this thread with totally unrelated stuff!

 

[JosEPh_II]

Thanks for spamming up this thread with totally unrelated stuff!

You are most welcome.

PS, get used to it! This Team does this ALL the time.

 

[MattCA]

The only thing that might be important is the thing I already mentioned, changeRouteChange() on CvTeam recalc.

There's a function where all those data containers are zeroed out in the recalc process

I had to look around for a min but that is done inside the CvTeam recalc function at the very beginning. I checked for routeChange and didn't find it.
I will post a fix if nobody else is interested.

The rest doesn't really belong in this thread, all minor stuff I think.

Spoiler Lake plots are not considered fresh water :

Looks to me like it's the isFreshWater() function.

Spoiler CvPlot.cpp :

bool CvPlot::isFreshWater(bool bIgnoreJungle) const
{
    CvPlot* pLoopPlot;
    int iDX, iDY;

    if (isWater())
    {
        return false;
    }

    if (isImpassable())
    {
        return false;
    }

    if (isRiver())
    {
        return true;
    }

    for (iDX = -1; iDX <= 1; iDX++)
    {
        for (iDY = -1; iDY <= 1; iDY++)
        {
            pLoopPlot    = plotXY(getX_INLINE(), getY_INLINE(), iDX, iDY);

            if (pLoopPlot != NULL)
            {
                if (pLoopPlot->isLake())
                {
                    return true;
                }

                if (pLoopPlot->getFeatureType() != NO_FEATURE)
                {
                    if (GC.getFeatureInfo(pLoopPlot->getFeatureType()).isAddsFreshWater() && (!bIgnoreJungle || GC.getFeatureInfo(pLoopPlot->getFeatureType()).getHealthPercent() >= 0))
                    {
                        return true;
                    }
                }

                if (pLoopPlot->isCity())
                {
                    CvCity* pCity = pLoopPlot->getPlotCity();
                    if (pCity->hasFreshWater())
                    {
                        return true;
                    }
                }
            }
        }
    }

    return false;
}

You can see when it loops, if an adjacent plot is a lake then freshwater is true but the plot is never checked itself for a lake.
Just throw in if (isLake()) { return true; } first, before isWater() and it should be all good.

The next 1 is just for info. I'm sure me and toffer could find code he likes, it's just figuring out what went wrong originally that will temporarily delay things.

Spoiler No unit is selected at gamestart (issue from github) :

I added between isHuman(): and else:
I put the code there (CvEventManager.py line 636) to share a few bits but then it's behind that bPrehistoricStart... So it might be better to make a separate loop. idk
Spoiler :

if bPrehistoricStart:
for iPlayer in xrange(self.MAX_PC_PLAYERS):
CyPlayer = GC.getPlayer(iPlayer)
CyUnit, i = CyPlayer.firstUnit(False)
if CyPlayer.isHuman():
while CyUnit:
if CyUnit.isFound():
CyUnit.setHasPromotion(self.PROMO_GUARDIAN_TRIBAL, True)
CyInterface().insertIntoSelectionList(CyUnit, False, False, False, False)
CyUnit, i = CyPlayer.nextUnit(i, False)
else:
while CyUnit:
if CyUnit.isFound():
CyUnit.setHasPromotion(self.PROMO_GUARDIAN_TRIBAL, True)
break
CyUnit, i = CyPlayer.nextUnit(i, False)

I feel a dll solution would be more appropriate in this case, more direct
Vanilla didn't use python to select a unit after loading/starting a game, which indicates that our dll probably has a bug that is causing this behaviour and it would be better to fix that bug than to hide it behind a python workaround.

The bug isn't hidden now, as the bug can easily be experienced, and it is probably not a python bug as python never handled this aspect before, but this python workaround will hide it. I don't like fixing bugs indirectly like that.
Adding this new autoselect feature because there is a dll bug that stops a unit from being auto-selected may hide a potentially bigger bug where this behaviour was the only easily visible symptom of something being wrong.
I'm not saying the bug cause more issues than just units not being autoselected, but we don't know that, it could have other, difficult to spot, symptoms that this python workaround will not fix.
I'm just saying it's imo bad practice to fix this bug the way you suggest.

A direct bug fix is one where you can point at the root of the bug and why the fix address it.
An indirect bug fix is when the fix apparently doesn't address the cause of the bug and is therefore pretty much per definition a workaround.

A few months ago somebody posted about the leader of one of the animal NPC civs becoming the leader of a new civ that emerged while using Revolutions. Yudishtra then posted he'd seen the same.

Spoiler :

        if (GC.getGameINLINE().isOption(GAMEOPTION_RANDOM_PERSONALITIES))
        {
            if (!isNPC() && !isMinorCiv())
            {
                iBestValue = 0;
                eBestPersonality = NO_LEADER;

                for (iI = 0; iI < GC.getNumLeaderHeadInfos(); iI++)
                {
                    if (iI != GC.getDefineINT("BARBARIAN_LEADER")) // XXX minor civ???
                    {
                        iValue = (1 + GC.getGameINLINE().getSorenRandNum(10000, "Choosing Personality"));

                        for (iJ = 0; iJ < MAX_PC_PLAYERS; iJ++)
                        {
                            if (GET_PLAYER((PlayerTypes)iJ).isAlive())
                            {
                                if (GET_PLAYER((PlayerTypes)iJ).getPersonalityType() == ((LeaderHeadTypes)iI))
                                {
                                    iValue /= 2;
                                }
                            }
                        }

                        if (iValue > iBestValue)
                        {
                            iBestValue = iValue;
                            eBestPersonality = ((LeaderHeadTypes)iI);
                        }
                    }
                }

                if (eBestPersonality != NO_LEADER)
                {
                    setPersonalityType(eBestPersonality);
                }
            }
        }

There are 3 places like this. It checks for barbarian leader and if leader is used by living player. That just leaves insectoides and something else I think.

And finally, I think this += should be an =. This appears a couple times in the code.

Spoiler :

                    int iMax = MAX_INT;
                    if (!GC.getGameINLINE().isOption(GAMEOPTION_INFINITE_XP))
                    {
                        iMax += (GC.getDefineINT("BARBARIAN_MAX_XP_VALUE"));
                    }
                    changeExperience(GC.getDefineINT("MIN_EXPERIENCE_PER_COMBAT"), bBarb ? iMax : -1, !bBarb, pCity->getOwnerINLINE() == getOwnerINLINE());

 

[Thunderbrd]

I had to look around for a min but that is done inside the CvTeam recalc function at the very beginning. I checked for routeChange and didn't find it.
I will post a fix if nobody else is interested.

There are similar functions in CvPlayer, CvPlot, etc (not CvUnit). Make sure that the Cv equates to the header file that the routeChange data is declared. If it's declared in CvPlayer.h then in CvPlayer.cpp is where you'll find the zeroing function where the routechange variable is supposed to be zeroed out. Of course, you probably won't find it already there because that's the bug in the first place right? The next thing would be to ensure that the reconstruction part of the recalculation process will recompile that data properly as well. I think it naturally IS, and by doing so on top of an uncleared variable is how this becomes a bug.

Point is make sure the variable you're clearing is declared in the equivalent header file.

 

[Thunderbrd]

And finally, I think this += should be an =. This appears a couple times in the code.

I think you're right. Good catch!

 

[billw2015]

all minor stuff

Doesn't matter, the only qualification for going in the patch is that it be a fix and not a gameplay change.
I also really want to get every last known bug into the github bug tracker, so if you have some list somewhere of things you know about please post them somewhere (here is fine), or even add them to the tracker directly if you want to be extra helpful!

 

[MattCA]

There are similar functions in CvPlayer, CvPlot, etc (not CvUnit). Make sure that the Cv equates to the header file that the routeChange data is declared. If it's declared in CvPlayer.h then in CvPlayer.cpp is where you'll find the zeroing function where the routechange variable is supposed to be zeroed out.

The member is named m_paiRouteChange and it is in CvTeam but I went to check CvPlayer anyways to see if somehow there's something that clears it and there's actually no recalc function in CvPlayer.

Of course, you probably won't find it already there because that's the bug in the first place right? The next thing would be to ensure that the reconstruction part of the recalculation process will recompile that data properly as well. I think it naturally IS, and by doing so on top of an uncleared variable is how this becomes a bug.

Basic code to clear the array, just gotta add it to the beginning of CvTeam::recalculateModifiers() where everything else is cleared. After that, like you say, the current code is what you need to calc the new value.

    for (iI = 0; iI < GC.getNumRouteInfos(); iI++)
    {
        m_paiRouteChange[iI] = 0;
    }

The second part needed for a fix is about the python event that's been mentioned.
There is 1 tech that gives this bonus and the only other way is by completing that python event.
When the value is recalculated the bonus from the tech will be added but any bonuses from the event will not.

I also really want to get every last known bug into the github bug tracker, so if you have some list somewhere of things you know about please post them somewhere (here is fine), or even add them to the tracker directly if you want to be extra helpful!

I like to post any time I think I've found or solved a bug and the team always checks them out, so that's all I got. One other bug that I heard of long ago, and I think it happened to me once, is when a player sets themselves up to receive more then 1 free tech the next turn from techs or wonders. When the next turn begins and it's time to chose those free techs, players will only ever get 1.
So CvPlayer probably has a bool that needs to be changed to an int.
It would be good if someone can confirm that that's a bug and it hasn't been fixed.

 

[Thunderbrd]

The second part needed for a fix is about the python event that's been mentioned.
There is 1 tech that gives this bonus and the only other way is by completing that python event.
When the value is recalculated the bonus from the tech will be added but any bonuses from the event will not.

And the event may be able to fire repeatedly and build up too much reduction. I say comment out the event for now.

I like to post any time I think I've found or solved a bug and the team always checks them out, so that's all I got. One other bug that I heard of long ago, and I think it happened to me once, is when a player sets themselves up to receive more then 1 free tech the next turn from techs or wonders. When the next turn begins and it's time to chose those free techs, players will only ever get 1.
So CvPlayer probably has a bool that needs to be changed to an int.
It would be good if someone can confirm that that's a bug and it hasn't been fixed.

It's a bug and hasn't been fixed. I'm not sure its as simple as a bool that should be an int but I don't remember enough about that system to say.

 

[Toffer90]

And the event may be able to fire repeatedly and build up too much reduction. I say comment out the event for now.

Commenting out the event is ok by me, it is basically saying that a random time after researching Radio and Mass transit, improve the effectiveness of modern roads.

The event as it is currently defined can only trigger once per player, but if there's more than one player on a team, and since the event adjust the movement cost for the entire team, then this means that each player on the team can get this event and reduce the movement cost for all players on the team in a cumulative way. It could be fixed by setting the bGlobal tag on the eventTrigger, but this would mean that only one single team in a game can get the event and its benefit; I don't think this was what the event creator envisioned with the event.

The real bug, however, needs to be fixed.
The old xml value for route cost adjustments due to techs for teams needs to be reset to zero before the xml values are reapplied in the recalculation, it reapplies it by adding the xml value to the current value in the array.

There are loads of python code that change stuff in the game that could store these changes in scriptData, if the dll at the end of the recalc code makes a call to a python function (e.g. "doRecalc()") , then that would enable python to look through the script data and reapply those python changes that the dll recalc wiped. This would empower python to be able to do more creative stuff, that may be evaluated later for full xml and dll implementation.
I'm not saying I think python code should often do changes to game object stats, but it would be nice to tie the recalc to python so that python modders could actually consider such modifications.
Currently, these game effects will only be experienced by players who doesn't update C2C, as they won't have to do recalcs and a recalc wipe away these effects. These effects work just fine in that ideal scenario; but it would be nice for us who do recalcs often to be able to see these effects in our games too.

It's imo a shortcoming in the recalc mechanic that python isn't involved.

 

[Toffer90]

Btw, I don't think MattCA is part of the C2C team yet, I think it's time for this to be rectified.

He's showed enthusiasm, a growing insight into the mod, and that he communicates well with the team.

@Thunderbrd: Do you agree?

 

[alberts8]

Commenting out the event is ok by me, it is basically saying that a random time after researching Radio and Mass transit, improve the effectiveness of modern roads.

The event as it is currently defined can only trigger once per player, but if there's more than one player on a team, and since the event adjust the movement cost for the entire team, then this means that each player on the team can get this event and reduce the movement cost for all players on the team in a cumulative way. It could be fixed by setting the bGlobal tag on the eventTrigger, but this would mean that only one single team in a game can get the event and its benefit; I don't think this was what the event creator envisioned with the event.

The real bug, however, needs to be fixed.
The old xml value for route cost adjustments due to techs for teams needs to be reset to zero before the xml values are reapplied in the recalculation, it reapplies it by adding the xml value to the current value in the array.

There are loads of python code that change stuff in the game that could store these changes in scriptData, if the dll at the end of the recalc code makes a call to a python function (e.g. "doRecalc()") , then that would enable python to look through the script data and reapply those python changes that the dll recalc wiped. This would empower python to be able to do more creative stuff, that may be evaluated later for full xml and dll implementation.
I'm not saying I think python code should often do changes to game object stats, but it would be nice to tie the recalc to python so that python modders could actually consider such modifications.
Currently, these game effects will only be experienced by players who doesn't update C2C, as they won't have to do recalcs and a recalc wipe away these effects. These effects work just fine in that ideal scenario; but it would be nice for us who do recalcs often to be able to see these effects in our games too.

It's imo a shortcoming in the recalc mechanic that python isn't involved.

Why is it necessary to
modify the game object stats directly from python?

It would be possible to implement things like random event effects as free traits or buildings.

 

[Thunderbrd]

Btw, I don't think MattCA is part of the C2C team yet, I think it's time for this to be rectified.

He's showed enthusiasm, a growing insight into the mod, and that he communicates well with the team.

@Thunderbrd: Do you agree?

Yes, though at the moment I'm not sure quite what actions that means to take now on the Git service.

Why is it necessary to
modify the game object stats directly from python?

It would be possible to implement things like random event effects as free traits or buildings.

In most cases that's probably the way to go. What about things like an individual plot being given a random extra yield output?

 

[Thunderbrd]

The real bug, however, needs to be fixed.
The old xml value for route cost adjustments due to techs for teams needs to be reset to zero before the xml values are reapplied in the recalculation, it reapplies it by adding the xml value to the current value in the array.

From what I was reading in Matt's post, this is what he is applying at the recalcModifiers function in the .cpp file where the .h file has that variable declared.

Commenting out the event is ok by me, it is basically saying that a random time after researching Radio and Mass transit, improve the effectiveness of modern roads.

I'd far prefer to see this actually applied to a particular tech in there somewhere than be random, but I do think there may be a way to program a tag on the event that gets checked to see if it's part of the ledger for the 'rebuild' portion of the recalculation that we could look into.

This is how events can be made to work with recalculations nicely. Put the modification the event gives into a tag on the event itself and then add a check to the events that have taken place for the player and add the value of the tag in the second stage of recalculation.

@MattCA : Are you following how this would work? It would be awesome if you'd be willing to set all that up. I guess I'd like to see it done for this situation as a test of putting the method to practice.