Thalassicus / cep-bnw

Civ V Communitas Expansion Pack
32 stars 22 forks source link

[3.13] Marketplace and East India do not give trade gold #145

Closed skodkim closed 10 years ago

skodkim commented 10 years ago

Marketplace and East India does not increase gold from trade routes (while the caranesary does).

Tested using 3.13 and only Ingame Editor enabled (saw it it another game using other mods and decided to test).

The city shown below just built a Marketplace, Before it was also +2 gold from trade routes.

\Skodkim

civ5screen0013

GrantSP commented 10 years ago

When was this change supposed to happen? Apart from the TXT change for the Market I can't see any code implemented for it or even when it was discussed to be a good idea.

If it is supposed to be a change that is to happen we can change the TradeRouteLandGoldBonus for those buildings to 200, the same as the Caravansary.

skodkim commented 10 years ago

I'm not in front of my gaming computer right now but I'm pretty sure the marketplace and East India Company should give gold from trade routes - As I recall it this is default BNW but I may be wrong. I', pretty sure t also says they do this in the (automatic) description of the buildings. Don't you agree?

Anyway I looked in the xml/sql and it seems to be implemented ok.

To test it I set up a scenario map with one AI city surrounded by three of mine (same distance). Then I made sure the cities had different combinations of the buildings in question and tried establishing trade routes. The conclusion was the buildings had no trade route related effect on gold. The test was however made unnecessarily difficult by this bug: https://github.com/Thalassicus/cep-bnw/issues/121

I suspect this may be yet another bug concerning the yield library.

\Skodkim

GrantSP commented 10 years ago

Yeah I'm still on holidays at the moment and have no access to my game system. I think you may right about the yield library though, after my last comment on #121 I did some looking and found it was happening most of the time, I just wasn't seeing it. I get back next week and will have a look myself. Glad to see you making inroads into these things. Well done.

skodkim commented 10 years ago

May be related to this issue: https://github.com/Thalassicus/cep-bnw/issues/179

\Skodkim

stackpoint commented 10 years ago

Probably not. If the vanilla BNW caravansary can increase trade route gold then I don't see why we'd need any YieldLibrary functions to do so too.

skodkim commented 10 years ago

The Marketplace and East India Company uses the TradeRouteRecipientBonus and TradeRouteTargetBonus fields while the caravansary uses the TradeRouteLandGoldBonus field

Just tested with new features build and the Marketplace and East India Company still does not give additional gold from trade routes while the caravansary does.

\Skodkim

GrantSP commented 10 years ago

What about the other buildings that use TradeRouteRecipientBonus & TradeRouteTargetBonus like the Colossus, Hanse, Bank, Satrap's Court or Bazaar? Do they get the increase?

I'm about to do some testing now.

GrantSP commented 10 years ago

Is this still current? I just tried a number of games with the unique variants of the market and all seemed to do as expected. The East India Company also seemed to give the bonus it said.

skodkim commented 10 years ago

Great if it's working. I haven't tested for a couple of weeks but the conclusion there was that it didn't work. Could it be yield library activation?

On 2. mar. 2014 06.13.38 CET, GrantSP notifications@github.com wrote:

Is this still current? I just tried a number of games with the unique variants of the market and all seemed to do as expected. The East India Company also seemed to give the bonus it said.


Reply to this email directly or view it on GitHub: https://github.com/Thalassicus/cep-bnw/issues/145#issuecomment-36446425

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

GrantSP commented 10 years ago

I've been using 3.14.1 for all my tests, so I would have to say no. Yield library hasn't been activated in this version

skodkim commented 10 years ago

Hmm, not in front of my computer today so I have no way of testing but I've tested this several times. Are you sure your gold output is changing after building them and that trade routes gets boosts (that it's not just display)?

On 2. mar. 2014 07.29.32 CET, GrantSP notifications@github.com wrote:

I've been using 3.14.1 for all my tests, so I would have to say no. Yield library hasn't been activated in this version


Reply to this email directly or view it on GitHub: https://github.com/Thalassicus/cep-bnw/issues/145#issuecomment-36447531

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

GrantSP commented 10 years ago

Not sure what you mean by 'being just a display thing'.

How did you test it to verify that?

skodkim commented 10 years ago

I think the tool tips displayed the bonus but after turnover I could tell that I didn't get increased gold output after building the buildings.

When looking at trade route yields they were the same (roughly) for cities with and without the buildings do in that case the display actually indicated it didn't work.

I'd certainly like to test this again before you close the issue but I'm happy if it works.

On 2. mar. 2014 07.39.21 CET, GrantSP notifications@github.com wrote:

Not sure what you mean by 'being just a display thing'.

How did you test it to verify that?


Reply to this email directly or view it on GitHub: https://github.com/Thalassicus/cep-bnw/issues/145#issuecomment-36447672

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

GrantSP commented 10 years ago

Okay, I'm doing another test with just a simple map and one city.

I've got 2 caravans giving 2 Gold each. The Gold Income in the CityView shows 11 Gold, 7 from buildings & terrain and 4 from the trade.

Next turn I build the bazaar (Arabic Market variant) The Gold now shows an extra 6(rounded down) 25% for the modifier and 2 extra for each trade.

Okay a couple of turns later and the values are: 3 Gold from terrain, 6 from Buildings and 5.21 from trade plus the 25% I'm just about to give the East India Company.

Now the city gets an extra 5 Gold into the treasury, 4 +25% from the Company plus, the trade from the city is 5 Gold 4 + 25%. The description seems to imply it is 4 Gold for each trade route, perhaps it is only a 4 Gold for trade out of the city, taken as a whole, not each route.

GrantSP commented 10 years ago

Looking closer at the descriptions and the code of all the buildings that use either: TradeRouteRecipientBonus &TradeRouteTargetBonus compared to those that have: TradeRouteLandGoldBonus or TradeRouteSeaGoldBonus I think that is the result.

The recipient & target bonuses are just a per city rate whereas the LandGold or SeaGold bonuses are on each route.

GrantSP commented 10 years ago

So it would seem your opening post is indeed correct. Only our understanding of the results needed to be enhanced.

skodkim commented 10 years ago

Sorry if I'm being daft here but I don't understand how the buildins work from your description.

Could you elaborate?

GrantSP commented 10 years ago

The buildings with TradeRouteRecipientBonus or TradeRouteSent donation get the 4 gold for each trade route. It is just for each city. So if city has 4 trades to only 2 cities it gets only 2 * 4 gold. But if 5 cities trade with it it will get 5 * 4 gold from them.

Buildings like caravansary get gold for each route. So 1 city with 7 trade routes to anywhere get the bonus for each route.

East India Company is designed to attract as many different routes as possible.

skodkim commented 10 years ago

Just tested this again today and I'm still not sure how it works:

\Skodkim