[Freeciv-Dev] Re: Cleaning up server/savegame.c (PR#998)

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

Raimar Falke wrote:

> Objections?

Try this one instead.

It now replaces all map iterations during saving (loading is
independent; the patches can be applied simultaneously but there's no
reason to keep them together).

jason

? rc
Index: common/map.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
retrieving revision 1.94
diff -u -r1.94 map.c
--- common/map.c        2001/10/08 12:11:16     1.94
+++ common/map.c        2001/10/08 19:48:54
@@ -1314,6 +1314,19 @@
 }
 
 /**************************************************************************
+Returns TRUE iff the map position is normal.  "Normal" here means that it
+is both a real/valid coordinate set and that the coordinates are in their
+canonical/proper form.  In plain English: the coordinates must be on the
+map.
+**************************************************************************/
+int is_normal_map_pos(int x, int y)
+{
+  int x1 = x, y1 = y;
+
+  return normalize_map_pos(&x1, &y1) && x1 == x && y1 == y;
+}
+
+/**************************************************************************
 Normalizes the map position. Returns TRUE if it is real, FALSE otherwise.
 **************************************************************************/
 int normalize_map_pos(int *x, int *y)
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.95
diff -u -r1.95 map.h
--- common/map.h        2001/10/08 12:11:16     1.95
+++ common/map.h        2001/10/08 19:48:55
@@ -231,6 +231,7 @@
 enum known_type tile_is_known(int x, int y);
 int check_coords(int *x, int *y);
 int is_real_tile(int x, int y);
+int is_normal_map_pos(int x, int y);
 int normalize_map_pos(int *x, int *y);
 void nearest_real_pos(int *x, int *y);
 
Index: server/savegame.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/savegame.c,v
retrieving revision 1.33
diff -u -r1.33 savegame.c
--- server/savegame.c   2001/10/03 09:16:56     1.33
+++ server/savegame.c   2001/10/08 19:48:56
@@ -51,9 +51,44 @@
 #include "savegame.h"
 
 
+#define DEC2HEX(int_value, halfbyte_wanted) \
+  dec2hex[ ((int_value) >> ((halfbyte_wanted) * 4)) & 0xf ]
 static const char dec2hex[] = "0123456789abcdef";
 static const char terrain_chars[] = "adfghjm prstu";
 
+/* This loops over the entire map to create save data, and stores it in the
+   file under the given secfile structure.  It works like this:
+     x_itr, y_itr -> x and y iterator variables
+     file -> the section_file we're saving to
+     get_xy_char -> a line of code that returns the character for each
+       (x, y) coordinate.  It should use the x/y iterator variables
+       appropriately.
+     secfile_index -> the indexor to the secfile, passed to secfile_insert.
+       It takes y_itr as an argument, so it should have a %03d in it. */
+#define save_map_data(secfile, get_xy_char, secfile_insert_line, x_itr, y_itr) 
\
+{                                                                              
\
+  char* pbuf = fc_malloc(sizeof(char) * (map.xsize+1));                        
\
+  int x_itr, y_itr;                                                            
\
+  for (y_itr=0; y_itr<map.ysize; y_itr++) {                                    
\
+    for (x_itr=0; x_itr<map.xsize; x_itr++) {                                  
\
+      if (is_normal_map_pos(x, y)) {                                           
\
+       pbuf[x_itr] = get_xy_char;                                             \
+      } else {                                                                 
\
+       pbuf[x_itr] = '#'; /* skipped over in loading */                       \
+      }                                                                        
\
+    }                                                                          
\
+    pbuf[map.xsize] = '\0';                                                    
\
+    secfile_insert_line;                                                       
\
+  }                                                                            
\
+  free(pbuf); /* running malloc/free each time is inefficient */               
\
+}
+
+#define save_reg_map_data(secfile, get_xy_char, secfile_name, x_itr, y_itr)   \
+       save_map_data(secfile, get_xy_char, secfile_insert_str(secfile, pbuf, 
secfile_name, y_itr), x_itr, y_itr)
+#define save_plr_map_data(secfile, get_xy_char, secfile_name, x_itr, y_itr, 
plrno) \
+       save_map_data(secfile, get_xy_char, secfile_insert_str(secfile, pbuf, 
secfile_name, plrno, y_itr), x_itr, y_itr)
+
+
 /* Following does not include "unirandom", used previously; add it if
  * appropriate.  (Code no longer looks at "unirandom", but should still
  * include it when appropriate for maximum savegame compatibility.)
@@ -230,27 +265,6 @@
 }
 
 /***************************************************************
-  Save the rivers overlay map; this is a special case to allow
-  re-saving scenarios which have rivers overlay data.  This
-  only applies if don't have rest of specials.
-***************************************************************/
-static void map_rivers_overlay_save(struct section_file *file)
-{
-  char *pbuf = fc_malloc(map.xsize+1);
-  int x, y;
-  
-  /* put "next" 4 bits of special flags */
-  for(y=0; y<map.ysize; y++) {
-    for(x=0; x<map.xsize; x++) {
-      pbuf[x]=dec2hex[(map_get_tile(x, y)->special&0xf00)>>8];
-    }
-    pbuf[x]='\0';
-    secfile_insert_str(file, pbuf, "map.n%03d", y);
-  }
-  free(pbuf);
-}
-
-/***************************************************************
 load a complete map from a savegame file
 ***************************************************************/
 static void map_load(struct section_file *file)
