Opened 8 years ago

Closed 5 years ago

#3697 closed enhancement (wontfix)

[PATCH] Improve the trading mechanism

Reported by: wraitii Owned by:
Priority: Should Have Milestone:
Component: Simulation Keywords: design patch
Cc: Patch:

Description (last modified by wraitii)

Following this initial post and this revised ideain the forums.

This ticket aims to discuss potential improvements to the trading system, which currently become the main source of resources in the late-game (in team games in particular) and lead to the construction of 100-traders long "lines" of caravans.

This is bad because:

  • It just looks bad.
  • It's slow on the pathfinder, which is a real problem in MP games
  • It makes distance too important, and small distance variations will drastically improve/decrease the efficiency (meaning you get the weird looking "markets at the border of maps" effect)
  • It makes it hard to raid trading effectively, you just have too many traders.
  • It also means you usually rely on one market, which is unrealistic and (paradoxically with the last point) dangerous.

My "v2" idea, which is implemented in a linked patch, is to shift the resource-creation from traders to markets (or rather entities with a "tradeIncome" component).

Basically, markets now generate income at a fixed rate, depending on the number of trade routes active between them and other markets.

This is further modulated by: -Each trade route has a "base rate bonus" depending on distance, where distance is logarithmic: so it increases drastically until a certain distance and then is only marginally interesting >> reducing the "edge of map" issue -You must garrison workers in your markets to leverage full efficiency. This is done because trading should only become viable when other resources have gotten depleted, or at least far into the game, so it still takes some pop.

I've tried making the interface as clear as currently possible without big changes. The actual implementations details are sorta tricky, but the basic idea is simple: garrison your markets and have more trade routes and more markets.

To make sure that players can't just build more markets, I link markets to CC. Each CC supports one market, and markets must be build not too far away from a CC. This particular link can be discussed, but some limit should be put.

I'm creating a trac ticket because response has been positive overall from many members in the forums, and we do have serious trouble with out current trading model.

Attachments (7)

Trade.patch (42.0 KB ) - added by wraitii 8 years ago.
First version (probably not RC)
trade.png (3.6 KB ) - added by wraitii 8 years ago.
Trade Icon used for markets and traders. In To be put in textures/ui/session/icons
trade-v2.patch (37.4 KB ) - added by wraitii 8 years ago.
trade-v2.2.patch (44.5 KB ) - added by wraitii 8 years ago.
trade-v2.3.patch (55.6 KB ) - added by wraitii 8 years ago.
commands_r17776.txt (759.1 KB ) - added by elexis 8 years ago.
A testgame with the latest patch and r17776, some errors were thrown.
trade-v2.4.patch (49.5 KB ) - added by elexis 8 years ago.
Rebased, style fixes, Ally -> MutualAlly, string changes, conn[0] and similar error fixed, whitespace fixes, remove unused bonus, missing semicolons, removed water-patch, let instead of var, typo "Strenght"

Download all attachments as: .zip

Change History (27)

by wraitii, 8 years ago

Attachment: Trade.patch added

First version (probably not RC)

by wraitii, 8 years ago

Attachment: trade.png added

Trade Icon used for markets and traders. In To be put in textures/ui/session/icons

comment:1 by wraitii, 8 years ago

Known caveats with the above patch:

  • Forgot to reimplement Loot using the basic "loot" components. I think traders should give a high loot (25 of each?) but the loot no longer special-cases trading.
  • Technologies have mostly become invalid
  • This functionality definitely needs a tutorial.

comment:2 by Palaxin, 8 years ago

Will this somehow affect #3428?

comment:3 by wraitii, 8 years ago

Yes, attached patch makes it completely irrelevant whether or not you garrison traders inside. This could be changed so that traders inside a merchant ship allow having fewer merchant ships to keep the trade route alive, but wouldn't otherwise change efficiency.

comment:4 by Palaxin, 8 years ago

Ok I would leave that ticket to you since it depends on this ticket. I also currently don't have the time to have a look at the new market mechanism in detail and rather work on my mineral mod instead.

comment:5 by elexis, 8 years ago

