[Freeciv-Dev] (PR#10702) RFC: removing ptile->move_cost

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=10702 >

Here's an updated patch to remove ptile->move_costs.  This one is
intended to be committed.

Again, the reason for removing the move_costs is that it's a fairly
bug-prone cache that doesn't actually speed things up very much. 
Various comments throughout the code (which are removed by the patch)
should make this clear.

-jason

Index: ai/aiferry.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aiferry.c,v
retrieving revision 1.18
diff -u -r1.18 aiferry.c
--- ai/aiferry.c        5 Apr 2005 20:36:09 -0000       1.18
+++ ai/aiferry.c        12 Apr 2005 07:03:41 -0000
@@ -248,7 +248,7 @@
     move_cost = PF_IMPOSSIBLE_MC;
   } else {
     /* Land-to-Land */
-    move_cost = src_tile->move_cost[dir];
+    move_cost = tile_move_cost_ai(src_tile, tgt_tile);
   }
 
   return move_cost;
Index: client/goto.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/goto.c,v
retrieving revision 1.82
diff -u -r1.82 goto.c
--- client/goto.c       19 Feb 2005 17:15:13 -0000      1.82
+++ client/goto.c       12 Apr 2005 07:03:41 -0000
@@ -102,7 +102,6 @@
       DRAWN(ptile, dir) = 0;
     }
   } whole_map_iterate_end;
-  initialize_move_costs();
 
   is_init = TRUE;
 }
Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.487
diff -u -r1.487 packhand.c
--- client/packhand.c   10 Apr 2005 23:55:24 -0000      1.487
+++ client/packhand.c   12 Apr 2005 07:03:42 -0000
@@ -219,7 +219,6 @@
 
   ptile = pcity->tile;
   client_remove_city(pcity);
-  reset_move_costs(ptile);
 
   /* update menus if the focus unit is on the tile. */
   if (get_unit_in_focus()) {
@@ -625,8 +624,6 @@
            get_nation_name(city_owner(pcity)->nation),
            pcity->name, pcity->id, TILE_XY(pcity->tile));
   }
-
-  reset_move_costs(pcity->tile);
 }
 
 /**************************************************************************
@@ -1945,8 +1942,6 @@
     }
   }
 
-  reset_move_costs(ptile);
-
   if (ptile->known <= TILE_KNOWN_FOGGED && old_known == TILE_KNOWN) {
     /* This is an error.  So first we log the error, then make an assertion.
      * But for NDEBUG clients we fix the error. */
Index: client/gui-gtk-2.0/menu.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/menu.c,v
retrieving revision 1.70
diff -u -r1.70 menu.c
--- client/gui-gtk-2.0/menu.c   8 Apr 2005 06:32:27 -0000       1.70
+++ client/gui-gtk-2.0/menu.c   12 Apr 2005 07:03:42 -0000
@@ -1196,7 +1196,6 @@
   /* Restore the original state of the tile. */
   ptile->terrain = old_terrain;
   ptile->special = old_special;
-  reset_move_costs(ptile);
 
   return text;
 }
Index: common/map.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
retrieving revision 1.213
diff -u -r1.213 map.c
--- common/map.c        16 Mar 2005 19:37:48 -0000      1.213
+++ common/map.c        12 Apr 2005 07:03:42 -0000
@@ -967,7 +967,6 @@
        * rivers in oceans, don't clear this! */
       map_clear_special(ptile, S_RIVER);
     }
-    reset_move_costs(ptile);
   }
   map_clear_special(ptile, S_MINE);
 }
@@ -994,7 +993,6 @@
        * rivers in oceans, don't clear this! */
       map_clear_special(ptile, S_RIVER);
     }
-    reset_move_costs(ptile);
   }
   map_clear_special(ptile, S_FARMLAND);
   map_clear_special(ptile, S_IRRIGATION);
@@ -1013,8 +1011,6 @@
                                           rivers in oceans, don't clear this! 
*/
   }
 
-  reset_move_costs(ptile);
-
   /* Clear mining/irrigation if resulting terrain type cannot support
      that feature.  (With current rules, this should only clear mines,
      but I'm including both cases in the most general form for possible
@@ -1138,9 +1134,10 @@
     - both tiles are ocean or
     - one of the tiles is ocean and the other is a city or is unknown
 ***************************************************************/
