YieldLibrary Activation

[stackpoint]

I'm going to start slowly reactivating YieldLibrary functions in hopes of restoring functionality to some CEP features.

• Activate City Yields
• Add Terrain Worked Trigger
• Activate Global Yields
• Activate Happiness Yields
• Sync CityView.lua Yields
• Sync ProductionPopup.lua Yields
• Sync TopPanel.lua Yields

There are also a lot of yield bugs that need to be confirmed fixed. Any outstanding issues should be brought up here so they can be troubleshooted and collected here.

[GrantSP]

Fantastic.
If it is not too much to ask, can you keep me apprised of your progress as I would really like to learn from this process.

Please! 😃

[stackpoint]

This commit should activate city specific yields handled by the mod. This also fixes the golden age modifiers to all the yields (faith/culture/surplus food).

Syncing the CityView.lua yields (tooltip vs window) will be next on the list to address.

[GrantSP]

So we don't need to add YIELD_FAITH into the City_ChangeYieldStored function?

[stackpoint]

It seems to have worked with GA yield modifier (set to 2000% for testing) so I assume that the player:ChangeYieldStored works for it. And I don't think that faith has any secondary effects like culture does.

However, I suspect that faith modifications may need to be added to other parts of the YieldLibrary. But that's something I'll look at as problems crop up.

[GrantSP]

I'll leave you to it and just read up afterward.
You don't need me looking over your shoulder every step.

Glad you have re-instated this.

[skodkim]

Thumbs up Stackpoint - can hardly wait to try this out!

\Skodkim

[stackpoint]

This commit should sync the CityView.lua yields up with the city yields from the YieldLibrary.

[skodkim]

Was thinking about looking at the bug below, but before doing so i'd like to hear if you think it will be solved by th yield library changes (in which case i'll wait for your changes):

http://forums.civfanatics.com/showthread.php?t=518620

\Skodkim

[stackpoint]

This is almost definitely a YieldLibrary problem. There is a section of the YieldLibrary that handles local and national happiness that has not been activated yet. However, I do not plan on trying to activate that section of code until I'm sure that all the other yields have been worked out.

However, you are free to look at the YieldLibrary and try to troubleshoot any problems bugs that come up.

[skodkim]

Just tested with New Features build:

• Sistine Chapel still doesn't work (no culture modifier - GlobalCultureRateModifier table)
• Wat ka Phraev still doesn't work (no bonus from shrines/temples - Building_BuildingClassYieldModifiers table)

I think I saw city yields being off but I dodn't go that deep into this part.

\Skodkim

[stackpoint]

Sistine Chapel should be fixed here: d5e5daf

[stackpoint]

It's also expected that the top panel and production panels yields will be incorrect until they are updated.

I also identified a problem with the yields from terrain with the YieldLibrary. It seems like the cache doesn't update whenever the worker changes. So if I reassign my workers or get an increase of population, the City View won't update until the turn ends. I'm think about adding an update cache trigger whenever terrain yields change but it'll require more thought.

[GrantSP]

Always thought there was a time difference with some of these components.
If it looks like a lot of work don't worry about it. If it only happens
every now and then it shouldn't worry too many people.

[stackpoint]

Currently, the following are the triggers that update the yield cache:

LuaEvents.NewPolicy                     .Add(ResetYieldCacheAll)
LuaEvents.NewTech                       .Add(ResetYieldCacheAll)
LuaEvents.PlotChanged                   .Add(ResetYieldCacheAll)
LuaEvents.BuildingConstructed           .Add(ResetYieldCacheAll)
LuaEvents.ActivePlayerTurnEnd_Player    .Add(ResetYieldCacheAll)

[GrantSP]

That's all? Not a lot.

[stackpoint]

This commit should address most of the CityView display problems. I also added an (excessive?) amount of event hooks to reset yields in order to try to solve the caching problem (no 1 turn lag now). I'll try deactivating some of the hooks later to improve performance.

Also the (display) Wat Phra Kaew should be fixed in this release. However, since gold is a global yield, it's won't be corrected until global yields are (fully?) activated.

[skodkim]

Fantastic work Stack - can't wait to try the next release!

\Skodkim

On 9. feb. 2014 15.53.37 CET, stackpointer notifications@github.com wrote:

This
commit
should address most of the CityView display problems. I also added an
(excessive?) amount of event hooks to reset yields in order to try to
solve the caching problem (no 1 turn lag now). I'll try deactivating
some of the hooks later to improve performance.

Also the (display) Wat Phra Kaew should be fixed in this release.
However, since gold is a global yield, it's won't be corrected until
global yields are (fully?) activated.

---

Reply to this email directly or view it on GitHub:
#179 (comment)

