[Freeciv-Dev] Re: [PATCH] bugfix for wrapping problem

[Jason Dorje Short <jshort@xxxxxxxxxxxxx>]

"Ross W. Wetmore" wrote:
> 
> Are there any GUI experts around?
> 
> IMHO, having the same position drawn more than once is an error. If
> you stretch the canvas to exceed the base map size, I'm not sure
> that seeing only a single valued map isn't the desired behaviour. If
> there is a vote, I would vote for this, and not the other.
> 
> Resize, if ever implemented, may make the argument mute though.
> 
> I think the patch should NOT be applied until this is worked out.

As I said before, the current system is fundamentally flawed.  It just
happens to get things right in most cases because normalize_map_pos does
what it expects.  It is much more "correct" to do things this way and
avoids many spurious normalizations.

Note that under the current system the same position *is* drawn more
than once in the case of a wrap-around - but the two drawings of that
position will be drawn one on top of each other in the same spot.

There is one problem with the improved system, which is what I described
before.  Any time you're refreshing the canvas, you'll iterate over
absolute map positions and use the normalized map positions to determine
what to draw.  However, any time you have a specific update (for
instance, of a unit) you only have the normal map positions to work
with.  These need to be converted to absolute map positions for drawing
- which isn't done right now.  The result is that the normal map
positions are drawn to, which means (1) the update can happen in only
one place on the mapview and (2) it may be possible for the update to
happen outside the mapview, and not be shown at all.  This is not IMO a
reason to reject this bugfix; it just means that the other parts of the
code need to be fixed as well.  In this case, the normal map positions
need to be used to find all absolute map positions that need to be
updated.

I don't see how you can argue that the mapview shouldn't wrap around. 
Playing with the proper wrapping is much, much nicer than with improper
wrapping!  The situation is rare enough that there shouldn't be much
problem with having to update units multiple times, and again there will
be the satisfaction of doing things the "right" way.

One problem I have with the current code is that it is duplicated
line-for-line between many of the GUI interfaces.  Functions such as
get_canvas_xy (and there are a _lot_ of others) really should be in the
common client code (in client/).  However, right now the differences are
just enough that this isn't immediately possible.  Instead of fixing the
code in all different GUI's, I'd rather see code moved into the common
area.  Obviously a certain degree of separation from the graphical
routines is needed, which isn't present in much of the code right now
(mostly the isometric code).

jason