[Freeciv-Dev] (PR#2815) use A_LAST for never-obsoleted impmrovements

["Jason Short via RT" <rt@xxxxxxxxxxxxxx>]

Currently A_NONE is used as a label for never-obsoleted improvements.

The disadvantage of this is that it has to always be special-cased.  It 
makes a lot more sense to use A_LAST for this.  (Units use -1, with a 
comment that U_LAST should probably be used instead.)

An example of the problems with the current system can be seen by 
changing the obsolete_by of Barracks to "None".  This should cause 
barracks never to go obsolete, and in many places the check is done 
correctly.  But in helpdata.c the check is just for tech_exists and so 
we get the incorrect help string: "Note that discovering None or Mobile 
Warfare will obsolete any existing Barracks."  But if we use A_LAST then 
the logical check will work.

Making this change does mean changing a few other places: any 
comparisons to A_NONE should be changed to tech_exists() queries.  Note 
that server and client may in theory have different values for A_LAST, 
so it's safest to avoid any direct comparisons.

jason

Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.274
diff -u -r1.274 packhand.c
--- client/packhand.c   2003/01/12 18:04:07     1.274
+++ client/packhand.c   2003/01/14 01:00:35
@@ -1880,8 +1880,12 @@
        freelog(LOG_DEBUG, "    %2d/%s",
                b->equiv_repl[inx], improvement_types[b->equiv_repl[inx]].name);
       }
-      freelog(LOG_DEBUG, "  obsolete_by %2d/%s",
-             b->obsolete_by, advances[b->obsolete_by].name);
+      if (tech_exists(b->obsolete_by)) {
+       freelog(LOG_DEBUG, "  obsolete_by %2d/%s",
+               b->obsolete_by, advances[b->obsolete_by].name);
+      } else {
+       freelog(LOG_DEBUG, "  obsolete_by %2d/Never", b->obsolete_by);
+      }
       freelog(LOG_DEBUG, "  is_wonder   %2d", b->is_wonder);
       freelog(LOG_DEBUG, "  build_cost %3d", b->build_cost);
       freelog(LOG_DEBUG, "  upkeep      %2d", b->upkeep);
Index: common/capstr.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/capstr.c,v
retrieving revision 1.116
diff -u -r1.116 capstr.c
--- common/capstr.c     2003/01/03 08:58:47     1.116
+++ common/capstr.c     2003/01/14 01:00:36
@@ -74,7 +74,8 @@
  * are not directly related to the capability strings discussed here.)
  */
 
-#define CAPABILITY "+1.14.0 conn_info +occupied team tech_impr_gfx"
+#define CAPABILITY "+1.14.0 conn_info +occupied team tech_impr_gfx " \
+                   "obsolete_last"
   
 /* "+1.14.0" is protocol for 1.14.0 release.
  *
@@ -88,6 +89,9 @@
  *
  * "tech_impr_gfx" is support for loading of ruleset-specified
  * technology and city improvement icons.
+ *
+ * "obsolete_last" means A_LAST is used to mark improvements that are never
+ * obsoleted.  Previously A_NONE was used.
  */
 
 void init_our_capability(void)
