[Freeciv-Dev] (PR#8863) suckiness of "citymap" images on mapview

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8863 >

If you use the 't' button within a city radius, you'll toggle the 
display of the citymap on that city.  You can then recenter anywhere to 
remove this display.

The problem is this is very buggy.  Just about anything will overwrite 
the citymap overlay.  If you have a unit within the city range this 
makes it almost useless.  This is the most glaring problem remaining 
with the mapview (now that city text drawing is fixed).

The most obvious question is whether this is a good system at all.  I 
talked a bit with Vasco and we came up with some alternative ideas.  But 
they'll all take a fair amount of work, so I decided to try to fix the 
current system first.

This patch changes the overlays in two ways:

- They are drawn as part of update_map_canvas rather than put to the 
screen directly.  This means updates will always work correctly.

- They are persistent.  The color is tied to the city and will remain 
until the user removes it.

This does have some drawbacks.  Because the overlays don't go away 
"automatically" when you recenter the mapview the user has to remove 
them himself.  Eventually I think the overlays should be toggleable via 
an option in the "View" menu.  But also in addition to that it is 
possible just to 't'oggle through the colors and then make the city 
color go away.

Another drawback is that an inefficient loop is used in 
update_map_canvas().  I don't see any way around this.

Only the gtk2 client is supported so far.  Unless someone objects to the 
design I will update the other clients.

-----

Other ideas:

- Tile production should be toggleable in the view menu.  If enabled, 
the production for all tiles is shown.

- The user should be able to change city workers on the mapview.  By 
ctrl-clicking (or something) he places or removes workers.  Of course 
when placing a worker you have to figure out what city to give it to. 
And when removing a worker you have to decide what specialist to make it 
(though this is less of a problem).

- You should be able to toggle a citymap overlay for settlers too.  This 
will probably actually be quite easy.

- You wouldn't want to play with the overlay on.  It would be nice to 
have a static form that is usable for the full game (also controlled by 
an option, of course).  This would probably have to take the form of a 
border outline.

jason

? diff
? ferries
? flags
? data/flags
Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.121
diff -u -r1.121 mapview_common.c
--- client/mapview_common.c     24 May 2004 21:04:53 -0000      1.121
+++ client/mapview_common.c     29 May 2004 02:30:19 -0000
@@ -750,6 +750,20 @@
   }
 }
 