@@ -494,8 +508,7 @@
 ***************************************************************/
 static void map_save(struct section_file *file)
 {
-  int i, x, y;
-  char *pbuf=fc_malloc(map.xsize+1);
+  int i;
 
   /* map.xsize and map.ysize (saved as map.width and map.height)
    * are now always saved in game_save()
@@ -512,126 +525,53 @@
   }
     
   /* put the terrain type */
-  for(y=0; y<map.ysize; y++) {
-    for(x=0; x<map.xsize; x++)
-      pbuf[x]=terrain_chars[map_get_tile(x, y)->terrain];
-    pbuf[x]='\0';
+  save_reg_map_data(file, terrain_chars[map_get_tile(x, y)->terrain],
+                       "map.t%03d", x, y);
 
-    secfile_insert_str(file, pbuf, "map.t%03d", y);
-  }
-
-  if (!map.have_specials) {
-    if (map.have_rivers_overlay) {
-      map_rivers_overlay_save(file);
-    }
-    free(pbuf);
+  if (!map.have_specials && map.have_rivers_overlay) {
+    /* Save the rivers overlay map; this is a special case to allow
+       re-saving scenarios which have rivers overlay data.  This
+       only applies if don't have rest of specials. */
+
+    /* bits 8-11 of special flags field */
+    save_reg_map_data(file,
+                       DEC2HEX(map_get_tile(x, y)->special, 2),
+                       "map.n%03d", x, y);
     return;
   }
-
-  /* put lower 4 bits of special flags */
-  for(y=0; y<map.ysize; y++) {
-    for(x=0; x<map.xsize; x++)
-      pbuf[x]=dec2hex[map_get_tile(x, y)->special&0xf];
-    pbuf[x]='\0';
-
-    secfile_insert_str(file, pbuf, "map.l%03d", y);
-  }
-
-  /* put upper 4 bits of special flags */
-  for(y=0; y<map.ysize; y++) {
-    for(x=0; x<map.xsize; x++)
-      pbuf[x]=dec2hex[(map_get_tile(x, y)->special&0xf0)>>4];
-    pbuf[x]='\0';
-
-    secfile_insert_str(file, pbuf, "map.u%03d", y);
-  }
-
-  /* put "next" 4 bits of special flags */
-  for(y=0; y<map.ysize; y++) {
-    for(x=0; x<map.xsize; x++)
-      pbuf[x]=dec2hex[(map_get_tile(x, y)->special&0xf00)>>8];
-    pbuf[x]='\0';
 
-    secfile_insert_str(file, pbuf, "map.n%03d", y);
-  }
+  /* put 4-bit segments of 12-bit "special flags" field */
+  save_reg_map_data(file, DEC2HEX(map_get_tile(x, y)->special, 0),
+                   "map.l%03d", x, y);
+  save_reg_map_data(file, DEC2HEX(map_get_tile(x, y)->special, 1),
+                   "map.u%03d", x, y);
+  save_reg_map_data(file, DEC2HEX(map_get_tile(x, y)->special, 2),
+                   "map.n%03d", x, y);
 
   secfile_insert_int(file, game.save_options.save_known, "game.save_known");
   if (game.save_options.save_known) {
     /* put "final" 4 bits of special flags */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=dec2hex[(map_get_tile(x, y)->special&0xf000)>>12];
-      pbuf[x]='\0';
-
-      secfile_insert_str(file, pbuf, "map.f%03d", y);
-    }
-
-    /* put bit 0-3 of known bits */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=dec2hex[(map_get_tile(x, y)->known&0xf)];
-      pbuf[x]='\0';
-      secfile_insert_str(file, pbuf, "map.a%03d", y);
-    }
-
-    /* put bit 4-7 of known bits */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=dec2hex[((map_get_tile(x, y)->known&0xf0))>>4];
-      pbuf[x]='\0';
-      secfile_insert_str(file, pbuf, "map.b%03d", y);
-    }
-
-    /* put bit 8-11 of known bits */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=dec2hex[((map_get_tile(x, y)->known&0xf00))>>8];
-      pbuf[x]='\0';
-      secfile_insert_str(file, pbuf, "map.c%03d", y);
-    }
-
-    /* put bit 12-15 of known bits */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=dec2hex[((map_get_tile(x, y)->known&0xf000))>>12];
-      pbuf[x]='\0';
-      secfile_insert_str(file, pbuf, "map.d%03d", y);
-    }
+    save_reg_map_data(file, DEC2HEX(map_get_tile(x, y)->special, 3),
+                       "map.f%03d", x, y);
 
