[Freeciv-Dev] Re: map tiles in city dialog drawn in wrong order (PR#1044)

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

Daniel Sjölie wrote:
> 
> On 2001-11-01 11:40:28, Raimar Falke wrote:
> > On Thu, Nov 01, 2001 at 02:22:17AM -0800, Andreas Beckmann wrote:
> > > From: Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>
> > > Date: Thursday, November 01, 2001 10:47 AM
> > >
> > > >How can this be reproduced? Can you give a screenshot?
> > >
> > > http://www.abeckmann.de/citydlg-error.png
> > > (black has been replaced by white during screen capture - its not a 
> > > freeciv
> > > bug)
> >
> > Indeed. Daniel?
> >
> > This all shows that the nobody has tested the isometric view.
> 
> Pieter J. Kersten reported this bug last week... I did test isometric
> view but I didn't notice this... Probably in part because this
> presumably worked before september 30th and since nothing related to
> this changed in the city dialog after that I probably didn't check
> everything... And since the city gfx is smaller in the civ2 isometric
> tileset I didn't notice anything there...
> 
> I'm not really into how those iterate functions are used all over so I
> was kinda hoping that someone else would have something to say on the
> matter... I will take a look though, if noone else comes forward...

The relevant code is in city_dialog_update_map_iso() in
client/gui-gtk/citydlg.c.  It was changed as part of the new city dialog
to use the city_map_checked_iterate.  A comment in the code says

/* This macro happens to iterate correct to draw the top tiles first,
   so getting the overlap right.
   Furthermore, we don't have to redraw fog on the part of a fogged tile
   that overlaps another fogged tile, as on the main map, as no tiles in
   the city radius can be fogged. */

but is wrong.  The tiles are supposed to be drawn from top-left to
bottom-right, which means looping over X, then Y.  The
city_map_checked_iterate macro was changed around the beginning of
September (!) to become a wrapper for city_map_iterate_outwards, so the
ordering is no longer correct.

Changing the loop to use an explicit ordering makes things work.  Patch
attached.

jason

Index: client/gui-gtk/citydlg.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/citydlg.c,v
retrieving revision 1.100
diff -u -r1.100 citydlg.c
--- client/gui-gtk/citydlg.c    2001/10/30 11:29:33     1.100
+++ client/gui-gtk/citydlg.c    2001/11/01 12:22:48
@@ -1753,6 +1753,7 @@
 static void city_dialog_update_map_iso(struct city_dialog *pdialog)
 {
   struct city *pcity = pdialog->pcity;
+  int city_x, city_y;
 
   gdk_gc_set_foreground(fill_bg_gc, colors_standard[COLOR_STD_BLACK]);
 
@@ -1760,19 +1761,22 @@
   gdk_draw_rectangle(pdialog->map_canvas_store, fill_bg_gc, TRUE,
                     0, 0, canvas_width, canvas_height);
 
-  /* This macro happens to iterate correct to draw the top tiles first,
-     so getting the overlap right.
-     Furthermore, we don't have to redraw fog on the part of a fogged tile
-     that overlaps another fogged tile, as on the main map, as no tiles in
-     the city radius can be fogged. */
-  city_map_checked_iterate(pcity->x, pcity->y, x, y, map_x, map_y) {
+  /* We have to draw the tiles in a particular order, so its best
+     to avoid using any iterator macro. */
+  for (city_x = 0; city_x<CITY_MAP_SIZE; city_x++)
+    for (city_y = 0; city_y<CITY_MAP_SIZE; city_y++) {
+      int map_x, map_y;
+      if (is_valid_city_coords(city_x, city_y) &&
+         city_map_to_map(&map_x, &map_y, pcity, city_x, city_y)) {
     if (tile_is_known(map_x, map_y)) {
       int canvas_x, canvas_y;
-      city_get_canvas_xy(x, y, &canvas_x, &canvas_y);
+      city_get_canvas_xy(city_x, city_y, &canvas_x, &canvas_y);
       put_one_tile_full(pdialog->map_canvas_store, map_x, map_y,
                        canvas_x, canvas_y, 1);
     }
-  } city_map_checked_iterate_end;
+      }
+    }
+
   /* We have to put the output afterwards or it will be covered. */
   city_map_checked_iterate(pcity->x, pcity->y, x, y, map_x, map_y) {
     if (tile_is_known(map_x, map_y)) {