+void toggle_city_color(struct city *pcity)
+{
+  int canvas_x, canvas_y;
+  int width = get_citydlg_canvas_width();
+  int height = get_citydlg_canvas_height();
+
+  pcity->client.color = (pcity->client.color + 1) % 4;
+
+  map_to_canvas_pos(&canvas_x, &canvas_y, pcity->x, pcity->y);
+  update_map_canvas(canvas_x - (width - NORMAL_TILE_WIDTH) / 2,
+                   canvas_y - (height - NORMAL_TILE_HEIGHT) / 2,
+                   width, height);
+}
+
 /****************************************************************************
   Return the vertices of the given edge of the tile.  This will return
   FALSE if the edge doesn't exist (or if it does exist but is part of an
@@ -1372,6 +1386,41 @@
     } gui_rect_iterate_end;
   }
 
+  gui_rect_iterate(gui_x0, gui_y0, width, height, map_x, map_y, draw) {
+    if (((draw & D_B) || (draw & D_M))
+       && normalize_map_pos(&map_x, &map_y)) {
+      struct city *pcity = map_get_tile(map_x, map_y)->worked;
+      int city_x, city_y, canvas_x2, canvas_y2;
+
+      if (!pcity) {
+       city_map_iterate_outwards(dx, dy) {
+         int x, y;
+         
+         if (base_city_map_to_map(&x, &y, map_x, map_y, dx, dy)
+             && (pcity = map_get_city(x, y))
+             && pcity->client.color != -1) {
+           /* Use this city. */
+           break;
+         }
+       } city_map_iterate_outwards_end;
+      }
+
+      if (pcity && pcity->client.color != -1
+         && map_to_city_map(&city_x, &city_y, pcity, map_x, map_y)
+         && map_to_canvas_pos(&canvas_x2, &canvas_y2, map_x, map_y)) {
+       enum city_tile_type worker = get_worker_city(pcity, city_x, city_y);
+
+       put_city_worker(mapview_canvas.store,
+                       pcity->client.color + 1, worker,
+                       canvas_x2, canvas_y2);
+       if (worker == C_TILE_WORKER) {
+         put_city_tile_output(pcity, city_x, city_y,
+                              mapview_canvas.store, canvas_x2, canvas_y2);
+       }
+      }
+    }
+  } gui_rect_iterate_end;
+
   show_city_descriptions(canvas_x, canvas_y, width, height);
 
   if (!full) {
Index: client/mapview_common.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.h,v
retrieving revision 1.65
diff -u -r1.65 mapview_common.h
--- client/mapview_common.h     22 May 2004 18:12:21 -0000      1.65
+++ client/mapview_common.h     29 May 2004 02:30:20 -0000
@@ -257,6 +257,7 @@
 void put_unit_city_overlays(struct unit *punit,
                            struct canvas *pcanvas,
                            int canvas_x, int canvas_y);
+void toggle_city_color(struct city *pcity);
 void put_red_frame_tile(struct canvas *pcanvas,
                        int canvas_x, int canvas_y);
 
Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.372
diff -u -r1.372 packhand.c
--- client/packhand.c   28 May 2004 19:13:07 -0000      1.372
+++ client/packhand.c   29 May 2004 02:30:20 -0000
@@ -396,7 +396,8 @@
 
   if(!pcity) {
     city_is_new = TRUE;
-    pcity=fc_malloc(sizeof(struct city));
+    pcity = create_city_virtual(get_player(packet->owner),
+                               packet->x, packet->y, packet->name);
     pcity->id=packet->id;
     idex_register_city(pcity);
     update_descriptions = TRUE;
@@ -596,7 +597,6 @@
   }
 
   if (city_workers_display==pcity)  {
-    put_city_workers(pcity, -1);
     city_workers_display=NULL;
   }
 
Index: client/gui-gtk-2.0/mapctrl.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/mapctrl.c,v
retrieving revision 1.39
diff -u -r1.39 mapctrl.c
--- client/gui-gtk-2.0/mapctrl.c        23 Apr 2004 22:58:05 -0000      1.39
+++ client/gui-gtk-2.0/mapctrl.c        29 May 2004 02:30:20 -0000
@@ -391,8 +391,7 @@
   }
 
   /* Shade tiles on usage */
-  city_workers_color = (city_workers_color % 3) + 1;
-  put_city_workers(pcity, city_workers_color);
+  toggle_city_color(pcity);
 }
 
 
Index: client/gui-gtk-2.0/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/mapview.c,v
retrieving revision 1.126
diff -u -r1.126 mapview.c
--- client/gui-gtk-2.0/mapview.c        24 May 2004 22:48:08 -0000      1.126
+++ client/gui-gtk-2.0/mapview.c        29 May 2004 02:30:20 -0000
@@ -849,67 +849,32 @@
   }
 }
 
