[Freeciv-Dev] Re: (PR#4381) some clean with get_map_tile(...) , get_map_terrain(...), get_map_special(...) ...

["Per I. Mathisen" <per@xxxxxxxxxxx>]

On Mon, 9 Jun 2003, Rafa³ Bursig wrote:
> Yesterday I was look into new veteran code and I found that many times
> in code (not only in veteran code) are call get_map_terrain(...) and
> get_map_special(...) , map_has_special(...) , etc , even if there is
> struct tile declared and defined. IMHO this is resource waste when we
> calc. map tile each time when we want check terrain, special, city,
> etc, many time in one fuction call.

I like most of it (modulo some style issues), but not these:

-      map_set_special(x, y, S_FARMLAND);
+      ptile->special |= S_FARMLAND;

We call map_set_special() for a reason, and that reason is code
abstraction. This code is run very rarely (compared to other code) so
optimizing them in this way is not a good idea.

This is not an optimization:

 bool can_unit_change_homecity(struct unit *punit)
 {
   struct city *pcity=map_get_city(punit->x, punit->y);
-  return pcity && pcity->owner==punit->owner;
+  return pcity && pcity->owner==punit->owner && pcity->id!=punit->homecity;

So if you fix the style issues in code you touch and remove the above,
I'll commit the patch.

  - Per

PS Hopefully also you realize that the speed improvement from your patch
will be quite insignificant. As opposed to the speed improvement that
would be gained from making map_get_tile() into a macro.