Sendt fra min Android telefon med K-9 Mail. Undskyld hvis jeg er lidt kortfattet.

[stackpoint]

Hagia Sophia (faith from specialist yields) should be fixed here.

[GrantSP]

I think you should put in for a pay raise. 😆

[EricB1]

Double his pay!

[skodkim]

Well, here's a couple of virtul 🍻 since I can't give an actual pay rise

👍

\Skodkim

[Apuity]

I was eager to play with a fixed Hagia Sophia, I followed stackpoint's changes in several YieldLibrary files. I believe I followed the changes closely. I am reporting this because I wish to have this bug fixed, but it is totally possible that I misused stackpoint's changes.

What I observed was that each specialist is given +2 faith in the specialist slot tooltip in the city screen. (why +2, I don't know...)
This +2 faith is not added to the "total faith produced in the city" within the city screen. When you mouseover the "Total faith in city", you can see +2 Faith from specialists in the breakdown tooltip. But this +2 is not added to the total number. Taking away specialists from the city removes the +2 Faith from the breakdown tooltip, but it does not alter the number shown in "Total faith in city".

Faith from specialists in "Total faith in empire" does not include faith from specialists.

[GrantSP]

@stackpoint City_GetBaseYieldRate looks to handle many different yields by calling additional functions geared specifically towards each defined type.

Can we add a function to handle Yields from Belief or other areas?

My other comment wasn't specifically about the tooltip code, just looking at seeing if the YieldLibrary can be made better/clearer/easier.

If the same block of code can be used in many ways it would make it more streamlined. Wouldn't it?

[stackpoint]

City_GetBaseYieldRate already has all the components of all the modifiers since it sums the total modifiers for the city. So it's just a matter of partitioning the components into it's individual functions (some of which have already been done).

[GrantSP]

Maybe we could get Thal to comment his code?
Try to make it easier to read.

[stackpoint]

The code is already there as I've stated:

    yieldMod = yieldMod + player:GetGoldenAgeYieldModifier(yieldID)
    yieldMod = yieldMod + City_GetBaseYieldModFromTraits(city, yieldID)
    yieldMod = yieldMod + City_GetBaseYieldModFromPuppet(city, yieldID)
    yieldMod = yieldMod + City_GetBaseYieldModifierFromPolicies(city, yieldID, itemTable, itemID, queueNum)
    yieldMod = yieldMod + City_GetBaseYieldModifierFromGlobalBuildings(cityOwner, yieldID)
    yieldMod = yieldMod + City_GetBaseYieldModFromBuildings(city, yieldID)

I won't have trouble finding the yield modifier code. I was having more problems with identifying the correct section of tooltip code to modify, especially earlier when I wasn't sure the YieldLibrary was granting the correct yields in the first place.

[GrantSP]

I'm not trying to tell you what to do, I'm trying to understand it for
myself. I just use the stuff you're on so you don't have to shift your
attention too much.

[GrantSP]

The OP has TopPanel.lua as having activated YieldLibrary support, I would question this in light of the discussion in CivFanatics regarding World Congress. The last comment I made there made mention of an apparent discrepancy in the yields shown. Though I may not be seeing the full picture there, perhaps there are some modifiers at play that aren't obvious.

You still have ProductionPopup.lua as needing work, I concur it is desperately in need of a fix. This is the same panel that shows on both the large right-hand side notification and when inside the CityView you choose either Purchase or Production? Specifically the list of yields at the top?
Gold in particular is way out. In one saved game the capital showed either 82 or 55 depending on where you look. Next worst yield was the Food which was on average 20% out.

There isn't a straight correlation either, it isn't a case of the yields from one file are less than the other, it is the the yields themselves vary up or down based on which city. I am imagining this is better as that seems to indicate the modifiers are not synced in the files.

Before you do start on those files though, I am trying to sync up the display of them to have them be as consistent as possible. For instance Tourism isn't shown in the list of yields in the ProductionPopup, I'm seeing if I can figure this out before you get to it.

[stackpoint]

The TopPanel.lua has activate YieldLibrary support. It's just a matter of making sure all the math checks out in the YieldLibrary and ToolTip files.

[stackpoint]

The ProductionPopup is this: http://img839.imageshack.us/img839/3581/mcsfood.jpg

We currently don't have ProductionPanel.lua active in our mod so there's no support for it at all. And there's no point in modifying the file until we actually get all the yields and tooltips working correctly in the CityView.lua file in the first place.

[stackpoint]

It seems like I had mistaken ProductionPopup for ProductionPanel. Anyway, I just took a quick look and it seems to be running all the vanilla code for the yield part of the popup so it doesn't have any YieldLibrary support at all. And there's no point in modifying the file until we actually get all the yields and tooltips working correctly in the CityView.lua file in the first place.

[stackpoint]

And isn't Tourism not included in the ProductionPopup in vanilla?

[GrantSP]

Putting aside your double negative in the question, 😃 nope. It has all the yields except that.
I saw somewhere some code that does what I am describing, I couldn't find it yesterday, but I know what I am after and it works, at least it did once.

Also I think your image shows more of the tooltip that occurs over the ProductionPopup, I believe but could be wrong, what is shown on that is determined by the InfoTooltipInclude.lua.

[GrantSP]

This is not a criticism of the work thus far, just an observation. It looks like for every step forward that solves a problem a new one emerges to take it's place.
It could almost be considered counter-productive to maintain the code-base as it is with so many undocumented/unclear functions. If it were cut back to the basic minimum and then new functions re-established, or re-written, as they become needed.

[stackpoint]

If you could list all the basic minimum functions of the yieldlibrary then it wouldn't be a huge problem to do this.

[GrantSP]

I barely understand the basics to make an informed list but I will say this.
On the wiki, @Thalassicus says this:

"I redesigned the yield functions so they are easier to use. For example, a single function player:GetYieldStored returns stored gold, science, culture, or so on. This is vastly simpler than the disorganized collection of functions required in the past."

On that function I don't understand why Production is not a yield accessed by it, Science, Food, Gold, Happiness, Faith and even an unused CS_MILITARY yield are, but not Production.

Speaking as an absolute novice, any time a yield is needed to be accessed by any vanilla function or modded function, the same values should be used.

So if we use Production as the reference, the displays in TopPanel, CityView, ProductionPopup, WorldView or anything else should be using the same value.
Further to that any modifier should likewise work on the same value. Percentage boosts or penalties from Beliefs, Policies, Wonders, CS alliances, Trade, Traits, Un/Happiness, Golden Ages, etc. should also being applied to the same value.

Imagine a city that is making 10Production, 4 from Buildings and 6 from Terrain. If the CityView applies a modifier to the total yield then every other instance the totals are used it needs to be the same. If we have an instance where a belief, for instance, modifies the production and it is shown on the TopPanel but not in the CityView, users have the right to say: "hey what is the true value?"

Looking at YieldLibrary.lua, I feel for you. The task is not an easy one.
There are functions to provide base yields, consumed yields, surplus yields, total yields, etc. assuming they are all working together then I can see it is fine, but if just one of these is referencing the incorrect value from an out-of-date function. Well, I don't know.

[stackpoint]

PlayerClass.GetYieldStored refers to national yields and production is a city and unit/building specific yield. There is no global yield stored for production for players.

[GrantSP]

Like I said, 'novice'. 😃

[stackpoint]

And I don't really understand what you're trying to say with the rest.

[GrantSP]

Well, that's not surprising.
All I'm saying is, I don't know why the yields are so wrong. I can't
believe it was like this before BNW.
Nothing seems to match.
Depending on where you look, the yields can vary dramatically.
If the yield says X in one place it should say X in the other places.
The totals and the breakdowns of each don't always match.
Look at the World Congress post in the forum to see what I mean.

[stackpoint]

This isn't surprising at all. The Yield Library was written for GnK. The World Congress alone adds many sources of yields that isn't handled by it.

[GrantSP]

If all the modded files in CEP are the latest, they should take into
consideration the WC. The only difference should be the YieldLibrary.

[stackpoint]

That's not how that works.

[GrantSP]

Clearly.
Look, As I've said before, I am not 100% aware of the ins and outs of the code.
All I know is the user experience now is not up to a standard that deserves to be released. The end user is NOT getting a clear indication of what is happening. Further, those that wish to edit the mod, are not sure the end results of their changes are being displayed correctly.

I'm tired of harping on and on over this issue. I have no idea what to offer as a way to rectify this.
I'll will no longer clog this issue with my frustration.

I am confident you have the problem well in hand.

[GrantSP]

Inside the function : City_UpdateModdedYields(city, player) there is a line that queries the Leader.AIBonus.

I see in CEL_End.sql that it is to assist certain leaders that struggle with their AI routines and is also referenced in CEAI_Events.lua with regard to starrting units.
I don't think we are using this AIBonus at the moment, does it make a difference whether this stays in or is removed? The log makes a mention of it, but it isn't flagged as an error. It simply says:
YieldLibrary.lua:2230: Cannot find key - AIBonus

Perhaps the value should be set to 0 or FALSE at the time of defining it in CAT_AlterTables.sql

e.g.
ALTER TABLE Leaders ADD AIBonus boolean default false;