[Freeciv-Dev] Re: (PR#9864) Terrain under cities changes during save+load

["Marko Lindqvist" <marko.lindqvist@xxxxxxxxxxx>]

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

Jason Short wrote:
> How come update_tile_knowledge does the send() but 
> update_player_tile_knowledge does not?  Seems like it would be better to 
> do the send direcly from within update_player_tile_knowledge().

  update_player_tile_knowledge() is low level function to adjust tile 
knowledge structure. It has other users in addition to 
update_tile_knowledge(). Hmm... maybe function names need some further 
adjusting to make clear that update_player_tile_knowledge() is much 
lower level function than update_tile_knowledge() ?
update_player_tile_last_seen() is same level as 
update_player_tile_knowledge() so if some function name is changed, it 
should be update_tile_knowledge().


   Of all
> the places where you changed send_tile_info(NULL, x, y) to 
> update_tile_knowledge(x, y)...there are none where you change any other 
> callers.

  Now I'm not sure which way english grammar goes... but I try to answer 
so that you should get answer to what ever your real question is :-)

  send_tile_info() used to be only function to update tile knowledge and 
to send it -> it was used in all places where update_tile_knowledge() 
should be called.
  Note that not _all_ send_tile_info() calls are changed into 
update_tile_knowledge(), but only those where send_tile_info() was used 
because terrain possibly changed. Border changes for example still use 
send_tile_info() directly.
  update_tile_knowledge() even refuses to send tile info if tile 
knowledge for player is not really changed. We could change this so that 
  (with small cost of iterating player connections) we leave it to delta 
code to determine if anything has changed. update_tile_knowledge() would 
send information which is _supposed_ to be already sent or 
update_tile_knowledge() is erroneously used where send_tile_info() 
should be used. This would probably be a bit better for release code 
(tile gets send even if only something else than knowledge has changed) 
but development wise it would hide bugs. #ifdef DEBUG ??

   Shouldn't send_tile_info_always users be fixed?  Or will it
> take more work to fix the "always" hack in there?

  There is more differences between send_tile_info() and 
send_tile_info_always() than names indicate.
  For one thing, send_tile_info_always() users modify tile knowledge 
themselves. Second: callers rely on send_tile_info_always() leaving 
wrong knowledge to players. We don't want to send correct tile info 
about fogged tiles to reconnecting client, but players current knowledge.
  But now I looked send_tile_info_always() _itself_ more closely I see 
that it _too_ (in addition to callers) tries to modify tile knowledge. 
This should be removed (and I have to check that no caller actually rely 
on this behavior). I would also point out that it modifies knowledge of 
source player, not of player to whom info is sent (Could this affect 
shared vision? I haven't looked that deep yet)


  - Caz