[Freeciv-Dev] Re: (PR#12787) YA icon update in city window

["Jason Dorje Short" <jdorje@xxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=12787 >

Christian Knoke wrote:
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=12787 >
> 
> 
> rc2 GTK2
> 
> Please load savegame and start.
> 
> In Ravenna, the marines unit is the single unit loaded on the transport
> present.
> 
> Activate marine from unit context menue. Still the loaded state is
> displayed. OK.
> 
> Now unload the marines. The transport still shows the '+' sign as part of its
> icon, but no units are loaded on it any more. Bug.

Oops.  This is a triple bug.

1.  There is code in the client that deduces when a transporter needs
repainting and repaints it.  This code looks correct but it's obsolete
and doesn't work because the transporter's occupancy (punit->occupy)
hasn't been updated at this point.

2.  When a unit's occupancy changes the server sends a new packet for
the unit.  However this doesn't trigger a redraw of the citydlg.

3.  Also changing a unit's transported_by field doesn't directly trigger
a redraw of the citydlg.  However because of #1 the citydlg gets redrawn
anyway (but only because the entire unit list is redrawn, not just the
changed unit).

This patch should fix all three.  There's also a patch to fix it in 2.0.

-jason

? diff
Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.518
diff -u -r1.518 packhand.c
--- client/packhand.c   22 May 2005 17:48:10 -0000      1.518
+++ client/packhand.c   29 May 2005 02:24:26 -0000
@@ -956,11 +956,8 @@
   }
 
   if (punit) {
-    const int old_transported_by = punit->transported_by;
-
     ret = TRUE;
     punit->activity_count = packet_unit->activity_count;
-    punit->occupy = packet_unit->occupy;
     if (punit->ai.control != packet_unit->ai.control) {
       punit->ai.control = packet_unit->ai.control;
       repaint_unit = TRUE;
@@ -973,6 +970,8 @@
 
     if (punit->activity != packet_unit->activity
        || punit->activity_target != packet_unit->activity_target
+       || punit->transported_by != packet_unit->transported_by
+       || punit->occupy != packet_unit->occupy
        || punit->has_orders != packet_unit->has_orders
        || punit->orders.repeat != packet_unit->orders.repeat
        || punit->orders.vigilant != packet_unit->orders.vigilant
@@ -1014,6 +1013,9 @@
       punit->activity = packet_unit->activity;
       punit->activity_target = packet_unit->activity_target;
 
+      punit->transported_by = packet_unit->transported_by;
+      punit->occupy = packet_unit->occupy;
+    
       punit->has_orders = packet_unit->has_orders;
       punit->orders.length = packet_unit->orders.length;
       punit->orders.index = packet_unit->orders.index;
@@ -1032,21 +1034,6 @@
       }
     } /*** End of Change in activity or activity's target. ***/
 
-    punit->transported_by = packet_unit->transported_by;
-    if (old_transported_by != packet_unit->transported_by) {
-      struct unit *ptrans;
-
-      if (old_transported_by != -1
-         && (ptrans = find_unit_by_id(old_transported_by))) {
-       refresh_unit_city_dialogs(ptrans);
-      }
-
-      if (packet_unit->transported_by != -1
-          && (ptrans = find_unit_by_id(packet_unit->transported_by))) {
-       refresh_unit_city_dialogs(ptrans);
-      }
-    }
-    
     /* These two lines force the menus to be updated as appropriate when
      * the focus unit changes. */
     if (punit == get_unit_in_focus()) {

Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.408.2.22
diff -u -r1.408.2.22 packhand.c
--- client/packhand.c   1 Apr 2005 00:05:55 -0000       1.408.2.22
+++ client/packhand.c   29 May 2005 02:20:04 -0000
@@ -987,7 +987,6 @@
   bool check_focus = FALSE;     /* conservative focus change */
   bool moved = FALSE;
   bool ret = FALSE;
-  int old_transported_by;
   
   punit = player_find_unit_by_id(get_player(packet_unit->owner),
                                 packet_unit->id);
@@ -995,7 +994,6 @@
   if (punit) {
     ret = TRUE;
     punit->activity_count = packet_unit->activity_count;
-    punit->occupy = packet_unit->occupy;
     if (punit->ai.control != packet_unit->ai.control) {
       punit->ai.control = packet_unit->ai.control;
       repaint_unit = TRUE;
@@ -1008,6 +1006,8 @@
 
     if (punit->activity != packet_unit->activity
        || punit->activity_target != packet_unit->activity_target
+       || punit->transported_by != packet_unit->transported_by
+       || punit->occupy != packet_unit->occupy
        || punit->has_orders != packet_unit->has_orders
        || punit->orders.repeat != packet_unit->orders.repeat
        || punit->orders.vigilant != packet_unit->orders.vigilant
@@ -1048,6 +1048,9 @@
       punit->activity = packet_unit->activity;
       punit->activity_target = packet_unit->activity_target;
 
+      punit->occupy = packet_unit->occupy;
+      punit->transported_by = packet_unit->transported_by;
+
       punit->has_orders = packet_unit->has_orders;
       punit->orders.length = packet_unit->orders.length;
       punit->orders.index = packet_unit->orders.index;
@@ -1066,22 +1069,6 @@
       }
     } /*** End of Change in activity or activity's target. ***/
 
-    old_transported_by = punit->transported_by;
-    punit->transported_by = packet_unit->transported_by;
-    if (old_transported_by != packet_unit->transported_by) {
-      struct unit *ptrans;
-
-      if (old_transported_by != -1
-         && (ptrans = find_unit_by_id(old_transported_by))) {
-       refresh_unit_city_dialogs(ptrans);
-      }
-
-      if (packet_unit->transported_by != -1
-          && (ptrans = find_unit_by_id(packet_unit->transported_by))) {
-       refresh_unit_city_dialogs(ptrans);
-      }
-    }
-    
     /* These two lines force the menus to be updated as appropriate when
      * the focus unit changes. */
     if (punit == get_unit_in_focus()) {