Index: common/improvement.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/improvement.c,v
retrieving revision 1.29
diff -u -r1.29 improvement.c
--- common/improvement.c        2003/01/05 23:24:52     1.29
+++ common/improvement.c        2003/01/14 01:00:36
@@ -306,8 +306,9 @@
 **************************************************************************/
 bool improvement_obsolete(struct player *pplayer, Impr_Type_id id) 
 {
-  if (improvement_types[id].obsolete_by==A_NONE) 
+  if (!tech_exists(improvement_types[id].obsolete_by)) {
     return FALSE;
+  }
 
   if (improvement_types[id].is_wonder) {
     /* a wonder is obsolette, as soon as *any* player researched the
@@ -386,10 +387,8 @@
 ...
 **************************************************************************/
 bool wonder_obsolete(Impr_Type_id id)
-{ 
-  if (improvement_types[id].obsolete_by==A_NONE)
-    return FALSE;
-  return (game.global_advances[improvement_types[id].obsolete_by] != 0);
+{
+  return improvement_obsolete(NULL, id);
 }
 
 /**************************************************************************
Index: common/improvement.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/improvement.h,v
retrieving revision 1.17
diff -u -r1.17 improvement.h
--- common/improvement.h        2003/01/05 23:24:52     1.17
+++ common/improvement.h        2003/01/14 01:00:36
@@ -187,7 +187,7 @@
   enum effect_range equiv_range;
   Impr_Type_id *equiv_dupl;            /* list; B_LAST terminated */
   Impr_Type_id *equiv_repl;            /* list; B_LAST terminated */
-  Tech_Type_id obsolete_by;            /* A_NONE = never obsolete */
+  Tech_Type_id obsolete_by;            /* A_LAST = never obsolete */
   bool is_wonder;
   int build_cost;
   int upkeep;
Index: common/packets.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/packets.c,v
retrieving revision 1.228
diff -u -r1.228 packets.c
--- common/packets.c    2003/01/03 08:58:47     1.228
+++ common/packets.c    2003/01/14 01:00:38
@@ -2109,7 +2109,15 @@
   dio_put_uint8(&dout, packet->equiv_range);
   dio_put_uint8_vec8(&dout, packet->equiv_dupl, B_LAST);
   dio_put_uint8_vec8(&dout, packet->equiv_repl, B_LAST);
-  dio_put_uint8(&dout, packet->obsolete_by);
+  if (has_capability("obsolete_last", pc->capability)) {
+    dio_put_uint8(&dout, packet->obsolete_by);
+  } else {
+    if (!tech_exists(packet->obsolete_by)) {
+      dio_put_uint8(&dout, A_NONE);
+    } else {
+      dio_put_uint8(&dout, packet->obsolete_by);
+    }
+  }
   dio_put_bool8(&dout, packet->is_wonder);
   dio_put_uint16(&dout, packet->build_cost);
   dio_put_uint8(&dout, packet->upkeep);
@@ -2169,6 +2177,11 @@
   dio_get_uint8_vec8(&din, &packet->equiv_dupl, B_LAST);
   dio_get_uint8_vec8(&din, &packet->equiv_repl, B_LAST);
   dio_get_uint8(&din, &packet->obsolete_by);
+  if (!has_capability("obsolete_last", pc->capability)) {
+    if (packet->obsolete_by == A_NONE) {
+      packet->obsolete_by = A_LAST;
+    }
+  }
   dio_get_bool8(&din, &packet->is_wonder);
   dio_get_uint16(&din, &packet->build_cost);
   dio_get_uint8(&din, &packet->upkeep);
Index: server/ruleset.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/ruleset.c,v
retrieving revision 1.130
diff -u -r1.130 ruleset.c
--- server/ruleset.c    2003/01/13 23:27:12     1.130
+++ server/ruleset.c    2003/01/14 01:00:40
@@ -1022,8 +1022,11 @@
 
     b->obsolete_by = lookup_tech(file, sec[i], "obsolete_by",
                                 FALSE, filename, b->name);
-    if ((b->obsolete_by == A_LAST) || !tech_exists(b->obsolete_by)) {
-      b->obsolete_by = A_NONE;
+    if (b->obsolete_by == A_NONE || !tech_exists(b->obsolete_by)) {
+      /* The ruleset can specify "None" for a never-obsoleted improvement.
+       * Currently this means A_NONE, which is an unnecessary special-case.
+       * We use A_LAST to flag it instead. */
+      b->obsolete_by = A_LAST;
     }
 
     b->is_wonder = secfile_lookup_bool(file, "%s.is_wonder", sec[i]);
@@ -1219,11 +1222,12 @@
                b->name, advances[b->tech_req].name, filename);
        b->tech_req = A_LAST;
       }
-      if (!tech_exists(b->obsolete_by)) {
+      if (b->obsolete_by != A_LAST
+         && (b->obsolete_by == A_NONE || !tech_exists(b->obsolete_by))) {
        freelog(LOG_ERROR,
                "improvement \"%s\": obsoleted by removed tech \"%s\" (%s)",
                b->name, advances[b->obsolete_by].name, filename);
-       b->obsolete_by = A_NONE;
+       b->obsolete_by = A_LAST;
       }
       for (j = 0; b->effect[j].type != EFT_LAST; j++) {
        if (!tech_exists(b->effect[j].cond_adv)) {