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

["Gregory Berkolaiko via RT" <rt@xxxxxxxxxxxxxx>]

Quoting "Per I. Mathisen via RT" <rt@xxxxxxxxxxxxxx>:

> 
> On Fri, 27 Dec 2002, Gregory Berkolaiko via RT wrote:
> > > I know that. Where is the error?
> ...
> > But the maths mistake is there, unless I am completely mad.
> >  time = cost / production
> 
> Ah, yes. You'll fix before committing, I suppose.

I have fixed it.  Also, I moved military_amortize to aitools (it doesn't have
any strong connections with units and aiunit is overinflated as it is) and
changed the  signature of create_unit_virtual (pass it a valid city always, it
is used to determine coordinates of the unit).

I run an autogame, it was ok, AI was using units as usual.

Unless there are objections, the patch will be committed soon.

G.



? saves
Index: ai/advmilitary.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/advmilitary.c,v
retrieving revision 1.127
diff -u -r1.127 advmilitary.c
--- ai/advmilitary.c    2002/12/31 18:15:49     1.127
+++ ai/advmilitary.c    2003/01/01 12:15:22
@@ -942,7 +942,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) {
@@ -1188,7 +1188,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, "ERROR: Non-virtual unit in kill_something_with");
@@ -1234,7 +1235,7 @@
   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, unit_type,
                                     do_make_unit_veteran(pcity, unit_type));
 
   if (choice->want < 100) {
@@ -1415,7 +1416,7 @@
      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, unit_type,
                               player_knows_improvement_tech(pplayer, B_PORT));
     kill_something_with(pplayer, pcity, virtualunit, choice);
     destroy_unit_virtual(virtualunit);
@@ -1424,8 +1425,7 @@
   /* 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, 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  2003/01/01 12:15:22
@@ -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, 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.6
diff -u -r1.6 aidata.c
--- ai/aidata.c 2002/12/28 20:36:18     1.6
+++ ai/aidata.c 2003/01/01 12:15:22
@@ -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 2003/01/01 12:15:22
@@ -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.4
diff -u -r1.4 aidiplomat.c
--- ai/aidiplomat.c     2002/12/29 18:39:09     1.4
+++ ai/aidiplomat.c     2003/01/01 12:15:22
@@ -202,7 +202,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        2003/01/01 12:15:23
@@ -42,29 +42,55 @@
 #include "unittools.h"
 
 #include "aicity.h"
+#include "aidata.h"
 #include "ailog.h"
 #include "aiunit.h"
 
 #include "aitools.h"
 
 /**************************************************************************
-  Create a virtual unit to use in build want estimation
+  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.  The
+  build time is claculated as the build cost divided by the production
+  output of the unit's homecity or the city where we want to produce
+  the unit. If the city has less than average shield output, we
+  instead use the average, to encourage long-term thinking.
 **************************************************************************/
-struct unit *create_unit_virtual(struct player *pplayer, int x, int y,
-                                Unit_Type_id type, bool make_veteran)
+int military_amortize(struct player *pplayer, struct city *pcity,
+                      int value, int delay, int build_cost)
 {
+  struct ai_data *ai = ai_data_get(pplayer);
+  int city_output = (pcity ? pcity->shield_surplus : 1);
+  int output = MAX(city_output, ai->stats.average_production);
+  int build_time = build_cost / output;
+
+  if (value <= 0) {
+    return 0;
+  }
+
+  return amortize(value, delay + build_time);
+}
+
+/**************************************************************************
+  Create a virtual unit to use in build want estimation. pcity can be 
+  NULL.
+**************************************************************************/
+struct unit *create_unit_virtual(struct player *pplayer, struct city *pcity,
+                                 Unit_Type_id type, bool make_veteran)
+{
   struct unit *punit;
   punit=fc_calloc(1, sizeof(struct unit));
 
+  assert(pcity);
   punit->type = type;
   punit->owner = pplayer->player_no;
-  CHECK_MAP_POS(x, y);
-  punit->x = x;
-  punit->y = y;
+  CHECK_MAP_POS(pcity->x, pcity->y);
+  punit->x = pcity->x;
+  punit->y = pcity->y;
   punit->goto_dest_x = 0;
   punit->goto_dest_y = 0;
   punit->veteran = make_veteran;
-  punit->homecity = 0;
+  punit->homecity = pcity->id;
   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        2003/01/01 12:15:23
@@ -27,8 +27,10 @@
   BODYGUARD_NONE
 };
 
-struct unit *create_unit_virtual(struct player *pplayer, int x, int y,
-                                Unit_Type_id type, bool make_veteran);
+int military_amortize(struct player *pplayer, struct city *pcity, 
+                      int value, int delay, int build_cost);
+struct unit *create_unit_virtual(struct player *pplayer, struct city *pcity,
+                                 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.247
diff -u -r1.247 aiunit.c
--- ai/aiunit.c 2002/12/31 18:15:49     1.247
+++ ai/aiunit.c 2003/01/01 12:15:24
@@ -775,26 +775,6 @@
 }
 
 /**************************************************************************
-Military "want" estimates are amortized in this complicated way.
-COMMENTME: Why not use simple amortize? -- GB
-**************************************************************************/
-int military_amortize(int value, int delay, int build_cost)
-{
-  int simply_amortized, fully_amortized;
-
-  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;
-}
-
-/**************************************************************************
   Calculates the value and cost of nearby allied units to see if we can
   expect any help in our attack. Base function.
 **************************************************************************/
@@ -1923,12 +1903,15 @@
       /* build_cost of ferry */
       needferry = (go_by_boat && !ferryboat ? unit_value(boattype) : 0);
       /* FIXME: add time to build the ferry? */
-      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;
           *y = acity->y;
@@ -2034,7 +2017,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.39
diff -u -r1.39 aiunit.h
--- ai/aiunit.h 2002/12/31 18:15:49     1.39
+++ ai/aiunit.h 2003/01/01 12:15:25
@@ -69,7 +69,6 @@
                                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);
 
 bool is_on_unit_upgrade_path(Unit_Type_id test, Unit_Type_id base);