Complete.Org: Mailing Lists: Archives: freeciv-dev: October 2001:
[Freeciv-Dev] Re: GUI PATCH: refresh_tile_mapcanvas cleanup
Home

[Freeciv-Dev] Re: GUI PATCH: refresh_tile_mapcanvas cleanup

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: jdorje@xxxxxxxxxxxx
Cc: freeciv-dev <freeciv-dev@xxxxxxxxxxx>
Subject: [Freeciv-Dev] Re: GUI PATCH: refresh_tile_mapcanvas cleanup
From: "Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>
Date: Fri, 12 Oct 2001 21:29:37 -0400

You may have introduced a few bugs, see explanation below :-).

At 12:00 PM 01/10/10 -0400, Jason Dorje Short wrote:
>The function refresh_tile_mapcanvas() is absolutely identical in every
>GUI.
[...]
>In summary, all of the GUI authors will need to sign off on such a patch
>for it to go through.
>
>jason? rc
>Index: client/climisc.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/client/climisc.c,v
>retrieving revision 1.62
>diff -u -r1.62 climisc.c
>--- client/climisc.c   2001/10/04 19:36:54     1.62
>+++ client/climisc.c   2001/10/10 15:46:22
>@@ -828,3 +828,19 @@
>   }
>   return cids_used;
> }
>+
>+/**************************************************************************
>+ GUI Independent (with new access functions).  However, this should go
>+ in client/mapview.c instead of client/climisc.c.
>+**************************************************************************/
>+void refresh_tile_mapcanvas(int x, int y, int write_to_screen)
>+{
>+  assert(is_real_tile(x, y));
>+  if (!normalize_map_pos(&x, &y))
>+    return;
>+
>+  if (tile_visible_mapcanvas(x, y)) {
>+    update_map_canvas(x,y, 1, 1, write_to_screen);
>+  }
>+  overview_update_tile(x, y);
>+}

I don't think this is a correct interpretation of the original code.

If you remove the assert(is_real_tile() and if condition around
normalize_map_pos() then I am much happier with it.

But I don't understand the GUI, so my belief that the code checking
in tile_visible_mapcanvas() and overview_update_tile() that expects
to draw unreal tiles as black tiles in the GUI window may be in 
error.

But I would get someone to confirm this before you really change the 
existing code flow by stopping the GUI from ever drawing unreal tiles
where it expects (and maybe needs) to.

[...]
>Index: client/gui-gtk/mapview.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/mapview.c,v
>retrieving revision 1.103
>diff -u -r1.103 mapview.c
>--- client/gui-gtk/mapview.c   2001/10/05 09:47:41     1.103
>+++ client/gui-gtk/mapview.c   2001/10/10 15:46:23
>@@ -640,21 +640,6 @@
> /**************************************************************************
> ...
> **************************************************************************/
>-void refresh_tile_mapcanvas(int x, int y, int write_to_screen)
>-{
>-  x = map_adjust_x(x);
>-  y = map_adjust_y(y);
>-
>-  if (tile_visible_mapcanvas(x, y)) {
>-    update_map_canvas(x, y, 1, 1, write_to_screen);
>-  }
>-  overview_update_tile(x, y);
>-}

Cheers,
RossW
=====



[Prev in Thread] Current Thread [Next in Thread]