[Freeciv-Dev] (PR#40235) [Patch] Fix /take of currently observed player

2008-05-07 Thread Marko Lindqvist

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40235 

 Player changes when trying to /take player one is currently observing.

 Reproducing in S2_1

 1) /set aifill 5
 2) Select nation for some of the players (this is just to make them
visibly unique)
 3) Observe player with nation selected
 4) /take that player

 Notice how selected nation is no longer applied to player you now
control. Also, /list may show that you control somebody else.

 All this is because server first detaches us from previous player
(which we were observing) - player removed.


 Fix attached.


 - ML

diff -Nurd -X.diff_ignore freeciv/server/stdinhand.c freeciv/server/stdinhand.c
--- freeciv/server/stdinhand.c  2008-05-07 17:44:45.0 +0300
+++ freeciv/server/stdinhand.c  2008-05-08 03:22:24.0 +0300
@@ -2900,7 +2900,8 @@
   struct connection *pconn = caller;
   struct player *pplayer = NULL;
   bool res = FALSE;
-  
+  bool was_observing;
+
   / PART I: fill pconn and pplayer /
 
   sz_strlcpy(buf, str);
@@ -2961,9 +2962,6 @@
 goto end;
   }
 
-  if (!pplayer) {
-  }
-
   /* Make sure there is free player slot if there is need to
* create new player. This is necessary for previously
* detached connections only. Others can reuse the slot
@@ -2990,6 +2988,11 @@
 /* others are sent below */
   }
 
+  if (pconn-playing == pplayer) {
+/* Connection was obserwing the very player it now /take */
+was_observing = TRUE;
+  }
+
   /* if the player is controlled by another user,
* forcibly convert the user to an observer.
*/
@@ -3013,9 +3016,10 @@
 } conn_list_iterate_end;
   }
 
-  /* if the connection is already attached to a player,
-   * unattach and cleanup old player (rename, remove, etc) */
-  if (NULL != pconn-playing) {
+  /* if the connection is already attached to another player,
+   * unattach and cleanup old player (rename, remove, etc)
+   * We may have been observing the player we now want to take */
+  if (NULL != pconn-playing  !was_observing) {
 char name[MAX_LEN_NAME], username[MAX_LEN_NAME];
 
 if (pplayer) {
@@ -3039,11 +3043,13 @@
 }
   } players_iterate_end;
 
-  /* now attach to new player */
-  res = attach_connection_to_player(pconn, pplayer, FALSE);
+  if (!was_observing) {
+/* now attach to new player */
+res = attach_connection_to_player(pconn, pplayer, FALSE);
 
-  /* Check aifill even if attach failed. Maybe we already detached. */
-  aifill(game.info.aifill);
+/* Check aifill even if attach failed. Maybe we already detached. */
+aifill(game.info.aifill);
+  }
 
   if (res) {
 /* Successfully attached */
diff -Nurd -X.diff_ignore freeciv/server/stdinhand.c freeciv/server/stdinhand.c
--- freeciv/server/stdinhand.c  2008-05-08 00:24:57.0 +0300
+++ freeciv/server/stdinhand.c  2008-05-08 03:30:59.0 +0300
@@ -2896,7 +2896,8 @@
   struct connection *pconn = caller;
   struct player *pplayer = NULL;
   bool res = FALSE;
-  
+  bool was_observing;
+
   / PART I: fill pconn and pplayer /
 
   sz_strlcpy(buf, str);
@@ -2979,6 +2980,11 @@
 /* others are sent below */
   }
 
+  if (pconn-player == pplayer) {
+/* Connection was obserwing the very player it now /take */
+was_observing = TRUE;
+  }
+
   /* if we're taking another player with a user attached, 
* forcibly detach the user from the player. */
   if (pplayer) {
@@ -2998,9 +3004,10 @@
 } conn_list_iterate_end;
   }
 
-  /* if the connection is already attached to a player,
-   * unattach and cleanup old player (rename, remove, etc) */
-  if (pconn-player) {
+  /* if the connection is already attached to another player,
+   * unattach and cleanup old player (rename, remove, etc)
+   * We may have been observing the player we now want to take */
+  if (pconn-player  !was_observing) {
 char name[MAX_LEN_NAME], username[MAX_LEN_NAME];
 
 if (pplayer) {
@@ -3026,7 +3033,11 @@
 
   /* now attach to new player */
   pconn-observer = FALSE; /* do this before attach! */
-  res = attach_connection_to_player(pconn, pplayer);
+
+  if (!was_observing) {
+/* now attach to new player */
+res = attach_connection_to_player(pconn, pplayer);
+  }
 
   if (res) {
 /* Successfully attached */
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#40235) [Patch] Fix /take of currently observed player

2008-05-07 Thread Marko Lindqvist

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40235 

2008/5/8 Marko Lindqvist:

   Player changes when trying to /take player one is currently observing.

 - Fixed crash when global observer does /take -


 - ML

diff -Nurd -X.diff_ignore freeciv/server/stdinhand.c freeciv/server/stdinhand.c
--- freeciv/server/stdinhand.c  2008-05-07 17:44:45.0 +0300
+++ freeciv/server/stdinhand.c  2008-05-08 04:20:18.0 +0300
@@ -2900,7 +2900,8 @@
   struct connection *pconn = caller;
   struct player *pplayer = NULL;
   bool res = FALSE;
-  
+  bool was_observing_this;
+
   / PART I: fill pconn and pplayer /
 
   sz_strlcpy(buf, str);
@@ -2961,9 +2962,6 @@
 goto end;
   }
 
-  if (!pplayer) {
-  }
-
   /* Make sure there is free player slot if there is need to
* create new player. This is necessary for previously
* detached connections only. Others can reuse the slot
@@ -2990,6 +2988,11 @@
 /* others are sent below */
   }
 
+  if (pconn-playing  pconn-playing == pplayer) {
+/* Connection was obserwing the very player it now /take */
+was_observing_this = TRUE;
+  }
+
   /* if the player is controlled by another user,
* forcibly convert the user to an observer.
*/
@@ -3013,9 +3016,10 @@
 } conn_list_iterate_end;
   }
 
