[Freeciv-Dev] Re: (PR#13468) Aifill problems in pregame screen

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=13468 >

Mateusz Stefek wrote:
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=13468 >
> 
> Try:
> /set aifill 2
> /set aifill 10
> or
> /set aifill 10
> /set aifill 2
> /set aifill 5
> 
> Strange things are displayed on player list.
> Output of the /list command looks ok.

I had thought that is-info was removed from all the packets.  But I
guess this is far from the case, and this is another bug from it.

This patch fixes this and (probably) other problems by removing is-info
from all packets.  It may be overkill, as explained in the warning.
is-info is safe to add to some packets, so long as the data provided by
the packet is purely "static" at the client and the packet reception has
no side effects.

Example 1: player info is updated at the client when a player_remove
packet is sent.  The server then doesn't resend the next player info
packet because it's a duplicate of the last one sent.  But the delta
code doesn't know that the player's info was changed by the delta
packet.  Thus this packet cannot be is-info because this data is not
read-only at the client side.

Example 2: game.info is sent to the client in the game_info packet.  But
this info is purely static in the client; it is read-only.  Thus it is
safe to mark this as is-info.  This doesn't save much network bandwidth
and doesn't save much server processing, but it does save some client
processing.

Example 3: a seconds_to_phasedone value is added to the game.info
packet.  As a result the client must remember when it received the last
game-info packet.  Now receiving the game-info packet has side effects.
 It may no longer be marked is-info.

Conclusion: is-info is very bug-prone and should not be used unless
there is a demonstrable gain from doing so.  The only likely candidate I
see is the packet_tile_info.  However in all cases it is better if
possible to remove the spurious send_xxx calls from the server entirely.

This probably also causes bugs in 2.0.  All these same packets are
is-info.  However removing is-info on all these packets *could* cause a
great client slowdown so it shouldn't be done unless there's testing
before a release.

-jason

? class.diff
Index: common/packets.def
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/packets.def,v
retrieving revision 1.140
diff -p -u -r1.140 packets.def
--- common/packets.def  4 Jul 2005 18:42:27 -0000       1.140
+++ common/packets.def  14 Jul 2005 05:50:02 -0000
@@ -57,6 +57,9 @@ Syntax:
     Packet flags is a comma seperated list of:
 
      is-info: a second packet with the same content can be discarded
+              WARNING: this flag is dangerous and should not be used for
+              a packet that contains information that may be changed in
+              the client outside of the reception of that packet.
 
      pre-send:
      post-recv: 
@@ -280,7 +283,7 @@ end
 PACKET_SERVER_SHUTDOWN=8;sc,lsend
 end
 
-PACKET_NATION_AVAILABLE=9;sc,lsend,is-info
+PACKET_NATION_AVAILABLE=9;sc,lsend
   NATION id; key
 
   BOOL is_unavailable;
@@ -324,7 +327,7 @@ end
 
 /************** Info packets **********************/
 
-PACKET_TILE_INFO=14; is-info,sc,lsend
+PACKET_TILE_INFO=14; sc,lsend
   COORD x, y; key
 
   TERRAIN type;
@@ -445,7 +448,7 @@ PACKET_GAME_INFO=15; sc
   UINT16 great_wonders[B_LAST]; diff
 end
 
-PACKET_MAP_INFO=16; is-info,sc,lsend
+PACKET_MAP_INFO=16; sc,lsend
   XYSIZE xsize;
   XYSIZE ysize;
   UINT8 topology_id;
@@ -474,7 +477,7 @@ PACKET_CITY_REMOVE=20;sc,dsend,lsend
   CITY city_id;
 end
 
-PACKET_CITY_INFO=21; is-info,sc,lsend
+PACKET_CITY_INFO=21; sc,lsend
   CITY id; key
   PLAYER owner;
   COORD x,y;
@@ -521,7 +524,7 @@ PACKET_CITY_INFO=21; is-info,sc,lsend
   TURN turn_founded;
 end
 
-PACKET_CITY_SHORT_INFO=22; is-info,sc,lsend
+PACKET_CITY_SHORT_INFO=22; sc,lsend
   CITY id;key
   PLAYER owner;
   COORD x;
@@ -620,7 +623,7 @@ PACKET_PLAYER_REMOVE=38; sc,dsend,lsend
   PLAYER player_id;
 end
 
-PACKET_PLAYER_INFO=39; is-info,sc
+PACKET_PLAYER_INFO=39; sc
   PLAYER playerno; key 
   STRING name[MAX_LEN_NAME];
   STRING username[MAX_LEN_NAME];
@@ -699,7 +702,7 @@ PACKET_UNIT_REMOVE=48;sc,dsend,lsend
   UNIT unit_id;
 end
 
-PACKET_UNIT_INFO=49; is-info,sc,lsend
+PACKET_UNIT_INFO=49; sc,lsend
   UNIT id; key
   PLAYER owner;
   COORD x,y;
@@ -725,7 +728,7 @@ PACKET_UNIT_INFO=49; is-info,sc,lsend
   ACTIVITY orders_activities[MAX_LEN_ROUTE:orders_length];
 end
 
-PACKET_UNIT_SHORT_INFO=50; is-info,sc,lsend
+PACKET_UNIT_SHORT_INFO=50; sc,lsend
   UNIT id; key
   PLAYER owner;
   COORD x,y;
@@ -926,7 +929,7 @@ end
 # For telling clients information about other connections to server.
 # Clients may not use all info, but supply now to avoid unnecessary
 # protocol changes later.
-PACKET_CONN_INFO=86; is-info,sc,lsend
+PACKET_CONN_INFO=86; sc,lsend
   CONNECTION id; key
 
   # 0 means client should forget its
@@ -989,7 +992,7 @@ PACKET_SPACESHIP_PLACE=94;cs,dsend
   UINT8 num;
 end
 
-PACKET_SPACESHIP_INFO=95; is-info,sc,lsend
+PACKET_SPACESHIP_INFO=95; sc,lsend
   PLAYER player_num; key
   UINT8 sship_state;
   UINT8 structurals;