-    /* put bit 16-19 of known bits */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=dec2hex[((map_get_tile(x, y)->known&0xf0000))>>16];
-      pbuf[x]='\0';
-      secfile_insert_str(file, pbuf, "map.e%03d", y);
-    }
-
-    /* put bit 20-23 of known bits */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=dec2hex[((map_get_tile(x, y)->known&0xf00000))>>20];
-      pbuf[x]='\0';
-      secfile_insert_str(file, pbuf, "map.g%03d", y); /* f already taken! */
-    }
-
-    /* put bit 24-27 of known bits */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=dec2hex[((map_get_tile(x, y)->known&0xf000000))>>24];
-      pbuf[x]='\0';
-      secfile_insert_str(file, pbuf, "map.h%03d", y);
-    }
-
-    /* put bit 28-31 of known bits */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=dec2hex[((map_get_tile(x, y)->known&0xf0000000))>>28];
-      pbuf[x]='\0';
-      secfile_insert_str(file, pbuf, "map.i%03d", y);
-    }
+    /* Put 4-bit segments of the 32-bit "known" field */
+    save_reg_map_data(file, DEC2HEX(map_get_tile(x, y)->known, 0),
+                     "map.a%03d", x, y);
+    save_reg_map_data(file, DEC2HEX(map_get_tile(x, y)->known, 1),
+                     "map.b%03d", x, y);
+    save_reg_map_data(file, DEC2HEX(map_get_tile(x, y)->known, 2),
+                     "map.c%03d", x, y);
+    save_reg_map_data(file, DEC2HEX(map_get_tile(x, y)->known, 3),
+                     "map.d%03d", x, y);
+    save_reg_map_data(file, DEC2HEX(map_get_tile(x, y)->known, 4),
+                     "map.e%03d", x, y);
+    save_reg_map_data(file, DEC2HEX(map_get_tile(x, y)->known, 5),
+                     "map.g%03d", x, y);
+    save_reg_map_data(file, DEC2HEX(map_get_tile(x, y)->known, 6),
+                     "map.h%03d", x, y);
+    save_reg_map_data(file, DEC2HEX(map_get_tile(x, y)->known, 7),
+                     "map.i%03d", x, y);
   }
-  
-  free(pbuf);
 }
 
 /***************************************************************
@@ -1458,10 +1398,9 @@
 static void player_save(struct player *plr, int plrno,
                        struct section_file *file)
 {
-  int i, x, y;
+  int i;
   char invs[A_LAST+1];
   struct player_spaceship *ship = &plr->spaceship;
-  char *pbuf=fc_malloc(map.xsize+1);
 
   secfile_insert_str(file, plr->name, "player%d.name", plrno);
   secfile_insert_str(file, plr->username, "player%d.username", plrno);
@@ -1724,78 +1663,34 @@
   if (game.fogofwar
       && game.save_options.save_private_map) {
     /* put the terrain type */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=terrain_chars[map_get_player_tile(x, y, plr)->terrain];
