[Freeciv-Dev] (PR#10368) RfP: #define ACTIVITY_FACTOR

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=10368 >

Here is a patch that both cleans up and fixes things.

- ACTIVITY_FACTOR 10 is #defined and used.

- get_settler_speed is renamed as get_activity_rate.  This same function
is used for military units when they fortify or pillage.

- A new function get_activity_rate_this_turn is added.  This does
exactly what it sounds like.

- A change: for units with no MP left, the rate this turn is 0.  This is
(sort of) the way it was in 1.14.1, before it was changed (1.14.1 was, I
think, buggy here).  If any MP are left at all, then the unit has it's
full move rate.  Otherwise there is a benefit for micromanagement since
it is better to begin an activity even if you have no MP left.

- A change: the base move rate rather than the modified move rate are
used to calculate the activity rate.  This is a reversion to the 1.14.1
behavior, which was correct here.  Otherwise lower HP meant lower
activity rates, and effects increasing move rate meant higher activity
rate (e.g. magellan's expidition would increase the activity rate of sea
formers).

- A fix: in text.c the above functions are used.  This means the worker
time text is fixed.  See PR#10564.

- Some comments are added.

I've tested this pretty thoroughly.  The turns-to-complete as shown on
the mapview middle-click popup and connect-orders should now be correct.

This should be applied to both branches.  It fixes at least 3 bugs.

jason

? newtiles
Index: client/goto.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/goto.c,v
retrieving revision 1.76
diff -u -r1.76 goto.c
--- client/goto.c       15 Oct 2004 20:23:31 -0000      1.76
+++ client/goto.c       17 Oct 2004 04:08:10 -0000
@@ -458,7 +458,8 @@
   (2) (the tie-breaker) time to build the path (travel plus activity time).
   In rail-connect the priorities are reversed.
 
-  param->data should contain the result of get_settler_speed(punit) / 10.
+  param->data should contain the result of
+  get_activity_rate(punit) / ACTIVITY_FACTOR.
 ****************************************************************************/
 static int get_connect_road(const struct tile *src_tile, enum direction8 dir,
                            const struct tile *dest_tile,
@@ -558,7 +559,7 @@
   PF jumbo callback for the cost of a connect by irrigation. 
   Here we are only interested in how long it will take to irrigate the path.
 
-  param->data should contain the result of get_settler_speed(punit) / 10.
+  param->data should contain the result of get_activity_rate(punit) / 10.
 ****************************************************************************/
 static int get_connect_irrig(const struct tile *src_tile,
                             enum direction8 dir,
@@ -648,8 +649,7 @@
     }
     parameter->is_pos_dangerous = NULL;
 
-    /* 10 is a factor used in activity times for some reason */
-    speed = get_settler_speed(punit) / 10;
+    speed = get_activity_rate(punit) / ACTIVITY_FACTOR;
     parameter->data = &speed;
 
     /* Take into account the activity time at the origin */
Index: client/text.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/text.c,v
retrieving revision 1.12
diff -u -r1.12 text.c
--- client/text.c       13 Oct 2004 18:15:21 -0000      1.12
+++ client/text.c       17 Oct 2004 04:08:11 -0000
@@ -289,21 +289,16 @@
   int activity_total[ACTIVITY_LAST];
   int activity_units[ACTIVITY_LAST];
   int num_activities = 0;
-  int remains, turns, i, mr, au;
+  int remains, turns, i;
   INIT;
 
   memset(activity_total, 0, sizeof(activity_total));
   memset(activity_units, 0, sizeof(activity_units));
 
   unit_list_iterate(ptile->units, punit) {
-    mr = get_unit_type(punit->type)->move_rate;
-    au = (mr > 0) ? mr / SINGLE_MOVE : 1;
     activity_total[punit->activity] += punit->activity_count;
-    if (punit->moves_left > 0) {
-      /* current turn */
-      activity_total[punit->activity] += au;
-    }
-    activity_units[punit->activity] += au;
+    activity_total[punit->activity] += get_activity_rate_this_turn(punit);
+    activity_units[punit->activity] += get_activity_rate(punit);
   } unit_list_iterate_end;
 
   for (i = 0; i < ACTIVITY_LAST; i++) {
Index: common/map.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
retrieving revision 1.201
diff -u -r1.201 map.c
--- common/map.c        15 Oct 2004 09:39:05 -0000      1.201
+++ common/map.c        17 Oct 2004 04:08:11 -0000
@@ -865,7 +865,7 @@
 ***************************************************************/
 int map_build_road_time(const struct tile *ptile)
 {
-  return tile_types[ptile->terrain].road_time * 10;
+  return tile_types[ptile->terrain].road_time * ACTIVITY_FACTOR;
 }
 
 /***************************************************************
@@ -873,7 +873,7 @@
 ***************************************************************/
 int map_build_irrigation_time(const struct tile *ptile)
 {
-  return tile_types[ptile->terrain].irrigation_time * 10;
+  return tile_types[ptile->terrain].irrigation_time * ACTIVITY_FACTOR;
 }
 
 /***************************************************************
@@ -881,7 +881,7 @@
 ***************************************************************/
 int map_build_mine_time(const struct tile *ptile)
 {
-  return tile_types[ptile->terrain].mining_time * 10;
+  return tile_types[ptile->terrain].mining_time * ACTIVITY_FACTOR;
 }
 
 /***************************************************************
@@ -889,7 +889,7 @@
 ***************************************************************/
 int map_transform_time(const struct tile *ptile)
 {
-  return tile_types[ptile->terrain].transform_time * 10;
+  return tile_types[ptile->terrain].transform_time * ACTIVITY_FACTOR;
 }
 
 /***************************************************************
@@ -897,7 +897,7 @@
 ***************************************************************/
 int map_build_rail_time(const struct tile *ptile)
 {
-  return tile_types[ptile->terrain].rail_time * 10;
+  return tile_types[ptile->terrain].rail_time * ACTIVITY_FACTOR;
 }
 
 /***************************************************************
@@ -905,7 +905,7 @@
 ***************************************************************/
 int map_build_airbase_time(const struct tile *ptile)
 {
-  return tile_types[ptile->terrain].airbase_time * 10;
+  return tile_types[ptile->terrain].airbase_time * ACTIVITY_FACTOR;
 }
 
 /***************************************************************
@@ -913,7 +913,7 @@
 ***************************************************************/
 int map_build_fortress_time(const struct tile *ptile)
 {
-  return tile_types[ptile->terrain].fortress_time * 10;
+  return tile_types[ptile->terrain].fortress_time * ACTIVITY_FACTOR;
 }
 
 /***************************************************************
@@ -921,7 +921,7 @@
 ***************************************************************/
 int map_clean_pollution_time(const struct tile *ptile)
 {
-  return tile_types[ptile->terrain].clean_pollution_time * 10;
+  return tile_types[ptile->terrain].clean_pollution_time * ACTIVITY_FACTOR;
 }
 
 /***************************************************************
@@ -929,7 +929,7 @@
 ***************************************************************/
 int map_clean_fallout_time(const struct tile *ptile)
 {
-  return tile_types[ptile->terrain].clean_fallout_time * 10;
+  return tile_types[ptile->terrain].clean_fallout_time * ACTIVITY_FACTOR;
 }
 
 /***************************************************************
@@ -1601,7 +1601,7 @@
 {
   struct tile *ptile;
   int tries = 0;
-  const int max_tries = map.xsize * map.ysize / 10;
+  const int max_tries = map.xsize * map.ysize / ACTIVITY_FACTOR;
 
   /* First do a few quick checks to find a spot.  The limit on number of
    * tries could use some tweaking. */
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.222
diff -u -r1.222 map.h
--- common/map.h        15 Oct 2004 09:39:05 -0000      1.222
+++ common/map.h        17 Oct 2004 04:08:12 -0000
@@ -708,4 +708,9 @@
          || ptile->nat_y >= map.ysize - ydist);
 }
 
+/* An arbitrary somewhat integer value.  Activity times are multiplied by
+ * this amount, and divided by them later before being used.  This may
+ * help to avoid rounding errors; however it should probably be removed. */
+#define ACTIVITY_FACTOR 10
+
 #endif  /* FC__MAP_H */
Index: common/unit.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/unit.c,v
retrieving revision 1.219
diff -u -r1.219 unit.c
--- common/unit.c       29 Sep 2004 02:24:23 -0000      1.219
+++ common/unit.c       17 Oct 2004 04:08:12 -0000
@@ -509,19 +509,42 @@
 }
 
 /**************************************************************************
-  Returns the speed of a settler.  This depends on the veteran level and
-  the adjusted move_rate of the settler (which depends on HP).
+  Returns the speed of a unit doing an activity.  This depends on the
+  veteran level and the base move_rate of the unit (regardless of HP or
+  effects).  Usually this is just used for settlers but the value is also
+  used for military units doing fortify/pillage activities.
+
+  The speed is multiplied by ACTIVITY_COUNT.
 **************************************************************************/
