[Freeciv-Dev] (PR#5568) add I_DEMOLISHED Impr_Status

["Mike Kaufman" <kaufman@xxxxxxxxxxxxxxxxxxxxxx>]

Here is a patch that adds I_DEMOLISHED Impr_Status. It is needed for
general effects.

If a building has an effect which survives the building's destruction, then
the client (and the savegame) needs to know about it.

Though I believe that this patch is needed, I'm becoming more and more
convinced that a generalized 'effect survival' is a bleedin' nightmare. I
already have it in the effects code that a building that is demolished and
rebuilt in the same city will NOT confer the same effect twice. That's no
problem and warranted.

The problem comes on city destruction. What happens if a building confers a
global effect that survives and then its city is destroyed. We still want
that effect active (Apollo and Manhattan are examples). Where is the effect
stored in a savegame? Currently we have a destroyed_wonders field which can
be transformed into a global_improvements field: a array of B_LAST elements
that counts the total number of improvments for everybody. But we would
also need such an array for each player (Player range) and each island
(Island range). Unless we restrict .survives in some way, I don't see
another way. 

Maybe it's time to think hard about breaking forward compatibility for
savegames (i.e. 1.14 servers won't be able to load [correctly] games saved
with 1.15 servers).

-mike 

diff -Nur -Xsnap/diff_ignore snap/client/packhand.c snap-i/client/packhand.c
--- snap/client/packhand.c      2003-08-27 21:59:08.000000000 -0500
+++ snap-i/client/packhand.c    2003-08-29 21:16:05.000000000 -0500
@@ -253,26 +253,37 @@
 
 /**************************************************************************
   Updates a city's list of improvements from packet data. "impr" identifies
-  the improvement, and "have_impr" specifies whether the improvement should
-  be added (TRUE) or removed (FALSE). "impr_changed" is set TRUE only if
-  the existing improvement status was changed by this call.
+  the improvement, and "status" specifies what the new status of the 
+  improvement is. "impr_changed" is set TRUE only if the existing improvement 
+  status was changed by this call.
 **************************************************************************/
 static void update_improvement_from_packet(struct city *pcity,
-                                          Impr_Type_id impr, bool have_impr,
+                                          Impr_Type_id impr, 
+                                           Impr_Status status,
                                           bool *impr_changed)
 {
-  if (have_impr && pcity->improvements[impr] == I_NONE) {
+  Impr_Status old = pcity->improvements[impr]; /* old Impr_Status */
+
+  if (status == old) {
+    return;
+  }
+
+  assert(!(status == I_NONE && old == I_DEMOLISHED));
+
+  if (status == I_ACTIVE && (old == I_NONE || old == I_DEMOLISHED)) {
     city_add_improvement(pcity, impr);
 
-    if (impr_changed) {
-      *impr_changed = TRUE;
-    }
-  } else if (!have_impr && pcity->improvements[impr] != I_NONE) {
+  } else if (status == I_DEMOLISHED && (old == I_NONE || old == I_ACTIVE)) {
+    /* the improvement has effects which survived destruction */
+    city_add_improvement(pcity, impr);
     city_remove_improvement(pcity, impr);
 
-    if (impr_changed) {
-      *impr_changed = TRUE;
-    }
+  } else if (status == I_NONE && old != I_NONE) {
+    city_remove_improvement(pcity, impr);
+  }
+
+  if (impr_changed) {
+    *impr_changed = TRUE;
   }
 }
 
@@ -459,12 +470,13 @@
   }
   
   impr_type_iterate(i) {
-    if (pcity->improvements[i] == I_NONE && packet->improvements[i] == '1'
-       && !city_is_new) {
+    if ((pcity->improvements[i] == I_NONE ||
+         pcity->improvements[i] == I_DEMOLISHED)
+        && packet->improvements[i] == I_ACTIVE && !city_is_new) {
       audio_play_sound(get_improvement_type(i)->soundtag,
                       get_improvement_type(i)->soundtag_alt);
     }
-    update_improvement_from_packet(pcity, i, packet->improvements[i] == '1',
+    update_improvement_from_packet(pcity, i, packet->improvements[i],
                                    &need_effect_update);
   } impr_type_iterate_end;
 
@@ -631,9 +643,11 @@
                            ARRAY_SIZE(pcity->improvements));
   }
 
-  update_improvement_from_packet(pcity, B_PALACE, packet->capital,
+  update_improvement_from_packet(pcity, B_PALACE,
+                                 packet->capital  ? I_ACTIVE : I_NONE,
                                  &need_effect_update);
-  update_improvement_from_packet(pcity, B_CITY, packet->walls,
+  update_improvement_from_packet(pcity, B_CITY, 
+                                 packet->walls ? I_ACTIVE : I_NONE,
                                  &need_effect_update);
 
   if (city_is_new) {
diff -Nur -Xsnap/diff_ignore snap/common/capstr.c snap-i/common/capstr.c
--- snap/common/capstr.c        2003-08-05 19:01:19.000000000 -0500
+++ snap-i/common/capstr.c      2003-08-29 21:13:13.000000000 -0500
@@ -79,7 +79,7 @@
                    "+impr_req +waste +fastfocus +continent +small_dipl " \
                    "+no_nation_selected +diplomacy +no_extra_tiles " \
                    "+diplomacy2 +citizens_style +root_tech auth " \
-                   "+nat_ulimit +retake +goto_pack borders"
+                   "+nat_ulimit +retake +goto_pack borders +impr_nbstr"
 
 /* "+1.14.0" is protocol for 1.14.0 release.
  *
@@ -142,6 +142,9 @@
  * map.xsize/map.ysize to mark the end of the goto chunk.
  *
  * "borders" is support for national borders.
+ *
+ * "impr_nbstr" mean that we no longer send improvements to the client
+ * as a bit string. We send it as a full char array.
  */
 
 void init_our_capability(void)
diff -Nur -Xsnap/diff_ignore snap/common/city.c snap-i/common/city.c
--- snap/common/city.c  2003-08-16 20:44:37.000000000 -0500
+++ snap-i/common/city.c        2003-08-29 21:01:29.000000000 -0500
@@ -468,10 +468,12 @@
 **************************************************************************/
 bool city_got_building(struct city *pcity,  Impr_Type_id id) 
 {
-  if (!improvement_exists(id))
+  if (!improvement_exists(id)) {
     return FALSE;
-  else 
-    return (pcity->improvements[id] != I_NONE);
+  } else {
+    return (pcity->improvements[id] != I_NONE
+            && pcity->improvements[id] != I_DEMOLISHED);
+  }
 }
 
 /**************************************************************************
@@ -2400,6 +2402,11 @@
   
   mark_improvement(pcity, impr, I_NONE);
 
+  /* replace with proper effects check soon */
+  if (impr == B_APOLLO || impr == B_MANHATTEN) {
+    mark_improvement(pcity, impr, I_DEMOLISHED);
+  }
+
   improvements_update_redundant(pplayer, pcity,
                                 map_get_continent(pcity->x, pcity->y),
                                 improvement_types[impr].equiv_range);
diff -Nur -Xsnap/diff_ignore snap/common/city.h snap-i/common/city.h
--- snap/common/city.h  2003-08-16 20:44:37.000000000 -0500
+++ snap-i/common/city.h        2003-08-29 21:02:35.000000000 -0500
@@ -482,7 +482,8 @@
  */
 #define built_impr_iterate(m_pcity, m_i)                                      \
   impr_type_iterate(m_i) {                                                    \
-    if((m_pcity)->improvements[m_i] == I_NONE) {                              \
+    if((m_pcity)->improvements[m_i] == I_NONE                                 \
+       || (m_pcity)->improvements[m_i] == I_DEMOLISHED) {                     \
       continue;                                                               \
     }
 
diff -Nur -Xsnap/diff_ignore snap/common/improvement.h 
snap-i/common/improvement.h
--- snap/common/improvement.h   2003-08-10 08:41:29.000000000 -0500
+++ snap-i/common/improvement.h 2003-08-29 21:04:16.000000000 -0500
@@ -32,11 +32,11 @@
  * An enum or bitfield would be neater here, but we use a typedef for
  * a) less memory usage and b) compatibility with old behaviour */
 typedef unsigned char Impr_Status;
-#define I_NONE       0   /* Improvement not built */
-#define I_ACTIVE     1   /* Improvement built, and having its effect */
-#define I_OBSOLETE   2   /* Built, but obsoleted by a tech */
-#define I_REDUNDANT  3   /* Built, but replaced by wonder/other building */
-
+#define I_NONE       '0'   /* Improvement not built */
+#define I_ACTIVE     '1'   /* Improvement built, and having its effect */
+#define I_OBSOLETE   '2'   /* Built, but obsoleted by a tech */
+#define I_REDUNDANT  '3'   /* Built, but replaced by wonder/other building */
+#define I_DEMOLISHED '4'   /* Destroyed, but effects survive */
 
 /* FIXME: Remove this define when there is per-file need for this enum. */
 #define OLD_IMPR_TYPE_ENUM
diff -Nur -Xsnap/diff_ignore snap/common/packets.c snap-i/common/packets.c
--- snap/common/packets.c       2003-08-07 17:13:16.000000000 -0500
+++ snap-i/common/packets.c     2003-08-29 21:08:55.000000000 -0500
@@ -1368,7 +1368,7 @@
                        COND_SET_BIT(req->changed_from_is_unit, 6)));
 
   dio_put_city_map(&dout, (char *) req->city_map);
-  dio_put_bit_string(&dout, (char *) req->improvements);
+  dio_put_string(&dout, (char *) req->improvements);
 
   /* only 8 options allowed before need to extend protocol */
   dio_put_uint8(&dout, req->city_options);
@@ -1459,8 +1459,8 @@
 
   dio_get_city_map(&din, (char *) packet->city_map,
                   sizeof(packet->city_map));
-  dio_get_bit_string(&din, (char *) packet->improvements,
-                    sizeof(packet->improvements));
+  dio_get_string(&din, (char *) packet->improvements,
+                sizeof(packet->improvements));
 
   dio_get_uint8(&din, &packet->city_options);
 
diff -Nur -Xsnap/diff_ignore snap/server/savegame.c snap-i/server/savegame.c
--- snap/server/savegame.c      2003-08-27 21:59:14.000000000 -0500
+++ snap-i/server/savegame.c    2003-08-29 21:14:18.000000000 -0500
@@ -935,8 +935,15 @@
                            ARRAY_SIZE(pcity->improvements));
 
     impr_type_iterate(x) {
-      if (*p != '\0' && *p++=='1') {
-        city_add_improvement(pcity,x);
+      if (*p != '\0') {
+        if (*p == I_ACTIVE || *p == I_OBSOLETE || *p == I_REDUNDANT) {
+          city_add_improvement(pcity, x);
+        }
+        if (*p == I_DEMOLISHED) {
+          city_add_improvement(pcity, x);
+          city_remove_improvement(pcity, x);
+        }
+        p++;
       }
     } impr_type_iterate_end;
 
@@ -1579,10 +1586,28 @@
     secfile_insert_int(file, pcity->currently_building, 
                       "player%d.c%d.currently_building", plrno, i);
 
+    /* we can't save redundant or obsolete improvement statuses or
+     * old servers will fail to load new savegames correctly :( */
     impr_type_iterate(id) {
-      buf[id] = (pcity->improvements[id] != I_NONE) ? '1' : '0';
+      switch (pcity->improvements[id]) {
+      case I_NONE:
+        buf[id] = I_NONE;
+        break;
+      case I_ACTIVE:
+      case I_OBSOLETE:
+      case I_REDUNDANT:
+        buf[id] = I_ACTIVE;
+        break;
+      case I_DEMOLISHED:
+        buf[id] = I_DEMOLISHED;
+        break;
+      default:
+        assert(0);
+      }
     } impr_type_iterate_end;
+
     buf[game.num_impr_types] = '\0';
+
     secfile_insert_str(file, buf, "player%d.c%d.improvements", plrno, i);
 
     worklist_save(file, "player%d.c%d", plrno, i, &pcity->worklist);