Thalassicus / ceg

(Deprecated: now in "cep-bnw" project)
http://civmodding.wordpress.com
0 stars 9 forks source link

Science Spikes #16

Closed stackpoint closed 10 years ago

stackpoint commented 10 years ago

It seems like the science spikes weren't caused by the AI science handicap function.

I was able to identify that the instant yields provided by AI China's UB is granted to the human player in addition/instead. You can replicate this by increasing the instant yield of the building to something outrageous like 2000 and playing with all the AI set to China. You'll be able to see that the science spikes line up with the construction of the UB.

I haven't tested whether or not this applies to the other instant yields as well.

GrantSP commented 10 years ago

That is a very creative way of finding the bug. Thinking outside the box. I like the progress.

On 14 November 2013 13:58, stackpointer notifications@github.com wrote:

It seems like the science spikes weren't caused by the AI science handicap function.

I was able to identify that the instant yields provided by AI China's UB is granted to the human player. You can replicate this by increasing the instant yield of the building to something outrageous like 2000 and playing with all the AI set to China. You'll be able to see that the science spikes line up with the construction of the UB.

— Reply to this email directly or view it on GitHubhttps://github.com/Thalassicus/ceg/issues/16 .

"There are 10 types of people in the world: those who understand binary, and those who don't."

GrantSP commented 10 years ago

Since the Building_YieldInstant is an addition by the mod, do you think all the other buildings that utilize it also cause errors? Satrap's Court, Greek Odeon, StoneHenge, Indian Vedi and Basilica all give instant yields via this table.

Paper Maker is the only one that gives a Science yield the other give Faith, Culture or National Happiness. Maybe the science bug was easier to see happen.

On 14 November 2013 14:01, Grant Pritchard fr00gyl@gmail.com wrote:

That is a very creative way of finding the bug. Thinking outside the box. I like the progress.

On 14 November 2013 13:58, stackpointer notifications@github.com wrote:

It seems like the science spikes weren't caused by the AI science handicap function.

I was able to identify that the instant yields provided by AI China's UB is granted to the human player. You can replicate this by increasing the instant yield of the building to something outrageous like 2000 and playing with all the AI set to China. You'll be able to see that the science spikes line up with the construction of the UB.

— Reply to this email directly or view it on GitHubhttps://github.com/Thalassicus/ceg/issues/16 .

"There are 10 types of people in the world: those who understand binary, and those who don't."

"There are 10 types of people in the world: those who understand binary, and those who don't."

stackpoint commented 10 years ago

It's certainly possible. The code in Cat_Events.lua:

function BuildingCreated(player, city, buildingID)
    local playerID      = player:GetID()
    local plot          = city:Plot()

    local buildingInfo  = GameInfo.Buildings[buildingID]
    local query         = ""
    local trait         = player:GetTraitInfo()

    -- ...

    query = string.format("BuildingType = '%s'", buildingInfo.Type)
    for info in GameInfo.Building_YieldInstant(query) do
        City_ChangeYieldStored(city, GameInfo.Yields[info.YieldType].ID, info.Yield)
    end

    -- ...
end

LuaEvents.BuildingConstructed.Add( BuildingCreated )

which leads to the code in YieldLibrary.lua:

function City_ChangeYieldStored(city, yieldID, amount, checkThreshold)
    if city == nil then
        log:Fatal("City_ChangeYieldStored city=nil")
    elseif itemTable and not itemID then
        log:Fatal("City_ChangeYieldStored itemID=nil")
    end
    local player = Players[city:GetOwner()]
    if yieldID == YieldTypes.YIELD_FOOD then
        city:ChangeFood(amount)
        local overflow = City_GetYieldStored(city, yieldID) - City_GetYieldNeeded(city, yieldID)
        if checkThreshold and overflow >= 0 then
            local totalYieldKept = 0
            for building in GameInfo.Buildings("FoodKept <> 0") do
                if city:IsHasBuilding(building.ID) then
                    totalYieldKept = totalYieldKept + building.FoodKept / 100
                end
            end
            city:ChangePopulation(1,true)
            city:SetFood(0)
            if totalYieldKept > 0 then
                log:Warn(
                    "%s add %s food = %s overflow + %s totalYieldKept * %s City_GetYieldNeeded", 
                    city:GetName(), 
                    overflow + totalYieldKept * City_GetYieldNeeded(city, yieldID), 
                    overflow, 
                    totalYieldKept, 
                    City_GetYieldNeeded(city, yieldID)
                )
            end
            City_ChangeYieldStored(city, yieldID, overflow + totalYieldKept * City_GetYieldNeeded(city, yieldID), true)
        end
    elseif yieldID == YieldTypes.YIELD_PRODUCTION then
        city:ChangeProduction(amount)
    elseif yieldID == YieldTypes.YIELD_CULTURE then
        city:ChangeJONSCultureStored(amount)
        player:ChangeYieldStored(YieldTypes.YIELD_CULTURE, amount)
        local overflow = City_GetYieldStored(city, yieldID) - City_GetYieldNeeded(city, yieldID)
        if checkThreshold and overflow >= 0 then
            city:DoJONSCultureLevelIncrease()
            city:SetJONSCultureStored(0)
            City_ChangeYieldStored(city, yieldID, overflow, true)
        end
    elseif yieldID == YieldTypes.YIELD_POPULATION then
        city:ChangePopulation(amount, true)
    else
        player:ChangeYieldStored(yieldID, amount)
    end
