Civic Expenses Query

[Davidr]

Koshling,

Re my save game previously downloaded there is another discrepancy I have just found - in Expenses , Civic Upkeep , the total figure shown is not the same as the total breakdown of Civic Expenses when the mouse hovers over the amount

DavidR

 

[Koshling]

Koshling,

Re my save game previously downloaded there is another discrepancy I have just found - in Expenses , Civic Upkeep , the total figure shown is not the same as the total breakdown of Civic Expenses when the mouse hovers over the amount

DavidR

This was another bug in applying he game speed modifiers. Fix will be pushed to the SVN shortly. Thank for finding these for us!

 

[Thunderbrd]

Good catch and a good reminder to watch out for that kind of thing!

I'm currently trying to plug in a LocalTechSpecialistYieldChange tag for buildings and ... whoah... I've interacted with the yield systems before in CvCity but either I'm getting lost all over again or I'm finding what appear to be some places where we have things double counted.

Could you do me a favor Koshling and quickly take a look at that to make sure I'm not going crazy here? The naming conventions in use throughout that system suck so bad its irritating and all too easy to get turned around in dyslexic misassumptions. I'd LOVE to just do some simplification and cleanup there, if nothing else just to make the function naming more clear!

My specific concern, however, is that it appears that ExtraYields, particularly those from specialists (you know we have a getSpecialistsExtraYields(specialist,yield) AND a getExtraSpecialistsYields(specialist,yield)???) may be double applying in some areas. Compiled then applied as well as applied after the compile.

 

[Koshling]

Good catch and a good reminder to watch out for that kind of thing!

I'm currently trying to plug in a LocalTechSpecialistYieldChange tag for buildings and ... whoah... I've interacted with the yield systems before in CvCity but either I'm getting lost all over again or I'm finding what appear to be some places where we have things double counted.

Could you do me a favor Koshling and quickly take a look at that to make sure I'm not going crazy here? The naming conventions in use throughout that system suck so bad its irritating and all too easy to get turned around in dyslexic misassumptions. I'd LOVE to just do some simplification and cleanup there, if nothing else just to make the function naming more clear!

My specific concern, however, is that it appears that ExtraYields, particularly those from specialists (you know we have a getSpecialistsExtraYields(specialist,yield) AND a getExtraSpecialistsYields(specialist,yield)???) may be double applying in some areas. Compiled then applied as well as applied after the compile.

I can't see anything called 'getSpecialistsExtraYields' - can you point me at where you're seeing that one (or presumably one named something like that since I cannot find an exact match) used please?

 

[Davidr]

Thanks,

What I was trying to say was that with the Civic Expenses , although the total of the mouse hover figure was identical to the total on the expenses screen , if you add up all the individual civic expenses they total 30 gold less than the total shows. I don't know if you noticed this.

DavidR

 

[Koshling]

Thanks,

What I was trying to say was that with the Civic Expenses , although the total of the mouse hover figure was identical to the total on the expenses screen , if you add up all the individual civic expenses they total 30 gold less than the total shows. I don't know if you noticed this.

DavidR

Yes, I fixed it earlier today.

 

[Thunderbrd]

I can't see anything called 'getSpecialistsExtraYields' - can you point me at where you're seeing that one (or presumably one named something like that since I cannot find an exact match) used please?

Sorry, I added an s to the end in haste to post.

Referred to in CvCity is the CvPlayer function:
int CvPlayer::getSpecialistExtraYield(YieldTypes eIndex) const
and ALSO (which is horribly confusing)
int CvPlayer::getExtraSpecialistYield(SpecialistTypes eIndex1, YieldTypes eIndex2) const

Upon some review, I think I can actually sort all this out and I may have done this myself, incidentally, when setting up some of the new trait tags... Apparently the first one up there used to have the (specialist,yield) parameters but now it only has the (yield) parameter which basically means that its what is coming from ALL specialists regardless of its type.

Nevertheless I'm still feeling that this structure must not be compiling correctly somewhere as I'm sure there must be some double counting going on. Anyhow, I'll spend some time mapping it out for myself if nothing else and I might change some names or add some notes to make it more possible to understand at a glance the next time I think "I've done that before so it shouldn't be too hard to sort out again..." (silly me!) If I do find a problem I'll let you know, but suffice it to say it could probably use some of your own review IF you have time to look it over before the release. (Particularly after my update!)

 

[Koshling]

Sorry, I added an s to the end in haste to post.

Referred to in CvCity is the CvPlayer function:
int CvPlayer::getSpecialistExtraYield(YieldTypes eIndex) const
and ALSO (which is horribly confusing)
int CvPlayer::getExtraSpecialistYield(SpecialistTypes eIndex1, YieldTypes eIndex2) const

