Re: [Freeciv-Dev] (PR#39609) BUG: Escape from goto/connect/etc clears update_unit_info_label()

2007-09-24 Thread William Allen Simpson 

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

Committed trunk revision 13639.

Index: client/control.c
===
--- client/control.c(revision 13638)
+++ client/control.c(working copy)
@@ -59,7 +59,6 @@
 static struct unit_list *previous_focus;
 
 /* These should be set via set_hover_state() */
-struct unit_list *hover_units;
 enum cursor_hover_state hover_state = HOVER_NONE;
 struct tile *hover_tile = NULL;
 enum unit_activity connect_activity;
@@ -97,7 +96,6 @@
 
   caravan_arrival_queue = genlist_new();
   diplomat_arrival_queue = genlist_new();
-  hover_units = unit_list_new();
   pfocus_units = unit_list_new();
   previous_focus = unit_list_new();
   for (i = 0; i < MAX_NUM_BATTLEGROUPS; i++) {
@@ -114,7 +112,6 @@
 
   genlist_free(caravan_arrival_queue);
   genlist_free(diplomat_arrival_queue);
-  unit_list_free(hover_units);
   unit_list_free(pfocus_units);
   unit_list_free(previous_focus);
   for (i = 0; i < MAX_NUM_BATTLEGROUPS; i++) {
@@ -130,7 +127,10 @@
   int i;
 
   unit_list_unlink(get_units_in_focus(), punit);
-  unit_list_unlink(hover_units, punit);
+  if (get_num_units_in_focus() < 1) {
+set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
+  }
+  update_unit_info_label(get_units_in_focus());
   unit_list_unlink(previous_focus, punit);
   for (i = 0; i < MAX_NUM_BATTLEGROUPS; i++) {
 unit_list_unlink(battlegroups[i], punit);
@@ -182,12 +182,6 @@
   assert((punits && unit_list_size(punits) > 0) || state == HOVER_NONE);
   assert(state == HOVER_CONNECT || activity == ACTIVITY_LAST);
   assert(state == HOVER_GOTO || order == ORDER_LAST);
-  unit_list_unlink_all(hover_units);
-  if (punits) {
-unit_list_iterate(punits, punit) {
-  unit_list_append(hover_units, punit);
-} unit_list_iterate_end;
-  }
   hover_state = state;
   connect_activity = activity;
   goto_last_order = order;
@@ -1893,12 +1887,12 @@
 void do_map_click(struct tile *ptile, enum quickselect_type qtype)
 {
   struct city *pcity = tile_get_city(ptile);
-  struct unit_list *punits = hover_units;
+  struct unit_list *punits = get_units_in_focus();
   bool maybe_goto = FALSE;
   bool possible = FALSE;
   struct tile *offender = NULL;
 
-  if (unit_list_size(punits) > 0 && hover_state != HOVER_NONE) {
+  if (hover_state != HOVER_NONE) {
 switch (hover_state) {
 case HOVER_NONE:
   die("well; shouldn't get here :)");
@@ -2112,7 +2106,7 @@
 }
 
 /**
- Finish the goto mode and let the units stored in hover_units move
+ Finish the goto mode and let the units stored in goto_map_list move
  to a given location.
 **/
 void do_unit_goto(struct tile *ptile)
@@ -2182,27 +2176,24 @@
 **/
 void key_cancel_action(void)
 {
-  bool popped = FALSE;
-
   cancel_tile_hiliting();
 
   switch (hover_state) {
   case HOVER_GOTO:
   case HOVER_PATROL:
   case HOVER_CONNECT:
-popped = goto_pop_waypoint();
+if (!goto_pop_waypoint()) {
+  set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
+  update_unit_info_label(get_units_in_focus());
+
+  keyboardless_goto_button_down = FALSE;
+  keyboardless_goto_active = FALSE;
+  keyboardless_goto_start_tile = NULL;
+}
+break;
   default:
 break;
   };
-
-  if (hover_state != HOVER_NONE && !popped) {
-set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
-update_unit_info_label(hover_units);
-
-keyboardless_goto_button_down = FALSE;
-keyboardless_goto_active = FALSE;
-keyboardless_goto_start_tile = NULL;
-  }
 }
 
 /**
Index: client/gui-gtk-2.0/mapctrl.c
===
--- client/gui-gtk-2.0/mapctrl.c(revision 13638)
+++ client/gui-gtk-2.0/mapctrl.c(working copy)
@@ -395,7 +395,6 @@
 **/
 gboolean leave_mapcanvas(GtkWidget *widget, GdkEventCrossing *event)
 {
-  struct unit_list *active_units = get_units_in_focus();
   int canvas_x, canvas_y;
 
   if (gtk_notebook_get_current_page(GTK_NOTEBOOK(top_notebook))
@@ -417,7 +416,7 @@
 update_mouse_cursor(CURSOR_DEFAULT);
   }
 
-  update_unit_info_label(active_units);
+  update_unit_info_label(get_units_in_focus());
   return TRUE;
 }
 
Index: client/text.c
===
--- client/text.c   (revision 13638)
+++ client/text.c   (working copy)
@@ -699,7 +699,7 @@
* itself. */
 
   /* Line 1. Goto or activity text. */
-  if (count > 0 && unit_list_size(hover_units) > 0) {
+  if (count > 0 && hover_state != HOVER_NONE) {
 int min, max;
 
 if (!goto_get_turns(&min, &max)) {
Index: cli

Re: [Freeciv-Dev] (PR#39609) BUG: Escape from goto/connect/etc clears update_unit_info_label()

2007-09-22 Thread William Allen Simpson 

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

William Allen Simpson wrote:
> Does anybody remember?  Otherwise, I'm getting rid of it.
> 
Going, going, gone.

Index: client/control.c
===
--- client/control.c(revision 13619)
+++ client/control.c(working copy)
@@ -58,7 +58,6 @@
 static struct unit_list *previous_focus;
 
 /* These should be set via set_hover_state() */
-struct unit_list *hover_units;
 enum cursor_hover_state hover_state = HOVER_NONE;
 struct tile *hover_tile = NULL;
 enum cursor_action_state action_state = CURSOR_ACTION_DEFAULT;
@@ -97,7 +96,6 @@
 
   caravan_arrival_queue = genlist_new();
   diplomat_arrival_queue = genlist_new();
-  hover_units = unit_list_new();
   pfocus_units = unit_list_new();
   previous_focus = unit_list_new();
   for (i = 0; i < MAX_NUM_BATTLEGROUPS; i++) {
@@ -114,7 +112,6 @@
 
   genlist_free(caravan_arrival_queue);
   genlist_free(diplomat_arrival_queue);
-  unit_list_free(hover_units);
   unit_list_free(pfocus_units);
   unit_list_free(previous_focus);
   for (i = 0; i < MAX_NUM_BATTLEGROUPS; i++) {
@@ -130,7 +127,10 @@
   int i;
 
   unit_list_unlink(get_units_in_focus(), punit);
-  unit_list_unlink(hover_units, punit);
+  if (get_num_units_in_focus() < 1) {
+set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
+  }
+  update_unit_info_label(get_units_in_focus());
   unit_list_unlink(previous_focus, punit);
   for (i = 0; i < MAX_NUM_BATTLEGROUPS; i++) {
 unit_list_unlink(battlegroups[i], punit);
@@ -182,12 +182,6 @@
   assert((punits && unit_list_size(punits) > 0) || state == HOVER_NONE);
   assert(state == HOVER_CONNECT || activity == ACTIVITY_LAST);
   assert(state == HOVER_GOTO || order == ORDER_LAST);
-  unit_list_unlink_all(hover_units);
-  if (punits) {
-unit_list_iterate(punits, punit) {
-  unit_list_append(hover_units, punit);
-} unit_list_iterate_end;
-  }
   hover_state = state;
   connect_activity = activity;
   goto_last_order = order;
@@ -1824,12 +1818,12 @@
 void do_map_click(struct tile *ptile, enum quickselect_type qtype)
 {
   struct city *pcity = tile_get_city(ptile);
-  struct unit_list *punits = hover_units;
+  struct unit_list *punits = get_units_in_focus();
   bool maybe_goto = FALSE;
   bool possible = FALSE;
   struct tile *offender = NULL;
 
-  if (unit_list_size(punits) > 0 && hover_state != HOVER_NONE) {
+  if (hover_state != HOVER_NONE) {
 switch (hover_state) {
 case HOVER_NONE:
   die("well; shouldn't get here :)");
@@ -2043,7 +2037,7 @@
 }
 
 /**
- Finish the goto mode and let the units stored in hover_units move
+ Finish the goto mode and let the units stored in goto_map_list move
  to a given location.
 **/
 void do_unit_goto(struct tile *ptile)
@@ -2113,27 +2107,24 @@
 **/
 void key_cancel_action(void)
 {
-  bool popped = FALSE;
-
   cancel_tile_hiliting();
 
   switch (hover_state) {
   case HOVER_GOTO:
   case HOVER_PATROL:
   case HOVER_CONNECT:
-popped = goto_pop_waypoint();
+if (!goto_pop_waypoint()) {
+  set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
+  update_unit_info_label(get_units_in_focus());
+
+  keyboardless_goto_button_down = FALSE;
+  keyboardless_goto_active = FALSE;
+  keyboardless_goto_start_tile = NULL;
+}
+break;
   default:
 break;
   };
-
-  if (hover_state != HOVER_NONE && !popped) {
-set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
-update_unit_info_label(hover_units);
-
-keyboardless_goto_button_down = FALSE;
-keyboardless_goto_active = FALSE;
-keyboardless_goto_start_tile = NULL;
-  }
 }
 
 /**
Index: client/gui-gtk-2.0/mapctrl.c
===
--- client/gui-gtk-2.0/mapctrl.c(revision 13619)
+++ client/gui-gtk-2.0/mapctrl.c(working copy)
@@ -411,10 +411,8 @@
 handle_mouse_cursor(canvas_pos_to_tile(canvas_x, canvas_y));
 /* update_unit_info_label is handled inside handle_mouse_cursor. */
   } else {
-struct unit_list *active_units = get_units_in_focus();
-
 action_state = CURSOR_ACTION_DEFAULT;
-update_unit_info_label(active_units);
+update_unit_info_label(get_units_in_focus());
   }
   return TRUE;
 }
Index: client/text.c
===
--- client/text.c   (revision 13619)
+++ client/text.c   (working copy)
@@ -682,7 +682,7 @@
* GUI widgets may be confused and try to resize themselves. */
 
   /* Line 1. Goto or activity text. */
-  if (count > 0 && unit_list_size(hover_units) > 0) {
+  if (count > 0 && hover_state != HOVER_NONE) {
 int min, max;

[Freeciv-Dev] (PR#39609) BUG: Escape from goto/connect/etc clears update_unit_info_label()

2007-08-24 Thread William Allen Simpson 

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

As I was testing several related bugs (PR#29982, PR#39339, PR#39430):
   "goto_get_turns: Assertion unit_is_in_focus(punit) failed"

I found that hitting escape caused the list of focused units to disappear.
Clicking anywhere on the map restores the list.

The simple explanation, in client/control.c key_cancel_action(),
 set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
 update_unit_info_label(hover_units);

set_hover_state() has just set hover_units to NULL.  This should actually be
get_units_in_focus(), as they are still in focus.

===

Further checking shows that this is one of only two places that hover_units
is actually used.

In fact, every other call to update_unit_info_label() is passing
get_units_in_focus() or the equivalent pfocus_units.  This is the only
exception -- and it's wrong!

(Ignoring the problem that gui-mui still only handles a single unit.)

Obviously, the assert() requires all goto units are in pfocus_units.

There's no explanation for why hover_units exist.  It is always either NULL
or identical to pfocus_units.  There's no code that changes hover_units to
ever make it any different from pfocus_units.

Does anybody remember?  Otherwise, I'm getting rid of it.



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