[Freeciv-Dev] Re: (PR#9864) Terrain under cities changes during save+load

["Marko Lindqvist" <marko.lindqvist@xxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=9864 >


  Attached patch should fix this.

  Basicly I just reversed logic. No longer is tile knowledge updated 
when tile is sent, but tile is sent to clients when player knowledge 
about it changes.

  - Renamed update_tile_knowledge() as update_player_tile_knowledge()
  - New function update_tile_knowledge() updates tile knowledge for 
everybody who sees tile and sends to tile info to clients as needed.
  - Removed tile knowledge update from send_tile_info()
  - Where ever send_tile_info() was called _for terrain change_, now 
update_tile_knowledge() is called.


  With this patch savegames are in much better shape for comparisons. AI 
players were not affected by false information (they read right info 
directly anyway) but in some rather rare situations human players would 
have been affected.


  - Caz


diff -Nurd -X.diff_ignore freeciv/server/barbarian.c freeciv/server/barbarian.c
--- freeciv/server/barbarian.c  2004-09-11 18:08:15.359375000 +0300
+++ freeciv/server/barbarian.c  2004-09-11 19:14:20.171875000 +0300
@@ -379,7 +379,7 @@
   if (map_has_special(xu, yu, S_HUT)) {
     /* remove the hut in place of uprising */
     map_clear_special(xu, yu, S_HUT);
-    send_tile_info(NULL, xu, yu);
+    update_tile_knowledge(xu, yu);
   }
 
   if (!is_ocean(map_get_terrain(xu,yu))) {
diff -Nurd -X.diff_ignore freeciv/server/citytools.c freeciv/server/citytools.c
--- freeciv/server/citytools.c  2004-09-11 18:08:16.171875000 +0300
+++ freeciv/server/citytools.c  2004-09-11 19:13:45.890625000 +0300
@@ -880,7 +880,7 @@
                    " the city with railroads."),
                  pcity->name);
     map_set_special(pcity->x, pcity->y, S_RAILROAD);
-    send_tile_info(NULL, pcity->x, pcity->y);
+    update_tile_knowledge(pcity->x, pcity->y);
   }
 
   map_fog_pseudo_city_area(pgiver, pcity->x, pcity->y);
@@ -915,9 +915,16 @@
 
   if (terrain_control.may_road) {
     map_set_special(x, y, S_ROAD);
-    if (player_knows_techs_with_flag(pplayer, TF_RAILROAD))
+    if (player_knows_techs_with_flag(pplayer, TF_RAILROAD)) {
       map_set_special(x, y, S_RAILROAD);
+      update_tile_knowledge(x, y);
+    }
   }
+
+  /* It is possible that update_tile_knowledge() already sent tile information
+     to some players, but we don't want to make any special handling for
+     those cases. Delta code will prevent second packet from being sent
+     anyway. */
   send_tile_info(NULL, x, y);
 
   pcity = create_city_virtual(pplayer, x, y, name);
@@ -982,7 +989,7 @@
     unit_list_iterate_end;
   }
   map_clear_special(x, y, S_FORTRESS);
-  send_tile_info(NULL, x, y);
+  update_tile_knowledge(x, y);
 
   reset_move_costs(x, y);
 /* I stupidly thought that setting S_ROAD took care of this, but of course
diff -Nurd -X.diff_ignore freeciv/server/cityturn.c freeciv/server/cityturn.c
--- freeciv/server/cityturn.c   2004-09-11 18:08:16.406250000 +0300
+++ freeciv/server/cityturn.c   2004-09-11 19:14:55.812500000 +0300
@@ -1176,7 +1176,7 @@
       if (!terrain_has_flag(map_get_terrain(mx, my), TER_NO_POLLUTION)
          && !map_has_special(mx, my, S_POLLUTION)) {
        map_set_special(mx, my, S_POLLUTION);
-       send_tile_info(NULL, mx, my);
+       update_tile_knowledge(mx, my);
        notify_player_ex(city_owner(pcity), pcity->x, pcity->y,
                         E_POLLUTION, _("Game: Pollution near %s."),
                         pcity->name);
diff -Nurd -X.diff_ignore freeciv/server/gamehand.c freeciv/server/gamehand.c
--- freeciv/server/gamehand.c   2004-09-11 18:08:18.093750000 +0300
+++ freeciv/server/gamehand.c   2004-09-11 19:15:25.890625000 +0300
@@ -76,7 +76,7 @@
    * tile (with the hut) earlier: */
   if (map_has_special(x, y, S_HUT)) {
     map_clear_special(x, y, S_HUT);
-    send_tile_info(NULL, x, y);
+    update_tile_knowledge(x, y);
     freelog(LOG_VERBOSE, "Removed hut on start position for %s",
            pplayer->name);
   }
