[Freeciv-Dev] Re: [Patch] Reduce superfluous tile_info packets version 2

[Jason Dorje Short <vze2zq63@xxxxxxxxxxx>]

Jason Dorje Short wrote:
> 
> Raimar Falke wrote:
> >
> > Ok here the cleaned up patch for official submission. This patch does:
> >  - removes the sent field of struct tile (AFAI see this was intended
> >  to solve a similar problem and it now obsolete)
> >  - adds a new server-only bit vector in struct tile: changed
> >  - if the server tries to sent a tile_info packet it is checked if
> >  this tile has changed since last sent for this player
> >  - if ther server has sent a tile_info packet the corresponding bit in
> >  the changed field is cleared
> >  - settler.c changes the special field calculates some stuff and
> >  reverts these changes. Such changes aren't user/client visible and
> >  have from now on to be done with the base_* methods.
> 
> Everything looks OK.  However:
> 
> - In savegame.c when the terrain is loaded from the savegame, an initial
> glance would make one think that map_set_terrain should be used here instead
> of manually setting the terrain and marking the tile as changed.  Of course,
> this would not work - a comment to this effect would be nice IMO.
> 
> - It is difficult to verify that you've picked up every place that changes
> the tile information.  Thus although the patch itself looks good, it is
> difficult to verify that it is complete/correct.  I'll take a deeper look at
> this later today.

What about the following code in unit_enter_hut() in unittools.c?

  map_get_tile(punit->x, punit->y)->special^=S_HUT;
  send_tile_info(0, punit->x, punit->y);

Although a unit entering a hut seems to be drawn properly, I do not think
the above code is technially coorect.  Seems to me it should be

  map_clear_special(punit->x, punit->y, S_HUT);

In fact I'd say any code that doesn't use the map functions to set tile
information should have an explicit comment as to why.

jason