[Freeciv-Dev] Re: (PR#13217) "trying to put a non-boolean" with civ1 ruleset

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

Jason Short wrote:
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=13217 >
> 
> When a client connects to a running-game server in the civ1 ruleset, I get:
> 
> 1: Trying to put a non-boolean: 255
> 1: last message repeated 2 times
> 1: last message repeated 1 time (total 3 repeats)
> 1: Trying to put a non-boolean: 2
> 1: Trying to put a non-boolean: 2
> 1: Trying to put a non-boolean: 8
> 1: Trying to put a non-boolean: 255

Actually it has nothing to do with the civ1 ruleset.  It is a bug in
send_player_info_c that doesn't initialize player packets properly when
sending to unattached players.  If there's just one player then garbage
will be sent (causing the error messages).  If there's more than one
connection then the data that was sent to one player is also sent to the
unattached connection (a mild form of cheating).

Start a server with the attached rc file.  Connect with an arbitrary
username and you can reproduce the bug.

This patch fixes it.  I also moved several fields into the player-common
packaging.

Although this bug exists in 2.0, it never comes up since player info
isn't sent to unattached connections.  I don't know why.  However the
attached patch fixes this for 2.0 anyway.  I haven't decided if this
should be committed or not.

-jason

create Cazfi
set aifill 9
#set endyear 5000

start

Index: server/plrhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/plrhand.c,v
retrieving revision 1.383
diff -u -r1.383 plrhand.c
--- server/plrhand.c    1 Jun 2005 01:02:34 -0000       1.383
+++ server/plrhand.c    7 Jun 2005 00:06:07 -0000
@@ -1420,7 +1420,9 @@
        } else if (pconn->player) {
          /* Players (including regular observers) */
          package_player_info(pplayer, &info, pconn->player, INFO_MINIMUM);
-       } 
+       } else {
+         package_player_info(pplayer, &info, NULL, INFO_MINIMUM);
+       }
 
         send_packet_player_info(pconn, &info);
       } conn_list_iterate_end;
@@ -1473,6 +1475,9 @@
     packet->small_wonders[i] = plr->small_wonders[i];
   }
   packet->science_cost = plr->ai.science_cost;
