Page MenuHomeWildfire Games

Make cart team bonus actually be +10% instead + 0.1
AbandonedPublic

Authored by Angen on Jul 19 2019, 9:12 AM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

While trying to understand problem of one players with cart trade bonus for ships and team bonus for markets (what I do not see what problem is even now) at this discussion https://wildfiregames.com/forum/index.php?/topic/26506-update-ur-game/page/5/&tab=comments#comment-380117, I found out there is + 0.1 trade bonus for cart allies instead adding 10%

Test Plan

:)

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8453
Build 13810: Vulcan BuildJenkins
Build 13809: arc lint + arc unit

Event Timeline

Angen created this revision.Jul 19 2019, 9:12 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/131/display/redirect

Angen added a reviewer: Restricted Owners Package.Jul 19 2019, 9:34 AM
Nescio added a subscriber: Nescio.Jul 19 2019, 10:45 AM

Please have a careful look before changing this.
This aura affects entities with the “Market” class; doing a grep -r Market reveals we should look at the following templates:

campaigns/campaign_city_minor_test.xml
campaigns/campaign_city_test.xml
template_structure_economic_market.xml
template_structure_military_dock.xml
structures/cart_dock.xml
structures/cart_market.xml

The first and second don't actually have a <Market> node, the third and fourth have:

<Market>
  <TradeType>land naval</TradeType>
  <InternationalBonus>0.2</InternationalBonus>
</Market>

to which the fifth and sixth add:

<Market>
  <InternationalBonus op="add">0.1</InternationalBonus>
</Market>

Then we also might want to have a look at simulation/data/technologies/trade_commercial_treaty.json, which has:

	"tooltip": "Market +10% International Bonus.",
	"modifications": [
		{ "value": "Market/InternationalBonus", "add": 0.1 }
	],
	"affects": ["Market"],

As far as I understand it—please correct me if I'm mistaken—the international bonus is a bonus based on what you'd get when trading with your own markets. In other words, 0.1 means 10% more than non-international trade.
What this proposed patch actually does is increase the bonus by 0.02 (= 10% of 0.2), i.e. 2% of normal gain, rather than the 10% it does now.
So if something is to be changed, then probably the tooltips, not the values.

Angen abandoned this revision.Jul 19 2019, 10:51 AM

yes, you are correct, my bad, internationbonus is used as multiplier

Currently I'm looking at the template_unit_ship_merchant.xml's tooltip. Is the “Garrison a Trader aboard for additional profit (+20% for each garrisoned).” part actually implemented? If so, where? If not, wouldn't it be better to remove that sentence then?

Angen added a comment.EditedJul 19 2019, 11:07 AM

yes it is, see Trader.js calculateGain

Thanks! Found it, simulation/components/Trader.js line 5:

// Additional gain for ships for each garrisoned trader, in percents
const GARRISONED_TRADER_ADDITION = 20;

Wouldn't it be better to define that value inside the trader template, rather than hard-code it in this javascript component?

sure it would :) hardcoded number is bad number :) wanna do patch ?

Nescio added a comment.EditedJul 19 2019, 11:26 AM

Yes, I'd like to have one; no idea how to do it myself, though. Perhaps you could help?
[EDIT]: the Persian trader has +25% base gain, so it should presumably add 25% (= 125% of 20%) when garrisoned in a ship, rather than the default 20%.