[Freeciv-Dev] Re: (PR#10317) Tile marked as worked but occupied by an enemy unit!

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

Jason Short wrote:
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=10317 >
> 
> Preliminary analysis:
> 
> Player 4's unit invades Player 8's city 155.  This causes a civil war. 
> The new player is player 16.  Player 8's city 1225 is transferred to
> player 16.  auto_arrange_workers is called when the borders are updated,
> because a border change makes one of the workers invalid.  However the
> units inside the city aren't transferred until a couple of lines later.
>  Hence the error.
> 
> My fix is to move the border recalculation down a bit.  It is done after
> the transfer of units (which is what's causing this bug).  It is also
> done after the resolving of stacks (which I guess will bounce any
> previously-allied units that are inside the city).
> 
> However I'm confused why this bug doesn't happen all the time, every
> time there is a civil war.  Maybe the border recalculation only
> occasionally causes a rearrangement of workers.  Or maybe cities only
> occasionally contain units...
> 
> It's also possible this isn't a bug.  However I'm not going to touch
> that question.  auto_arrange_workers is ugly enough as it is.

But the problem is worse.  We can't exchange these calls because 
transfer_city_units also causes auto_arrange_workers to be called.  So 
it causes an even worse error because no tiles (even the city center) 
are eligible for workers.

Did I mention auto_arrange_workers is ugly?  What I meant was its 
callers are ugly.  The problem is we call it left and right every time 
anything changes.  But here 2 things are changing, and the city doesn't 
reenter a sane state until both have been changed.  In fact 
transfer_city already calls auto_arrange_workers at the end.  But it 
will get called earlier "automatically" in most or all cases.

One hack fix would be to add a new field pcity->freeze_workers.  If this 
is set then no auto_arrange_workers will be done for that city.  So 
transfer_city could set this for the city and then clear it at the end, 
right before calling aaw for itself.  Note this field must be per-city 
since other cities may still need to be rearranged during this time (as 
a result of changing border information or whatever).  We can only hope 
that other cities won't be in an inconsistent state when this happens!

jason