[freeciv-ai] Re: definitely last version of active diplomats patch

[Gregory Berkolaiko <Gregory.Berkolaiko@xxxxxxxxxxxx>]

Few points:

* You should put more comments.  They don't hurt.

* In ai_choose_diplomat_offensive you should either use or synchronize 
code with is_diplomat_action_available.  In particular, it seems that one 
cannot steal tech from barbarians.

* Some code is tri-plicated in ai_choose_diplomat_offensive, 
find_city_to_diploma and ai_diplomat_city: 

+    city_incite_cost(acity); /* prime variable */
+    incite_cost = acity->incite_revolt_cost;
+    if (pplayer->player_no == acity->original) {
+      incite_cost /= 2;
+    }
....
+        && (acity->incite_revolt_cost <
+            pplayer->economic.gold - pplayer->ai.est_upkeep)) 

You should separate it into a function, say 
bool ai_is_incitement_affordable.  This will also help you avoid mistakes 
like taking pplayer->ai.est_upkeep into account twice in 
ai_diplomat_city ;)  Unless, of course, it is intentional (then why? 
please comment!):
+  if (pplayer->ai.control) {
+    gold_avail -= pplayer->ai.est_upkeep;
+  }


On Tue, 3 Sep 2002, Per I. Mathisen wrote:

> But:
> Please don't complain about the incite algorithm. Once the code is in cvs,
> it is much easier to test out new, experimental algorithms. I've tested
> this one for many, many hours and it seems to work, so I don't want to
> change it now.

Please add a comment to that effect in the code, so next generations don't 
have to spend sleepless nights trying to figure out what you meant by this 
particular multiplication by SHIELD_WEIGHTING ;)

G.