Re: [Freeciv-Dev] terrain ruleset implementation

[David Pfitzner <dwp@xxxxxxxxxxxxxx>]

Jeff Mallatt <jjm@xxxxxxxxxxxx> wrote:

> I started off to make the various terrain types more like those in Civ2...
> .. What I wound up doing was implementing the terrain ruleset!

>     ftp://dumuzi.codewell.com/pub/prj/freeciv/terrain_patch.tar.gz

I tried it out, and I like it a lot!
I think it should be a good "bullet point" for the next release :-)

Some comments:

The main problem is that it does not read old savefiles.  
IMO the server should _always_ read old savefiles as best
it can.

The attached patch fixes this, I think.  In this case it means 
using a default value for game.farmfood, and treating the extra 
specials as zeros.  Also you must save the game.ruleset.terrain 
string!  I assume that for old-style savegames the "classic"
terrain.ruleset would be appropriate.

Concerning the ruleset files, it looks to me now that the
Civ2 terrain.ruleset is actually the same as the new default.
So I guess the only reason for introducing it (and the data/civ2
dir) is for future use?  I guess on reflection I agree that this
is a good idea.  But the classic terrain.ruleset should be
data/classic_terrain.ruleset (after my other patch) because the
classic dir, while still in CVS, is no longer included in the
tar distribution!

Also, it looks like you have the default terrain data both
compiled into the executable, and in the default ruleset file.  
In general I am opposed to this duplication, because it means
that updates to the default set would have to be carefully
done to both, or become out of sync.  I would prefer just the
ruleset file, and no default compiled-in terrain data.

Since this patch breaks old tiles.xpm, I wonder if this would
be a good time to split tiles.xpm into terrain.xpm and 
special.xpm?  (Would this be good?  The latter would be 
numbers, hit-point bars, etc).  And perhaps also give hills, 
forests and mountains a full terrain line, rather than just 
four tiles (no vertical continuity).  (Initially the full line 
could just be duplications of those four, but it would allow 
for later improvement).  On the other hand I don't think the 
terrain.ruleset patch should be delayed for this or other 
unnecessary reasons.  Maybe after terrain.ruleset but before 
the next release...

Regards,
-- David

diff -u -r --exclude-from exclude freeciv-mod/server/gamehand.c 
fc-adv/server/gamehand.c
--- freeciv-mod/server/gamehand.c       Sun Jul 18 18:49:28 1999
+++ fc-adv/server/gamehand.c    Sun Jul 18 18:43:00 1999
@@ -275,7 +275,8 @@
     game.rail_food  = secfile_lookup_int(file, "game.rail_food");
     game.rail_prod  = secfile_lookup_int(file, "game.rail_prod");
     game.rail_trade = secfile_lookup_int(file, "game.rail_trade");
-    game.farmfood   = secfile_lookup_int(file, "game.farmfood");
+    game.farmfood   = secfile_lookup_int_default(file, GAME_DEFAULT_FARMFOOD,
+                                                "game.farmfood");
 
     game.foodbox     = secfile_lookup_int(file, "game.foodbox");
     game.techpenalty = secfile_lookup_int(file, "game.techpenalty");
@@ -329,6 +330,9 @@
     strcpy(game.ruleset.buildings,
           secfile_lookup_str(file, "game.ruleset.buildings"));
   }
+  strcpy(game.ruleset.terrain,
+        secfile_lookup_str_default(file, "classic",
+                                   "game.ruleset.terrain"));
 
   game.spacerace = secfile_lookup_int_default(file, game.spacerace,
                                              "game.spacerace");
@@ -496,6 +500,7 @@
   secfile_insert_str(file, game.ruleset.techs, "game.ruleset.techs");
   secfile_insert_str(file, game.ruleset.units, "game.ruleset.units");
   secfile_insert_str(file, game.ruleset.buildings, "game.ruleset.buildings");
+  secfile_insert_str(file, game.ruleset.terrain, "game.ruleset.terrain");
 
   if (1) {
     /* Now always save these, so the server options reflect the
diff -u -r --exclude-from exclude freeciv-mod/server/maphand.c 
fc-adv/server/maphand.c
--- freeciv-mod/server/maphand.c        Sun Jul 18 18:49:28 1999
+++ fc-adv/server/maphand.c     Sun Jul 18 18:46:59 1999
@@ -498,17 +498,19 @@
 
   /* get "next" 4 bits of special flags */
   for(y=0; y<map.ysize; y++) {
-    char *terline=secfile_lookup_str(file, "map.n%03d", y);
+    char *terline=secfile_lookup_str_default(file, NULL, "map.n%03d", y);
 
-    for(x=0; x<map.xsize; x++) {
-      char ch=terline[x];
+    if (terline) {
+      for(x=0; x<map.xsize; x++) {
+       char ch=terline[x];
 
-      if(isxdigit(ch)) {
-       map_get_tile(x, y)->special|=(ch-(isdigit(ch) ? '0' : 'a'-10))<<8;
-      } else if(ch!=' ') {
-       freelog(LOG_FATAL, "unknown special flag(next) (map.n) in map "
-               "at position(%d,%d): %d '%c'", x, y, ch, ch);
-       exit(1);
+       if(isxdigit(ch)) {
+         map_get_tile(x, y)->special|=(ch-(isdigit(ch) ? '0' : 'a'-10))<<8;
+       } else if(ch!=' ') {
+         freelog(LOG_FATAL, "unknown special flag(next) (map.n) in map "
+                 "at position(%d,%d): %d '%c'", x, y, ch, ch);
+         exit(1);
+       }
       }
     }
   }