Re: [Freeciv-Dev] PATCH: Fixing problems with traderoutes when a city c
[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
Rizos Sakellariou wrote:
>
> The following patch fixes several problems with the traderoutes of
> a city that changes owner. At the moment such a change is handled in
> a way that leads to erroneous situations where other cities maintaining
> a traderoute with this city may keep a traderoute with a non-existent
> city or none at all (note that, in the change of conquest, the
> conquered city will change its id number, but this is not updated in
> the fields of cities having traderoutes with it), whereas the city
> that changes owner will have its traderoutes reestablished properly.
> In some other cases, the current code performs better than one
> might expect because of the obvious mistake in remove_city():
> for (o=0; o<4; o++)
> remove_trade_route(pcity->trade[0], pcity->id);
Thanks for your patch Rizos.
I got a few rejects when trying to apply your patch, probably
due to whitespace problems (sending patches as MIME or
uuencoded may help).
Attached is a revised version of your patch; as well as fixing
the rejects and re-diffing, I've made some minor changes, and
fixed the civil war (and maybe other?) cases. In particular,
the traderoute stuff in transfer_city(), which was similar to
the code you fixed in handle_unit_enter_city().
(Actually some of this code is cut-and-paste-ish, and should
probably be reorganised.)
I also put an extra coment at the start of transfer_city(),
and fixed some related cases. And removed some now unused vars.
I left off the "if" where before:
+ if (city_num_trade_routes(pnewcity))
+ reestablish_city_trade_routes(pnewcity);
I figure just call the function and let _it_ work it out.
> PS: Not sure how to handle the traderoutes in the case of cities
> revolted in the civil war case... I'd suggest, in this case,
> to remove all traderoutes of revolted cities... Any opinions?
I think it should now work such that all traderoutes are kept
where possible.
One curious case is if you make a traderoute to a very close
enemy city. Then if you then capture that city, it may be too
close to your city, so that can_establish_traderoute() fails
when trying to re-do the route. Actually, maybe this works out
ok, just with the traderoute gone.
Regards,
-- David "but this isn't really tested"
diff -u -r --exclude-from exclude freeciv-cvs/server/cityhand.c
freeciv-mod/server/cityhand.c
--- freeciv-cvs/server/cityhand.c Thu Feb 11 18:05:46 1999
+++ freeciv-mod/server/cityhand.c Wed Feb 24 21:57:49 1999
@@ -675,7 +675,7 @@
get_race_name_plural(game.players[pcity->owner].race),
pcity->name,pcity->x,pcity->y);
for (o=0; o<4; o++)
- remove_trade_route(pcity->trade[0], pcity->id);
+ remove_trade_route(pcity->trade[o], pcity->id);
packet.value=pcity->id;
for(o=0; o<game.nplayers; o++) /* dests */
send_packet_generic_integer(game.players[o].conn,
@@ -693,7 +693,8 @@
/**************************************************************************
-...
+The following has to be called every time a city, pcity, has changed
+owner to update the tile->worked pointer.
**************************************************************************/
void update_map_with_city_workers(struct city *pcity)
@@ -703,4 +704,25 @@
if (pcity->city_map[x][y] == C_TILE_WORKER)
set_worker_city(pcity, x, y, C_TILE_WORKER);
}
+}
+
+/**************************************************************************
+The following has to be called every time a city, pcity, has changed
+owner to update the city's traderoutes.
+**************************************************************************/
+
+void reestablish_city_trade_routes(struct city *pcity)
+{
+ int i;
+ struct city *oldtradecity;
+
+ for (i=0;i<4;i++) {
+ if (pcity->trade[i]) {
+ oldtradecity=find_city_by_id(pcity->trade[i]);
+ pcity->trade[i]=0;
+ if (can_establish_trade_route(pcity, oldtradecity)) {
+ establish_trade_route(pcity, oldtradecity);
+ }
+ }
+ }
}
diff -u -r --exclude-from exclude freeciv-cvs/server/cityhand.h
freeciv-mod/server/cityhand.h
--- freeciv-cvs/server/cityhand.h Thu Feb 11 18:05:46 1999
+++ freeciv-mod/server/cityhand.h Wed Feb 24 21:46:57 1999
@@ -45,7 +45,7 @@
void remove_trade_route(int c1, int c2);
void send_player_cities(struct player *pplayer);
void update_map_with_city_workers(struct city *pcity);
-
+void reestablish_city_trade_routes(struct city *pcity);
void handle_city_options(struct player *pplayer,
struct packet_generic_values *preq);
diff -u -r --exclude-from exclude freeciv-cvs/server/citytools.c
freeciv-mod/server/citytools.c
--- freeciv-cvs/server/citytools.c Wed Feb 24 20:20:31 1999
+++ freeciv-mod/server/citytools.c Wed Feb 24 22:04:31 1999
@@ -851,6 +851,7 @@
remove_city(pcity); /* don't forget this! */
map_set_city(pnewcity->x, pnewcity->y, pnewcity);
+ reestablish_city_trade_routes(pnewcity);
city_check_workers(cplayer,pnewcity);
city_refresh(pnewcity);
initialize_infrastructure_cache(pnewcity);
@@ -881,12 +882,21 @@
* be used to trade cities in the diplomatic section too.
*
* - Kris Bubendorfer
+ *
+ * Note that the old city is not yet destroyed, which means we
+ * can't yet transfer (re-establish) the trade routes for
+ * the new city. (Though the new city struct has the same
+ * traderoute data as the old city.) Thus you should call
+ * reestablish_trade_routes(p_new_city);
+ * sometime _after_ calling
+ * remove_city(p_old_city);
+ * after this function. --dwp
+ *
*/
-
struct city *transfer_city(struct player *pplayer, struct player *cplayer,
struct city *pcity)
{
- struct city *pnewcity, *pc2;
+ struct city *pnewcity;
int i;
if (!pcity)
@@ -901,14 +911,9 @@
pnewcity=(struct city *)malloc(sizeof(struct city));
*pnewcity=*pcity;
- for (i=0;i<4;i++) {
- pc2=find_city_by_id(pnewcity->trade[i]);
- if (can_establish_trade_route(pnewcity, pc2))
- establish_trade_route(pnewcity, pc2);
- }
-
pnewcity->id=get_next_id_number();
add_city_to_cache(pnewcity);
+
for (i = 0; i < B_LAST; i++) {
if (is_wonder(i) && city_got_building(pnewcity, i))
game.global_wonders[i] = pnewcity->id;
diff -u -r --exclude-from exclude freeciv-cvs/server/diplhand.c
freeciv-mod/server/diplhand.c
--- freeciv-cvs/server/diplhand.c Thu Feb 11 18:05:50 1999
+++ freeciv-mod/server/diplhand.c Wed Feb 24 21:58:03 1999
@@ -192,6 +192,7 @@
remove_city(pcity); /* don't forget this! */
map_set_city(pnewcity->x, pnewcity->y, pnewcity);
+ reestablish_city_trade_routes(pnewcity);
city_check_workers(pdest ,pnewcity);
update_map_with_city_workers(pnewcity);
city_refresh(pnewcity);
diff -u -r --exclude-from exclude freeciv-cvs/server/unitfunc.c
freeciv-mod/server/unitfunc.c
--- freeciv-cvs/server/unitfunc.c Sun Feb 21 13:53:31 1999
+++ freeciv-mod/server/unitfunc.c Wed Feb 24 21:59:08 1999
@@ -496,6 +496,8 @@
send_tile_info(0, pnewcity->x, pnewcity->y, TILE_KNOWN);
}
+ reestablish_city_trade_routes(pnewcity);
+
city_check_workers(pplayer,pnewcity);
update_map_with_city_workers(pnewcity);
city_refresh(pnewcity);
diff -u -r --exclude-from exclude freeciv-cvs/server/unithand.c
freeciv-mod/server/unithand.c
--- freeciv-cvs/server/unithand.c Wed Feb 24 20:17:28 1999
+++ freeciv-mod/server/unithand.c Wed Feb 24 22:00:45 1999
@@ -985,7 +985,6 @@
{
int i, x, y, old_id;
int coins;
- struct city *pc2;
struct player *cplayer;
if(pplayer->player_no!=pcity->owner) {
struct city *pnewcity;
@@ -1063,16 +1062,12 @@
make_partisans(pcity);
*pnewcity=*pcity;
remove_city(pcity);
- for (i=0;i<4;i++) {
- pc2=find_city_by_id(pnewcity->trade[i]);
- if (can_establish_trade_route(pnewcity, pc2))
- establish_trade_route(pnewcity, pc2);
- }
+
/* now set things up for the new owner */
old_id = pnewcity->id;
-
pnewcity->id=get_next_id_number();
+
add_city_to_cache(pnewcity);
for (i = 0; i < B_LAST; i++) {
if (is_wonder(i) && city_got_building(pnewcity, i))
@@ -1104,6 +1099,8 @@
}
}
}
+
+ reestablish_city_trade_routes(pnewcity);
/* relocate workers of tiles occupied by enemy units */
city_check_workers(pplayer,pnewcity);
|
|
