[Freeciv-Dev] Re: [PATCH] city_map_size fix and idea

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

This is cleaned up in corecleanup_06a which also fixes the iterate macro
to get rid of all the unchecked pcity accesses. However, I think I missed 
the savegame spot.

We need to get together, merge our changes and submit a joint patch, maybe.
Having two eyes pull out these chagnes is good, having overlapping patches
is probably not.

Cheers,
RossW
=====

At 01:14 AM 01/08/22 -0400, Jason Dorje Short wrote:
>Jason Dorje Short wrote:
>> 
>> The attached patch fixes all incorrect uses of "2" by replacing it with
>> CITY_MAP_SIZE/2.  Note that this is just a very simple correctness fix
>> and doesn't address any of the other outstanding issues with the current
>> system (namely: city_map_iterate really sucks, and pcity->x + x +
>> CITY_MAP_SIZE/2 could be replaced with a macro).
>
>Doh! again.  Here's the patch.
>
>I feel like Thue.
>
>jasonJason Short
>jdorje@xxxxxxxxxxxx
>
>This small patch changes most or all incorrect uses of "2" and replaces it
>with CITY_MAP_SIZE/2.
>
>Advantages: Slightly more correct code
>Disadvantages: none
>Chance of bugs: low or none
>
>Index: ai/advdomestic.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/ai/advdomestic.c,v
>retrieving revision 1.70
>diff -u -r1.70 advdomestic.c
>--- ai/advdomestic.c   2001/08/14 14:31:18     1.70
>+++ ai/advdomestic.c   2001/08/22 04:57:09
>@@ -91,7 +91,7 @@
> {
>   int i = 0;
>   city_map_iterate(x, y) {
>-    if (map_get_tile(pcity->x+x-2, pcity->y+y-2)->terrain == T_OCEAN) {
>+    if (map_get_tile(pcity->x+x-CITY_MAP_SIZE/2,
pcity->y+y-CITY_MAP_SIZE/2)->terrain == T_OCEAN) {
>       i++; /* this is a kluge; wasn't getting enough harbors because
> often everyone was stuck farming grassland. */
>       if (is_worker_here(pcity, x, y)) i++;
>@@ -105,7 +105,7 @@
>   int i = 0; 
>   city_map_iterate(x, y) {
>     if (is_worker_here(pcity, x, y)) {
>-      if (map_get_special(pcity->x+x-2, pcity->y+y-2) & S_ROAD) i++;
>+      if (map_get_special(pcity->x+x-CITY_MAP_SIZE/2,
pcity->y+y-CITY_MAP_SIZE/2) & S_ROAD) i++;
>     }
>   } city_map_iterate_end;
>   return(i); 
>@@ -116,7 +116,7 @@
>   int i = 0; 
>   city_map_iterate(x, y) {
>     if (is_worker_here(pcity, x, y)) {
>-      if (map_get_special(pcity->x+x-2, pcity->y+y-2) & S_FARMLAND) i++;
>+      if (map_get_special(pcity->x+x-CITY_MAP_SIZE/2,
pcity->y+y-CITY_MAP_SIZE/2) & S_FARMLAND) i++;
>     }
>   } city_map_iterate_end;
>   return(i); 
>Index: client/gui-gtk/citydlg.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/citydlg.c,v
>retrieving revision 1.89
>diff -u -r1.89 citydlg.c
>--- client/gui-gtk/citydlg.c   2001/08/04 21:06:12     1.89
>+++ client/gui-gtk/citydlg.c   2001/08/22 04:57:11
>@@ -1381,8 +1381,8 @@
>   } city_map_iterate_end;
>   /* We have to put the output afterwards or it will be covered. */
>   city_map_iterate(x, y) {
>-    int map_x = pcity->x + x - 2;
>-    int map_y = pcity->y + y - 2;
>+    int map_x = pcity->x + x - CITY_MAP_SIZE/2;
>+    int map_y = pcity->y + y - CITY_MAP_SIZE/2;
>     if (normalize_map_pos(&map_x, &map_y)
>       && tile_is_known(map_x, map_y)) {
>       int canvas_x, canvas_y;
>@@ -1402,8 +1402,8 @@
>      to fix this, but maybe it wouldn't be a good idea because the
>      lines would get obscured. */
>   city_map_iterate(x, y) {
>-    int map_x = pcity->x + x - 2;
>-    int map_y = pcity->y + y - 2;
>+    int map_x = pcity->x + x - CITY_MAP_SIZE/2;
>+    int map_y = pcity->y + y - CITY_MAP_SIZE/2;
>     if (normalize_map_pos(&map_x, &map_y)
>       && tile_is_known(map_x, map_y)) {
>       int canvas_x, canvas_y;
>Index: client/gui-mui/mapclass.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/client/gui-mui/mapclass.c,v
>retrieving revision 1.58
>diff -u -r1.58 mapclass.c
>--- client/gui-mui/mapclass.c  2001/08/20 07:55:59     1.58
>+++ client/gui-mui/mapclass.c  2001/08/22 04:57:14
>@@ -2361,8 +2361,8 @@
>          the city radius can be fogged. */
> 
>       city_map_iterate(x, y) {
>-              int map_x = pcity->x + x - 2;
>-              int map_y = pcity->y + y - 2;
>+              int map_x = pcity->x + x - CITY_MAP_SIZE/2;
>+              int map_y = pcity->y + y - CITY_MAP_SIZE/2;
>       if (normalize_map_pos(&map_x, &map_y) && tile_is_known(map_x, map_y)) {
>         int canvas_x, canvas_y;
>         city_get_canvas_xy(x, y, &canvas_x, &canvas_y);
>@@ -2372,8 +2372,8 @@
> 
>       /* We have to put the output afterwards or it will be covered. */
>       city_map_iterate(x, y) {
>-      int map_x = pcity->x + x - 2;
>-      int map_y = pcity->y + y - 2;
>+      int map_x = pcity->x + x - CITY_MAP_SIZE/2;
>+      int map_y = pcity->y + y - CITY_MAP_SIZE/2;
>       if (normalize_map_pos(&map_x, &map_y) && tile_is_known(map_x, map_y)) {
>         int canvas_x, canvas_y;
>         city_get_canvas_xy(x, y, &canvas_x, &canvas_y);
>@@ -2392,8 +2392,8 @@
>          to fix this, but maybe it wouldn't be a good idea because the
>          lines would get obscured. */
>       city_map_iterate(x, y) {
>-      int map_x = pcity->x + x - 2;
>-      int map_y = pcity->y + y - 2;
>+      int map_x = pcity->x + x - CITY_MAP_SIZE/2;
>+      int map_y = pcity->y + y - CITY_MAP_SIZE/2;
>       if (normalize_map_pos(&map_x, &map_y) && tile_is_known(map_x, map_y))
>       {
>         int canvas_x, canvas_y;
>Index: common/city.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/common/city.c,v
>retrieving revision 1.109
>diff -u -r1.109 city.c
>--- common/city.c      2001/07/05 19:09:46     1.109
>+++ common/city.c      2001/08/22 04:57:15
>@@ -517,8 +517,8 @@
> int city_get_shields_tile(int x, int y, struct city *pcity)
> {
>   int s=0;
>-  enum tile_special_type spec_t=map_get_special(pcity->x+x-2, pcity->y+y-2);
>-  enum tile_terrain_type tile_t=map_get_terrain(pcity->x+x-2, pcity->y+y-2);
>+  enum tile_special_type
spec_t=map_get_special(pcity->x+x-CITY_MAP_SIZE/2,
pcity->y+y-CITY_MAP_SIZE/2);
>+  enum tile_terrain_type
tile_t=map_get_terrain(pcity->x+x-CITY_MAP_SIZE/2,
pcity->y+y-CITY_MAP_SIZE/2);
>   struct government *g = get_gov_pcity(pcity);
>   int celeb = city_celebrating(pcity);
>   int before_penalty = (celeb ? g->celeb_shields_before_penalty
>@@ -595,8 +595,8 @@
> **************************************************************************/
> int city_get_trade_tile(int x, int y, struct city *pcity)
> {
>-  enum tile_special_type spec_t=map_get_special(pcity->x+x-2, pcity->y+y-2);
>-  enum tile_terrain_type tile_t=map_get_terrain(pcity->x+x-2, pcity->y+y-2);
>+  enum tile_special_type
spec_t=map_get_special(pcity->x+x-CITY_MAP_SIZE/2,
pcity->y+y-CITY_MAP_SIZE/2);
>+  enum tile_terrain_type
tile_t=map_get_terrain(pcity->x+x-CITY_MAP_SIZE/2,
pcity->y+y-CITY_MAP_SIZE/2);
>   struct government *g = get_gov_pcity(pcity);
>   int t;
>  
>@@ -687,8 +687,8 @@
> int city_get_food_tile(int x, int y, struct city *pcity)
> {
>   int f;
>-  enum tile_special_type spec_t=map_get_special(pcity->x+x-2, pcity->y+y-2);
>-  enum tile_terrain_type tile_t=map_get_terrain(pcity->x+x-2, pcity->y+y-2);
>+  enum tile_special_type
spec_t=map_get_special(pcity->x+x-CITY_MAP_SIZE/2,
pcity->y+y-CITY_MAP_SIZE/2);
>+  enum tile_terrain_type
tile_t=map_get_terrain(pcity->x+x-CITY_MAP_SIZE/2,
pcity->y+y-CITY_MAP_SIZE/2);
>   struct tile_type *type;
>   struct government *g = get_gov_pcity(pcity);
>   int celeb = city_celebrating(pcity);
>Index: server/savegame.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/server/savegame.c,v
>retrieving revision 1.21
>diff -u -r1.21 savegame.c
>--- server/savegame.c  2001/05/23 18:21:29     1.21
>+++ server/savegame.c  2001/08/22 04:57:18
>@@ -945,7 +945,7 @@
>         if (*p=='0') {
>         set_worker_city(pcity, x, y, C_TILE_EMPTY);
>       } else if (*p=='1') {
>-        if (map_get_tile(pcity->x+x-2, pcity->y+y-2)->worked) {
>+        if (map_get_tile(pcity->x+x-CITY_MAP_SIZE/2,
pcity->y+y-CITY_MAP_SIZE/2)->worked) {
>           /* oops, inconsistent savegame; minimal fix: */
>           freelog(LOG_VERBOSE, "Inconsistent worked for %s (%d,%d), "
>                   "converting to elvis", pcity->name, x, y);
>