+
+  packet->gold = plr->economic.gold;
+  packet->government = plr->government;
 }
 
 /**************************************************************************
@@ -1481,8 +1486,8 @@
   to plr, we send a little to players we are in contact with and almost
   nothing to everyone else.
 
- Note: if reciever is NULL and info < INFO_EMBASSY the info related to the
-       receiving player are not set correctly.
+  Receiver may be NULL in which cases dummy values are sent for some
+  fields.
 **************************************************************************/
 static void package_player_info(struct player *plr,
                                 struct packet_player_info *packet,
@@ -1492,14 +1497,13 @@
   int i;
   enum plr_info_level info_level;
 
-  info_level = player_info_level(plr, receiver);
-  if (info_level < min_info_level) {
+  if (receiver) {
+    info_level = player_info_level(plr, receiver);
+    info_level = MAX(min_info_level, info_level);
+  } else {
     info_level = min_info_level;
   }
 
-  packet->gold            = plr->economic.gold;
-  packet->government      = plr->government;
-
   /* Only send score if we have contact */
   if (info_level >= INFO_MEETING) {
     packet->score = plr->score.game;
@@ -1510,7 +1514,8 @@
   /* Send diplomatic status of the player to everyone they are in
    * contact with. */
   if (info_level >= INFO_EMBASSY
-      || receiver->diplstates[plr->player_no].contact_turns_left > 0) {
+      || (receiver
+         && receiver->diplstates[plr->player_no].contact_turns_left > 0)) {
     packet->target_government = plr->target_government;
     memset(&packet->embassy, 0, sizeof(packet->embassy));
     players_iterate(pother) {
@@ -1558,7 +1563,8 @@
 
   /* Make absolutely sure - in case you lose your embassy! */
   if (info_level >= INFO_EMBASSY 
-      || pplayer_get_diplstate(plr, receiver)->type == DS_TEAM) {
+      || (receiver
+         && pplayer_get_diplstate(plr, receiver)->type == DS_TEAM)) {
     packet->bulbs_last_turn = plr->bulbs_last_turn;
   } else {
     packet->bulbs_last_turn = 0;
@@ -1599,7 +1605,8 @@
   packet->inventions[A_NONE] = plr->research->inventions[A_NONE].state + '0';
 
   if (info_level >= INFO_FULL
-      || plr->diplstates[receiver->player_no].type == DS_TEAM) {
+      || (receiver
+         && plr->diplstates[receiver->player_no].type == DS_TEAM)) {
     packet->tech_goal       = plr->research->tech_goal;
   } else {
     packet->tech_goal       = A_UNSET;

Index: server/plrhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/plrhand.c,v
retrieving revision 1.330.2.21
diff -u -r1.330.2.21 plrhand.c
--- server/plrhand.c    27 May 2005 08:32:10 -0000      1.330.2.21
+++ server/plrhand.c    7 Jun 2005 00:14:02 -0000
@@ -1435,7 +1435,9 @@
        } else if (pconn->player) {
          /* Players (including regular observers) */
          package_player_info(pplayer, &info, pconn->player, INFO_MINIMUM);
-       } 
+       } else {
+         package_player_info(pplayer, &info, NULL, INFO_MINIMUM);
+       }
 
         send_packet_player_info(pconn, &info);
       } conn_list_iterate_end;
@@ -1490,8 +1492,8 @@
   to plr, we send a little to players we are in contact with and almost
   nothing to everyone else.
 
- Note: if reciever is NULL and info < INFO_EMBASSY the info related to the
-       receiving player are not set correctly.
+  Receiver may be NULL in which cases dummy values are sent for some
+  fields.
 **************************************************************************/
 static void package_player_info(struct player *plr,
                                 struct packet_player_info *packet,
@@ -1501,8 +1503,12 @@
   int i;
   enum plr_info_level info_level;
 
-  info_level = player_info_level(plr, receiver);
-  if (info_level < min_info_level) {
+  if (receiver) {
+    info_level = player_info_level(plr, receiver);
+    if (info_level < min_info_level) {
+      info_level = min_info_level;
+    }
+  } else {
     info_level = min_info_level;
   }
 
@@ -1512,7 +1518,8 @@
   /* Send diplomatic status of the player to everyone they are in
    * contact with. */
   if (info_level >= INFO_EMBASSY
-      || receiver->diplstates[plr->player_no].contact_turns_left > 0) {
+      || (receiver
+         && receiver->diplstates[plr->player_no].contact_turns_left > 0)) {
     packet->target_government = plr->target_government;
     packet->embassy = plr->embassy;
     packet->gives_shared_vision = plr->gives_shared_vision;
@@ -1557,7 +1564,8 @@
 
   /* Make absolutely sure - in case you lose your embassy! */
   if (info_level >= INFO_EMBASSY 
-      || pplayer_get_diplstate(plr, receiver)->type == DS_TEAM) {
+      || (receiver
+         && pplayer_get_diplstate(plr, receiver)->type == DS_TEAM)) {
     packet->bulbs_last_turn = plr->research.bulbs_last_turn;
   } else {
     packet->bulbs_last_turn = 0;
@@ -1598,7 +1606,8 @@
   packet->inventions[A_NONE] = plr->research.inventions[A_NONE].state + '0';
 
   if (info_level >= INFO_FULL
-      || plr->diplstates[receiver->player_no].type == DS_TEAM) {
+      || (receiver
+         && plr->diplstates[receiver->player_no].type == DS_TEAM)) {
     packet->tech_goal       = plr->ai.tech_goal;
   } else {
     packet->tech_goal       = A_UNSET;