end

It all looks correct so...

GrantSP commented 10 years ago

That last else statement, after Food, Production, Culture and Population are processed is that then saying "ok it isn't those things so lets give that yield to the players total of that specified yield type."?

player:ChangeYieldStored (yieldID, amount)

Should that be narrowed to apply to only the city yield total, not the overall total for the player?

Maybe the specificity of the yield type depends on the location of its storage. In the same way happiness can be city or national.

I'm only thinking out loud here, no facts to support me.

On 14 November 2013 14:20, stackpointer notifications@github.com wrote:

It's certainly possible. The code in Cat_Events.lua:

function BuildingCreated(player, city, buildingID) local playerID = player:GetID() local plot = city:Plot()

local buildingInfo  = GameInfo.Buildings[buildingID]
local query         = ""
local trait         = player:GetTraitInfo()

-- ...

query = string.format("BuildingType = '%s'", buildingInfo.Type)
for info in GameInfo.Building_YieldInstant(query) do
    City_ChangeYieldStored(city, GameInfo.Yields[info.YieldType].ID, info.Yield)
end

-- ...end

LuaEvents.BuildingConstructed.Add( BuildingCreated )

which leads to the code in YieldLibrary.lua:

function City_ChangeYieldStored(city, yieldID, amount, checkThreshold) if city == nil then log:Fatal("City_ChangeYieldStored city=nil") elseif itemTable and not itemID then log:Fatal("City_ChangeYieldStored itemID=nil") end local player = Players[city:GetOwner()] if yieldID == YieldTypes.YIELD_FOOD then city:ChangeFood(amount) local overflow = City_GetYieldStored(city, yieldID) - City_GetYieldNeeded(city, yieldID) if checkThreshold and overflow >= 0 then local totalYieldKept = 0 for building in GameInfo.Buildings("FoodKept <> 0") do if city:IsHasBuilding(building.ID) then totalYieldKept = totalYieldKept + building.FoodKept / 100 end end city:ChangePopulation(1,true) city:SetFood(0) if totalYieldKept > 0 then log:Warn( "%s add %s food = %s overflow + %s totalYieldKept * %s City_GetYieldNeeded", city:GetName(), overflow + totalYieldKept * City_GetYieldNeeded(city, yieldID), overflow, totalYieldKept, City_GetYieldNeeded(city, yieldID) ) end City_ChangeYieldStored(city, yieldID, overflow + totalYieldKept * City_GetYieldNeeded(city, yieldID), true) end elseif yieldID == YieldTypes.YIELD_PRODUCTION then city:ChangeProduction(amount) elseif yieldID == YieldTypes.YIELD_CULTURE then city:ChangeJONSCultureStored(amount) player:ChangeYieldStored(YieldTypes.YIELD_CULTURE, amount) local overflow = City_GetYieldStored(city, yieldID) - City_GetYieldNeeded(city, yieldID) if checkThreshold and overflow >= 0 then city:DoJONSCultureLevelIncrease() city:SetJONSCultureStored(0) City_ChangeYieldStored(city, yieldID, overflow, true) end elseif yieldID == YieldTypes.YIELD_POPULATION then city:ChangePopulation(amount, true) else player:ChangeYieldStored(yieldID, amount) endend

It all looks correct so...

— Reply to this email directly or view it on GitHubhttps://github.com/Thalassicus/ceg/issues/16#issuecomment-28456421 .

"There are 10 types of people in the world: those who understand binary, and those who don't."

stackpoint commented 10 years ago

It looks like the function takes 1 yieldType at a time and process certain yields different from the others. You are right that the type of yield determines if the yield is placed in the city or nationally.

It looks like the problem isn't in Cat_Events.lua

MT_Initialize: INFO   Turn 35  BUILDING_PAPER_MAKER built in Beijing with yield YIELD_SCIENCE
GrantSP commented 10 years ago

Hmm... I've used up my quota of brain cells for the moment. So the YieldLibrary.lua then? Sorry I can offer more constructive opinions.

On 14 November 2013 14:57, stackpointer notifications@github.com wrote:

It looks like the function takes 1 yieldType at a time and process certain yields different from the others. You are right that the type of yield determines if the yield is placed in the city or nationally.

It looks like the problem isn't in Cat_events.lua

MT_Initialize: INFO Turn 35 BUILDING_PAPER_MAKER built in Beijing with yield YIELD_SCIENCE

— Reply to this email directly or view it on GitHubhttps://github.com/Thalassicus/ceg/issues/16#issuecomment-28457695 .

