[Freeciv-Dev] Re: check_map_pos update

[Jason Dorje Short <vze2zq63@xxxxxxxxxxx>]

"Ross W. Wetmore" wrote:
> 
> The code comment says it stops the unit from being drawn on top of
> city tiles when it is moving.
> 
> Try ending your turn on top of a city and see what it does.
> 
> Or look in the unit drawing code to see if it has a city check that
> it didn't used to, or if there is a different draw path when you
> are moving or in focus that pops you to the top of the heap.
> 
> Is there a corresponding check for this magic coordinate later
> in the code flow, or a unit position reset?
> 
> Maybe the code below gives clues.
> 
> Is there a bug report or CVS annotation?
> 
> Not being a gooey person, I can only make educated guesses at why
> one does such icky things :-).

This "focus hack" code, and most code around it, dates from before control.c
was imported into CVS.  There is no useful log entry.

All this is in do_move_unit in client/control.c:

(x, y) take the old coordinates of the unit.  Then the coordinates of the
unit are changed to be something different (presumably so that the unit is
not drawn during that time).  The unit is also removed from the unit list
for the tile it is on.

20 lines later, the unit's coordinates are set to the new coordinates.

During those 20 lines, refresh_tile_mapcanvas is called.  Interestingly,
this is a (slightly) newer piece of code, dating from September 1999. 
Although the unit has theoretically been unlinked from the list up above,
somewhere in refresh_tile_mapcanvas(x,y,...) the unit is found and its
coordinates are accessed.  Even if the focus hack is necessary, this is
pretty certainly a bug.

The access occurs deep within refresh_tile_mapcanvas. 
find_visible_unit(ptile) is called with "ptile" being the original tile at
(x, y).  Here it calls the following line of code:

  if (punit_focus && map_get_tile(punit_focus->x,punit_focus->y) == ptile)

which is the problem.  If the hack is to be kept this line should be changed
to

  if (punit_focus && 
      is_normal_map_pos(punit_focus->x, punit_focus->y) &&
      map_get_tile(punit_focus->x, punit_focus->y) == ptile)

to avoid the problem.


But the question remains: is the focus hack necessary?  There is other code
in find_visible_unit that checks to see if a city is present, and although I
don't see how it could affect this situation I think more study should be
done.

As I said before, I see no ill effects of simply removing the line.  Perhaps
we could just comment it out for a while and see if anyone complained?

jason