[Freeciv-Dev] Re: (PR#7345) auto_arrange_workers callers need to be redesigned

["Raimar Falke" <i-freeciv-lists@xxxxxxxxxxxxx>]

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

On Thu, Jan 29, 2004 at 12:21:59PM -0800, Jason Short wrote:
> 
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=7345 >
> 
> Currently if you want to refresh a worker for a city you call
> update_city_tile_status_map or update_city_tile_status.  These functions
> will call auto_arrange_workers if the change requires a recalculation of
> the CM.
> 
> In most cases this is okay: for instance if an enemy unit moves onto a
> tile, we call update_city_tile_status_map and find that it's no longer
> available, triggering a call to auto_arrange_workers.
> 
> But with the introduction of borders the shortcomings of this system are
> revealed: it only works if you're refreshing _just one_ tile.  Nor is
> this explained in the interface.  For instance if you conquer a city,
> the whole city has to be updated and many tiles may change.  The first
> tile to change will call update_city_tile_status which will call
> auto_arrange_workers to rearrange the workers.  But since the other
> tiles haven't been updated yet auto_arrange_workers is given the status
> map in an inconsistent state.  In the past this has lead to failed
> assertions or weird, bizarre bugs (like extra workers placed all over
> the citymap).
> 
> I added a hack to auto_arrange_workers to make sure the city map is in a
> consistent state: we call update_city_tile_status on every tile before
> proceeding.  This means if you call auto_arrange_workers on a city with
> 3 changes, the auto_arrange_workers will call update_city_tile_status,
> which will trigger another auto_arrange_workers, which will trigger
> another update_city_tile_status which will trigger another
> auto_arrange_workers.  This last AAW gets the map in a consistent state,
> so it runs.  Then the previous AAW gets the already-updated map and runs
> again.  Then the first AAW gets the already-updated map and runs a third
> time.
> 
> So although we've avoided problems with recursiveness, it is very
> inefficient.  And the whole thing is like a glass tower (hmm, what's the
> correct metaphore here?) ready to break as soon as anyone changes any
> part of the code.
> 
> A better design is needed.
> 
> See also PR#742, PR#3546, PR#6222, PR#6260, PR#6544, PR#7003.  These are
> basically all different symptoms of the same bug.  After PR#6544 I added
> the hack fix described above.  I don't know if PR#7003 means the bug is
> back or if this was an older version of the code.

Suggestion 1:

Change update_city_tile_status so that it accepts a list of
tiles. Change all the callers to use it if
required. update_city_tile_status would carry out the changes and call
aaw if anything has changed which require aaw.

Suggestion 2:

Create a new update_city_tile_status0 which gets the code of the
current update_city_tile_status except the calling of aaw. Change
update_city_tile_status so that it calls update_city_tile_status0 and
then aaw if required. Change the problematic caller of
update_city_tile_status to use update_city_tile_status0 and call aaw
by themselfs.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Many of my assistants were fans of Tolkien, who wrote 'Lord of the Rings'
  and a number of other children's stories for adults.  The first character
  alphabet that was programmed for my plotter was Elvish rather than Latin."
    -- from SAIs "life as a computer for a quarter of a century"