[Freeciv-Dev] (PR#9561) bugs in settler activities

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

This patch fixes (I think) 3 bugs in settler activities.

1.  The biggest bug is that activity times are multiplied by 10.  But 
settlers.c doesn't know about this.  Thus the calculated times to do any 
activity are 10x too high (on top of the move time, which is correct). 
No doubt this has something to do with the reported problems with 
autosettlers.

2.  A second bug is in the considering of making a road plus a rail in 
settlers.c.  There is a can_rr variable that is used to see if the 
settler can make a railroad.  But it's not used to see if the settler 
can make a road plus a railroad.  The fix is easy.

3.  The third bug (at least, I think it's a bug) is that in unittools.c 
when adding on to the activity_count there is a special case for when 
the unit has nove move points left.  I think this is a bug because the 
amount added on has nothing to do with the MP.  So an engineer with 1/3 
MP left gets the same amount of work done as if it had 2 MP, but with 0 
MP left it only gets half this amount.

In the process I added a new function get_turns_for_activity (equivalent 
to the function needed by gui-ftwl).  I moved get_settler_speed into 
unit.c and changed it to do all the work of calculating the speed 
(instead of just half of the work).

jason

? data/themes
Index: common/unit.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/unit.c,v
retrieving revision 1.212
diff -u -r1.212 unit.c
--- common/unit.c       20 Jul 2004 11:05:37 -0000      1.212
+++ common/unit.c       30 Jul 2004 15:28:57 -0000
@@ -518,6 +518,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).
+**************************************************************************/
+int get_settler_speed(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 */
+
+  /* All settler actions are multiplied by 10. */
+  return 10 * fact * move_rate / SINGLE_MOVE;
+}
+
+/**************************************************************************
+  Return the estimated number of turns for the worker unit to start and
+  complete the activity at the given location.  This assumes no other
+  worker units are helping out.
+**************************************************************************/
+int get_turns_for_activity_at(struct unit *punit,
+                             enum unit_activity activity, int x, int y)
+{
+  /* This is just an approximation since the speed depends on the unit's
+   * HP, which may change. */
+  int speed = get_settler_speed(punit);
+  int time = map_activity_time(activity, x, y);
+
+  if (time >= 0 && speed >= 0) {
+    return (time - 1) / speed + 1; /* round up */
+  } else {
+    return FC_INFINITY;
+  }
+}
+
+/**************************************************************************
 Return whether the unit can be put in auto-mode.
 (Auto-settler for settlers, auto-attack for military units.)
 **************************************************************************/
Index: common/unit.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/unit.h,v
retrieving revision 1.119
diff -u -r1.119 unit.h
--- common/unit.h       18 Jul 2004 04:08:21 -0000      1.119
+++ common/unit.h       30 Jul 2004 15:28:57 -0000
@@ -269,6 +269,9 @@
 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_turns_for_activity_at(struct unit *punit,
+                             enum unit_activity activity, int x, int y);
 bool can_unit_do_auto(struct unit *punit); 
 bool is_unit_activity_on_tile(enum unit_activity activity, int x, int y);
 enum tile_special_type get_unit_tile_pillage_set(int x, int y);
Index: server/settlers.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/settlers.c,v
retrieving revision 1.184
diff -u -r1.184 settlers.c
--- server/settlers.c   20 Jul 2004 10:05:43 -0000      1.184
+++ server/settlers.c   30 Jul 2004 15:28:58 -0000
@@ -1102,50 +1102,52 @@
 
        /* now, consider various activities... */
 
-       time = (map_build_irrigation_time(x, y)*SINGLE_MOVE + mv_rate - 
1)/mv_rate +
-         mv_turns;
+       time = mv_turns
+         + get_turns_for_activity_at(punit, ACTIVITY_IRRIGATE, x, y);
        consider_settler_action(pplayer, ACTIVITY_IRRIGATE, -1,
                                pcity->ai.irrigate[i][j], oldv, in_use, time,
                                &best_newv, &best_oldv, best_act, gx, gy,
                                x, y);
 
        if (unit_flag(punit, F_TRANSFORM)) {
-         time = (map_transform_time(x, y)*SINGLE_MOVE + mv_rate - 1)/mv_rate +
-           mv_turns;
+         time = mv_turns
+           + get_turns_for_activity_at(punit, ACTIVITY_TRANSFORM, x, y);
          consider_settler_action(pplayer, ACTIVITY_TRANSFORM, -1,
                                  pcity->ai.transform[i][j], oldv, in_use, time,
                                  &best_newv, &best_oldv, best_act, gx, gy,
                                  x, y);
        }
 
-       time = (map_build_mine_time(x, y)*SINGLE_MOVE + mv_rate - 1)/mv_rate +
-         mv_turns;
+       time = mv_turns
+         + get_turns_for_activity_at(punit, ACTIVITY_MINE, x, y);
        consider_settler_action(pplayer, ACTIVITY_MINE, -1,
                                pcity->ai.mine[i][j], oldv, in_use, time,
                                &best_newv, &best_oldv, best_act, gx, gy,
                                x, y);
 
        if (!map_has_special(x, y, S_ROAD)) {
-         time = (map_build_road_time(x, y)*SINGLE_MOVE + mv_rate - 1)/mv_rate +
-           mv_turns;
+         time = mv_turns
+           + get_turns_for_activity_at(punit, ACTIVITY_ROAD, x, y);
          consider_settler_action(pplayer, ACTIVITY_ROAD,
                                  road_bonus(x, y, S_ROAD) * 5,
                                  pcity->ai.road[i][j], oldv, in_use, time,
                                  &best_newv, &best_oldv, best_act, gx, gy,
                                  x, y);
 
-         time = (map_build_rail_time(x, y) * SINGLE_MOVE
-                 + map_build_road_time(x, y) * SINGLE_MOVE
-                 + mv_rate - 1)/mv_rate + mv_turns;
-         consider_settler_action(pplayer, ACTIVITY_ROAD,
-                                 road_bonus(x, y, S_RAILROAD) * 3,
-                                 pcity->ai.railroad[i][j], oldv, in_use, time,
-                                 &best_newv, &best_oldv, best_act, gx, gy,
-                                 x, y);
+         if (can_rr) {
+           /* Count road time plus rail time. */
+           time += get_turns_for_activity_at(punit, ACTIVITY_RAILROAD, x, y);
+           consider_settler_action(pplayer, ACTIVITY_ROAD,
+                                   road_bonus(x, y, S_RAILROAD) * 3,
+                                   pcity->ai.railroad[i][j], oldv,
+                                   in_use, time,
+                                   &best_newv, &best_oldv, best_act, gx, gy,
+                                   x, y);
+         }
        } else if (!map_has_special(x, y, S_RAILROAD)
                   && can_rr) {
-         time = (map_build_rail_time(x, y) *SINGLE_MOVE
-                 + mv_rate - 1)/mv_rate + mv_turns;
+         time = mv_turns
+           + get_turns_for_activity_at(punit, ACTIVITY_RAILROAD, x, y);
          consider_settler_action(pplayer, ACTIVITY_RAILROAD,
                                  road_bonus(x, y, S_RAILROAD) * 3,
                                  pcity->ai.railroad[i][j], oldv, in_use, time,
@@ -1154,8 +1156,8 @@
        } /* end S_ROAD else */
 
        if (map_has_special(x, y, S_POLLUTION)) {
-         time = (map_clean_pollution_time(x, y) * SINGLE_MOVE
-                 + mv_rate - 1)/mv_rate + mv_turns;
+         time = mv_turns
+           + get_turns_for_activity_at(punit, ACTIVITY_POLLUTION, x, y);
          consider_settler_action(pplayer, ACTIVITY_POLLUTION,
                                  pplayer->ai.warmth,
                                  pcity->ai.detox[i][j], oldv, in_use, time,
@@ -1164,8 +1166,8 @@
        }
       
        if (map_has_special(x, y, S_FALLOUT)) {
-         time = (map_clean_fallout_time(x, y) * SINGLE_MOVE
-                 + mv_rate - 1)/mv_rate + mv_turns;
+         time = mv_turns
+           + get_turns_for_activity_at(punit, ACTIVITY_FALLOUT, x, y);
          consider_settler_action(pplayer, ACTIVITY_FALLOUT,
                                  pplayer->ai.warmth,
                                  pcity->ai.derad[i][j], oldv, in_use, time,
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.294
diff -u -r1.294 unittools.c
--- server/unittools.c  1 Jun 2004 19:44:24 -0000       1.294
+++ server/unittools.c  30 Jul 2004 15:28:58 -0000
@@ -544,13 +544,6 @@
   return total;
 }
 
-/**************************************************************************
-  Returns the speed of a settler.
-**************************************************************************/
-int get_settler_speed(struct unit *punit) {
-   return (10 * unit_type(punit)->veteran[punit->veteran].power_fact);
-}
-
 /***************************************************************************
   Maybe settler/worker gains a veteran level?
 ****************************************************************************/
@@ -579,7 +572,6 @@
 {
   struct player *pplayer = unit_owner(punit);
   int id = punit->id;
-  int mr = unit_move_rate(punit);
   bool unit_activity_done = FALSE;
   enum unit_activity activity = punit->activity;
   enum ocean_land_change solvency = OLC_NONE;
@@ -588,20 +580,7 @@
   if (activity != ACTIVITY_IDLE && activity != ACTIVITY_FORTIFIED
       && activity != ACTIVITY_GOTO && activity != ACTIVITY_EXPLORE) {
     /*  We don't need the activity_count for the above */
-    if (punit->moves_left > 0) {
-      /* 
-       * To allow a settler to begin a task with no moves left without
-       * it counting toward the time to finish 
-       */
-
-      /* update settler activity 
-       * note that all activity costs are multiplied
-       * by a factor of 10. */
-      punit->activity_count +=
-             (mr * get_settler_speed(punit)) / SINGLE_MOVE;
-    } else if (mr == 0) {
-      punit->activity_count += get_settler_speed(punit);
-    }
+    punit->activity_count += get_settler_speed(punit);
 
     /* settler may become veteran when doing something useful */
     if (activity != ACTIVITY_FORTIFYING && activity != ACTIVITY_SENTRY
Index: server/unittools.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.h,v
retrieving revision 1.68
diff -u -r1.68 unittools.h
--- server/unittools.h  14 Apr 2004 11:19:45 -0000      1.68
+++ server/unittools.h  30 Jul 2004 15:28:58 -0000
@@ -34,7 +34,6 @@
 /* turn update related */
 void player_restore_units(struct player *pplayer);
 void update_unit_activities(struct player *pplayer);
-int get_settler_speed(struct unit *punit);
 
 /* various */
 char *get_location_str_in(struct player *pplayer, int x, int y);