diff -Nurd -X.diff_ignore freeciv/server/maphand.c freeciv/server/maphand.c
--- freeciv/server/maphand.c    2004-09-11 18:08:18.734375000 +0300
+++ freeciv/server/maphand.c    2004-09-11 19:09:42.625000000 +0300
@@ -205,7 +205,7 @@
     if (new != T_NONE && old != new) {
       effect--;
       change_terrain(x, y, new);
-      send_tile_info(NULL, x, y);
+      update_tile_knowledge(x, y);
       unit_list_iterate(map_get_tile(x, y)->units, punit) {
        if (!can_unit_continue_current_activity(punit)) {
          handle_unit_activity_request(punit, ACTIVITY_IDLE);
@@ -246,7 +246,7 @@
     if (new != T_NONE && old != new) {
       effect--;
       change_terrain(x, y, new);
-      send_tile_info(NULL, x, y);
+      update_tile_knowledge(x, y);
       unit_list_iterate(map_get_tile(x, y)->units, punit) {
        if (!can_unit_continue_current_activity(punit)) {
          handle_unit_activity_request(punit, ACTIVITY_IDLE);
@@ -295,7 +295,7 @@
   
   city_list_iterate(pplayer->cities, pcity) {
     map_set_special(pcity->x, pcity->y, S_RAILROAD);
-    send_tile_info(NULL, pcity->x, pcity->y);
+    update_tile_knowledge(pcity->x, pcity->y);
   }
   city_list_iterate_end;
 
@@ -422,8 +422,8 @@
 
 /**************************************************************************
   Send tile information to all the clients in dest which know and see
-  the tile.  Also updates player knowledge.  If dest is NULL, sends to
-  all clients (game.game_connections) which know and see tile.
+  the tile. If dest is NULL, sends to all clients (game.game_connections)
+  which know and see tile.
 **************************************************************************/
 void send_tile_info(struct conn_list *dest, int x, int y)
 {
@@ -453,9 +453,6 @@
       info.type = ptile->terrain;
       info.special = ptile->special;
       info.continent = ptile->continent;
-      if (pplayer) {
-       update_tile_knowledge(pplayer,x,y);
-      }
       send_packet_tile_info(pconn, &info);
     } else if (pplayer && map_is_known(x, y, pplayer)
               && map_get_seen(x, y, pplayer) == 0) {
@@ -507,7 +504,7 @@
     if (map_get_seen(x, y, pplayer) != 0) {
       /* Known and seen. */
       /* Visible; update info. */
-      update_tile_knowledge(pplayer,x,y);
+      update_player_tile_knowledge(pplayer,x,y);
       info.known = TILE_KNOWN;
     } else {
       /* Known but not seen. */
@@ -841,7 +838,7 @@
     map_set_known(x, y, pplayer);
 
     /* as the tile may be fogged send_tile_info won't always do this for us */
-    update_tile_knowledge(pplayer, x, y);
+    update_player_tile_knowledge(pplayer, x, y);
     update_player_tile_last_seen(pplayer, x, y);
 
     send_tile_info_always(pplayer, &pplayer->connections, x, y);
@@ -1072,15 +1069,45 @@
 }
 
 /***************************************************************
-...
+ Give player correct knowledge about tile, return true iff
+ knowledge changed.
 ***************************************************************/
-void update_tile_knowledge(struct player *pplayer, int x, int y)
+bool update_player_tile_knowledge(struct player *pplayer, int x, int y)
 {
   struct tile *ptile = map_get_tile(x, y);
   struct player_tile *plrtile = map_get_player_tile(x, y, pplayer);
 
-  plrtile->terrain = ptile->terrain;
-  plrtile->special = ptile->special;
+  if (plrtile->terrain != ptile->terrain ||
+      plrtile->special != ptile->special) {
+    plrtile->terrain = ptile->terrain;
+    plrtile->special = ptile->special;
+    return TRUE;
+  }
+  return FALSE;
+}
+
+
+/***************************************************************
+ Update tile knowledge for everybody who sees tile.
+***************************************************************/
+void update_tile_knowledge(int x, int y)
+{
+  /* Players */
+  players_iterate(pplayer) {
+    if (map_is_known_and_seen(x, y, pplayer)) {
+      if (update_player_tile_knowledge(pplayer, x, y)) {
+        send_tile_info(&pplayer->connections, x, y);
+      }
+    }
+  } players_iterate_end;
+
+  /* Global observers */
+  conn_list_iterate(game.game_connections, pconn) {
+    struct player *pplayer = pconn->player;
+    if (!pplayer && pconn->observer) {
+      send_tile_info(&pconn->self, x , y);
+    }
+  } conn_list_iterate_end;
 }
 
 /***************************************************************
diff -Nurd -X.diff_ignore freeciv/server/maphand.h freeciv/server/maphand.h
--- freeciv/server/maphand.h    2004-09-11 18:08:18.750000000 +0300
+++ freeciv/server/maphand.h    2004-09-11 18:43:15.234375000 +0300
@@ -86,7 +86,8 @@
 void player_map_allocate(struct player *pplayer);
 void player_map_free(struct player *pplayer);
 struct player_tile *map_get_player_tile(int x, int y, struct player *pplayer);
-void update_tile_knowledge(struct player *pplayer,int x, int y);
+bool update_player_tile_knowledge(struct player *pplayer,int x, int y);
+void update_tile_knowledge(int x, int y);
 void update_player_tile_last_seen(struct player *pplayer, int x, int y);
 
 void give_shared_vision(struct player *pfrom, struct player *pto);
diff -Nurd -X.diff_ignore freeciv/server/savegame.c freeciv/server/savegame.c
--- freeciv/server/savegame.c   2004-09-11 18:08:19.046875000 +0300
+++ freeciv/server/savegame.c   2004-09-11 18:41:27.640625000 +0300
@@ -2127,7 +2127,7 @@
        is a bug in some pre-1.11 savegames. Anyway, it can't hurt */
     whole_map_iterate(x, y) {
       if (map_is_known_and_seen(x, y, plr)) {
-       update_tile_knowledge(plr, x, y);
+       update_player_tile_knowledge(plr, x, y);
        reality_check_city(plr, x, y);
        if (map_get_city(x, y)) {
          update_dumb_city(plr, map_get_city(x, y));
@@ -2143,7 +2143,7 @@
       if (map_is_known(x, y, plr)) {
        struct city *pcity = map_get_city(x, y);
        update_player_tile_last_seen(plr, x, y);
-       update_tile_knowledge(plr, x, y);
+       update_player_tile_knowledge(plr, x, y);
        if (pcity)
          update_dumb_city(plr, pcity);
       }
diff -Nurd -X.diff_ignore freeciv/server/unittools.c freeciv/server/unittools.c
--- freeciv/server/unittools.c  2004-09-11 18:08:19.984375000 +0300
+++ freeciv/server/unittools.c  2004-09-11 19:12:40.750000000 +0300
@@ -632,7 +632,7 @@
 
        if (what != S_NO_SPECIAL) {
          map_clear_special(punit->x, punit->y, what);
-         send_tile_info(NULL, punit->x, punit->y);
+         update_tile_knowledge(punit->x, punit->y);
          set_unit_activity(punit, ACTIVITY_IDLE);
        }
 
@@ -665,7 +665,7 @@
          send_unit_info(NULL, punit2);
        }
       } unit_list_iterate_end;
-      send_tile_info(NULL, punit->x, punit->y);
+      update_tile_knowledge(punit->x, punit->y);
       
       /* If a watchtower has been pillaged, reduce sight to normal */
       if (what_pillaged == S_FORTRESS
@@ -776,7 +776,7 @@
   }
 
   if (unit_activity_done) {
-    send_tile_info(NULL, punit->x, punit->y);
+    update_tile_knowledge(punit->x, punit->y);
     unit_list_iterate (map_get_tile(punit->x, punit->y)->units, punit2) {
       if (punit2->activity == activity) {
        bool alive = TRUE;
@@ -2081,12 +2081,12 @@
     if (game.rgame.nuke_contamination == CONTAMINATION_POLLUTION) {
       if (!map_has_special(x, y, S_POLLUTION)) {
        map_set_special(x, y, S_POLLUTION);
-       send_tile_info(NULL, x, y);
+       update_tile_knowledge(x, y);
       }
     } else {
       if (!map_has_special(x, y, S_FALLOUT)) {
        map_set_special(x, y, S_FALLOUT);
-       send_tile_info(NULL, x, y);
+       update_tile_knowledge(x, y);
       }
     }
   }
@@ -2386,7 +2386,7 @@
   }
 
   map_clear_special(punit->x, punit->y, S_HUT);
-  send_tile_info(NULL, punit->x, punit->y);
+  update_tile_knowledge(punit->x, punit->y);
 
   if (game.rgame.hut_overflight==OVERFLIGHT_FRIGHTEN && is_air_unit(punit)) {
     notify_player_ex(pplayer, punit->x, punit->y, E_NOEVENT,