Thalassicus / cep-bnw

Civ V Communitas Expansion Pack
32 stars 22 forks source link

[Pre-release] Trade routes not working? #210

Closed CunningGabe closed 10 years ago

CunningGabe commented 10 years ago

I just started a new game today with the pre-release as Ethiopia. After a while, I had the feeling that my gold was lower than it should be -- and sure enough, it seems that the income from my caravans wasn't being added to my total. It did show up in the income, so I have Gold: 62 (+31), but then next turn I just have 71 gold instead of 93. This continued over several turns.

GrantSP commented 10 years ago

Confirmed. The TopPanel is only totaling the Gold generated from the cities internal sources, the Trade Gold is not included. I will try a game without CIN. Perhaps the code for the TopPanel isn't quite right just yet.

Ok, the game without CIN appears to show the Trade Gold being added to the totals but there are some weird 'Rounding' issues. My initial Trade route is with Kabul, the Caravan selection panel showed 2 Gold to me and 1 to them. So with a NET income from the Capital of 4 Gold I was supposed to get 6. The total on the TopPanel showed an increase of Gold but it was only 1 Gold toppanelgoldsummary toppaneltradesummary traderouteoverview citygold

stackpoint commented 10 years ago

TopPanel should be in CAT. I will take a look at the YieldLibrary tonight.

GrantSP commented 10 years ago

I was updating the comment with some screenshots when you posted. I'll have a look as well. I didn't actually realize you had restored some of the interface features from GEM in the new-features release. Nice.

GrantSP commented 10 years ago

Did a bit of comparison with how vanilla and CEP handles these figures. On second look I don't think there is much difference. The biggest issue I see is the way the Trade Route Selection panel and the other panels handle decimal points. It looks like the Trade Selection Panel rounds ALL decimal points UP while they are rounded DOWN everywhere else.

I'm actually getting more confused the more I look at this. Sometimes I think it looks fine and then I'm not so sure. As far as I can tell the biggest change that may affect this is the function:

Vanilla

local iGoldPerTurn = pPlayer:CalculateGoldRate();

CEP

local iGoldPerTurn = activePlayer:GetYieldRate(YieldTypes.YIELD_GOLD);

Still, I don't think it is that much of a difference.

stackpoint commented 10 years ago

I don't believe the error is a rounding issue. At least not enough to cause a 20 gpt difference.

stackpoint commented 10 years ago

@CunningGabe, can you attach your save game?

GrantSP commented 10 years ago

I have now done about a dozen tests and the more I do them the more I conclude there is no issue. Admittedly they are only with just trade route so any rounding isn't great but I must concur with you, there simply can't be enough routes available to account for 20+ difference. That would mean at the least 20+ trade routes!

stackpoint commented 10 years ago

Errors I've encountered so far with Gold:

CunningGabe commented 10 years ago

I'm pretty sure that my issue is not with the top panel -- it is showing an income of 31, but only giving 9 gold per turn. It's hard to catch with small trade routes, but I have a caravanssery, and my routes are each generating about 10 gold. Yesterday I started a new game to test, and ran into the same problem.

Anyway, here is my savefile: haile selassie_0067 bc-1320

stackpoint commented 10 years ago

Thanks for the save file. From that I was able to identify that the function CityClass.GetYieldRate includes income from trade routes going to and from the city. I'll have to move the gold modded sources to national yields.

GrantSP commented 10 years ago

That was very quick. Well done.

stackpoint commented 10 years ago

See the above commit for the disable. This should only affect the 10% gold from shrines/temples for the Wat Phra Kaew as I believe that's our only modded gold source that isn't handled by vanilla methods. I recommend a new build/release with this fix as income from trade routes are pretty critical.

stackpoint commented 10 years ago

Also, i'll try to work on national yields sometime this week.

GrantSP commented 10 years ago

Not at my system right now, are these fixes committed to the source? I can do a release first thing tomorrow.

CunningGabe commented 10 years ago

Hooray! I'll be glad to get my civ fix again :)

Off-topic: I registered for civFanatics a few weeks ago and never received the activation email (I checked my spam filter). I can't seem to find a way to have it re-sent. What can I do to become a fully-fledged member?

stackpoint commented 10 years ago

I can PM a mod for you. What's your username?

stackpoint commented 10 years ago

@GrantSP, See: https://github.com/Thalassicus/cep-bnw/commit/766da3d6234e5ca8b1eb9e45ae37214260a806be

CunningGabe commented 10 years ago

@stackpoint I'm CunningGabe on the forums too. Thanks!

stackpoint commented 10 years ago

Your account should be activated now.