Complete.Org: Mailing Lists: Archives: freeciv-dev: August 2002:
[Freeciv-Dev] Re: normalize_map_pos and invalid map positions

[Freeciv-Dev] Re: normalize_map_pos and invalid map positions

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
Cc: freeciv-dev@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: normalize_map_pos and invalid map positions
From: Jason Short <jdorje@xxxxxxxxxxxxxxxxxxxxx>
Date: Thu, 22 Aug 2002 11:09:28 -0500

Ross W. Wetmore wrote:
At 01:15 PM 02/08/15 +0000, Per I. Mathisen wrote:

On Tue, 13 Aug 2002, Ross W. Wetmore wrote:

Normalize_map_pos() is the swiss army knife for coordinate testing.

It returns false if the coordinate is unreal - generally terminal error.

It returns true and a valid normalized coordinate pair otherwise. This
may involve normalizing the coordinates in the process but that is always
possible if they are real.

This should be used at all portal points, i.e. places where you should
check for unreal coordinates. This includes source creation, network
input points or any place where one might want to check for corruption
of any sort, or even just a change in the definition of "normalized".

In some cases it is better to use unnormalized or differently normalized
coordinates for a given operation. In this case one would normalize before
handing off such coordinates to more picky code outside of this module,
or do it on input to any module that might receive such coordinates.

This should always be the case. It doesn't hurt to fix things up and
continue in any situation, and a few judiciously placed checks like
this will be useful to catch the unreal cases which are terminal.

So even the big reason for normalizing everywhere is not really that
sensible with truly generalized topologies.

So what you are suggesting is that normalize_map_pos() is used at the
lowest level where coordinates are generated, changed or received from the
client, and that the the higher level functions receive properly
normalized map positions which are checked with is_normal_map_pos().


I'm juggling lower/higher but I think I know what you mean.


Very nice explanation, Ross.  I only disagree with you on one point:

Any real coordinates are fine. They are always fine as they can always
be normalized on the fly with comnplete safety in all cases. They are
never wrong, and can never be made more wrong. Any other interpretation
is pure religious hogwash used to justify an unnecessary constraint.

This is not true.  In the vnotify_conn_ex case, it is certainly not true.

Here's the explanation again, from scratch. Suppose we are dealing with a topology where (-1,-1) is real (but not normal).

Then, first of all, we certainly can't call normalize_map_pos() in vnotify_conn_ex on the coordinates without first checking for this special case condition; that would result in "invalid" (special-case) coordinates being translated into "valid" (normal) ones.

Secondly, note that (-1,-1) is a real, non-normal position. Now what happens if the calling code happens to pass in real, non-normal positions to vnotify_conn_ex? Usually this would be a soft error; we could just call normalize_map_pos and be done with it. But in this case there is a special problem introduced by the special-case coordinates. If a caller can pass in real, non-normal positions then eventually it may pass in (-1,-1) - and when it does this the function will erronously treat it as the special-case condition. The special-case will be passed on to the client where it will cause some slightly weird behavior: (I think) the message window will know about the event, but double-clicking it will have no effect. The player will probably think nothing of it, and the bug will never be reported.

There is only one way to detect this bug. We can make sure that no real, non-normal coordinates *except* (-1,-1) are passed in to vnotify_conn_ex. Although this isn't strictly an error, it is the only indication that an error may be happening. It is easy to have a single assertion to check this case.

This is what the current vnotify_conn_ex does.


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