[Freeciv-Dev] (PR#40773) Bug: Changing Nation/Name of different Player

2009-05-12 Thread Madeline Book

http://bugs.freeciv.org/Ticket/Display.html?id=40773 >

Version 2 fixes the sdl client too.


---
こしょうも。
 client/gui-gtk-2.0/dialogs.c |   47 -
 client/gui-sdl/dialogs.c |   21 +-
 2 files changed, 51 insertions(+), 17 deletions(-)

diff --git a/client/gui-gtk-2.0/dialogs.c b/client/gui-gtk-2.0/dialogs.c
index a4efb61..0f78848 100644
--- a/client/gui-gtk-2.0/dialogs.c
+++ b/client/gui-gtk-2.0/dialogs.c
@@ -60,7 +60,7 @@
 
 /**/
 static GtkWidget  *races_shell;
-struct player *races_player;
+static char races_player_name[MAX_LEN_NAME];
 static GtkWidget  *races_nation_list[MAX_NUM_NATION_GROUPS + 1];
 static GtkWidget  *races_leader;
 static GList  *races_leader_list;
@@ -88,6 +88,7 @@ static void races_city_style_callback(GtkTreeSelection *select, gpointer data);
 static gboolean races_selection_func(GtkTreeSelection *select,
  GtkTreeModel *model, GtkTreePath *path,
  gboolean selected, gpointer data);
+static const struct player *get_races_player(void);
 
 static int selected_nation;
 static int selected_sex;
@@ -625,6 +626,7 @@ static GtkWidget* create_list_of_nations_in_group(struct nation_group* group,
   GtkTreeSelection *select;
   GtkCellRenderer *render;
   GtkTreeViewColumn *column;
+  const struct player *races_player = get_races_player();
 
   store = gtk_list_store_new(5, G_TYPE_INT, G_TYPE_BOOLEAN,
   GDK_TYPE_PIXBUF, G_TYPE_STRING, G_TYPE_STRING);
@@ -777,7 +779,11 @@ static void create_races_dialog(struct player *pplayer)
   GTK_RESPONSE_ACCEPT,
   NULL);
   races_shell = shell;
-  races_player = pplayer;
+  if (pplayer) {
+sz_strlcpy(races_player_name, player_name(pplayer));
+  } else {
+races_player_name[0] = '\0';
+  }
   setup_dialog(shell, toplevel);
 
   gtk_window_set_position(GTK_WINDOW(shell), GTK_WIN_POS_CENTER_ON_PARENT);
@@ -1233,13 +1239,22 @@ static void races_city_style_callback(GtkTreeSelection *select, gpointer data)
 **/
 static void races_response(GtkWidget *w, gint response, gpointer data)
 {
+  const struct player *races_player;
+  int plrno;
+
+  races_player = get_races_player();
+  if (!races_player) {
+popdown_races_dialog();
+return;
+  }
+  plrno = player_number(races_player);
+
   if (response == GTK_RESPONSE_ACCEPT) {
 const char *s;
 
 if (selected_nation == -1) {
-  dsend_packet_nation_select_req(&aconnection,
- races_player->player_no,
- -1, FALSE, "", 0);
+  dsend_packet_nation_select_req(&aconnection, plrno,
+ -1, FALSE, "", 0);
   popdown_races_dialog();
   return;
 }
@@ -1263,13 +1278,10 @@ static void races_response(GtkWidget *w, gint response, gpointer data)
   return;
 }
 
-dsend_packet_nation_select_req(&aconnection,
-   player_number(races_player), selected_nation,
-   selected_sex, s, selected_city_style);
+dsend_packet_nation_select_req(&aconnection, plrno, selected_nation,
+   selected_sex, s, selected_city_style);
   } else if (response == GTK_RESPONSE_NO) {
-dsend_packet_nation_select_req(&aconnection,
-   player_number(races_player),
-   -1, FALSE, "", 0);
+dsend_packet_nation_select_req(&aconnection, plrno, -1, FALSE, "", 0);
   } else if (response == GTK_RESPONSE_CANCEL) {
 /* Nothing - this allows the player to keep his currently selected
  * nation. */
@@ -1334,3 +1346,16 @@ void popdown_all_game_dialogs(void)
   gui_dialog_destroy_all();
 }
 
+/**
+  Helper function to work-around the fact that players may be renumbered
+  over the life-time of the nation selection dialog. It uses player names
+  ('races_player_name') to try to uniquely determine the player that the
+  user wants to modify.
+
+  NB: May return NULL.
+**/
+static const struct player *get_races_player(void)
+{
+  return find_player_by_name(races_player_name);
+}
+
diff --git a/client/gui-sdl/dialogs.c b/client/gui-sdl/dialogs.c
index ac9aab4..bb1164e 100644
--- a/client/gui-sdl/dialogs.c
+++ b/client/gui-sdl/dialogs.c
@@ -72,7 +72,7 @@
 
 #include "dialogs.h"
 
-struct player *races_player;
+static char races_player_name[MAX_LEN_NAME];
 
 extern bool is_unit_move_blocked;
 extern void popdown_diplomat_dialog(void);
@@ -2197,6 +2197,7 @@ static int races_dialog_ok_callback(struct widget *pStart_Button)
   if (Main.event.button.button == SDL_BUTTON_LEFT) {
 struct NAT *pSetup = (struct NAT *)(pNationDlg->pEndWidgetList->data.ptr);
 char *pStr = convert_to_chars(pSetup->pName_Edit->string16->text);
+const struct player *races_player;
   
 /* perform

Re: [Freeciv-Dev] (PR#40773) Bug: Changing Nation/Name of different Player

2009-05-12 Thread Karl-Ingo Friese

http://bugs.freeciv.org/Ticket/Display.html?id=40773 >

Hello Madeleine,

sounds great! Thanks for the fast patch.

Ingo

Madeline Book schrieb:
> http://bugs.freeciv.org/Ticket/Display.html?id=40773 >
> 
>> [...@gdv.uni-hannover.de - Mon May 11 14:07:45 2009]:
>>
>> Hi there,
>>
>> this is a bug we stumble upon every couple of weeks:
>> Sometimes before the game starts, when you choose nation
>> and name, the nation and name of _another_ player is
>> changed instead. Example; Peter, Paul and Mary are logged
>> in and Peter chooses to be called english ruler "Peter the
>> 1st", may result in Mary becoming english with leader name
>> "Peter the 1st", usualy spreading "some" confusion ;)
>>
>> I belive (without having looked at the code yet) that
>> the following might be the cause: We play with aifill
>> 17. Player A opens nation selection dialog. Player B
>> connects, removing an ai. Player A chooses his/her nation
>> with a now wrong player id.
> 
> Thank you for the detailed report; you are exactly right
> with your guess, giving me the enough correct information
> to easily find and fix the problem.
> 
> I have confirmed this bug in S2_1; trunk seems unaffected.
> The cause of this bug appears to be due to the nation
> picking dialog code assuming that players will never be
> renumbered between its invocation and closing (i.e. that
> player pointers are unique). This is not correct and it
> is made apparent when aifill is in effect (players may
> be renumbered when one is removed).
> 
> The attached patch is a somewhat hackish workaround that
> uses player names to try and uniquely identify which
> player should be affected by the nation picking dialog.
> Some cursory testing shows that it avoids the reported
> issue.
> 
> The solution is not terribly good since it just replaces
> one assumption (unique player pointers) with another
> (unique player names), but thankfully the design of
> player handling is already improved in trunk (i.e. no
> renumbering), so this temporary fix for the S2_1 branch
> will not need to be kept in the future.
> 
> 
>> PS: The guest account of rt web interface is not working ...
> 
> Use the GNA bug tracker from now on:
> https://gna.org/bugs/?group=freeciv
> 
> 
> ---
> 塩ください。
> 


-- 
-

Karl-Ingo Friese
Division of Computer Graphics - Welfen Laboratory
Institute of Man-Machine-Communication
Leibniz Universität Hannover



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


[Freeciv-Dev] (PR#40773) Bug: Changing Nation/Name of different Player

2009-05-11 Thread Madeline Book

http://bugs.freeciv.org/Ticket/Display.html?id=40773 >

> [...@gdv.uni-hannover.de - Mon May 11 14:07:45 2009]:
> 
> Hi there,
> 
> this is a bug we stumble upon every couple of weeks:
> Sometimes before the game starts, when you choose nation
> and name, the nation and name of _another_ player is
> changed instead. Example; Peter, Paul and Mary are logged
> in and Peter chooses to be called english ruler "Peter the
> 1st", may result in Mary becoming english with leader name
> "Peter the 1st", usualy spreading "some" confusion ;)
>
> I belive (without having looked at the code yet) that
> the following might be the cause: We play with aifill
> 17. Player A opens nation selection dialog. Player B
> connects, removing an ai. Player A chooses his/her nation
> with a now wrong player id.

Thank you for the detailed report; you are exactly right
with your guess, giving me the enough correct information
to easily find and fix the problem.

I have confirmed this bug in S2_1; trunk seems unaffected.
The cause of this bug appears to be due to the nation
picking dialog code assuming that players will never be
renumbered between its invocation and closing (i.e. that
player pointers are unique). This is not correct and it
is made apparent when aifill is in effect (players may
be renumbered when one is removed).

The attached patch is a somewhat hackish workaround that
uses player names to try and uniquely identify which
player should be affected by the nation picking dialog.
Some cursory testing shows that it avoids the reported
issue.

The solution is not terribly good since it just replaces
one assumption (unique player pointers) with another
(unique player names), but thankfully the design of
player handling is already improved in trunk (i.e. no
renumbering), so this temporary fix for the S2_1 branch
will not need to be kept in the future.


> PS: The guest account of rt web interface is not working ...

Use the GNA bug tracker from now on:
https://gna.org/bugs/?group=freeciv


---
塩ください。
 client/gui-gtk-2.0/dialogs.c |   47 -
 1 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/client/gui-gtk-2.0/dialogs.c b/client/gui-gtk-2.0/dialogs.c
index a4efb61..0f78848 100644
--- a/client/gui-gtk-2.0/dialogs.c
+++ b/client/gui-gtk-2.0/dialogs.c
@@ -60,7 +60,7 @@
 
 /**/
 static GtkWidget  *races_shell;
-struct player *races_player;
+static char races_player_name[MAX_LEN_NAME];
 static GtkWidget  *races_nation_list[MAX_NUM_NATION_GROUPS + 1];
 static GtkWidget  *races_leader;
 static GList  *races_leader_list;
@@ -88,6 +88,7 @@ static void races_city_style_callback(GtkTreeSelection *select, gpointer data);
 static gboolean races_selection_func(GtkTreeSelection *select,
  GtkTreeModel *model, GtkTreePath *path,
  gboolean selected, gpointer data);
+static const struct player *get_races_player(void);
 
 static int selected_nation;
 static int selected_sex;
@@ -625,6 +626,7 @@ static GtkWidget* create_list_of_nations_in_group(struct nation_group* group,
   GtkTreeSelection *select;
   GtkCellRenderer *render;
   GtkTreeViewColumn *column;
+  const struct player *races_player = get_races_player();
 
   store = gtk_list_store_new(5, G_TYPE_INT, G_TYPE_BOOLEAN,
   GDK_TYPE_PIXBUF, G_TYPE_STRING, G_TYPE_STRING);
@@ -777,7 +779,11 @@ static void create_races_dialog(struct player *pplayer)
   GTK_RESPONSE_ACCEPT,
   NULL);
   races_shell = shell;
-  races_player = pplayer;
+  if (pplayer) {
+sz_strlcpy(races_player_name, player_name(pplayer));
+  } else {
+races_player_name[0] = '\0';
+  }
   setup_dialog(shell, toplevel);
 
   gtk_window_set_position(GTK_WINDOW(shell), GTK_WIN_POS_CENTER_ON_PARENT);
@@ -1233,13 +1239,22 @@ static void races_city_style_callback(GtkTreeSelection *select, gpointer data)
 **/
 static void races_response(GtkWidget *w, gint response, gpointer data)
 {
+  const struct player *races_player;
+  int plrno;
+
+  races_player = get_races_player();
+  if (!races_player) {
+popdown_races_dialog();
+return;
+  }
+  plrno = player_number(races_player);
+
   if (response == GTK_RESPONSE_ACCEPT) {
 const char *s;
 
 if (selected_nation == -1) {
-  dsend_packet_nation_select_req(&aconnection,
- races_player->player_no,
- -1, FALSE, "", 0);
+  dsend_packet_nation_select_req(&aconnection, plrno,
+ -1, FALSE, "", 0);
   popdown_races_dialog();
   return;
 }
@@ -1263,13 +1278,10 @@ static void races_response(GtkWidget *w, gint response, gpointer data)
   return;
 }
 
-dsend_packet_nation_select_req(&aconnection,
-   player_number(races_player), selected_nation,
-   selected_sex, s, selected_city_style);
+dsend_

[Freeciv-Dev] (PR#40773) Bug: Changing Nation/Name of different Player

2009-05-11 Thread Karl-Ingo Friese

http://bugs.freeciv.org/Ticket/Display.html?id=40773 >

Hi there,

this is a bug we stumble upon every couple of weeks: Sometimes before 
the game starts, when you choose nation and name, the nation and name of 
_another_ player is changed instead. Example; Peter, Paul and Mary are 
logged in and Peter chooses to be called english ruler "Peter the 1st", 
may result in Mary becoming english with leader name "Peter the 1st", 
usualy spreading "some" confusion ;)

I belive (without having looked at the code yet) that the following 
might be the cause: We play with aifill 17. Player A opens nation 
selection dialog. Player B connects, removing an ai. Player A chooses 
his/her nation with a now wrong player id.

Just a guess,
Ingo

PS: The guest account of rt web interface is not working ...




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