I didn't test the patch ingame yet, but it sounds promising :)

With regards to the coding style:

  • Having font-tags inside the translation is just asking for trouble imo
  • There is a + +
  • Do you have to check for === false or isn't a negation enough?!
  • Some var inside if-blocks. var shouldn't be used anywhere at all imo
  • for each is deprecated
  • for each (let conn in nextMarket.tradeIncome.connections) don't define connection inside of it. You can probably just remove the whole loop and use an array function (probably find)
  • missing translate for This trader is trying to establish a trade route, but the number of traders isn't high enough for this trade route to be viable yet
  • Don't have commented out code
  • let caption can be nuked
  • Trader.prototype.GetOriginMarket parenthesis not needed (check JS operator precedence)
  • Start comments with a capital letter
  • TradeIncome.prototype.GetConnections could just use the array function map
  • connectionEffect = Math.sqrt(connectionEffect); and the two lines after that can be unified
  • var currentCount whitespace issue

comment:6 by wraitii, 8 years ago

Missed your review, sorry about that.

There's a few other places where it's the same but I guess that could be improved.

The "=== false" is mostly because I like to be as explicit as possible about my checks when it's not necessary to encompass several answers, change it if you want.

I'll try to get around to changing the other ones later, particularly the for each.

comment:7 by elexis, 8 years ago

Keywords: review removed

Any updates? Tried to rebase the patch, but couldn't integrate the AI trade multiplier #3722 before fully grasping the patch. Checked the partially broken version which still made a good impression on me :)

comment:8 by wraitii, 8 years ago

Rebased patch, added the AI trade modifier as a modifier of income, fixed the trader loot issue simply, fixed other issues noted by elexis above (might have a missed a few).

Still need a solution for docks which can currently be put next to each other and cheat, perhaps a negative aura?

Haven't really been able to reproduce the trade exploit but maybe I understand it wrong?

by wraitii, 8 years ago

Attachment: trade-v2.patch added

by wraitii, 8 years ago

Attachment: trade-v2.2.patch added

by wraitii, 8 years ago

Attachment: trade-v2.3.patch added

comment:9 by mimo, 8 years ago

I've tried the patch on the demo trade map, and I must say i'm not convinced that it is better for gameplay than the previous approach. I admit than I've just played with it a few minutes, so maybe it is that it needs more time to get used to it, but in general i have a feeling of things being muddled, with the player not knowing what is best to gain resources, and what are the best ways to achieve maximum performances with minimum costs (i.e. garrisoning workers in the market or having more traders).

I've only tried to make a trade route with one trader, and noticed several problems:

  • when a trader goes from A to B, B is said to have no route while A has one. Then when the trader arrives a B and goes towards A, it is A which is said to have no route while B has. This is quite disturbing and adds to the impression of being muddled.
  • also, the trader is said "trying to establish a trade route" all that time, while it already did some round trips. Also it is said that the "number of traders is not enough to have a viable route" (so there is in fact a route, not only trying to establish one ? and what is a non-viable route, does it loose resource ?)
  • in addition, the trade efficiency and income seems to change with time ? is it expected ? not clear what is the difference between the efficiency and the average trade efficiency ?

by elexis, 8 years ago

Attachment: commands_r17776.txt added

A testgame with the latest patch and r17776, some errors were thrown.

comment:10 by elexis, 8 years ago

  • one can build unlimited docks -> traderoutes, is this intended considering one can build only one market per CC?
  • Should be more transparent
  • The water patch water_high.fs should vanish

comment:11 by wraitii, 8 years ago

  1. Known issue. A fix would be committing my "upgrade" patch as well and only allowing trading in docks that have been upgraded to trade, so that you can apply distance restrictions.We can just blindly use a number because you might actually need multiple docks close by to build transports. Another fix, not as good, would be to give docks a negative aura so that multiple docks are useless. If we implement the upgrading stuff (my preferred solution), we could also tie it up with the CCs ( see below why)
  1. Yup, my bad.
  1. So it seems like the biggest complaint is that the GUI isn't very clear. I can only agree, but here is the data you'd need to show:

