OOS Candidates (4.0)

[devolution79]

CvPlayer:8454

        //WTP, ray, Settler Professsion - START
        //only if player is still alive
        if (!bKill)
        {
            // Only Colonial Players, but not Kings, not Natives, not Animals, ...
            if (getParent() != NO_PLAYER)
            {
                // we check that there is no War of Independence and other small things
                if(kParent.isAlive() && kParent.isEurope() && !::atWar(getTeam(), kParent.getTeam()) && (GC.getGameINLINE().getAIAutoPlay() == 0 || GC.getGameINLINE().getActivePlayer() != getID()))
                {
                    //we check if AI is landlocked and needs more cities
                    int iNumPlayerCities = getNumCities();
                    int iNumSettlers = AI_getNumAIUnits(UNITAI_SETTLER);
                    CvPlayerAI& kPlayer = GET_PLAYER(getID());
                    int iAIdesiredCities = kPlayer.AI_desiredCityCount();

                    // ONLY for AI: landlock check
                    if (!isHuman() && (iAIdesiredCities > (iNumPlayerCities + iNumSettlers)) && kPlayer.AI_isLandLocked())
                    {
                        //create a Unit in Profession Settler in Europe - having to pay equipment
                        initEuropeSettler(true);
                    }
                }
            }
        }
        //WTP, ray, Settler Professsion - END

[devolution79]

I suspect that cpp GC.getGameINLINE().getActivePlayer() != getID()

Will yield different results in MP since each human player will be different active players

The check will work in SP but in MP there will be multiple human players and thus we cannot safely use getActivePlayer() in this manner IMHO

[Nightinggale]

The code is refactored into CvPlayer::buyEuropeSettlerIfLandlockedAI() meaning it's now in line 8197.

I think this is ok because the full check is:

(GC.getGameINLINE().getAIAutoPlay() == 0 || GC.getGameINLINE().getActivePlayer() != getID())

This means if autoplay is not active or active player is not getID. In multiplayer autoplay is always off, hence first part is always true.

I will say that this check could be written in a more human readable manner. getAIAutoPlay could be bool isAutoplayActive or something like that.

[Nightinggale]

Looks like an easy fix. In compareUnitValue, if value A and B are equal, use getID instead. This way there shouldn't be any randomness to the order.

[devolution79]

False alarm, so I deleted the post (for some reason I thought we compared pointer addresses but this turned out to be a bit of a brainfart!)

[Nightinggale]

CvCity::setProductionAutomated // if automated and not network game and all 3 modifiers down, clear the queue and choose again

The function then proceeds to clear the queue without checking for either modifiers to network status.

[raystuttgart]

@Nightinggale Is this ticket still valid or can it be closed?