Project

General

Profile

Feature #797697

loosing some gold while diplomat/spy failed to incite revolt

Added by Gyubal Wahazar 8 months ago. Updated 3 months ago.

Status:
Closed
Priority:
Normal
Category:
Server
Target version:
Start date:
Due date:
% Done:

80%

Estimated time:

Description

If diplomat or spy is eliminated during attempt to incite revolt, he/she may already carry some gold to support this operation.
This gold would be lost forever or transferred to the defender authorities (transfer tax is deduced in such case).

Two parameters are used to control this behavior:
diplgoldlostprop - probability that gold will be lost (because was carried by eliminated agent)
diplgoldcaptprop - probability that gold lost by attacker will be transferred to defender.

Only half of the gold prepared for action can be lost
(or even less than half in case of diplomat with higher veteran level,
if he/she was caught during first phase of infiltration, before diplomatic battle)

incitegoldlost.patch (7.29 KB) incitegoldlost.patch Gyubal Wahazar, 2019-01-13 12:55 PM
incitegoldlost2.patch (7.25 KB) incitegoldlost2.patch revised version Gyubal Wahazar, 2019-02-10 01:21 PM
0031-Add-server-settings-for-chance-of-losing-inciting-mo.patch (7.26 KB) 0031-Add-server-settings-for-chance-of-losing-inciting-mo.patch Marko Lindqvist, 2019-06-14 09:52 PM

History

#1 Updated by Gyubal Wahazar 8 months ago

Target version is master (3.1 by now)

#2 Updated by Zoltán Žarkov 8 months ago

First some coding style and nits:

  • Suggest calling the options incite_gold_loss and incite_gold_recovery (nit).
  • Whichever name you pick, do it with the underscores in the setting name and in the variable names.
  • Follow doc/CodingStyle https://github.com/freeciv/freeciv/blob/master/doc/CodingStyle, keeping line length limit 77 and indents. Sorry that there is no way to configure clang-tidy to keep those stupid doxygen //.
  • No need to declare int odds, just compare fc_rand to the variable in server directly.
Beyond the coding style, I have some preferences on behavior:
  • Power factor of diplo should not affect the amount of gold lost in either case.
  • Diplgoldcost should not be reused for the "tax" on recovered gold. My preference for games would be for gold loss and gold recovery probabilities be 100, and there are cases in longturn.net games where you would want the tax on recovered incite failures to be higher than the tax of treaty gold from teammates or allies.

#3 Updated by Akechi . 8 months ago

I think if odds = 0, there are 1% chance to pass condition.
It seems
if (fc_rand (100) < odds)
instead of
if (fc_rand (100) <= odds)
(incitegoldlost.patch server/diplomats.c Line 819 and 827)

////////
I think power_fact is 100 or higher, so revolt_cost of below calculation is divided by 102+.
It is ok?

    if (diplomat_goldlost(pplayer, cplayer,
                         revolt_cost / (2 + vunit->power_fact))) {

(incitegoldlost.patch server/diplomats.c Line 917)

#4 Updated by Marko Lindqvist 8 months ago

  • Target version set to 3.1.0

#5 Updated by Marko Lindqvist 7 months ago

Is a new version of the patch coming?

#6 Updated by Gyubal Wahazar 7 months ago

Marko Lindqvist wrote:

Is a new version of the patch coming?

Yes, tomorrow, I need to test it prior.

#7 Updated by Gyubal Wahazar 7 months ago

Cleaned up and simplified code.
Removed tax and veterancy influence.
Because diplomat can be eliminated in two stages, amount of lost money depend on elimination case:
quarter, if diplomat was caught due to diplchance,
half, if was eliminated during diplomatic battle. Thus, trying to incite city defended by diplomat can cost more.

#8 Updated by Marko Lindqvist 7 months ago

- In the prototype of diplomat_infiltrate_tile() first parameter is renamed as actlayer. 1) should be actplayer. 2) function definition should be changed to match.
- diplomat_gold_lost() has issues with usage of '{}'. 1) Even single line after if or else should be surrounded by '{}' 2) Beginning '{' should come at the end of the if or else -line, not in separate line

#9 Updated by Marko Lindqvist 4 months ago

Is there a fixed (see previous comment) version of the patch coming?

#10 Updated by Marko Lindqvist 3 months ago

  • Status changed from New to In Progress

#11 Updated by Marko Lindqvist 3 months ago

Overall facelift for the patch. Corrected coding style and renamed things (notably: '_prop' -> '_chance' to be consistent with other settings)

#12 Updated by Marko Lindqvist 3 months ago

  • Status changed from Resolved to Closed
  • Assignee changed from Gyubal Wahazar to Marko Lindqvist

Also available in: Atom PDF