Complete.Org: Mailing Lists: Archives: freeciv-dev: January 2004:
[Freeciv-Dev] (PR#7345) auto_arrange_workers callers need to be redesign
Home

[Freeciv-Dev] (PR#7345) auto_arrange_workers callers need to be redesign

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: undisclosed-recipients: ;
Subject: [Freeciv-Dev] (PR#7345) auto_arrange_workers callers need to be redesigned
From: "Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>
Date: Thu, 29 Jan 2004 12:21:59 -0800
Reply-to: rt@xxxxxxxxxxx

<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.

jason



[Prev in Thread] Current Thread [Next in Thread]