"There are 10 types of people in the world: those who understand binary, and those who don't."

stackpoint commented 10 years ago

Hmm, YieldLibrary.lua doesn't seem to be the problem:

MT_Initialize: DEBUG  Wu Zetian Beijing gets 1000 YIELD_SCIENCE

I'll look into the player:ChangeYieldStored() function

GrantSP commented 10 years ago

You'll find it. I'm sure of it.

On 14 November 2013 15:10, stackpointer notifications@github.com wrote:

Hmm, YieldLibrary.lua doesn't seem to be the problem:

MT_Initialize: DEBUG Wu Zetian Beijing gets 1000 YIELD_SCIENCE

I'll look into the player:ChangeYieldStored() function

— Reply to this email directly or view it on GitHubhttps://github.com/Thalassicus/ceg/issues/16#issuecomment-28458113 .

"There are 10 types of people in the world: those who understand binary, and those who don't."

stackpoint commented 10 years ago

Looks like PlayerClass.ChangeYieldStored was the problem:

    elseif yieldID == YieldTypes.YIELD_SCIENCE then
        local sciString = ""
        local teamID    = player:GetTeam()
        local team      = Teams[teamID]
        local teamTechs = team:GetTeamTechs()

        sciString = "Sci bonus for "..player:GetName()..": "
        local targetTech = itemID or player:GetCurrentResearch()
        if targetTech ~= -1 then
            targetTech = GameInfo.Technologies[targetTech]
            sciString = string.format("%-40s %s +%-3d  @ %s (%s needed)", sciString, Game.Round(teamTechs:GetResearchProgress(targetTech.ID)), Game.Round(yield), targetTech.Type, teamTechs:GetResearchCost(targetTech.ID))
            teamTechs:ChangeResearchProgress(targetTech.ID, yield)
        else
            local researchableTechs = {}
            for techInfo in GameInfo.Technologies() do
                if player:CanResearch(techInfo.ID) and not team:IsHasTech(techInfo.ID) then
                    table.insert(researchableTechs, techInfo.ID)
                end
            end
            if #researchableTechs > 0 then
                targetTech = researchableTechs[1 + Map.Rand(#researchableTechs, "player:ChangeYieldStored: Random Tech")]
                targetTech = GameInfo.Technologies[targetTech]
                sciString = string.format("%-40s +%-3d  @ %s (random)", sciString, Game.Round(yield), targetTech.Type)
                teamTechs:ChangeResearchProgress(targetTech.ID, yield)
            end
        end
        log:Warn(sciString)
        if player:IsHuman() then
            --log:Warn(sciString)
        end
    end

The ChangeResearchProgress function requires the playerID:

void TeamTechs:ChangeResearchProgress(TechType index, int change, PlayerID player) 

Since the function was not given one, lua must've passed on a nil value that the C++ code took as 0 which is the playerID of the human player.

I tested this and the science spikes from the UB no longer appear for the human player.

GrantSP commented 10 years ago

Excellent. Did I not say you would find it? That means I can say I helped you find it. ;) 99.9999% you and 0.0001% me. A win all round.

On 14 November 2013 15:55, stackpointer notifications@github.com wrote:

Closed #16 https://github.com/Thalassicus/ceg/issues/16.

— Reply to this email directly or view it on GitHubhttps://github.com/Thalassicus/ceg/issues/16 .

"There are 10 types of people in the world: those who understand binary, and those who don't."

stackpoint commented 10 years ago

Hehe. Your words of encouragement helped me through this grueling ordeal :P

As an aside, this would explain why AIPerTurnBonuses() was bugging out. The player was receiving all of the science bonuses instead of the AI.

GrantSP commented 10 years ago

Where did that AIPerTurnBonuses() error present itself? Still connected with the science spikes? Or something else again?

On 14 November 2013 16:06, stackpointer notifications@github.com wrote:

Hehe. Your words of encouragement helped me through this grueling ordeal :P

As an aside, this would explain why AIPerTurnBonuses() was bugging out. The player was receiving all of the science bonuses instead of the AI.

— Reply to this email directly or view it on GitHubhttps://github.com/Thalassicus/ceg/issues/16#issuecomment-28459964 .

"There are 10 types of people in the world: those who understand binary, and those who don't."

stackpoint commented 10 years ago

It was using the same function as the Building_InstantYields to give the AI small science boosts every turn. But unlike the 200 you'd get from China's UB it was something like 1-3 science in the early game.

GrantSP commented 10 years ago

Ahh. I don't know if you drink or not, but if you do, I think you deserve a beer and a break. Some truly great debugging.

Thanks, Grant

On 14 November 2013 16:12, stackpointer notifications@github.com wrote:

It was using the same function as the Building_InstantYields to give the AI small science boosts every turn. But unlike the 200 you'd get from China's UB it's something like 1-3 science in the ancient era.

— Reply to this email directly or view it on GitHubhttps://github.com/Thalassicus/ceg/issues/16#issuecomment-28460161 .

"There are 10 types of people in the world: those who understand binary, and those who don't."