Upon some review, I think I can actually sort all this out and I may have done this myself, incidentally, when setting up some of the new trait tags... Apparently the first one up there used to have the (specialist,yield) parameters but now it only has the (yield) parameter which basically means that its what is coming from ALL specialists regardless of its type.

Nevertheless I'm still feeling that this structure must not be compiling correctly somewhere as I'm sure there must be some double counting going on. Anyhow, I'll spend some time mapping it out for myself if nothing else and I might change some names or add some notes to make it more possible to understand at a glance the next time I think "I've done that before so it shouldn't be too hard to sort out again..." (silly me!) If I do find a problem I'll let you know, but suffice it to say it could probably use some of your own review IF you have time to look it over before the release. (Particularly after my update!)

One comes from traits, and the other from civics. Why we didn't just adopt the same scheme on traits as civics had I don't know, and certainly that adds complexity. However, I don't think there is any double counting. They are held separately in CvPlayer and applied separately in CvCity (in CvCity::getExtraSpecialistYield()) which is the only place they have any impact apart from use in display texts, like hover texts, so far as I can see.

 

[Thunderbrd]

I take it you're pointing at CvPlayer::getSpecialistYieldPercentChanges() for the one that comes from civics and my understanding on that is its a different affect really, not intended to mean the same as additional base yield values though oddly it was plugged into those tallies along with a /100 against the total. This is one thing that doesn't seem right and that would've been pre-existing.

One getExtraSpecialistYield (Yield Types eIndex) applies to all specialists regardless of the specialist type and the other, with two parameters (Specialist/Yield) applies to a specified specialist only.

What I think was throwing me was my attempt to integrate an 'only for this city' calculation into the mix. I believe I've done it correctly but on THIS, an audit is desired once I get it pushed later. I'll be looking over and over that structure myself in hopes of spotting some logic flaw. (well I don't HOPE there IS one but if there is I sure hope to find it!)

 

[Koshling]

I take it you're pointing at CvPlayer::getSpecialistYieldPercentChanges() for the one that comes from civics and my understanding on that is its a different affect really, not intended to mean the same as additional base yield values though oddly it was plugged into those tallies along with a /100 against the total. This is one thing that doesn't seem right and that would've been pre-existing.

One getExtraSpecialistYield (Yield Types eIndex) applies to all specialists regardless of the specialist type and the other, with two parameters (Specialist/Yield) applies to a specified specialist only.

What I think was throwing me was my attempt to integrate an 'only for this city' calculation into the mix. I believe I've done it correctly but on THIS, an audit is desired once I get it pushed later. I'll be looking over and over that structure myself in hopes of spotting some logic flaw. (well I don't HOPE there IS one but if there is I sure hope to find it!)

No. The percentage mechanism is separate again.

int CvTraitInfo::getSpecialistExtraYield(int i) const

That's the one param one, and comes from traits.

The two param one has two sources (it's not civics - that was mistaken of me to state previously). These are:

Traits again:

int CvTraitInfo::getSpecialistYieldChange(int i, int j) const

Buildings:

int CvBuildingInfo::getSpecialistYieldChange(int i, int j) const

Both are applied to CvPlayer via CvPlayer::changeExtraSpecialistYield() when traits or buildings are processed.

I still don't see any double counting.

 

[Thunderbrd]

There was certainly cause to generate the

int CvTraitInfo::getSpecialistYieldChange(int i, int j) const

function so that national sources could tally before being plugged into changeExtraSpecialistYield().

Yeah, so far so good. But I'm still a bit suspicious of CvPlayer::getSpecialistYieldPercentChanges() in that it seems to be intended as a percentage modifier but is not appearing to quite be calculated in in this way. It seems that the method employed is to divide it by 100 then add it to the rest of the base. I do get twisted about in my head trying to follow this all but that one spot doesn't seem right, though I haven't attempted to fix it.

 

[Koshling]

There was certainly cause to generate the

int CvTraitInfo::getSpecialistYieldChange(int i, int j) const

function so that national sources could tally before being plugged into changeExtraSpecialistYield().

Yeah, so far so good. But I'm still a bit suspicious of CvPlayer::getSpecialistYieldPercentChanges() in that it seems to be intended as a percentage modifier but is not appearing to quite be calculated in in this way. It seems that the method employed is to divide it by 100 then add it to the rest of the base. I do get twisted about in my head trying to follow this all but that one spot doesn't seem right, though I haven't attempted to fix it.

It's just bad naming. It's not meant to be a percentage modifier, its just a way to allow fractional (percentage of 1) extras. This is how priests get +0.75gold with state church

 

[Thunderbrd]

Ah... thanks for clearing that up then.