-      pbuf[x]='\0';
-      
-      secfile_insert_str(file, pbuf, "player%d.map_t%03d", plrno, y);
-    }
-    
-    /* put lower 4 bits of special flags */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-      pbuf[x]=dec2hex[map_get_player_tile(x, y, plr)->special&0xf];
-      pbuf[x]='\0';
-      
-      secfile_insert_str(file, pbuf, "player%d.map_l%03d",plrno,  y);
-    }
-    
-    /* put upper 4 bits of special flags */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=dec2hex[(map_get_player_tile(x, y, plr)->special&0xf0)>>4];
-      pbuf[x]='\0';
-      
-      secfile_insert_str(file, pbuf, "player%d.map_u%03d", plrno,  y);
-    }
-    
-    /* put "next" 4 bits of special flags */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=dec2hex[(map_get_player_tile(x, y, plr)->special&0xf00)>>8];
-      pbuf[x]='\0';
-      
-      secfile_insert_str(file, pbuf, "player%d.map_n%03d", plrno, y);
-    }
-    
-    /* put lower 4 bits of updated */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=dec2hex[map_get_player_tile(x, y, plr)->last_updated&0xf];
-      pbuf[x]='\0';
-      
-      secfile_insert_str(file, pbuf, "player%d.map_ua%03d",plrno,  y);
-    }
-    
-    /* put upper 4 bits of updated */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=dec2hex[(map_get_player_tile(x, y, plr)->last_updated&0xf0)>>4];
-      pbuf[x]='\0';
-      
-      secfile_insert_str(file, pbuf, "player%d.map_ub%03d", plrno,  y);
-    }
-    
-    /* put "next" 4 bits of updated */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=dec2hex[(map_get_player_tile(x, y, 
plr)->last_updated&0xf00)>>8];
-      pbuf[x]='\0';
-      
-      secfile_insert_str(file, pbuf, "player%d.map_uc%03d", plrno, y);
-    }
-    
-    /* put "yet next" 4 bits of updated */
-    for(y=0; y<map.ysize; y++) {
-      for(x=0; x<map.xsize; x++)
-       pbuf[x]=dec2hex[(map_get_player_tile(x, y, 
plr)->last_updated&0xf000)>>12];
-      pbuf[x]='\0';
-      
-      secfile_insert_str(file, pbuf, "player%d.map_ud%03d", plrno, y);
-    }
-    
-    free(pbuf);
+    save_plr_map_data(file,
+                 terrain_chars[map_get_player_tile(x, y, plr)->terrain],
+                  "player%d.map_t%03d", x, y, plrno);
+
+    /* Put 4-bit segments of 12-bit "special flags" field */
+    save_plr_map_data(file,
+                  DEC2HEX(map_get_player_tile(x, y, plr)->special, 0),
+                 "player%d.map_l%03d", x, y, plrno);
+    save_plr_map_data(file,
+                  DEC2HEX(map_get_player_tile(x, y, plr)->special, 1),
+                  "player%d.map_u%03d", x, y, plrno);
+    save_plr_map_data(file,
+                  DEC2HEX(map_get_player_tile(x, y, plr)->special, 2),
+                 "player%d.map_n%03d", x, y, plrno);
+
+    /* put 4-bit segments of 16-bit "updated" field */
+    save_plr_map_data(file,
+                 DEC2HEX(map_get_player_tile(x, y, plr)->last_updated, 0),
+                 "player%d.map_ua%03d", x, y, plrno);
+    save_plr_map_data(file,
+                 DEC2HEX(map_get_player_tile(x, y, plr)->last_updated, 1),
+                 "player%d.map_ub%03d", x, y, plrno);  
+    save_plr_map_data(file,
+                 DEC2HEX(map_get_player_tile(x, y, plr)->last_updated, 2),
+                 "player%d.map_uc%03d", x, y, plrno);
+    save_plr_map_data(file,
+                 DEC2HEX(map_get_player_tile(x, y, plr)->last_updated, 3),
+                 "player%d.map_ud%03d", x, y, plrno);
     
     if (1) {
       struct dumb_city *pdcity;