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

[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]
To: freeciv-dev@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: normalize_map_pos and invalid map positions
From: Jason Short <jdorje@xxxxxxxxxxxxxxxxxxxxx>
Date: Fri, 09 Aug 2002 17:24:40 -0500

Mike Kaufman wrote:
On Fri, Aug 09, 2002 at 06:04:36PM +0000, Per I. Mathisen wrote:

Ok, this is probably a stupid question, but anyway.

I take it the point of normalize_map_pos is to ensure the given map
positions don't point to an illegal point on the map that would case an
error.

However, sometimes, as with goto_dest_?, I believe the correct thing to do
is to make the positions illegal, so that they don't get used and if they
get used, an error is generated so that the problem can be fixed.

So what would be an illegal map position? (-1, -1)?

Passing around illegal map positions is generally a bad idea. Back before normalize_map_pos, there was a void_tile which was the tile all invalid coordinates pointed to. Although this may sound like a good idea, it (supposedly) lead to all sorts of hard-to-track errors downstream. These days illegal map positions are usually detected and dealt with on creation - for instance in the iterator macros they are just skipped over. But there are some places where this is difficult.

Using (-1,-1) as an "illegal" map position is already done: for instance, vnotify_conn_ex() takes (-1,-1) as the position indicating that a given event has no valid position. This can be made to work so long as (-1,-1) is not a *normal* map position. Soon it will be a *real* map position, but as long as all valid coordinates used are *normal* we can tell (-1,-1) from a valid position.

Elsewhere in the code, the goto_dest_x and goto_dest_y fields of a unit are initialized to 0, and only later are they correctly set. If you were to play on a map where (0,0) is a worthwhile position, I don't know if there would be problems.

Both of these situations are both ugly and dangerous. A better solution would be desirable.

urk, -1,-1 is (will be) a valid position. (at least that's my
understanding of the whole thing...)
The solution should be to add an illegal position bit?

example: a unit at (0,0) is ordered to move "northwest" by:
a) the server (by proxy of the ai)
b) a remote client.

What should ideally happen in a general topological framework?

Assuming we're talking about a torus map (wraps in both directions), the coordinates of the destination should have been normalized at startup. So for instance on a 100x80 map, a unit at (0,0) who moves northwest will move to position (99,79). Using (-1,-1) as a special-case would be workable (but again a hack).

The problem comes in when we consider worst-case scenarios. Suppose a particular piece of code does not correctly normalize the coordinates before passing them in. Usually we could just normalize them and be done with it, but in this case that's dangerous. If (-1,-1) is passed in we cannot tell if we should normalize or treat it as a special case. And what if (-2,-2) is passed in? Ross disagrees with me here, but I think that (1) such a situation should be avoided and (2) if we do code like this, liberal assertions are needed to make sure things are working properly (otherwise they could fail, and it would be very hard to track).

Jason, Ross, could we have a consensus on this particular point, devoid of
any other considerations? If other considerations, what do they need to be?

The ideal solution IMO would be to have a real special-case indicator.

One way would be to use a struct map_pos *, which could then be NULL to indicate no coordinates (or invalid coordinates). This would be ideal for vnotify_conn_ex, but less helpful in other places since it would involve a lot of allocating and deallocating. Raimar also doesn't like it because pointer handling is slow. Unfortunately, this method will not help when we're sending information over the network.

Another would be to introduce a new validity flag. In the vnotify_conn_ex case, this would be an extra parameter indicating whether there were coordinates associated with the event. This data could then be passed over the network (without breaking network compatability).

The biggest problem with any system is implementing it. The vnotify_conn_ex case is pretty isolated, but other code that has similar problems (for instance the goto_dest_[xy] mentioned above) are spread throughout the code. In many cases it's possible to ignore the problem, but this has the potential to cause weird, hard-to-trace, usually minor bugs.

Ross?  Raimar?

jason



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