-static int tile_move_cost_ai(struct tile *tile0, struct tile *tile1,
-                            int maxcost)
+int tile_move_cost_ai(const struct tile *tile0, const struct tile *tile1)
 {
+  const int maxcost = 72; /* Arbitrary. */
+
   assert(!is_server
         || (tile0->terrain != T_UNKNOWN && tile1->terrain != T_UNKNOWN));
 
@@ -1166,65 +1163,6 @@
 }
 
 /***************************************************************
- ...
-***************************************************************/
-static void debug_log_move_costs(const char *str, struct tile *tile0)
-{
-  /* the %x don't work so well for oceans, where
-     move_cost[]==-3 ,.. --dwp
-  */
-  freelog(LOG_DEBUG, "%s (%d, %d) [%x%x%x%x%x%x%x%x]", str,
-         tile0->x, tile0->y,
-         tile0->move_cost[0], tile0->move_cost[1],
-         tile0->move_cost[2], tile0->move_cost[3],
-         tile0->move_cost[4], tile0->move_cost[5],
-         tile0->move_cost[6], tile0->move_cost[7]);
-}
-
-/***************************************************************
-  Recalculate tile->move_cost[] for (x,y), and for adjacent
-  tiles in direction back to (x,y).  That is, call this when
-  something has changed on (x,y), eg roads, city, transform, etc.
-***************************************************************/
-void reset_move_costs(struct tile *ptile)
-{
-  int maxcost = 72; /* should be big enough without being TOO big */
-
-  debug_log_move_costs("Resetting move costs for", ptile);
-
-  /* trying to move off the screen is the default */
-  memset(ptile->move_cost, maxcost, sizeof(ptile->move_cost));
-
-  adjc_dir_iterate(ptile, tile1, dir) {
-    ptile->move_cost[dir] = tile_move_cost_ai(ptile, tile1, maxcost);
-    /* reverse: not at all obfuscated now --dwp */
-    tile1->move_cost[DIR_REVERSE(dir)] =
-       tile_move_cost_ai(tile1, ptile, maxcost);
-  } adjc_dir_iterate_end;
-  debug_log_move_costs("Reset move costs for", ptile);
-}
-
-/***************************************************************
-  Initialize tile->move_cost[] for all tiles, where move_cost[i]
-  is the unit-independent cost to move _from_ that tile, to
-  adjacent tile in direction specified by i.
-***************************************************************/
-void initialize_move_costs(void)
-{
-  int maxcost = 72; /* should be big enough without being TOO big */
-
-  whole_map_iterate(ptile) {
-    /* trying to move off the screen is the default */
-    memset(ptile->move_cost, maxcost, sizeof(ptile->move_cost));
-
-    adjc_dir_iterate(ptile, tile1, dir) {
-      ptile->move_cost[dir] = tile_move_cost_ai(ptile, tile1, maxcost);
-    }
-    adjc_dir_iterate_end;
-  } whole_map_iterate_end;
-}
-
-/***************************************************************
   The cost to move punit from where it is to tile x,y.
   It is assumed the move is a valid one, e.g. the tiles are adjacent.
 ***************************************************************/
@@ -1324,10 +1262,6 @@
 void map_set_special(struct tile *ptile, enum tile_special_type spe)
 {
   ptile->special |= spe;
-
-  if (contains_special(spe, S_ROAD) || contains_special(spe, S_RAILROAD)) {
-    reset_move_costs(ptile);
-  }
 }
 
 /***************************************************************
@@ -1336,10 +1270,6 @@
 void map_clear_special(struct tile *ptile, enum tile_special_type spe)
 {
   ptile->special &= ~spe;
-
-  if (contains_special(spe, S_ROAD) || contains_special(spe, S_RAILROAD)) {
-    reset_move_costs(ptile);
-  }
 }
 
 /***************************************************************
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.235
diff -u -r1.235 map.h
--- common/map.h        13 Mar 2005 17:50:54 -0000      1.235
+++ common/map.h        12 Apr 2005 07:03:43 -0000
@@ -50,7 +50,6 @@
   int assigned; /* these can save a lot of CPU usage -- Syela */
   struct city *worked;      /* city working tile, or NULL if none */
   Continent_id continent;
-  signed char move_cost[8]; /* don't know if this helps! */
   struct player *owner;     /* Player owning this tile, or NULL. */
   char *spec_sprite;
 };
@@ -168,9 +167,6 @@
 void map_set_continent(struct tile *ptile, Continent_id val);
 Continent_id map_get_continent(const struct tile *ptile);
 