-number of trade routes -Efficiency based on the garrisoned workers (potentially detailed if different workers can give different efficiency) -For each trade route, find a way to show with which market we are trading (since they aren't named). Then show the number of active traders on that route, and the connection strength, and whether this strength is sustainable given the traders.

Since the gain formula isn't linear (adding a new trade route always improves the gain, but by a lower amount each time) it night not be worth showing an absolute gain per trade route. I dunno. With the way the numbers work now, it's probably best to get to 2/3 working trade routes then garrison up.

In my opinion, showing all this info needs a separate screen. It's not very complicated to code, but it'd need to be added.

Finally, if we go with this solution, I think that markets need a simple way to designate them. The simplest solution I can think of is to tie them with a CC (as is already done), including trading docks, and to give a name to the CC. SO this would be CITY's market. It would also make the game feel a lot more alive and vibrant imo.

comment:12 by wraitii, 8 years ago

FYI, if you activate "Display selection state" on a market, you can see this information, so you can get a hunch of how it really works.

comment:13 by fatherbushido, 8 years ago

Your ideas / patch are very interesting and in any case trading must be improved. Some remarks and some questions about gameplay.

  • Some Math.round() are needed (for stats and tooltip).
  • I got those warnings while playing with some AIs on Oasis map
WARNING: JavaScript warning: simulation/ai/petra/mapModule.js line 71
reference to undefined property template.buildDistance(...).MinDistance
WARNING: JavaScript warning: simulation/components/BuildRestrictions.js line 287
reference to undefined property this.template.Distance.MinDistance
WARNING: JavaScript warning: gui/session/selection_details.js line 185
  • don't you think it will incite to turtling instead of territory control ?
  • Some civs like ptolemies or seleucids could'nt build a second market before phase 3. It can be annoying in 1v1 (especially for seleucids, since ptolemies have the free houses).
  • i don't like the name idea, i think it would be better for an eco mod.
Last edited 8 years ago by fatherbushido (previous) (diff)

comment:14 by elexis, 8 years ago

  • I see docks have -20% efficiency, this would help addressing the dock problem
  • Repeating the steps in #3277 doesnt trigger an error here, besides showing NaN in the route tooltip of the trader
  • Garrisoning units seem to be a bit overpowered. One trade cart between markets is enough if they are garrissoned (doesn't seem right to me)
  • The techs need to be changed or removed, otherwise committing this patch doesn't make sense
  • Also got this warning:
    WARNING: JavaScript warning: gui/session/selection_details.js line 185
    reference to undefined property conn[0]
    
  • Trading only works with a MutalAlly
  • Still the water patch is incorrectly included in this one
  • Typo ConnectionStrenght
Last edited 8 years ago by elexis (previous) (diff)

by elexis, 8 years ago

Attachment: trade-v2.4.patch added

Rebased, style fixes, Ally -> MutualAlly, string changes, conn[0] and similar error fixed, whitespace fixes, remove unused bonus, missing semicolons, removed water-patch, let instead of var, typo "Strenght"

comment:15 by Itms, 8 years ago

Milestone: Alpha 20Alpha 21

comment:16 by elexis, 8 years ago

Notice GetGarrisonedArcherCount had been removed with #3196.

comment:17 by elexis, 8 years ago

Everyone who has checked this patch should make sure to delete those two files, otherwise it can produce an OOS when playing with a reverted svn:

binaries/data/mods/public/simulation/components/TradeIncome.js
binaries/data/mods/public/simulation/components/interfaces/TradeIncome.js

comment:18 by elexis, 8 years ago

Milestone: Alpha 21Backlog

Backlogging due to lack of progress.

comment:19 by Imarok, 5 years ago

Component: UI & SimulationSimulation

Move tickets to Simulation as UI & Simulation got some sub components.

comment:20 by wraitii, 5 years ago

Description: modified (diff)
Milestone: Backlog
Resolution: wontfix
Status: newclosed

Won't commit as is - I believe the arguments outlined in the ticket description remain valid, but we'll have to find another way.

Note: See TracTickets for help on using tickets.