[Freeciv-Dev] Re: Remove map_adjust_[xy] invocations from server (PR#1003)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Tue, Oct 23, 2001 at 11:38:12PM +0100, Gaute B Strokkenes wrote:
> On Tue, 23 Oct 2001, hawk@xxxxxxxxxxxxxxxxxxxxxxx wrote:
> > 
> > We have to go through the code and remove is_real_tile,
> > normalize_map_pos and is_normal_map_pos. Everything is fine if the
> > positions these methods work on are used in the function as an
> > arguement to some access method. What happens if this isn't the
> > case?
> 
> I don't see what you're getting at.  We will have to go through and
> replace these things anyway.  Also, failure to run a given coordinate
> pair through CHECK_POS() or whatever is not a bug in itself, it is
> just a missed opportunity to find down a bug.  It's not worthwhile
> being paranoid about this; in practise a bug will manifest itself soon
> enough.

Ack.

> >> It would be a good idea to add those checks in places where it is
> >> obvious that a check in map_inx() is not enough, such as
> >> same_pos().
> > 
> > This is my first suggestion.
> 
> Then we are in violent agreement...

Fine.

> >> The idea is to disable it by default, and let people turn it back
> >> on when they make changes to iteration macros etc. that need
> >> testing of this sort.  Since that is a distinct minority of the
> >> time, we do not need to care that much about how much time it
> >> takes.  I don't think it is going to take that much more time than
> >> the current approach (always check, always fix).
> > 
> > I would vote for the ignore the case.
> 
> Sorry, what do you mean?

We remove (allmost) all is_real_tile, normalize_map_pos and
is_normal_map_pos and don't replace them with anything other. We just
ignore same_pos and co.

> >  - used for an outgoing packet.
> > 
> > For the last case (and also on general) I would wish a more strict
> > testing of map positions which came from packets (server rejects,
> > client aborts).
> 
> I'll leave that to someone who knows the packet protocol better.
> 
> A priori the most sensible thing to do is to always emit normalised
> coordinates, and reject coordinates which are not.  

> But there is also backwards compatibility to consider.

This isn't a problem:
  /*
   * Discard invalid packet. Replace this with is_normal_map_pos if
   * capstr is set to "1.12.0".
   */

New mandatory capstr means no backwards compatibility.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "On the eigth day, God started debugging"