Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Science Spikes #16

Closed
stackpoint opened this issue Nov 14, 2013 · 14 comments
Closed

Science Spikes #16

stackpoint opened this issue Nov 14, 2013 · 14 comments
Assignees
Labels
Milestone

Comments

@stackpoint
Copy link
Contributor

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.

@ghost ghost assigned stackpoint Nov 14, 2013
@GrantSP
Copy link
Collaborator

GrantSP commented Nov 14, 2013

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//issues/16
.

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

@GrantSP
Copy link
Collaborator

GrantSP commented Nov 14, 2013

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//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
Copy link
Contributor Author

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
Copy link
Collaborator

GrantSP commented Nov 14, 2013

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//issues/16#issuecomment-28456421
.

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

@stackpoint
Copy link
Contributor Author

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
Copy link
Collaborator

GrantSP commented Nov 14, 2013

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//issues/16#issuecomment-28457695
.

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

@stackpoint
Copy link
Contributor Author

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
Copy link
Collaborator

GrantSP commented Nov 14, 2013

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//issues/16#issuecomment-28458113
.

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

stackpoint added a commit to Thalassicus/cat that referenced this issue Nov 14, 2013
@stackpoint
Copy link
Contributor Author

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
Copy link
Collaborator

GrantSP commented Nov 14, 2013

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 #16.


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

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

@stackpoint
Copy link
Contributor Author

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
Copy link
Collaborator

GrantSP commented Nov 14, 2013

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//issues/16#issuecomment-28459964
.

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

@stackpoint
Copy link
Contributor Author

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
Copy link
Collaborator

GrantSP commented Nov 14, 2013

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//issues/16#issuecomment-28460161
.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants