[PATCH] Remove the hardcoded trading gain in petra

[mimo]

Since r18108, the trading gain is no more hardcoded in the simulation, but available in the trader template (GainMultiplier). Petra should now use it, instead of the distUnitGain value hardcoded in ai/petra/config.js. This value is used at two different places inside petra to compute the expected gain from a new market, and should be replaced by the one extracted from the template (either the trader template for a land route, or the merchant template for a naval route).

[mimo]

As there was a question on IRC today on how to get template values for the ai, the best is to add a function in the Template class of ai/common-api/entity.js (look at the other functions there as example), so that it is accessible from any petra entity (see how the other functions from entity.js are used in the petra code).

[Stan]

While cleaning whitespaces is good I guess you shouldn't be doing it in files where you change no code =) Apart from that looks good.

[mimo]

Thanks for working on the patch.

in programming.json, the list is in alphabetical order, so insert your name at the right place. And don't forget the comma (was missing on the previous line in your patch).

we do not use braces for single-line block (lines 550-552 of entity.js)

For new code, we should foresee mods which have disabled templates. You can use the gameState.isDisabledTemplates function (there are several examples in the code). And also for mods who do not provide the expected templates, so test that gameState.getTemplate(templateName) is not undefined.

Then, you should use the gain corresponding to the route you are testing. So let's take the case of tradeManager (the other one should be similar). We loop there on all possible pairs of markets, so there are no need to query the templates for each pair. This could be done at the beginning of the function, storing both the landGain and the navalGain (undefined if template is disabled or does not exist). And in line 411, according to the type of route, you use the right gain (or continue if the gain is undefined).

[mimo]

tradeManager:

• lines 396 and 397: these temporary variables are only used once, so no need to define them.
• a same route can be both land and naval (between two docks on the same island). So the lines 421 to 423 should be

  let gainMultiplier;
  if (land && landGainMultiplier)
  	gainMultiplier = landGainMultiplier;
  else if (sea && navalGainMultiplier)
  	gainMultiplier = navalGainMultiplier;
  else
  	continue;
  

• also it is good practise to test again the patch between iterations (line 421 you have navalGainMultiplierland, and line 396 shipMerchantTemplate should be shipMerchantTemplateName).

headquarter:

• here, we look for the best market position, choosing the tile which give the maximum dist*dist. But as the gain could be different for land and naval trade, we should change it to maximize gainMultiplier*dist*dist [if (isNavalMarket && market.hasClass("NavalMarket")), we should use the navalGain, otherwise the landGain].
• so as in tradermanager, both gain should be computed at the entrance of the function. And as the same code would thus be used twice, maybe put it in a function which would return an object with both gains.

[mimo]

the patch looks good to me (i've not yet tested it, will do it this weekend). Just a few details:

line 1083, i would replace if (gainMultiplier === undefined) by if (!gainMultiplier) so to skip also if the multiplier = 0 for any reason

replace some ternary operators with logical expressions as suggested by elexis

[mimo]

In 18168:

remove the hardcoded gain in petra, fixes #3943, patch by LittleDev

[mimo]

Thanks for the patch