-  /* if the connection is already attached to a player,
-   * unattach and cleanup old player (rename, remove, etc) */
-  if (NULL != pconn-playing) {
+  /* if the connection is already attached to another player,
+   * unattach and cleanup old player (rename, remove, etc)
+   * We may have been observing the player we now want to take */
+  if (NULL != pconn-playing  !was_observing_this) {
 char name[MAX_LEN_NAME], username[MAX_LEN_NAME];
 
 if (pplayer) {
@@ -3039,11 +3043,13 @@
 }
   } players_iterate_end;
 
-  /* now attach to new player */
-  res = attach_connection_to_player(pconn, pplayer, FALSE);
+  if (!was_observing_this || !pconn-playing) {
+/* Now attach to new player */
+res = attach_connection_to_player(pconn, pplayer, FALSE);
 
-  /* Check aifill even if attach failed. Maybe we already detached. */
-  aifill(game.info.aifill);
+/* Check aifill even if attach failed. Maybe we already detached. */
+aifill(game.info.aifill);
+  }
 
   if (res) {
 /* Successfully attached */
diff -Nurd -X.diff_ignore freeciv/server/stdinhand.c freeciv/server/stdinhand.c
--- freeciv/server/stdinhand.c  2008-05-08 00:24:57.0 +0300
+++ freeciv/server/stdinhand.c  2008-05-08 04:04:24.0 +0300
@@ -2896,7 +2896,8 @@
   struct connection *pconn = caller;
   struct player *pplayer = NULL;
   bool res = FALSE;
-  
+  bool was_observing_this;
+
   / PART I: fill pconn and pplayer /
 
   sz_strlcpy(buf, str);
@@ -2979,6 +2980,11 @@
 /* others are sent below */
   }
 
+  if (pconn-player  pconn-player == pplayer) {
+/* Connection was obserwing the very player it now /take */
+was_observing_this = TRUE;
+  }
+
   /* if we're taking another player with a user attached, 
* forcibly detach the user from the player. */
   if (pplayer) {
@@ -2998,9 +3004,10 @@
 } conn_list_iterate_end;
   }
 
-  /* if the connection is already attached to a player,
-   * unattach and cleanup old player (rename, remove, etc) */
-  if (pconn-player) {
+  /* if the connection is already attached to another player,
+   * unattach and cleanup old player (rename, remove, etc)
+   * We may have been observing the player we now want to take */
+  if (pconn-player  !was_observing_this) {
 char name[MAX_LEN_NAME], username[MAX_LEN_NAME];
 
 if (pplayer) {
@@ -3026,7 +3033,11 @@
 
   /* now attach to new player */
   pconn-observer = FALSE; /* do this before attach! */
-  res = attach_connection_to_player(pconn, pplayer);
+
+  if (!was_observing_this || !pplayer) {
+/* now attach to new player */
+res = attach_connection_to_player(pconn, pplayer);
+  }
 
   if (res) {
 /* Successfully attached */
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#40235) [Patch] Fix /take of currently observed player

2008-05-07 Thread Madeline Book

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40235 

 [EMAIL PROTECTED] - Thu May 08 00:52:08 2008]:
 
  Player changes when trying to /take player one is
  currently observing.
 
  Reproducing in S2_1
 
  1) /set aifill 5
  2) Select nation for some of the players (this is just to make them
 visibly unique)
  3) Observe player with nation selected
  4) /take that player
 
  Notice how selected nation is no longer applied to player you now
 control. Also, /list may show that you control somebody else.
 
  All this is because server first detaches us from previous player
 (which we were observing) - player removed.

I have confirmed this for S2_1.

When compiling the patch I get

stdinhand.c: In function `take_command':
stdinhand.c:2899: warning: 'was_observing_this' might be used 
uninitialized in this function

There is also typo in a comment: obserwing.


--
好きなわけないぞ

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev