[Freeciv-Dev] Re: (PR#6735) Server crashed upgrading a frigate
[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
<URL: http://rt.freeciv.org/Ticket/Display.html?id=6735 >
On Wed, Dec 03, 2003 at 03:56:03PM -0800, Jason Short wrote:
>
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=6735 >
>
> Jason Short wrote:
>
> > Nor by doing this do we cut down on the duplicated rules. The rules are
> > still duplicated in upgrade_unit and unit_upgrade. For instance
> > upgrade_unit does not reassign any transported units, and thus
> > unit_upgrade must check to make sure the transport capacity is large enough.
>
> We can get around this to some extend by doing the operations in the
> test function (unit_upgrade): for instance unit_upgrade can remove the
> gold for upgrading in addition to testing that the player has enough
> gold. But at some point server data will need to be accessed and it is
> not sufficient to do it in common code. So fundamentally there must be
> some duplication between common and server.
I have made a new patch (untested). The common code now doesn't
execute the actual upgrade anymore.
Raimar
--
email: rf13@xxxxxxxxxxxxxxxxx
"A common mistake that people make, when trying to design
something completely foolproof is to underestimate the
ingenuity of complete fools."
-- Douglas Adams in Mostly Harmless
Index: client/gui-gtk/citydlg.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/citydlg.c,v
retrieving revision 1.174
diff -u -u -r1.174 citydlg.c
--- client/gui-gtk/citydlg.c 2003/11/28 17:37:19 1.174
+++ client/gui-gtk/citydlg.c 2003/12/04 17:24:25
@@ -2688,14 +2688,13 @@
*****************************************************************/
static void unit_upgrade_callback(gpointer data)
{
- struct unit *punit;
+ struct unit *punit =
+ player_find_unit_by_id(game.player_ptr, (size_t) data);
char buf[512];
- int ut1, ut2;
- int value;
- if ((punit = player_find_unit_by_id(game.player_ptr, (size_t) data))) {
- ut1 = punit->type;
- ut2 = can_upgrade_unittype(game.player_ptr, ut1);
+ if (punit) {
+ Unit_Type_id ut1 = punit->type;
+ Unit_Type_id ut2 = can_upgrade_unittype(game.player_ptr, ut1);
if (ut2 == -1) {
/* this shouldn't generally happen, but it is conceivable */
@@ -2703,12 +2702,23 @@
_("Sorry: cannot upgrade %s."), unit_types[ut1].name);
popup_message_dialog(top_vbox,
_("Upgrade Unit!"), buf,
- dummy_close_callback, NULL,
+ dummy_close_callback, NULL,
_("Darn"), NULL, 0, NULL);
} else {
- value = unit_upgrade_price(game.player_ptr, ut1, ut2);
+ enum unit_upgrade_result result =
+ unit_upgrade(punit, ut2, TRUE, TRUE);
+ int value = unit_upgrade_price(game.player_ptr, ut1, ut2);
- if (game.player_ptr->economic.gold >= value) {
+ if (result == UR_NO_MONEY) {
+ my_snprintf(buf, sizeof(buf),
+ _("Upgrading %s to %s costs %d gold.\n"
+ "Treasury contains %d gold."), unit_types[ut1].name,
+ unit_types[ut2].name, value,
+ game.player_ptr->economic.gold);
+ popup_message_dialog(top_vbox, _("Upgrade Unit!"), buf,
+ dummy_close_callback, NULL,
+ _("Darn"), NULL, 0, NULL);
+ } else if (result == UR_OK) {
my_snprintf(buf, sizeof(buf), _("Upgrade %s to %s for %d gold?\n"
"Treasury contains %d gold."),
unit_types[ut1].name, unit_types[ut2].name,
@@ -2721,12 +2731,10 @@
NULL, 0, NULL);
} else {
my_snprintf(buf, sizeof(buf),
- _("Upgrading %s to %s costs %d gold.\n"
- "Treasury contains %d gold."), unit_types[ut1].name,
- unit_types[ut2].name, value,
- game.player_ptr->economic.gold);
- popup_message_dialog(top_vbox,_("Upgrade Unit!"),buf,
- dummy_close_callback, NULL,
+ _("Sorry: cannot upgrade %s."), unit_types[ut1].name);
+ popup_message_dialog(top_vbox,
+ _("Upgrade Unit!"), buf,
+ dummy_close_callback, NULL,
_("Darn"), NULL, 0, NULL);
}
}
Index: common/unit.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/unit.c,v
retrieving revision 1.190
diff -u -u -r1.190 unit.c
--- common/unit.c 2003/11/28 17:37:21 1.190
+++ common/unit.c 2003/12/04 17:24:26
@@ -1513,3 +1513,113 @@
punit->pgr = NULL;
}
}
+
+/****************************************************************************
+ Expensive function to check how many units are in the transport.
+****************************************************************************/
+int get_transporter_occupancy(struct unit *ptrans)
+{
+ int occupied = 0;
+
+ unit_list_iterate(map_get_tile(ptrans->x, ptrans->y)->units, pcargo) {
+ if (pcargo->transported_by == ptrans->id) {
+ occupied++;
+ }
+ } unit_list_iterate_end;
+
+ return occupied;
+}
+
+/***************************************************************************
+ Return TRUE if upgrading this unit could cause passengers to
+ get stranded at sea, due to transport capacity changes
+ caused by the upgrade.
+ Return FALSE if it is ok to upgrade (as far as stranding goes).
+***************************************************************************/
+static bool upgrade_would_strand(struct unit *punit,
+ Unit_Type_id upgrade_type)
+{
+ int old_cap, new_cap, tile_cap, tile_ncargo;
+
+ if (!is_sailing_unit(punit)) {
+ return FALSE;
+ }
+
+ if (!is_ocean(map_get_terrain(punit->x, punit->y))) {
+ return FALSE;
+ }
+
+ /* With weird non-standard unit types, upgrading these could
+ cause air units to run out of fuel; too bad. */
+ if (unit_flag(punit, F_CARRIER)
+ || unit_flag(punit, F_MISSILE_CARRIER)) {
+ return FALSE;
+ }
+
+ old_cap = get_transporter_capacity(punit);
+ new_cap = unit_types[upgrade_type].transport_capacity;
+
+ if (new_cap >= old_cap) {
+ return FALSE;
+ }
+
+ tile_cap = 0;
+ tile_ncargo = 0;
+ unit_list_iterate(map_get_tile(punit->x, punit->y)->units, punit2) {
+ if (punit2->owner == punit->owner
+ || pplayers_allied(unit_owner(punit2), unit_owner(punit))) {
+ if (is_sailing_unit(punit2) && is_ground_units_transport(punit2)) {
+ tile_cap += get_transporter_capacity(punit2);
+ } else if (is_ground_unit(punit2)) {
+ tile_ncargo++;
+ }
+ }
+ } unit_list_iterate_end;
+
+ if (tile_ncargo <= tile_cap - old_cap + new_cap) {
+ return FALSE;
+ }
+
+ freelog(LOG_VERBOSE, "Can't upgrade %s at (%d,%d)"
+ " because would strand passenger(s)",
+ unit_type(punit)->name, punit->x, punit->y);
+ return TRUE;
+}
+
+/***************************************************************************
+ Tests if the unit could be updated. Returns UR_OK if is this is
+ possible.
+
+ need_money: Leonardo upgrade for free, in all other cases the unit
+ owner has to pay
+ need_city: Leonardo upgrade everywhere, all other cases the unit has
+ to be inside an owned city
+
+ Note that this function is strongly tied to unittools.c:upgrade_unit().
+***************************************************************************/
+enum unit_upgrade_result unit_upgrade(struct unit *punit,
+ Unit_Type_id to_unittype,
+ bool need_money, bool need_city)
+{
+ struct player *pplayer = unit_owner(punit);
+ struct city *pcity = map_get_city(punit->x, punit->y);
+ const int cost = unit_upgrade_price(pplayer, punit->type, to_unittype);
+
+ if (need_money && pplayer->economic.gold < cost) {
+ return UR_NO_MONEY;
+ }
+
+ if (need_city && !pcity) {
+ return UR_NOT_IN_CITY;
+ }
+ if (need_city && pcity && city_owner(pcity) != pplayer) {
+ return UR_NOT_CITY_OWNER;
+ }
+ if (get_transporter_occupancy(punit) >
+ unit_types[to_unittype].transport_capacity
+ || upgrade_would_strand(punit, to_unittype)) {
+ return UR_NOT_ENOUGH_ROOM;
+ }
+
+ return UR_OK;
+}
Index: common/unit.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/unit.h,v
retrieving revision 1.105
diff -u -u -r1.105 unit.h
--- common/unit.h 2003/12/01 18:14:22 1.105
+++ common/unit.h 2003/12/04 17:24:26
@@ -92,6 +92,14 @@
sewer but city has no sewer */
};
+enum unit_upgrade_result {
+ UR_OK,
+ UR_NO_MONEY,
+ UR_NOT_IN_CITY,
+ UR_NOT_CITY_OWNER,
+ UR_NOT_ENOUGH_ROOM
+};
+
struct unit_ai {
bool control; /* 0: not automated 1: automated */
enum ai_unit_task ai_role;
@@ -293,5 +301,11 @@
Unit_Type_id type, bool make_veteran);
void destroy_unit_virtual(struct unit *punit);
void free_unit_goto_route(struct unit *punit);
+
+int get_transporter_occupancy(struct unit *ptrans);
+
+enum unit_upgrade_result unit_upgrade(struct unit *punit,
+ Unit_Type_id to_unittype,
+ bool need_money, bool need_city);
#endif /* FC__UNIT_H */
Index: server/unithand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unithand.c,v
retrieving revision 1.279
diff -u -u -r1.279 unithand.c
--- server/unithand.c 2003/11/28 17:37:22 1.279
+++ server/unithand.c 2003/12/04 17:24:29
@@ -153,49 +153,45 @@
const Unit_Type_id from_unittype = type;
const Unit_Type_id to_unittype = can_upgrade_unittype(pplayer,
from_unittype);
+ int number_of_upgraded_units = 0;
if (to_unittype == -1) {
notify_player(pplayer,
_("Game: Illegal packet, can't upgrade %s (yet)."),
- unit_types[from_unittype].name);
- } else {
- const int cost = unit_upgrade_price(pplayer, from_unittype, to_unittype);
- int number_of_upgraded_units = 0;
-
- if (pplayer->economic.gold >= cost) {
- const int player_no = pplayer->player_no;
-
- /* Try to upgrade units. The order we upgrade in is arbitrary (if
- * the player really cared they should have done it manually). */
- conn_list_do_buffer(&pplayer->connections);
- unit_list_iterate(pplayer->units, punit) {
- if (punit->type == from_unittype) {
- const struct city * const pcity = map_get_city(punit->x, punit->y);
+ unit_types[from_unittype].name);
+ return;
+ }
- /* Only units in cities can be upgraded. */
- if (pcity && pcity->owner == player_no) {
- upgrade_unit(punit, to_unittype);
- number_of_upgraded_units++;
- if ((pplayer->economic.gold -= cost) < cost) {
- /* We can't upgrade any more units. */
- break;
- }
- }
- }
- } unit_list_iterate_end;
- conn_list_do_unbuffer(&pplayer->connections);
+ /*
+ * Try to upgrade units. The order we upgrade in is arbitrary (if
+ * the player really cared they should have done it manually).
+ */
+ conn_list_do_buffer(&pplayer->connections);
+ unit_list_iterate(pplayer->units, punit) {
+ if (punit->type == from_unittype) {
+ enum unit_upgrade_result result =
+ unit_upgrade(punit, to_unittype, TRUE, TRUE);
+ if (result == UR_OK) {
+ number_of_upgraded_units++;
+ upgrade_unit(punit, to_unittype, TRUE);
+ }
+ if (result == UR_NO_MONEY) {
+ break;
+ }
}
+ } unit_list_iterate_end;
+ conn_list_do_unbuffer(&pplayer->connections);
- /* Alert the player about what happened. */
- if (number_of_upgraded_units > 0) {
- notify_player(pplayer, _("Game: %d %s upgraded to %s for %d gold."),
- number_of_upgraded_units, unit_types[from_unittype].name,
- unit_types[to_unittype].name,
- cost * number_of_upgraded_units);
- send_player_info(pplayer, pplayer);
- } else {
- notify_player(pplayer, _("Game: No units could be upgraded."));
- }
+ /* Alert the player about what happened. */
+ if (number_of_upgraded_units > 0) {
+ const int cost = unit_upgrade_price(pplayer, from_unittype, to_unittype);
+ notify_player(pplayer, _("Game: %d %s upgraded to %s for %d gold."),
+ number_of_upgraded_units, unit_types[from_unittype].name,
+ unit_types[to_unittype].name,
+ cost * number_of_upgraded_units);
+ send_player_info(pplayer, pplayer);
+ } else {
+ notify_player(pplayer, _("Game: No units could be upgraded."));
}
}
@@ -225,8 +221,7 @@
cost);
return;
}
- pplayer->economic.gold -= cost;
- upgrade_unit(punit, to_unit);
+ upgrade_unit(punit, to_unit, TRUE);
send_player_info(pplayer, pplayer);
notify_player(pplayer, _("Game: %s upgraded to %s for %d gold."),
unit_name(from_unit), unit_name(to_unit), cost);
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.266
diff -u -u -r1.266 unittools.c
--- server/unittools.c 2003/12/01 18:14:22 1.266
+++ server/unittools.c 2003/12/04 17:24:30
@@ -59,7 +59,6 @@
static void unit_restore_movepoints(struct player *pplayer, struct unit
*punit);
static void update_unit_activity(struct unit *punit);
static void wakeup_neighbor_sentries(struct unit *punit);
-static bool upgrade_would_strand(struct unit *punit, int upgrade_type);
static void handle_leonardo(struct player *pplayer);
static void sentry_transported_idle_units(struct unit *ptrans);
@@ -158,63 +157,11 @@
}
/***************************************************************************
- Return 1 if upgrading this unit could cause passengers to
- get stranded at sea, due to transport capacity changes
- caused by the upgrade.
- Return 0 if ok to upgrade (as far as stranding goes).
-***************************************************************************/
-static bool upgrade_would_strand(struct unit *punit, int upgrade_type)
-{
- int old_cap, new_cap, tile_cap, tile_ncargo;
-
- if (!is_sailing_unit(punit))
- return FALSE;
- if (!is_ocean(map_get_terrain(punit->x, punit->y))) {
- return FALSE;
- }
-
- /* With weird non-standard unit types, upgrading these could
- cause air units to run out of fuel; too bad. */
- if (unit_flag(punit, F_CARRIER)
- || unit_flag(punit, F_MISSILE_CARRIER))
- return FALSE;
-
- old_cap = get_transporter_capacity(punit);
- new_cap = unit_types[upgrade_type].transport_capacity;
-
- if (new_cap >= old_cap)
- return FALSE;
-
- tile_cap = 0;
- tile_ncargo = 0;
- unit_list_iterate(map_get_tile(punit->x, punit->y)->units, punit2) {
- if (punit2->owner == punit->owner
- || pplayers_allied(unit_owner(punit2), unit_owner(punit))) {
- if (is_sailing_unit(punit2) && is_ground_units_transport(punit2)) {
- tile_cap += get_transporter_capacity(punit2);
- } else if (is_ground_unit(punit2)) {
- tile_ncargo++;
- }
- }
- }
- unit_list_iterate_end;
-
- if (tile_ncargo <= tile_cap - old_cap + new_cap)
- return FALSE;
-
- freelog(LOG_VERBOSE, "Can't upgrade %s at (%d,%d)"
- " because would strand passenger(s)",
- unit_type(punit)->name, punit->x, punit->y);
- return TRUE;
-}
-
-/***************************************************************************
Do Leonardo's Workshop upgrade(s). Select unit to upgrade by random. --Zamar
Now be careful not to strand units at sea with the Workshop. --dwp
****************************************************************************/
static void handle_leonardo(struct player *pplayer)
{
- int upgrade_type;
int leonardo_variant;
struct unit_list candidates;
@@ -227,9 +174,11 @@
unit_list_init(&candidates);
unit_list_iterate(pplayer->units, punit) {
- if ((upgrade_type=can_upgrade_unittype(pplayer, punit->type))!=-1 &&
- !upgrade_would_strand(punit, upgrade_type))
- unit_list_insert(&candidates, punit); /* Potential candidate :) */
+ Unit_Type_id upgrade_type = can_upgrade_unittype(pplayer, punit->type);
+ if (upgrade_type != -1
+ && unit_upgrade(punit, upgrade_type, FALSE, FALSE) == UR_OK) {
+ unit_list_insert(&candidates, punit); /* Potential candidate :) */
+ }
} unit_list_iterate_end;
if (unit_list_size(&candidates) == 0)
@@ -241,7 +190,9 @@
i=0;
unit_list_iterate(candidates, punit) {
if (leonardo_variant != 0 || i == candidate_to_upgrade) {
- upgrade_type=can_upgrade_unittype(pplayer, punit->type);
+ enum unit_upgrade_result result;
+ Unit_Type_id upgrade_type = can_upgrade_unittype(pplayer, punit->type);
+
notify_player(pplayer,
_("Game: %s has upgraded %s to %s%s."),
improvement_types[B_LEONARDO].name,
@@ -249,7 +200,9 @@
get_unit_type(upgrade_type)->name,
get_location_str_in(pplayer, punit->x, punit->y));
punit->veteran = FALSE;
- upgrade_unit(punit, upgrade_type);
+ result = unit_upgrade(punit, upgrade_type, FALSE, FALSE);
+ assert(result == UR_OK);
+ upgrade_unit(punit, upgrade_type, FALSE);
}
i++;
} unit_list_iterate_end;
@@ -1418,30 +1371,29 @@
}
}
-/****************************************************************************
- Expensive function to check how many units are in the transport.
-****************************************************************************/
-int get_transporter_occupancy(struct unit *ptrans)
-{
- int occupied = 0;
+/**************************************************************************
+ Really upgrades a single unit.
- unit_list_iterate(map_get_tile(ptrans->x, ptrans->y)->units, pcargo) {
- if (pcargo->transported_by == ptrans->id) {
- occupied++;
- }
- } unit_list_iterate_end;
+ Before calling this function you should use unit_upgrade to test if
+ this is possible.
- return occupied;
-}
+ has_to_pay: Leonardo upgrade for free, in all other cases the unit
+ owner has to pay
-/**************************************************************************
-...
+ Note that this function is strongly tied to unit.c:unit_upgrade().
**************************************************************************/
-void upgrade_unit(struct unit *punit, Unit_Type_id to_unit)
+void upgrade_unit(struct unit *punit, Unit_Type_id to_unit, bool has_to_pay)
{
struct player *pplayer = unit_owner(punit);
int range;
int old_mr = unit_move_rate(punit), old_hp = unit_type(punit)->hp;
+
+ assert(unit_upgrade(punit, to_unit, has_to_pay, FALSE) == UR_OK);
+
+ if (has_to_pay) {
+ pplayer->economic.gold -=
+ unit_upgrade_price(pplayer, punit->type, to_unit);
+ }
/* save old vision range */
if (map_has_special(punit->x, punit->y, S_FORTRESS)
Index: server/unittools.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.h,v
retrieving revision 1.58
diff -u -u -r1.58 unittools.h
--- server/unittools.h 2003/11/28 17:37:22 1.58
+++ server/unittools.h 2003/12/04 17:24:31
@@ -29,7 +29,6 @@
/* move check related */
bool is_airunit_refuel_point(int x, int y, struct player *pplayer,
Unit_Type_id type, bool unit_is_on_tile);
-int get_transporter_occupancy(struct unit *ptrans);
/* turn update related */
void player_restore_units(struct player *pplayer);
@@ -51,7 +50,7 @@
void bounce_unit(struct unit *punit, bool verbose);
/* creation/deletion/upgrading */
-void upgrade_unit(struct unit *punit, Unit_Type_id to_unit);
+void upgrade_unit(struct unit *punit, Unit_Type_id to_unit, bool has_to_pay);
struct unit *create_unit(struct player *pplayer, int x, int y, Unit_Type_id
type,
bool make_veteran, int homecity_id, int moves_left);
struct unit *create_unit_full(struct player *pplayer, int x, int y,
|
|
