[Freeciv-Dev] Re: (PR#7280) Tile pointer in unit struct

["rwetmore@xxxxxxxxxxxx" <rwetmore@xxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=7280 >


Raimar Falke wrote:

 > <URL: http://rt.freeciv.org/Ticket/Display.html?id=7280 >
 >
 > On Tue, Jan 20, 2004 at 09:24:10AM -0800, Arnstein Lindgard wrote:
 >
 >> <URL: http://rt.freeciv.org/Ticket/Display.html?id=7280 >
 >>
 >> I added a tile pointer to the unit struct, and applied some grep
 >> dexterity.
 >>
 >> + Old syntax:
 >>
 >>  "map_get_tile(punit->x, punit->y)"
 >>  "map_get_city(punit->x, punit->y)"
 >>  "map_get_continent(punit->x, punit->y)"
 >>  "map_get_terrain(punit->x, punit->y)"
 >>
 >>  New syntax:
 >>
 >>  "punit->tile"
 >>  "punit->tile->city"
 >>  "punit->tile->continent"
 >>  "punit->tile->terrain"

As a minor point of note, storing the tile pointer, the linear index of
the tile, or the coordinates are all equivalent and interconvertible,
though going to or from coordinates is typically more expensive.

The key question is which form is currently used more often, and in an
ideal future scenario would this change.

Renier has long been waiting for a tile based map system where neighbour
relationships rather than geometrical relationships were the fundamental
elements. In some ways the cleanup of the core by adding iterators and
adjacent movement by APIs with indexed lookup rather than raw coordinate
operations was a step in this direction.

Trying to do away with coordinates for all lowest level operations would
be another step. This suggests that replacing punit->x, punit->y with a
tile location is part of this process.

But if there is a significant use of both coordinates and location which
cannot be tipped one way or the other, and the conversion is found to be
a performance problem, then caching both forms at generation is not a bad
solution.


Note, one could cache the coordinates in the tile, rather than the unit,
and since the tile never moves (well not yet anyway), this involves only
a trivial operation at mapgeneration time.

Moreover, this would be a fast way to do inverse coordinate generation,
i.e. lookup using the tile pointer or index one currently has for
index_to_mappos(). Eventually one might endup with a location object
attached to a tile which was used for handling all adjacency, distance
and other operations which are currently coordinate based (aka computed
functions).

[...]
 >> I prefere the new syntax because it's it's shorter to write, and I
 >> would like to use those functions only when working in coordinates
 >> like with tile iterators. The relationship between unit's (x,y) and
 >> tilepointer can be handled in one place.
 >
 >
 > I dislike it because:
 >  - it is redundant

All caching is in some sense redundant.

 >  - provide more than two ways to solve a thing

This second reason is redundant, so clearly redundancy is not something
you are religiously opposed to

 >  - showns internal data

This doesn't make a lot of sense, care to explain?

 > The last point could be solved by having functions/macros like:
 >
 >   unit_tile(punit)
 >   unit_tile_city(punit)


Whether one uses punit->tile and (void) unit_mappos(punit, &x, &y) plus
(point *) unit_mappos(punit), or conversely  punit->pos, punit->x, punit->y
and (tile *) unit_tile(punit) pretty much depends on which end of the egg
you want to open, errr ... what is data and what is derived.

One solution is to use macros (aka defined APIs) for both and hide the
implementation, whether big or little eggian, as something that is best left
to the future needs of the implementation.

One needn't commit to whether the coordinates or the tile location are
the real data or cached derivative or where in fact any of them ultimately
reside which to the user is probably never of pressing concern anyway.

 > which than use or don't use a cached tile pointer. So what about a
 > single new function/macro unit_tile(punit) and than writing
 > unit_tile(punit)->city?

The general idea of always accessing tile values through a tile pointer
like punit_get_tile(punit)->city, rather than explicit coordinates as in
map_get_tile(punit->x, punit->y)->city is the key point here.


Any object that has a location should have a location API, i.e. a method
to return its location. All such methods should have a standard naming
convention which is perhaps most easily understood if written as

    <object>_<method>(<args>)

where in a true object oriented language the leading <object>_ would be
dropped as part of the method name (because of scoping) and replaced with
the instance variable reference in code, i.e. punit->get_tile().

 >     Raimar

Cheers,
RossW
=====