Re: [Freeciv-Dev] (PR#40235) [Patch] Fix /take of currently observed player
http://bugs.freeciv.org/Ticket/Display.html?id=40235 > 2008/5/8 Madeline Book: > > stdinhand.c: In function `take_command': > stdinhand.c:2899: warning: 'was_observing_this' might be used > uninitialized in this function Indeed. After spending some time hunting problems this caused, I'd like to know why my gcc was silent. > There is also typo in a comment: "obserwing". - Fixed GTK-2 connection dialog seems to be broken in many ways. - 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 10:00:33.0 +0300 @@ -2900,7 +2900,8 @@ struct connection *pconn = caller; struct player *pplayer = NULL; bool res = FALSE; - + bool was_observing_this = FALSE; + / 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 observing 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 09:59:18.0 +0300 @@ -2896,7 +2896,8 @@ struct connection *pconn = caller; struct player *pplayer = NULL; bool res = FALSE; - + bool was_observing_this = FALSE; + / 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 observing 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
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
Re: [Freeciv-Dev] (PR#40235) [Patch] Fix /take of currently observed player
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
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