[Freeciv-Dev] Re: (PR#2650) Overflow in military_amortize

["Per I. Mathisen via RT" <rt@xxxxxxxxxxxxxx>]

On Thu, 26 Dec 2002, Gregory Berkolaiko via RT wrote:
> > This avoids the pathological case of most-cities-have-zero-surplus, which
> > means the one city with lots of surplus shields will think "gee, it takes
> > 1243928 turns to build that tanks and drive it to the enemy, so lets
> > not!", even though it could build it in 3 turns and drive it there in 1!
>
> true true
>
> but it also tells the city with two shield surplus to give up hope building
> units although it couldbe useful too.

MAX(pcity->shield_surplus, average_production), then?

> > See the attached patch which implements this.
>
> Not attached :(

Just as well. It had a grevious bug. Now a new one is attached using the
MAX above.

> > I would guess the maintainer to commit such a patch would do some play
> > testing and comparisons of it, though ;)
>
> Testing? No thanks :)
>
> But,seriously, I can't really think of a way to test it other than visually.
> Maybe only how longer/shorter it takes the dominant civ to take over the 
> world?

You could compare the output from old military_amortize() with the new
one and see which is best for various cases? Or we could commit&forget and
deal with any problems afterwards. Up to you.

> Also, maybe it'smy impression, but I feel the game gets slower and slower,
> especially GUI...Makes it very time-consuming to test-run things :(

No idea.

> But I think your way of doing now it is the most scientific-like, so we
> can remove the comment.

Scientific... mmmm, I like that ;)

  - Per

Index: ai/advmilitary.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/advmilitary.c,v
retrieving revision 1.126
diff -u -r1.126 advmilitary.c
--- ai/advmilitary.c    2002/12/24 19:38:50     1.126
+++ ai/advmilitary.c    2002/12/27 10:37:11
@@ -972,7 +972,7 @@
       desire -= move_time * (unhap ? SHIELD_WEIGHTING + 2 * TRADE_WEIGHTING
                              : SHIELD_WEIGHTING);
 