-int get_settler_speed(struct unit *punit)
+int get_activity_rate(struct unit *punit)
 {
   int fact = get_unit_type(punit->type)->veteran[punit->veteran].power_fact;
 
-  /* The speed of the settler depends on its adjusted move_rate, not on
-   * the number of moves actually remaining. */
-  int move_rate = unit_move_rate(punit); /* Never less than SINGLE_MOVE */
+  /* The speed of the settler depends on its base move_rate, not on
+   * the number of moves actually remaining or the adjusted move rate.
+   * This means sea formers won't have their activity rate increased by
+   * Magellan's, and it means injured units work just as fast as
+   * uninjured ones.  Note the value is never less than SINGLE_MOVE. */
+  int move_rate = unit_type(punit)->move_rate;
+
+  /* All settler actions are multiplied by ACTIVITY_COUNT. */
+  return ACTIVITY_FACTOR * fact * move_rate / SINGLE_MOVE;
+}
+
+/**************************************************************************
+  Returns the amount of work a unit does (will do) on an activity this
+  turn.  Units that have no MP do no work.
 
-  /* All settler actions are multiplied by 10. */
-  return 10 * fact * move_rate / SINGLE_MOVE;
+  The speed is multiplied by ACTIVITY_COUNT.
+**************************************************************************/
+int get_activity_rate_this_turn(struct unit *punit)
+{
+  /* This logic is also coded in client/goto.c. */
+  if (punit->moves_left > 0) {
+    return get_activity_rate(punit);
+  } else {
+    return 0;
+  }
 }
 
 /**************************************************************************
@@ -533,9 +556,9 @@
                              enum unit_activity activity,
                              const struct tile *ptile)
 {
-  /* This is just an approximation since the speed depends on the unit's
-   * HP, which may change. */
-  int speed = get_settler_speed(punit);
+  /* FIXME: This is just an approximation since we don't account for
+   * get_activity_rate_this_turn. */
+  int speed = get_activity_rate(punit);
   int time = map_activity_time(activity, ptile);
 
   if (time >= 0 && speed >= 0) {
Index: common/unit.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/unit.h,v
retrieving revision 1.127
diff -u -r1.127 unit.h
--- common/unit.h       14 Oct 2004 21:01:04 -0000      1.127
+++ common/unit.h       17 Oct 2004 04:08:12 -0000
@@ -138,7 +138,12 @@
   struct unit_ai ai;
   enum unit_activity activity;
   struct tile *goto_tile; /* May be NULL. */
+
+  /* The amount of work that has been done on the current activity.  This
+   * is counted in turns but is multiplied by ACTIVITY_COUNT (which allows
+   * fractional values in some cases). */
   int activity_count;
+
   enum tile_special_type activity_target;
   enum unit_focus_status focus_status;
   int ord_map, ord_city;
@@ -244,7 +249,8 @@
 void set_unit_activity_targeted(struct unit *punit,
                                enum unit_activity new_activity,
                                enum tile_special_type new_target);
-int get_settler_speed(struct unit *punit);
+int get_activity_rate(struct unit *punit);
+int get_activity_rate_this_turn(struct unit *punit);
 int get_turns_for_activity_at(struct unit *punit,
                              enum unit_activity activity,
                              const struct tile *ptile);
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.307
diff -u -r1.307 unittools.c
--- server/unittools.c  14 Oct 2004 21:01:05 -0000      1.307
+++ server/unittools.c  17 Oct 2004 04:08:15 -0000
@@ -578,7 +578,7 @@
   if (activity != ACTIVITY_IDLE && activity != ACTIVITY_FORTIFIED
       && activity != ACTIVITY_GOTO && activity != ACTIVITY_EXPLORE) {
     /*  We don't need the activity_count for the above */
-    punit->activity_count += get_settler_speed(punit);
+    punit->activity_count += get_activity_rate_this_turn(punit);
 
     /* settler may become veteran when doing something useful */
     if (activity != ACTIVITY_FORTIFYING && activity != ACTIVITY_SENTRY