[Freeciv-Dev] (PR#7327) another occupied-city bug

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

> [i-freeciv-lists@xxxxxxxxxxxxx - Tue Jan 27 11:15:44 2004]:

> +    /*
> +     * For its own cities the client will update the occupied field
> +     * based on its own unit list at this position. For enemy cities
> +     * the client will use the occupied flag that the server sends in
> +     * the city_short_info packet.
> +     */
> 
> So the client relies entirely on the server. If there is a bug the
> server should be changed so that it sends the correct value in the
> city_short_info packet.

There is certainly no bug in the server!

Ideally there would be one boolean condition, can_player_see_into_city.
 However instead right now we have two: can_player_see_units_in_city and
can_player_see_city_internals.  All allies can see units in your city,
however only the city owner can see the internals (citizens, etc.) of
the city.

Thus we have three cases at the client:

1. Non-allied player: pcity->occupied is set by server; units aren't visible
2. Allied player: pcity->occupied is set by server; units are visible
3. Owner: pcity->occupied isn't set by server; units are visible

In case 1 we have to preserve pcity->occupied and not look at the unit
list.  In case 3 we have to look at the unit list and overwrite
pcity->occupied.  In case 2 we can do either.  But it's safest to update
based on the unit list: that way we don't have to rely on another packet
being sent from the server.

This piece of code treated all three cases by overwriting
pcity->occupied: obviously wrong.  My fix is correct but treats case #2
by overwriting pcity->occupied.

In the future I would like to formalize this by adding the above
functions.  Note that

!can_player_see_units_in_city => !can_player_see_city_internals
  or conversely
can_player_see_city_internals => can_player_see_units_in_city

or else the player would have no way to know about the units.

jason