-void initialize_move_costs(void);
-void reset_move_costs(struct tile *ptile);
-
 /* Number of index coordinates (for sanity checks and allocations) */
 #define MAP_INDEX_SIZE (map.xsize * map.ysize)
 
@@ -334,6 +330,7 @@
 bool is_move_cardinal(const struct tile *src_tile,
                      const struct tile *dst_tile);
 int map_move_cost(struct unit *punit, const struct tile *ptile);
+int tile_move_cost_ai(const struct tile *tile0, const struct tile *tile1);
 enum tile_special_type get_special_by_name(const char * name);
 const char *get_special_name(enum tile_special_type type);
 bool is_safe_ocean(const struct tile *ptile);
Index: common/aicore/pf_tools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/aicore/pf_tools.c,v
retrieving revision 1.29
diff -u -r1.29 pf_tools.c
--- common/aicore/pf_tools.c    31 Mar 2005 18:32:10 -0000      1.29
+++ common/aicore/pf_tools.c    12 Apr 2005 07:03:43 -0000
@@ -65,7 +65,7 @@
                              struct pf_parameter *param)
 {
   /* MOVE_COST_FOR_VALID_SEA_STEP means ships can move between */
-  if (ptile->move_cost[dir] == MOVE_COST_FOR_VALID_SEA_STEP
+  if (tile_move_cost_ai(ptile, ptile1) == MOVE_COST_FOR_VALID_SEA_STEP
       && !is_non_allied_city_tile(ptile1, param->owner)) {
     return SINGLE_MOVE;
   } else {
@@ -140,7 +140,7 @@
       move_cost = get_tile_type(terrain1)->movement_cost * SINGLE_MOVE;
     }
   } else {
-    move_cost = ptile->move_cost[dir];
+    move_cost = tile_move_cost_ai(ptile, ptile1);
   }
 
   return move_cost;
@@ -189,7 +189,7 @@
       move_cost = SINGLE_MOVE;
     } else {
       /* Normal move */
-      move_cost = src_tile->move_cost[dir];
+      move_cost = tile_move_cost_ai(src_tile, tgt_tile);
     }
   }
 
@@ -225,7 +225,7 @@
   } else if (is_ocean(ptile->terrain)) {
     move_cost = get_tile_type(terrain1)->movement_cost * SINGLE_MOVE;
   } else {
-    move_cost = ptile->move_cost[dir];
+    move_cost = tile_move_cost_ai(ptile, ptile1);
   }
 
   return move_cost;
@@ -287,7 +287,8 @@
       move_cost = MOVE_COST_ROAD;
     }
   } else {
-    move_cost = (ptile->move_cost[dir] != 0 ? MOVE_COST_ROAD : 0);
+    move_cost = (tile_move_cost_ai(ptile, ptile1) != 0
+                ? MOVE_COST_ROAD : 0);
   }
   return move_cost;
 }
Index: server/citytools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
retrieving revision 1.309
diff -u -r1.309 citytools.c
--- server/citytools.c  1 Apr 2005 00:42:27 -0000       1.309
+++ server/citytools.c  12 Apr 2005 07:03:43 -0000
@@ -800,7 +800,6 @@
   /* The units themselves are allready freed by transfer_city_units. */
   unit_list_unlink_all(old_city_units);
   unit_list_free(old_city_units);
