Complete.Org: Mailing Lists: Archives: freeciv-dev: October 2003:
[Freeciv-Dev] (PR#6260) server bug: city tile status
Home

[Freeciv-Dev] (PR#6260) server bug: city tile status

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: paul@xxxxxxxxxxxxx
Subject: [Freeciv-Dev] (PR#6260) server bug: city tile status
From: "Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>
Date: Wed, 8 Oct 2003 06:59:52 -0700
Reply-to: rt@xxxxxxxxxxxxxx

Here's another example of the type of problems we have here.

create_city first initializes all values in the city_map to either EMPTY
or UNAVAILABLE, depending on the return value of city_can_work_tile. 
Then it updates the border information for the city, then marks the city
center as WORKER.

There is no end to the problems this simple code has because of the
current design.

- Updating the border information updates the city tile status for this
city, among others.  This should result in calling
auto_arrange_workers().  But the city tile status for this city isn't in
a sane state because the center tile is unworked.  Only by chance is
auto_arrange_workers not called in this case - update_city_tile_status
only calles it when a worked tile becomes unavailable; when an
unavailable tile becomes available it relies on the caller to do it.

- Marking the center tile as worked can't be done before the border
update, since city_can_work_tile() may not return true for this tile (if
the city is in enemy territory).

- Even initializing the city_map array makes little sense, since the
border information hasn't been updated at this point.  E.g., if we found
a city in enemy territory we'll initialize the city center to UNAVAILABLE.

In this situation, as in most similar places in the code, the problem is
that we're trying to do everything in one step.  The above code needs to
use a borders update function that will not call auto_arrange_workers
for the created city (but, in general, should call it for other cities).

update_city_tile_status should never call auto_arrange_workers.  It
should instead return a TRUE value if the caller needs to call it.  This
value may need to be propogated back upward (as in the above case) so
that it's not called until the city map is in a sane state.

One option might be to not call auto_arrange_workers at all at this
point, but rather to set the pcity->workers_need_arrangement flag.  This
flag can be checked when sync_cities is called.  This would be good,
since then auto_arrange_workers needn't be recursive.

jason



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