[PATCH] Changing the trading gain

[mimo]

In the present implimentation, when a trader arrives at a market, it has all the gains. It would be nice, in order to add possibilities with alliances, that the gains be shared between the trader and the market. The attach patch does that, giving a 1/3 share to the trader and 2/3 share to the market (this percentage can still be easily modified as it is defined in a const).

[gudo]

Forum Thread: http://www.wildfiregames.com/forum/index.php?showtopic=16618
See also: http://www.wildfiregames.com/forum/index.php?showtopic=15258&st=20#entry245186

[mimo]

small changes following discussions on forums

[Kieran P]

I'm pushing this to the next release. I'd like to see technology hooks implemented before committing this. Anything new we add should have technology hooks wherever possible. In otherwards, there could be a technology that gives the trader more of the trade gain for example. You don't have to make the technology, just add the right calls to ApplyModifications, and make the tech name "Trade/Gain" or something similar (easy to change later though).

[mimo]

Replying to k776:

I'm pushing this to the next release. I'd like to see technology hooks implemented before committing this. Anything new we add should have technology hooks wherever possible. In otherwards, there could be a technology that gives the trader more of the trade gain for example. You don't have to make the technology, just add the right calls to ApplyModifications, and make the tech name "Trade/Gain" or something similar (easy to change later though).

I'm not sure to follow what you mean : I suppose technologies will affect the trading gain or the trader's travelling speed, but do you also want it to change the sharing of the gain between the trader and the market ? the only thing that this patch does is to add such a sharing, without modifying anything in the way the total gain is computed.

So while I agree that this gain computation should be tech-aware, this is completely independant of this patch and I think this should go with a more general review of the way the trading gain is computed.

If people still believe that the sharing should also depend on technology, I can still make this change. But I think that's a bit of overkill : the main interest of this change was to add some ways to cooperate with your allies, and this is much reduced since the introduction of tributes.

[historic_bruno]

Replying to k776:

I'm pushing this to the next release. I'd like to see technology hooks implemented before committing this. Anything new we add should have technology hooks wherever possible. In otherwards, there could be a technology that gives the trader more of the trade gain for example. You don't have to make the technology, just add the right calls to ApplyModifications, and make the tech name "Trade/Gain" or something similar (easy to change later though).

On a related note, global variables are really evil, especially in component scripts. I wish we would find a solution for barter and trade that doesn't involve global constants (by storing those values in a per-entity or per-player component as needed).

[mimo]

Here is a new version of the patch, following http://www.wildfiregames.com/forum/index.php?showtopic=17114 In addition, the tooltips when creating the trade routes are now updated.

[historic_bruno]

Updated with conflicts resolved up to r13500

[sanderd17]

Looks like a good patch, but I have two remarks:

• The tooltip could be structured a bit better, something like

  Your gain: 12 metal
  Player2's gain: 2 metal
  

instead of trying to cram everything up in one line.

• You seem to have removed the information about how much resources a trader is carrying. Or did I miss that? It would be nice if the trader could show (12 + 2) when he is carrying 12 metal for the owner, and 2 for the second market.

[mimo]

Replying to sanderd17:

Looks like a good patch, but I have two remarks:

• The tooltip could be structured a bit better, something like

  Your gain: 12 metal
  Player2's gain: 2 metal
  

instead of trying to cram everything up in one line.

I would tend to agree with you. But I think everybody has its own idea of the best tooltip's layout. There may also be discussion on its content. So improving it would be a second step, after this patch is included.

• You seem to have removed the information about how much resources a trader is carrying. Or did I miss that? It would be nice if the trader could show (12 + 2) when he is carrying 12 metal for the owner, and 2 for the second market.

nice catch, thanks. I just forgot to modify it ! I'm going to upload a fixed version.

[mimo]

Version of the patch adapted to the latest svn

[historic_bruno]

For some reason the patch still wouldn't apply with TortoiseSVN, something to do with finding a better path, every file appears conflicted. I've manually fixed it and am attaching an updated patch

[historic_bruno]

The patch seems to work correctly. I agree with sanderd17 that the UI should be improved, and Michael was very insistent on the UI being clear before committing this patch.

If I'm understanding the way this works, it says for example, "60+12" on a route with one of my allies. So that means the base gain is 48, the international bonus of 25% is 12, and both I and my ally are getting that bonus?

One thing I don't like is the gain values shown in the selection details panel, it's just some numbers separated by plus signs. It's not clear but it also doesn't look good, especially if you trade between allies, it often takes up 2 lines, and with larger gain values there's no way it could fit in that space. Maybe we could list the owner's gain there, since it's most important, then add a tooltip with ally shares? It's not something the player needs to see often, it's more important for route setup.

For the origin/destination market tooltips, I agree the gain should be on a separate line, maybe this is enough:

Set a destination trade market.
Gain: 43 metal (you), 9 metal (player 3)

Click to establish a default trade router for new traders.
Gain: 21 metal (you), 4 metal (player 2) <-- should say player name instead of "the allied"?

Origin trade market. Right click to create a new trade route.
Gain: 16 metal (you), 3 metal (player 2)

(Notice how inconsistent the tooltips already are, "set" vs. "click" vs. "right click")

[historic_bruno]

Also it doesn't currently mention the 25% bonus, the user has to deduce that from the gain values, which isn't intuitive and would be even worse with tech modifications. Is that important for the user to know, how could we convey it?

[mimo]

I still believe it is quite inefficient to discuss tooltips at this stage, as I'm sure there will be new propositions after the patch is commited, and managing 2-lines patches for a tooltip is much easier. But nonetheless, I've just noticed that the original tooltips (in the present svn) were quite ambiguous: for example, the tooltip saying to right-click to set the second market appears only after we have right-clicked. So I've fixed that, and taken this occasion to modify a bit the tooltips according to your and sanderd17's propositions. And to allow anybody to easily modify it, it is now in a function in utility_functions.js.

For the gain value in selection_details panel, I've modified it to only show the total gain while the details are in a tooltip (thanks to ticket #1871 patch, this panel has now a tooltip).

To keep track of the bonus, the gain is now indicated as "Gain (metal): 13+3 (you), 3 (player 2)" instead of "Gain (metal): 16 (you), 3 (player 2)"

And last, I've noticed that I forgot to update the Looter part. This is now done in the new version of the patch.

[sanderd17]

There seems to be something wrong with v3. It looks like it's no longer possible to create trade routes and I get no tooltips when hoovering a market with a player selected.

[mimo]

I've just tried again with a clean version and applying the patch, and everything works (I can create trade routes and have tooltips). On which svn version are you ?

[historic_bruno]

The tooltips look much better now, thanks. I'll do some final testing and commit.

Replying to sanderd17:

There seems to be something wrong with v3. It looks like it's no longer possible to create trade routes and I get no tooltips when hoovering a market with a player selected.

Were you testing in Atlas? I've noticed that for some reason tooltips don't work reliably in Atlas. Also note trading only works with your own or allies' markets.

[ben]

In 13567:

Implements shared trade gain with allies, patch by mimo, fixes #1691