-  reset_move_costs(pcity->tile);
 
   if (resolve_stack) {
     resolve_unit_stacks(pgiver, ptaker, transfer_unit_verbose);
@@ -994,13 +993,6 @@
   map_clear_special(ptile, S_FORTRESS);
   update_tile_knowledge(ptile);
 
-  reset_move_costs(ptile);
-/* I stupidly thought that setting S_ROAD took care of this, but of course
-the city_id isn't set when S_ROAD is set, so reset_move_costs doesn't allow
-sea movement at the point it's called.  This led to a problem with the
-warmap (but not the GOTOmap warmap) which meant the AI was very reluctant
-to use ferryboats.  I really should have identified this sooner. -- Syela */
-
   pcity->synced = FALSE;
   send_city_info(NULL, pcity);
   sync_cities(); /* Will also send pcity. */
@@ -1134,8 +1126,6 @@
 
   map_fog_pseudo_city_area(pplayer, ptile);
 
-  reset_move_costs(ptile);
-
   /* Update available tiles in adjacent cities. */
   map_city_radius_iterate(ptile, tile1) {
     /* For every tile the city could have used. */
Index: server/gotohand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/gotohand.c,v
retrieving revision 1.189
diff -u -r1.189 gotohand.c
--- server/gotohand.c   14 Mar 2005 20:26:25 -0000      1.189
+++ server/gotohand.c   12 Apr 2005 07:03:44 -0000
@@ -315,15 +315,18 @@
          move_cost = igter ? MOVE_COST_ROAD : MIN(base_cost, 
unit_move_rate(punit));
         } else if (igter)
          /* NOT c = 1 (Syela) [why not? - Thue] */
-         move_cost = (ptile->move_cost[dir] != 0 ? SINGLE_MOVE : 0);
+         move_cost = (tile_move_cost_ai(ptile, tile1) != 0
+                      ? SINGLE_MOVE : 0);
         else if (punit)
-         move_cost = MIN(ptile->move_cost[dir], unit_move_rate(punit));
+         move_cost = MIN(tile_move_cost_ai(ptile, tile1),
+                         unit_move_rate(punit));
        /* else c = ptile->move_cost[k]; 
           This led to a bad bug where a unit in a swamp was considered too far 
away */
         else { /* we have a city */
-         int tmp = tile1->move_cost[DIR_REVERSE(dir)];
-          move_cost = (ptile->move_cost[dir] + tmp +
-                      (ptile->move_cost[dir] > tmp ? 1 : 0))/2;
+         int tmp = tile_move_cost_ai(tile1, ptile);
+         int tmp2 = tile_move_cost_ai(ptile, tile1);
+
+          move_cost = (tmp2 + tmp + (tmp2 > tmp ? 1 : 0)) / 2;
         }
 
         move_cost += cost;
@@ -342,7 +345,8 @@
             can move between we allow for shore bombardment/transport
             to inland positions/etc. */
           WARMAP_SEACOST(tile1) = move_cost;
-         if (ptile->move_cost[dir] == MOVE_COST_FOR_VALID_SEA_STEP) {
+         if (tile_move_cost_ai(ptile, tile1)
+             == MOVE_COST_FOR_VALID_SEA_STEP) {
            add_to_mapqueue(move_cost, tile1);
          }
        }
@@ -647,9 +651,11 @@
          int base_cost = get_tile_type(pdesttile->terrain)->movement_cost * 
SINGLE_MOVE;
          move_cost = igter ? 1 : MIN(base_cost, unit_move_rate(punit));
        } else if (igter)
-         move_cost = (psrctile->move_cost[dir] != 0 ? SINGLE_MOVE : 0);
+         move_cost = (tile_move_cost_ai(psrctile, pdesttile) != 0
+                      ? SINGLE_MOVE : 0);
        else
-         move_cost = MIN(psrctile->move_cost[dir], unit_move_rate(punit));
+         move_cost = MIN(tile_move_cost_ai(psrctile, pdesttile),
+                         unit_move_rate(punit));
 
        if (!pplayer->ai.control && !map_is_known(tile1, pplayer)) {
          /* Don't go into the unknown. 5*SINGLE_MOVE is an arbitrary 
deterrent. */
@@ -709,7 +715,8 @@
          continue; /* No need for all the calculations */
 
        /* allow ships to target a shore */
-       if (psrctile->move_cost[dir] != MOVE_COST_FOR_VALID_SEA_STEP
+       if (tile_move_cost_ai(psrctile, pdesttile)
+           != MOVE_COST_FOR_VALID_SEA_STEP
            && !same_pos(tile1, dest_tile)) {
          continue;
        } else if (unit_loss_pct(unit_owner(punit), tile1, punit) > 0) {
@@ -982,7 +989,7 @@
     if (is_ground_unit(punit)) {
       /* assuming move is valid, but what if unit is trying to board? 
         -- GB */
-      base_move_cost = punit->tile->move_cost[dir];
+      base_move_cost = tile_move_cost_ai(punit->tile, ptile);
     } else {
       base_move_cost = SINGLE_MOVE;
     }
Index: server/srv_main.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/srv_main.c,v
retrieving revision 1.242
diff -u -r1.242 srv_main.c
--- server/srv_main.c   5 Apr 2005 20:40:47 -0000       1.242
+++ server/srv_main.c   12 Apr 2005 07:03:44 -0000
@@ -1910,7 +1910,6 @@
     gamelog(GAMELOG_TEAM, pteam);
   } team_iterate_end;
 
-  initialize_move_costs(); /* this may be the wrong place to do this */
   init_settlers(); /* create minimap and other settlers.c data */
   ai_data_movemap_init();