-      want = military_amortize(desire, MAX(1, move_time),
+      want = military_amortize(pplayer, pcity, desire, MAX(1, move_time),
                                bcost_balanced + needferry);
       
       if (want > 0) {
@@ -1257,7 +1257,8 @@
   }
   want -= move_time * (unhap ? SHIELD_WEIGHTING + 2 * TRADE_WEIGHTING 
                      : SHIELD_WEIGHTING);
-  want = military_amortize(want, MAX(1, move_time), bcost_bal + needferry);
+  want = military_amortize(pplayer, pcity, want, MAX(1, move_time), 
+                           bcost_bal + needferry);
   
   if (myunit->id != 0) {
     freelog(LOG_ERROR, "Non-virtual unit in kill_something_with");
@@ -1304,7 +1305,8 @@
   struct unit *aunit = NULL;
   struct city *acity = NULL;
 
-  virtualunit = create_unit_virtual(pplayer, pcity->x, pcity->y, unit_type,
+  virtualunit = create_unit_virtual(pplayer, pcity, pcity->x, pcity->y, 
+                                    unit_type,
                                     do_make_unit_veteran(pcity, unit_type));
 
   if (choice->want < 100) {
@@ -1485,7 +1487,8 @@
      before we mung the seamap */
   unit_type = ai_choose_attacker(pcity, SEA_MOVING);
   if (unit_type >= 0) {
-    virtualunit = create_unit_virtual(pplayer, pcity->x, pcity->y, unit_type,
+    virtualunit = create_unit_virtual(pplayer, pcity, pcity->x, pcity->y, 
+                                      unit_type,
                               player_knows_improvement_tech(pplayer, B_PORT));
     kill_something_with(pplayer, pcity, virtualunit, choice);
     destroy_unit_virtual(virtualunit);
@@ -1494,8 +1497,8 @@
   /* Consider a land attacker */
   unit_type = ai_choose_attacker(pcity, LAND_MOVING);
   if (unit_type >= 0) {
-    virtualunit = create_unit_virtual(pplayer, pcity->x, pcity->y, unit_type,
-                                      TRUE); /* why assume veteran? -- Per */
+    virtualunit = create_unit_virtual(pplayer, pcity, pcity->x, pcity->y, 
+                                      unit_type, TRUE);
     kill_something_with(pplayer, pcity, virtualunit, choice);
     destroy_unit_virtual(virtualunit);
   }
Index: ai/aiair.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aiair.c,v
retrieving revision 1.6
diff -u -r1.6 aiair.c
--- ai/aiair.c  2002/12/22 18:14:43     1.6
+++ ai/aiair.c  2002/12/27 10:37:11
@@ -144,7 +144,9 @@
   profit = kill_desire(victim_cost, unit_attack, unit_cost, victim_defence, 1) 
     - SHIELD_WEIGHTING + 2 * TRADE_WEIGHTING;
   if (profit > 0) {
-    profit = military_amortize(profit, sortie_time, balanced_cost);
+    profit = military_amortize(unit_owner(punit), 
+                               find_city_by_id(punit->homecity),
+                               profit, sortie_time, balanced_cost);
     freelog(LOG_DEBUG, 
            "%s at (%d, %d) is a worthy target with profit %d", 
            unit_type(pdefender)->name, dest_x, dest_y, profit);
@@ -409,7 +411,7 @@
     if (get_unit_type(u_type)->move_type != AIR_MOVING) continue;
     if (can_build_unit(pcity, u_type)) {
       struct unit *virtual_unit = 
-       create_unit_virtual(pplayer, pcity->x, pcity->y, u_type, TRUE);
+       create_unit_virtual(pplayer, pcity, pcity->x, pcity->y, u_type, TRUE);
       int profit = find_something_to_bomb(virtual_unit, pcity->x, pcity->y);
       if (profit > choice->want){
        /* Update choice */
Index: ai/aidata.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aidata.c,v
retrieving revision 1.5
diff -u -r1.5 aidata.c
--- ai/aidata.c 2002/12/23 18:09:57     1.5
+++ ai/aidata.c 2002/12/27 10:37:11
@@ -178,10 +178,13 @@
 
   ai->stats.workers = fc_calloc(map.num_continents + 1, sizeof(int));
   ai->stats.cities = fc_calloc(map.num_continents + 1, sizeof(int));
+  ai->stats.average_production = 0;
   city_list_iterate(pplayer->cities, pcity) {
     struct tile *ptile = map_get_tile(pcity->x, pcity->y);
     ai->stats.cities[ptile->continent]++;
+    ai->stats.average_production += pcity->shield_surplus;
   } city_list_iterate_end;
+  ai->stats.average_production /= MAX(1, city_list_size(&pplayer->cities));
   unit_list_iterate(pplayer->units, punit) {
     struct tile *ptile = map_get_tile(punit->x, punit->y);
     if (ptile->terrain != T_OCEAN && unit_flag(punit, F_SETTLERS)) {
Index: ai/aidata.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aidata.h,v
retrieving revision 1.4
diff -u -r1.4 aidata.h
--- ai/aidata.h 2002/12/23 18:09:58     1.4
+++ ai/aidata.h 2002/12/27 10:37:11
@@ -47,6 +47,7 @@
   struct {
     int *workers; /* cities to workers on continent*/
     int *cities;  /* number of cities on continent */
+    int average_production;
   } stats;
 
   int num_continents; /* last time we updated our continent data */
Index: ai/aidiplomat.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aidiplomat.c,v
retrieving revision 1.3
diff -u -r1.3 aidiplomat.c
--- ai/aidiplomat.c     2002/12/18 17:36:18     1.3
+++ ai/aidiplomat.c     2002/12/27 10:37:11
@@ -198,7 +198,8 @@
       return;
     }
 
-    want = military_amortize(want, time_to_dest, ut->build_cost);
+    want = military_amortize(pplayer, pcity, want, time_to_dest, 
+                             ut->build_cost);
 
     if (!player_has_embassy(pplayer, city_owner(acity))) {
         freelog(LOG_DIPLOMAT, "A diplomat desired in %s to establish an "
Index: ai/aitools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aitools.c,v
retrieving revision 1.69
diff -u -r1.69 aitools.c
--- ai/aitools.c        2002/12/24 18:08:36     1.69
+++ ai/aitools.c        2002/12/27 10:37:11
@@ -48,10 +48,12 @@
 #include "aitools.h"
 
 /**************************************************************************
-  Create a virtual unit to use in build want estimation
+  Create a virtual unit to use in build want estimation. pcity can be 
+  NULL.
 **************************************************************************/
-struct unit *create_unit_virtual(struct player *pplayer, int x, int y,
-                                Unit_Type_id type, bool make_veteran)
+struct unit *create_unit_virtual(struct player *pplayer, struct city *pcity,
+                                 int x, int y, Unit_Type_id type, 
+                                 bool make_veteran)
 {
   struct unit *punit;
   punit=fc_calloc(1, sizeof(struct unit));
@@ -64,7 +66,7 @@
   punit->goto_dest_x = 0;
   punit->goto_dest_y = 0;
   punit->veteran = make_veteran;
-  punit->homecity = 0;
+  punit->homecity = (pcity ? pcity->id : 0);
   punit->upkeep = 0;
   punit->upkeep_food = 0;
   punit->upkeep_gold = 0;
Index: ai/aitools.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aitools.h,v
retrieving revision 1.32
diff -u -r1.32 aitools.h
--- ai/aitools.h        2002/12/24 18:08:36     1.32
+++ ai/aitools.h        2002/12/27 10:37:11
@@ -27,8 +27,9 @@
   BODYGUARD_NONE
 };
 
-struct unit *create_unit_virtual(struct player *pplayer, int x, int y,
-                                Unit_Type_id type, bool make_veteran);
+struct unit *create_unit_virtual(struct player *pplayer, struct city *pcity,
+                                 int x, int y, Unit_Type_id type,
+                                 bool make_veteran);
 void destroy_unit_virtual(struct unit *punit);
 bool is_stack_vulnerable(int x, int y);
 
Index: ai/aiunit.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aiunit.c,v
retrieving revision 1.245
diff -u -r1.245 aiunit.c
--- ai/aiunit.c 2002/12/25 13:17:44     1.245
+++ ai/aiunit.c 2002/12/27 10:37:11
@@ -775,23 +775,26 @@
 }
 
 /**************************************************************************
-Military "want" estimates are amortized in this complicated way.
-COMMENTME: Why not use simple amortize? -- GB
+  Amortize a want modified by the shields (build_cost) we risk losing.
+  We add the build time of the unit(s) we risk to amortize delay, which
+  is production of unit's homecity or city where we want to produce it
+  divided by unit's build cost. If the city has less than average 
+  shield surplus, we instead use the average, to encourage long-term 
+  thinking (a target seen in short-term is likely representative of 
+  future targets as well).
 **************************************************************************/
-int military_amortize(int value, int delay, int build_cost)
+int military_amortize(struct player *pplayer, struct city *pcity,
+                      int value, int delay, int build_cost)
 {
-  int simply_amortized, fully_amortized;
+  struct ai_data *ai = ai_data_get(pplayer);
+  int build_time = MAX((pcity ? pcity->shield_surplus : 1),
+                       ai->stats.average_production) / build_cost;
 
   if (value <= 0) {
     return 0;
   }
 
-  simply_amortized = amortize(value, delay);
-  fully_amortized = ((value * simply_amortized) * 100
-                     / (MAX(1, value - simply_amortized))
-                     / (build_cost * MORT));
-
-  return fully_amortized;
+  return amortize(value, delay + build_time);
 }
 
 /**************************************************************************
@@ -1863,11 +1866,13 @@
       /* FIXME: build_cost of ferry */
       needferry = 
         (go_by_boat && !ferryboat && is_ground_unit(punit) ? 40 : 0);
-      want = military_amortize(want, MAX(1, move_time), bcost_bal + needferry);
+      want = military_amortize(pplayer, find_city_by_id(punit->homecity),
+                               want, MAX(1, move_time), bcost_bal + needferry);
 
       /* BEGIN STEAM-ENGINES-ARE-OUR-FRIENDS KLUGE */
       if (want <= 0 && punit->id == 0 && best == 0) {
-        int bk_e = military_amortize(benefit * SHIELD_WEIGHTING, 
+        int bk_e = military_amortize(pplayer, find_city_by_id(punit->homecity),
+                                     benefit * SHIELD_WEIGHTING, 
                                      MAX(1, move_time), bcost_bal + needferry);
         if (bk_e > bk) {
           *x = acity->x;
@@ -1989,7 +1994,8 @@
          * (costs 2 luxuries to compensate) */
         want -= (unhap ? 2 * move_time * TRADE_WEIGHTING : 0);
       }
-      want = military_amortize(want, MAX(1, move_time), bcost_bal);
+      want = military_amortize(pplayer, find_city_by_id(punit->homecity),
+                               want, MAX(1, move_time), bcost_bal);
       if (want > best && ai_fuzzy(pplayer, TRUE)) {
         best = want;
         *x = aunit->x;
Index: ai/aiunit.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aiunit.h,v
retrieving revision 1.38
diff -u -r1.38 aiunit.h
--- ai/aiunit.h 2002/12/25 13:17:44     1.38
+++ ai/aiunit.h 2002/12/27 10:37:12
@@ -64,7 +64,8 @@
                                int x, int y, bool fortified, bool veteran,
                                bool use_alternative_hp, int alternative_hp);
 int kill_desire(int benefit, int attack, int loss, int vuln, int attack_count);
-int military_amortize(int value, int delay, int build_cost);
+int military_amortize(struct player *pplayer, struct city *pcity, 
+                      int value, int delay, int build_cost);
 
 bool is_on_unit_upgrade_path(Unit_Type_id test, Unit_Type_id base);