-/**************************************************************************
-...
-**************************************************************************/
-void put_city_workers(struct city *pcity, int color)
-{
-  int canvas_x, canvas_y;
-  static struct city *last_pcity=NULL;
-  struct canvas store = {.type = CANVAS_PIXMAP, .v.pixmap = 
map_canvas->window};
-
-  if (color==-1) {
-    if (pcity!=last_pcity)
-      city_workers_color = city_workers_color%3 + 1;
-    color=city_workers_color;
+void put_city_worker(struct canvas *pcanvas,
+                    enum color_std color, enum city_tile_type worker,
+                    int canvas_x, int canvas_y)
+{
+  if (worker == C_TILE_EMPTY) {
+    gdk_gc_set_stipple(fill_tile_gc, gray25);
+  } else if (worker == C_TILE_WORKER) {
+    gdk_gc_set_stipple(fill_tile_gc, gray50);
+  } else {
+    return;
   }
-  gdk_gc_set_foreground(fill_tile_gc, colors_standard[color]);
-
-  city_map_checked_iterate(pcity->x, pcity->y, i, j, x, y) {
-    enum city_tile_type worked = get_worker_city(pcity, i, j);
 
-    if (!map_to_canvas_pos(&canvas_x, &canvas_y, x, y)) {
-      continue;
-    }
+  gdk_gc_set_foreground(fill_tile_gc, colors_standard[color]);
 
-    /* stipple the area */
-    if (!is_city_center(i, j)) {
-      if (worked == C_TILE_EMPTY) {
-       gdk_gc_set_stipple(fill_tile_gc, gray25);
-      } else if (worked == C_TILE_WORKER) {
-       gdk_gc_set_stipple(fill_tile_gc, gray50);
-      } else
-       continue;
-
-      if (is_isometric) {
-       gdk_gc_set_clip_origin(fill_tile_gc, canvas_x, canvas_y);
-       gdk_gc_set_clip_mask(fill_tile_gc, sprites.black_tile->mask);
-       gdk_draw_drawable(map_canvas->window, fill_tile_gc, map_canvas_store,
-                         canvas_x, canvas_y,
-                         canvas_x, canvas_y,
-                         NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
-       gdk_draw_rectangle(map_canvas->window, fill_tile_gc, TRUE,
-                          canvas_x, canvas_y,
-                          NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
-       gdk_gc_set_clip_mask(fill_tile_gc, NULL);
-      } else {
-       gdk_draw_drawable(map_canvas->window, civ_gc, map_canvas_store,
-                         canvas_x, canvas_y,
-                         canvas_x, canvas_y,
-                         NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
-       gdk_draw_rectangle(map_canvas->window, fill_tile_gc, TRUE,
-                          canvas_x, canvas_y,
-                          NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
-      }
-    }
+  if (is_isometric) {
+    gdk_gc_set_clip_origin(fill_tile_gc, canvas_x, canvas_y);
+    gdk_gc_set_clip_mask(fill_tile_gc, sprites.black_tile->mask);
+  }
 
-    /* draw tile output */
-    if (worked == C_TILE_WORKER) {
-      put_city_tile_output(pcity, i, j, &store, canvas_x, canvas_y);
-    }
-  } city_map_checked_iterate_end;
+  gdk_draw_rectangle(pcanvas->v.pixmap, fill_tile_gc, TRUE,
+                    canvas_x, canvas_y,
+                    NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
 
-  last_pcity=pcity;
+  if (is_isometric) {
+    gdk_gc_set_clip_mask(fill_tile_gc, NULL);
+  }
 }
 
 /**************************************************************************
Index: client/include/mapview_g.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/include/mapview_g.h,v
retrieving revision 1.52
diff -u -r1.52 mapview_g.h
--- client/include/mapview_g.h  17 May 2004 07:16:43 -0000      1.52
+++ client/include/mapview_g.h  29 May 2004 02:30:20 -0000
@@ -71,7 +71,9 @@
 void update_map_canvas_scrollbars_size(void);
 
 void put_cross_overlay_tile(int x,int y);
-void put_city_workers(struct city *pcity, int color);
+void put_city_worker(struct canvas *pcanvas,
+                    enum color_std color, enum city_tile_type worker,
+                    int canvas_x, int canvas_y);
 
 void draw_selection_rectangle(int canvas_x, int canvas_y, int w, int h);
 void tileset_changed(void);
Index: common/city.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.c,v
retrieving revision 1.212
diff -u -r1.212 city.c
--- common/city.c       27 May 2004 22:14:18 -0000      1.212
+++ common/city.c       29 May 2004 02:30:21 -0000
@@ -2639,6 +2639,10 @@
   pcity->tax_bonus = 100;
   pcity->science_bonus = 100;
 
+  pcity->client.occupied = FALSE;
+  pcity->client.happy = pcity->client.unhappy = FALSE;
+  pcity->client.color = -1;
+
   unit_list_init(&pcity->units_supported);
   pcity->debug = FALSE;
 
Index: common/city.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.h,v
retrieving revision 1.144
diff -u -r1.144 city.h
--- common/city.h       27 May 2004 22:14:18 -0000      1.144
+++ common/city.h       29 May 2004 02:30:21 -0000
@@ -268,6 +268,7 @@
     /* Only used at the client (the serer is omniscient). */
     bool occupied;
     bool happy, unhappy;
+    int color;
   } client;
 
   int steal;                 /* diplomats steal once; for spies, gets harder */