Re: [Freeciv-Dev] (PR#39620) [Bug] gtk2: Cannot sell buildings

2008-01-30 Thread Marko Lindqvist

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

On 30/01/2008, William Allen Simpson  wrote:
>
> Marko, the requisite time has passed -- are you committing this, or am I?

 I can commit tonight - but go ahead if you have opportunity earlier.


 - ML



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


Re: [Freeciv-Dev] (PR#39620) [Bug] gtk2: Cannot sell buildings

2008-01-29 Thread William Allen Simpson

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

Marko, the requisite time has passed -- are you committing this, or am I?



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


Re: [Freeciv-Dev] (PR#39620) [Bug] gtk2: Cannot sell buildings

2008-01-28 Thread William Allen Simpson

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

Marko Lindqvist wrote:
>  That's why I don't like GTK's generic APIs. You don't have type checking with
> them.
> 
Yeah, and I'm not a gui expert enough to catch them.  Here's a slightly better
version of the patch, telling the interface a pointer is passed.  Presumably,
the code actually worked because ints and pointers are the same size.

Index: client/gui-gtk-2.0/citydlg.c
===
--- client/gui-gtk-2.0/citydlg.c(revision 14338)
+++ client/gui-gtk-2.0/citydlg.c(working copy)
@@ -253,7 +253,7 @@
 static void buy_callback(GtkWidget * w, gpointer data);
 static void change_production_callback(GtkWidget* w, struct city_dialog*);
 
-static void sell_callback(Impr_type_id id, gpointer data);
+static void sell_callback(struct impr_type *pimprove, gpointer data);
 static void sell_callback_response(GtkWidget *w, gint response, gpointer data);
 
 static void impr_callback(GtkTreeView *view, GtkTreePath *path,
@@ -759,7 +759,7 @@
   gtk_box_pack_start(GTK_BOX(top), vbox, TRUE, TRUE, 0);
 
   /* improvements */
-  store = gtk_list_store_new(4, G_TYPE_INT, GDK_TYPE_PIXBUF, G_TYPE_STRING,
+  store = gtk_list_store_new(4, G_TYPE_POINTER, GDK_TYPE_PIXBUF, G_TYPE_STRING,
 G_TYPE_INT);
 
   view = gtk_tree_view_new_with_model(GTK_TREE_MODEL(store));
@@ -1618,7 +1618,7 @@
 
 gtk_list_store_append(store, &it);
 gtk_list_store_set(store, &it,
-  0, target.value,
+  0, target.value.building,
   1, sprite_get_pixbuf(sprite),
2, items[item].descr,
3, upkeep,
@@ -2484,11 +2484,11 @@
 /
 ...
 */
-static void sell_callback(Impr_type_id id, gpointer data)
+static void sell_callback(struct impr_type *pimprove, gpointer data)
 {
-  struct impr_type *pimprove = improvement_by_number(id);
-  struct city_dialog *pdialog = (struct city_dialog *) data;
   GtkWidget *shl;
+  struct city_dialog *pdialog = (struct city_dialog *) data;
+  pdialog->sell_id = improvement_number(pimprove);
   
   if (!can_client_issue_orders()) {
 return;
@@ -2502,8 +2502,6 @@
 return;
   }
 
-  pdialog->sell_id = id;
-
   shl = gtk_message_dialog_new(NULL,
 GTK_DIALOG_DESTROY_WITH_PARENT,
 GTK_MESSAGE_QUESTION,
@@ -2547,7 +2545,7 @@
   GtkTreeModel *model;
   GtkTreeIter it;
   GdkModifierType mask;
-  int id;
+  struct impr_type *pimprove;
 
   model = gtk_tree_view_get_model(view);
 
@@ -2555,13 +2553,12 @@
 return;
   }
 
-  gtk_tree_model_get(model, &it, 0, &id, -1);
+  gtk_tree_model_get(model, &it, 0, &pimprove, -1);
   gdk_window_get_pointer(NULL, NULL, NULL, &mask);
 
   if (!(mask & GDK_CONTROL_MASK)) {
-sell_callback(id, data);
+sell_callback(pimprove, data);
   } else {
-struct impr_type *pimprove = improvement_by_number(id);
 if (is_great_wonder(pimprove)) {
   popup_help_dialog_typed(improvement_name_translation(pimprove), 
HELP_WONDER);
 } else {
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#39620) [Bug] gtk2: Cannot sell buildings

2008-01-28 Thread Marko Lindqvist

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

On 28/01/2008, William Allen Simpson  wrote:
> >
> The generic interface somehow didn't cause a compile
> error.

 That's why I don't like GTK's generic APIs. You don't have type checking with
them.

>  Your original report didn't include a savegame,

 I don't attach savegames to 100% reproducible reports.

> The value passed is supposed to be a pointer to the improvement, and the
> mistake is the (ancient code) treating it as an int (now Impr_type_id).

 This definitely seems better way to go.

 While debugging, I just approached problem from another direction
(where we had illegal Impr_type_id)


 - ML



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


Re: [Freeciv-Dev] (PR#39620) [Bug] gtk2: Cannot sell buildings

2008-01-28 Thread William Allen Simpson

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

Marko Lindqvist wrote:
>  Includes some generic cleanup-style changes I made while debugging
> problem. Actual fix is citydlg.c:1621. It was storing improvement
> pointer instead of number.
> 
>  Original report referring to TRUNK only is older than S2_2 branch.
> 
Thanks, good catch!  The generic interface somehow didn't cause a compile
error.  Tough to find.  Your original report didn't include a savegame,
and I'm glad you finally found the problem.

However, I strongly disagree with your solution.  I've been trying to
remove all cid_* calls, and concomitant GINT_TO_POINTER() scattered all
over the code.  We've had an awful lot of reports about this behaving
badly across compiler versions!

The value passed is supposed to be a pointer to the improvement, and the
mistake is the (ancient code) treating it as an int (now Impr_type_id).

Here's my (lightly tested) alternative:

Index: client/gui-gtk-2.0/citydlg.c
===
--- client/gui-gtk-2.0/citydlg.c(revision 14338)
+++ client/gui-gtk-2.0/citydlg.c(working copy)
@@ -253,7 +253,7 @@
 static void buy_callback(GtkWidget * w, gpointer data);
 static void change_production_callback(GtkWidget* w, struct city_dialog*);
 
-static void sell_callback(Impr_type_id id, gpointer data);
+static void sell_callback(struct impr_type *pimprove, gpointer data);
 static void sell_callback_response(GtkWidget *w, gint response, gpointer data);
 
 static void impr_callback(GtkTreeView *view, GtkTreePath *path,
@@ -1618,7 +1618,7 @@
 
 gtk_list_store_append(store, &it);
 gtk_list_store_set(store, &it,
-  0, target.value,
+  0, target.value.building,
   1, sprite_get_pixbuf(sprite),
2, items[item].descr,
3, upkeep,
@@ -2484,11 +2484,11 @@
 /
 ...
 */
-static void sell_callback(Impr_type_id id, gpointer data)
+static void sell_callback(struct impr_type *pimprove, gpointer data)
 {
-  struct impr_type *pimprove = improvement_by_number(id);
-  struct city_dialog *pdialog = (struct city_dialog *) data;
   GtkWidget *shl;
+  struct city_dialog *pdialog = (struct city_dialog *) data;
+  pdialog->sell_id = improvement_number(pimprove);
   
   if (!can_client_issue_orders()) {
 return;
@@ -2502,8 +2502,6 @@
 return;
   }
 
-  pdialog->sell_id = id;
-
   shl = gtk_message_dialog_new(NULL,
 GTK_DIALOG_DESTROY_WITH_PARENT,
 GTK_MESSAGE_QUESTION,
@@ -2547,7 +2545,7 @@
   GtkTreeModel *model;
   GtkTreeIter it;
   GdkModifierType mask;
-  int id;
+  struct impr_type *pimprove;
 
   model = gtk_tree_view_get_model(view);
 
@@ -2555,13 +2553,12 @@
 return;
   }
 
-  gtk_tree_model_get(model, &it, 0, &id, -1);
+  gtk_tree_model_get(model, &it, 0, &pimprove, -1);
   gdk_window_get_pointer(NULL, NULL, NULL, &mask);
 
   if (!(mask & GDK_CONTROL_MASK)) {
-sell_callback(id, data);
+sell_callback(pimprove, data);
   } else {
-struct impr_type *pimprove = improvement_by_number(id);
 if (is_great_wonder(pimprove)) {
   popup_help_dialog_typed(improvement_name_translation(pimprove), 
HELP_WONDER);
 } else {
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev