[PATCH] Moving the international bonus constant from simulation/helpers/TraderGain.js to template

[fatherbushido]

In simulation/helpers/TraderGain.js, there is a global constant for the internation bonus trading. Then there value modification compute the tech/auramodification of Trade/International. So i suggest to put that constant in the Trader component. So it can be modified in templates.

[fatherbushido]

As an example, i put an extra bonus for carth relative to something from the design docs (it can be perhaps implemented too as team bonus for all allies).

TB1 Name: Trademasters History: The Phoenicians and Carthaginians were broadly known as the greatest trading civilization of the ancient and classical world. Effect: +33% trade profit ‘international’ routes for allies.

[fatherbushido]

I will rework the patch and clean TraderGain.js

[mimo]

Thanks for working on this patch. Here are some comments:

• InternationalBonus: wouldn't it be more logical to make it a Market element (we now have a Market component) rather than a Trader element ?
• be careful with the GetTradeRateMultiplier ! lines 57 and 61 were an assignment in cmpPlayer to have the right TradeRateMultiplier of the market owner, while in the patch, it is always the one from the trader (a player should not take advantage of the AI bonus when an AI is trading with him).

While working on it, we could also add (either in this ticket or a new one):

• a tech for this internationalBonus (a placeholder would be to decrease a bit its initial value to 0.20, and having a cheap tech which would add 0.10, could be a "Commercial Treaty" or something like that)
• we could also remove the DISTANCE_FACTOR const and merge it with the GainMultiplier element inside a new GainPerUnitSquaredDistance (or some better name as we may in the future go away from the gain proportionnal to the squared distance). That would be cleaner, and would avoid to have to redefine this constant in the AI as is presently done.

[fatherbushido]

Thx for the review, i hope i adressed all the remarks. I didn't yet merge the DISTANCE_FACTOR.

[fatherbushido]

V6 with the constant merge. I didn't yet change the name of GainMultiplier. I didn't look at Petra. (Perhaps the Commercial Treaty should require the unlock_shared_los or unlock_shared_dropsite tech ?).

[mimo]

thanks for the changes. Here are some comments still on v5 (i didn't noticed you had a new version). And don't worry for the petra changes, they can be done independantly (so after this one is commited).

in Trader.js, the InternationalBonus in the schema example should be removed.

in TraderGain.js lines 6-7 and 23-26 would be better imo as: let gainMultiplier; if (trader) {

let cmpTrader = Engine.QueryInterface(trader, IID_Trader); if (!cmpTrader)

return null;

gainMultiplier = cmpTrader.GetTraderGainMultiplier();

} else called from the gui, modifications already applied {

if (!traderTemplate)

return null;

gainMultiplier = traderTemplate.GainMultiplier

1;

}

lines 27-30 could be moved just after 16, as well as 43-45

As the internationalBonus applies only when market1Owner != market2Owner, i think it is cleaner to have all the code related to it inside a block if (ownerMarket1 != ownerMarket2) {

...

} instead of all these ternaries

so lines 40-41 would go in this block and be simply let market1PlayerBonus = cmpMarket1.GetTradeRateMultiplier(); let market2PlayerBonus = cmpMarket2.GetTradeRateMultiplier(); also lines 47-48 would go in this block, as well as 50-51.

[fatherbushido]

Thx for the remarks!

[mimo]

The patch looks nearly ok now :) although I have not yet tested it. Just a few last comments

Trader.js

• change the help in the Trader Schema for the GainMultiplier, it should be now something like "trader gain for a 1m distance"
• and I propose to change it to "trader gain for a 100m distance" so that the value will be more readable (0.756 instead of 0.0000756, and maybe round it to 0.75).

TraderGain.js

• tabs in lines 3 to 17
• lines 21 to 37 should be the first when entering the function (the lines dealing with position being last). There is no need to compute the gainMultiplier if we have to return null
• line 43 -> gain.traderGain = distanceSq * gainMultiplier / 10000; if switching to 0.75

[mimo]

In 18108:

Move the const from simulation/helpers/TraderGain.js into the templates (InternationalBonus and GainMultiplier)
Add a tech to improve the internalBonus and give an improved bonus to cart
Patch from fatherbushido, fixes #3917

[mimo]

Thanks for the patch