LoneGazebo / Community-Patch-DLL

Community Patch for Civilization V - Brave New World
Other
285 stars 158 forks source link

roman UA #11071

Closed ilteroi closed 3 months ago

ilteroi commented 3 months ago
  1. in acquireCity() we have the following code:
        if (isMajorCiv() && GetPlayerTraits()->IsAnnexedCityStatesGiveYields())
        {
            ChangeNumAnnexedCityStates(GET_PLAYER(eOriginalOwner).GetMinorCivAI()->GetTrait(), 1);
            if (GET_PLAYER(eOriginalOwner).GetMinorCivAI()->GetTrait() == MINOR_CIV_TRAIT_MILITARISTIC)
                addAnnexedMilitaryCityStates(eOriginalOwner);
        }

shouldn't this be limited to capturing the capital of a minor? i quite often see minors with mini-empires of multiple cities ...

  1. in UpdateFoodInCapitalPerTurnFromAnnexedMinors() there is this bit
        //update if necessary
        if (iBonus != m_iFoodInCapitalFromAnnexedMinors && getCapitalCity())
        {
            getCapitalCity()->ChangeBaseYieldRateFromCSAlliance(YIELD_FOOD, iBonus - m_iFoodInCapitalFromAnnexedMinors);
            m_iFoodInCapitalFromAnnexedMinors = iBonus;
            updateYield();
        }

what is the point of the ChangeBaseYieldRateFromCSAlliance() call here? why mix alliances and annexations?

RecursiveVision commented 3 months ago

@axatin

axatin commented 3 months ago

shouldn't this be limited to capturing the capital of a minor

that's checked literally one line before the code you quoted: if (GET_PLAYER(eOriginalOwner).isMinorCiv())

what is the point of the ChangeBaseYieldRateFromCSAlliance() call here?

All yields from maritime city-states are given through YieldRateFromCSAlliance, see for example CvCity::UpdateYieldsFromExistingFriendsAndAllies

if (pMinor && pMinor->GetTrait() == MINOR_CIV_TRAIT_MARITIME)
{
    if (pMinor->IsAllies(getOwner()))
    {
        int iBonus = isCapital() ? pMinor->GetAlliesCapitalFoodBonus() : pMinor->GetAlliesOtherCityFoodBonus();
        if (iBonus != 0)
        {
            ChangeBaseYieldRateFromCSAlliance(YIELD_FOOD, iSign * iBonus / 100);
            //CUSTOMLOG("adjusted food in %s by %d/100 for alliance with %s, current value is %d", getNameKey(), iSign * iBonus, GET_PLAYER(ePlayer).getNameKey(), GetBaseYieldRateFromCSAlliance(YIELD_FOOD));
        }
    }
ilteroi commented 3 months ago
  1. i think you misread? minor players can have multiple cities ... in fact we probably need to check isOriginalCapital()

  2. what is the point of m_iFoodInCapitalFromAnnexedMinors then? it's not needed ...

axatin commented 3 months ago
  1. ah I understand now. you're right, will fix it.
  2. It is necessary to keep track of the food from annexed minors separately from YieldRateFromCSAlliance. The food bonuses depend on the era, and on an era change all regular city-states update YieldRateFromCSAlliance in MinorCivAI. We need m_iFoodInCapitalFromAnnexedMinors to know how to update the remaining yields from the dead CS.